We occassionally see _beginthread failing with EACCES, meaning
"insufficient resources" according to the Microsoft documentation.
Exactly which resources is unclear.
Similar to pthread_create on unix systems, we can wait a bit and retry
to try to get success. The alternative is to abort, so we may as well
give it a try.
Fixes#52572.
Change-Id: I6e05add53b4ae36c61e53b1ee3fed6bc74e17dfa
Reviewed-on: https://go-review.googlesource.com/c/go/+/410355
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
The bodies of cgo_sys_thread_start (and x_cgo_sys_thread_create) are
nearly identical on all of the windows ports.
Create a single _cgo_beginthread implementation that contains the body
and is used on all ports. This will reduce churn in an upcoming CL to
add retry logic.
We could theoretically have a single implementation of
_cgo_sys_thread_start shared by all ports, but I keep them separate for
ease of searching. Right now every single port implements this function
in their gcc_GOOS_GOARCH.c file, so it is nice to keep this symmetry.
_cgo_dummy_export must move out of libcgo_windows.h because it is a
definition and the inclusion of libcgo_windows.h in multiple files
creates duplicate definitions.
For #52572.
Change-Id: I9fa22009389349c754210274c7db2631b061f9c7
Reviewed-on: https://go-review.googlesource.com/c/go/+/410354
Run-TryBot: Michael Pratt <mpratt@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
In CL 404885, we avoid infinite expansion of type instances by sharing a
context between the expanding type and new instances created during
expansion. This ensures that we do not create an infinite number of
identical but distinct instances in the presence of reference cycles.
This pins additional memory to the new instance, but no more
(approximately) than would be pinned by the original expanding instance.
However, we can do better: since type cycles are only possible within a
single package, we only need to share the local context if the two types
are in the same package. This reduces the scope of the shared local
context, and in particular can avoid pinning the package of the
expanding type to the package of the newly created instance.
Updates #52728
Change-Id: Iad2c85f4ecf60125f1da0ba22a7fdec7423e0338
Reviewed-on: https://go-review.googlesource.com/c/go/+/410416
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
The test should skip if it's not on windows *or* it's a short test, but
instead is now skipping if it's not on windows *and* it's a short test,
causing it to be run on non-windows longtest builders.
Change-Id: Ica011bab632b713b0564fefabd5b42878d401844
Reviewed-on: https://go-review.googlesource.com/c/go/+/411122
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Michael Matloob <matloob@golang.org>
Auto-Submit: Michael Matloob <matloob@golang.org>
In faketime mode, checkdead is responsible for jumping time forward to
the next timer expiration, and waking an M to handle the newly ready
timer.
Currently it pulls the exact P that owns the next timer off of the pidle
list. In theory this is efficient because that P is immediately eligible
to run the timer without stealing. Unfortunately it is also fraught with
peril because we are skipping all of the bookkeeping in pidleget:
* Skipped updates to timerpMask mean that our timers may not be eligible
for stealing, as they should be.
* Skipped updates to idlepMask mean that our runq may not be eligible
for stealing, as they should be.
* Skipped updates to sched.npidle may break tracking of spinning Ms,
potentially resulting in lost work.
* Finally, as of CL 410122, skipped updates to p.limiterEvent may affect
the GC limiter, or cause a fatal throw when another event occurs.
The last case has finally undercovered this issue since it quickly
results in a hard crash.
We could add all of these updates into checkdead, but it is much more
maintainable to keep this logic in one place and use pidleget here like
everywhere else in the runtime. This means we probably won't wake the
P owning the timer, meaning that the P will need to steal the timer,
which is less efficient, but faketime is not a performance-sensitive
build mode. Note that the M will automatically make itself a spinning M
to make it eligible to steal since it is the only one running.
Fixes#53294
For #52890
Change-Id: I4acc3d259b9b4d7dc02608581c8b4fd259f272e9
Reviewed-on: https://go-review.googlesource.com/c/go/+/411119
Run-TryBot: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
We use str.TrimFilePathPrefix to trim the module root prefix and get the
relative path of each package in the module when scanning the module
and in the RelPath function. Make sure to clean the path before
indexing and in RelPath to ensure that each path starts with that
prefix, because walk will clean the root path before joining each
subdirectory path to it.
Change-Id: I1dc1eddbd42030eb6d5d8e76a8675f94216447c3
Reviewed-on: https://go-review.googlesource.com/c/go/+/411118
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Very rarely, GDB will successfully run the whole test and the inferior
will exit successfully, and then GDB itself hangs and never exits.
Detect this and skip the test as flaky. We could just continue the
test since all of the output we need is there, but by skipping it
we're less likely to notice serious regressions in this test.
Fixes#37405.
Change-Id: I016cbb06f48673f064733da3e3f1ddcbefd58159
Reviewed-on: https://go-review.googlesource.com/c/go/+/411117
Reviewed-by: Cherry Mui <cherryyz@google.com>
CL 410754 introduces a race accessing the global testHookDialTCP hook.
Avoiding this race is difficult, since Dial can return while
goroutines it starts are still running. Add a version of this
hook to sysDialer, so it can be set on a per-test basis.
(Perhaps other uses of this hook should be moved to use the
sysDialer-local hook, but this change fixes the immediate data race.)
For #52173.
Change-Id: I8fb9be13957e91f92919cae7be213c38ad2af75a
Reviewed-on: https://go-review.googlesource.com/c/go/+/410957
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This change replicates the behavior filed in issue #37015 for packages
imported from the module index. That behavior is that packages that
happen to exist in a GOPATH src directory have p.Root and p.Target set
even when the packages are loaded from modules. This is likely
unintentional behavior because in module mode, packages shouldn't behave
differently depending on whether their directories exist in GOPATH. But
for uniformity, (and because two of our tests depend on this behavior),
this CL will implement this behavior. We can remove it from the module
index when we remove it from the go/build logic.
Change-Id: I3f501c92fbb76eaf86b6b9275539f2129b67f884
Reviewed-on: https://go-review.googlesource.com/c/go/+/410822
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
libfuzzerHookStrCmp is manually reformatted into a proper go doc list.
We don't always format testdata, but these test programs are standard Go
programs that can be formatted.
Change-Id: I4dde398bca225ae8c72e787e4d43fd0ccfd0a90b
Reviewed-on: https://go-review.googlesource.com/c/go/+/411114
Auto-Submit: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Pratt <mpratt@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TestDialParallel is testing the Happy Eyeballs algorithm implementation,
which dials IPv4 and IPv6 addresses in parallel with the preferred
address family getting a head start. This test doesn't care about
the actual network operations, just the handling of the parallel
connections.
Use testHookDialTCP to replace socket creation with a function that
returns successfully, with an error, or after context cancellation
as required.
Limit tests of elapsed times to a check that the fallback deadline
has been exceeded in cases where this is expected.
This should fix persistent test flakiness.
Fixes#52173.
Change-Id: Ic93f270fccb63b24a91105a4d541479fc33a2de4
Reviewed-on: https://go-review.googlesource.com/c/go/+/410754
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
With this change, the termlist String() function prints termlists
in the usual Go notation and thus we can use it in error reporting.
Preparation for fixing #40350.
For #40350.
Change-Id: Ia28318841305de234a71af3146ce0c59f5e601a5
Reviewed-on: https://go-review.googlesource.com/c/go/+/410894
Reviewed-by: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Use the program counter to compute the address of the first instruction
of the ret sled. The ret sled is located after 5 instructions from the
MOVD instruction saving the value of the program counter.
Change-Id: Ie7ae7a0807785d6fea035cf7a770dba7f37de0ec
GitHub-Last-Rev: 2719208c6a
GitHub-Pull-Request: golang/go#53039
Reviewed-on: https://go-review.googlesource.com/c/go/+/407895
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Both GNU and LLVM linkers de facto accept `-zPARAM`, and Go sometimes
does it. Inconsistently: there are more uses of `-z PARAM` than
`-zPARAM`:
$ git grep -E -- '-Wl,-z[^,]' master | wc -l
4
$ git grep -E -- '-Wl,-z,' master | wc -l
7
However, not adding a space between `-z` and the param is not
documented:
llvm-13:
$ man ld.lld-13 | grep -E -A1 -w -- "^ +-z"
-z option
Linker option extensions.
gnu ld:
$ man ld | grep -E -A1 -w -- "^ +-z"
-z keyword
The recognized keywords are:
--
-z defs
Report unresolved symbol references from regular object files. This is done even if the linker is creating a non-symbolic
--
-z muldefs
Normally when a symbol is defined multiple times, the linker will report a fatal error. These options allow multiple definitions
--
-z
--imagic
... and thus should be avoided.
`zig cc`, when used as the C compiler (`CC="zig cc" go build ...`), will
bark, because `zig cc` accepts only `-z PARAM`, as documented.
Closesziglang/zig#11669
Change-Id: I758054ecaa3ce01a72600bf65d7f7b5c3ec46d09
GitHub-Last-Rev: e068e007da
GitHub-Pull-Request: golang/go#53030
Reviewed-on: https://go-review.googlesource.com/c/go/+/407834
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
CL 410244 changes relnote to look for api file changes as well
as references to proposal issues, finding various things that
were missing from the release notes.
This CL adds the TODOs that the updated relnote found.
For #51400.
Change-Id: I512a9b8f1349a6c68c8a6979f55a07964d630175
Reviewed-on: https://go-review.googlesource.com/c/go/+/410361
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This change resolves some TODOs in the release notes, and while we're
here, also clarifies how CPU profile samples are represented in runtime
traces.
Change-Id: Idaa36ccf65b03fd5463b2d5da682d3fa578d2f46
Reviewed-on: https://go-review.googlesource.com/c/go/+/410356
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Austin Clements <austin@google.com>
In a set of 55M Go doc comments drawn from the latest version of
all public Go modules known to the module proxy in spring 2020,
the current Go 1.19 gofmt reformats about 1.57M of them.
Out of those 1.57M comments, inspection of random samples
shows that around 5% of the changed comments contain
unindented code snippets, multiline shell commands, or lists.
For example:
// Here is a greeting:
//
// func main() {
// fmt.Println("hello, world")
// }
// Run this command:
//
// path/to/your/program -flag1=longargument1 \
// -flag2=longargument2 \
// -flag3
// There are three possibilities:
//
// - Unindented code snippets (or JSON objects)
// in which the first and last line are unindented
// but end in { and start with }, respectively.
// - Unindented multiline shell commands
// in which the lines end in \
// - Unindented lists, in which wrapped lines are indented.
All three of these cases involve unindented lines next to indented
lines that would according to the usual rules begin a pre block.
Before this CL, they'd be reformatted to:
// Here is a greeting:
//
// func main() {
//
// fmt.Println("hello, world")
//
// }
// Run this command:
//
// path/to/your/program -flag1=longargument1 \
//
// -flag2=longargument2 \
// -flag3
// There are three possibilities:
//
// - Unindented code snippets (or JSON objects)
//
// in which the first and last line are unindented
// but end in { and start with }, respectively.
//
// - Unindented multiline shell commands
//
// in which the lines end in \
//
// - Unindented lists, in which wrapped lines are indented.
The fact that they are not already in canonical format gives us
a signal that they might not mean what the usual rules would say.
This CL takes advantage of that opening to apply a few heuristics
to better handle these cases:
1. If an indented code block immediately follows (without a blank line)
an unindented line ending in { or \, include the unindented line
in the code block.
2. If an indented code block immediately precedes (without a blank line)
an unindented line beginning with }, include the unindented line
in the code block.
3. If an indented line immediately follows (without a blank line)
an unindented line that starts with a list marker, assume this is
an unindented list with a wrapped indented line, and treat all
adjacent unindented lines starting with list markers as part of
the list, stopping at any surrounding blank lines.
This raises the fraction of “correctly” reformatted doc comments
in the corpus from approximately 87% to approximately 93%.
Change-Id: I7ac542eb085032d607a7caf3ba9020787b2978b5
Reviewed-on: https://go-review.googlesource.com/c/go/+/410360
Auto-Submit: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
``` is Markdown, not Go doc comment, but some small fraction of users get confused.
In a set of 55M Go doc comments drawn from the latest version of
all public Go modules known to the module proxy in spring 2020,
the current Go 1.19 gofmt reformats about 1.57M of them.
Out of those 1.57M comments, 8k of them (about 0.5%) contain ```.
Instead of rewriting ``` to “`, leave it alone.
For #51082.
Change-Id: I1c8c88aac7ef75ec03e1a396b84ffe711c46f941
Reviewed-on: https://go-review.googlesource.com/c/go/+/410359
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
When building CGO internal linking on windows 386, make sure to avoid
rewriting references to "_main" to "main" when reading symbols during
host object loading; the main routine defined by the Go runtime is
still named "_main" (not "main"). If we don't do this, we wind up with
an SXREF symbol named "main", which can then cause the loader to pull
an actual "main" symbol out of a host archive, which is undesirable.
Updates #35006.
Change-Id: I3768e3617b560552f4522e9e72af879c6adf7705
Reviewed-on: https://go-review.googlesource.com/c/go/+/410124
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Auto-Submit: Than McIntosh <thanm@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Now that validType is using the correct type nest (CL 409694),
the top entry of the type nest corresponds to the instantiated
type. Thus we can use that type instance to look up the value
of type parameters, there's no need anymore to create an environment
to look up type arguments.
Remove the need to pass around the environment and remove all
associated types and functions.
Updates #52698.
Change-Id: Ie37eace88896386e667ef93c77a4fc3cd0be6eb9
Reviewed-on: https://go-review.googlesource.com/c/go/+/410294
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
validType was using a global type info map to detect invalid recursive
types, which was incorrect. Instead, change the algorithm as follows:
- Rather than using a "seen" (or typeInfo) map which is cumbersome to
update correctly, use the stack of embedding types (the type nest)
to check whether a type is embedded within itself, directly or
indirectly.
- Use Identical for type comparisons which correctly considers identity
of instantiated generic types.
- As before, maintain the full path of types leading to a cycle. But
unlike before, track the named types rather than their objects, for
a smaller slice ([]*Named rather than []Object), and convert to an
object list only when needed for error reporting.
- As an optimization, keep track of valid *Named types (Checker.valids).
This prevents pathological cases from consuming excessive computation
time.
- Add clarifying comments and document invariants.
Based on earlier insights by David Chase (see also CL 408818).
Fixes#52698.
Change-Id: I5e4598c58afcf4ab987a426c5c4b7b28bdfcf5ea
Reviewed-on: https://go-review.googlesource.com/c/go/+/409694
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Add TODO items for significant changes to go/types: the inclusion of
Origin methods for Var and Func, and a re-working of Named types to
ensure finiteness of reachable types via their API.
Updates #51400
Change-Id: I0f2a972023a5d5f995de3c33e9e2b0a4213e900a
Reviewed-on: https://go-review.googlesource.com/c/go/+/410614
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Before this change, `startParse` would write `lex.breakOK` and `lex.continueOK` when the lexer goroutine is already running, which is a potential race condition.
Makes `breakOK` and `continueOK` configuration flags passed when `lexer` is created, similarly to how `emitComment` works.
Fixes#53234
Change-Id: Ia65f6135509a758cd4c5a453b249a174f4fb3e21
Reviewed-on: https://go-review.googlesource.com/c/go/+/410414
Reviewed-by: Eli Bendersky <eliben@google.com>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
During type-checking, newly created instances share a type checking
Context which de-duplicates identical instances. However, when
unexpanded types escape the type-checking pass or are created via calls
to Instantiate, they lack this shared context. As reported in #52728,
this may lead to infinitely many identical but distinct types that are
reachable via the API.
This CL introduces a new invariant that ensures we don't create such
infinitely expanding chains: instances created during expansion share a
context with the type that led to their creation. During expansion, the
expanding type passes its Context to any newly created instances.
This ensures that cycles will eventually terminate with a previously
seen instance. For example, if we have an instantiation chain
T1[P]->T2[P]->T3[P]->T1[P], by virtue of this Context passing the
expansion of T3[P] will find the instantiation T1[P].
In general, storing a Context in a Named type could lead to pinning
types in memory unnecessarily, but in this case the Context pins only
those types that are reachable from the original instance. This seems
like a reasonable compromise between lazy and eager expansion.
Our treatment of Context was a little haphazard: Checker.bestContext
made it easy to get a context at any point, but made it harder to reason
about which context is being used. To fix this, replace bestContext with
Checker.context, which returns the type-checking context and panics on a
nil receiver. Update all call-sites to verify that the Checker is
non-nil when context is called.
Also make it a panic to call subst with a nil context. Instead, update
subst to explicitly accept a local (=instance) context along with a
global context, and require that one of them is non-nil. Thread this
through to the call to Checker.instance, and handle context updating
there.
Fixes#52728
Change-Id: Ib7f26eb8c406290325bc3212fda25421a37a1e8e
Reviewed-on: https://go-review.googlesource.com/c/go/+/404885
Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
Separate instance information into an instance struct, to reduce memory
footprint for non-instance Named types. This may induce a sense of
deja-vu: we had a similar construct in the past that was removed as
unnecessary. With additional new fields being added that only apply to
instances, having a separate struct makes sense again.
Updates #52728
Change-Id: I0bb5982d71c27e6b574bfb4f7b886a6aeb9c5390
Reviewed-on: https://go-review.googlesource.com/c/go/+/404884
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
In order to clean up context after fully expanding a type (in subsequent
CLs), we must use a common mutex. Eliminate the lazy methodList type,
which keeps a sync.Once per method, in favor of Named.mu.
Updates #52728
Change-Id: I2d13319276df1330ee53046ef1823b0167a258d8
Reviewed-on: https://go-review.googlesource.com/c/go/+/404883
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Introduce a monotonic state variable to track the lifecycle of a named
type, replacing the existing sync.Once. Having a single guard for the
state of underlying and methods will allow for cleaning-up when the type
is fully expanded. In the future, this state may also be used for
detecting access to information such as underlying or methods before the
type is fully set-up, though that will require rethinking our
type-checking of invalid cyclic types.
Also remove support for type-type inference. If we ever support this
feature in the future (inference of missing type arguments for named
type instances), it will likely involve additional machinery that does
not yet exist. Remove the current partial support to simplify our
internal APIs. In particular, this means that Named.resolver is only
used for lazy loading. As a result, we can revert the lazy loader
signature to its previous form.
A lot of exposition is added for how Named types work. Along the way,
the terminology we use to describe them is refined.
Some microbenchmarks are added that were useful in evaluating the
tradeoffs between synchronization mechanisms.
Updates #52728
Change-Id: I4e147360bc6e5d8cd4f37e32e86fece0530a6480
Reviewed-on: https://go-review.googlesource.com/c/go/+/404875
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
The documentation for strconv.ParseFloat mentions that it "accepts
decimal and hexadecimal floating-point number syntax", but it doesn't
specify what those formats entail. For example, "0x10" is not allowed;
you need an explicit exponent, as in "0x10p0".
This clarifies that ParseFloat accepts the Go syntax for floating-point
literals, and links to that spec section. I've also linked to the
relevant spec section for ParseInt's doc comment, which already said
"as defined by the Go syntax for integer literals".
Change-Id: Ib5d2b408bdd01ea0b9f69381a9dbe858f6d1d424
Reviewed-on: https://go-review.googlesource.com/c/go/+/410335
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Following discussion on #47141, make the following changes:
- Document Go's overall approach.
- Document that multiword races can cause crashes.
- Document happens-before for runtime.SetFinalizer.
- Document (or link to) happens-before for more sync types.
- Document happens-before for sync/atomic.
- Document disallowed compiler optimizations.
See also https://research.swtch.com/gomm for background.
Fixes#50859.
Change-Id: I17d837756a77f4d8569f263489c2c45de20a8778
Reviewed-on: https://go-review.googlesource.com/c/go/+/381315
Reviewed-by: Ian Lance Taylor <iant@google.com>
Currently the GC CPU limiter only tracks idle GC work time. However, in
very undersubscribed situations, it's possible that all this extra idle
time prevents the enabling of the limiter, since it all gets account for
as mutator time. Fix this by tracking all idle time via pidleget and
pidleput. To support this, pidleget and pidleput also accept and return
"now" parameters like the timer code.
While we're here, let's clean up some incorrect assumptions that some of
the scheduling code makes about "now."
Fixes#52890.
Change-Id: I4a97893d2e5ad1e8c821f8773c2a1d449267c951
Reviewed-on: https://go-review.googlesource.com/c/go/+/410122
Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Either due to a new nowritebarrierrec annotation or a change in escape
analysis, printDebuglog can't be called from sighandler anymore.
Fix this by avoiding a string allocation that's the primary culprit.
Change-Id: Ic84873a453f45852b0443a46597ed3ab8c9443fd
Reviewed-on: https://go-review.googlesource.com/c/go/+/410121
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Currently the GC CPU limiter consumes CPU time from a few pools, but
because the events that flush to those pools may overlap, rather than be
strictly contained within, the update window for the GC CPU limiter, the
limiter's accounting is ultimately sloppy.
This sloppiness complicates accounting for idle time more completely,
and makes reasoning about the transient behavior of the GC CPU limiter
much more difficult.
To remedy this, this CL adds a field to the P struct that tracks the
start time of any in-flight event the limiter might care about, along
with information about the nature of that event. This timestamp is
managed atomically so that the GC CPU limiter can come in and perform a
read of the partial CPU time consumed by a given event. The limiter also
updates the timestamp so that only what's left over is flushed by the
event itself when it completes.
The end result of this change is that, since the GC CPU limiter is aware
of all past completed events, and all in-flight events, it can much more
accurately collect the CPU time of events since the last update. There's
still the possibility for skew, but any leftover time will be captured
in the following update, and the magnitude of this leftover time is
effectively bounded by the update period of the GC CPU limiter, which is
much easier to consider.
One caveat of managing this timestamp-type combo atomically is that they
need to be packed in 64 bits. So, this CL gives up the top 3 bits of the
timestamp and places the type information there. What this means is we
effectively have only a 61-bit resolution timestamp. This is fine when
the top 3 bits are the same between calls to nanotime, but becomes a
problem on boundaries when those 3 bits change. These cases may cause
hiccups in the GC CPU limiter by not accounting for some source of CPU
time correctly, but with 61 bits of resolution this should be extremely
rare. The rate of update is on the order of milliseconds, so at worst
the runtime will be off of any given measurement by only a few
CPU-milliseconds (and this is directly bounded by the rate of update).
We're probably more inaccurate from the fact that we don't measure real
CPU time but only approximate it.
For #52890.
Change-Id: I347f30ac9e2ba6061806c21dfe0193ef2ab3bbe9
Reviewed-on: https://go-review.googlesource.com/c/go/+/410120
Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
After CL 381317 there exist values that may have an alignment greater
than the pointer size for that platform. Specifically, atomic.{Ui|I}nt64
may be aligned to 8 bytes on a 32-bit platform. If such a value, or
a container for the value, gets stack-allocated, it's possible that it
won't be aligned correctly, because the maximum alignment we enforce on
stacks is governed by the pointer size. Changing that would be a
significant undertaking, so just escape these values to the heap
instead, where we're sure they'll actually be aligned correctly.
Change is by rsc@, I'm just shepherding it through code review.
For #50860.
Change-Id: I51669561c0a13ecb84f821020e144c58cb528418
Reviewed-on: https://go-review.googlesource.com/c/go/+/410131
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This CL makes the changes to actually use the module index when loading
packages and instead of scanning their directories to see if they
contain go files or to extract imports.
Change-Id: I70106181cf64d6fd5a416644ba518b6b90030e0a
Reviewed-on: https://go-review.googlesource.com/c/go/+/403778
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
The data read is used for three primary functions: ImportPackage,
IsDirWithGoFiles and ScanDir. Functions are also provided to get this
information from the intermediate package representation to cache
the information from reads for non-indexed packages.
Change-Id: I5eed629bb0d6ee5b88ab706d06b074475004c081
Reviewed-on: https://go-review.googlesource.com/c/go/+/403975
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Do not need to add single quotes '' when passing the parameter value of
the -ldflags option, otherwise the following error will be reported:
invalid value "'-linkmode=external'" for flag -ldflags: parameter may
not start with quote character.
Change-Id: I322fa7079ac24c8a68d9cb0872b0a20dbc4893d1
Reviewed-on: https://go-review.googlesource.com/c/go/+/410074
Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Benny Siegert <bsiegert@gmail.com>
Run-TryBot: Eric Fang <eric.fang@arm.com>
Fix a build failure when bootstrapping the Go compiler with go-bootstrap 1.4
while the environment contains GOARCH=riscv64.
Building Go toolchain1 using go-1.4-bootstrap-20171003.
src/cmd/compile/internal/ssa/rewriteRISCV64.go:4814
invalid operation: y << x (shift count type int64, must be unsigned integer)
This is because:
- buildtool.go:198: calls bootstrapRewriteFile(src)
- bootstrapRewriteFile: buildtool.go:283 calls:
- isUnneededSSARewriteFile: checks os.Getenv("GOARCH")
- isUnneededSSARewriteFile: returns "", false
- bootstrapRewriteFile: calls bootstrapFixImports
- boostrapFixImports: generates code go1.4 cannot compile
Instead of checking "GOARCH" in the environment, use the gohostarch variable.
Change-Id: Ie9c190498555c4068461fead6278a62e924062c6
GitHub-Last-Rev: 300d7a7fea
GitHub-Pull-Request: golang/go#52362
Reviewed-on: https://go-review.googlesource.com/c/go/+/400376
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Joel Sing <joel@sing.id.au>
Run-TryBot: Joel Sing <joel@sing.id.au>
For instantiated generic functions, all implicit dot operations are
resolved. Thus unsafe.Offsetof may calculating the offset against the
wrong base selector.
To fix it, we must remove any implicit dot operations to find the first
non-implicit one, which is the right base selector for calculating the
offset.
Fixes#53137
Change-Id: I38504067ce0f274615b306edc8f7d7933bdb631a
Reviewed-on: https://go-review.googlesource.com/c/go/+/409355
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
In the windows version of OutBuf.munmap, call syscall.FlushFileBuffers
after the call to syscall.FlushViewOfFile, on the theory that this
will help flush all associated meta-data for the file the linker is
writing.
Updates #44817.
Change-Id: Ibff7d05008a91eeed7634d2760153851e15e1c18
Reviewed-on: https://go-review.googlesource.com/c/go/+/406814
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
This reverts commit CL 401754 (440c9312c8) which reverted CL 400654,
thus reapplying CL 400654, re-adding the func init() { netGo = true }
to cgo_stub.go CL 400654 had originally removed (mistakenly during
development?) that had broken the darwin nocgo builder.
Fixes#33097
Change-Id: I90f59746d2ceb6b5d2bd832c9fc90068f8ff7417
Reviewed-on: https://go-review.googlesource.com/c/go/+/409234
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Keith Randall <khr@google.com>
As far as I can tell, this test suffers from #52433. For some reason,
this seems to become more of a problem on the windows/386 than anywhere
else. This CL is an attempt at a mitigation by slowing down the
allocation rate by inserting runtime.Gosched call in the inner loop. It
also cuts the iteration count which should help too (as less memory is
allocated in total), but the main motivation is to make sure the test
doesn't take too long to run.
Fixes#49564.
Change-Id: I8cc622b06a69cdfa66f680a30e1ccf334eea2164
Reviewed-on: https://go-review.googlesource.com/c/go/+/408825
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This change forces mark and scavenge assists to be cancelled early if
the limiter is enabled. This avoids goroutines getting stuck in really
long assists if the limiter happens to be disabled when they first come
into the assist. This can get especially bad for mark assists, which, in
dire situations, can end up "owing" the GC a really significant debt.
For #52890.
Change-Id: I4bfaa76b8de3e167d49d2ffd8bc2127b87ea566a
Reviewed-on: https://go-review.googlesource.com/c/go/+/408816
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Cgo TSAN (not the Go race detector) intercepts signals and calls
the signal handler at a later time. When the signal handler is
called, the memory may have changed, but the signal context
remains old. As the signal context and the memory don't match, it
is unsafe to unwind the stack from the signal PC and SP. We have
to ignore the signal.
It is probably also not safe to do async preemption, which relies
on the signal PC, and inspects and even writes to the stack (for
call injection).
We also inspect the stack for fatal signals (e.g. SIGSEGV), but I
think they are not delayed. For other signals we don't inspect
the stack, so they are probably fine.
Fixes#27540.
Change-Id: I5c80a7512265b8ea4a91422954dbff32c6c3a0d0
Reviewed-on: https://go-review.googlesource.com/c/go/+/408218
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cherry Mui <cherryyz@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
The linux/loong64 kernel ABI has changed a little since the inception
of the Go port; most notably fstat and fstatat are being removed [1],
leaving only statx as the stat mechanism. Fortunately the structs are
easy enough to translate, and we now exclusively use statx across the
board on loong64 for best compatibility with past and future kernels
(due to the architecture's young age, statx is always available).
In wiring up the statx calls, it turned out the linux/loong64 syscall
definitions were out-of-date, so the generation script received some
tweaking as well.
[1]: https://lore.kernel.org/all/20220518092619.1269111-1-chenhuacai@loongson.cn/
Change-Id: Ifebb9ab9fef783683e453fa331d623575e824a48
Reviewed-on: https://go-review.googlesource.com/c/go/+/407694
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: abner chenc <chenguoqi@loongson.cn>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alex Rakoczy <alex@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
As of CL 334732 `go build` can accept `$CC` with spaces and quotes,
which lets us easily use `zig cc` as the C compiler, or easily pass
extra compiler parameters:
```
CC="zig cc" go build <...>
CC="clang-13 -v" go build <...>
CC="zig cc -Wl,--print-gc-sections" go build <...>
```
However, the same does not apply for building go itself:
```
$ CC="zig cc" ./make.bash
Building Go cmd/dist using /usr/local/go. (go1.18.2 linux/amd64)
go tool dist: cannot invoke C compiler "zig cc": exec: "zig cc": executable file not found in $PATH
Go needs a system C compiler for use with cgo.
To set a C compiler, set CC=the-compiler.
To disable cgo, set CGO_ENABLED=0.
```
With this change Go can be built directly with `zig cc` (the linker arg
will disappear with CL 405414):
```
$ CC="zig cc -Wl,--no-gc-sections" ./make.bash
Building Go cmd/dist using /usr/local/go. (go1.18.2 linux/amd64)
Building Go toolchain1 using /usr/local/go.
Building Go bootstrap cmd/go (go_bootstrap) using Go toolchain1.
Building Go toolchain2 using go_bootstrap and Go toolchain1.
Building Go toolchain3 using go_bootstrap and Go toolchain2.
Building packages and commands for linux/amd64.
---
Installed Go for linux/amd64 in /home/motiejus/code/go
Installed commands in /home/motiejus/code/go/bin
$ ../bin/go version
go version devel go1.19-811f1913a8 Thu May 19 09:44:49 2022 +0300 linux/amd64
```
Fixes#52990
Change-Id: I66b3525d47db488d3c583c1aee3af78060fd5a38
GitHub-Last-Rev: ecc70d7224
GitHub-Pull-Request: golang/go#52991
Reviewed-on: https://go-review.googlesource.com/c/go/+/407216
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Alex Rakoczy <alex@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
zig cc passes `--gc-sections` to the underlying linker, which then
causes undefined symbol errors when compiling with cgo but without C
code. Add `-Wl,--no-gc-sections` to make it work with zig cc. Minimal
example:
**main.go**
package main
import _ "runtime/cgo"
func main() {}
Run (works after the patch, doesn't work before):
CC="zig cc" go build main.go
Among the existing code, `src/runtime/testdata/testprognet` fails to
build:
src/runtime/testdata/testprognet$ CC="zig cc" go build .
net(.text): relocation target __errno_location not defined
net(.text): relocation target getaddrinfo not defined
net(.text): relocation target freeaddrinfo not defined
net(.text): relocation target gai_strerror not defined
runtime/cgo(.text): relocation target stderr not defined
runtime/cgo(.text): relocation target fwrite not defined
runtime/cgo(.text): relocation target vfprintf not defined
runtime/cgo(.text): relocation target fputc not defined
runtime/cgo(.text): relocation target abort not defined
runtime/cgo(.text): relocation target pthread_create not defined
runtime/cgo(.text): relocation target nanosleep not defined
runtime/cgo(.text): relocation target pthread_detach not defined
runtime/cgo(.text): relocation target stderr not defined
runtime/cgo(.text): relocation target strerror not defined
runtime/cgo(.text): relocation target fprintf not defined
runtime/cgo(.text): relocation target abort not defined
runtime/cgo(.text): relocation target pthread_mutex_lock not defined
runtime/cgo(.text): relocation target pthread_cond_wait not defined
runtime/cgo(.text): relocation target pthread_mutex_unlock not defined
runtime/cgo(.text): relocation target pthread_cond_broadcast not defined
runtime/cgo(.text): relocation target malloc not defined
With the patch both examples build as expected.
@ianlancetaylor suggested:
> It would be fine with me if somebody wants to send a cgo patch that
passes -Wl,--no-gc-sections, with a fallback if that option is not
supported.
... and this is what we are doing. Tested with zig
0.10.0-dev.2252+a4369918b
This is a continuation of CL 405414: the original one broke AIX and iOS
builds. To fix that, added `unknown option` to the list of strings
under lookup.
Fixes#52690
Change-Id: Id6743e1e759a02627b0fc6d2ac89bb69b706d04c
GitHub-Last-Rev: 86f227a14e
GitHub-Pull-Request: golang/go#53028
Reviewed-on: https://go-review.googlesource.com/c/go/+/407814
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
This test expects dials of a closed port to complete in about the same
amount of time: an initial probe value +/- 20%. Reduce test flakes on
Windows by increasing the slop to +/- 50% of the original value.
Fixes#52173
Change-Id: I813492c36aca2b0264b3b5b8c96e8bf97193af76
Reviewed-on: https://go-review.googlesource.com/c/go/+/408354
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
If a function type has no type parameters, note when it
is visited and do not recur. (It must be visited
at least once because of closures and their associated
types occurring in a generic context).
Fixes#51832.
Change-Id: Iee20612ffd0a03b838b9e59615f4a0206fc8940b
Reviewed-on: https://go-review.googlesource.com/c/go/+/406714
Reviewed-by: Keith Randall <khr@google.com>
The resolved status of a Named type should be owned by its API, and
callers should access resolved data via methods. Remove several
instances where Named.resolve is explicitly invoked, only to be followed
by a method that also resolves.
Also make two minor cleanups:
- Remove the tparams parameter to Checker.newNamed, as it was unused.
- Include position information when assertions fail, so that one doesn't
need to go digging in the panicking stack to find the assertion
location.
Updates #52728
Change-Id: Icbe8c89e9cfe02d60af7d9ba907eaebe1f00193e
Reviewed-on: https://go-review.googlesource.com/c/go/+/404874
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
The Read method on the Body returned from a net/http.Transport does
not wrap errors returned by the underlying io.Reader and returns a
bare io.ErrUnexpectedEOF if the body is shorter than the declared
Content-Length.
Since we can't feasibly add detail on the net/http side without
breaking established users, we must instead add detail on the caller
side. Since the net/http client uses url.Error for most of its own
errors, we use that same error type here.
I have not added a regression test for this change. (While it is
theoretically possible to set up a GOPROXY that returns incorrect
Content-Length headers, the change seems straightforward enough that
it isn't worth the complex test setup.)
Fixes#52727.
Change-Id: Id00b04ae4fd518148106a49188fe169aadbcce2c
Reviewed-on: https://go-review.googlesource.com/c/go/+/406675
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Add cycle detection to hasVarType to avoid infinite recursions
caused by invalid cyclic types. This catches cases where the
validType check has not yet run or has checked differently
instantiated types.
As an alternative, validType could mark invalid *Named types
by setting their underlying types to Typ[Invalid]. That does
work but discards information which leads to undesired effects
with other errors. A better mechanism might be to explicitly
track in *Named if a type is invalid and why it is invalid,
and connect that with a general validity attribute on types.
That's a more invasive change we might consider down the road.
Fixes#52915.
Change-Id: I9e798b348f4a88b1655e1ff422bd50aaefd9dc50
Reviewed-on: https://go-review.googlesource.com/c/go/+/406849
Run-TryBot: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Use the existing versionErrorf mechanism to report use of undeclared
any and comparable.
Also, port versionErrorf mechanism to go/types and use it in this
case as well.
Adjust tests as needed.
For #52880.
Change-Id: I6a646f16a849692ee0cb57e362d5f3d77e2c25f9
Reviewed-on: https://go-review.googlesource.com/c/go/+/407896
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
os/signal.Notify requires that "the caller must ensure that c has
sufficient buffer space to keep up with the expected signal rate"
as it does a nonblocking send when it receives a signal. The test
currently using a unbuffered channel, which means it may miss the
signal if the signal arrives before the channel receive operation.
Fixes#52998.
Change-Id: Icdcab9396d735506480ef880fb45a4669fa7cc8f
Reviewed-on: https://go-review.googlesource.com/c/go/+/407888
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
For map composite literals where the key type is a suitably constrained
type parameter, the existing key duplicate detection mechanism doesn't
work when the keys are numeric values of different types but equal value.
For instance, given
func _[P int64|float64]() {
_ = map[P]string{0: "foo", 0.0: "bar"}
}
the key values 0 and 0.0 have the same numeric value 0 but currently
are treated as different values int64(0) and float64(0.0). For any
valid instantiation of P, the keys will collide.
This CL changes the keyVal function to map numeric types to the
"smallest" numeric type in which a value can be represented. For
instance, float64(0.0) is mapped to int64(0). This ensures that
numerically equal values are always represented the same way so
that they can be detected as duplicates.
Fixes#51610.
Change-Id: I3eb71142bbe6b13453282a7f71ee48950e58ecbd
Reviewed-on: https://go-review.googlesource.com/c/go/+/406555
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
The referenced address is p.From, not p.To.
Separate from CL 403980, as this is a bug fix. Also, ADR is used
in CL 387336. This is needed to make it work correctly.
Change-Id: Ie0baaeb359b9a7f233458d2becf25dc6a1f8ecbf
Reviewed-on: https://go-review.googlesource.com/c/go/+/407884
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
If the test hangs due to a deadlock in a subprocess, we want a
goroutine dump of that process to figure out the nature of the
deadlock. SIGQUIT causes the Go runtime to produce exactly
such a dump (unless the runtime itself is badly deadlocked).
For #52998.
Change-Id: Id9b3ba89d8f705e14f6cd789353fc2b7f4774ad3
Reviewed-on: https://go-review.googlesource.com/c/go/+/407954
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
The code comment says that the receiver doesn't need to go into
the pointer slot as it will be kept alive in this frame. But it
doesn't. There is no direct reference of rcvr or v (the receiver)
after storing the arguments. Also, it is clearer to explicitly
keep it alive.
Fixes#52800.
Change-Id: Ie3fa8e83f6ecc69d62e8bfab767314d5181f5dc0
Reviewed-on: https://go-review.googlesource.com/c/go/+/407508
Run-TryBot: Cherry Mui <cherryyz@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
libFuzzer provides a special mode known as “value profiling” in which it
tracks the bit-wise progress made by the fuzzer in satisfying tracked
comparisons. Furthermore, libFuzzer uses the value of the return address
in its hooks to distinguish the progress for different comparisons.
The original implementation of the interception for integer comparisons
in Go simply called the libFuzzer hooks from a function written in Go
assembly. The libFuzzer hooks thus always see the same return address
(i.e., the address of the call instruction in the assembly snippet) and
thus can’t distinguish individual comparisons anymore. This drastically
reduces the usefulness of value profiling.
This is fixed by using an assembly trampoline that injects synthetic but
valid return addresses on the stack before calling the libFuzzer hook,
otherwise preserving the calling convention of the respective platform
(for starters, x86_64 Windows or Unix). These fake PCs are generated
deterministically based on the location of the compare instruction in
the IR representation.
Change-Id: Iea68057c83aea7f9dc226fba7128708e8637d07a
GitHub-Last-Rev: f9184baafd
GitHub-Pull-Request: golang/go#51321
Reviewed-on: https://go-review.googlesource.com/c/go/+/387336
Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
On windows hosts, when code is checked out using git with the default
setting of autocrlf=true, carriage returns are appended to source lines
which then prevent the version check from being successful. This removes
carriage returns to allow version matching.
Fixes#52268
Change-Id: I9acc4e907c93a20305f8742cc01687a122a88645
Reviewed-on: https://go-review.googlesource.com/c/go/+/402074
Reviewed-by: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Dan Kortschak <dan@kortschak.io>
Reviewed-by: Ian Lance Taylor <iant@google.com>
IR string compares as well as calls to string comparison functions such
as `strings.EqualFold` are intercepted and the corresponding libFuzzer
callbacks are invoked with the corresponding arguments. As a result, the
compared strings will be added to libFuzzer’s table of recent compares,
which feeds future mutations performed by the fuzzer and thus allow it
to reach into branches guarded by string comparisons.
The list of methods to intercept is maintained in
`cmd/compile/internal/walk/expr.go` and can easily be extended to cover
more standard library functions in the future.
Change-Id: I5c8b89499c4e19459406795dea923bf777779c51
GitHub-Last-Rev: 6b8529b555
GitHub-Pull-Request: golang/go#51319
Reviewed-on: https://go-review.googlesource.com/c/go/+/387335
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
Currently, physical-page-aligned allocations for stacks (where the
physical page size is greater than the runtime page size) first
overallocates some memory, then frees the unaligned portions back to the
heap.
However, because allocating via h.pages.alloc causes scavenged bits to
get cleared, we need to account for that memory correctly in heapFree
and heapReleased. Currently that is not the case, leading to throws at
runtime.
Trying to get that accounting right is complicated, because information
about exactly which pages were scavenged needs to get plumbed up.
Instead, find the oversized region first, and then only allocate the
aligned part. This avoids any accounting issues.
However, this does come with some performance cost, because we don't
update searchAddr (which is safe, it just means the next allocation
potentially must look harder) and we skip the fast path that
h.pages.alloc has for simplicity.
Fixes#52682.
Change-Id: Iefa68317584d73b187634979d730eb30db770bb6
Reviewed-on: https://go-review.googlesource.com/c/go/+/407502
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
By using libFuzzer’s 8-bit counters instead of extra counters, the
coverage instrumentation in libFuzzer mode is improved in three ways:
1- 8-bit counters are supported on all platforms, including macOS and
Windows, with all relevant versions of libFuzzer, whereas extra
counters are a Linux-only feature that only recently received
support on Windows.
2- Newly covered blocks are now properly reported as new coverage by
libFuzzer, not only as new features.
3- The NeverZero strategy is used to ensure that coverage counters
never become 0 again after having been positive once. This resolves
issues encountered when fuzzing loops with iteration counts that
are multiples of 256 (e.g., larger powers of two).
Change-Id: I9021210d7fbffd07c891ad08750402ee91cb3df5
GitHub-Last-Rev: 9057e4b21d
GitHub-Pull-Request: golang/go#51318
Reviewed-on: https://go-review.googlesource.com/c/go/+/387334
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
https://reviews.llvm.org/D123534 is emitting DW_TAG_variable's
that don't have a DW_AT_name. This is allowed in the DWARF
standard. It is adding DIE's for string literals for better
symbolization on buffer overlows etc on these strings. They
no associated name because they are not user provided variables.
Fixes#53000
Change-Id: I2cf063160508687067c7672cef0517bccd707d7b
Reviewed-on: https://go-review.googlesource.com/c/go/+/406816
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
zig cc passes `--gc-sections` to the underlying linker, which then
causes undefined symbol errors when compiling with cgo but without C
code. Add `-Wl,--no-gc-sections` to make it work with zig cc. Minimal
example:
**main.go**
package main
import _ "runtime/cgo"
func main() {}
Run (works after the patch, doesn't work before):
CC="zig cc" go build main.go
Among the existing code, `src/runtime/testdata/testprognet` fails to
build:
src/runtime/testdata/testprognet$ CC="zig cc" go build .
net(.text): relocation target __errno_location not defined
net(.text): relocation target getaddrinfo not defined
net(.text): relocation target freeaddrinfo not defined
net(.text): relocation target gai_strerror not defined
runtime/cgo(.text): relocation target stderr not defined
runtime/cgo(.text): relocation target fwrite not defined
runtime/cgo(.text): relocation target vfprintf not defined
runtime/cgo(.text): relocation target fputc not defined
runtime/cgo(.text): relocation target abort not defined
runtime/cgo(.text): relocation target pthread_create not defined
runtime/cgo(.text): relocation target nanosleep not defined
runtime/cgo(.text): relocation target pthread_detach not defined
runtime/cgo(.text): relocation target stderr not defined
runtime/cgo(.text): relocation target strerror not defined
runtime/cgo(.text): relocation target fprintf not defined
runtime/cgo(.text): relocation target abort not defined
runtime/cgo(.text): relocation target pthread_mutex_lock not defined
runtime/cgo(.text): relocation target pthread_cond_wait not defined
runtime/cgo(.text): relocation target pthread_mutex_unlock not defined
runtime/cgo(.text): relocation target pthread_cond_broadcast not defined
runtime/cgo(.text): relocation target malloc not defined
With the patch both examples build as expected.
@ianlancetaylor suggested:
> It would be fine with me if somebody wants to send a cgo patch that
passes -Wl,--no-gc-sections, with a fallback if that option is not
supported.
... and this is what we are doing. Tested with zig
0.10.0-dev.2252+a4369918b
Fixes#52690
Change-Id: Ib6d1b2bd59335e9663afefd360ddad7da358a938
GitHub-Last-Rev: 58406b36ca
GitHub-Pull-Request: golang/go#52815
Reviewed-on: https://go-review.googlesource.com/c/go/+/405414
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This benchmark is added to test improvements in memclr_amd64.
As it is stated in Intel Optimization Manual 15.16.3.3, AVX2-implemented
memclr can produce a skewed result with the branch predictor being
trained by the large loop iteration count.
This benchmark generates sizes between some specified range. This should
help to measure how memclr works when branch predictors may be incorrectly
trained.
Change-Id: I14d173cafe43ca47198ed920e655547a66b3909f
Reviewed-on: https://go-review.googlesource.com/c/go/+/373362
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
Auto-Submit: Michael Pratt <mpratt@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Keith Randall <khr@google.com>
For a host test we build the test using "go test -c" and then run the
test binary. A test binary run in this way has no default timeout.
This CL gives it a timeout of 5 minutes, scaled for the target.
We can adjust the timeout if necessary.
For #52998
Change-Id: Ib759142f3e71cbb37ec858182998fc5d4fba7ab6
Reviewed-on: https://go-review.googlesource.com/c/go/+/407374
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
In https://golang.org/cl/263542 we added BuildID to the Package struct
in the docs for "go list", correctly pointing out that it's only set
when -export is used.
Further down, the doc details the -export flag on its own.
It already mentioned the Export field, and we forgot to add a mention to
BuildID as well. Do that.
Change-Id: I5838a8900edae8012fe333937d86baea3066c5f1
Reviewed-on: https://go-review.googlesource.com/c/go/+/392114
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Make sure that all the targets of 64-bit atomic operations
are actually aligned to 8 bytes. This has been a source of
bugs on 32-bit systems. (e.g. CL 399754)
The strategy is to have a simple test that just checks the
alignment of some explicitly listed fields and global variables.
Then there's a more complicated test that makes sure the list
used in the simple test is exhaustive. That test has some
limitations, but it should catch most cases, particularly new
uses of atomic operations on new or existing fields.
Unlike a runtime assert, this check is free and will catch
accesses that occur even in very unlikely code paths.
Change-Id: I25ac78df471ac33b57cb91375bd8453d6ce2814f
Reviewed-on: https://go-review.googlesource.com/c/go/+/407034
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
The "simplify" feature used go/ast's object tracking in only one place -
to replace s[a:len(s)] with s[a:].
Using go/ast.Object did allow us to not simplify code like:
len := func(s []int) int { ... }
s = s[a:len(s)]
The existing code already noted the limitation with that approach,
such as "len" being redeclared in a different file in the same package.
Since go/ast's object tracking is file-based and very basic,
it wouldn't work with edge cases like those.
The reasoning is that redeclaring len and abusing it that way is
extremely unlikely, and hasn't been a problem in about a decade now.
I reason that the same applies to len being redeclared in the same file,
so we should be able to safely remove the use of go/ast.Object here.
Per https://go.dev/cl/401454, this makes "gofmt -s" about 5% faster.
If we ever wanted to truly get rid of false positive simplifications,
I imagine we'd want to reimplement the feature under go/analysis,
which is able to fully typecheck packages and suggest edits.
That seems unnecessary at this point, but we can always course correct
in the presumably unlikely scenario that users start reporting bugs.
See #46485.
For #52463.
Change-Id: I77fc97adceafde8f0fe6887ace83ae325bfa7416
Reviewed-on: https://go-review.googlesource.com/c/go/+/401875
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Robert Griesemer <gri@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Currently gctrace and gcpacertrace recompute the heap goal for
end-of-cycle information but this is incorrect.
Because both of these traces are printing stats from the previous cycle
in this case, they should print the heap goal at the end of the previous
cycle.
Change-Id: I967621cbaff9f331cd3e361de8850ddfe0cfc099
Reviewed-on: https://go-review.googlesource.com/c/go/+/407138
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: David Chase <drchase@google.com>
Excluding vendor and testdata.
CL 384268 already reformatted most, but these slipped past.
The struct in the doc comment in debug/dwarf/type.go
was fixed up by hand to indent the first and last lines as well.
For #51082.
Change-Id: Iad020f83aafd671ff58238fe491907e85923d0c7
Reviewed-on: https://go-review.googlesource.com/c/go/+/407137
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
The fix for #19534 (in CL 40994) adjusted escaping in the
dynamically-linked name lookup logic for the plugin package. However,
the regression test added for it incorrectly included quotes within
the -ldflags flag, causing the flag to inadvertently be ignored.
Possibly in that same CL or possibly at some other point, the
condition that the test thought it was checking stopped working: the
dynamic lookup used the path passed to ldflags, but the object file
actually contained the symbol indexed by the original package name.
Ideally we should stop mucking around with ldflags in this test and
run 'go build' from a suitably-named directory instead, to mimic the
actual conditions in which the original bug was reported. For now, as
a more targeted fix, we can pass the '-p' flag to the compiler to
adjust the package path used at compile time to match the one that
will be set at link time.
For #43177.
Updates #19534.
Change-Id: I9763961feb37cfb05dee543f273492e91a350663
Reviewed-on: https://go-review.googlesource.com/c/go/+/407314
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
If they are doc comments then gofmt will put a space between // and sys.
Most of syscall was already this way, following CL 7324056 (in 2013).
These were not.
Change-Id: Ie6ebf82809c199d0d06b87c86045bbb62b687d5f
Reviewed-on: https://go-review.googlesource.com/c/go/+/407136
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
If PATH doesn't contain GOROOT/bin as the first element, this could
otherwise end up running entirely the wrong command (and from the
wrong GOROOT, even).
I pre-tested this change on release-branch.go1.17 using a gomote.
I believe that it will fix the test failure on that branch,
but will need to be backported.
For #52995.
Change-Id: Ib0c43289a1e0ccf9409f0f0ef8046501a955ce65
Reviewed-on: https://go-review.googlesource.com/c/go/+/407294
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
The test checks that the scheduling of the goroutines are within
a small factor, to ensure the scheduler handing off the P
correctly. There have been flaky failures on the builder (probably
due to OS scheduling delays). Increase the threshold to make it
less flaky. The gap would be much bigger if the scheduler doesn't
work correctly.
For the long term maybe it is better to test it more directly
with the scheduler, e.g. with scheduler instrumentation.
May fix#52207.
Change-Id: I50278b70ab21b7f04761fdc8b38dd13304c67879
Reviewed-on: https://go-review.googlesource.com/c/go/+/407134
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
As required by RFC 8446, section 4.6.1, ticket_age_add now holds a
random 32-bit value. Before this change, this value was always set
to 0.
This change also documents the reasoning for always setting
ticket_nonce to 0. The value ticket_nonce must be unique per
connection, but we only ever send one ticket per connection.
Fixes#52814
Fixes CVE-2022-30629
Change-Id: I6c2fc6ca0376b7b968abd59d6d3d3854c1ab68bb
Reviewed-on: https://go-review.googlesource.com/c/go/+/405994
Reviewed-by: Tatiana Bradley <tatiana@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Run-TryBot: Tatiana Bradley <tatiana@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
A composite literal assignment
x = T{field: v}
may be compiled to
x = T{}
x.field = v
We already do not use this form is RHS uses LHS. If LHS is
address-taken, RHS may uses LHS implicitly, e.g.
v = &x.field
x = T{field: *v}
The lowering above would change the value of RHS (*v).
Fixes#52953.
Change-Id: I3f798e00598aaa550b8c17182c7472fef440d483
Reviewed-on: https://go-review.googlesource.com/c/go/+/407014
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
A set domain attribute in a cookie in a Set-Cookie header is intended to
create a domain cookie, i.e. a cookie that is not only sent back to the
domain the Set-Cookie was received from, but to all subdomains thereof
too. Sometimes people set this domain attribute to an IP address. This
seems to be allowed by RFC 6265 albeit it's not really sensible as there
are no "subdomains" of an IP address.
Contemporary browsers allow such cookies, currently Jar forbids them.
This CL allows to persist such cookies in the Jar and send them back
again in subsequent requests. Jar allows those cookies that all
contemporary browsers allow (not all browsers behave the same and none
seems to conform to RFC 6265 in regards to these cookies, see below).
The following browsers in current version) were tested:
- Chrome (Mac and Windows)
- Firefox (Mac and Windows)
- Safari (Mac)
- Opera (Mac)
- Edge (Windows)
- Internet Explorer (Windows)
- curl (Mac, Linux)
All of them allow a cookie to be set via the following HTTP header if
the request was made to e.g. http://35.206.97.83/ :
Set-Cookie: a=1; domain=35.206.97.83
They differ in handling a leading dot "." before the IP address as in
Set-Cookie: a=1; domain=.35.206.97.83
sets a=1 only in curl and in Internet Explorer, the other browsers just
reject such cookies.
As far as these internals can be observed the browsers do not treat such
cookies as domain cookies but as host cookies. RFC 6265 would require to
treat them as domain cookies; this is a) nonsensical and b) doesn't make
an observable difference. As we do not expose Jar entries and their
HostOnly flag it probably is still okay to claim that Jar implements a
RFC 6265 cookie jar.
RFC 6265 would allow cookies with dot-prefixed domains like
domain=.35.206.97.83 but it seems as if this feature of RFC 6265 is not
used in real life and not requested by users of package cookiejar (probably
because it doesn't work in browsers) so we refrain from documenting this
detail.
Fixes#12610
Change-Id: Ibd883d85bde6b958b732cbc3618a1238ac4fc84a
Reviewed-on: https://go-review.googlesource.com/c/go/+/326689
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
There is no need to build with -a. The go command should do the
right thing to pass the flags. Also, we only care packages
mentioned on the command line, so no need to add -gcflags=all=....
May fix#52081.
Change-Id: Idabcfe285c90ed5d25ea6d42abd7617078d3283a
Reviewed-on: https://go-review.googlesource.com/c/go/+/407015
Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Cherry Mui <cherryyz@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
When running TestHostname, the location of the hostname binary
is hardcoded as /bin/hostname. However, on some systems the actual
location is /usr/bin/hostname.
Change this behaviour to perform a lookup for hostname in PATH,
and skip the test when it cannot be found there.
Fixes#52402
Change-Id: I5418bf77258f5ffb2a9f834b8c68d8a7b7a452d7
GitHub-Last-Rev: 750f36fcf9
GitHub-Pull-Request: golang/go#52403
Reviewed-on: https://go-review.googlesource.com/c/go/+/400794
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
As discussed in CL 401434 there are substantial misuses of these in the
wild, and they are a potential source of unsafety even for code that
does not use them directly.
We should either keep them as-is and document when/how they can be used
safely, or deprecate them so that uses will eventually die out.
After some discussion, it was decided to deprecate them outright.
Since the docs already mentioned that they may be unstable across
releases, it should be possible to get rid of them completely later on.
Change-Id: I3b75819409177b5a286c1e9861a2edb6fd1301b8
Reviewed-on: https://go-review.googlesource.com/c/go/+/401434
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
In TestCgoPprofThread, the (fake) cgo traceback function pretends
all C CPU samples are in cpuHogThread. But if a profiling signal
lands in C code but outside of that thread, e.g. before/when the
thread is created, we will get a sample which looks like Go calls
into cpuHogThread. This CL makes the cgo traceback function only
return cpuHogThread PCs when a signal lands on that thread.
May fix#52726.
Change-Id: I21c40f974d1882508626faf3ac45e8347fec31c4
Reviewed-on: https://go-review.googlesource.com/c/go/+/406934
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
types2 uses nopos as the position for predeclared objects, so it's
expected that we'll see !pos.IsKnown() when translating types2
representations into IR.
Change-Id: I8708c2e9815e3dd27da8066c67c73f5586ac4617
Reviewed-on: https://go-review.googlesource.com/c/go/+/406896
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Historically, Objects in go/types were canonical, meaning each entity
was represented by exactly one variable and could thus be identified by
its address. With object instantiation this is no longer the case: Var
and Func objects must be copied to hold substituted type information,
and there may be more than one Var or Func variable representing the
same source-level entity.
This CL adds Origin methods to *Var and *Func, so users can efficiently
navigate to the corresponding canonical object on the generic type.
Fixes#51682
Change-Id: Ia49e15bd6515e1db1eb3b09b88ba666659601316
Reviewed-on: https://go-review.googlesource.com/c/go/+/395535
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
In reviewing CL 406835, I missed that one of the edited files was in
src/vendor. This change reverts that file, fixing the failing
moddeps test on the longtest builders.
Change-Id: Id04b45c3379cf6c17b333444eb7be1301ffcb5f1
Reviewed-on: https://go-review.googlesource.com/c/go/+/406895
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
These are straightforward variants of the existing Sprintf etc.,
but append the resulting bytes to a provided buffer rather than
returning a string.
Internally, there is potentially some allocation because the package
uses a pool of buffers to build its output. We make no attempt to
override that, so the result is first printed into the pool and
then copied to the output. Since it is a managed pool, asymptotically
there should be no extra allocation.
Fixes#47579
RELNOTE=yes
Change-Id: Icef797f9b6f0c84d03e7035d95c06cdb819e2649
Reviewed-on: https://go-review.googlesource.com/c/go/+/406177
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Currently, it's not possible to send informational responses such as
103 Early Hints or 102 Processing.
This patch allows calling WriteHeader() multiple times in order
to send informational responses before the final one.
If the status code is in the 1xx range, the current content of the header map
is also sent. Its content is not removed after the call to WriteHeader()
because the headers must also be included in the final response.
The Chrome and Fastly teams are starting a large-scale experiment to measure
the real-life impact of the 103 status code.
Using Early Hints is proposed as a (partial) alternative to Server Push,
which are going to be removed from Chrome:
https://groups.google.com/a/chromium.org/g/blink-dev/c/K3rYLvmQUBY/m/21anpFhxAQAJ
Being able to send this status code from servers implemented using Go would
help to see if implementing it in browsers is worth it.
Fixes#26089Fixes#36734
Updates #26088
Change-Id: Ib7023c1892c35e8915d4305dd7f6373dbd00a19d
GitHub-Last-Rev: 06d749d345
GitHub-Pull-Request: golang/go#42597
Reviewed-on: https://go-review.googlesource.com/c/go/+/269997
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
The method Location.lookup returns the "start" and "end" times bracketing seconds when that zone is in effect.
This CL does these things:
1. Exported the "start" and "end" times as time.Time form
2. Keep the "Location" of the returned times be the same as underlying time
Fixes#50062.
Change-Id: I88888a100d0fc68f4984a85c75a85a83aa3e5d80
Reviewed-on: https://go-review.googlesource.com/c/go/+/405374
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
This requirement ensures that ReadDir implementations are as compatible
as possible with "*os.File".ReadDir.
The testing/fstest package already tests for equality to io.EOF.
Updates #47062.
Fixes#47086.
Change-Id: I54f911a34e507a3db0abc4da55a19b7a50b35041
Reviewed-on: https://go-review.googlesource.com/c/go/+/333149
Reviewed-by: Michael Knyszek <mknyszek@google.com>
CL 395854 made inline pass to not inlining function with shape params,
but pass no shape arguments. This is intended to be the reverse case of
CL 361260.
However, CL 361260 is using wider condition than necessary. Though it
only needs to check against function parameters, it checks whether the
function type has no shape. It does not cause any issue, because
!fn.Type().HasShape() implies !fn.Type().Params().HasShape().
But for the reverse case, it's not true. Function may have shape type,
but has no shape arguments. Thus, we must tighten the condition to
explicitly check against the function parameters only.
Fixes#52907
Change-Id: Ib87e87ff767c31d99d5b36aa4a6c1d8baf32746d
Reviewed-on: https://go-review.googlesource.com/c/go/+/406475
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Generic functions require instantiation, which package plugin doesn't
support, and likely never will. So instead, we can just skip writing
out any generic functions, which avoids an ICE in the plugin
generation code.
This issue doesn't affect GOEXPERIMENT=unified, because it avoids
leaking any non-instantiated types/functions to the rest of the
compiler backend.
Fixes#52937.
Change-Id: Ie35529c5c241e46b77fcb5b8cca48bb99ce7bfcb
Reviewed-on: https://go-review.googlesource.com/c/go/+/406358
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
The Go 1.19 code freeze has recently started. This is a time to update
all golang.org/x/... module versions that contribute packages to the
std and cmd modules in the standard library to latest master versions.
This CL updates the rest of the modules with x/build/cmd/updatestd.
For #36905.
Change-Id: I4751ca477365b036a8e5ad6a9256293b44ddcd2f
Reviewed-on: https://go-review.googlesource.com/c/go/+/406356
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
The Go 1.19 code freeze has recently started. This is a time to update
all golang.org/x/... module versions that contribute packages to the
std and cmd modules in the standard library to latest master versions.
This CL updates only the lower-level modules arch, sys, term for better
bisection. The next CL will update further ones.
For #36905.
Change-Id: I15f6f8b015f8e425571f4f072d6942c806c6ec3e
Reviewed-on: https://go-review.googlesource.com/c/go/+/406355
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
The change https://go.dev/cl/398475 was too complicated and expensive.
Since the whole string is always available, all that's needed
is a call to strings.HasPrefix.
While we're here, change the way lexer.backup works
so it can be called repeatedly to back up more than one
rune, in case that becomes necessary. This change also
requires less state to maintain, as lexer.width was only
there for backup, and prevented multiple steps.
Fixes#52191
Change-Id: I43b64fc66edeb8ba73ba5aa72f3b727c377dc067
Reviewed-on: https://go-review.googlesource.com/c/go/+/406476
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Rob Pike <r@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Historically, the compiler set types.LocalPkg.Path to "", so a lot of
compiler code checks for this, and then falls back to using
base.Ctxt.Pkgpath instead.
Since CL 393715, we now initialize types.LocalPkg.Path to
base.Ctxt.Pkgpath, so these code paths can now simply rely on Pkg.Path
always being meaningful.
Updates #51734.
Change-Id: I0aedbd7cf8e14edbfef781106a9510344d468f2c
Reviewed-on: https://go-review.googlesource.com/c/go/+/406317
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: David Chase <drchase@google.com>
This test is currently overly sensitive to compiler optimizations,
because inlining can affect the order in which cmd/link emits field
references. The order doesn't actually matter though, so this CL just
tweaks the test to sort the tracked fields before printing them.
Updates #51734.
Change-Id: I3b65ca265856b2e1102f40406d5ce34610c70d40
Reviewed-on: https://go-review.googlesource.com/c/go/+/406674
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
The goroutine profiler tests include one that launches a steady stream
of goroutines. That creates a scheduler busy loop that can prevent
forward progress in the rest of the program. Slow down the launches a
bit so other goroutines have a chance to run.
Fixes#52916
For #52934
Change-Id: I748557201b94918b1fa4960544a51a48d9cacc6b
Reviewed-on: https://go-review.googlesource.com/c/go/+/406654
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Since CL 391014, cmd/compile now requires the -p flag to be set the
build system. This CL changes it to initialize LocalPkg.Path to the
provided path, rather than relying on writing out `"".` into object
files and expecting cmd/link to substitute them.
However, this actually involved a rather long tail of fixes. Many have
already been submitted, but a few notable ones that have to land
simultaneously with changing LocalPkg:
1. When compiling package runtime, there are really two "runtime"
packages: types.LocalPkg (the source package itself) and
ir.Pkgs.Runtime (the compiler's internal representation, for synthetic
references). Previously, these ended up creating separate link
symbols (`"".xxx` and `runtime.xxx`, respectively), but now they both
end up as `runtime.xxx`, which causes lsym collisions (notably
inittask and funcsyms).
2. test/codegen tests need to be updated to expect symbols to be named
`command-line-arguments.xxx` rather than `"".foo`.
3. The issue20014 test case is sensitive to the sort order of field
tracking symbols. In particular, the local package now sorts to its
natural place in the list, rather than to the front.
Thanks to David Chase for helping track down all of the fixes needed
for this CL.
Updates #51734.
Change-Id: Iba3041cf7ad967d18c6e17922fa06ba11798b565
Reviewed-on: https://go-review.googlesource.com/c/go/+/393715
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
The testCPUProfile helper function iterates until the profile contains
enough samples. However, in general very slow builders may need longer
to complete tests, and may have less-responsive schedulers (leading to
longer durations required to collect profiles with enough samples).
To compensate, slower builders generally run tests with longer timeouts.
Since this test helper already dynamically scales the profile duration
based on the collected samples, allow it to continue to retry and
rescale until it would exceed the test's deadline.
Fixes#52656 (hopefully).
Change-Id: I4561e721927503f33a6d23336efa979bb9d3221f
Reviewed-on: https://go-review.googlesource.com/c/go/+/406614
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
The TestForCompiler/LookupCustom test tries to read in the export data
for "math/big", but with a package path of "math/bigger" instead. This
has historically worked because the export data formats were designed
to not assume the package's own path, but I expect we can safely
remove support for this now.
However, since that would be a user-visible change, for now just
disable the test for GOEXPERIMENT=unified so we can land CL 393715. We
can revisit whether it's actually safe to break that go/importer use
case later.
Updates #51734.
Change-Id: I5e89314511bd1352a9f5e14a2e218a5ab00cab3c
Reviewed-on: https://go-review.googlesource.com/c/go/+/406319
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
When compiling package runtime, cmd/compile logically has two copies
of package runtime: the actual source files being compiled, and the
internal description used for emitting compiler-generated calls.
Notably, CL 393715 will cause the compiler's write barrier validation
to start recognizing that compiler-generated calls are actually calls
to the corresponding functions from the source package. And today,
there are some code paths in nowritebarrierrec code paths that
actually end up generating code to call panicshift or panicdivide.
In preparation, this CL marks those functions as
//go:yeswritebarrierrec. We probably want to actually cleanup those
code paths to avoid these calls actually (e.g., explicitly convert
shift count expressions to an unsigned integer type). But for now,
this at least unblocks CL 393715 while preserving the status quo.
Updates #51734.
Change-Id: I01f89adb72466c0260a9cd363e3e09246e39cff9
Reviewed-on: https://go-review.googlesource.com/c/go/+/406316
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
CL 395854 made inline pass to not inlining function with shape params,
but pass no shape arguments. But it does not consider the case where
function has shape params, but passing zero arguments. In this case, the
un-safe interface conversion that may be applied to a shape argument can
not happen, so it's safe to inline the function.
Fixes#52907
Change-Id: Ifa7b23709bb47b97e27dc1bf32343d92683ef783
Reviewed-on: https://go-review.googlesource.com/c/go/+/406176
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Currently the GC CPU limiter doesn't account for idle application time
at all. This means that the GC could start thrashing, for example if the
live heap exceeds the max heap set by the memory limit, but the limiter
will fail to kick in when there's a lot of available idle time. User
goroutines will still be assisting at a really high rate because of
assist pacing rules, but the GC CPU limiter will fail to kick in because
the actual fraction of GC CPU time will be low if there's a lot of
otherwise idle time (for example, on an overprovisioned system).
Luckily, that idle time is usually eaten up entirely by idle mark
workers, at least during the GC cycle. And in these cases where we're
GCing continuously, that's all of our idle time. So we can take idle
mark work time and subtract it from the mutator time accumulated in the
GC CPU limiter, and that will give us a more accurate picture of how
much CPU is being spent by user goroutines on GC. This will allow the GC
CPU limiter to kick in, and reduce the impact of the thrashing.
There is a corner case here if the idle mark workers are disabled, for
example for the periodic GC, but in the case of the periodic GC, I don't
think it's possible for us to be thrashing at all, so it doesn't really
matter.
Fixes#52890.
Change-Id: Ie133a7d1f89b603434b415d51eb8733c2708a858
Reviewed-on: https://go-review.googlesource.com/c/go/+/405898
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Prep refactoring for CL 393715, after which LocalPkg.Path will no
longer be the empty string. Instead of testing `pkg.Path == ""`, we
can just test `pkg == LocalPkg`.
Updates #51734.
Change-Id: I74fff7fb383e275c9f294389d30b2220aced19e0
Reviewed-on: https://go-review.googlesource.com/c/go/+/406059
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Now that Ntype is gone, we no longer require separate sym and nod
fields for Type. It's now always the case that t.sym == t.nod.Sym(),
or that t.sym and t.nod are both nil.
While here, rename nod to obj, to better reflect that in fact it's
always an object (i.e., *ir.Name), not merely a type literal (which no
longer exists in package ir).
Change-Id: Iba4c1590ca585b816ff6b70947ad2a1109918955
Reviewed-on: https://go-review.googlesource.com/c/go/+/405656
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
NewBasic and NewNamed take an Object (i.e., *ir.Name), so that callers
don't need to call SetNod. This CL changes NewTypeParam to follow the
same convention. Following up on recent Ntype removal, this allows
getting rid of Type.SetNod entirely.
While here, Type.SetSym is unused too.
Change-Id: Ibe0f5747e2ab4a9512b65142b6d3006704b60bd3
Reviewed-on: https://go-review.googlesource.com/c/go/+/405654
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Historically, the compiler used to use a name node to represent "nil".
Now, "nil" is represented by NilExpr, so it's not necessary to associate
a Sym with it anymore.
Passes toolstash-check.
Change-Id: Ied1ddefa06ea55ada18ca52c8fcf71defa4c23b2
Reviewed-on: https://go-review.googlesource.com/c/go/+/406174
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Not strictly necessary for CL 393715, but this is necessary if we want
to remove the logic from cmd/internal/obj for substituting `""` in
linker symbol names.
Updates #51734.
Change-Id: Ib13cb12fa3973389ca0c1c9a9209e00c30dc9431
Reviewed-on: https://go-review.googlesource.com/c/go/+/406058
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
The indexed export data format encodes the local package's path as "",
because that's historically how we've represented it within
cmd/compile. The format also requires the local package to be first in
the exported list of packages, and was implicitly relying on ""
sorting before other, non-empty package paths.
We can't change the format without breaking existing importers (e.g.,
go/internal/gcimporter), but we can at least remove the dependency on
LocalPkg.Path being "".
Prep refactoring for CL 393715.
Updates #51734.
Change-Id: I6dd4eafd2d538f4e81376948ef9e92fc44a5462a
Reviewed-on: https://go-review.googlesource.com/c/go/+/406057
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: David Chase <drchase@google.com>
This CL moves the call to base.ParseFlags() earlier in compiler
startup. This is necessary so CL 393715 can use base.Ctxt.Pkgpath to
construct types.LocalPkg.
Updates #51734.
Change-Id: I9f5f75dc9d5fd1b1d22e98523efc95e6cec64385
Reviewed-on: https://go-review.googlesource.com/c/go/+/406055
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
mvs.Req performs an unnecessary search for the maximum version when building minimal requirement list. Someone may be confused when reading this piece of code. The comment of the BuildList function also states that the build list contains the maximum version of each module. We just need to create a maximum version cache that maps from path to version, in the beginning of the Req function body.
Change-Id: I4b353e167f2dcc96bc13cc2e1c602bce47c72bc9
GitHub-Last-Rev: fce11d3c72
GitHub-Pull-Request: golang/go#50345
Reviewed-on: https://go-review.googlesource.com/c/go/+/374277
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This metric exports the the last GC cycle index that the GC limiter was
enabled. This metric is useful for debugging and identifying the root
cause of OOMs, especially when SetMemoryLimit is in use.
For #48409.
Change-Id: Ic6383b19e88058366a74f6ede1683b8ffb30a69c
Reviewed-on: https://go-review.googlesource.com/c/go/+/403614
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
runtime code for js contains possible write barriers that fail
the nowritebarrierrec check when internal local package naming
conventions are changed. The problem was there all already; this
allows the code to compile, and it seems to work anyway in the
(single-threaded) js/wasm environment. The offending operations
are noted with TODO, which is an improvement.
runtime code for plan9 contained an apparent allocation that was
not really an allocation; rewrite to remove the potential allocation
to avoid nowritebarrierrec problems.
This CL is a prerequisite for a pending code cleanup,
https://go-review.googlesource.com/c/go/+/393715
Updates #51734.
Change-Id: I93f31831ff9b92632137dd7b0055eaa721c81556
Reviewed-on: https://go-review.googlesource.com/c/go/+/405901
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The compiler may choose to inline multiple layers of function call, such
that A calling B calling C may end up with all of the instructions for B
and C written as part of A's function body.
Within that function body, some PCs will represent code from function A.
Some will represent code from function B, and for each of those the
runtime will have an instruction attributable to A that it can report as
its caller. Others will represent code from function C, and for each of
those the runtime will have an instruction attributable to B and an
instruction attributable to A that it can report as callers.
When a profiling signal arrives at an instruction in B (as inlined in A)
that the runtime also uses to describe calls to C, the profileBuilder
ends up with an incorrect cache of allFrames results. That PC should
lead to a location record in the profile that represents the frames
B<-A, but the allFrames cache's view should expand the PC only to the B
frame.
Otherwise, when a profiling signal arrives at an instruction in C (as
inlined in B in A), the PC stack C,B,A can get expanded to the frames
C,B<-A,A as follows: The inlining deck starts empty. The first tryAdd
call proposes PC C and frames C, which the deck accepts. The second
tryAdd call proposes PC B and, due to the incorrect caching, frames B,A.
(A fresh call to allFrames with PC B would return the frame list B.) The
deck accepts that PC and frames. The third tryAdd call proposes PC A and
frames A. The deck rejects those because a call from A to A cannot
possibly have been inlined. This results in a new location record in the
profile representing the frames C<-B<-A (good), as called by A (bad).
The bug is the cached expansion of PC B to frames B<-A. That mapping is
only appropriate for the resulting protobuf-format profile. The cache
needs to reflect the results of a call to allFrames, which expands the
PC B to the single frame B.
For #50996
For #52693Fixes#52764
Change-Id: I36d080f3c8a05650cdc13ced262189c33b0083b0
Reviewed-on: https://go-review.googlesource.com/c/go/+/404995
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Rhys Hiltner <rhys@justin.tv>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Panic avoids any write barriers in the runtime by checking first
and throwing if called inappropriately, so it is "okay". Adding
this annotation repairs recursive write barrier checking, which
becomes more thorough when the local package naming convention
is changed from "" to the actual package name.
This CL is a prerequisite for a pending code cleanup,
https://go-review.googlesource.com/c/go/+/393715
Updates #51734.
Change-Id: If831a3598c6c8cd37a8e9ba269f822cd81464a13
Reviewed-on: https://go-review.googlesource.com/c/go/+/405900
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: David Chase <drchase@google.com>
This (1) "just makes sense"
and (2) avoids a weird bug in some name-dependent calling conventions
in wasm code generation, when the local pkg has a real name instead of "".
The calling conventions are triggered for a "wrapper" function, and somehow
an abiwrapper was taken to be a "wrapper" function, resulting in the use of
an invalid register. But abiwrapping has no business being in js/wasm code
generation, so just turn that off.
Updates #51734.
For posterity, that crash is:
GOSSAFUNC=wasmTruncU GOMAXPROCS=1 \
GOOS=js GOARCH=wasm GOEXPERIMENT=regabi,regabiargs
/Users/drchase/work/go-quick/bin/go build \
-gcflags=all=-d=abiwrap -o a.exe \
GOROOT/test/abi/bad_select_crash.go
<autogenerated>:1: internal compiler error: panic: bad Get: invalid register
goroutine 1 [running]:
runtime/debug.Stack()
runtime/debug/stack.go:24 +0x65
cmd/compile/internal/base.FatalfAt({0xc80?, 0x0?}, {0x195c85e, 0x9}, {0xc005ef72c8, 0x1, 0x1})
/Users/drchase/work/go-quick/src/cmd/compile/internal/base/print.go:227 +0x1d7
cmd/compile/internal/base.Fatalf(...)
/Users/drchase/work/go-quick/src/cmd/compile/internal/base/print.go:196
cmd/compile/internal/gc.handlePanic()
/Users/drchase/work/go-quick/src/cmd/compile/internal/gc/main.go:48 +0x85
panic({0x18bf3c0, 0x1ad0430})
runtime/panic.go:854 +0x26d
cmd/internal/obj/wasm.assemble(0xc0000f8200, 0xc001c74880, 0x0?)
/Users/drchase/work/go-quick/src/cmd/internal/obj/wasm/wasmobj.go:920 +0x1958
cmd/internal/obj.Flushplist(0xc0000f8200, 0xc005ef79a8, 0xc0022264c0, {0x7ff7bfefdd17, 0x7})
/Users/drchase/work/go-quick/src/cmd/internal/obj/plist.go:151 +0x784
cmd/compile/internal/objw.(*Progs).Flush(...)
/Users/drchase/work/go-quick/src/cmd/compile/internal/objw/prog.go:124
cmd/compile/internal/ssagen.Compile(0xc000707e00, 0xc001b4d620?)
/Users/drchase/work/go-quick/src/cmd/compile/internal/ssagen/pgen.go:208 +0x495
cmd/compile/internal/gc.compileFunctions.func4.1(0xc005ef7a01?)
/Users/drchase/work/go-quick/src/cmd/compile/internal/gc/compile.go:153 +0x3a
cmd/compile/internal/gc.compileFunctions.func2(0x0?)
/Users/drchase/work/go-quick/src/cmd/compile/internal/gc/compile.go:125 +0x1e
cmd/compile/internal/gc.compileFunctions.func4({0xc004685000, 0x79f, 0xa00?})
/Users/drchase/work/go-quick/src/cmd/compile/internal/gc/compile.go:152 +0x53
cmd/compile/internal/gc.compileFunctions()
/Users/drchase/work/go-quick/src/cmd/compile/internal/gc/compile.go:163 +0x162
cmd/compile/internal/gc.Main(0x198d3d8)
/Users/drchase/work/go-quick/src/cmd/compile/internal/gc/main.go:297 +0x108a
main.main()
/Users/drchase/work/go-quick/src/cmd/compile/main.go:55 +0xdd
Change-Id: I79f039e2494f78efba60e52ab1110d62656fb7ef
Reviewed-on: https://go-review.googlesource.com/c/go/+/405899
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
When the local package has an explicit name instead of "",
this is necessary to get past a cgo plugin test that fails
because of a package signature mismatch. There's something
questionable going on in the package hash generation, and
in particular it went wrong here. Updating the sort order
helps.
This CL is a prerequisite for a pending code cleanup,
https://go-review.googlesource.com/c/go/+/393715
Updates #51734.
The failure:
GOROOT/misc/cgo/testplugin$ go test .
mkdir -p $TMPDIR/src/testplugin
rsync -a testdata/ $TMPDIR/src/testplugin
echo 'module testplugin' > $TMPDIR/src/testplugin/go.mod
mkdir -p $TMPDIR/alt/src/testplugin
rsync -a altpath/testdata/ $TMPDIR/alt/src/testplugin
echo 'module testplugin' > $TMPDIR/alt/src/testplugin/go.mod
cd $TMPDIR/alt/src/testplugin
( PWD=$TMPDIR/alt/src/testplugin GOPATH=$TMPDIR/alt go build -gcflags '' -buildmode=plugin -o $TMPDIR/src/testplugin/plugin-mismatch.so ./plugin-mismatch )
cd $TMPDIR/src/testplugin
( PWD=$TMPDIR/src/testplugin GOPATH=$TMPDIR LD_LIBRARY_PATH=$TMPDIR/src/testplugin go build -gcflags '' -buildmode=plugin ./plugin1 )
( PWD=$TMPDIR/src/testplugin GOPATH=$TMPDIR LD_LIBRARY_PATH=$TMPDIR/src/testplugin go build -gcflags '' -buildmode=plugin ./plugin2 )
cp plugin2.so plugin2-dup.so
( PWD=$TMPDIR/src/testplugin GOPATH=$TMPDIR LD_LIBRARY_PATH=$TMPDIR/src/testplugin go build -gcflags '' -buildmode=plugin -o=sub/plugin1.so ./sub/plugin1 )
( PWD=$TMPDIR/src/testplugin GOPATH=$TMPDIR LD_LIBRARY_PATH=$TMPDIR/src/testplugin go build -gcflags '' -buildmode=plugin -o=unnamed1.so ./unnamed1/main.go )
( PWD=$TMPDIR/src/testplugin GOPATH=$TMPDIR LD_LIBRARY_PATH=$TMPDIR/src/testplugin go build -gcflags '' -buildmode=plugin -o=unnamed2.so ./unnamed2/main.go )
( PWD=$TMPDIR/src/testplugin GOPATH=$TMPDIR LD_LIBRARY_PATH=$TMPDIR/src/testplugin go build -gcflags '' -o host.exe ./host )
( PWD=$TMPDIR/src/testplugin GOPATH=$TMPDIR LD_LIBRARY_PATH=$TMPDIR/src/testplugin go run -gcflags '' ./checkdwarf/main.go plugin2.so plugin2.UnexportedNameReuse )
( PWD=$TMPDIR/src/testplugin GOPATH=$TMPDIR LD_LIBRARY_PATH=$TMPDIR/src/testplugin go run -gcflags '' ./checkdwarf/main.go ./host.exe main.main )
( PWD=$TMPDIR/src/testplugin GOPATH=$TMPDIR LD_LIBRARY_PATH=$TMPDIR/src/testplugin ./host.exe )
--- FAIL: TestRunHost (0.02s)
plugin_test.go:187: ./host.exe: exit status 1
2022/05/13 11:26:37 plugin.Open failed: plugin.Open("plugin1"): plugin was built with a different version of package runtime
and many more after that.
Change-Id: I0780decc5bedeea640ed0b3710867aeda5b3f725
Reviewed-on: https://go-review.googlesource.com/c/go/+/405995
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
In the float test in test_fuzz_mutate_crash, don't assume the mutator
will generate a decimal during mutation. The probability it will is
quite high, but it is not guaranteed, which can cause a flake. Since we
are not really testing that the mutator will do this kind of mutation,
just that a mutation happens, just check that the input is not the zero
value like the rest of the targets.
Fixes#52852
Change-Id: I4640be640204ced01b4dc749c74b46da968ea7df
Reviewed-on: https://go-review.googlesource.com/c/go/+/405855
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
At the expense of performance (having to update another atomic counter)
this change makes CPU limiter assist time much less error-prone to
manage. There are currently a number of issues with respect to how
scavenge assist time is treated, and this change resolves those by just
having the limiter maintain its own internal pool that's drained on each
update.
While we're here, clear the measured assist time each cycle, which was
the impetus for the change.
Change-Id: I84c513a9f012b4007362a33cddb742c5779782b7
Reviewed-on: https://go-review.googlesource.com/c/go/+/404304
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
The test doesn't need to be as aggressive, it _should_ still tickle
the right paths with high enough probability. This should
significantly reduce the memory it consumes, which is at a premium
when testing fuzzing things.
Fixes#52744
Change-Id: I4d8dd5b29e65fb429962850b3f4477982452c856
Reviewed-on: https://go-review.googlesource.com/c/go/+/404634
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
There is no requirement for how the expanded keys are stored
in memory. They are only accessed by asm routines. If keys
are stored directly with stxvd2x, they can be loaded directly
with lxvd2x.
This speeds up ppc64le key expansion and crypting a bit too.
POWER9 aes benchmark delta:
name old time/op new time/op delta
Encrypt 15.0ns ± 0% 13.0ns ± 0% -13.17%
Decrypt 14.6ns ± 0% 13.0ns ± 0% -11.02%
Expand 49.1ns ± 0% 45.1ns ± 0% -8.01%
name old time/op new time/op delta
AESCBCEncrypt1K 1.08µs ± 0% 1.08µs ± 0% -0.46%
AESCBCDecrypt1K 744ns ± 0% 562ns ± 0% -24.46%
Change-Id: I91f3cdc770a178aee849301e4e6aa5a4a517ad10
Reviewed-on: https://go-review.googlesource.com/c/go/+/405135
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Paul Murphy <murp@ibm.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
This reworks how we load/store vector registers using the new
bi-endian P9 instruction emulation macros. This also removes
quite a bit of asm used to align and reorder vector registers.
This is also a slight improvement on P9 ppc64le/linux:
name old speed new speed delta
AESCBCEncrypt1K 936MB/s ± 0% 943MB/s ± 0% +0.80%
AESCBCDecrypt1K 1.28GB/s ± 0% 1.37GB/s ± 0% +6.76%
Updates #18499
Change-Id: Ic5ff71d217d7302b6ae4e8d877c25004bfda5ecd
Reviewed-on: https://go-review.googlesource.com/c/go/+/405134
Run-TryBot: Paul Murphy <murp@ibm.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Measure the average stack size used by goroutines at every GC. When
starting a new goroutine, allocate an initial goroutine stack of that
average size. Intuition is that we'll waste at most 2x in stack space
because only half the goroutines can be below average. In turn, we
avoid some of the early stack growth / copying needed in the average
case.
More details in the design doc at: https://docs.google.com/document/d/1YDlGIdVTPnmUiTAavlZxBI1d9pwGQgZT7IKFKlIXohQ/edit?usp=sharing
name old time/op new time/op delta
Issue18138 95.3µs ± 0% 67.3µs ±13% -29.35% (p=0.000 n=9+10)
Fixes#18138
Change-Id: Iba34d22ed04279da7e718bbd569bbf2734922eaa
Reviewed-on: https://go-review.googlesource.com/c/go/+/345889
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
The types of embedded fields must be named, but they don't
need to be defined types (e.g. if the type name is an alias).
Fixes#41687.
Change-Id: Ib9de65dfab0e23c27d8303875fa45c217aa03331
Reviewed-on: https://go-review.googlesource.com/c/go/+/406054
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
In TestHybridPool attempt to prime to the windows root pool before
the real test actually happens. This is a bit of a band-aid, with
a better long term solution discussed in #52108.
Updates #51599
Change-Id: I406add8d9cd9e3fae37bfc20b97f5479c10a52c2
Reviewed-on: https://go-review.googlesource.com/c/go/+/405914
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
This change makes the modifications to the copies of the files of
go/build used by the modindex package needed for them to be used by
modindex. It also removes the parts of the files not needed by the
modindex package.
Change-Id: I72607868bd7e1ca5fc7c5a496cc836e7922e3786
Reviewed-on: https://go-review.googlesource.com/c/go/+/403974
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
These files are all copied as is from the go/build package, to files
with the same name in modindex (with the exception of build_read, which
was copied from go/build/read.go).
This is being done so that the next CL can show exactly the changes that
were made against the go/build versions.
Unfortunately, git doesn't recognize these as copies, which is annoying.
Change-Id: I27b05b23dc5ccefe5252956bf75025bd57b36c66
Reviewed-on: https://go-review.googlesource.com/c/go/+/403777
Reviewed-by: Michael Matloob <matloob@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
The metasyntax used in the spec is exactly the Wirth Syntax
Notation (WSN), which eventually influenced EBNF. Add a link
but keep mentioning EBNF which is likely more commonly known.
Use the original terms in the productions. Specifically, use
the words "Term" and "Factor" rather than "Alternative" and
"Term".
The terminology cleanup also resolves an inconsistency in the
subsequent prose which is referring to the correct "terms" now.
While at it, add a production for the entire Syntax itself,
matching the original WSN definition.
Also, replace the two uses of "grammar" with "syntax" for
consistency ("syntax" is the prevalent term used throughout
the spec).
Fixes#50074.
Change-Id: If770d5f32f56f509f85893782c1dafbb0eb29b2e
Reviewed-on: https://go-review.googlesource.com/c/go/+/405814
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
- refer to character "categories" rather than "classes" per the
definitions in the Unicode standard
- use "uppercase", "lowercase" (one word) instead of "upper case"
or "upper-case", matching the spelling in the Unicode standard
- clarify that that the blank character "_" is considered a lowercase
letter for Go's purposes (export of identifiers)
Fixes#44715.
Change-Id: I54ef177d26c6c56624662fcdd6d1da60b9bb8d02
Reviewed-on: https://go-review.googlesource.com/c/go/+/405758
Reviewed-by: Robert Griesemer <gri@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Now both the compiler and the assembler require the -p flag and
emit full package path in symbol names, we no longer need to do
the name expansion in the linker. Delete it.
Change-Id: I771d4d97987a0a17414881b52806d600ef4cc351
Reviewed-on: https://go-review.googlesource.com/c/go/+/404300
Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cherry Mui <cherryyz@google.com>
CL 403914 introduced TestAddrStringAllocs which checks that there is
only 1 alloc in Addr.String for v4-in-v6 addresses. This requires
optimizations to be enabled, otherwise there are 2 allocs. Skip the
ipv4-in-ipv6 sub-tests on noopt builders to fix failing
TestAddrStringAllocs on the noopt builders.
Change-Id: I0285264260b264b53cf822dc7cec4829e9854531
Reviewed-on: https://go-review.googlesource.com/c/go/+/405834
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Tobias Klauser <tobias.klauser@gmail.com>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Taking into account the discussion and relevant feedback on a
change proposed in 2013 (see e-mail thread mentioned in issue).
Fixes#48864.
Change-Id: I811d518b7cbdf6b815695174f1da3d4251f491c3
Reviewed-on: https://go-review.googlesource.com/c/go/+/405756
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This reverts CL 353714.
The change closes accepted connection also in graceful shutdown which
breaks the fix for #33313 (and apparent duplicate #36819).
The proper fix should close accepted connection only if server is closed
but not in graceful shutdown.
Updates #48642
Change-Id: I2f7005f3f3037e6563745731bb2693923b654004
GitHub-Last-Rev: f6d885aa37
GitHub-Pull-Request: golang/go#52823
Reviewed-on: https://go-review.googlesource.com/c/go/+/405454
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
CL 391014 requires the compiler to be invoked with the -p flag, to
specify the package path. Later, CL 394217 makes the compiler to
produce an unlinkable object file, so "go tool compile x.go" can
still be used on the command line. This CL does the same for the
assembler, requiring -p, otherwise generating an unlinkable object.
No special case for the main package, as the main package cannot
be only assembly code, and there is no way to tell if it is the
main package from an assembly file.
Now we guarantee that we always have an expanded package path in
the object file. A later CL will delete the name expansion code
in the linker.
Change-Id: I8c10661aaea2ff794614924ead958d80e7e2487d
Reviewed-on: https://go-review.googlesource.com/c/go/+/404298
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The Go object file references (some of) symbols from other
packages by indices, not by names. The linker doesn't need the
symbol names to do the linking. The names are included in the
object file so it is self-contained and tools (objdump, nm) can
read the referenced symbol names. Including the names increases
object file size. Add a flag to disable it on demand (off by
default).
Change-Id: I143a0eb656997497c750b8eb1541341b2aee8f30
Reviewed-on: https://go-review.googlesource.com/c/go/+/404297
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cherry Mui <cherryyz@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This CL replaces a not-very-shared linear-sized set
representation with a much more shared representation.
For the annoying test program in question, it reduces
the heap size by 95%, and the time slightly.
However, for some programs build time is longer.
This also includes at least one bug fix for problems
uncovered while ensuring compatibility with what it
replaces.
Fixes#51543.
Change-Id: Ie7a4c6ea460775faeed2b0378ab21ddffd15badc
Reviewed-on: https://go-review.googlesource.com/c/go/+/397318
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Go tests don't include timestamps by default, but we would like to
have them in order to correlate builder failures with server and
network logs.
Since many of the Go tests with external network and service
dependencies are script tests for the 'go' command, logging timestamps
here adds a lot of logging value with one simple and very low-risk
change.
For #50541.
For #52490.
For #52545.
For #52851.
Change-Id: If3fa86deb4a216ec6a1abc4e6f4ee9b05030a729
Reviewed-on: https://go-review.googlesource.com/c/go/+/405714
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
This reverts commit 4907c62f99.
Reason for revert: Race detector v3, which we just upgraded to, no longer has a goroutine limit.
(small caveat: openbsd/amd64 can't be updated, windows/amd64 isn't updated yet but should be by release time.)
Change-Id: I90017834501e81d3990d888f1b2baf3432452846
Reviewed-on: https://go-review.googlesource.com/c/go/+/405595
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Keith Randall <khr@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
Auto-Submit: Keith Randall <khr@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Change go/build.readDir to use os.ReadDir instead of ioutil.ReadDir.
This addresses a TODO and improves performance on Darwin and Linux.
Darwin: Apple M1
name old time/op new time/op delta
ImportVendor-10 39.8µs ± 1% 37.0µs ± 1% -6.91% (p=0.000 n=10+10)
Linux: Intel(R) Core(TM) i9-9900K CPU @ 3.60GHz
name old time/op new time/op delta
ImportVendor-16 22.9µs ±11% 21.2µs ± 5% -7.47% (p=0.001 n=10+9)
Updates #45557
Change-Id: Ib1bd2e66210e714e499a035847d6261b61b7e2c6
Reviewed-on: https://go-review.googlesource.com/c/go/+/392074
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
The compiler use to compile f()(g()) as:
t1, t2 := g()
f()(t1, t2)
That violates the Go spec, since when "..., all function calls, ... are
evaluated in lexical left-to-right order"
This PR fixes the bug by compiling f()(g()) as:
t0 := f()
t1, t2 := g()
t0(t1, t2)
to make "f()" to be evaluated before "g()".
Fixes#50672
Change-Id: I6a766f3dfc7347d10f8fa3a151f6a5ea79bcf818
Reviewed-on: https://go-review.googlesource.com/c/go/+/392834
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
So prevent heavy runtime call overhead, and the compiler will have a
chance to optimize the bound check.
With this optimization, changing runtime/stack.go to use unsafe.Slice
no longer negatively impacts stack copying performance:
name old time/op new time/op delta
StackCopyWithStkobj-8 16.3ms ± 6% 16.5ms ± 5% ~ (p=0.382 n=8+8)
name old alloc/op new alloc/op delta
StackCopyWithStkobj-8 17.0B ± 0% 17.0B ± 0% ~ (all equal)
name old allocs/op new allocs/op delta
StackCopyWithStkobj-8 1.00 ± 0% 1.00 ± 0% ~ (all equal)
Fixes#48798
Change-Id: I731a9a4abd6dd6846f44eece7f86025b7bb1141b
Reviewed-on: https://go-review.googlesource.com/c/go/+/362934
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
This avoids a dependency on the compiler statically initializing
maxSearchAddr, which is necessary so we can disable the (overly
aggressive and spec non-conforming) optimizations in cmd/compile and
gccgo.
Updates #51913.
Change-Id: I424e62c81c722bb179ed8d2d8e188274a1aeb7b6
Reviewed-on: https://go-review.googlesource.com/c/go/+/396194
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
When we synthesize a playable example, prune declarations that may be
in the original example file but aren't used by the example.
This is ported from pkgsite, where it fixed#43658.
Change-Id: I41e6d4c28afa993c77c8a82b47bd86ba15ed13b7
Reviewed-on: https://go-review.googlesource.com/c/go/+/401758
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
These tests appear to be using timeouts to check for deadlocks or to
cause the test to fail earlier. However, on slower machines these
short timeouts can cause spurious failures, and even on faster
machines if the test locks up we usually want a goroutine dump instead
of a short failure message anyway.
Fixes#52818 (maybe).
Change-Id: Ib8f18d679f9443721e8a924caef6dc8d214fca1b
Reviewed-on: https://go-review.googlesource.com/c/go/+/405434
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
GC requires the whole zeroed word to be visible for a memory subsystem.
While the implementation of Enhanced REP STOSB tries to use as efficient
stores as possible, e.g writing the whole cache line and not byte-after-byte,
we should use REP STOSQ to guarantee the requirements of the GC.
The performance is not affected.
Change-Id: I1b0fd1444a40bfbb661541291ab96eba11bcc762
Reviewed-on: https://go-review.googlesource.com/c/go/+/405274
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
On ARM, when GOARM<=6 the TLS pointer is fetched via a call to a
kernel helper. This call clobbers LR, even just temporarily. If
the function is NOFRAME, if a profiling signal lands right after
the call returns, the unwinder will find the wrong LR. Not mark it
NOFRAME, so the LR will be saved in the usual way and stack
unwinding should work.
May fix#52829.
Change-Id: I419a31dcf4afbcff8d7ab8f179eec3c477589e60
Reviewed-on: https://go-review.googlesource.com/c/go/+/405482
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
The PPC64 maintainers are testing on P10 hardware, so it is helpful
to report the correct cpu, even if this information is not used
elsewhere yet.
Note, AIX will report the current CPU of the host system, so a
POWER10 will not set the IsPOWER9 flag. This is existing behavior,
and should be fixed in a separate patch.
Change-Id: Iebe23dd96ebe03c8a1c70d1ed2dc1506bad3c330
Reviewed-on: https://go-review.googlesource.com/c/go/+/404394
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Paul Murphy <murp@ibm.com>
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
math/bits.Add64 and math/bits.Sub64 now lower and optimize
directly in SSA form.
The optimization of carry chains focuses around eliding
XER<->GPR transfers of the CA bit when used exclusively as an
input to a single carry operations, or when the CA value is
known.
This also adds support for handling XER spills in the assembler
which could happen if carry chains contain inter-dependencies
on each other (which seems very unlikely with practical usage),
or a clobber happens (SRAW/SRAD/SUBFC operations clobber CA).
With PPC64 Add64/Sub64 lowering into SSA and this patch, the net
performance difference in crypto/elliptic benchmarks on P9/ppc64le
are:
name old time/op new time/op delta
ScalarBaseMult/P256 46.3µs ± 0% 46.9µs ± 0% +1.34%
ScalarBaseMult/P224 356µs ± 0% 209µs ± 0% -41.14%
ScalarBaseMult/P384 1.20ms ± 0% 0.57ms ± 0% -52.14%
ScalarBaseMult/P521 3.38ms ± 0% 1.44ms ± 0% -57.27%
ScalarMult/P256 199µs ± 0% 199µs ± 0% -0.17%
ScalarMult/P224 357µs ± 0% 212µs ± 0% -40.56%
ScalarMult/P384 1.20ms ± 0% 0.58ms ± 0% -51.86%
ScalarMult/P521 3.37ms ± 0% 1.44ms ± 0% -57.32%
MarshalUnmarshal/P256/Uncompressed 2.59µs ± 0% 2.52µs ± 0% -2.63%
MarshalUnmarshal/P256/Compressed 2.58µs ± 0% 2.52µs ± 0% -2.06%
MarshalUnmarshal/P224/Uncompressed 1.54µs ± 0% 1.40µs ± 0% -9.42%
MarshalUnmarshal/P224/Compressed 1.54µs ± 0% 1.39µs ± 0% -9.87%
MarshalUnmarshal/P384/Uncompressed 2.40µs ± 0% 1.80µs ± 0% -24.93%
MarshalUnmarshal/P384/Compressed 2.35µs ± 0% 1.81µs ± 0% -23.03%
MarshalUnmarshal/P521/Uncompressed 3.79µs ± 0% 2.58µs ± 0% -31.81%
MarshalUnmarshal/P521/Compressed 3.80µs ± 0% 2.60µs ± 0% -31.67%
Note, P256 uses an asm implementation, thus, little variation is expected.
Change-Id: I88a24f6bf0f4f285c649e40243b1ab69cc452b71
Reviewed-on: https://go-review.googlesource.com/c/go/+/346870
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Paul Murphy <murp@ibm.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
A user noticed that, given the input
{
S: "Hello World",
Integer: 42,
},
{
S: " ", // an actual <tab>
Integer: 42,
},
gofmt would incorrectly format the code as
{
S: "Hello World",
Integer: 42,
},
{
S: " ", // an actual <tab>
Integer: 42,
},
The problem was in the nodeSize method, used to get the printed length
of a node before it's actually printed to the final buffer.
The exprList method calls nodeSize to see if one expression in a list
changes too drastically in size from the previous, which means the
vertical alignment should be broken.
It is worth noting that nodeSize only reports valid lengths if the node
fits into a single line; otherwise, it returns a large number, larger
than an "infinity" currently set to 1e6.
However, the "does it fit in a single line" logic was broken;
it checked if any of the to-be-printed characters is less than ' ',
which does include '\n' and '\f' (the latter used by tabwriter as well),
but also includes '\t', which would make nodeSize incorrectly conclude
that our key-value expression with a tab does not fit into a single line.
While here, make the testdata test cases run as sub-tests,
as I used "-run TestRewrite/tabs.input" to help debug this.
Fixes#51910.
Change-Id: Ib7936e02652bc58f99772b06384ae271fddf09e0
Reviewed-on: https://go-review.googlesource.com/c/go/+/404955
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This fixes an obscure bug in 'go list -versions' if the repo contains
a tag with an explicit "+incompatible" suffix. However, I've never
seen such a repo in the wild; mostly it's an attempt to wrap my brain
around the code and simplify things a bit for the future.
Updates #51324
Updates #51312
Change-Id: I1b078b5db36470cf61aaa85b5244c99b5ee2c842
Reviewed-on: https://go-review.googlesource.com/c/go/+/387917
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Following up on CL 403694, there is a bit of confusion about
when Path is and isn't set, along with now the exported Err field.
Catch the case where Path and Err (and lookPathErr) are all unset
and give a helpful error.
Fixes#52574
Followup after #43724.
Change-Id: I03205172aef3801c3194f5098bdb93290c02b1b6
Reviewed-on: https://go-review.googlesource.com/c/go/+/403759
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
This edge represents the case of executing a write barrier under the
trace lock: we might use the wbufSpans lock to get a new trace buffer,
or mheap to allocate a totally new one.
Fixes#52794.
Change-Id: Ia1ac2c744b8284ae29f4745373df3f9675ab1168
Reviewed-on: https://go-review.googlesource.com/c/go/+/405476
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
The playExample function is very long. Move the code that finds
top-level declarations and unresolved identifiers to a separate
function.
In a future CL, we will be improving that function by removing
unused declarations.
Change-Id: I5632012674687f23094b2bc90615daaecb2cf525
Reviewed-on: https://go-review.googlesource.com/c/go/+/401757
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Move the test cases for doc.Examples from example_test.go into
their own files under testdata/examples.
This makes example_test.go easier to read and collapses several
similar test functions into one.
It will also make it less cumbersome to add large examples later.
Change-Id: Id220c1205e94027d14291898e541b69344842686
Reviewed-on: https://go-review.googlesource.com/c/go/+/401756
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
When constructing a string for a method that will match an example
function's name, remove brackets from the receiver. This makes it
possible to write an example associated with a method of a generic
type.
Also, modify the test for classifying examples to check that all the
expected examples actually appear.
Fixesgolang/go#52496.
Change-Id: Iebc5768f6cb91df9671dd701b97958fb8081f986
Reviewed-on: https://go-review.googlesource.com/c/go/+/401761
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
CL 214429, among other things, created gccgo_link_c.txt as a copy of a
test formerly in go_test.go, but accidentally did so incorrectly:
it used -r instead of -n. This was not noticed because the new test
also incorrectly used [gccgo] when it should have used [exec:gccgo].
Fixing both of those, and also fixing the test to use a go.mod file,
revealed that "go build -n -compiler gccgo" doesn't work, because
it passes a non-existent tmpdir to pkgpath.ToSymbolFunc. This CL
fixes that too.
Change-Id: Id89296803b55412af3bd87aab992f32e26dbce0e
Reviewed-on: https://go-review.googlesource.com/c/go/+/341969
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Named type identity is no longer canonical. For correctness, named types
need to be compared with types.Identical. Our method set algorithm was
not doing this: it was using a map to de-duplicate named types, relying
on their pointer identity. As a result it was possible to get incorrect
results or even infinite recursion, as encountered in #52715.
To fix this, look up types by identity in NewMethodSet and
LookupFieldOrMethod. This does a linear search among types with equal
origin. Alternatively we could use a *Context to do a hash lookup, but
in practice we will be considering a small number of types, and so
performance is not a concern and a linear lookup is simpler. This also
means we don't have to rely on our type hash being perfect, which we
don't depend on elsewhere.
Also add more tests for NewMethodSet and LookupFieldOrMethod involving
generics.
Fixes#52715Fixes#51580
Change-Id: I04dfeff54347bc3544d95a30224c640ef448e9b7
Reviewed-on: https://go-review.googlesource.com/c/go/+/404099
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
A successful invocation of the hostname command prints the hostname
to stdout and exits with code 0. No part of the hostname is printed
to stderr, so don't consider it.
This avoids false positive failures in environments where hostname
prints some extraneous information (such as performance warnings)
to stderr, and makes the test a bit more robust.
Fixes#52781.
Change-Id: I46aa6fbf95b6616bacf9c2b5e412b0851b230744
Reviewed-on: https://go-review.googlesource.com/c/go/+/405014
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Jason Donenfeld <Jason@zx2c4.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Following CL 405114, the extension rule is also wrong. It is safe
to drop the extension if the value is from a boolean-generating
instruction, but not a boolean-typed Value in general (e.g. a Phi
or a in-register parameter). Fix it.
Updates #52788.
Change-Id: Icf3028fe8e90806f9f57fbe2b38d47da27a97e2a
Reviewed-on: https://go-review.googlesource.com/c/go/+/405115
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
CL 266638 marked racewriterange (and some other race functions) as
ABIinternal but missed racereadrange.
arm64 and ppc64le (the other two register ABI platforms at the moment)
already have racereadrange marked as such.
The other two instrumented calls are to racefuncenter/racefuncexit.
Do you think they would need this treatment as well? arm64 already does,
but amd64 and ppc64le do not.
Fixes#51459
Change-Id: I3f54e1298433b6d67bfe18120d9f86205ff66a73
Reviewed-on: https://go-review.googlesource.com/c/go/+/393154
Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
On ARM64, an If block is lowered to (NZ cond yes no). This is
incorrect because cond is a boolean value and therefore only the
last byte is meaningful (same as AMD64, see ARM64Ops.go). But here
we are comparing a full register width with 0. Correct it by
comparing only the last bit.
Fixes#52788.
Change-Id: I2cacf9f3d2f45e149c361a290f511b2d4ed845c4
Reviewed-on: https://go-review.googlesource.com/c/go/+/405114
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
This results in a 1.7-2.4x improvement in native go crypto/elliptic
multiplication operations on PPC64, and similar improvements might
be possible on other architectures which use flags or similar to
represent the carry bit in SSA form.
If it is possible, schedule carry chains independently of each
other to avoid clobbering the carry flag. This is very expensive.
This is done by:
1. Identifying carry bit using, but not creating ops, and lowering
their priority below all other ops which do not need to be
placed at the top of a block. This effectively ensures only
one carry chain will be placed at a time in most important
cases (crypto/elliptic/internal/fiat contains most of them).
2. Raising the priority of carry bit generating ops to schedule
later in a block to ensure they are placed as soon as they
are ready.
Likewise, tuple ops which separate carrying ops are scored
similar to 2 above. This prevents unrelated ops from being
scheduled between carry-dependent operations. This occurs
when unrelated ops are ready to schedule alongside such
tuple ops. This reduces the chances a flag clobbering op
might be placed between two carry-dependent operations.
With PPC64 Add64/Sub64 lowering into SSA and this patch, the net
performance difference in crypto/elliptic benchmarks on P9/ppc64le
are:
name old time/op new time/op delta
ScalarBaseMult/P256 46.3µs ± 0% 46.9µs ± 0% +1.34%
ScalarBaseMult/P224 356µs ± 0% 209µs ± 0% -41.14%
ScalarBaseMult/P384 1.20ms ± 0% 0.57ms ± 0% -52.14%
ScalarBaseMult/P521 3.38ms ± 0% 1.44ms ± 0% -57.27%
ScalarMult/P256 199µs ± 0% 199µs ± 0% -0.17%
ScalarMult/P224 357µs ± 0% 212µs ± 0% -40.56%
ScalarMult/P384 1.20ms ± 0% 0.58ms ± 0% -51.86%
ScalarMult/P521 3.37ms ± 0% 1.44ms ± 0% -57.32%
MarshalUnmarshal/P256/Uncompressed 2.59µs ± 0% 2.52µs ± 0% -2.63%
MarshalUnmarshal/P256/Compressed 2.58µs ± 0% 2.52µs ± 0% -2.06%
MarshalUnmarshal/P224/Uncompressed 1.54µs ± 0% 1.40µs ± 0% -9.42%
MarshalUnmarshal/P224/Compressed 1.54µs ± 0% 1.39µs ± 0% -9.87%
MarshalUnmarshal/P384/Uncompressed 2.40µs ± 0% 1.80µs ± 0% -24.93%
MarshalUnmarshal/P384/Compressed 2.35µs ± 0% 1.81µs ± 0% -23.03%
MarshalUnmarshal/P521/Uncompressed 3.79µs ± 0% 2.58µs ± 0% -31.81%
MarshalUnmarshal/P521/Compressed 3.80µs ± 0% 2.60µs ± 0% -31.67%
Note, P256 uses an asm implementation, thus, little variation is expected.
Updates #40171
Change-Id: I810850e8ff429505424c92d6fe37f99aaa0c6e84
Reviewed-on: https://go-review.googlesource.com/c/go/+/393656
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
Run-TryBot: Paul Murphy <murp@ibm.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Filippo Valsorda <valsorda@google.com>
For this code:
z &= 63
_ = x<<z | x>>(64-z)
Now can prove 'x<<z' in bound. In ppc64 lowering pass, it will not
produce an extra '(ANDconst <typ.Int64> [63] z)' causing
codegen/rotate.go failed. Just remove the type check in rewrite rules
as the workaround.
Removes 32 bounds checks during make.bat.
Fixes#52563.
Change-Id: I14ed2c093ff5638dfea7de9bc7649c0f756dd71a
Reviewed-on: https://go-review.googlesource.com/c/go/+/404315
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: David Chase <drchase@google.com>
Auto-Submit: Keith Randall <khr@golang.org>
Run-TryBot: Wayne Zuo <wdvxdr@golangcn.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
An invalid program may produce invalid types. If the program
calls unsafe.Sizeof on such a type, which is a compile-time
computation, the size-computation must be able to handle it.
Add the invalid type to the list of permissible basic types
and give it a size of 1 (word).
Fixes#52748.
Change-Id: I6c409628f9b77044758caf71cdcb199f9e77adea
Reviewed-on: https://go-review.googlesource.com/c/go/+/404894
Run-TryBot: Robert Griesemer <gri@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
When the procUnpin is placed after shared.pushHead, there is
no need for x as a flag to indicate the previous process.
This CL can make the logic clear, and at the same time reduce
a redundant judgment.
Change-Id: I34ec9ba4cb5b5dbdf13a8f158b90481fed248cf5
Reviewed-on: https://go-review.googlesource.com/c/go/+/360059
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
When using windows some users got a weird error (File not found) when the timezone database is not found. It happens because some methods in the time package don't treat ERROR_PATH_NOT_FOUND and ENOTDIR. To solve it was added a conversion to ENOTENT error.
Fixes#50248
Change-Id: I11c84cf409e01eafb932aea43c7293c8218259b8
GitHub-Last-Rev: fe7fff90cb
GitHub-Pull-Request: golang/go#50906
Reviewed-on: https://go-review.googlesource.com/c/go/+/381957
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
When calling a C function, line information will be
incorrect if the function call's closing parenthesis
is not on the same line as the last argument. We add
a comment with the line info for the return statement
to guide debuggers to the correct line.
Fixes#49839.
Change-Id: I8bc2ce35fec9cbcafbbe8536d5a79dc487eb24bb
GitHub-Last-Rev: 8b28646d2e
GitHub-Pull-Request: golang/go#49840
Reviewed-on: https://go-review.googlesource.com/c/go/+/367454
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
The assignment operation in the program seems to be redundant, the first judgment will continue to overwrite the previous value.
The subsequent slicing operation will cut all the values without frequency.
Change-Id: Id59fc36dd5bacfde881edaf0d9c1af5348286611
Reviewed-on: https://go-review.googlesource.com/c/go/+/244157
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Joe Tsai <joetsai@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
In the file net/dnsclient_unix.go in the function newRequest
error handling is missing after calling b.Finish(). If
the implementation of dnsmessage.Builder.Finish changes
it is theoretically possible that the missing error handling
introduces a nil pointer exception.
Fixes#50946
Change-Id: I3f0785f71def6649d6089d0af71c9e50f5ccb259
GitHub-Last-Rev: 2a2197f7e6
GitHub-Pull-Request: golang/go#50948
Reviewed-on: https://go-review.googlesource.com/c/go/+/381966
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This provides clearer synchronization invariants: if it occurs at all,
the call to c.Process.Kill always occurs before Wait returns. It also
allows any unexpected errors from the goroutine to be propagated back
to Wait.
For #50436.
Change-Id: I7ddadc73e6e67399596e35393f5845646f6111ab
Reviewed-on: https://go-review.googlesource.com/c/go/+/401896
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
`go mod tidy` results in panic due to nil pointer dereference with the
current implementation. Though the panic occurs only in a limited situation
described as below, we had better fix it.
Situation:
- go.mod is in the exactly system's temporary directory (i.e. temp root)
- `go mod tidy` in temp root or in the child directory not having go.mod
No go.mod are found in the situation (i.e. *modFile is nil), however,
*modFile is referred without nil check.
Although just adding nil check works well, the better solution is using
ModFile() function. It works as same as the current implementation and,
in addition, it has either nil check and user friendly error indication.
With using it, users can get a proper error message like "go.mod file not
found in current directory or any parent directory" instead of a panic.
Fixes#51992
Change-Id: I2ba26762778acca6cd637c8eb8c615fb747063f4
Reviewed-on: https://go-review.googlesource.com/c/go/+/400554
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: David Chase <drchase@google.com>
CL 404296 breaks the PPC64LE build because the .TOC. symbol is
visibility hidden and was skipped from the "unresolved symbol"
check (the check needs to be fix). In face, the .TOC. symbol is
special in that it doesn't have a type but we have special logic
to assign a value to it in the address pass. So we can actually
resolve a relocation to .TOC.. We already have a special case
for PIE. It also applies to non-PIE as well.
Fix PPC64LE builds.
Change-Id: Iaf7e36f10c4d0a40fc56b2135e5ff38815e203b1
Reviewed-on: https://go-review.googlesource.com/c/go/+/404302
Run-TryBot: Cherry Mui <cherryyz@google.com>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This patch reworks CL 394534 to fix things so that reading auxiliary
symbol info works properly in a cross-endian mode (running
debug/pe-based tool on a big-endian system). The previous
implementation read in all symbol records using the primary symbol
format, then just used a pointer cast to convert to the auxiliary
format, which doesn't play well if host and target have different
endianness.
Fixes#52079.
Change-Id: I143d94d9313a265f11ca7befd254bdb150698834
Reviewed-on: https://go-review.googlesource.com/c/go/+/397485
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
This unrolls the counter loop and simplifies the load/storing
of text/ciphertext and keys by using unaligned VSX memory
operations.
Performance delta on POWER9:
name old time/op new time/op delta
Encrypt 19.9ns ± 0% 14.9ns ± 0% -24.95%
Decrypt 19.8ns ± 0% 14.6ns ± 0% -26.12%
Change-Id: Iba98d5c1d88c6bead45bc04c97ae64bcb6fc9f21
Reviewed-on: https://go-review.googlesource.com/c/go/+/404354
Run-TryBot: Paul Murphy <murp@ibm.com>
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Filippo Valsorda <valsorda@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
They are usually needed when internally linking gcc code
compiled with -Os. These are typically generated by ld
or gold, but are missing when linking internally.
The PPC64 ELF ABI describes a set of functions to save/restore
non-volatile, callee-save registers using R1/R0/R12:
_savegpr0_n: Save Rn-R31 relative to R1, save LR (in R0), return
_restgpr0_n: Restore Rn-R31 from R1, and return to saved LR
_savefpr_n: Save Fn-F31 based on R1, and save LR (in R0), return
_restfpr_n: Restore Fn-F31 from R1, and return to 16(R1)
_savegpr1_n: Save Rn-R31 based on R12, return
_restgpr1_n: Restore Rn-R31 based on R12, return
_savevr_m: Save VRm-VR31 based on R0, R12 is scratch, return
_restvr_m: Restore VRm-VR31 based on R0, R12 is scratch, return
m is a value 20<=m<=31
n is a value 14<=n<=31
Add several new functions similar to those suggested by the
PPC64 ELFv2 ABI. And update the linker to scan external relocs
for these calls, and redirect them to runtime.elf_<func>+offset
in runtime/asm_ppc64x.go.
Similarly, code which generates plt stubs is moved into
a dedicated function. This avoids an extra scan of relocs.
fixes#52336
Change-Id: I2f0f8b5b081a7b294dff5c92b4b1db8eba9a9400
Reviewed-on: https://go-review.googlesource.com/c/go/+/400796
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Paul Murphy <murp@ibm.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Avoid coercing the CR bit into a GPR register type argument, and
move the existing usage to CRx_y register types. And, update the
compiler usage to this. This transformation is done internally,
so it should not alter existing assembly code.
Likewise, add assembly tests for all optab entries of BC/BR. This
found some cases which were not possible to realize with handwritten
asm, or assemble to something very unexpected if generated by the
compiler. The following optab entries are removed, and the cases
simplified or removed:
{as: ABR, a3: C_SCON, a6: C_LR, type_: 18, size: 4}
This existed only to pass the BH hint to JMP (LR) from compiler
generated code. It cannot be matched with asm. Instead, add and
support 4-operand form "BC{,L} $BO, $BI, $BH, (LR)".
{as: ABR, a1: C_REG, a6: C_CTR, type_: 18, size: 4}
Could be used like "BR R1, (CTR)", but always compiles to bctr
irrespective of arg 1. Any usage should be rewritten as "JMP (CTR)",
or rewritten if this was not the intended behavior.
{as: ABR, a6: C_ZOREG, type_: 15, size: 8}:
{as: ABC, a6: C_ZOREG, type_: 15, size: 8},
Not reachable: 0(reg) is coerced to reg in assembler frontend.
{as: ABC, a2: C_REG, a6: C_LR, type_: 18, size: 4}
{as: ABC, a2: C_REG, a6: C_CTR, type_: 18, size: 4}
Only usable from the compiler. However, the compiler does not
generate this form today. Without a BO operand (usually in a1), it
is not clear what this should assemble to.
Change-Id: I1b5151f884a5877e4a610e6fd41261e8e64c5454
Reviewed-on: https://go-review.googlesource.com/c/go/+/357775
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
Run-TryBot: Paul Murphy <murp@ibm.com>
Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
There is a TODO comment that checking hidden visibility is
probably not the right thing to do. I think it is indeed not. Here
we are not referencing symbols across DSO boundaries, just within
an executable binary. The hidden visibility is for references from
another DSO. So it doesn't actually matter.
This makes cgo internal linking tests work on ARM64 with newer
GCC. It failed and was disabled due to a visibility hidden symbol
in libgcc.a that we didn't handle correctly. Specifically, the
problem is that we didn't mark visibility hidden symbol references
SXREF, which caused the loader to not think it is an unresolved
external symbol, which in turn made it not loading an object file
from the libgcc.a archive which contains the actual definition.
Later stage when we try to resolve the relocation, we couldn't
resolve it. Enable the test as it works now.
Fixes#39466.
Change-Id: I2759e3ae15e7a7a1ab9a820223b688ad894509ed
Reviewed-on: https://go-review.googlesource.com/c/go/+/404296
Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
The path building rework broke the enforcement of EKU nesting, this
change goes back to using the old method of enforcement, since it ends
up being more efficient to check the chains after building, rather than
at each step during path building.
Fixes#52659
Change-Id: Ic7c3717a10c33905677cf7bc4bc0a20f5f15f259
Reviewed-on: https://go-review.googlesource.com/c/go/+/403554
Reviewed-by: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <valsorda@google.com>
Run-TryBot: Roland Shoemaker <roland@golang.org>
Auto-Submit: Roland Shoemaker <roland@golang.org>
This is a port of the printer changes from CLs 402256 and 404397
in the syntax package to go/printer, with adjustments for the
different AST structure and test framework.
For #52559.
Change-Id: Ib7165979a4bd9df91f7f0f1c23b756a41ca31eb3
Reviewed-on: https://go-review.googlesource.com/c/go/+/404194
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Without this change, the type parameter list "[P T | T]" is printed
as "[P T | T,]" in an attempt to avoid an ambiguity. But the type
parameter P cannot syntactically combine with the constraint T | T
and make a new valid expression.
This change introduces a specific combinesWithName predicate that
reports whether a constraint expression can combine with a type
parameter name to form a new valid (value) expression.
Use combinesWithName to accurately determine when a comma is needed.
For #49482.
Change-Id: Id1d17a18f0c9af04495da7b0453e83798f32b04a
Reviewed-on: https://go-review.googlesource.com/c/go/+/404397
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Add support for ppc64le assembler to p256. Most of the changes
are due to the change in nistec interfaces.
There is a change to p256MovCond based on a reviewer's comment.
LXVD2X replaces the use of LXVW4X in one function.
In addition, some refactoring has been done to this file to
reduce size and improve readability:
- Eliminate the use of defines to switch between V and VSX
registers. V regs can be used for instructions some that
previously required VSX.
- Use XXPERMDI instead of VPERM to swap bytes loaded and
stored with LXVD2X and STXVD2X instructions. This eliminates
the need to load the byte swap string into a vector.
- Use VMRGEW and VMRGOW instead of VPERM in the VMULT
macros. This also avoids the need to load byte strings to
swap the high and low values.
These changes reduce the file by about 10% and shows an
improvement of about 2% at runtime.
For #52182
Change-Id: Ic48050fc81bb273b7b4023e54864f4255dcc2a4f
Reviewed-on: https://go-review.googlesource.com/c/go/+/399755
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Filippo Valsorda <valsorda@google.com>
Reviewed-by: Paul Murphy <murp@ibm.com>
Use the batched reader to chunk large Read calls on windows to a max of
1 << 31 - 1 bytes. This prevents an infinite loop when trying to read
more than 1 << 32 -1 bytes, due to how RtlGenRandom works.
This change moves the batched function from rand_unix.go to rand.go,
since it is now needed for both windows and unix implementations.
Fixes#52561
Change-Id: Id98fc4b1427e5cb2132762a445b2aed646a37473
Reviewed-on: https://go-review.googlesource.com/c/go/+/402257
Run-TryBot: Roland Shoemaker <roland@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Filippo Valsorda <valsorda@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
The goal of this CL is to move the implementation to the new interface
with the least amount of changes possible. A follow-up CL will add
documentation and cleanup the assembly API.
* SetBytes does the element and point validity checks now, which were
previously implemented with big.Int.
* p256BaseMult would return (0:0:1) if the scalar was zero, which is
not a valid encoding of the point at infinity, but would get
flattened into (0,0) by p256PointToAffine. The rest of the code can
cope with any encoding with Z = 0, not just (t²:t³:0) with t != 0.
* CombinedMult was only avoiding the big.Int and affine conversion
overhead, which is now gone when operating entirely on nistec types,
so it can be implemented entirely in the crypto/elliptic wrapper,
and will automatically benefit all NIST curves.
* Scalar multiplication can't operate on arbitrarily sized scalars (it
was using big.Int to reduce them), which is fair enough. Changed the
nistec point interface to let ScalarMult and ScalarBaseMult reject
scalars. The crypto/elliptic wrapper still does the big.Int
reduction as needed.
The ppc64le/s390x assembly is disabled but retained to make review of
the change that will re-enable it easier.
Very small performance changes, which we will more then recoup when
crypto/ecdsa moves to invoking nistec directly.
name old time/op new time/op delta
pkg:crypto/elliptic goos:darwin goarch:arm64
ScalarBaseMult/P256-8 11.3µs ± 0% 11.4µs ± 0% +0.87% (p=0.000 n=8+10)
ScalarMult/P256-8 42.2µs ± 0% 42.2µs ± 0% ~ (p=0.825 n=10+9)
MarshalUnmarshal/P256/Uncompressed-8 801ns ± 1% 334ns ± 0% -58.29% (p=0.000 n=9+10)
MarshalUnmarshal/P256/Compressed-8 798ns ± 0% 334ns ± 0% -58.13% (p=0.000 n=10+10)
pkg:crypto/ecdsa goos:darwin goarch:arm64
Sign/P256-8 19.3µs ± 1% 19.4µs ± 0% +0.81% (p=0.003 n=8+9)
Verify/P256-8 56.6µs ± 0% 56.3µs ± 1% -0.48% (p=0.003 n=7+10)
GenerateKey/P256-8 11.9µs ± 0% 12.0µs ± 0% +1.22% (p=0.000 n=7+9)
For #52182
Change-Id: I0690a387e20018f38da55141c0d2659280b1a630
Reviewed-on: https://go-review.googlesource.com/c/go/+/395775
Reviewed-by: Fernando Lobato Meeser <felobato@google.com>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Move the aesCipherGCM struct definition into cipher_asm.go, it is
needed to compile this file, but isn't used on PPC64.
Also, generate a KeySizeError if the key length is not supported
as was done in the ppc64le implementation, and is done in the
generic code.
Change-Id: I025fc63d614b57dac65a18d1ac3dbeec99356292
Reviewed-on: https://go-review.googlesource.com/c/go/+/399254
Reviewed-by: Filippo Valsorda <valsorda@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Paul Murphy <murp@ibm.com>
Currently all typecheck.DeclFunc callers already construct a fresh new
ir.FuncType, which is the last type expression kind that we represent
in IR.
This CL pushes all of the ir.FuncType construction down into
typecheck.DeclFunc. The next CL will simplify the internals so that we
can get rid of ir.FuncType altogether.
Change-Id: I221ed324f157eb38bb57c8886609f53cc4fd99fe
Reviewed-on: https://go-review.googlesource.com/c/go/+/403848
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Ident, ParenExpr, SelectorExpr, and StarExpr used to need to be
allowed as Ntypes for the old -G=0 type checker to represent some type
expressions before type checking, but now they're only ever used to
represent value expressions.
Change-Id: Idd4901ae6149ecc81acf1c52de3bc914d9e73418
Reviewed-on: https://go-review.googlesource.com/c/go/+/403844
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Type arguments are always type expressions, which are semantically
represented by Ntype.
In fact, the slice should probably just be []*types.Type instead, and
that would remove a lot of ir.TypeNode wrapping/unwrapping. But this
lead to issues within the stenciling code, and I can't immediately
make sense why.
Change-Id: Ib944db30e4d21284bc2d8d954b68ecb70b4205a9
Reviewed-on: https://go-review.googlesource.com/c/go/+/403843
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Prep refactoring for the next CL, which removes Name.Ntype
entirely. Pulled out separately because this logic is a little subtle,
so this should be easier to bisect in case there's something I'm
missing here.
Change-Id: I4ffec6ee62fcd036582e8d2c963edcbd8bac184f
Reviewed-on: https://go-review.googlesource.com/c/go/+/403837
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
The CPU profiler adds goroutine labels to its samples based on
getg().m.curg. That allows the profile to correctly attribute work that
the runtime does on behalf of that goroutine on the M's g0 stack via
systemstack calls, such as using runtime.Callers to record the call
stack.
Those labels also cover work on the g0 stack via mcall. When the active
goroutine calls runtime.Gosched, it will receive attribution of its
share of the scheduler work necessary to find the next runnable
goroutine.
The execution tracer's attribution of CPU samples to specific goroutines
should match. When curg is set, attribute the CPU samples to that
goroutine's ID.
Fixes#52693
Change-Id: Ic9af92e153abd8477559e48bc8ebaf3739527b94
Reviewed-on: https://go-review.googlesource.com/c/go/+/404055
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Rhys Hiltner <rhys@justin.tv>
TryBot-Result: Gopher Robot <gobot@golang.org>
We used to use SHA1 for content hashes, but CL 402595 changed
all the “don't care” hashes to cmd/internal/notsha256 (negated SHA256).
This made object files a little bit bigger: fmt.a on my Mac laptop grows
from 910678 to 937612 bytes (+3%).
To remove that growth, truncate the hash we use for these purposes
to 128 bits (half a SHA256), and also use base64 instead of hex for
encoding it when a string form is needed. This brings fmt.a down to
901706 bytes (-1% from original, -4% from current).
Change-Id: Id81da1cf3ee85ed130b3cda73aa697d8c0053a62
Reviewed-on: https://go-review.googlesource.com/c/go/+/404294
Auto-Submit: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
In walkCompare, any ir.OCONVNOP was removed from both operands. So when
constructing assignments for them to preserve any side-effects, using
temporary variables can cause type mismatched with original type.
Instead, using blank assignments will prevent that issue and still make
sure that the operands will be evaluated.
Fixes#52701
Change-Id: I229046acb154890bb36fe441d258563687fdce37
Reviewed-on: https://go-review.googlesource.com/c/go/+/403997
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Robert Griesemer <gri@google.com>
The only fields of the go list output that require BuildInfo to be
computed are the Stale and StaleReason fields. If a user explicitly
requests JSON fields and does not ask for Stale or StaleReason, skip
the computation of BuildInfo.
For #29666
Change-Id: Ie77581c44babedcb5cb7f3dc7d6ed1078b56eee4
Reviewed-on: https://go-review.googlesource.com/c/go/+/402736
Run-TryBot: Michael Matloob <matloob@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Michael Matloob <matloob@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Using the CR bit register arguments makes it more easy to
understand which condition and CR field is being tested when
using ISEL.
Likewise, cleanup optab setup for ISEL. ISEL should only
accept a 5 bit unsigned constant (C_U5CON), and C_ZCON
arguments are accepted by a C_U5CON optab arg.
Change-Id: I2495dbe3595dd3f16c510b3492a88133af9f7e1a
Reviewed-on: https://go-review.googlesource.com/c/go/+/402375
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
Run-TryBot: Paul Murphy <murp@ibm.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
It is hit ~70k times building go.
This make the go binary, 0.04% smaller.
I didn't included benchmarks because this is just constant foldings
and is hard to mesure objectively.
For example, this enable rewriting things like:
if x == 20 {
return x + 30 + z
}
Into:
if x == 20 {
return 50 + z
}
It's not just fixing programer's code,
the ssa generator generate code like this sometimes.
Change-Id: I0861f342b27f7227b5f1c34d8267fa0057b1bbbc
GitHub-Last-Rev: 4c2f9b5216
GitHub-Pull-Request: golang/go#52669
Reviewed-on: https://go-review.googlesource.com/c/go/+/403735
Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: David Chase <drchase@google.com>
This is a port of CL 402256 from the syntax package to go/parser
with adjustments because of the different AST structure, and
excluding any necessary go/printer changes (separate CL).
Type parameter lists starting with the form [name *T|...] or
[name (X)|...] may look like an array length expression [x].
Only after parsing the entire initial expression and checking
whether the expression contains type elements or is followed
by a comma can we make the final decision.
This change simplifies the existing parsing strategy: instead
of trying to make an upfront decision with limited information
(which is insufficient), the parser now parses the start of a
type parameter list or array length specification as expression.
In a second step, if the expression can be split into a name
followed by a type element, or a name followed by an ordinary
expression which is succeeded by a comma, we assume a type
parameter list (because it can't be an array length).
In all other cases we assume an array length specification.
Fixes#52559.
Change-Id: I11ab6e62b073b78b2331bb6063cf74d2a9eaa236
Reviewed-on: https://go-review.googlesource.com/c/go/+/403937
Run-TryBot: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
With this patch, -asan option can detect the error memory
access to global variables.
So this patch makes a few changes:
1. Add the asanregisterglobals runtime support function,
which calls asan runtime function _asan_register_globals
to register global variables.
2. Create a new initialization function for the package
being compiled. This function initializes an array of
instrumented global variables and pass it to function
runtime.asanregisterglobals. An instrumented global
variable has trailing redzone.
3. Writes the new size of instrumented global variables
that have trailing redzones into object file.
4. Notice that the current implementation is only compatible with
the ASan library from version v7 to v9. Therefore, using the
-asan option requires that the gcc version is not less than 7
and the clang version is less than 4, otherwise a segmentation
fault will occur. So this patch adds a check on whether the compiler
being used is a supported version in cmd/go.
(This is a redo of CL 401775 with a fix for a build break due to an
intervening commit that removed the internal/execabs package.)
Updates #44853.
Change-Id: I719d4ef2b22cb2d5516e1494cd453c3efb47d6c7
Reviewed-on: https://go-review.googlesource.com/c/go/+/403851
Auto-Submit: 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>
These implementations will inline to the lower-level primitives,
but they hide the underlying values so that all accesses are
forced to use the atomic APIs. They also allow the use of shorter
names (methods instead of functions) at call sites, making code
more readable.
Pointer[T] also avoids conversions using unsafe.Pointer at call sites.
Discussed on #47141.
See also https://research.swtch.com/gomm for background.
Fixes#50860.
Change-Id: I0b178ee0c7747fa8985f8e48cd7b01063feb7dcc
Reviewed-on: https://go-review.googlesource.com/c/go/+/381317
Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
With this patch, -asan option can detect the error memory
access to global variables.
So this patch makes a few changes:
1. Add the asanregisterglobals runtime support function,
which calls asan runtime function _asan_register_globals
to register global variables.
2. Create a new initialization function for the package
being compiled. This function initializes an array of
instrumented global variables and pass it to function
runtime.asanregisterglobals. An instrumented global
variable has trailing redzone.
3. Writes the new size of instrumented global variables
that have trailing redzones into object file.
4. Notice that the current implementation is only compatible with
the ASan library from version v7 to v9. Therefore, using the
-asan option requires that the gcc version is not less than 7
and the clang version is less than 4, otherwise a segmentation
fault will occur. So this patch adds a check on whether the compiler
being used is a supported version in cmd/go.
Updates #44853.
Change-Id: Ib877a817209ab2be68a8e22c418fe4a4a20880fc
Reviewed-on: https://go-review.googlesource.com/c/go/+/401775
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This is a port of CL 402255 from the syntax package to go/parser
with adjustments because of the different AST structure.
Accept ~x as ordinary unary expression in the parser but recognize
such expressions as invalid in the type checker.
This change opens the door to recognizing complex type constraint
literals such as `*E|~int` in `[P *E|~int]` and parse them correctly
instead of reporting a parse error because `P*E|~int` syntactically
looks like an incorrect array length expression (binary expression
where the RHS of | is an invalid unary expression ~int).
As a result, the parser is more forgiving with expressions but the
type checker will reject invalid uses as before.
We could pass extra information into the binary/unary expression
parse functions to prevent the use of ~ in invalid situations but
it doesn't seem worth the trouble. In fact it may be advantageous
to allow a more liberal expression syntax especially in the presence
of errors (better parser synchronization after an error).
Preparation for fixing #52559.
Change-Id: I48562cf40ccf5f14c20fcd92c40a0303b2d8b2b8
Reviewed-on: https://go-review.googlesource.com/c/go/+/403696
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: David Chase <drchase@google.com>
methodName was brittle in that it assumed exactly where
in the call stack the exported Value method is.
This broke since recent inlining optimizations changed
exactly which frame the exported method was located.
Instead, iterate through a sufficient number of stack entries
and dynamically determined the exported Value method name.
This is more maintainable, but slightly slower.
The slowdown is acceptable since panics are not the common case.
Change-Id: I9fc939627007d7bae004b4969516ad44be09c270
Reviewed-on: https://go-review.googlesource.com/c/go/+/403494
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Russ Cox <rsc@golang.org>
Auto-Submit: Russ Cox <rsc@golang.org>
This reverts commit a41e37f56a, and
updates the ASM usage to be go1.8 compliant. go 1.18 added support
for using VR's in place of VSR arguments.
The following transformations are made:
XXLOR Vx, Vx, VSy -> XXLORQ VSx+32, VSx+32, VSy
XXLOR VSx, VSx, Vy -> XXLORQ VSx, VSx, VSy+32
XXLOR is broken on 1.8, but XXLORQ is identical and still supported
today.
Change-Id: Icc9cd5511b412c30a14e6afd07a51839aaaf6021
Reviewed-on: https://go-review.googlesource.com/c/go/+/403734
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Paul Murphy <murp@ibm.com>
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
Reviewed-by: David Chase <drchase@google.com>
The profiles for memory allocations, sync.Mutex contention, and general
blocking store their data in a shared hash table. The bookkeeping work
at the end of a garbage collection cycle involves maintenance on each
memory allocation record. Previously, a single lock guarded access to
the hash table and the contents of all records. When a program has
allocated memory at a large number of unique call stacks, the
maintenance following every garbage collection can hold that lock for
several milliseconds. That can prevent progress on all other goroutines
by delaying acquirep's call to mcache.prepareForSweep, which needs the
lock in mProf_Free to report when a profiled allocation is no longer in
use. With no user goroutines making progress, it is in effect a
multi-millisecond GC-related stop-the-world pause.
Split the lock so the call to mProf_Flush no longer delays each P's call
to mProf_Free: mProf_Free uses a lock on the memory records' N+1 cycle,
and mProf_Flush uses locks on the memory records' accumulator and their
N cycle. mProf_Malloc also no longer competes with mProf_Flush, as it
uses a lock on the memory records' N+2 cycle. The profiles for
sync.Mutex contention and general blocking now share a separate lock,
and another lock guards insertions to the shared hash table (uncommon in
the steady-state). Consumers of each type of profile take the matching
accumulator lock, so will observe consistent count and magnitude values
for each record.
For #45894
Change-Id: I615ff80618d10e71025423daa64b0b7f9dc57daa
Reviewed-on: https://go-review.googlesource.com/c/go/+/399956
Reviewed-by: Carlos Amedee <carlos@golang.org>
Run-TryBot: Rhys Hiltner <rhys@justin.tv>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
When the CPU profiler and execution tracer are both active, report the
CPU profile samples in the execution trace data stream.
Include only samples that arrive on the threads known to the runtime,
but include them even when running g0 (such as near the scheduler) or if
there's no P (such as near syscalls).
Render them in "go tool trace" as instantaneous events.
For #16895
Change-Id: I0aa501a7b450c971e510961c0290838729033f7f
Reviewed-on: https://go-review.googlesource.com/c/go/+/400795
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Rhys Hiltner <rhys@justin.tv>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
The race annotations for goroutine label maps covered the special type
of read necessary to create CPU profiles. Extend that to include
goroutine profiles. Annotate the copy involved in creating new
goroutines.
Fixes#50292
Change-Id: I10f69314e4f4eba85c506590fe4781f4d6b8ec2d
Reviewed-on: https://go-review.googlesource.com/c/go/+/385660
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Austin Clements <austin@google.com>
Currently the consistent total allocation stats are managed as uintptrs,
which means they can easily overflow on 32-bit systems. Fix this by
storing these stats as uint64s. This will cause some minor performance
degradation on 32-bit systems, but there really isn't a way around this,
and it affects the correctness of the metrics we export.
Fixes#52680.
Change-Id: I7e6ca44047d46b4bd91c6f87c2d29f730e0d6191
Reviewed-on: https://go-review.googlesource.com/c/go/+/403758
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Austin Clements <austin@google.com>
I landed the bottom CL of my stack without rebasing or retrying trybots,
but in the rebase "escape" was removed in favor of "Escape."
Change-Id: Icdc4d8de8b6ebc782215f2836cd191377cc211df
Reviewed-on: https://go-review.googlesource.com/c/go/+/403755
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Currently the runtime's scavenging algorithm involves running from the
top of the heap address space to the bottom (or as far as it gets) once
per GC cycle. Once it treads some ground, it doesn't tread it again
until the next GC cycle.
This works just fine for the background scavenger, for heap-growth
scavenging, and for debug.FreeOSMemory. However, it breaks down in the
face of a memory limit for small heaps in the tens of MiB. Basically,
because the scavenger never retreads old ground, it's completely
oblivious to new memory it could scavenge, and that it really *should*
in the face of a memory limit.
Also, every time some thread goes to scavenge in the runtime, it
reserves what could be a considerable amount of address space, hiding it
from other scavengers.
This change modifies and simplifies the implementation overall. It's
less code with complexities that are much better encapsulated. The
current implementation iterates optimistically over the address space
looking for memory to scavenge, keeping track of what it last saw. The
new implementation does the same, but instead of directly iterating over
pages, it iterates over chunks. It maintains an index of chunks (as a
bitmap over the address space) that indicate which chunks may contain
scavenge work. The page allocator populates this index, while scavengers
consume it and iterate over it optimistically.
This has a two key benefits:
1. Scavenging is much simpler: find a candidate chunk, and check it,
essentially just using the scavengeOne fast path. There's no need for
the complexity of iterating beyond one chunk, because the index is
lock-free and already maintains that information.
2. If pages are freed to the page allocator (always guaranteed to be
unscavenged), the page allocator immediately notifies all scavengers
of the new source of work, avoiding the hiding issues of the old
implementation.
One downside of the new implementation, however, is that it's
potentially more expensive to find pages to scavenge. In the past, if
a single page would become free high up in the address space, the
runtime's scavengers would ignore it. Now that scavengers won't, one or
more scavengers may need to iterate potentially across the whole heap to
find the next source of work. For the background scavenger, this just
means a potentially less reactive scavenger -- overall it should still
use the same amount of CPU. It means worse overheads for memory limit
scavenging, but that's not exactly something with a baseline yet.
In practice, this shouldn't be too bad, hopefully since the chunk index
is extremely compact. For a 48-bit address space, the index is only 8
MiB in size at worst, but even just one physical page in the index is
able to support up to 128 GiB heaps, provided they aren't terribly
sparse. On 32-bit platforms, the index is only 128 bytes in size.
For #48409.
Change-Id: I72b7e74365046b18c64a6417224c5d85511194fb
Reviewed-on: https://go-review.googlesource.com/c/go/+/399474
Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This change does everything necessary to make the memory allocator and
the scavenger respect the memory limit. In particular, it:
- Adds a second goal for the background scavenge that's based on the
memory limit, setting a target 5% below the limit to make sure it's
working hard when the application is close to it.
- Makes span allocation assist the scavenger if the next allocation is
about to put total memory use above the memory limit.
- Measures any scavenge assist time and adds it to GC assist time for
the sake of GC CPU limiting, to avoid a death spiral as a result of
scavenging too much.
All of these changes have a relatively small impact, but each is
intimately related and thus benefit from being done together.
For #48409.
Change-Id: I35517a752f74dd12a151dd620f102c77e095d3e8
Reviewed-on: https://go-review.googlesource.com/c/go/+/397017
Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This change makes the memory limit functional by including it in the
heap goal calculation. Specifically, we derive a heap goal from the
memory limit, and compare that to the GOGC-based goal. If the goal based
on the memory limit is lower, we prefer that.
To derive the memory limit goal, the heap goal calculation now takes
a few additional parameters as input. As a result, the heap goal, in the
presence of a memory limit, may change dynamically. The consequences of
this are that different parts of the runtime can have different views of
the heap goal; this is OK. What's important is that all of the runtime
is able to observe the correct heap goal for the moment it's doing
something that affects it, like anything that should trigger a GC cycle.
On the topic of triggering a GC cycle, this change also allows any
manually managed memory allocation from the page heap to trigger a GC.
So, specifically workbufs, unrolled GC scan programs, and goroutine
stacks. The reason for this is that now non-heap memory can effect the
trigger or the heap goal.
Most sources of non-heap memory only change slowly, like GC pointer
bitmaps, or change in response to explicit function calls like
GOMAXPROCS. Note also that unrolled GC scan programs and workbufs are
really only relevant during a GC cycle anyway, so they won't actually
ever trigger a GC. Our primary target here is goroutine stacks.
Goroutine stacks can increase quickly, and this is currently totally
independent of the GC cycle. Thus, if for example a goroutine begins to
recurse suddenly and deeply, then even though the heap goal and trigger
react, we might not notice until its too late. As a result, we need to
trigger a GC cycle.
We do this trigger in allocManual instead of in stackalloc because it's
far more general. We ultimately care about memory that's mapped
read/write and not returned to the OS, which is much more the domain of
the page heap than the stack allocator. Furthermore, there may be new
sources of memory manual allocation in the future (e.g. arenas) that
need to trigger a GC if necessary. As such, I'm inclined to leave the
trigger in allocManual as an extra defensive measure.
It's worth noting that because goroutine stacks do not behave quite as
predictably as other non-heap memory, there is the potential for the
heap goal to swing wildly. Fortunately, goroutine stacks that haven't
been set up to shrink by the last GC cycle will not shrink until after
the next one. This reduces the amount of possible churn in the heap goal
because it means that shrinkage only happens once per goroutine, per GC
cycle. After all the goroutines that should shrink did, then goroutine
stacks will only grow. The shrink mechanism is analagous to sweeping,
which is incremental and thus tends toward a steady amount of heap
memory used. As a result, in practice, I expect this to be a non-issue.
Note that if the memory limit is not set, this change should be a no-op.
For #48409.
Change-Id: Ie06d10175e5e36f9fb6450e26ed8acd3d30c681c
Reviewed-on: https://go-review.googlesource.com/c/go/+/394221
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
As of the last CL, the heap trigger is computed as-needed. This means
that some of the niceties we assumed (that the float64 computations
don't matter because we're doing this rarely anyway) are no longer true.
While we're not exactly on a hot path right now, the trigger check still
happens often enough that it's a little too hot for comfort.
This change optimizes the computation by replacing the float64
multiplication with a shift and a constant integer multiplication.
I ran an allocation microbenchmark for an allocation size that would hit
this path often. CPU profiles seem to indicate this path was ~0.1% of
cycles (dwarfed by other costs, e.g. zeroing memory) even if all we're
doing is allocating, so the "optimization" here isn't particularly
important. However, since the code here is executed significantly more
frequently, and this change isn't particularly complicated, let's err
on the size of efficiency if we can help it.
Note that because of the way the constants are represented now, they're
ever so slightly different from before, so this change technically isn't
a total no-op. In practice however, it should be. These constants are
fuzzy and hand-picked anyway, so having them shift a little is unlikely
to make a significant change to the behavior of the GC.
For #48409.
Change-Id: Iabb2385920f7d891b25040226f35a3f31b7bf844
Reviewed-on: https://go-review.googlesource.com/c/go/+/397015
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
As it stands, the heap goal and the trigger are set once by
gcController.commit, and then read out of gcController. However with the
coming memory limit we need the GC to be able to respond to changes in
non-heap memory. The simplest way of achieving this is to compute the
heap goal and its associated trigger dynamically.
In order to make this easier to implement, the GC trigger is now based
on the heap goal, as opposed to the status quo of computing both
simultaneously. In many cases we just want the heap goal anyway, not
both, but we definitely need the goal to compute the trigger, because
the trigger's bounds are entirely based on the goal (the initial runway
is not). A consequence of this is that we can't rely on the trigger to
enforce a minimum heap size anymore, and we need to lift that up
directly to the goal. Specifically, we need to lift up any part of the
calculation that *could* put the trigger ahead of the goal. Luckily this
is just the heap minimum and minimum sweep distance. In the first case,
the pacer may behave slightly differently, as the heap minimum is no
longer the minimum trigger, but the actual minimum heap goal. In the
second case it should be the same, as we ensure the additional runway
for sweeping is added to both the goal *and* the trigger, as before, by
computing that in gcControllerState.commit.
There's also another place we update the heap goal: if a GC starts and
we triggered beyond the goal, we always ensure there's some runway.
That calculation uses the current trigger, which violates the rule of
keeping the goal based on the trigger. Notice, however, that using the
precomputed trigger for this isn't even quite correct: due to a bug, or
something else, we might trigger a GC beyond the precomputed trigger.
So this change also adds a "triggered" field to gcControllerState that
tracks the point at which a GC actually triggered. This is independent
of the precomputed trigger, so it's fine for the heap goal calculation
to rely on it. It also turns out, there's more than just that one place
where we really should be using the actual trigger point, so this change
fixes those up too.
Also, because the heap minimum is set by the goal and not the trigger,
the maximum trigger calculation now happens *after* the goal is set, so
the maximum trigger actually does what I originally intended (and what
the comment says): at small heaps, the pacer picks 95% of the runway as
the maximum trigger. Currently, the pacer picks a small trigger based
on a not-yet-rounded-up heap goal, so the trigger gets rounded up to the
goal, and as per the "ensure there's some runway" check, the runway ends
up at always being 64 KiB. That check is supposed to be for exceptional
circumstances, not the status quo. There's a test introduced in the last
CL that needs to be updated to accomodate this slight change in
behavior.
So, this all sounds like a lot that changed, but what we're talking about
here are really, really tight corner cases that arise from situations
outside of our control, like pathologically bad behavior on the part of
an OS or CPU. Even in these corner cases, it's very unlikely that users
will notice any difference at all. What's more important, I think, is
that the pacer behaves more closely to what all the comments describe,
and what the original intent was.
Another note: at first, one might think that computing the heap goal and
trigger dynamically introduces some raciness, but not in this CL: the heap
goal and trigger are completely static.
Allocation outside of a GC cycle may now be a bit slower than before, as
the GC trigger check is now significantly more complex. However, note
that this executes basically just as often as gcController.revise, and
that makes up for a vanishingly small part of any CPU profile. The next
CL cleans up the floating point multiplications on this path
nonetheless, just to be safe.
For #48409.
Change-Id: I280f5ad607a86756d33fb8449ad08555cbee93f9
Reviewed-on: https://go-review.googlesource.com/c/go/+/397014
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Fundamentally, all of these memstats exist to serve the runtime in
managing memory. For the sake of simpler testing, couple these stats
more tightly with the GC.
This CL was mostly done automatically. The fields had to be moved
manually, but the references to the fields were updated via
gofmt -w -r 'memstats.<field> -> gcController.<field>' *.go
For #48409.
Change-Id: Ic036e875c98138d9a11e1c35f8c61b784c376134
Reviewed-on: https://go-review.googlesource.com/c/go/+/397678
Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
The inconsistent heaps stats in memstats are a bit messy. Primarily,
heap_sys is non-orthogonal with heap_released and heap_inuse. In later
CLs, we're going to want heap_sys-heap_released-heap_inuse, so clean
this up by replacing heap_sys with an orthogonal metric: heapFree.
heapFree represents page heap memory that is free but not released.
I think this change also simplifies a lot of reasoning about these
stats; it's much clearer what they mean, and to obtain HeapSys for
memstats, we no longer need to do the strange subtraction from heap_sys
when allocating specifically non-heap memory from the page heap.
Because we're removing heap_sys, we need to replace it with a sysMemStat
for mem.go functions. In this case, heap_released is the most
appropriate because we increase it anyway (again, non-orthogonality). In
which case, it makes sense for heap_inuse, heap_released, and heapFree
to become more uniform, and to just represent them all as sysMemStats.
While we're here and messing with the types of heap_inuse and
heap_released, let's also fix their names (and last_heap_inuse's name)
up to the more modern Go convention of camelCase.
For #48409.
Change-Id: I87fcbf143b3e36b065c7faf9aa888d86bd11710b
Reviewed-on: https://go-review.googlesource.com/c/go/+/397677
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This change adds a field to memstats called mappedReady that tracks how
much memory is in the Ready state at any given time. In essence, it's
the total memory usage by the Go runtime (with one exception which is
documented). Essentially, all memory mapped read/write that has either
been paged in or will soon.
To make tracking this not involve the many different stats that track
mapped memory, we track this statistic at a very low level. The downside
of tracking this statistic at such a low level is that it managed to
catch lots of situations where the runtime wasn't fully accounting for
memory. This change rectifies these situations by always accounting for
memory that's mapped in some way (i.e. always passing a sysMemStat to a
mem.go function), with *two* exceptions.
Rectifying these situations means also having the memory mapped during
testing being accounted for, so that tests (i.e. ReadMemStats) that
ultimately check mappedReady continue to work correctly without special
exceptions. We choose to simply account for this memory in other_sys.
Let's talk about the exceptions. The first is the arenas array for
finding heap arena metadata from an address is mapped as read/write in
one large chunk. It's tens of MiB in size. On systems with demand
paging, we assume that the whole thing isn't paged in at once (after
all, it maps to the whole address space, and it's exceedingly difficult
with today's technology to even broach having as much physical memory as
the total address space). On systems where we have to commit memory
manually, we use a two-level structure.
Now, the reason why this is an exception is because we have no mechanism
to track what memory is paged in, and we can't just account for the
entire thing, because that would *look* like an enormous overhead.
Furthermore, this structure is on a few really, really critical paths in
the runtime, so doing more explicit tracking isn't really an option. So,
we explicitly don't and call sysAllocOS to map this memory.
The second exception is that we call sysFree with no accounting to clean
up address space reservations, or otherwise to throw out mappings we
don't care about. In this case, also drop down to a lower level and call
sysFreeOS to explicitly avoid accounting.
The third exception is debuglog allocations. That is purely a debugging
facility and ideally we want it to have as small an impact on the
runtime as possible. If we include it in mappedReady calculations, it
could cause GC pacing shifts in future CLs, especailly if one increases
the debuglog buffer sizes as a one-off.
As of this CL, these are the only three places in the runtime that would
pass nil for a stat to any of the functions in mem.go. As a result, this
CL makes sysMemStats mandatory to facilitate better accounting in the
future. It's now much easier to grep and find out where accounting is
explicitly elided, because one doesn't have to follow the trail of
sysMemStat nil pointer values, and can just look at the function name.
For #48409.
Change-Id: I274eb467fc2603881717482214fddc47c9eaf218
Reviewed-on: https://go-review.googlesource.com/c/go/+/393402
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
This change adds a parser for the GOMEMLIMIT environment variable's
input. This environment variable accepts a number followed by an
optional prefix expressing the unit. Acceptable units include
B, KiB, MiB, GiB, TiB, where *iB is a power-of-two byte unit.
For #48409.
Change-Id: I6a3b4c02b175bfcf9c4debee6118cf5dda93bb6f
Reviewed-on: https://go-review.googlesource.com/c/go/+/393400
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
This change adds a GC CPU utilization limiter to the GC. It disables
assists to ensure GC CPU utilization remains under 50%. It uses a leaky
bucket mechanism that will only fill if GC CPU utilization exceeds 50%.
Once the bucket begins to overflow, GC assists are limited until the
bucket empties, at the risk of GC overshoot. The limiter is primarily
updated by assists. The scheduler may also update it, but only if the
GC is on and a few milliseconds have passed since the last update. This
second case exists to ensure that if the limiter is on, and no assists
are happening, we're still updating the limiter regularly.
The purpose of this limiter is to mitigate GC death spirals, opting to
use more memory instead.
This change turns the limiter on always. In practice, 50% overall GC CPU
utilization is very difficult to hit unless you're trying; even the most
allocation-heavy applications with complex heaps still need to do
something with that memory. Note that small GOGC values (i.e.
single-digit, or low teens) are more likely to trigger the limiter,
which means the GOGC tradeoff may no longer be respected. Even so, it
should still be relatively rare.
This change also introduces the feature flag for code to support the
memory limit feature.
For #48409.
Change-Id: Ia30f914e683e491a00900fd27868446c65e5d3c2
Reviewed-on: https://go-review.googlesource.com/c/go/+/353989
Reviewed-by: Michael Pratt <mpratt@google.com>
Type parameter lists starting with the form [name *T|...] or
[name (X)|...] may look like an array length expression [x].
Only after parsing the entire initial expression and checking
whether the expression contains type elements or is followed
by a comma can we make the final decision.
This change simplifies the existing parsing strategy: instead
of trying to make an upfront decision with limited information
(which is insufficient), the parser now parses the start of a
type parameter list or array length specification as expression.
In a second step, if the expression can be split into a name
followed by a type element, or a name followed by an ordinary
expression which is succeeded by a comma, we assume a type
parameter list (because it can't be an array length).
In all other cases we assume an array length specification.
Fixes#49482.
Change-Id: I269b6291999bf60dc697d33d24a5635f01e065b9
Reviewed-on: https://go-review.googlesource.com/c/go/+/402256
Reviewed-by: Benny Siegert <bsiegert@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
In #52529, we observed that checking types for duplicate fields and
methods during method collection can result in incorrect early expansion
of the base type. Fix this by delaying the check for duplicate fields.
Notably, we can't delay the check for duplicate methods as we must
preserve the invariant that added method names are unique.
After this change, it may be possible in the presence of errors to have
a type-checked type containing a method name that conflicts with a field
name. With the previous logic conflicting methods would have been
skipped. This is a change in behavior, but only for invalid code.
Preserving the existing behavior would likely require delaying method
collection, which could have more significant consequences.
As a result of this change, the compiler test fixedbugs/issue28268.go
started passing with types2, being previously marked as broken. The fix
was not actually related to the duplicate method error, but rather the
fact that we stopped reporting redundant errors on the calls to x.b()
and x.E(), because they are now (valid!) methods.
Fixes#52529
Change-Id: I850ce85c6ba76d79544f46bfd3deb8538d8c7d00
Reviewed-on: https://go-review.googlesource.com/c/go/+/403455
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
The compressor methods already have logic for handling a sticky error.
Merge the logic from CL 136475 into that.
This slightly changes the error message to be more sensible
in the situation where it's returned by Flush.
Updates #27741
Change-Id: Ie34cf3164d0fa6bd0811175ca467dbbcb3be1395
Reviewed-on: https://go-review.googlesource.com/c/go/+/403514
Reviewed-by: Dmitri Shuralyov <dmitshur@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>
The loopreschedchecks pass (GOEXPERIMENT=preemptibleloops) had
bit-rotted in two ways because of the regabi experiment:
1. The call to goschedguarded was generating a pre-regabi StaticCall.
This CL updates it to construct a new-style StaticCall.
2. The mem finder did not account for tuples or results containing a
mem. This caused it to construct phis that were supposed to thread
the mem into the added blocks, but they could instead thread a
tuple or results containing a mem, causing things to go wrong
later. This CL updates the mem finder to add an op to select out
the mem if it finds the last live mem in a block is a tuple or
results. This isn't ideal since we'll deadcode out most of these,
but it's the easiest thing to do and this is just an experiment.
Tested by running the runtime tests. Ideally we'd have a real test for
this, but I don't think it's worth the effort for code that clearly
hasn't been enabled by anyone for at least a year.
Change-Id: I8ed01207637c454b68a551d38986c947e17d520b
Reviewed-on: https://go-review.googlesource.com/c/go/+/403475
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
This CL:
1. extracts typecheck.LookupNum into a method on *types.Pkg, so that
it can be used with any package, not just types.LocalPkg,
2. adds a new helper function closureSym to generate symbols in the
appropriate package as needed within stencil.go, and
3. updates the existing typecheck.LookupNum+Name.SetSym code to call
closureSym instead.
No functional change (so no need to backport to Go 1.18), but a little
cleaner, and avoids polluting types.LocalPkg.Syms with symbols that we
won't end up using.
Updates #52117.
Change-Id: Ifc8a3b76a37c830125e9d494530d1f5b2e3e3e2a
Reviewed-on: https://go-review.googlesource.com/c/go/+/403197
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
We added internal/execabs back in January 2021 in order to fix
a security problem caused by os/exec's handling of the current
directory. Now that os/exec has that code, internal/execabs is
superfluous and can be deleted.
This commit rewrites all the imports back to os/exec and
deletes internal/execabs.
For #43724.
Change-Id: Ib9736baf978be2afd42a1225e2ab3fd5d33d19df
Reviewed-on: https://go-review.googlesource.com/c/go/+/381375
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Auto-Submit: Russ Cox <rsc@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
cmd/internal/moddeps was failing.
Ran the commands it suggested:
% go mod tidy # to remove extraneous dependencies
% go mod vendor # to vendor dependencies
% go generate -run=bundle std # to regenerate bundled packages
% go generate syscall internal/syscall/... # to regenerate syscall packages
cmd/internal/moddeps is happy now.
Change-Id: I4ee212cdc323f62a6cdcfdddb6813397b23d89e5
Reviewed-on: https://go-review.googlesource.com/c/go/+/403454
Run-TryBot: Russ Cox <rsc@golang.org>
Auto-Submit: Russ Cox <rsc@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
CL 381374 was reverted because x/sys/execabs broke.
This CL reapplies CL 381374, but adding a lookPathErr error
field back, for execabs to manipulate with reflect.
That field will just be a bit of scar tissue in this package forever,
to keep old code working with new toolchains.
CL 403256 fixes x/sys/execabs's test to be ready for the change.
Older versions of x/sys/execabs will keep working
(that is, will keep rejecting what they should reject),
but they will return a slightly different error from LookPath
without that CL, and the test fails because of the different
error text.
For #43724.
This reverts commit f2b674756b.
Change-Id: Iee55f8cd9939e1bd31e5cbdada50681cdc505117
Reviewed-on: https://go-review.googlesource.com/c/go/+/403274
Auto-Submit: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
The settleTime is arbitrary. Ideally we should refactor the test to
avoid it (using subprocesses instead of sleeps to isolate tests from
each others' delayed signals), but as a shorter-term workaround let's
try scaling it back to match linux/ppc64 (the other builder that
empirically requires a longer settleTime).
For #51054.
Updates #33174.
Change-Id: I574fffaadd74c52c13d63974e87f20b6d3cf3c4b
Reviewed-on: https://go-review.googlesource.com/c/go/+/403199
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
We need to use the same marker everywhere. My CL to rename the
marker (CL 241661) and the CL to add more uses of the marker
under the old name (CL 241678) weren't coordinated with each other.
Fixes#52612
Change-Id: I97023c0769e518491924ef457fe03bf64a2cefa6
Reviewed-on: https://go-review.googlesource.com/c/go/+/403094
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Following discussion on #43724, change os/exec to take the
approach of golang.org/x/sys/execabs, refusing to respect
path entries mentioning relative paths by default.
Code that insists on being able to find executables in relative
directories in the path will need to add a couple lines to override the error.
See the updated package docs in exec.go for more details.
Fixes#43724.
Fixes#43947.
Change-Id: I73c1214f322b60b4167a23e956e933d50470fe13
Reviewed-on: https://go-review.googlesource.com/c/go/+/381374
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Within the unified IR export format, I was treating package unsafe as
a normal package, but expecting importers to correctly handle
deduplicating it against their respective representation of package
unsafe.
However, the surrounding importer logic differs slightly between
cmd/compile/internal/noder (which unified IR was originally
implemented against) and go/importer (which it was more recently
ported to). In particular, noder initializes its packages map as
`map[string]*types2.Package{"unsafe": types2.Unsafe}`, whereas
go/importer initializes it as just `make(map[string]*types.Package)`.
This CL makes them all consistent. In particular, it:
1. changes noder to initialize packages to an empty map to prevent
further latent issues from the discrepency,
2. adds the same special handling of package unsafe already present in
go/internal/gcimporter's unified IR reader to both of cmd/compile's
implementations, and
3. changes the unified IR writer to treat package unsafe as a builtin
package, to force that readers similarly handle it correctly.
Fixes#52623.
Change-Id: Ibbab9b0a1d2a52d4cc91b56c5df49deedf81295a
Reviewed-on: https://go-review.googlesource.com/c/go/+/403196
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
replace for string's end trimming TrimFunc -> TrimRightFunc
strings.TrimSpace string's end trimming should use more specific TrimRightFunc instead of common TrimFunc (because start has already trimmed before)
Change-Id: I827f1a25c141e61edfe1f8b11f6e8cd685f8b384
GitHub-Last-Rev: 040607a831
GitHub-Pull-Request: golang/go#46862
Reviewed-on: https://go-review.googlesource.com/c/go/+/329731
Auto-Submit: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
This extends CL 402190 from Linux to the rest of the Unix OSes.
Marking sigtramp as TOPFRAME allows gentraceback to stop tracebacks at
the end of a signal handler, since there is not much beyond sigtramp.
Change-Id: I8b7f5d55d41889f59c0a79c65351b9b0b2d77717
Reviewed-on: https://go-review.googlesource.com/c/go/+/402934
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Austin Clements <austin@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Pratt <mpratt@google.com>
On LR machine, consider F calling G calling H, which grows stack.
The stack looks like
...
G's frame:
... locals ...
saved LR = return PC in F <- SP points here at morestack
H's frame (to be created)
At morestack, we save
gp.sched.pc = H's morestack call
gp.sched.sp = H's entry SP (the arrow above)
gp.sched.lr = return PC in G
Currently, when unwinding through morestack (if _TraceJumpStack
is set), we switch PC and SP but not LR. We then have
frame.pc = H's morestack call
frame.sp = H's entry SP (the arrow above)
As LR is not set, we load it from stack at *sp, so
frame.lr = return PC in F
As the SP hasn't decremented at the morestack call,
frame.fp = frame.sp = H's entry SP
Unwinding a frame, we have
frame.pc = old frame.lr = return PC in F
frame.sp = old frame.fp = H's entry SP a.k.a. G's SP
The PC and SP don't match. The unwinding will go off if F and G
have different frame sizes.
Fix this by preserving the LR when switching stack.
Also add code to detect infinite loop in unwinding.
TODO: add some test. I can reproduce the infinite loop (or throw
with added check) but the frequency is low.
May fix#52116.
Change-Id: I6e1294f1c6e55f664c962767a1cf6c466a0c0eff
Reviewed-on: https://go-review.googlesource.com/c/go/+/400575
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cherry Mui <cherryyz@google.com>
Reviewed-by: Eric Fang <eric.fang@arm.com>
Reviewed-by: Benny Siegert <bsiegert@gmail.com>
Now, 1.17 is the least supported version, the compiler always write
type information when exporting function bodies. So we can get rid of
go117ExportTypes constant and all its conditional checking codes.
Change-Id: I9ac616509c30601e94f99426049d814328253395
Reviewed-on: https://go-review.googlesource.com/c/go/+/402974
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Keith Randall <khr@golang.org>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
There are several tests in the runtime that need to force various
things to escape to the heap. This CL centralizes this functionality
into runtime.Escape, defined in export_test.
Change-Id: I2de2519661603ad46c372877a9c93efef8e7a857
Reviewed-on: https://go-review.googlesource.com/c/go/+/402178
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Austin Clements <austin@google.com>
Auto-Submit: Austin Clements <austin@google.com>
This gives explicit names to the possible states of throwing (-1, 0, 1).
m.throwing is now one of:
throwTypeOff: not throwing, previously == 0
throwTypeUser: user throw, previously == -1
throwTypeRuntime: runtime throw, previously == 1
For runtime throws, we now always include frame metadata and system
goroutines regardless of GOTRACEBACK to aid in debugging the runtime.
For user throws, we no longer include frame metadata or runtime frames,
unless GOTRACEBACK=system or higher.
For #51485.
Change-Id: If252e2377a0b6385ce7756b937929be4273a56c0
Reviewed-on: https://go-review.googlesource.com/c/go/+/390421
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Austin Clements <austin@google.com>
"User" throws are throws due to some invariant broken by the application.
"System" throws are due to some invariant broken by the runtime,
environment, etc (i.e., not the fault of the application).
This CL sends "user" throws through the new fatal. Currently this
function is identical to throw, but with a different name to clearly
differentiate the throw type in the stack trace, and hopefully be a bit
more clear to users what it means.
This CL changes a few categories of throw to fatal:
1. Concurrent map read/write.
2. Deadlock detection.
3. Unlock of unlocked sync.Mutex.
4. Inconsistent results from syscall.AllThreadsSyscall.
"Thread exhaustion" and "out of memory" (usually address space full)
throws are additional throws that are arguably the fault of user code,
but I've left off for now because there is no specific invariant that
they have broken to get into these states.
For #51485
Change-Id: I713276a6c290fd34a6563e6e9ef378669d74ae32
Reviewed-on: https://go-review.googlesource.com/c/go/+/390420
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
Currently throw() in the signal handler results in "fatal error: unknown
return pc from runtime.sigreturn ...".
Marking sigtramp as TOPFRAME allows gentraceback to stop tracebacks at
the end of a signal handler, since there is not much beyond sigtramp.
This is just done on Linux for now, but may apply to other Unix systems
as well.
Change-Id: I96edcb945283f417a5bfe00ce2fb2b1a0d578692
Reviewed-on: https://go-review.googlesource.com/c/go/+/402190
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TestAlias checks systematically for aliasing issues, where passing the
same value for an argument and the receiver leads to incorrect results.
We had a number of issues like that over the years:
- #31084: Lsh on arm64
- #30217: GCD
- #22830: Exp due to divLarge
- #22265: ModSqrt
- #20490: Add and Sub
- #11284: GCD
This CL also fixes two new minor bugs that the test found. A wrong
result would be returned by
- Exp when the modulo and the receiver alias
- Rand when the limit is negative and it aliases the receiver
The test runs in ~0.05s with the default -quickchecks value.
Change-Id: I8354069ec9886e40c60f2642342ee08e604befb7
Reviewed-on: https://go-review.googlesource.com/c/go/+/168257
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <valsorda@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
2022-04-27 17:24:48 +00:00
1205 changed files with 74598 additions and 24506 deletions
The Go memory model specifies the conditions under which
@@ -22,7 +19,7 @@ observe values produced by writes to the same variable in a different goroutine.
</p>
<h2>Advice</h2>
<h3id="advice">Advice</h3>
<p>
Programs that modify data being simultaneously accessed by multiple goroutines
@@ -44,90 +41,237 @@ you are being too clever.
Don't be clever.
</p>
<h2>Happens Before</h2>
<h3id="overview">Informal Overview</h3>
<p>
Within a single goroutine, reads and writes must behave
as if they executed in the order specified by the program.
That is, compilers and processors may reorder the reads and writes
executed within a single goroutine only when the reordering
does not change the behavior within that goroutine
as defined by the language specification.
Because of this reordering, the execution order observed
by one goroutine may differ from the order perceived
by another. For example, if one goroutine
executes <code>a = 1; b = 2;</code>, another might observe
the updated value of <code>b</code> before the updated value of <code>a</code>.
Go approaches its memory model in much the same way as the rest of the language,
aiming to keep the semantics simple, understandable, and useful.
This section gives a general overview of the approach and should suffice for most programmers.
The memory model is specified more formally in the next section.
</p>
<p>
To specify the requirements of reads and writes, we define
<i>happens before</i>, a partial order on the execution
of memory operations in a Go program. If event <spanclass="event">e<sub>1</sub></span>happens
before event <spanclass="event">e<sub>2</sub></span>, then we say that <spanclass="event">e<sub>2</sub></span> happens after <spanclass="event">e<sub>1</sub></span>.
Also, if <spanclass="event">e<sub>1</sub></span> does not happen before <spanclass="event">e<sub>2</sub></span> and does not happen
after <spanclass="event">e<sub>2</sub></span>, then we say that <spanclass="event">e<sub>1</sub></span> and <spanclass="event">e<sub>2</sub></span> happen concurrently.
</p>
<pclass="rule">
Within a single goroutine, the happens-before order is the
order expressed by the program.
A <em>data race</em> is defined as
a write to a memory location happening concurrently with another read or write to that same location,
unless all the accesses involved are atomic data accesses as provided by the <code>sync/atomic</code>package.
As noted already, programmers are strongly encouraged to use appropriate synchronization
to avoid data races.
In the absence of data races, Go programs behave as if all the goroutines
were multiplexed onto a single processor.
This property is sometimes referred to as DRF-SC: data-race-free programs
execute in a sequentially consistent manner.
</p>
<p>
A read <spanclass="event">r</span> of a variable <code>v</code> is <i>allowed</i> to observe a write <spanclass="event">w</span> to <code>v</code>
if both of the following hold:
While programmers should write Go programs without data races,
there are limitations to what a Go implementation can do in response to a data race.
An implementation may always react to a data race by reporting the race and terminating the program.
Otherwise, each read of a single-word-sized or sub-word-sized memory location
must observe a value actually written to that location (perhaps by a concurrent executing goroutine)
and not yet overwritten.
These implementation constraints make Go more like Java or JavaScript,
in that most races have a limited number of outcomes,
and less like C and C++, where the meaning of any program with a race
is entirely undefined, and the compiler may do anything at all.
Go's approach aims to make errant programs more reliable and easier to debug,
while still insisting that races are errors and that tools can diagnose and report them.
</p>
<h2id="model">Memory Model</h2>
<p>
The following formal definition of Go's memory model closely follows
the approach presented by Hans-J. Boehm and Sarita V. Adve in
“<ahref="https://www.hpl.hp.com/techreports/2008/HPL-2008-56.pdf">Foundations of the C++ Concurrency Memory Model</a>”,
published in PLDI 2008.
The definition of data-race-free programs and the guarantee of sequential consistency
for race-free progams are equivalent to the ones in that work.
</p>
<p>
The memory model describes the requirements on program executions,
which are made up of goroutine executions,
which in turn are made up of memory operations.
</p>
<p>
A <i>memory operation</i> is modeled by four details:
</p>
<ul>
<li>its kind, indicating whether it is an ordinary data read, an ordinary data write,
or a <i>synchronizing operation</i> such as an atomic data access,
a mutex operation, or a channel operation,
<li>its location in the program,
<li>the memory location or variable being accessed, and
<li>the values read or written by the operation.
</ul>
<p>
Some memory operations are <i>read-like</i>, including read, atomic read, mutex lock, and channel receive.
Other memory operations are <i>write-like</i>, including write, atomic write, mutex unlock, channel send, and channel close.
Some, such as atomic compare-and-swap, are both read-like and write-like.
</p>
<p>
A <i>goroutine execution</i> is modeled as a set of memory operations executed by a single goroutine.
</p>
<p>
<b>Requirement 1</b>:
The memory operations in each goroutine must correspond to a correct sequential execution of that goroutine,
given the values read from and written to memory.
That execution must be consistent with the <i>sequenced before</i> relation,
defined as the partial order requirements set out by the <ahref="/ref/spec">Go language specification</a>
for Go's control flow constructs as well as the <ahref="/ref/spec#Order_of_evaluation">order of evaluation for expressions</a>.
</p>
<p>
A Go <i>program execution</i> is modeled as a set of goroutine executions,
together with a mapping <i>W</i> that specifies the write-like operation that each read-like operation reads from.
(Multiple executions of the same program can have different program executions.)
</p>
<p>
<b>Requirement 2</b>:
For a given program execution, the mapping <i>W</i>, when limited to synchronizing operations,
must be explainable by some implicit total order of the synchronizing operations
that is consistent with sequencing and the values read and written by those operations.
</p>
<p>
The <i>synchronized before</i> relation is a partial order on synchronizing memory operations,
derived from <i>W</i>.
If a synchronizing read-like memory operation <i>r</i>
observes a synchronizing write-like memory operation <i>w</i>
(that is, if <i>W</i>(<i>r</i>) = <i>w</i>),
then <i>w</i> is synchronized before <i>r</i>.
Informally, the synchronized before relation is a subset of the implied total order
mentioned in the previous paragraph,
limited to the information that <i>W</i> directly observes.
</p>
<p>
The <i>happens before</i> relation is defined as the transitive closure of the
union of the sequenced before and synchronized before relations.
</p>
<p>
<b>Requirement 3</b>:
For an ordinary (non-synchronizing) data read <i>r</i> on a memory location <i>x</i>,
<i>W</i>(<i>r</i>) must be a write <i>w</i> that is <i>visible</i> to <i>r</i>,
where visible means that both of the following hold:
<ol>
<li><spanclass="event">r</span> does not happen before <spanclass="event">w</span>.</li>
<li>There is no other write <spanclass="event">w'</span> to <code>v</code> that happens
after <spanclass="event">w</span> but before <spanclass="event">r</span>.</li>
<li><i>w</i> happens before <i>r</i>.
<li><i>w</i> does not happen before any other write <i>w'</i>(to <i>x</i>) that happens before <i>r</i>.
</ol>
<p>
To guarantee thata read <spanclass="event">r</span> of a variable<code>v</code> observes a
particular write <spanclass="event">w</span> to<code>v</code>, ensure that <spanclass="event">w</span> is the only
write <spanclass="event">r</span>is allowed to observe.
That is, <spanclass="event">r</span> is <i>guaranteed</i> to observe <spanclass="event">w</span> if both of the following hold:
</p>
<ol>
<li><spanclass="event">w</span> happens before <spanclass="event">r</span>.</li>
<li>Any other write to the shared variable <code>v</code>
either happens before <spanclass="event">w</span> or after <spanclass="event">r</span>.</li>
</ol>
<p>
This pair of conditions is stronger than the first pair;
it requires that there are no other writes happening
concurrently with <spanclass="event">w</span> or <spanclass="event">r</span>.
A <i>read-write data race</i> on memory location<i>x</i>
consists of a read-like memory operation<i>r</i> on <i>x</i>
and a write-like memory operation <i>w</i>on <i>x</i>,
at least one of which is non-synchronizing,
which are unordered by happens before
(that is, neither <i>r</i> happens before <i>w</i>
nor <i>w</i> happens before <i>r</i>).
</p>
<p>
Within a single goroutine,
there is no concurrency, so the two definitions are equivalent:
a read <spanclass="event">r</span> observes the value written by the most recent write <spanclass="event">w</span> to <code>v</code>.
When multiple goroutines access a shared variable <code>v</code>,
they must use synchronization events to establish
happens-before conditions that ensure reads observe the
desired writes.
A <i>write-write data race</i> on memory location <i>x</i>
consists of two write-like memory operations <i>w</i> and <i>w'</i> on <i>x</i>,
at least one of which is non-synchronizing,
which are unordered by happens before.
</p>
<p>
The initialization of variable<code>v</code> with the zero value
for <code>v</code>'s type behaves as a write in the memory model.
Note that if there are no read-write or write-write data races on memory location<i>x</i>,
then any read <i>r</i> on <i>x</i> has only one possible <i>W</i>(<i>r</i>):
the single <i>w</i> that immediately precedes it in the happens before order.
</p>
<p>
Reads and writes of values larger than a single machine word
behave as multiple machine-word-sized operations in an
unspecified order.
More generally, it can be shown that any Go program that is data-race-free,
meaning it has no program executions with read-write or write-write data races,
can only have outcomes explained by some sequentially consistent interleaving
of the goroutine executions.
(The proof is the same as Section 7 of Boehm and Adve's paper cited above.)
This property is called DRF-SC.
</p>
<h2>Synchronization</h2>
<p>
The intent of the formal definition is to match
the DRF-SC guarantee provided to race-free programs
by other languages, including C, C++, Java, JavaScript, Rust, and Swift.
</p>
<h3>Initialization</h3>
<p>
Certain Go language operations such as goroutine creation and memory allocation
act as synchronization opeartions.
The effect of these operations on the synchronized-before partial order
is documented in the “Synchronization” section below.
Individual packages are responsible for providing similar documentation
for their own operations.
</p>
<h2id="restrictions">Implementation Restrictions for Programs Containing Data Races</h2>
<p>
The preceding section gave a formal definition of data-race-free program execution.
This section informally describes the semantics that implementations must provide
for programs that do contain races.
</p>
<p>
First, any implementation can, upon detecting a data race,
report the race and halt execution of the program.
Implementations using ThreadSanitizer
(accessed with “<code>go</code><code>build</code><code>-race</code>”)
do exactly this.
</p>
<p>
Otherwise, a read <i>r</i> of a memory location <i>x</i>
that is not larger than a machine word must observe
some write <i>w</i> such that <i>r</i> does not happen before <i>w</i>
and there is no write <i>w'</i> such that <i>w</i> happens before <i>w'</i>
and <i>w'</i> happens before <i>r</i>.
That is, each read must observe a value written by a preceding or concurrent write.
</p>
<p>
Additionally, observation of acausal and “out of thin air” writes is disallowed.
</p>
<p>
Reads of memory locations larger than a single machine word
are encouraged but not required to meet the same semantics
as word-sized memory locations,
observing a single allowed write <i>w</i>.
For performance reasons,
implementations may instead treat larger operations
as a set of individual machine-word-sized operations
in an unspecified order.
This means that races on multiword data structures
can lead to inconsistent values not corresponding to a single write.
When the values depend on the consistency
of internal (pointer, length) or (pointer, type) pairs,
as can be the case for interface values, maps,
slices, and strings in most Go implementations,
such races can in turn lead to arbitrary memory corruption.
</p>
<p>
Examples of incorrect synchronization are given in the
“Incorrect synchronization” section below.
</p>
<p>
Examples of the limitations on implementations are given in the
“Incorrect compilation” section below.
</p>
<h2id="synchronization">Synchronization</h2>
<h3id="init">Initialization</h3>
<p>
Program initialization runs in a single goroutine,
@@ -141,15 +285,15 @@ If a package <code>p</code> imports package <code>q</code>, the completion of
</p>
<pclass="rule">
The start of the function<code>main.main</code>happens after
all<code>init</code> functions have finished.
The completion of all<code>init</code>functions is synchronized before
the start of the function<code>main.main</code>.
</p>
<h3>Goroutine creation</h3>
<h3id="go">Goroutine creation</h3>
<pclass="rule">
The <code>go</code> statement that starts a new goroutine
happens before the goroutine's execution begins.
is synchronized before the start of the goroutine's execution.
</p>
<p>
@@ -174,11 +318,12 @@ calling <code>hello</code> will print <code>"hello, world"</code>
at some point in the future (perhaps after <code>hello</code> has returned).
</p>
<h3>Goroutine destruction</h3>
<h3id="goexit">Goroutine destruction</h3>
<p>
The exit of a goroutine is not guaranteed to happen before
any event in the program. For example, in this program:
The exit of a goroutine is not guaranteed to be synchronized before
any event in the program.
For example, in this program:
</p>
<pre>
@@ -203,7 +348,7 @@ use a synchronization mechanism such as a lock or channel
communication to establish a relative ordering.
</p>
<h3>Channel communication</h3>
<h3id="chan">Channel communication</h3>
<p>
Channel communication is the main method of synchronization
@@ -213,8 +358,8 @@ usually in a different goroutine.
</p>
<pclass="rule">
A send on a channel happens before the corresponding
receive from that channel completes.
A send on a channel is synchronized before the completion of the
corresponding receive from that channel.
</p>
<p>
@@ -239,13 +384,13 @@ func main() {
<p>
is guaranteed to print <code>"hello, world"</code>. The write to <code>a</code>
happens before the send on <code>c</code>, which happens before
the corresponding receive on <code>c</code> completes, which happens before
is sequenced before the send on <code>c</code>, which is synchronized before
the corresponding receive on <code>c</code> completes, which is sequenced before
the <code>print</code>.
</p>
<pclass="rule">
The closing of a channel happens before a receive that returns a zero value
The closing of a channel is synchronized before a receive that returns a zero value
because the channel is closed.
</p>
@@ -256,8 +401,8 @@ yields a program with the same guaranteed behavior.
</p>
<pclass="rule">
A receive from an unbuffered channel happens before
the send on that channel completes.
A receive from an unbuffered channel is synchronized before the completion of
the corresponding send on that channel.
</p>
<p>
@@ -283,8 +428,8 @@ func main() {
<p>
is also guaranteed to print <code>"hello, world"</code>. The write to <code>a</code>
happens before the receive on <code>c</code>, which happens before
the corresponding send on <code>c</code> completes, which happens
is sequenced before the receive on <code>c</code>, which is synchronized before
the corresponding send on <code>c</code> completes, which is sequenced
before the <code>print</code>.
</p>
@@ -296,7 +441,7 @@ crash, or do something else.)
</p>
<pclass="rule">
The <i>k</i>th receive on a channel with capacity <i>C</i>happens before the <i>k</i>+<i>C</i>th send from that channel completes.
The <i>k</i>th receive on a channel with capacity <i>C</i>is synchronized before the completion of the <i>k</i>+<i>C</i>th send from that channel completes.
</p>
<p>
@@ -330,7 +475,7 @@ func main() {
}
</pre>
<h3>Locks</h3>
<h3id="locks">Locks</h3>
<p>
The <code>sync</code> package implements two lock data types,
@@ -339,7 +484,7 @@ The <code>sync</code> package implements two lock data types,
<pclass="rule">
For any <code>sync.Mutex</code> or <code>sync.RWMutex</code> variable <code>l</code> and <i>n</i><<i>m</i>,
call <i>n</i> of <code>l.Unlock()</code>happens before call <i>m</i> of <code>l.Lock()</code> returns.
call <i>n</i> of <code>l.Unlock()</code>is synchronized before call <i>m</i> of <code>l.Lock()</code> returns.
</p>
<p>
@@ -365,19 +510,29 @@ func main() {
<p>
is guaranteed to print <code>"hello, world"</code>.
The first call to <code>l.Unlock()</code> (in <code>f</code>) happens
The first call to <code>l.Unlock()</code> (in <code>f</code>) is synchronized
before the second call to <code>l.Lock()</code> (in <code>main</code>) returns,
which happens before the <code>print</code>.
which is sequenced before the <code>print</code>.
</p>
<pclass="rule">
For any call to <code>l.RLock</code> on a <code>sync.RWMutex</code> variable <code>l</code>,
there is an <i>n</i> such that the <code>l.RLock</code> happens (returns) after call <i>n</i> to
<code>l.Unlock</code> and the matching<code>l.RUnlock</code> happens
before call <i>n</i>+1 to <code>l.Lock</code>.
there is an <i>n</i> such that the <i>n</i>th call to <code>l.Unlock</code>
is synchronized before the return from<code>l.RLock</code>,
and the matching call to <code>l.RUnlock</code> is synchronized before the return from call <i>n</i>+1 to <code>l.Lock</code>.
</p>
<h3>Once</h3>
<pclass="rule">
A successful call to <code>l.TryLock</code> (or <code>l.TryRLock</code>)
is equivalent to a call to <code>l.Lock</code> (or <code>l.RLock</code>).
An unsuccessful call has no synchronizing effect at all.
// TODO(mdempsky): Remove after submitting CL 393715? This matches
// how PkgFuncName has historically handled local functions, but
// drchase points out it contradicts the documentation.
ifpkg==types.LocalPkg{
returns.Name
}
returnp+"."+s.Name
returnpkg.Path+"."+s.Name
}
varCurFunc*Func
Some files were not shown because too many files have changed in this diff
Show More
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
