1
0
mirror of https://github.com/golang/go synced 2024-11-14 08:00:22 -07:00
Commit Graph

38675 Commits

Author SHA1 Message Date
Austin Clements
5cf2b4c2d3 cmd/compile: fix race on initializing Sym symFunc flag
SSA lowering can create PFUNC ONAME nodes when compiling method calls.
Since we generally initialize the node's Sym to a func when we set its
class to PFUNC, we did this here, too. Unfortunately, since SSA
compilation is concurrent, this can cause a race if two function
compilations try to initialize the same symbol.

Luckily, we don't need to do this at all, since we're actually just
wrapping an ONAME node around an existing Sym that's already marked as
a function symbol.

Fixes the linux-amd64-racecompile builder, which was broken by CL
147158.

Updates #27539.

Change-Id: I8ddfce6e66a08ce53998c5bfa6f5a423c1ffc1eb
Reviewed-on: https://go-review.googlesource.com/c/149158
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
2018-11-12 22:38:02 +00:00
Austin Clements
b52db19b98 cmd/compile: create "init" symbol earlier
We create the "init" symbol and mark it as a function before compiling
to SSA because SSA can initialize this symbol, but it turns out we do
it slightly too late. peekitabs, at least, can also create the "init"
LSym. Move this initialization to just after type-checking.

Fixes the linux-amd64-ssacheck and the android-arm64-wiko-fever
builders.

Updates #27539.

Change-Id: If145952c79d39f75c93b24e35e67fe026dd08329
Reviewed-on: https://go-review.googlesource.com/c/149137
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
2018-11-12 22:25:35 +00:00
Austin Clements
891f99eb43 cmd/compile: fix TestFormats
This fixes the linux-amd64-longtest builder, which was broken by CL
147160.

Updates #27539.

Change-Id: If6e69581ef503bba2449ec9bacaa31f34f59beb1
Reviewed-on: https://go-review.googlesource.com/c/149157
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-11-12 21:54:58 +00:00
Austin Clements
685aca45dc cmd/compile, cmd/link: separate stable and internal ABIs
This implements compiler and linker support for separating the
function calling ABI into two ABIs: a stable and an internal ABI. At
the moment, the two ABIs are identical, but we'll be able to evolve
the internal ABI without breaking existing assembly code that depends
on the stable ABI for calling to and from Go.

The Go compiler generates internal ABI symbols for all Go functions.
It uses the symabis information produced by the assembler to create
ABI wrappers whenever it encounters a body-less Go function that's
defined in assembly or a Go function that's referenced from assembly.

Since the two ABIs are currently identical, for the moment this is
implemented using "ABI alias" symbols, which are just forwarding
references to the native ABI symbol for a function. This way there's
no actual code involved in the ABI wrapper, which is good because
we're not deriving any benefit from it right now. Once the ABIs
diverge, we can eliminate ABI aliases.

The linker represents these different ABIs internally as different
versions of the same symbol. This way, the linker keeps us honest,
since every symbol definition and reference also specifies its
version. The linker is responsible for resolving ABI aliases.

Fixes #27539.

Change-Id: I197c52ec9f8fc435db8f7a4259029b20f6d65e95
Reviewed-on: https://go-review.googlesource.com/c/147160
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
2018-11-12 20:46:55 +00:00
Austin Clements
1794ee6829 cmd/link: nice error message on ABI mismatch
Currently, if a symbol is only defined under one ABI and referenced
under another ABI, you simply get a "relocation target X not defined".
This is confusing because it seems like the symbol is defined.

This CL enhances the error message in this case to be "relocation
target X not defined for <ABI> (but is defined for <ABI>)".

For #27539.

Change-Id: If857a1882c3fe9af5346797d5295ca1fe50ae565
Reviewed-on: https://go-review.googlesource.com/c/147159
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
2018-11-12 20:46:52 +00:00
Austin Clements
16e6cd9a4d cmd/compile: mark function Syms
In order to mark the obj.LSyms produced by the compiler with the
correct ABI, we need to know which types.Syms refer to function
symbols. This CL adds a flag to types.Syms to mark symbols for
functions, and sets this flag everywhere we create a PFUNC-class node,
and in the one place where we directly create function symbols without
always wrapping them in a PFUNC node (methodSym).

We'll use this information to construct obj.LSyms with correct ABI
information.

For #27539.

Change-Id: Ie3ac8bf3da013e449e78f6ca85546a055f275463
Reviewed-on: https://go-review.googlesource.com/c/147158
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
2018-11-12 20:46:50 +00:00
Austin Clements
c5718b6b26 cmd/internal/obj, cmd/link: record ABIs and aliases in Go obj files
This repurposes the "version" field of a symbol reference in the Go
object file format to be an ABI field. Currently, this is just 0 or 1
depending on whether the symbol is static (the linker turns it into a
different internal version number), so it's already only tenuously a
symbol version. We change this to be -1 for static symbols and
otherwise by the ABI number.

This also adds a separate list of ABI alias symbols to be recorded in
the object file. The ABI aliases must be a separate list and not just
part of the symbol definitions because it's possible to have a symbol
defined in one package and the alias "defined" in a different package.
For example, this can happen if a symbol is defined in assembly in one
package and stubbed in a different package. The stub triggers the
generation of the ABI alias, but in a different package from the
definition.

For #27539.

Change-Id: I015c9fe54690c027de6ef77e22b5585976a01587
Reviewed-on: https://go-review.googlesource.com/c/147157
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
2018-11-12 20:46:48 +00:00
Austin Clements
07544c7e80 cmd/go, cmd/dist: collect known cross-package uses of runtime symbols
This extends cmd/go's symabis support to collect known cross-package
uses of runtime symbols from other "basically runtime" packages in
std. This avoids having to declare a large number of ABI0 symbols in
the runtime for a small number of known cross-package references.

For cmd/dist, we use a simpler but less efficient approach and tell
the compiler to generate ABI wrappers for everything.

