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>
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>
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>
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.
Filippo Valsorda [Sat, 3 Nov 2018 22:29:09 +0000 (18:29 -0400)]
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.
Austin Clements [Fri, 19 Oct 2018 21:42:11 +0000 (17:42 -0400)]
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.
Austin Clements [Mon, 22 Oct 2018 20:36:24 +0000 (16:36 -0400)]
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.
Austin Clements [Thu, 1 Nov 2018 18:34:21 +0000 (14:34 -0400)]
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.
Austin Clements [Thu, 1 Nov 2018 16:25:41 +0000 (12:25 -0400)]
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>
Austin Clements [Thu, 1 Nov 2018 00:39:59 +0000 (20:39 -0400)]
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>
Austin Clements [Wed, 31 Oct 2018 19:04:04 +0000 (15:04 -0400)]
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>
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.
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.
Nikhil Benesch [Fri, 9 Nov 2018 05:55:13 +0000 (00:55 -0500)]
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.
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.
Richard Musiol [Thu, 11 Oct 2018 10:46:14 +0000 (12:46 +0200)]
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.
Martin Möhrmann [Fri, 2 Nov 2018 13:38:33 +0000 (14:38 +0100)]
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.
Ian Lance Taylor [Fri, 2 Nov 2018 23:58:42 +0000 (16:58 -0700)]
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>
Ainar Garipov [Fri, 9 Nov 2018 21:01:14 +0000 (00:01 +0300)]
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>
Nikhil Benesch [Fri, 9 Nov 2018 05:55:13 +0000 (00:55 -0500)]
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.
Michael Anthony Knyszek [Tue, 6 Nov 2018 00:10:33 +0000 (00:10 +0000)]
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>
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).
Alan Donovan [Fri, 13 Apr 2018 18:27:33 +0000 (14:27 -0400)]
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>
runtime: reduce linear search through pcvalue cache
This change introduces two optimizations together,
one for recursive and one for non-recursive stacks.
For recursive stacks, we introduce the new entry
at the beginning of the cache, so it can be found first.
This adds an extra read and write.
While we're here, switch from fastrandn, which does a multiply,
to fastrand % n, which does a shift.
For non-recursive stacks, split the cache from [16]pcvalueCacheEnt
into [2][8]pcvalueCacheEnt, and add a very cheap associative lookup.
Lynn Boger [Thu, 25 Oct 2018 15:13:16 +0000 (11:13 -0400)]
crypto/md5: fix md5block asm to work on big endian ppc64
This handles a TODO in the md5block_ppc64le.s file to
make use of byte reverse loads so the function works for
big endian as well as little endian. File name is now
md5block_ppc64x.s.
Daniel Martí [Tue, 6 Nov 2018 21:35:23 +0000 (21:35 +0000)]
cmd/vet: fix printf false negative with nested pointers
Pointers to compound objects (structs, slices, arrays, maps) are only
followed by fmt if the pointer is at the top level of an argument. This
is to minimise the chances of fmt running into loops.
However, vet did not follow this rule. It likely doesn't help that fmt
does not document that restriction well, which is being tracked in
#28625.
Updates #27672.
Change-Id: Ie9bbd9b974eda5ab9a285986d207ef92fca4453e
Reviewed-on: https://go-review.googlesource.com/c/147997
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Alan Donovan <adonovan@google.com>
Brad Fitzpatrick [Thu, 8 Nov 2018 20:06:05 +0000 (20:06 +0000)]
cmd/go/internal/cache: fix wrong/old function name in comment
Change-Id: Ia0caf2fb06097ac184f78779334460900e8c0149
Reviewed-on: https://go-review.googlesource.com/c/148580 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Alan Donovan [Thu, 8 Nov 2018 15:43:40 +0000 (10:43 -0500)]
cmd/go: vet: revert $GOVETTOOL env var, restore -vettool flag
The environment variable is no longer necessary as we now plan to
transition to the new vet by replacing it in a single step,
and we really don't want to add more environment variables.
Fixes #28636
Change-Id: Ib85e5c0d61213b7b9f6a53d9376fec29525df971
Reviewed-on: https://go-review.googlesource.com/c/148497
Run-TryBot: Alan Donovan <adonovan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Keith Randall [Mon, 5 Nov 2018 03:23:08 +0000 (19:23 -0800)]
cmd/compile: encourage inlining of functions with single-call bodies
This is a simple tweak to allow a bit more mid-stack inlining.
In cases like this:
func f() {
g()
}
We'd really like to inline f into its callers. It can't hurt.
We implement this optimization by making calls a bit cheaper, enough
to afford a single call in the function body, but not 2.
The remaining budget allows for some argument modification, or perhaps
a wrapping conditional:
func f(x int) {
g(x, 0)
}
func f(x int) {
if x > 0 {
g()
}
}
Filippo Valsorda [Tue, 6 Nov 2018 02:00:52 +0000 (21:00 -0500)]
crypto/tls: remove a forgotten note to future self
Now, this is embarrassing. While preparing CL 142818, I noticed a
possible vulnerability in the existing code which I was rewriting. I
took a note to go back and assess if it was indeed an issue, and in case
start the security release process. The note unintentionally slipped
into the commit. Fortunately, there was no vulnerability.
What caught my eye was that I had fixed the calculation of the minimum
encrypted payload length from
roundUp(explicitIVLen+macSize+1, blockSize)
to (using the same variable names)
explicitIVLen + roundUp(macSize+1, blockSize)
The explicit nonce sits outside of the encrypted payload, so it should
not be part of the value rounded up to the CBC block size.
You can see that for some values of the above, the old result could be
lower than the correct value. An unexpectedly short payload might cause
a panic during decryption (a DoS vulnerability) or even more serious
issues due to the constant time code that follows it (see for example
Yet Another Padding Oracle in OpenSSL CBC Ciphersuites [1]).
In practice, explicitIVLen is either zero or equal to blockSize, so it
does not change the amount of rounding up necessary and the two
formulations happen to be identical. Nothing to see here.
It looked more suspicious than it is in part due to the fact that the
explicitIVLen definition moved farther into hc.explicitNonceLen() and
changed name from IV (which suggests a block length) to nonce (which
doesn't necessarily). But anyway it was never meant to surface or be
noted, except it slipped, so here we are for a boring explanation.
Ian Lance Taylor [Fri, 2 Nov 2018 23:00:33 +0000 (16:00 -0700)]
cmd/go: pass go language version to cmd/compile
Pass the Go language version specified in the go.mod file to
cmd/compile.
Also, change the behavior when the go.mod file requests a Go version
that is later than the current one. Previously cmd/go would give a
fatal error in this situation. With this change it attempts the
compilation, and if (and only if) the compilation fails it adds a note
saying that the requested Go version is newer than the known version.
This is as described in https://golang.org/issue/28221.
Updates #28221.
Change-Id: I46803813e7872d4a418a3fd5299880be3b73a971
Reviewed-on: https://go-review.googlesource.com/c/147278
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Ian Lance Taylor [Fri, 2 Nov 2018 22:42:15 +0000 (15:42 -0700)]
cmd/go: add cmpenv command to testing script language
This will be used by later commits in this sequence.
Updates #28221
Change-Id: I2b22b9f88a0183636cde9509606f03f079eb33f1
Reviewed-on: https://go-review.googlesource.com/c/147277
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Keith Randall [Mon, 24 Sep 2018 14:13:36 +0000 (07:13 -0700)]
syscall: implement syscalls on Darwin using libSystem
There are still some references to the bare Syscall functions
in the stdlib. I will root those out in a following CL.
(This CL is big enough as it is.)
Most are in vendor directories:
Robert Griesemer [Mon, 5 Nov 2018 23:51:11 +0000 (15:51 -0800)]
go/types: avoid certain problems with recursive alias type declarations
It is possible to create certain recursive type declarations involving
alias types which cause the type-checker to produce an (invalid) type
for the alias because it is not yet available. By type-checking alias
declarations in a 2nd phase, the problem is mitigated a bit since it
requires more convoluted alias declarations for the problem to appear.
Also re-enable testing of fixedbugs/issue27232.go again (which was the
original cause for this change).
Updates #28576.
Change-Id: If6f9656a95262e6575b01c4a003094d41551564b
Reviewed-on: https://go-review.googlesource.com/c/147597 Reviewed-by: Alan Donovan <adonovan@google.com>
Also update misc/nacl/testzip.proto to include new testdata.
Change-Id: If41590be9f395a591056e89a417b589c4ba71b1a
Reviewed-on: https://go-review.googlesource.com/c/147979
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
David Chase [Tue, 6 Nov 2018 21:42:13 +0000 (16:42 -0500)]
cmd/compile: update TestNexting golden file for Delve
This change updates the expected output of the delve debugging session
in the TestNexting internal/ssa test, aligning it with the changes
introduced in CL 147360 and earlier.
Change-Id: I1cc788d02433624a36f4690f24201569d765e5d3
Reviewed-on: https://go-review.googlesource.com/c/147998
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Alberto Donizetti [Tue, 6 Nov 2018 21:08:30 +0000 (22:08 +0100)]
cmd/compile: update TestNexting golden file
This change updates the expected output of the gdb debugging session
in the TestNexting internal/ssa test, aligning it with the changes
introduced in CL 147360.
Fixes the longtest builder.
Change-Id: I5b5c22e1cf5e205967ff8359dc6c1485c815428e
Reviewed-on: https://go-review.googlesource.com/c/147957
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Daniel Martí [Thu, 25 Oct 2018 12:42:46 +0000 (13:42 +0100)]
cmd/go: make 'go test -h' print two lines
Like every other command's -h flag. To achieve this, pass the command's
usage function to the cmdflag package, since that package is used by
multiple commands and cannot directly access *base.Command.
This also lets us get rid of testFlag1 and testFlag2, and instead have
contiguous raw strings for the test and testflag help docs.
Fixes #26999.
Change-Id: I2ebd66835ee61fa83270816a01fa312425224bb3
Reviewed-on: https://go-review.googlesource.com/c/144558
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Alan Donovan <adonovan@google.com>
Alberto Donizetti [Tue, 6 Nov 2018 18:43:55 +0000 (19:43 +0100)]
cmd/compile: add new format to known_formats
This change fixes a TestFormat failure in fmt_test by adding a
recently introduced new known format (%q for syntax.Error).
Fixes #28621
Change-Id: I026ec88c334549a957a692c1652a860c57e23dae
Reviewed-on: https://go-review.googlesource.com/c/147837 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Cherry Zhang [Fri, 2 Nov 2018 20:51:14 +0000 (16:51 -0400)]
cmd/asm: rename R18 to R18_PLATFORM on ARM64
In ARM64 ABI, R18 is the "platform register", the use of which is
OS specific. The OS could choose to reserve this register. In
practice, it seems fine to use R18 on Linux but not on darwin (iOS).
Rename R18 to R18_PLATFORM to prevent accidental use. There is no
R18 usage within the standard library (besides tests, which are
updated).
This change adds the vet-lite command (the future cmd/vet) and all its
dependencies from x/tools, but not its tests and their dependencies.
It was created with these commands:
I feel sure I am holding govendor wrong. Please advise.
A followup CL will make cmd/vet behave like vet-lite, initially just
for users that opt in, and soon after for all users, at which point
cmd/vet will be replaced in its entirety by a copy of vet-lite's small
main.go.
In the meantime, anyone can try the new tool using these commands:
$ go build cmd/vendor/golang.org/x/tools/go/analysis/cmd/vet-lite
$ export GOVETTOOL=$(which vet-lite)
$ go vet your/project/...
Lynn Boger [Fri, 5 Oct 2018 18:21:39 +0000 (14:21 -0400)]
runtime: improve performance of memclr, memmove on ppc64x
This improves the asm implementations for memmove and memclr on
ppc64x through use of vsx loads and stores when size is >= 32 bytes.
For memclr, dcbz is used when the size is >= 512 and aligned to 128.
Clément Chigot [Fri, 2 Nov 2018 12:14:07 +0000 (13:14 +0100)]
cmd/compile/internal/gc: update cgo_import_dynamic for AIX
On AIX, cmd/link needs two information in order to generate a dynamic
import, the library and its object needed. Currently, cmd/link isn't
able to retrieve this object only with the name of the library.
Therefore, the library pattern in cgo_import_dynamic must be
"lib.a/obj.o".
Change-Id: Ib8b8aaa9807c9fa6af46ece4e312d58073ed6ec1
Reviewed-on: https://go-review.googlesource.com/c/146957
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Raghavendra Nagaraj [Tue, 6 Nov 2018 09:02:03 +0000 (09:02 +0000)]
reflect: fix StructOf panics from too many methods in embedded fields
Previously we panicked if the number of methods present for an embedded
field was >= 32. This change removes that limit and now StructOf
dynamically calls itself to create space for the number of methods.
Fixes #25402
Change-Id: I3b1deb119796d25f7e6eee1cdb126327b49a0b5e
GitHub-Last-Rev: 16da71ad6b23563f3ed26f1914adf41e3d42de69
GitHub-Pull-Request: golang/go#26865
Reviewed-on: https://go-review.googlesource.com/c/128479
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
We collapse OpOffPtrs during generic rewrites.
However, we also use disjoint at the same time.
Instead of waiting for all OpOffPtrs to be collapsed
before the disjointness rules can kick in,
burrow through all OpOffPtrs immediately.
Change-Id: I60d0a70a9b4605b1817db7c4aab0c0d789651c90
Reviewed-on: https://go-review.googlesource.com/c/145206
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com> Reviewed-by: Michael Munday <mike.munday@ibm.com> Reviewed-by: Keith Randall <khr@golang.org>
convT2E16 and other specialized type-to-interface routines
accept a type/itab argument and return a complete interface value.
However, we know enough in the routine to do without the type.
And the caller can construct the interface value using the type.
Doing so shrinks the call sites of ten of the specialized convT2x routines.
It also lets us unify the empty and non-empty interface routines.
Prior to this change, since f and g have identical signatures,
they would share a single generated func type.
types.SubstAny makes a shallow type copy, even after instantiation,
f and g share a single generated Result type.
So if you instantiate f with any=T, call dowidth,
instantiate g with any=U, and call dowidth,
and if sizeof(T) != sizeof(U),
then the Offset of the result for f is now wrong.
I don't believe this happens at all right now, but it bit me hard when
experimenting with some other compiler changes.
And it's hard to debug. It results in rare stack corruption, causing
problems far from the actual source of the problem.
To fix this, change SubstAny to make deep copies of TSTRUCTs.
Michael Anthony Knyszek [Mon, 5 Nov 2018 19:26:25 +0000 (19:26 +0000)]
runtime: clean up MSpan* MCache* MCentral* in docs
This change cleans up references to MSpan, MCache, and MCentral in the
docs via a bunch of sed invocations to better reflect the Go names for
the equivalent structures (i.e. mspan, mcache, mcentral) and their
methods (i.e. MSpan_Sweep -> mspan.sweep).
Austin Clements [Mon, 5 Nov 2018 17:36:42 +0000 (12:36 -0500)]
runtime: deflake TestTracebackAncestors
TestTracebackAncestors has a ~0.1% chance of failing with more
goroutines in the traceback than expected. This happens because
there's a window between each goroutine starting its child and that
goroutine actually exiting. The test captures its own stack trace
after everything is "done", but if this happens during that window, it
will include the goroutine that's in the process of being torn down.
Here's an example of such a failure:
https://build.golang.org/log/fad10d0625295eb79fa879f53b8b32b9d0596af8
This CL fixes this by recording the goroutines that are expected to
exit and removing them from the stack trace. With this fix, this test
passed 15,000 times with no failures.
Change-Id: I71e7c6282987a15e8b74188b9c585aa2ca97cbcd
Reviewed-on: https://go-review.googlesource.com/c/147517
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Yury Smolsky [Tue, 30 Oct 2018 22:19:35 +0000 (00:19 +0200)]
go/doc: inspect function signature for building playground examples
This documentation example was broken:
https://golang.org/pkg/image/png/#example_Decode.
It did not have the "io" package imported,
The package was referenced in the result type of the function.
The "playExample" function did not inspect
the result types of declared functions.
This CL adds inspecting of parameters and result types of functions.
Robert Griesemer [Sat, 3 Nov 2018 06:28:26 +0000 (23:28 -0700)]
cmd/compile: reintroduce work-around for cyclic alias declarations
This change re-introduces (temporarily) a work-around for recursive
alias type declarations, originally in https://golang.org/cl/35831/
(intended as fix for #18640). The work-around was removed later
for a more comprehensive cycle detection check. That check
contained a subtle error which made the code appear to work,
while in fact creating incorrect types internally. See #25838
for details.
By re-introducing the original work-around, we eliminate problems
with many simple recursive type declarations involving aliases;
specifically cases such as #27232 and #27267. However, the more
general problem remains.
This CL also fixes the subtle error (incorrect variable use when
analyzing a type cycle) mentioned above and now issues a fatal
error with a reference to the relevant issue (rather than crashing
later during the compilation). While not great, this is better
than the current status. The long-term solution will need to
address these cycles (see #25838).
As a consequence, several old test cases are not accepted anymore
by the compiler since they happened to work accidentally only.
This CL disables parts or all code of those test cases. The issues
are: #18640, #23823, and #24939.
One of the new test cases (fixedbugs/issue27232.go) exposed a
go/types issue. The test case is excluded from the go/types test
suite and an issue was filed (#28576).
Michael Anthony Knyszek [Wed, 17 Oct 2018 20:16:45 +0000 (20:16 +0000)]
runtime: fix stale comments about mheap and mspan
As of 07e738e all spans are allocated out of a treap, and not just
large spans or spans for large objects. Also, now we have a separate
treap for spans that have been scavenged.
Austin Clements [Thu, 17 Aug 2017 22:11:01 +0000 (18:11 -0400)]
cmd/trace: display p99.9, p99 and p95 MUT
This uses the mutator utilization distribution to compute the p99.9,
p99, and p95 mutator utilization topograph lines and display them
along with the MMU.
Change-Id: I8c7e0ec326aa4bc00619ec7562854253f01cc802
Reviewed-on: https://go-review.googlesource.com/c/60800
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
The current MMU analysis considers all Ps together, so if, for
example, one of four Ps is blocked, mutator utilization is 75%.
However, this is less useful for understanding the impact on
individual goroutines because that one blocked goroutine could be
blocked for a very long time, but we still appear to have good
utilization.
Hence, this introduces a new flag that does a "per-P" analysis where
the utilization of each P is considered independently. The MMU is then
the combination of the MMU for each P's utilization function.
Change-Id: Id67b980d4d82b511d28300cdf92ccbb5ae8f0c78
Reviewed-on: https://go-review.googlesource.com/c/60797
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
cmd/trace: list and link to worst mutator utilization windows
This adds the ability to click a point on the MMU graph to show a list
of the worst 10 mutator utilization windows of the selected size. This
list in turn links to the trace viewer to drill down on specifically
what happened in each specific window.
Change-Id: Ic1b72d8b37fbf2212211c513cf36b34788b30133
Reviewed-on: https://go-review.googlesource.com/c/60795
Run-TryBot: Austin Clements <austin@google.com> Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Austin Clements [Tue, 8 Aug 2017 22:15:32 +0000 (18:15 -0400)]
internal/trace: use banding to optimize MMU computation
This further optimizes MMU construction by first computing a
low-resolution summary of the utilization curve. This "band" summary
lets us compute the worst-possible window starting in each of these
low-resolution bands (even without knowing where in the band the
window falls). This in turn lets us compute precise minimum mutator
utilization only in the worst low-resolution bands until we can show
that any remaining bands can't possibly contain a worse window.
This slows down MMU construction for small traces, but these are
reasonably fast to compute either way. For large traces (e.g.,
150,000+ utilization changes) it's significantly faster.
Change-Id: Ie66454e71f3fb06be3f6173b6d91ad75c61bda48
Reviewed-on: https://go-review.googlesource.com/c/60792
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Austin Clements [Mon, 28 Aug 2017 16:29:07 +0000 (12:29 -0400)]
internal/trace: use MU slope to optimize MMU
This commit speeds up MMU construction by ~10X (and reduces the number
of windows considered by ~20X) by using an observation about the
maximum slope of the windowed mutator utilization function to advance
the window time in jumps if the window's current mean mutator
utilization is much larger than the current minimum.
Change-Id: If3cba5da0c4adc37b568740f940793e491e96a51
Reviewed-on: https://go-review.googlesource.com/c/60791
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
This adds an endpoint to the trace tool that plots the minimum mutator
utilization curve using information on mark assists and GC pauses from
the trace.
This commit implements a fairly straightforward O(nm) algorithm for
computing the MMU (and tests against an even more direct but slower
algorithm). Future commits will extend and optimize this algorithm.
This should be useful for debugging and understanding mutator
utilization issues like #14951, #14812, #18155. #18534, #21107,
particularly once follow-up CLs add trace cross-referencing.
Change-Id: Ic2866869e7da1e6c56ba3e809abbcb2eb9c4923a
Reviewed-on: https://go-review.googlesource.com/c/60790
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Tobias Klauser [Mon, 5 Nov 2018 11:39:11 +0000 (12:39 +0100)]
runtime/internal/sys: regenerate zgoos_*.go files
zgoos_aix.go is missing GoosJs, the order of GoosAndroid and GoosAix is
mixed up in all files and GoosHurd was added after CL 146023 introduced
GOOS=hurd.
Change-Id: I7e2f5a15645272e9020cfca86e44c364fc072a2b
Reviewed-on: https://go-review.googlesource.com/c/147397
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>