Ian Lance Taylor [Wed, 13 Apr 2022 21:35:15 +0000 (14:35 -0700)]
misc/cgo/test: remove timing dependency from TestParallelSleep
Rename it TestIssue1560 since it no longer sleeps.
For #1560
Fixes #45586
Change-Id: I338eee9de43e871da142143943e9435218438e90
Reviewed-on: https://go-review.googlesource.com/c/go/+/400194 Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Michael Matloob [Tue, 12 Apr 2022 19:17:18 +0000 (15:17 -0400)]
cmd/go: add a better error message when in a module outside workspace
When the user is trying to list or build a package in a module that's
outside of the workspace provide a more clear message hinting to the
user that they can add the module to the workspace using go work use.
Fixes #51604
Change-Id: I1202ecb2f22fd6351bfdec88ed613b8167687fb7
Reviewed-on: https://go-review.googlesource.com/c/go/+/400014
Run-TryBot: Michael Matloob <matloob@golang.org> Reviewed-by: Michael Matloob <matloob@golang.org>
Auto-Submit: Michael Matloob <matloob@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Michael Pratt [Mon, 28 Feb 2022 18:41:33 +0000 (13:41 -0500)]
syscall: move Syscall declarations to OS files
Future CLs will be changing the provenance of these functions. Move the
declarations to the individual OS files now so that future CLs can
change only 1 OS at a time rather than changing all at once.
For #51087
Change-Id: I5e1bca71e670263d8c0faa586c1b6b4de1a114b6
Reviewed-on: https://go-review.googlesource.com/c/go/+/388474
Run-TryBot: Michael Pratt <mpratt@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Michael Pratt [Fri, 25 Feb 2022 19:21:25 +0000 (14:21 -0500)]
cmd/compile: add //go:uintptrkeepalive
This CL exports the existing ir.UintptrKeepAlive via the new directive
//go:uintptrkeepalive. This makes the compiler insert KeepAlives for
pointers converted to uintptr in calls, keeping them alive for the
duration of the call.
//go:uintptrkeepalive requires //go:nosplit, as stack growth can't
handle these arguments (it cannot know which are pointers). We currently
check this on the immediate function, but the actual restriction applies
to all transitive calls.
The existing //go:uintptrescapes is an extension of
//go:uintptrkeepalive which forces pointers to escape to the heap, thus
eliminating the stack growth issue.
This pragma is limited to the standard library.
For #51087
Change-Id: If9a19d484d3561b4219e5539b70c11a3cc09391e
Reviewed-on: https://go-review.googlesource.com/c/go/+/388095
Run-TryBot: Michael Pratt <mpratt@google.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
cmd/go: replace some calls to base.AppendPWD with cmd.Environ
With #50599 implemented, base.AppendPWD is redundant if cmd.Env would
otherwise be nil, and calls to os.Environ followed by base.AppendPWD
can be replaced by a simpler call to cmd.Environ.
Updates #50599.
Change-Id: I94a22e2a4cc8e83c815ac41702ea0b1ee5034ecc
Reviewed-on: https://go-review.googlesource.com/c/go/+/401534
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Change-Id: If3d8391d81e8de869dbb3c857f0570817e8aa440
Reviewed-on: https://go-review.googlesource.com/c/go/+/400914 Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Wayne Zuo <wdvxdr@golangcn.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Change-Id: I5698c7576a0f39ae62de7bea64286ac8e578d421
Reviewed-on: https://go-review.googlesource.com/c/go/+/400916 Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Wayne Zuo <wdvxdr@golangcn.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Change-Id: I5ccbcea4c53658136b25ca608faec19eeec2e908
Reviewed-on: https://go-review.googlesource.com/c/go/+/400915 Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Wayne Zuo <wdvxdr@golangcn.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Roland Shoemaker [Tue, 19 Apr 2022 19:05:10 +0000 (12:05 -0700)]
crypto/x509: use SAN when comparing certs during path building
Per RFC 4158 Section 2.4.2, when we are discarding candidate
certificates during path building, use the SANs as well as subject and
public key when checking whether a certificate is already present in
the built path. This supports the case where a certificate in the chain
(typically a leaf) has the exact same subject and public key as another
certificate in the chain (typically its parent) but has SANs which don't
match.
Change-Id: I212c234e94a1f6afbe9691e4a3ba257461db3a7e
Reviewed-on: https://go-review.googlesource.com/c/go/+/401115 Reviewed-by: Roland Shoemaker <roland@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Damien Neil <dneil@google.com>
os/exec: preserve original order of entries in dedupEnv
Once #50599 is implemented, the entries will be observable via the
Environ method. I find it confusing for later entries in the list to
jump arbitrarily far forward based on entries for the same key that no
longer exist.
This also fixes the deduplication logic for the degenerate Windows
keys observed in #49886, which were previously deduplicated as empty
keys.
(It does not do anything about the even-more-degenerate keys observed
in #52436.)
Andrew Gerrand [Tue, 19 Apr 2022 05:11:37 +0000 (15:11 +1000)]
cmd/go/internal/test: wrap os.Stdout always
There is an issue where 'go test' will hang after the tests complete if
a test starts a sub-process that does not exit (see #24050).
However, go test only exhibits that behavior when a package name is
explicitly passed as an argument. If 'go test' is invoked without any
package arguments then the package in the working directory is assumed,
however in that case (and only that case) os.Stdout is used as the test
process's cmd.Stdout, which does *not* cause 'go test' wait for the
sub-process to exit (see #23019).
This change wraps os.Stdout in an io.Writer struct in this case, hiding
the *os.File from the os/exec package, causing cmd.Wait to always wait
for the full output from the test process and any of its sub-processes.
In other words, this makes 'go test' exhibit the same behavior as
'go test .' (or 'go test ./...' and so on).
Update #23019
Update #24050
Change-Id: Ica09bf156f3b017f9a31aad91ed0f16a7837195b
Reviewed-on: https://go-review.googlesource.com/c/go/+/400877 Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Andrew Gerrand <adg@golang.org>
Auto-Submit: Andrew Gerrand <adg@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Andrew Gerrand <adg@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Support fastrand64 in the runtime, although fastrand uses wyrand to generate 64-bit random number, it still returns uint32. In some cases, we need to generate a 64-bit random number, the new API would be faster and easier to use, and at least we can use the new function in these places:
name time/op
Fastrand-16 0.09ns ± 5%
Fastrand64-16 0.09ns ± 6%
Change-Id: Ibb97378c7ca59bc7dc15535d4872fa58ea112e6a
Reviewed-on: https://go-review.googlesource.com/c/go/+/400734 Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
Auto-Submit: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Phil Kulin [Wed, 20 Apr 2022 14:13:29 +0000 (14:13 +0000)]
reflect: remove unused overflowPad variable
overflowPad variable in bucketOf function is a holdover from a NaCl port
and never used now.
Change-Id: Ib68fdb054e1b6a655ffbfd34521a3f8773a22694
GitHub-Last-Rev: f281be9c115a87605fd28b39c0b09eed54cc774a
GitHub-Pull-Request: golang/go#52449
Reviewed-on: https://go-review.googlesource.com/c/go/+/401274
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Auto-Submit: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Robert Griesemer [Wed, 20 Apr 2022 19:05:30 +0000 (12:05 -0700)]
cmd/compile/internal/syntax: correct an error string
When we have an error in a function type used in an expression
we don't know until we see an opening { whether we have a function
literal or a function type. Use "function type" as context because
that's always correct in the specific error message.
Change-Id: I9aad8fcddf31ae53daa53cebd2c2001f08eabde0
Reviewed-on: https://go-review.googlesource.com/c/go/+/401316 Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Robert Griesemer <gri@google.com> Reviewed-by: Robert Griesemer <gri@google.com>
This test forces GOARCH to amd64, but currently uses the default GOOS.
This works on every OS that supports amd64, which is every OS we
support except AIX. Hence, on AIX this fails with an unsupported
GOOS/GOARCH combination.
go/internal/srcimporter: add context to cgo errors
An error message like "could not import os/user (exit status 1)"
(observed in https://go.dev/issue/52407) is fairly inscrutable.
On the other hand, srcimporter doesn't report errors with quite enough
structure to dump the entire stderr output from 'go tool cgo' without
potentially overwhelming the caller. Here, we split the difference by
describing which command failed but not printing the output of that
command.
For #52407, that would at least provide a stronger clue connecting
to #52408.
Robert Griesemer [Tue, 15 Mar 2022 17:15:53 +0000 (10:15 -0700)]
spec: clarify rules for type set construction of an interface
Be explicit that we always mean non-interface types when we
talk about sets of types.
Also, clarify that the quantification "all non-interface types"
means all such types in all possible programs, not just the
current program.
Per suggestion from Philip Wadler.
Change-Id: Ibc7b5823164e547bfcee85d4e523e58c7c27ac8a
Reviewed-on: https://go-review.googlesource.com/c/go/+/398655 Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Robert Griesemer <gri@google.com>
Robert Griesemer [Tue, 19 Apr 2022 20:46:15 +0000 (13:46 -0700)]
cmd/compile/internal/types2: use correct value of iota
Fixes #52438.
Change-Id: I5cbf8c448dba037e9e0c5fe8f209401d6bf7d43f
Reviewed-on: https://go-review.googlesource.com/c/go/+/401134 Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Robert Griesemer <gri@google.com>
Robert Griesemer [Tue, 19 Apr 2022 01:43:22 +0000 (18:43 -0700)]
cmd/compile/internal/types2: don't crash in overflow check
Be careful before accessing an operand's expr field (which may
be nil in some rare cases).
While at it, factor out position information so that it's only
computed when there's an error, which is almost never.
In go/types, remove an unnecessary argument to Checker.overflow.
The code is otherwise ok as it's structured slightly differently
due to the way positions are recorded in AST nodes.
Fixes #52401.
Change-Id: I447ebd9bb0c33eb6bff5e7b4d5aee37ceb0a4b14
Reviewed-on: https://go-review.googlesource.com/c/go/+/400798 Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Robert Griesemer <gri@google.com>
Robert Griesemer [Tue, 19 Apr 2022 00:19:47 +0000 (17:19 -0700)]
cmd/compile/internal/types2: permit parentheses around types in interfaces
Before Go 1.18, an embedded type name in an interface could not be
parenthesized. With generalized embedding of types in interfaces,
where one might write ~(chan<- int) for clarity (making clear that
the ~ applies to the entire channel type), it also makes sense to
permit (chan<- int), or (int) for that matter.
Adjust the parser accordingly to match the spec.
(go/types already accepts the notation as specified by the spec.)
Fixes #52391.
Change-Id: Ifdd9a199c5ccc3473b2dac40dbca31d2df10d12b
Reviewed-on: https://go-review.googlesource.com/c/go/+/400797 Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Robert Griesemer <gri@golang.org> Reviewed-by: Robert Griesemer <gri@google.com>
Jason A. Donenfeld [Fri, 4 Mar 2022 20:03:53 +0000 (21:03 +0100)]
crypto/rand: remove all buffering
The kernel's RNG is fast enough, and buffering means taking locks, which
we don't want to do. So just remove all buffering. This also means the
randomness we get is "fresher". That also means we don't need any
locking, making this potentially faster if multiple cores are hitting
GetRandom() at the same time on newer Linuxes.
Also, change the build tag of the tests to be 'unix' instead of
enumerating them.
Change-Id: Ia773fab768270d2aa20c0649f4171c5326b71d02
Reviewed-on: https://go-review.googlesource.com/c/go/+/390038 Reviewed-by: Filippo Valsorda <valsorda@google.com>
Run-TryBot: Jason Donenfeld <Jason@zx2c4.com>
Auto-Submit: Jason Donenfeld <Jason@zx2c4.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Jason A. Donenfeld [Fri, 10 Dec 2021 16:23:08 +0000 (17:23 +0100)]
crypto/rand: batch and buffer calls to getrandom/getentropy
We're using bufio to batch reads of /dev/urandom to 4k, but we weren't
doing the same on newer platforms with getrandom/getentropy. Since the
overhead is the same for these -- one syscall -- we should batch reads
of these into the same 4k buffer. While we're at it, we can simplify a
lot of the constant dispersal.
This also adds a new test case to make sure the buffering works as
desired.
Change-Id: I7297d4aa795c00712e6484b841cef8650c2be4ef
Reviewed-on: https://go-review.googlesource.com/c/go/+/370894 Reviewed-by: Filippo Valsorda <valsorda@google.com>
Run-TryBot: Jason Donenfeld <Jason@zx2c4.com>
Auto-Submit: Jason Donenfeld <Jason@zx2c4.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
cmd/compile: more negation related generic SSA rewrite rules
The x + (-y) => x - y rule is hitted 75 times while building stage 3 and tools
and make the linux-amd64 go binary 0.007% smaller.
It transform:
NEG AX
ADD BX, AX
Into:
SUB BX, AX
Which is 2X faster (assuming this assembly in a vacum).
The x ^ (-1) => ^x rule is not hitted in the toolchain.
It transforms:
XOR $-1, AX
Into:
NOT AX
Which is more compact as it doesn't encode the immediate.
Cache usage aside, this does not affect performance
(assuming this assembly in a vacum).
On my ryzen 3600, with some surrouding code, this randomly might be 2X faster,
I guess this has to do with loading the immediate into a temporary register.
Combined to an other rule that already exists it also rewrite manual two's
complement negation from:
XOR $-1, AX
INC AX
Into:
NEG AX
Which is 2X faster.
The other rules just eliminates similar trivial cases and help constants
folding.
This should generalise to other architectures.
Change-Id: Ia1e51b340622e7ed88e5d856f3b1aa424aa039de
GitHub-Last-Rev: ce35ff2efdd8911f1e812806ec41a41e8cabc4c7
GitHub-Pull-Request: golang/go#52395
Reviewed-on: https://go-review.googlesource.com/c/go/+/400714 Reviewed-by: Keith Randall <khr@golang.org>
Auto-Submit: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Keith Randall [Tue, 19 Apr 2022 17:33:46 +0000 (10:33 -0700)]
reflect: adjust MapRange allocation test for noopt builder, take 2
Change-Id: If2887f84b3d14fac3c059fc5bad4186ec9d69d0f
Reviewed-on: https://go-review.googlesource.com/c/go/+/401077 Reviewed-by: Joseph Tsai <joetsai@digital-static.net>
Run-TryBot: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com>
Auto-Submit: Keith Randall <khr@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
doc/go1.19: move the description of the runtime.GOROOT change from 'cmd/go' to 'runtime'
Even though the change in the behavior of 'runtime.GOROOT' was
not actually due to a change in the runtime package proper, I
suspect that users who notice it will look for the release note
in that section, not the 'cmd/go' section.
Fixes #51461.
Change-Id: I271752968d4152a7fdf3e170537e3072bf87ce86
Reviewed-on: https://go-review.googlesource.com/c/go/+/400814 Reviewed-by: Ian Lance Taylor <iant@google.com>
Daniel Martí [Tue, 12 Apr 2022 06:11:28 +0000 (07:11 +0100)]
io/ioutil: provide an equivalent for the deprecated ReadDir
All APIs in the now-deprecated io/ioutil package have a direct
replacement in either the io or os package with the same signature,
with the notable exception of ioutil.ReadDir, as os.ReadDir has a
slightly different signature with fs.DirEntry rather than fs.FileInfo.
New code can easily make use of []fs.DirEntry directly,
but existing code may need to continue using []fs.FileInfo for backwards
compatibility reasons. For instance, I had a bit of code that exposed
the slice as a public API, like:
return ioutil.ReadDir(name)
It took me a couple of minutes to figure out what the exact equivalent
in terms of os.ReadDir would be, and a code sample would have helped.
Add one for future reference.
For #42026.
For #51927.
Change-Id: I76d46cd7d68fc609c873821755fdcfc299ffd56c
Reviewed-on: https://go-review.googlesource.com/c/go/+/399854 Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
cmd/link: faster algorithm for nosplit stack checking, better errors
The linker performs a global analysis of all nosplit call chains to
check they fit in the stack space ensured by splittable functions.
That analysis has two problems right now:
1. It's inefficient. It performs a top-down analysis, starting with
every nosplit function and the nosplit stack limit and walking *down*
the call graph to compute how much stack remains at every call. As a
result, it visits the same functions over and over, often with
different remaining stack depths. This approach is historical: this
check was originally written in C and this approach avoided the need
for any interesting data structures.
2. If some call chain is over the limit, it only reports a single call
chain. As a result, if the check does fail, you often wind up playing
whack-a-mole by guessing where the problem is in the one chain, trying
to reduce the stack size, and then seeing if the link works or reports
a different path.
This CL completely rewrites the nosplit stack check. It now uses a
bottom-up analysis, computing the maximum stack height required by
every function's call tree. This visits every function exactly once,
making it much more efficient. It uses slightly more heap space for
intermediate storage, but still very little in the scheme of the
overall link. For example, when linking cmd/go, the new algorithm
virtually eliminates the time spent in this pass, and reduces overall
link time:
│ before │ after │
│ sec/op │ sec/op vs base │
Dostkcheck 7.926m ± 4% 1.831m ± 6% -76.90% (p=0.000 n=20)
TotalTime 301.3m ± 1% 296.4m ± 3% -1.62% (p=0.040 n=20)
│ before │ after │
│ B/op │ B/op vs base │
Dostkcheck 40.00Ki ± 0% 212.15Ki ± 0% +430.37% (p=0.000 n=20)
Most of this time is spent analyzing the runtime, so for larger
binaries, the total time saved is roughly the same, and proportionally
less of the overall link.
If the new implementation finds an error, it redoes the analysis,
switching to preferring quality of error reporting over performance.
For error reporting, it computes stack depths top-down (like the old
algorithm), and reports *all* paths that are over the stack limit,
presented as a tree for compactness. For example, this is the output
from a simple test case from test/nosplit with two over-limit paths
from f1:
test/nosplit: apply stack limit adjustment in the right place
The nosplit test was originally written when the stack limit was a
mere 128 bytes. Now it's much larger, but rather than rewriting all of
the tests, we apply a hack to just add the extra space into the stack
frames of the existing tests.
Unfortunately, we add it in the wrong place. The extra space should be
added just once per chain of nosplit functions, but instead we add it
to every frame that appears first on a line in the test's little
script language. This means that for tests like
we add 672 bytes to *every* frame, meaning that we wind up way over
the stack limit by the end of the stanza, rather than just a little as
originally intended.
Fix this by instead adding the extra space to the first nosplit
function in a stanza. This isn't perfect either, since we could have a
nosplit -> split -> nosplit chain, but it's the best we can do without
a graph analysis.
internal/sys: add LR and fixed frame size to sys.Arch
Storing this information in the Arch eliminates some code duplication
between the compiler and linker. This information is entirely
determined by the Arch, so the current approach of attaching it to an
entire Ctxt is a little silly. This will also make it easier to use
this information from tests.
The next CL will be a rote refactoring to eliminate the
Ctxt.FixedFrameSize methods.
Paul E. Murphy [Mon, 3 May 2021 16:00:54 +0000 (11:00 -0500)]
cmd/link: use TOC-relative trampolines on PPC64 when needed
When linking a PIE binary with the internal linker, TOC relative
relocations need to be generated. Update trampolines to indirect
call using R12 to more closely match the AIX/ELFv2 regardless of
buildmode, and work with position-indepdent code.
Likewise, update the check for offseting R_CALLPOWER relocs to
make a local call. It should be checking ldr.AttrExternal, not
ldr.IsExternal. This offset should not be adjusted for external
(non-go) object files, it is handled when ELF reloc are translated
into go relocs.
And, update trampoline tests to verify these are generated correctly
and produce a working binary using -buildmode=pie on ppc64le.
Fixes #52337
Change-Id: I8a2dea06c3237bdf0e87888b56a17b6c4c99a7de
Reviewed-on: https://go-review.googlesource.com/c/go/+/400234 Reviewed-by: Than McIntosh <thanm@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Paul Murphy <murp@ibm.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Joe Tsai [Sun, 17 Apr 2022 03:23:28 +0000 (20:23 -0700)]
reflect: make Value.MapRange inlineable
This allows the caller to decide whether MapIter should be
stack allocated or heap allocated based on whether it escapes.
In most cases, it does not escape and thus removes the utility
of MapIter.Reset (#46293). In fact, use of sync.Pool with MapIter
and calling MapIter.Reset is likely to be slower.
Change-Id: Ic93e7d39e5dd4c83e7fca9e0bdfbbcd70777f0e1
Reviewed-on: https://go-review.googlesource.com/c/go/+/400675 Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
net/http: eliminate arbitrary timeouts in TestServerRequestContextCancel_ConnClose
These timeouts are empirically sometimes (but rarely) too short on
slower builders, and at any rate if this test fails “for real” we'll
want a goroutine dump in order to debug it anyway. A goroutine dump is
exactly what we get if we let the test time out on its own.
cmd/compile: fix missing source information in ssa view
Endlineno is lost when we call "genericSubst" to create the new
instantiation of the generic function. This will cause "readFuncLines"
to fail to read the target function.
To fix this issue, as @mdempsky pointed out, add the line in
cmd/compile/internal/noder/stencil.go:
newf.Endlineno = gf.Endlineno
Fixes #51988
Change-Id: Ib408e4ed0ceb68df8dedda4fb551309e8385aada
Reviewed-on: https://go-review.googlesource.com/c/go/+/399057 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Austin Clements <austin@google.com> Reviewed-by: Than McIntosh <thanm@google.com>
This improves performance for memclr for sizes >= 64 and < 512 by
unrolling the loop to clear 64 bytes at a time, whereas before it was
doing 32 bytes.
Joe Tsai [Sat, 16 Apr 2022 01:09:48 +0000 (18:09 -0700)]
reflect: make Value.Type inlineable
This allows the result of Type to be computed much faster.
Performance:
old new delta
1.76ns 0.66ns -62.27%
Change-Id: Ie007fd175aaa41b2f67c71fa2a34ab8d292dd0e0
Reviewed-on: https://go-review.googlesource.com/c/go/+/400335 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Daniel Martí <mvdan@mvdan.cc> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Philippe Antoine [Fri, 15 Apr 2022 11:36:32 +0000 (11:36 +0000)]
bytes: explode checks for n too large
As is already done in strings package.
Change-Id: Ia45e6443ddf6beac5e70a1cc493119030e173139
GitHub-Last-Rev: 1174c250350f31eced1513169d62a8a3e679dcf6
GitHub-Pull-Request: golang/go#52348
Reviewed-on: https://go-review.googlesource.com/c/go/+/400239
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
go/doc: fix incorrect identifier parsing in comments
This code was trying to iterate codepoints, but didn't reslice the string,
so it was reading the first codepoint over and over, if the string length was
not a multiple of the first codepoint length, this would cause to overshoot
past the end of the string.
This was a latent bug introduced in CL 384265 but was revealed to
Ngolo-fuzzing in OSS-Fuzz in CL 397277.
Fixes #52353
Change-Id: I13f0352e6ad13a42878927f3b1c18c58360dd40c
GitHub-Last-Rev: 424f6cfad1bc7d66314911e6b4b4ce6751330435
GitHub-Pull-Request: golang/go#52356
Reviewed-on: https://go-review.googlesource.com/c/go/+/400240 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Ian Lance Taylor [Fri, 15 Apr 2022 20:46:00 +0000 (13:46 -0700)]
runtime: don't block preemption signal in new M's or ensureSigM
No test because we already have a test in the syscall package.
The issue reports 1 failure per 100,000 iterations, which is rare enough
that our builders won't catch the problem.
Fixes #52226
Change-Id: I17633ff6cf676b6d575356186dce42cdacad0746
Reviewed-on: https://go-review.googlesource.com/c/go/+/400315
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Ian Lance Taylor [Thu, 14 Apr 2022 23:25:43 +0000 (16:25 -0700)]
debug/pe: read string table in 10M chunks
No separate test because this makes no difference for valid PE files.
Fixes #52350
Change-Id: I2aa011a4e8b34cb08052222e94c52627ebe99fbf
Reviewed-on: https://go-review.googlesource.com/c/go/+/400378
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Than McIntosh <thanm@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
João Penteado [Tue, 30 Nov 2021 18:59:41 +0000 (18:59 +0000)]
net/http: optimize StatusText implementation
The current implementation, although more succinct, relies on a runtime
lookup to a "constant" unexported map (which also needs to be
initialized at runtime).
The proposed implementation is able to be optimized by the compiler at
build-time, resulting in *much* more efficient instructions.
Additionally, unused string literals may even be removed altogether
from the generated binary in some cases.
This change is fully backwards-compatible behavior-wise with the
existing implementation.
Change-Id: I36450320aacff5b322195820552f2831d4fecd52
GitHub-Last-Rev: e2058f132ef7a193529d4b0e84329ac93e5d1dcb
GitHub-Pull-Request: golang/go#49811
Reviewed-on: https://go-review.googlesource.com/c/go/+/367201
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Damien Neil <dneil@google.com>
Roland Shoemaker [Fri, 15 Apr 2022 00:57:22 +0000 (17:57 -0700)]
crypto/x509: don't create certs with negative serials
Refuse to create certificates with negative serial numbers, as they
are explicitly disallowed by RFC 5280.
We still allow parsing certificates with negative serial numbers,
because in the past there were buggy CA implementations which would
produce them (although there are currently *no* trusted certificates
that have this issue). We may want to revisit this decision if we can
find metrics about the prevalence of this issue in enterprise settings.
Change-Id: I131262008db99b6354f542f335abc68775a2d6d0
Reviewed-on: https://go-review.googlesource.com/c/go/+/400494 Reviewed-by: Damien Neil <dneil@google.com> Reviewed-by: Roland Shoemaker <roland@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
Auto-Submit: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Archana R [Fri, 1 Apr 2022 17:05:07 +0000 (12:05 -0500)]
internal/bytealg: optimize indexbyte function for ppc64le/power9
Added specific code For POWER9 that does not need prealignment prior
to load vector. Optimized vector loop to jump out as soon as there is
a match instead of accumulating matches for 4 indices and then processing
the same. For small input size 10, the caller function dominates
performance.
Bobby Powers [Fri, 8 Apr 2022 19:21:33 +0000 (12:21 -0700)]
net/http: remove cloneURL call in WithContext
Fixes #52239
Change-Id: I08b75e613e3c976855e39d01a6757d94e4207bf8
Reviewed-on: https://go-review.googlesource.com/c/go/+/399155
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Damien Neil <dneil@google.com>
Run-TryBot: Damien Neil <dneil@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Joe Tsai [Wed, 13 Apr 2022 20:21:30 +0000 (13:21 -0700)]
encoding/binary: add AppendVarint AppendUvarint
This adds a straight-forward implementation of the functionality.
A more performant version could be added that unrolls the loop
as is done in google.golang.org/protobuf/encoding/protowire,
but usages that demand high performance can use that package instead.
Fixes #51644
Change-Id: I9d3b615a60cdff47e5200e7e5d2276adf4c93783
Reviewed-on: https://go-review.googlesource.com/c/go/+/400176 Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Joseph Tsai <joetsai@digital-static.net>
TryBot-Result: Gopher Robot <gobot@golang.org>
runtime: don't discard value from panic while panicking
In issue #17671, there are a endless loop if printing
the panic value panics, CL 30358 has fixed that.
As issue #52257 pointed out, above change should not
discard the value from panic while panicking.
With this CL, when we recover from a panic in error.Error()
or stringer.String(), and the recovered value is string,
then we can print it normally.
Fixes #52257
Change-Id: Icfcc4a1a390635de405eea04904b4607ae9e3055
Reviewed-on: https://go-review.googlesource.com/c/go/+/399874
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Keith Randall [Thu, 14 Apr 2022 22:04:34 +0000 (15:04 -0700)]
cmd/compile: turn jump tables off with -N
The noopt builder is broken, because with -N we get two OpSB opcodes
(one for the function as a whole, one introduced by the jumptable
rewrite rule), and they fight each other for a register.
Without -N, the two OpSB get CSEd, so optimized builds are ok.
Maybe we fix regalloc to deal with this case, but it's simpler
(and maybe more correct?) to disable jump tables with -N.
Change-Id: I75c87f12de6262955d1df787f47c53de976f8a5f
Reviewed-on: https://go-review.googlesource.com/c/go/+/400455
Run-TryBot: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com>
Auto-Submit: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Keith Randall [Mon, 4 Oct 2021 19:17:46 +0000 (12:17 -0700)]
cmd/compile: implement jump tables
Performance is kind of hard to exactly quantify.
One big difference between jump tables and the old binary search
scheme is that there's only 1 branch statement instead of O(n) of
them. That can be both a blessing and a curse, and can make evaluating
jump tables very hard to do.
The single branch can become a choke point for the hardware branch
predictor. A branch table jump must fit all of its state in a single
branch predictor entry (technically, a branch target predictor entry).
With binary search that predictor state can be spread among lots of
entries. In cases where the case selection is repetitive and thus
predictable, binary search can perform better.
The big win for a jump table is that it doesn't consume so much of the
branch predictor's resources. But that benefit is essentially never
observed in microbenchmarks, because the branch predictor can easily
keep state for all the binary search branches in a microbenchmark. So
that benefit is really hard to measure.
So predictable switch microbenchmarks are ~useless - they will almost
always favor the binary search scheme. Fully unpredictable switch
microbenchmarks are better, as they aren't lying to us quite so
much. In a perfectly unpredictable situation, a jump table will expect
to incur 1-1/N branch mispredicts, where a binary search would incur
lg(N)/2 of them. That makes the crossover point at about N=4. But of
course switches in real programs are seldom fully unpredictable, so
we'll use a higher crossover point.
Beyond the branch predictor, jump tables tend to execute more
instructions per switch but have no additional instructions per case,
which also argues for a larger crossover.
As far as code size goes, with this CL cmd/go has a slightly smaller
code segment and a slightly larger overall size (from the jump tables
themselves which live in the data segment).
This is a case where some FDO (feedback-directed optimization) would
be really nice to have. #28262
Some large-program benchmarks might help make the case for this
CL. Especially if we can turn on branch mispredict counters so we can
see how much using jump tables can free up branch prediction resources
that can be gainfully used elsewhere in the program.
Joe Tsai [Wed, 13 Apr 2022 20:31:24 +0000 (13:31 -0700)]
math: improve documentation of Copysign
Name the arguments in a way that is more self-describing.
Many code editor tools show a snippet of the function and
its arguments. However, "x" and "y" are not helpful in determining
which is the sign and which is the magnitude,
short of reading the documentation itself.
Name the sign argument as "sign" to be explicit.
This follows the same naming convention as IsInf.
Change-Id: Ie3055009e475f96c92d5ea7bfe9828eed908c78b
Reviewed-on: https://go-review.googlesource.com/c/go/+/400177
Run-TryBot: Joseph Tsai <joetsai@digital-static.net>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
zhangyunhao [Tue, 14 Dec 2021 05:52:05 +0000 (13:52 +0800)]
sort: use pdqsort
- Across all benchmarks, pdqsort is never significantly slower than the previous algorithm.
- In common patterns, pdqsort is often faster (i.e. 10x faster in sorted slices).
The pdqsort is described at https://arxiv.org/pdf/2106.05123.pdf
This CL is inspired by both C++ implementation and Rust implementation.
- C++ implementation: https://github.com/orlp/pdqsort
- Rust implementation: https://docs.rs/pdqsort/latest/pdqsort/
Roland Shoemaker [Wed, 13 Apr 2022 04:22:22 +0000 (21:22 -0700)]
crypto/x509: omit empty extensions SEQUENCE
In CreateCertificate, if there are no extensions, don't include the
extensions SEQUENCE in the encoded certificate.
Why, you might ask, does the encoding/asn1 tag 'optional' not do
the same thing as 'omitempty'? Good question, no clue, fixing that
would probably break things in horrific ways.
Fixes #52319
Change-Id: I84fdd5ff3e4e0b0a59e3bf86e7439753b1e1477b
Reviewed-on: https://go-review.googlesource.com/c/go/+/399827
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Damien Neil <dneil@google.com> Reviewed-by: Roland Shoemaker <roland@golang.org>
Auto-Submit: Roland Shoemaker <roland@golang.org>
Change-Id: I400110c33e69decf133fe9c4b582a450b7258b39
Reviewed-on: https://go-review.googlesource.com/c/go/+/399514 Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Cherry Mui [Mon, 11 Apr 2022 22:57:44 +0000 (18:57 -0400)]
cmd/link: mangle symbol ABI name for linker-generated symbols
The ABI mangling code skips symbols that are not loaded from Go
objects. Usually that is fine, as other symbols don't need name
mangling. But trampolines are linker generated and have the same
symbol version (ABI) as the underlying symbol. We need to avoid
symbol name collisions for trampolines, such as a trampoline to
f<ABI0> and a trampoline to f<ABIInternal>. We could explicitly
incorportate the ABI into the trampoline name. But as we already
have the name mangling scheme we could just use that.
The original code excludes external symbols probably because
symbols from C object don't need mangling. But a C symbol and a
Go symbol shouldn't have same name, and so the condition won't
apply.
Also exclude static symbols as they don't need mangling.
Change-Id: I298eb1d64bc0c3da0154f0146b95c4d26ca2f47a
Reviewed-on: https://go-review.googlesource.com/c/go/+/399894
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Than McIntosh <thanm@google.com>
Keith Randall [Tue, 12 Apr 2022 20:34:42 +0000 (13:34 -0700)]
cmd/link: don't sort pclntab entries
They are already in a good order. The sort here does nothing, as
all the SymValues are 0. Sorting just arbitrarily permutes items
because everything is equal and the sort isn't stable.
Not sure why the ordering of these symbols matter. That ordering was
added in CL 243223.
Change-Id: Iee153394afdb39387701cfe0375bc022cf4bd489
Reviewed-on: https://go-review.googlesource.com/c/go/+/399540
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Keith Randall <khr@google.com>
Auto-Submit: Keith Randall <khr@google.com>
check_testdata/check_testdata.go used the encoding of the corpus entry
file, rather than the input string itself, when checking the expected
size of the minimized value. Instead, use the actual byte length, which
should bypass flakiness.
While we are here, use somewhat simpler fuzz targets, that use byte
slices rather than strings, and only execute the targets when fuzzing (
skipping the 'run' phase.)
Fixes #52285
Change-Id: I48c3780934891eec4a9e38d93abb4666091cb580
Reviewed-on: https://go-review.googlesource.com/c/go/+/399814
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Bryan Mills <bcmills@google.com>
Archana R [Wed, 9 Feb 2022 13:37:12 +0000 (07:37 -0600)]
math/big: Implement shlVU and shrVU in ASM for PPC64
Currently the shift left and shift right functions are coded in .go
on PPC64. Implementing them in ASM just like AMD and ARM results in
overall speedup of shift benchmarks on POWER8/9/10.
Damien Neil [Tue, 12 Apr 2022 20:38:17 +0000 (13:38 -0700)]
syscall: check correct group in Faccessat
The Faccessat call checks the user, group, or other permission bits of a
file to see if the calling process can access it. The test to see if the
group permissions should be used was made with the wrong group id, using
the process's group id rather than the file's group id. Fix this to use
the correct group id.
No test since we cannot easily change file permissions when not running
as root and the test is meaningless if running as root.
For #52313
Change-Id: I4e2c84754b0af7830b40fd15dedcbc58374d75ee
Reviewed-on: https://go-review.googlesource.com/c/go/+/399539 Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
cmd/go: allow '-buildvcs=auto' and treat it as the default
When we added VCS stamping in the Go 1.18 release, we defaulted to
-buildvcs=true, on the theory that most folks will actually want VCS
information stamped.
We also made -buildvcs=true error out if a VCS directory is found and
no VCS tool is available, on the theory that a user who builds with
'-buildvcs=true' will be very surprised if the VCS metadata is
silently missing.
However, that causes a problem for CI environments that don't have the
appropriate VCS tool installed. (And we know that's a common situation
because we're in that situation ourselves — see #46693!)
The new '-buildvcs=auto' setting provides a middle ground: it stamps
VCS information by default when the tool is present (and reports
explicit errors if the tool errors out), but omits the metadata
when the tool isn't present at all.
Wayne Zuo [Fri, 8 Apr 2022 15:44:40 +0000 (23:44 +0800)]
cmd/compile: always write fun[0] in incomplete itab
runtime.getitab need filled fun[0] to identify whether
implemented the interface.
Fixes #51700
Fixes #52228
Change-Id: I0173b98f4e1b45e3a0183a5b60229d289140d1e6
Reviewed-on: https://go-review.googlesource.com/c/go/+/399058 Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
Auto-Submit: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: David Chase <drchase@google.com>
tenkoh [Wed, 30 Mar 2022 06:30:19 +0000 (15:30 +0900)]
cmd/go: open correct path when loading embeds from root directory
The existing implementation of `load.resolveEmbed`
uses an expression like `path[len(pkgdir)+1:]`.
Though the `+1` is intended to remove a prefix slash,
the expression returns an incorrect path when `pkgdir`
is "/". (ex.: when removing "/" from "/foo", want "foo",
but got "oo")
It seems that `str.TrimFilePathPrefix` would solve
the problem, but the function contains the same bug.
So, this commit fixes `str.TrimFilePathPrefix` then
applies it to `load.resolveEmbed` to solve the issue.
The fix is quite simple. First, remove prefix. Then
check whether the remained first letter is equal to
`filepath.Separator`. If so, remove it then return.