Change-Id: Ifaed94efdcff42e7345ab11b4d2fb880fb1a24e8
Reviewed-on: https://go-review.googlesource.com/c/147257
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2018-11-12 20:46:43 +00:00
Austin Clements
0f5dfbcfd7 cmd/go, cmd/dist: plumb symabis from assembler to compiler
For #27539.

Change-Id: I0e27f142224e820205fb0e65ad03be7eba93da14
Reviewed-on: https://go-review.googlesource.com/c/146999
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2018-11-12 20:46:41 +00:00
Austin Clements
7f1dd3ae4d test: minor simplification to run.go
This is a little clearer, and we're about to need the .s file list in
one more place, so this will cut down on duplication.

Change-Id: I4da8bf03a0469fb97565b0841c40d505657b574e
Reviewed-on: https://go-review.googlesource.com/c/146998
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-11-12 20:46:39 +00:00
Austin Clements
97e4010fd4 cmd/compile: accept and parse symabis
This doesn't yet do anything with this information.

For #27539.

Change-Id: Ia12c905812aa1ed425eedd6ab2f55ec75d81c0ce
Reviewed-on: https://go-review.googlesource.com/c/147099
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2018-11-12 20:46:37 +00:00
Austin Clements
ba2e8a629b cmd/asm: add mode to collect symbol ABIs
This adds a -symabis flag that runs the assembler in a special mode
that outputs symbol definition and reference ABIs rather than
assembling the code. This uses a fast and somewhat lax parser because
the go_asm.h definitions may not be available.

For #27539.

Change-Id: I248ba0ebab7cc75dcb2a90e82a82eb445da7e88e
Reviewed-on: https://go-review.googlesource.com/c/147098
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2018-11-12 20:46:33 +00:00
Austin Clements
52b2220559 cmd/asm: factor out line parsing from assembling
Currently cmd/asm's Parser.line both consumes a line of assembly from
the lexer and assembles it. This CL separates these two steps so that
the line parser can be reused for purposes other than generating a
Prog stream.

For #27539.
Updates #17544.

Change-Id: I452c9a2112fbcc1c94bf909efc0d1fcc71014812
Reviewed-on: https://go-review.googlesource.com/c/147097
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2018-11-12 20:46:25 +00:00
Filippo Valsorda
30cc978085 crypto/tls: enable TLS 1.3 and update tests
To disable TLS 1.3, simply remove VersionTLS13 from supportedVersions,
as tested by TestEscapeRoute, and amend documentation. To make it
opt-in, revert the change to (*Config).supportedVersions from this CL.

I did not have the heart to implement the early data skipping feature
when I realized that it did not offer a choice between two
abstraction-breaking options, but demanded them both (look for handshake
type in case of HelloRetryRequest, trial decryption otherwise). It's a
lot of complexity for an apparently small gain, but if anyone has strong
opinions about it let me know.

Note that in TLS 1.3 alerts are encrypted, so the close_notify peeking
to return (n > 0, io.EOF) from Read doesn't work. If we are lucky, those
servers that unexpectedly close connections after serving a single
request will have stopped (maybe thanks to H/2) before they got updated
to TLS 1.3.

Relatedly, session tickets are now provisioned on the client first Read
instead of at Handshake time, because they are, well, post-handshake
messages. If this proves to be a problem we might try to peek at them.

Doubled the tests that cover logic that's different in TLS 1.3.

The benchmarks for TLS 1.2 compared to be0f3c286b (before TLS 1.3 and
its refactors, after CL 142817 changed them to use real connections)
show little movement.

name                                       old time/op   new time/op   delta
HandshakeServer/RSA-8                        795µs ± 1%    798µs ± 1%    ~     (p=0.057 n=10+18)
HandshakeServer/ECDHE-P256-RSA-8             903µs ± 0%    909µs ± 1%  +0.68%  (p=0.000 n=8+17)
HandshakeServer/ECDHE-P256-ECDSA-P256-8      198µs ± 0%    204µs ± 1%  +3.24%  (p=0.000 n=9+18)
HandshakeServer/ECDHE-X25519-ECDSA-P256-8    202µs ± 3%    208µs ± 1%  +2.98%  (p=0.000 n=9+20)
HandshakeServer/ECDHE-P521-ECDSA-P521-8     15.5ms ± 1%   15.9ms ± 2%  +2.49%  (p=0.000 n=10+20)
Throughput/MaxPacket/1MB-8                  5.81ms ±23%   6.14ms ±44%    ~     (p=0.605 n=8+18)
Throughput/MaxPacket/2MB-8                  8.91ms ±22%   8.74ms ±33%    ~     (p=0.498 n=9+19)
Throughput/MaxPacket/4MB-8                  12.8ms ± 3%   14.0ms ±10%  +9.74%  (p=0.000 n=10+17)
Throughput/MaxPacket/8MB-8                  25.1ms ± 7%   24.6ms ±16%    ~     (p=0.129 n=9+19)
Throughput/MaxPacket/16MB-8                 46.3ms ± 4%   45.9ms ±12%    ~     (p=0.340 n=9+20)
Throughput/MaxPacket/32MB-8                 88.5ms ± 4%   86.0ms ± 4%  -2.82%  (p=0.004 n=10+20)
Throughput/MaxPacket/64MB-8                  173ms ± 2%    167ms ± 7%  -3.42%  (p=0.001 n=10+19)
Throughput/DynamicPacket/1MB-8              5.88ms ± 4%   6.59ms ±64%    ~     (p=0.232 n=9+18)
Throughput/DynamicPacket/2MB-8              9.08ms ±12%   8.73ms ±21%    ~     (p=0.408 n=10+18)
Throughput/DynamicPacket/4MB-8              14.2ms ± 5%   14.0ms ±11%    ~     (p=0.188 n=9+19)
Throughput/DynamicPacket/8MB-8              25.1ms ± 6%   24.0ms ± 7%  -4.39%  (p=0.000 n=10+18)
Throughput/DynamicPacket/16MB-8             45.6ms ± 3%   43.3ms ± 1%  -5.22%  (p=0.000 n=10+8)
Throughput/DynamicPacket/32MB-8             88.4ms ± 3%   84.8ms ± 2%  -4.06%  (p=0.000 n=10+10)
Throughput/DynamicPacket/64MB-8              175ms ± 3%    167ms ± 2%  -4.63%  (p=0.000 n=10+10)
Latency/MaxPacket/200kbps-8                  694ms ± 0%    694ms ± 0%  -0.02%  (p=0.000 n=9+9)
Latency/MaxPacket/500kbps-8                  279ms ± 0%    279ms ± 0%  -0.09%  (p=0.000 n=10+10)
Latency/MaxPacket/1000kbps-8                 140ms ± 0%    140ms ± 0%  -0.15%  (p=0.000 n=10+9)
Latency/MaxPacket/2000kbps-8                71.1ms ± 0%   71.0ms ± 0%  -0.09%  (p=0.001 n=8+9)
Latency/MaxPacket/5000kbps-8                30.5ms ± 6%   30.1ms ± 6%    ~     (p=0.905 n=10+9)
Latency/DynamicPacket/200kbps-8              134ms ± 0%    134ms ± 0%    ~     (p=0.796 n=9+9)
Latency/DynamicPacket/500kbps-8             54.8ms ± 0%   54.7ms ± 0%  -0.18%  (p=0.000 n=8+10)
Latency/DynamicPacket/1000kbps-8            28.5ms ± 0%   29.1ms ± 8%    ~     (p=0.173 n=8+10)
Latency/DynamicPacket/2000kbps-8            15.3ms ± 6%   15.9ms ±10%    ~     (p=0.905 n=9+10)
Latency/DynamicPacket/5000kbps-8            9.14ms ±21%   9.65ms ±82%    ~     (p=0.529 n=10+10)

