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>
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>
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>
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>
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>
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>
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.
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>
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>
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>
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>
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>
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>
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>
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>
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.
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>
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>
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>
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>
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>
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>
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>
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#26045Fixes#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>
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>
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>
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>
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>
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>
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>
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).
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>
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>
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>
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.
name old time/op new time/op delta
StackCopyPtr-8 118ms ± 1% 106ms ± 2% -9.56% (p=0.000 n=17+18)
StackCopy-8 95.8ms ± 1% 87.0ms ± 3% -9.11% (p=0.000 n=19+20)
StackCopyNoCache-8 135ms ± 2% 139ms ± 1% +3.06% (p=0.000 n=19+18)
During make.bash, the association function used has this return distribution:
percent count return value
53.23% 678797 1
46.74% 596094 0
It is definitely not perfect, but it is pretty good,
and that's all we need.
Change-Id: I2cabb1d26b99c5111bc28f427016a2a5e6c620fd
Reviewed-on: https://go-review.googlesource.com/c/110564
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
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>
fmt's godoc reads:
For compound objects, the elements are printed using these
rules, recursively, laid out like this:
struct: {field0 field1 ...}
array, slice: [elem0 elem1 ...]
maps: map[key1:value1 key2:value2 ...]
pointer to above: &{}, &[], &map[]
That is, a pointer to a struct, array, slice, or map, can be correctly
printed by fmt if the type pointed to can be printed without issues.
vet was only following this rule for pointers to structs, omitting
arrays, slices, and maps. Fix that, and add tests for all the
combinations.
Updates #27672.
Change-Id: Ie61ebe1fffc594184f7b24d7dbf72d7d5de78309
Reviewed-on: https://go-review.googlesource.com/c/147758
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
This optimization is not sound if A, B, or C
might overlap with each other.
Thanks to Michael Munday for pointing this
out during the review of CL 143479.
This reduces the number of times this optimization
triggers during make.bash from 386 to 74.
This is unfortunate, but I don't see an obvious way around it,
short of souping up the disjointness analysis.
name old object-bytes new object-bytes delta
Template 507kB ± 0% 507kB ± 0% +0.13% (p=0.008 n=5+5)
Unicode 225kB ± 0% 225kB ± 0% ~ (all equal)
GoTypes 1.85MB ± 0% 1.85MB ± 0% +0.02% (p=0.008 n=5+5)
Flate 328kB ± 0% 328kB ± 0% ~ (all equal)
GoParser 402kB ± 0% 402kB ± 0% ~ (all equal)
Reflect 1.41MB ± 0% 1.41MB ± 0% ~ (all equal)
Tar 457kB ± 0% 458kB ± 0% +0.20% (p=0.008 n=5+5)
XML 600kB ± 0% 601kB ± 0% +0.03% (p=0.008 n=5+5)
Change-Id: Ida408cb627145ba9faf473a78606f050c2f3f51c
Reviewed-on: https://go-review.googlesource.com/c/145208
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Munday <mike.munday@ibm.com>
This CL add support for converting a returned cursor (presented
to this package as a driver.Rows) and scanning it into a *Rows.
Fixes#28515
Change-Id: Id8191c568dc135af9e5e8555efcd01987708edcb
Reviewed-on: https://go-review.googlesource.com/c/145738
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Cleans things up quite a bit.
There's still a few more, like runtime.cmpstring, which might also
be worth fixing.
Change-Id: Ide18dd621efc129cc686db223f47fa0b044b5580
Reviewed-on: https://go-review.googlesource.com/c/148578
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
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>
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()
}
}
Update #19348
Change-Id: Ifb1ea0dd1db216c3fd5c453c31c3355561fe406f
Reviewed-on: https://go-review.googlesource.com/c/147361
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: David Chase <drchase@google.com>
Add unexported unlinkat, openat, and fstatat calls, so that
the internal/syscall/unix package can use them.
Change-Id: I1df81ecae6427211dd392ec68c9f020fe131a526
Reviewed-on: https://go-review.googlesource.com/c/148457
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
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.
[1] https://blog.cloudflare.com/yet-another-padding-oracle-in-openssl-cbc-ciphersuites/
Change-Id: I365560dfe006513200fa877551ce7afec9115fdf
Reviewed-on: https://go-review.googlesource.com/c/147637
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Miscellaneous additional conversions from raw syscalls
to using their libc equivalent.
Update #17490
Change-Id: If9ab22cc1d676c1f20fb161ebf02b0c28f71585d
Reviewed-on: https://go-review.googlesource.com/c/148257
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
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>
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>
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:
cmd/vendor/golang.org/x/sys/unix/
vendor/golang_org/x/net/route/syscall.go
syscall/bpf_bsd.go
syscall/exec_unix.go
syscall/flock.go
Update #17490
Change-Id: I69ab707811530c26b652b291cadee92f5bf5c1a4
Reviewed-on: https://go-review.googlesource.com/c/141639
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Elias Naur <elias.naur@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
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>
Sync @ fde099a (Oct 26, 2018)
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>
This mainly entails writing compiler output files to a temporary
directory, as well as the corrupted files in TestVersionHandling.
Updates #28387
Change-Id: I6b3619a91fff27011c7d73daa4febd14a6c5c348
Reviewed-on: https://go-review.googlesource.com/c/146119
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
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>
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>
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>
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>
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).
Fixes#26110
Change-Id: Icef7b9549e2049db1df307a0180a3c90a12d7a84
Reviewed-on: https://go-review.googlesource.com/c/147218
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Dead-code eliminating labels is tricky because there might
be gotos that can still reach them.
Bug probably introduced with CL 91056
Fixes#28616
Change-Id: I6680465134e3486dcb658896f5172606cc51b104
Reviewed-on: https://go-review.googlesource.com/c/147817
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Iskander Sharipov <iskander.sharipov@intel.com>
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:
$ (cd $GOPATH/src/golang.org/x/tools && git checkout c76e1ad)
$ cd GOROOT/src/cmd
$ govendor add $(go list -deps golang.org/x/tools/go/analysis/cmd/vet-lite | grep golang.org/x/tools)
$ rm -fr $(find vendor/golang.org/x/tools/ -name testdata)
$ rm $(find vendor/golang.org/x/tools/ -name \*_test.go)
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/...
Change-Id: Iea168111a32ce62f82f9fb706385ca0f368bc869
Reviewed-on: https://go-review.googlesource.com/c/147444
Reviewed-by: Russ Cox <rsc@golang.org>
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>
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: 16da71ad6b
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>
Consider these functions:
func f(a any) int
func g(a any) int
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.
name old alloc/op new alloc/op delta
Template 35.3MB ± 0% 35.4MB ± 0% +0.23% (p=0.008 n=5+5)
Unicode 29.1MB ± 0% 29.1MB ± 0% +0.16% (p=0.008 n=5+5)
GoTypes 122MB ± 0% 122MB ± 0% +0.16% (p=0.008 n=5+5)
Compiler 513MB ± 0% 514MB ± 0% +0.19% (p=0.008 n=5+5)
SSA 1.94GB ± 0% 1.94GB ± 0% +0.01% (p=0.008 n=5+5)
Flate 24.2MB ± 0% 24.2MB ± 0% +0.08% (p=0.008 n=5+5)
GoParser 28.5MB ± 0% 28.5MB ± 0% +0.24% (p=0.008 n=5+5)
Reflect 86.2MB ± 0% 86.3MB ± 0% +0.09% (p=0.008 n=5+5)
Tar 34.9MB ± 0% 34.9MB ± 0% +0.13% (p=0.008 n=5+5)
XML 47.0MB ± 0% 47.1MB ± 0% +0.18% (p=0.008 n=5+5)
[Geo mean] 80.9MB 81.0MB +0.15%
name old allocs/op new allocs/op delta
Template 348k ± 0% 349k ± 0% +0.38% (p=0.008 n=5+5)
Unicode 340k ± 0% 340k ± 0% +0.21% (p=0.008 n=5+5)
GoTypes 1.27M ± 0% 1.28M ± 0% +0.27% (p=0.008 n=5+5)
Compiler 4.90M ± 0% 4.92M ± 0% +0.36% (p=0.008 n=5+5)
SSA 15.3M ± 0% 15.3M ± 0% +0.03% (p=0.008 n=5+5)
Flate 232k ± 0% 233k ± 0% +0.14% (p=0.008 n=5+5)
GoParser 291k ± 0% 292k ± 0% +0.42% (p=0.008 n=5+5)
Reflect 1.05M ± 0% 1.05M ± 0% +0.14% (p=0.008 n=5+5)
Tar 343k ± 0% 344k ± 0% +0.22% (p=0.008 n=5+5)
XML 428k ± 0% 430k ± 0% +0.36% (p=0.008 n=5+5)
[Geo mean] 807k 809k +0.25%
Change-Id: I62134db642206cded01920dc1d8a7da61f7ca0ac
Reviewed-on: https://go-review.googlesource.com/c/147038
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
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).
Change-Id: Ie911ac975a24bd25200a273086dd835ab78b1711
Reviewed-on: https://go-review.googlesource.com/c/147557
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
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>
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.
Fixes#28492
Updates #9679
Change-Id: I6d8b11bad2db8ea8ba69039cfaa914093bdd5132
Reviewed-on: https://go-review.googlesource.com/c/146118
Run-TryBot: Yury Smolsky <yury@smolsky.by>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
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).
Updates #18640.
Updates #23823.
Updates #24939.
Updates #25838.
Updates #28576.
Fixes#27232.
Fixes#27267.
Change-Id: I6c2d10da98bfc6f4f445c755fcaab17fc7b214c5
Reviewed-on: https://go-review.googlesource.com/c/147286
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
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.
Change-Id: I9c2cb7b6798fc536bbd34835da2e888224fd7ed4
Reviewed-on: https://go-review.googlesource.com/c/142958
Reviewed-by: Austin Clements <austin@google.com>
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>
This adds support for computing the quantiles of a mutator utilization
distribution.
Change-Id: Ia8b3ed14bf415c234e2f567360fd1b361d28bd40
Reviewed-on: https://go-review.googlesource.com/c/60799
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>
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>
This will let the trace viewer show specifically when poor utilization
happened and link to specific instances in the trace.
Change-Id: I1f03a0f9d9a7570009bb15762e7b8b6f215e9423
Reviewed-on: https://go-review.googlesource.com/c/60793
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
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>
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>
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>
The kernel on some Samsung S9+ models reports support for arm64 8.1
atomics, but in reality only some of the cores support them. Go
programs scheduled to cores without support will crash with SIGILL.
This change unconditionally disables the optimization on Android.
A better fix is to precisely detect the offending chipset.
Fixes#28431
Change-Id: I35a1273e5660603824d30ebef2ce7e429241bf1f
Reviewed-on: https://go-review.googlesource.com/c/147377
Run-TryBot: Elias Naur <elias.naur@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
At each comparison, we're making a copy of the whole string.
Instead, use unsafe to share the string backing store with a []byte.
It reduces the test time from ~4sec to ~1sec on my machine
(darwin/amd64). Some builders were having much more trouble with this
test (>3min), it may help more there.
Fixes#26174Fixes#28573Fixes#26155
Update #26473
Change-Id: Id5856fd26faf6ff46e763a088f039230556a4116
Reviewed-on: https://go-review.googlesource.com/c/147358
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Was only ever filled with one Etype (TFORW) and only used
in one place. Easier to just check for TFORW.
Change-Id: Icc96da3a22b0af1d7e60bc5841c744916c53341e
Reviewed-on: https://go-review.googlesource.com/c/147285
Reviewed-by: Martin Möhrmann <moehrmann@google.com>
The Fatalf mechanism already prints "compiler internal error:"
when reporting an error. There's no need to have "internal error"
in the error message passed to Fatalf calls. Removed them.
Fixes#28575.
Change-Id: I12b1bea37bc839780f257c27ef9e2005bf334925
Reviewed-on: https://go-review.googlesource.com/c/147287
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Martin Möhrmann <moehrmann@google.com>
Currently, WriteObjFile deduplicates symbols by name. This is a
strange and unexpected place to do this. But, worse, there's no
checking that it's reasonable to deduplicate two symbols, so this
makes it incredibly easy to mask errors involving duplicate symbols.
Dealing with duplicate symbols is better left to the linker. We're
also about to introduce multiple symbols with the same name but
different ABIs/versions, which would make this deduplication more
complicated. We just removed the only part of the compiler that
actually depended on this behavior.
This CL removes symbol deduplication from WriteObjFile, since it is no
longer needed.
For #27539.
Change-Id: I650c550e46e83f95c67cb6c6646f9b2f7f10df30
Reviewed-on: https://go-review.googlesource.com/c/146558
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Currently, liveness produces a distinct obj.LSym for each GC bitmap
for each function. These are then named by content hash and only
ultimately deduplicated by WriteObjFile.
For various reasons (see next commit), we want to remove this
deduplication behavior from WriteObjFile. Furthermore, it's
inefficient to produce these duplicate symbols in the first place.
GC bitmaps are the only source of duplicate symbols in the compiler.
This commit eliminates these duplicate symbols by declaring them in
the Ctxt symbol hash just like every other obj.LSym. As a result, all
GC bitmaps with the same content now refer to the same obj.LSym.
The next commit will remove deduplication from WriteObjFile.
For #27539.
Change-Id: I4f15e3d99530122cdf473b7a838c69ef5f79db59
Reviewed-on: https://go-review.googlesource.com/c/146557
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
32-bit negated logical instructions (BICW, ORNW, EONW) with
constants were mis-encoded, because they were missing in the
cases where we handle 32-bit logical instructions. This CL
adds the missing cases.
Fixes#28548
Change-Id: I3d6acde7d3b72bb7d3d5d00a9df698a72c806ad5
Reviewed-on: https://go-review.googlesource.com/c/147077
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Ben Shi <powerman1st@163.com>
Reviewed-by: Ben Shi <powerman1st@163.com>
Scanning through all path patterns is not necessary, since the
paths do not change frequently. Instead, maintain a sorted list
of path prefixes and return the first match.
name old time/op new time/op delta
ServerMatch-12 134ns ± 3% 17ns ± 4% -86.95% (p=0.000 n=19+20)
Change-Id: I15b4483dc30db413321435ee6815fc9bf2bcc546
Reviewed-on: https://go-review.googlesource.com/c/144937
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Go documentation style for boolean funcs is to say:
// Foo reports whether ...
func Foo() bool
(rather than "returns true if")
This CL also replaces 4 uses of "iff" with the same "reports whether"
wording, which doesn't lose any meaning, and will prevent people from
sending typo fixes when they don't realize it's "if and only if". In
the past I think we've had the typo CLs updated to just say "reports
whether". So do them all at once.
(Inspired by the addition of another "returns true if" in CL 146938
in fd_plan9.go)
Created with:
$ perl -i -npe 's/returns true if/reports whether/' $(git grep -l "returns true iff" | grep -v vendor)
$ perl -i -npe 's/returns true if/reports whether/' $(git grep -l "returns true if" | grep -v vendor)
Change-Id: Ided502237f5ab0d25cb625dbab12529c361a8b9f
Reviewed-on: https://go-review.googlesource.com/c/147037
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The compiler must first be built with the constant enableTrace set
to true (typecheck.go). After that, the -t flag becomes available
which enables tracing output of type-checking functions.
With enableTrace == false, the tracing code becomes dead code
and won't affect the compiler.
Typical output might look like this:
path/y.go:4:6: typecheck 0xc00033e180 DCLTYPE <node DCLTYPE> tc=0
path/y.go:4:6: . typecheck1 0xc00033e180 DCLTYPE <node DCLTYPE> tc=2
path/y.go:4:6: . . typecheck 0xc000331a40 TYPE T tc=1
path/y.go:4:6: . . . typecheck1 0xc000331a40 TYPE T tc=2
path/y.go:4:6: . . . . typecheckdef 0xc000331a40 TYPE T tc=2
path/y.go:4:6: . . . . => 0xc000331a40 TYPE T tc=2 type=*T
path/y.go:4:6: . . . => 0xc000331a40 TYPE T tc=2 type=*T
path/y.go:4:6: . . => 0xc000331a40 TYPE T tc=1 type=*T
path/y.go:4:6: . => 0xc00033e180 DCLTYPE <node DCLTYPE> tc=2 type=<T>
path/y.go:4:6: => 0xc00033e180 DCLTYPE <node DCLTYPE> tc=1 type=<T>
Disabled by default.
Change-Id: Ifd8385290d1cf0d3fc5e8468b2f4ab84e8eff338
Reviewed-on: https://go-review.googlesource.com/c/146782
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Implement a basic TLS 1.3 server handshake, only enabled if explicitly
requested with MaxVersion.
This CL intentionally leaves for future CLs:
- PSK modes and resumption
- client authentication
- compatibility mode ChangeCipherSpecs
- early data skipping
- post-handshake messages
- downgrade protection
- KeyLogWriter support
- TLS_FALLBACK_SCSV processing
It also leaves a few areas up for a wider refactor (maybe in Go 1.13):
- the certificate selection logic can be significantly improved,
including supporting and surfacing signature_algorithms_cert, but
this isn't new in TLS 1.3 (see comment in processClientHello)
- handshake_server_tls13.go can be dried up and broken into more
meaningful, smaller functions, but it felt premature to do before
PSK and client auth support
- the monstrous ClientHello equality check in doHelloRetryRequest can
get both cleaner and more complete with collaboration from the
parsing layer, which can come at the same time as extension
duplicates detection
Updates #9671
Change-Id: Id9db2b6ecc2eea21bf9b59b6d1d9c84a7435151c
Reviewed-on: https://go-review.googlesource.com/c/147017
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
crypto/x509 already supports PSS signatures (with rsaEncryption OID),
and crypto/tls support was added in CL 79736. Advertise support for the
algorithms and accept them as a peer.
Note that this is about PSS signatures from regular RSA public keys.
RSA-PSS only public keys (with RSASSA-PSS OID) are supported in neither
crypto/tls nor crypto/x509. See RFC 8446, Section 4.2.3.
testdata/Server-TLSv12-ClientAuthRequested* got modified because the
CertificateRequest carries the supported signature algorithms.
The net/smtp tests changed because 512 bits keys are too small for PSS.
Based on Peter Wu's CL 79738, who did all the actual work in CL 79736.
Updates #9671
Change-Id: I4a31e9c6e152ff4c50a5c8a274edd610d5fff231
Reviewed-on: https://go-review.googlesource.com/c/146258
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
RFC 8446 recommends using the supported_versions extension to negotiate
lower versions as well, so begin by implementing it to negotiate the
currently supported versions.
Note that pickTLSVersion was incorrectly negotiating the ServerHello
version down on the client. If the server had illegally sent a version
higher than the ClientHello version, the client would have just
downgraded it, hopefully failing later in the handshake.
In TestGetConfigForClient, we were hitting the record version check
because the server would select TLS 1.1, the handshake would fail on the
client which required TLS 1.2, which would then send a TLS 1.0 record
header on its fatal alert (not having negotiated a version), while the
server would expect a TLS 1.1 header at that point. Now, the client gets
to communicate the minimum version through the extension and the
handshake fails on the server.
Updates #9671
Change-Id: Ie33c7124c0c769f62e10baad51cbed745c424e5b
Reviewed-on: https://go-review.googlesource.com/c/146217
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
Note that there is significant code duplication due to extensions with
the same format appearing in different messages in TLS 1.3. This will be
cleaned up in a future refactor once CL 145317 is merged.
Enforcing the presence/absence of each extension in each message is left
to the upper layer, based on both protocol version and extensions
advertised in CH and CR. Duplicated extensions and unknown extensions in
SH, EE, HRR, and CT will be tightened up in a future CL.
The TLS 1.2 CertificateStatus message was restricted to accepting only
type OCSP as any other type (none of which are specified so far) would
have to be negotiated.
Updates #9671
Change-Id: I7c42394c5cc0af01faa84b9b9f25fdc6e7cfbb9e
Reviewed-on: https://go-review.googlesource.com/c/145477
Reviewed-by: Adam Langley <agl@golang.org>
Unlikely to happen in practice, but easy enough to prevent and might
as well do so for completeness.
Fixes#28243.
Change-Id: I848c3af49cb923f088e9490c6a79373e182fad08
Reviewed-on: https://go-review.googlesource.com/c/142719
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Austin Clements <austin@google.com>
There are some OpenGL functions that take more than 15 arguments.
This CL adds Syscall18 to enable to call such functions on Windows
via syscall functions.
Fixes#28434
Change-Id: Ic7e37dda9cadf4516183e98166bfc52844ad2bbe
Reviewed-on: https://go-review.googlesource.com/c/147117
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
This commit skips tests which aren't yet supported on AIX.
nosplit.go is disabled because stackGuardMultiplier is increased for
syscalls.
Change-Id: Ib5ff9a4539c7646bcb6caee159f105ff8a160ad7
Reviewed-on: https://go-review.googlesource.com/c/146939
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
This commit fixes tests which fail on some versions of AIX 7.2 due
to internal bugs.
getsockname isn't working properly with unix networks.
Timezone files aren't returning a correct output.
Change-Id: I4ff15683912be62ab86dfbeeb63b73513404d086
Reviewed-on: https://go-review.googlesource.com/c/146940
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This commit changes poll.PollDescriptor by poll.IsPollDescriptor. This
is needed for OS like AIX which have more than one FD using inside their
netpoll implementation.
Change-Id: I49e12a8d74045c501e19fdd8527cf166a3c64850
Reviewed-on: https://go-review.googlesource.com/c/146938
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
We only need the memory barrier from these stores,
and we only store nil over nil or over a static function value.
The write barrier is unnecessary.
name old time/op new time/op delta
TCP4OneShotTimeout-6 17.0µs ± 0% 17.0µs ± 0% -0.43% (p=0.032 n=5+5)
SetReadDeadline-6 205ns ± 1% 205ns ± 1% ~ (p=0.683 n=5+5)
Update #25729
Change-Id: I66c097a1db7188697ddfc381f31acec053dfed2c
Reviewed-on: https://go-review.googlesource.com/c/146345
Run-TryBot: Dmitry Vyukov <dvyukov@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Follow CL 146020 and enable RemoveAll based on Unlinkat and Openat on
aix.
Updates #27029
Change-Id: I78b34ed671166ee6fa651d5f2025b88548ee6c68
Reviewed-on: https://go-review.googlesource.com/c/146937
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Clément Chigot <clement.chigot@atos.net>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
runtimeNano is slower than nanotime, so pass the duration
to runtime_pollSetDeadline as is. netpoll can add nanotime itself.
Arguably a bit simpler because, say, a negative duration
clearly represents already expired timer, no need to compare to
nanotime again.
This may also fix an obscure corner case when a deadline in past
which happens to be nanotime 0 is confused with no deadline at all,
which are radically different things.
Also don't compute any durations and times if Time is zero
(currently we first compute everything and then reset d back to 0,
which is wasteful).
name old time/op new time/op delta
TCP4OneShotTimeout-6 17.1µs ± 0% 17.0µs ± 0% ~ (p=0.421 n=5+5)
SetReadDeadline-6 230ns ± 0% 205ns ± 1% -10.63% (p=0.008 n=5+5)
Change-Id: I2aad699270289a5b9ead68f5e44ec4ec6d96baa0
Reviewed-on: https://go-review.googlesource.com/c/146344
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Dmitry Vyukov <dvyukov@google.com>
We only need the memory barrier in poll_runtime_pollSetDeadline only
when one of the timers has fired, which is not the expected case.
Memory barrier can be somewhat expensive on some archs,
so execute it only if one of the timers has in fact fired.
name old time/op new time/op delta
TCP4OneShotTimeout-6 17.0µs ± 0% 17.1µs ± 0% +0.35% (p=0.032 n=5+5)
SetReadDeadline-6 232ns ± 0% 230ns ± 0% -1.03% (p=0.000 n=4+5)
Update #25729
Change-Id: Ifce6f505b9e7ba3717bad8f454077a2e94ea6e75
Reviewed-on: https://go-review.googlesource.com/c/146343
Reviewed-by: Ian Lance Taylor <iant@golang.org>
time.now is somewhat expensive (much more expensive than nanotime),
in the common case when Time has monotonic time we don't actually
need to call time.now in Since/Until as we can do calculation
based purely on monotonic times.
name old time/op new time/op delta
TCP4OneShotTimeout-6 17.0µs ± 0% 17.1µs ± 1% ~ (p=0.151 n=5+5)
SetReadDeadline-6 261ns ± 0% 234ns ± 1% -10.35% (p=0.008 n=5+5)
Benchmark that only calls Until:
benchmark old ns/op new ns/op delta
BenchmarkUntil 54.0 29.5 -45.37%
Update #25729
Change-Id: I5ac5af3eb1fe9f583cf79299f10b84501b1a0d7d
Reviewed-on: https://go-review.googlesource.com/c/146341
Run-TryBot: Dmitry Vyukov <dvyukov@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Move startNano from runtime to time package.
In preparation for a subsequent change that speeds up Since and Until.
This also makes code simpler as we have less assembly as the result,
monotonic time handling is better localized in time package.
This changes values returned from nanotime on windows
(it does not account for startNano anymore), current comments state
that it's important, but it's unclear how it can be important
since no other OS does this.
Update #25729
Change-Id: I2275d57b7b5ed8fd0d53eb0f19d55a86136cc555
Reviewed-on: https://go-review.googlesource.com/c/146340
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Currently when netpoll deadline is incrementally prolonged,
we delete and re-add timer each time.
Add modtimer function that does both and use it when we need
to modify an existing netpoll timer to avoid unnecessary lock/unlock.
TCP4OneShotTimeout-6 17.2µs ± 0% 17.0µs ± 0% -0.82% (p=0.008 n=5+5)
SetReadDeadline-6 274ns ± 2% 261ns ± 0% -4.89% (p=0.008 n=5+5)
Update #25729
Change-Id: I08b89dbbc1785dd180e967a37b0aa23b0c4613a8
Reviewed-on: https://go-review.googlesource.com/c/146339
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Currently we always delete both read and write timers and then
add them again. However, if user setups read and write deadline
separately, then we don't need to touch the other one.
name old time/op new time/op delta
TCP4OneShotTimeout-6 17.2µs ± 0% 17.2µs ± 0% ~ (p=0.310 n=5+5)
SetReadDeadline-6 319ns ± 1% 274ns ± 2% -13.94% (p=0.008 n=5+5)
Update #25729
Change-Id: I4c869c3083521de6d0cd6ca99a7609d4dd84b4e4
Reviewed-on: https://go-review.googlesource.com/c/146338
Reviewed-by: Ian Lance Taylor <iant@golang.org>
It's not always necessary to wake timerproc even if we add
a new timer to the top of the heap. Since we don't wake and
reset timerproc when we remove timers, it still can be sleeping
with shorter timeout. It such case it's more profitable to let it
sleep and then update timeout when it wakes on its own rather than
proactively wake it, let it update timeout and go to sleep again.
name old time/op new time/op delta
TCP4OneShotTimeout-6 18.6µs ± 1% 17.2µs ± 0% -7.66% (p=0.008 n=5+5)
SetReadDeadline-6 562ns ± 5% 319ns ± 1% -43.27% (p=0.008 n=5+5)
Update #25729
Change-Id: Iec8eacb8563dbc574a82358b3bac7ac479c16826
Reviewed-on: https://go-review.googlesource.com/c/146337
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This change uses library functions such as bits.RotateLeft32 to
reduce the amount of code needed in the generic implementation.
Since the code is now shorter I've also removed the option to
generate a non-unrolled version of the code.
I've also tried to remove bounds checks where possible to make
the new version performant, however that is not the primary goal
of this change since most architectures have assembly
implementations already.
Assembly performance:
name old speed new speed delta
Hash8Bytes 50.3MB/s ± 1% 59.1MB/s ± 0% +17.63% (p=0.000 n=9+8)
Hash1K 590MB/s ± 0% 597MB/s ± 0% +1.25% (p=0.000 n=9+9)
Hash8K 636MB/s ± 1% 638MB/s ± 1% ~ (p=0.072 n=10+10)
Hash8BytesUnaligned 50.5MB/s ± 0% 59.1MB/s ± 1% +17.09% (p=0.000 n=10+10)
Hash1KUnaligned 589MB/s ± 1% 596MB/s ± 1% +1.23% (p=0.000 n=9+10)
Hash8KUnaligned 638MB/s ± 1% 640MB/s ± 0% +0.35% (p=0.002 n=10+10)
Pure Go performance:
name old speed new speed delta
Hash8Bytes 30.3MB/s ± 1% 42.8MB/s ± 0% +41.20% (p=0.000 n=9+9)
Hash1K 364MB/s ± 4% 394MB/s ± 1% +8.27% (p=0.000 n=10+10)
Hash8K 404MB/s ± 1% 420MB/s ± 0% +4.17% (p=0.000 n=10+9)
Hash8BytesUnaligned 30.3MB/s ± 1% 42.8MB/s ± 1% +40.92% (p=0.000 n=9+10)
Hash1KUnaligned 368MB/s ± 0% 394MB/s ± 0% +7.07% (p=0.000 n=9+9)
Hash8KUnaligned 404MB/s ± 1% 411MB/s ± 3% +1.91% (p=0.026 n=9+10)
Change-Id: I9a91fb52ea8d62964d5351bdf121e9fbc9282852
Reviewed-on: https://go-review.googlesource.com/c/137355
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Stat uses Windows FindFirstFile + CreateFile to gather symlink
information - FindFirstFile determines if file is a symlink,
and then CreateFile follows symlink to capture target details.
Lstat only uses FindFirstFile.
This CL replaces current approach with just a call to CreateFile.
Lstat uses FILE_FLAG_OPEN_REPARSE_POINT flag, that instructs
CreateFile not to follow symlink. Other than that both Stat and
Lstat look the same now. New code is simpler.
CreateFile + GetFileInformationByHandle (unlike FindFirstFile)
does not report reparse tag of a file. I tried to ignore reparse
tag altogether. And it works for symlinks and mount points.
Unfortunately (see https://github.com/moby/moby/issues/37026),
files on deduped disk volumes are reported with
FILE_ATTRIBUTE_REPARSE_POINT attribute set and reparse tag set
to IO_REPARSE_TAG_DEDUP. So, if we ignore reparse tag, Lstat
interprets deduped volume files as symlinks. That is incorrect.
So I had to add GetFileInformationByHandleEx call to gather
reparse tag after calling CreateFile and GetFileInformationByHandle.
Fixes#27225Fixes#27515
Change-Id: If60233bcf18836c147597cc17450d82f3f88c623
Reviewed-on: https://go-review.googlesource.com/c/143578
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Kirill Kolyshkin <kolyshkin@gmail.com>
This CL changes IsAbs to return true for "NUL" and other Windows
reserved filenames (search
https://docs.microsoft.com/en-us/windows/desktop/fileio/naming-a-file
for NUL for details). os.Open("NUL") and os.Stat("NUL") work
regardless of what current directory is, and it is mistake to join
"NUL" with current directory when building full path. Changing
IsAbs("NUL") to return true fixes that mistake.
Fixes#28035
Change-Id: Ife8f8aee48400702613ede8fc6834fd43e6e0f03
Reviewed-on: https://go-review.googlesource.com/c/145220
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Updating each call in place broke when there were multiple cgo calls
used as arguments to another cgo call where some required rewriting.
Instead, rewrite calls to strings via the existing mangling mechanism,
and only substitute the top level call in place.
Fixes#28540
Change-Id: Ifd66f04c205adc4ad6dd5ee8e79e57dce17e86bb
Reviewed-on: https://go-review.googlesource.com/c/146860
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Austin Clements
Filippo Valsorda
Ainar Garipov
Michael Anthony Knyszek
Josh Bleecher Snyder
Nikhil Benesch
Richard Musiol
Martin Möhrmann
Ian Lance Taylor
Alex Brainman
Lynn Boger
Ryan Dahl
Brad Fitzpatrick
Alan Donovan
Daniel Martí
Daniel Theophanes
Keith Randall
Agniva De Sarker
Keith Randall
沈涛
Mikio Hara
diplozoon
Keith Randall
Robert Griesemer
fanzha02
Elias Naur
Hana (Hyang-Ah) Kim
Diogo Pinela
David Chase
Alberto Donizetti
Cherry Zhang
Clément Chigot
Raghavendra Nagaraj
Michael Stapelberg
Yury Smolsky
Muhammad Falak R Wani
Tobias Klauser
Carl Mastrangelo
Matthew Dempsky
Hajime Hoshi
Vladimir Kovpak
Bryan C. Mills
Dmitry Vyukov
Michael Munday