name                                       old speed     new speed     delta
Throughput/MaxPacket/1MB-8                 175MB/s ±13%  167MB/s ±64%    ~     (p=0.646 n=7+20)
Throughput/MaxPacket/2MB-8                 241MB/s ±25%  241MB/s ±40%    ~     (p=0.660 n=9+20)
Throughput/MaxPacket/4MB-8                 328MB/s ± 3%  300MB/s ± 9%  -8.70%  (p=0.000 n=10+17)
Throughput/MaxPacket/8MB-8                 335MB/s ± 7%  340MB/s ±17%    ~     (p=0.212 n=9+20)
Throughput/MaxPacket/16MB-8                363MB/s ± 4%  367MB/s ±11%    ~     (p=0.340 n=9+20)
Throughput/MaxPacket/32MB-8                379MB/s ± 4%  390MB/s ± 4%  +2.93%  (p=0.004 n=10+20)
Throughput/MaxPacket/64MB-8                388MB/s ± 2%  401MB/s ± 7%  +3.25%  (p=0.004 n=10+20)
Throughput/DynamicPacket/1MB-8             178MB/s ± 4%  157MB/s ±73%    ~     (p=0.127 n=9+20)
Throughput/DynamicPacket/2MB-8             232MB/s ±11%  243MB/s ±18%    ~     (p=0.415 n=10+18)
Throughput/DynamicPacket/4MB-8             296MB/s ± 5%  299MB/s ±15%    ~     (p=0.295 n=9+20)
Throughput/DynamicPacket/8MB-8             334MB/s ± 6%  350MB/s ± 7%  +4.58%  (p=0.000 n=10+18)
Throughput/DynamicPacket/16MB-8            368MB/s ± 3%  388MB/s ± 1%  +5.48%  (p=0.000 n=10+8)
Throughput/DynamicPacket/32MB-8            380MB/s ± 3%  396MB/s ± 2%  +4.20%  (p=0.000 n=10+10)
Throughput/DynamicPacket/64MB-8            384MB/s ± 3%  403MB/s ± 2%  +4.83%  (p=0.000 n=10+10)

Comparing TLS 1.2 and TLS 1.3 at tip shows a slight (~5-10%) slowdown of
handshakes, which might be worth looking at next cycle, but the latency
improvements are expected to overshadow that.

name                                       old time/op   new time/op   delta
HandshakeServer/ECDHE-P256-RSA-8             909µs ± 1%    963µs ± 0%   +5.87%  (p=0.000 n=17+18)
HandshakeServer/ECDHE-P256-ECDSA-P256-8      204µs ± 1%    225µs ± 2%  +10.20%  (p=0.000 n=18+20)
HandshakeServer/ECDHE-X25519-ECDSA-P256-8    208µs ± 1%    230µs ± 2%  +10.35%  (p=0.000 n=20+18)
HandshakeServer/ECDHE-P521-ECDSA-P521-8     15.9ms ± 2%   15.9ms ± 1%     ~     (p=0.444 n=20+19)
Throughput/MaxPacket/1MB-8                  6.14ms ±44%   7.07ms ±46%     ~     (p=0.057 n=18+19)
Throughput/MaxPacket/2MB-8                  8.74ms ±33%   8.61ms ± 9%     ~     (p=0.552 n=19+17)
Throughput/MaxPacket/4MB-8                  14.0ms ±10%   14.1ms ±12%     ~     (p=0.707 n=17+20)
Throughput/MaxPacket/8MB-8                  24.6ms ±16%   25.6ms ±14%     ~     (p=0.107 n=19+20)
Throughput/MaxPacket/16MB-8                 45.9ms ±12%   44.7ms ± 6%     ~     (p=0.607 n=20+19)
Throughput/MaxPacket/32MB-8                 86.0ms ± 4%   87.9ms ± 8%     ~     (p=0.113 n=20+19)
Throughput/MaxPacket/64MB-8                  167ms ± 7%    169ms ± 2%   +1.26%  (p=0.011 n=19+19)
Throughput/DynamicPacket/1MB-8              6.59ms ±64%   6.79ms ±43%     ~     (p=0.480 n=18+19)
Throughput/DynamicPacket/2MB-8              8.73ms ±21%   9.58ms ±13%   +9.71%  (p=0.006 n=18+20)
Throughput/DynamicPacket/4MB-8              14.0ms ±11%   13.9ms ±10%     ~     (p=0.687 n=19+20)
Throughput/DynamicPacket/8MB-8              24.0ms ± 7%   24.6ms ± 8%   +2.36%  (p=0.045 n=18+17)
Throughput/DynamicPacket/16MB-8             43.3ms ± 1%   44.3ms ± 2%   +2.48%  (p=0.001 n=8+9)
Throughput/DynamicPacket/32MB-8             84.8ms ± 2%   86.7ms ± 2%   +2.27%  (p=0.000 n=10+10)
Throughput/DynamicPacket/64MB-8              167ms ± 2%    170ms ± 3%   +1.89%  (p=0.005 n=10+10)
Latency/MaxPacket/200kbps-8                  694ms ± 0%    699ms ± 0%   +0.65%  (p=0.000 n=9+10)
Latency/MaxPacket/500kbps-8                  279ms ± 0%    280ms ± 0%   +0.68%  (p=0.000 n=10+10)
Latency/MaxPacket/1000kbps-8                 140ms ± 0%    141ms ± 0%   +0.59%  (p=0.000 n=9+9)
Latency/MaxPacket/2000kbps-8                71.0ms ± 0%   71.3ms ± 0%   +0.42%  (p=0.000 n=9+9)
Latency/MaxPacket/5000kbps-8                30.1ms ± 6%   30.7ms ±10%   +1.93%  (p=0.019 n=9+9)
Latency/DynamicPacket/200kbps-8              134ms ± 0%    138ms ± 0%   +3.22%  (p=0.000 n=9+10)
Latency/DynamicPacket/500kbps-8             54.7ms ± 0%   56.3ms ± 0%   +3.03%  (p=0.000 n=10+8)
Latency/DynamicPacket/1000kbps-8            29.1ms ± 8%   29.1ms ± 0%     ~     (p=0.173 n=10+8)
Latency/DynamicPacket/2000kbps-8            15.9ms ±10%   16.4ms ±36%     ~     (p=0.633 n=10+8)
Latency/DynamicPacket/5000kbps-8            9.65ms ±82%   8.32ms ± 8%     ~     (p=0.573 n=10+8)

name                                       old speed     new speed     delta
Throughput/MaxPacket/1MB-8                 167MB/s ±64%  155MB/s ±55%     ~     (p=0.224 n=20+19)
Throughput/MaxPacket/2MB-8                 241MB/s ±40%  244MB/s ± 9%     ~     (p=0.407 n=20+17)
Throughput/MaxPacket/4MB-8                 300MB/s ± 9%  298MB/s ±11%     ~     (p=0.707 n=17+20)
Throughput/MaxPacket/8MB-8                 340MB/s ±17%  330MB/s ±13%     ~     (p=0.201 n=20+20)
Throughput/MaxPacket/16MB-8                367MB/s ±11%  375MB/s ± 5%     ~     (p=0.607 n=20+19)
Throughput/MaxPacket/32MB-8                390MB/s ± 4%  382MB/s ± 8%     ~     (p=0.113 n=20+19)
Throughput/MaxPacket/64MB-8                401MB/s ± 7%  397MB/s ± 2%   -0.96%  (p=0.030 n=20+19)
Throughput/DynamicPacket/1MB-8             157MB/s ±73%  156MB/s ±39%     ~     (p=0.738 n=20+20)
Throughput/DynamicPacket/2MB-8             243MB/s ±18%  220MB/s ±14%   -9.65%  (p=0.006 n=18+20)
Throughput/DynamicPacket/4MB-8             299MB/s ±15%  303MB/s ± 9%     ~     (p=0.512 n=20+20)
Throughput/DynamicPacket/8MB-8             350MB/s ± 7%  342MB/s ± 8%   -2.27%  (p=0.045 n=18+17)
Throughput/DynamicPacket/16MB-8            388MB/s ± 1%  378MB/s ± 2%   -2.41%  (p=0.001 n=8+9)
Throughput/DynamicPacket/32MB-8            396MB/s ± 2%  387MB/s ± 2%   -2.21%  (p=0.000 n=10+10)
Throughput/DynamicPacket/64MB-8            403MB/s ± 2%  396MB/s ± 3%   -1.84%  (p=0.005 n=10+10)

Fixes #9671

Change-Id: Ieb57c5140eb2c083b8be0d42b240cd2eeec0dcf6
Reviewed-on: https://go-review.googlesource.com/c/147638
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
2018-11-12 20:44:39 +00:00
Filippo Valsorda
039c2081d1 crypto/tls: set ServerName and unset TLSUnique in ConnectionState in TLS 1.3
Fix a couple overlooked ConnectionState fields noticed by net/http
tests, and add a test in crypto/tls. Spun off CL 147638 to keep that one
cleanly about enabling TLS 1.3.

Change-Id: I9a6c2e68d64518a44be2a5d7b0b7b8d78c98c95d
Reviewed-on: https://go-review.googlesource.com/c/148900
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2018-11-12 20:44:22 +00:00
Filippo Valsorda
46d4aa273d crypto/tls: implement TLS 1.3 downgrade protection
TLS_FALLBACK_SCSV is extremely fragile in the presence of sparse
supported_version, but gave it the best try I could.

Set the server random canaries but don't check them yet, waiting for the
browsers to clear the way of misbehaving middleboxes.

Updates #9671

Change-Id: Ie55efdec671d639cf1e716acef0c5f103e91a7ce
Reviewed-on: https://go-review.googlesource.com/c/147617
Reviewed-by: Adam Langley <agl@golang.org>
2018-11-12 20:44:07 +00:00
Filippo Valsorda
106db71f37 crypto/tls: implement TLS 1.3 client authentication
Note that the SignatureSchemes passed to GetClientCertificate in TLS 1.2
are now filtered by the requested certificate type. This feels like an
improvement anyway, and the full list can be surfaced as well when
support for signature_algorithms_cert is added, which actually matches
the semantics of the CertificateRequest signature_algorithms in TLS 1.2.

Also, note a subtle behavior change in server side resumption: if a
certificate is requested but not required, and the resumed session did
not include one, it used not to invoke VerifyPeerCertificate. However,
if the resumed session did include a certificate, it would. (If a
certificate was required but not in the session, the session is rejected
in checkForResumption.) This inconsistency could be unexpected, even
dangerous, so now VerifyPeerCertificate is always invoked. Still not
consistent with the client behavior, which does not ever invoke
VerifyPeerCertificate on resumption, but it felt too surprising to
entirely change either.

Updates #9671

Change-Id: Ib2b0dbc30e659208dca3ac07d6c687a407d7aaaf
Reviewed-on: https://go-review.googlesource.com/c/147599
Reviewed-by: Adam Langley <agl@golang.org>
2018-11-12 20:43:55 +00:00
Filippo Valsorda
6435d0cfbf crypto/tls: implement TLS 1.3 PSK authentication (server side)
Added some assertions to testHandshake, but avoided checking the error
of one of the Close() because the one that would lose the race would
write the closeNotify to a connection closed on the other side which is
broken on js/wasm (#28650). Moved that Close() after the chan sync to
ensure it happens second.

Accepting a ticket with client certificates when NoClientCert is
configured is probably not a problem, and we could hide them to avoid
confusing the application, but the current behavior is to skip the
ticket, and I'd rather keep behavior changes to a minimum.

Updates #9671

Change-Id: I93b56e44ddfe3d48c2bef52c83285ba2f46f297a
Reviewed-on: https://go-review.googlesource.com/c/147445
Reviewed-by: Adam Langley <agl@golang.org>
2018-11-12 20:43:35 +00:00
Filippo Valsorda
d669cc47ad crypto/tls: implement TLS 1.3 PSK authentication (client side)
Also check original certificate validity when resuming TLS 1.0–1.2. Will
refuse to resume a session if the certificate is expired or if the
original connection had InsecureSkipVerify and the resumed one doesn't.

Support only PSK+DHE to protect forward secrecy even with lack of a
strong session ticket rotation story.

Tested with NSS because s_server does not provide any way of getting the
same session ticket key across invocations. Will self-test like TLS
1.0–1.2 once server side is implemented.

Incorporates CL 128477 by @santoshankr.

Fixes #24919
Updates #9671

Change-Id: Id3eaa5b6c77544a1357668bf9ff255f3420ecc34
Reviewed-on: https://go-review.googlesource.com/c/147420
Reviewed-by: Adam Langley <agl@golang.org>
2018-11-12 20:43:23 +00:00
Filippo Valsorda
dc0be727dc crypto/tls: implement TLS 1.3 middlebox compatibility mode
Looks like the introduction of CCS records in the client second flight
gave time to s_server to send NewSessionTicket messages in between the
client application data and close_notify. There seems to be no way of
turning NewSessionTicket messages off, neither by not sending a
psk_key_exchange_modes extension, nor by command line flag.

Interleaving the client write like that tickled an issue akin to #18701:
on Windows, the client reaches Close() before the last record is drained
from the send buffer, the kernel notices and resets the connection,
cutting short the last flow. There is no good way of synchronizing this,
so we sleep for a RTT before calling close, like in CL 75210. Sigh.

Updates #9671

Change-Id: I44dc1cca17b373695b5a18c2741f218af2990bd1
Reviewed-on: https://go-review.googlesource.com/c/147419
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
2018-11-12 20:43:06 +00:00
Filippo Valsorda
db27e78278 crypto/tls: implement TLS 1.3 KeyUpdate messages
Since TLS 1.3 delivers handshake messages (including KeyUpdate) after
the handshake, the want argument to readRecord had became almost
pointless: it only meant something when set to recordTypeChangeCipherSpec.
Replaced it with a bool to reflect that, and added two shorthands to
avoid anonymous bools in calls.

Took the occasion to simplify and formalize the invariants of readRecord.

The maxConsecutiveEmptyRecords loop became useless when readRecord
started retrying on any non-advancing record in CL 145297.

Replaced panics with errors, because failure is better than undefined
behavior, but contained failure is better than a DoS vulnerability. For
example, I suspect the panic at the top of readRecord was reachable from
handleRenegotiation, which calls readHandshake with handshakeComplete
false. Thankfully it was not a panic in 1.11, and it's allowed now.

Removed Client-TLSv13-RenegotiationRejected because OpenSSL isn't
actually willing to ask for renegotiation over TLS 1.3, the expected
error was due to NewSessionTicket messages, which didn't break the rest
of the tests because they stop too soon.

Updates #9671

Change-Id: I297a81bde5c8020a962a92891b70d6d70b90f5e3
Reviewed-on: https://go-review.googlesource.com/c/147418
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
2018-11-12 20:42:36 +00:00
Filippo Valsorda
29b01d556d crypto/tls: implement TLS 1.3 KeyLogWriter support
Also, add support for the SSLKEYLOGFILE environment variable to the
tests, to simplify debugging of unexpected failures.

Updates #9671

Change-Id: I20a34a5824f083da93097b793d51e796d6eb302b
Reviewed-on: https://go-review.googlesource.com/c/147417
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
2018-11-12 20:42:20 +00:00
Austin Clements
14560da7e4 cmd/link: start file-local symbols at version 10
We're going to use the linker's symbol versions to track ABIs.
Currently, version 0 is used for global symbols and version > 0 is
used for file-local symbols. This CL reserves versions 0 to 9 for
global symbols with ABIs and uses version 10 and up for file-local
symbols. To make this clean, it also introduces a method on Symbol for
querying whether it's file-local.

For #27539.

Change-Id: Id3bc7369268f35128b14318a62e86335181a80e5
Reviewed-on: https://go-review.googlesource.com/c/146859
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2018-11-12 20:27:22 +00:00
Austin Clements
ec4ae29f52 cmd/link: abstract DWARF metadata symbol lookup
The compiler passes a lot of DWARF metadata about functions to the
linker via symbols whose names are derived from the function's own
symbol name. We look up these symbols in several places. This is about
to get slightly more complex as we introduce ABIs as symbol versions,
so abstract this lookup pattern into a helper function.

For #27539.

Change-Id: Ic71f6b5dc6608a5a5f5f515808981e6d6f5d728e
Reviewed-on: https://go-review.googlesource.com/c/146858
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2018-11-12 20:27:21 +00:00
Austin Clements
571236543f debug/gosym: use "go build" instead of hand-running asm and link
Currently, TestPCLine manually invokes asm and link on its test data.
Once we introduce symbol ABIs this is going to become problematic
because the test program defines main.main and main.init in assembly
so they use ABI0, but the runtime expects to find them with the
internal ABI.

There are various ways we could solve this. This CL moves main.main
and main.init into Go code and switches to using "go build" to compile
and link the test binary. This has the added advantage of simplifying
this test.

For #27539.

Change-Id: I4c0cf6467f7a39e6b1500eca6ad2620b5ef2b73c
Reviewed-on: https://go-review.googlesource.com/c/146857
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-11-12 20:27:20 +00:00
Austin Clements
af1bfe0aa3 runtime: correct ABI information for all functions
There are three cases where we don't currently have the visibility to
get the ABIs of runtime symbols right, which this CL fixes:

1. For Go functions referenced from non-Go code in other packages.
   This is runtime.morestackc (which is referenced from function
   prologues) and a few syscall symbols. For these we need to generate
   ABI0 wrappers, so this CL adds dummy calls in the assembly code to
   force wrapper generation. There are many other cross-package
   references to runtime and runtime/internal/atomic, but these are
   handled specially by cmd/go.

2. For calls generated by the compiler to runtime Go functions, there
   are a few symbols that aren't declared in builtins.go because we've
   never needed their type information before. Now we at least need
   their ABI information, so these are added to builtins.go.

3. For calls generated by the compiler to runtime assembly functions,
   the compiler is going to assume the internal ABI is available, so
   we add Go stubs to the runtime to trigger wrapper generation. For
   these we're probably going to want to provide internal ABI
   definitions directly in the assembly for performance, but for now
   the ABIs are the same so it doesn't matter.

For #27539.

Change-Id: I9c224e7408d2ef4dd9b0e4c9d7e962ddfe111245
Reviewed-on: https://go-review.googlesource.com/c/146822
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
2018-11-12 20:27:18 +00:00
Austin Clements
6096b85b13 runtime: avoid variable/function alias on runtime._cgo_panic_internal
The symbol runtime._cgo_panic_internal is defined both as a function
in package runtime and as a (linknamed) variable in package
runtime/cgo. Since we're introducing function ABIs, this is going to
cause problems with resolving the ABI-marked function symbol with the
unmarked data symbol. It's also confusing.

Fix this by declaring runtime._cgo_panic_internal as a function in
runtime/cgo as well and extracting the PC from the function object.

For #27539.

Change-Id: I148a458a600cf9e57791cf4cbe92e79bddbf58d4
Reviewed-on: https://go-review.googlesource.com/c/146821
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2018-11-12 20:27:17 +00:00
Austin Clements
ef7ce57ac2 internal/bytealg, runtime: provide linknames for pushed symbols
The internal/bytealg package defines several symbols in the runtime,
bytes, and strings packages in assembly, and the runtime package
defines symbols in reflect and sync/atomic. Currently, there's no
corresponding Go prototype for these symbols in the defining package.

We're going to start depending on Go prototypes in the same package as
their assembly definitions in order to provide ABI wrappers. Plus,
these are good documentation and colocate type information with
definitions, which could be useful for vet if it learned a little
about linkname.

This CL adds linknamed Go prototypes for all pushed symbols in
internal/bytealg and runtime.

For #27539.

Change-Id: I9b0c12d935a75bb6af46b6761180d451c00f11b8
Reviewed-on: https://go-review.googlesource.com/c/146820
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
2018-11-12 20:27:16 +00:00
Austin Clements
4f3604d3f2 runtime, reflect: access runtime.reflectcall directly
Currently, package runtime contains the definition of reflect.call,
even though it's just a jump to runtime.reflectcall. This "push"
symbol is confusing, since it's not clear where the definition of
reflect.call comes from when you're in the reflect package.

Replace this with a "pull" symbol: the runtime now defines only
runtime.reflectcall and package reflect uses a go:linkname to access
this symbol directly. This makes it clear where reflect.call is coming
from without any spooky action at a distance and eliminates all of the
definitions of reflect.call in the runtime.

Change-Id: I3ec73cd394efe9df8d3061a57c73aece2e7048dd
Reviewed-on: https://go-review.googlesource.com/c/148657
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
2018-11-12 20:27:14 +00:00
Ainar Garipov
f58b02a29c go/build: remove superfluous continues
This cleanup was proposed in CL 148937. The branch is already ended with
a continue, so remove continues from subbranches and use an else-if.

Change-Id: Iaf6eb57afc84e25862f99a342f5824e315bcdcb7
Reviewed-on: https://go-review.googlesource.com/c/148922
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-11-11 18:49:40 +00:00
Michael Anthony Knyszek
3a7a56cc70 runtime: gofmt all improperly formatted code
This change fixes incorrect formatting in mheap.go (the result of my
previous heap scavenging changes) and map_test.go.

Change-Id: I2963687504abdc4f0cdf2f0c558174b3bc0ed2df
Reviewed-on: https://go-review.googlesource.com/c/148977
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2018-11-11 16:09:05 +00:00
Josh Bleecher Snyder
8607b2e825 cmd/compile: optimize A->B->C Moves that include VarDefs
We have an existing optimization that recognizes
memory moves of the form A -> B -> C and converts
them into A -> C, in the hopes that the store to
B will be end up being dead and thus eliminated.

However, when A, B, and C are large types,
the front end sometimes emits VarDef ops for the moves.
This change adds an optimization to match that pattern.

This required changing an old compiler test.
The test assumed that a temporary was required
to deal with a large return value.
With this optimization in place, that temporary
ended up being eliminated.

Triggers 649 times during 'go build -a std cmd'.

Cuts 16k off cmd/go.

name        old object-bytes  new object-bytes  delta
Template          507kB ± 0%        507kB ± 0%  -0.15%  (p=0.008 n=5+5)
Unicode           225kB ± 0%        225kB ± 0%    ~     (all equal)
GoTypes          1.85MB ± 0%       1.85MB ± 0%    ~     (all equal)
Flate             328kB ± 0%        328kB ± 0%    ~     (all equal)
GoParser          402kB ± 0%        402kB ± 0%  -0.00%  (p=0.008 n=5+5)
Reflect          1.41MB ± 0%       1.41MB ± 0%  -0.20%  (p=0.008 n=5+5)
Tar               458kB ± 0%        458kB ± 0%    ~     (all equal)
XML               601kB ± 0%        599kB ± 0%  -0.21%  (p=0.008 n=5+5)

Change-Id: I9b5f25c8663a0b772ad1ee51fa61f74b74d26dd3
Reviewed-on: https://go-review.googlesource.com/c/143479
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Munday <mike.munday@ibm.com>
2018-11-11 14:18:33 +00:00
Ainar Garipov
f9fff4554c go/build, go/doc: fix tautological conditions
These issues were found by the new vet's nilness check. The variables
were already checked against nil, so remove extra checks.

Change-Id: Ie252ccfcc755f3d06f691f354bf13d5a623fe17b
Reviewed-on: https://go-review.googlesource.com/c/148937
Reviewed-by: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2018-11-11 01:02:11 +00:00
Nikhil Benesch
8e0ec5ec09 runtime: ensure m.p is never stale
When a goroutine enters a syscall, its M unwires from its P to allow
the P to be retaken by another M if the syscall is slow. The M retains a
reference to its old P, however, so that if its old P has not been
retaken when the syscall returns, it can quickly reacquire that P.

The implementation, however, was confusing, as it left the reference to
the potentially-retaken P in m.p, which implied that the P was still
wired.

Make the code clearer by enforcing the invariant that m.p is never
stale. entersyscall now moves m.p to m.oldp and sets m.p to 0;
exitsyscall does the reverse, provided m.oldp has not been retaken.

With this scheme in place, the issue described in #27660 (assertion
failures in the race detector) would have resulted in a clean segfault
instead of silently corrupting memory.

Change-Id: Ib3e03623ebed4f410e852a716919fe4538858f0a
Reviewed-on: https://go-review.googlesource.com/c/148899
Run-TryBot: Dmitry Vyukov <dvyukov@google.com>
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2018-11-10 19:26:41 +00:00
Josh Bleecher Snyder
e4c1feef74 cmd/compile: re-run stringer
Some of the Ops got re-ordered with OSLICEHEADER. Re-generate.

Change-Id: I8ec3f5056537dd0b18cac2d4267453451ade2b00
Reviewed-on: https://go-review.googlesource.com/c/148824
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Martin Möhrmann <martisch@uos.de>
2018-11-10 16:04:18 +00:00
Josh Bleecher Snyder
fe2c588b1c cmd/compile: simplify walk OCONVIFACE
n.Type and n.Left.Type are used heavily. Give them useful names.

We generate the type word frequently. Make it a closure.
(We don't want to generate it up front, since there are some code
paths that don't need it, and generating it has side-effects.)

Simplify and document the final call construction.

Follow-up to address feedback on CL 147360.

Change-Id: I251134a55cf80d8b1676280a345d150f2288c09a
Reviewed-on: https://go-review.googlesource.com/c/147538
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Martin Möhrmann <moehrmann@google.com>
2018-11-10 13:39:01 +00:00
Richard Musiol
6dd70fc5e3 all: add support for synchronous callbacks to js/wasm
With this change, callbacks returned by syscall/js.NewCallback
get executed synchronously. This is necessary for the APIs of
many JavaScript libraries.

A callback triggered during a call from Go to JavaScript gets executed
on the same goroutine. A callback triggered by JavaScript's event loop
gets executed on an extra goroutine.

Fixes #26045
Fixes #27441

Change-Id: I591b9e85ab851cef0c746c18eba95fb02ea9e85b
Reviewed-on: https://go-review.googlesource.com/c/142004
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2018-11-10 11:57:17 +00:00
Martin Möhrmann
e3e043bea4 cmd/compile: improve typechecking of OSLICEHEADER nodes
Create a new node for OSLICEHEADER nodes to ensure typechecks are applied.
Add nil checks for OSLICEHEADER type and pointer parameters
for better error messages when these are not set.
Improve formatting of OSLICEHEADER nodes in compiler error messages.

Change-Id: Idea8f41bb4beb636f0e1fc381ff8d79b1d44fbae
Reviewed-on: https://go-review.googlesource.com/c/146997
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
2018-11-10 07:18:20 +00:00
Ian Lance Taylor
c5aea7a494 cmd/go: add go mod edit -go flag
It can be used to set the Go language version used by the module.

RELNOTES=yes

Updates #28221

Change-Id: Ief0dd185c01195a17be20dff8627c80943c436e7
Reviewed-on: https://go-review.googlesource.com/c/147282
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
2018-11-10 01:42:03 +00:00
Ian Lance Taylor
6887d8b1e2 cmd/go: add go statement when initializing go.mod
When creating a go.mod file, add a go statement mentioning the current
Go version. We can be reasonably confident that the current version is
able to build the module. This is as described in the language
transition proposal at https://golang.org/issue/28221.

Updates #28221

Change-Id: I70a99b3a53f4b6c0288da07473c5a71bb28cd86f
Reviewed-on: https://go-review.googlesource.com/c/147281
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
2018-11-10 01:22:06 +00:00
Ainar Garipov
084f2eafcd runtime: don't check _defer against nil twice
This issue was found by the new vet's nilness check. _defer was already
checked against nil, so don't check it again.

Change-Id: I78725eaec7234b262b3c941e06441ca57f82bdd9
Reviewed-on: https://go-review.googlesource.com/c/148917
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2018-11-09 23:56:58 +00:00
Alex Brainman
ff7b245a31 Revert "os: remove sleep in windows Process.Wait"
This reverts CL 145221 (commit 5c359736f8)

Reason for revert: breaks the build occasionally.

Updates #23171
Updates #25965

Change-Id: Ie1e3c76ab9bcd8d28b6118440b5f80c76f9b1852
Reviewed-on: https://go-review.googlesource.com/c/148957
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-11-09 23:24:35 +00:00
Ian Lance Taylor
03e3fbe421 cmd/cgo: make the gccgo init function no_split_stack
This works around what appears to be a bug in current clang (2018-11-09).
Details are in the comment in the code.

Change-Id: Ib4783b6c03d531c69ebc4cb0ac023bea5bee7d40
Reviewed-on: https://go-review.googlesource.com/c/148819
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-11-09 23:06:59 +00:00
Nikhil Benesch
e496e612b7 runtime: never call into race detector with retaken P
cgocall could previously invoke the race detector on an M whose P had
been retaken. The race detector would attempt to use the P-local state
from this stale P, racing with the thread that was actually wired to
that P. The result was memory corruption of ThreadSanitizer's internal
data structures that presented as hard-to-understand assertion failures
and segfaults.

Reorder cgocall so that it always acquires a P before invoking the race
detector, and add a test that stresses the interaction between cgo and
the race detector to protect against future bugs of this kind.

Fixes #27660.

Change-Id: Ide93f96a23490314d6647547140e0a412a97f0d4
Reviewed-on: https://go-review.googlesource.com/c/148717
Run-TryBot: Dmitry Vyukov <dvyukov@google.com>
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
2018-11-09 21:47:48 +00:00
Michael Anthony Knyszek
06be7cbf3c runtime: stop unnecessary span scavenges on free
This change fixes a bug wherein freeing a scavenged span that didn't
coalesce with any neighboring spans would result in that span getting
scavenged again. This case may actually be a common occurance because
"freeing" span trimmings and newly-grown spans end up using the same
codepath. On systems where madvise is relatively expensive, this can
have a large performance impact.

This change also cleans up some of this logic in freeSpanLocked since
a number of factors made the coalescing code somewhat difficult to
reason about with respect to scavenging. Notably, the way the
needsScavenge boolean is handled could be better expressed and the
inverted conditions (e.g. !after.released) can make things even more
confusing.

Fixes #28595.

Change-Id: I75228dba70b6596b90853020b7c24fbe7ab937cf
Reviewed-on: https://go-review.googlesource.com/c/147559
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
2018-11-09 20:57:57 +00:00
Josh Bleecher Snyder
78c0e1f81d cmd/compile: eliminate dead code
During walkexpr, we were assessing whether shifts were bounded.
However, that information was dropped on the floor during SSA conversion.
The SSA backend already finds all bounded shifts that walkexpr could have,
and at negligible extra cost (0.02% in alloc, CPU undetectable).

Change-Id: Ieda1af1a2a3ec99bfdc2b0b704c9b80ce8a34486
Reviewed-on: https://go-review.googlesource.com/c/148897
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2018-11-09 19:57:57 +00:00
Lynn Boger
4ae49b5921 cmd/compile: use ANDCC, ORCC, XORCC to avoid CMP on ppc64x
This change makes use of the cc versions of the AND, OR, XOR
instructions, omitting the need for a CMP instruction.

In many test programs and in the go binary, this reduces the
size of 20-30 functions by at least 1 instruction, many in
runtime.

Testcase added to test/codegen/comparisons.go

Change-Id: I6cc1ca8b80b065d7390749c625bc9784b0039adb
Reviewed-on: https://go-review.googlesource.com/c/143059
Reviewed-by: Carlos Eduardo Seo <cseo@linux.vnet.ibm.com>
Reviewed-by: Michael Munday <mike.munday@ibm.com>
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2018-11-09 19:40:52 +00:00
Ryan Dahl
410d63dbe9 bufio: fix indexes in TestWriter
Change-Id: I393c53d6f7b526d156226502544725a4cb9fb118
GitHub-Last-Rev: 5d53406c70
GitHub-Pull-Request: golang/go#28693
Reviewed-on: https://go-review.googlesource.com/c/148818
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
2018-11-09 19:15:11 +00:00
Brad Fitzpatrick
2b534f28ce Revert "cmd/vet: lostcancel: suppress the check in the main.main function"
This reverts CL 148758 (commit 5e17ce22ec)

Reason for revert: breaks the build.

Change-Id: I6ed15b7b8f6b74d84edab9402ddf7ae87a0d0387
Reviewed-on: https://go-review.googlesource.com/c/148817
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
2018-11-09 17:37:16 +00:00
Alan Donovan
5e17ce22ec cmd/vet: lostcancel: suppress the check in the main.main function
When main.main returns, the process exits, so there's no need to cancel contexts.

This change was initially reviewed as
https://go-review.googlesource.com/c/go/+/106915/4
but somehow I messed up and committed patchset 5, which was
effectively empty.

Change-Id: Ic4250eb6563af9bc734e429aafc7081ca7d0e012
Reviewed-on: https://go-review.googlesource.com/c/148758
Reviewed-by: Alan Donovan <adonovan@google.com>
2018-11-09 16:11:42 +00:00