This change adds support for identifying cleanups and finalizers
attached to tiny blocks to checkfinalizers mode. It also notes a subtle
pitfall, which is that the cleanup arg, if tiny-allocated, could end up
co-located with the object with the cleanup attached! Oops...
For #72949.
Change-Id: Icbe0112f7dcfc63f35c66cf713216796a70121ce
Reviewed-on: https://go-review.googlesource.com/c/go/+/662037
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
This change adds a new special kind called CheckFinalizer which is used
to annotate finalizers and cleanups with extra information about where
that cleanup or finalizer came from.
For #72949.
Change-Id: I3c1ace7bd580293961b7f0ea30345a6ce956d340
Reviewed-on: https://go-review.googlesource.com/c/go/+/662135
Reviewed-by: Carlos Amedee <carlos@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Windows API's TransmitFile function is limited to two concurrent
operations on workstation and client versions of Windows. This change
modifies the net.sendFile function to perform no work in such cases
so that TransmitFile is avoided.
Fixes#73746
Change-Id: Iba70d5d2758bf986e80c78254c8e9e10b39bb368
GitHub-Last-Rev: 315ddc0cd8
GitHub-Pull-Request: golang/go#73758
Reviewed-on: https://go-review.googlesource.com/c/go/+/673855
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Quim Muntal <quimmuntal@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
Before this change, when we tried to compute the set of packages in
'all', we'd add packages with invalid import paths to the set and try to
load them, which would fail. Instead, do not add them to the list of
packages to load in the second iteration of the loader. We'll still
return errors for invalid imports in the importing packages.
Change-Id: I682229011f555ed1d0c827f79100c1c43bf7f93a
Reviewed-on: https://go-review.googlesource.com/c/go/+/673655
Reviewed-by: Michael Matloob <matloob@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This new debug mode detects cleanup/finalizer leaks using checkmark
mode. It runs a partial GC using only specials as roots. If the GC can
find a path from one of these roots back to the object the special is
attached to, then the object might never be reclaimed. (The cycle could
be broken in the future, but it's almost certainly a bug.)
This debug mode is very barebones. It contains no type information and
no stack location for where the finalizer or cleanup was created.
For #72949.
Change-Id: Ibffd64c1380b51f281950e4cfe61f677385d42a5
Reviewed-on: https://go-review.googlesource.com/c/go/+/634599
Reviewed-by: Carlos Amedee <carlos@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
When an errorcheck test uses -m and instantiates an imported generic
function, the errors will include -m messages from the imported package
(since the new function has not previously been walked). These errors
cannot be matched since we can't write errors in files outside the test
input.
To fix this (and enable the other CLs in this stack), drop any unmatched
errors that occur in files outside those in the input set.
Change-Id: I2fcf0dd4693125d2e5823ea4437011730d8b1b1f
Reviewed-on: https://go-review.googlesource.com/c/go/+/672515
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
We want gcflags, which control builder type (e.g. noopt) to be used
for these tests also.
Should fix noopt and maybe other builders.
Change-Id: Iad34beab51714f0c38989ec0fc8778cf79087f72
Reviewed-on: https://go-review.googlesource.com/c/go/+/674455
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
This adds additional logging for the work that walk does to reduce
how often an interface conversion results in an allocation.
Also, as part of #71359, we will be updating how escape analysis and
walk handle basic literals, composite literals, and zero values,
so add some tests that uses this new logging.
By the end of our CL stack, we address all of these tests.
Updates #71359
Change-Id: I43fde8343d9aacaec1e05360417908014a86c8bd
Reviewed-on: https://go-review.googlesource.com/c/go/+/649076
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>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This change makes purego implementation of maphash.Comparable consistent
with the one in runtime and fixes hashing of channels.
Fixes#73657
Change-Id: If78a21d996f0c20c0224d4014e4a4177b09c3aa3
GitHub-Last-Rev: 2537216a1e
GitHub-Pull-Request: golang/go#73660
Reviewed-on: https://go-review.googlesource.com/c/go/+/671655
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: qiu laidongfeng2 <2645477756@qq.com>
This enable http.RoundTripper implementation to retry POST request (let's
say after a 500) after a 307/308 redirect.
Fixes#73439
Change-Id: I4365ff58b012c7f0d60e0317a08c98b1d48f657e
Reviewed-on: https://go-review.googlesource.com/c/go/+/666735
Reviewed-by: Sean Liao <sean@liao.dev>
Auto-Submit: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
CL 614257 refactored mallocgc but lost an optimization: if a span for a
large object is already backed by memory fresh from the OS (and thus
zeroed), we don't need to zero it. CL 614257 unconditionally zeroed
spans for large objects that contain pointers.
This change restores the optimization from before CL 614257, which seems
to matter in some real-world programs.
While we're here, let's also fix a hole with the garbage collector being
able to observe uninitialized memory of the large object is observed
by the conservative scanner before being published. The gory details are
in a comment in heapSetTypeLarge. In short, this change makes
span.largeType an atomic variable, such that the GC can only observe
initialized memory if span.largeType != nil.
Fixes#72991.
Change-Id: I2048aeb220ab363d252ffda7d980b8788e9674dc
Reviewed-on: https://go-review.googlesource.com/c/go/+/659956
Reviewed-by: Keith Randall <khr@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com>
Currently, it's possible for asynchronous preemption to observe a
partially initialized object. The sequence of events goes like this:
- The GC is in the mark phase.
- Thread T1 is allocating object O1.
- Thread T1 zeroes the allocation, runs the publication barrier, and
updates freeIndexForScan. It has not yet updated the mark bit on O1.
- Thread T2 is conservatively scanning some stack frame.
That stack frame has a dead pointer with the same address as O1.
- T2 picks up the pointer, checks isFree (which checks
freeIndexForScan without an import barrier), and sees that O1 is
allocated. It marks and queues O1.
- T2 then goes to scan O1, and observes uninitialized memory.
Although a publication barrier was executed, T2 did not have an import
barrier. T2 may thus observe T1's writes to zero the object out-of-order
with the write to freeIndexForScan.
Normally this would be impossible if T2 got a pointer to O1 from
somewhere written by T1. The publication barrier guarantees that if the
read side is data-dependent on the write side then we'd necessarily
observe all writes to O1 before T1 published it. However, T2 got the
pointer 'out of thin air' by scanning a stack frame with a dead pointer
on it.
One fix to this problem would be to add the import barrier in the
conservative scanner. We would then also need to put freeIndexForScan
behind the publication barrier, or make the write to freeIndexForScan
exactly that barrier.
However, there's a simpler way. We don't actually care if conservative
scanning observes a stale freeIndexForScan during the mark phase.
Newly-allocated memory is always marked at the point of allocation (the
allocate-black policy part of the GC's design). So it doesn't actually
matter that if the garbage collector scans that memory or not.
This change modifies the allocator to only update freeIndexForScan
outside the mark phase. This means freeIndexForScan is essentially
a snapshot of freeindex at the point the mark phase started. Because
there's no more race between conservative scanning and newly-allocated
objects, the complicated scenario above is no longer a possibility.
One thing we do have to be careful of is other callers of isFree.
Previously freeIndexForScan would always track freeindex, now it no
longer does. This change thus introduces isFreeOrNewlyAllocated which is
used by the conservative scanner, and uses freeIndexForScan. Meanwhile
isFree goes back to using freeindex like it used to. This change also
documents the requirement on isFree that the caller must have obtained
the pointer not 'out of thin air' but after the object was published.
isFree is not currently used anywhere particularly sensitive (heap dump
and checkmark mode, where the world is stopped in both cases) so using
freeindex is both conceptually simple and also safe.
Change-Id: If66b8c536b775971203fb4358c17d711c2944723
Reviewed-on: https://go-review.googlesource.com/c/go/+/672340
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
For modules that have already been indexed, we can skip ignored paths.
We already skip 'testdata' and '_' for this case so we can extend the
ignore directive for this case as well.
Updates: #42965
Change-Id: I076a242ba65c7b905b9dc65dcfb0a0247cbd68d6
Reviewed-on: https://go-review.googlesource.com/c/go/+/674076
Reviewed-by: Michael Matloob <matloob@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
Auto-Submit: Sam Thanawalla <samthanawalla@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Fast follow to golang.org/cl/636475 with a couple script tests that
build/runs a module that depends on a function inside a git repo using
sha256 hashes. (one with go get of a branch-name and the other
configuring go.mod directly)
Change-Id: Ief6c7efaf6d5c066dc54a3e4a63aad109f625abe
Reviewed-on: https://go-review.googlesource.com/c/go/+/672435
Reviewed-by: Michael Matloob <matloob@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
Auto-Submit: Sam Thanawalla <samthanawalla@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Sam Thanawalla <samthanawalla@google.com>
CL 518776 deleted the cmd/go/internal/modconv package and dropped the
ability to import dependency requirements from ~nine or so legacy
pre-module dependency configuration files. Part of the rationale from
Russ in 2023 for dropping that support was that "by now no one is
running into those configs anymore during 'go mod init'".
For two of those legacy file formats, Godeps.json and vendor.json, the
ability to import their listed dependencies was dropped in CL 518776,
but what remained for those two formats was the ability to guess the
resulting module name in the absence of a name being supplied to 'go mod
init'.
This could be explained by the fact that this smaller functionality for
guessing a module name was separate, did not rely on the deleted modconv
package, and instead only relied on simple JSON parsing.
The name guessing was helpful as part of the transition when module
support was initially released, but it was never perfect, including the
various third-party dependency managers did not all have the same naming
rules that were enforced by modules.
In short, it is very unlikely anyone is relying on this now, so we
delete it.
This CL was spawned from discussion in two related documentation CLs
(CL 662675 and CL 662695).
Updates #71537
Change-Id: I9e087aa296580239562a0ecee58913c5edc533ee
Reviewed-on: https://go-review.googlesource.com/c/go/+/664315
Reviewed-by: Michael Matloob <matloob@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Sam Thanawalla <samthanawalla@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Sam Thanawalla <samthanawalla@google.com>
Because that's what mallocgc did and some user code came to rely on it.
Fixes#73199
Change-Id: I45ca00d2ea448e6729ef9ac4cec3c1eb0ceccc89
Reviewed-on: https://go-review.googlesource.com/c/go/+/666116
Reviewed-by: t hepudds <thepudds1460@gmail.com>
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Keith Randall <khr@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
In regalloc, we allocate some values to registers before loop entry,
so that they don't need to be loaded (from spill locations) during
the loop.
But it is pointless if we've already regalloc'd the loop body.
Whatever restores we needed for the body are already generated.
It's not clear if this code is ever useful. No tests fail if I just
remove it. But at least this change is worthwhile. It doesn't help,
and it actively inserts more restores than we really need (mostly
because the desired register list is approximate - I have seen cases
where the loads implicated here end up being dead because the restores
hit the wrong registers and the edge shuffle pass knows it needs
the restores in different registers).
While we are here, might as well have layoutRegallocOrder return
the standard layout order instead of recomputing it.
Change-Id: Ia624d5121de59b6123492603695de50b272b277f
Reviewed-on: https://go-review.googlesource.com/c/go/+/672735
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: David Chase <drchase@google.com>
When appending, if the backing store doesn't escape and a
constant-sized backing store is big enough, use a constant-sized
stack-allocated backing store instead of allocating it from the heap.
cmd/go is <0.1% bigger.
As an example of how this helps, if you edit strings/strings.go:FieldsFunc
to replace
spans := make([]span, 0, 32)
with
var spans []span
then this CL removes the first 2 allocations that are part of the growth sequence:
│ base │ exp │
│ allocs/op │ allocs/op vs base │
FieldsFunc/ASCII/16-24 3.000 ± ∞ ¹ 2.000 ± ∞ ¹ -33.33% (p=0.008 n=5)
FieldsFunc/ASCII/256-24 7.000 ± ∞ ¹ 5.000 ± ∞ ¹ -28.57% (p=0.008 n=5)
FieldsFunc/ASCII/4096-24 11.000 ± ∞ ¹ 9.000 ± ∞ ¹ -18.18% (p=0.008 n=5)
FieldsFunc/ASCII/65536-24 18.00 ± ∞ ¹ 16.00 ± ∞ ¹ -11.11% (p=0.008 n=5)
FieldsFunc/ASCII/1048576-24 30.00 ± ∞ ¹ 28.00 ± ∞ ¹ -6.67% (p=0.008 n=5)
FieldsFunc/Mixed/16-24 2.000 ± ∞ ¹ 2.000 ± ∞ ¹ ~ (p=1.000 n=5)
FieldsFunc/Mixed/256-24 7.000 ± ∞ ¹ 5.000 ± ∞ ¹ -28.57% (p=0.008 n=5)
FieldsFunc/Mixed/4096-24 11.000 ± ∞ ¹ 9.000 ± ∞ ¹ -18.18% (p=0.008 n=5)
FieldsFunc/Mixed/65536-24 18.00 ± ∞ ¹ 16.00 ± ∞ ¹ -11.11% (p=0.008 n=5)
FieldsFunc/Mixed/1048576-24 30.00 ± ∞ ¹ 28.00 ± ∞ ¹ -6.67% (p=0.008 n=5)
(Of course, people have spotted and fixed a bunch of allocation sites
like this, but now we're ~automatically doing it everywhere going forward.)
No significant increases in frame sizes in cmd/go.
Change-Id: I301c4d9676667eacdae0058960321041d173751a
Reviewed-on: https://go-review.googlesource.com/c/go/+/664299
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Keith Randall <khr@golang.org>
-N+1 <= x % N <= N-1
This is useful for cases like:
func setBit(b []byte, i int) {
b[i/8] |= 1<<(i%8)
}
The shift does not need protection against larger-than-7 cases.
(It does still need protection against <0 cases.)
Change-Id: Idf83101386af538548bfeb6e2928cea855610ce2
Reviewed-on: https://go-review.googlesource.com/c/go/+/672995
Reviewed-by: Jorropo <jorropo.pgm@gmail.com>
Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Adding zero usually does not change the original value.
However, there is an exception with negative zero. (e.g. (-0) + (+0) = (+0))
This applies when x * y is negative and underflows.
Fixes#73757
Change-Id: Ib7b54bdacd1dcfe3d392802ea35cdb4e989f9371
GitHub-Last-Rev: 30d74883b2
GitHub-Pull-Request: golang/go#73759
Reviewed-on: https://go-review.googlesource.com/c/go/+/673856
Auto-Submit: Keith Randall <khr@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
This was just enabled in CL 643897. It seems to work fine on Linux, but
there are traceback issues on Darwin. We could disable just on Darwin,
but I'm not sure SIGSEGV inside of TSAN is something we care to support.
Fixes#73784.
Cq-Include-Trybots: luci.golang.try:gotip-darwin-arm64-race
Change-Id: I6a6a636cb15d7affaeb22c4c13d8f2a5c9bb31fd
Reviewed-on: https://go-review.googlesource.com/c/go/+/674276
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
Sync is used in the definition of primitives and documented by pkgbits.
It's not much help to also document it here.
Change-Id: I18bd0c7816f8249483550a1f0af7c76b9cfe09fb
Reviewed-on: https://go-review.googlesource.com/c/go/+/674156
Auto-Submit: Mark Freeman <mark@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Bypass: Mark Freeman <mark@golang.org>
ncpu is the total logical CPU count at startup. It is never updated. For
#73193, we will start using updated CPU counts for updated GOMAXPROCS,
making the ncpu name a bit ambiguous. Change to a less ambiguous name.
While we're at it, give the OS specific lookup functions a common name,
so it can be used outside of osinit later.
For #73193.
Change-Id: I6a6a636cf21cc60de36b211f3c374080849fc667
Reviewed-on: https://go-review.googlesource.com/c/go/+/672277
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
Moving to a smaller package allows its use in other internal/runtime
packages.
This isn't internal/strconvlite since it can't be used directly by
strconv.
For #73193.
Change-Id: I6a6a636c9c8b3f06b5fd6c07fe9dd5a7a37d1429
Reviewed-on: https://go-review.googlesource.com/c/go/+/672697
Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
The package section holds package stubs, which are a package
(path, name) pair and a series of declared imports.
Change-Id: If2a260c5e0a3522851be9808de46a3f128902002
Reviewed-on: https://go-review.googlesource.com/c/go/+/674175
Auto-Submit: Mark Freeman <mark@golang.org>
TryBot-Bypass: Mark Freeman <mark@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
This just wraps column width to 72 and indents production definitions
so they are easier to distinguish from prose.
Change-Id: I386b122b4f617db4b182ebb549fbee4f35a0122c
Reviewed-on: https://go-review.googlesource.com/c/go/+/673536
TryBot-Bypass: Mark Freeman <mark@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Mark Freeman <mark@golang.org>
Positions mostly borrow their representation from package syntax. Of
note, constants (such as the zero value for positions) are not encoded
directly. Rather, a flag typically signals such values.
Change-Id: I6b4bafc6e96bb21902dd2d6e164031e7dd5aabdd
Reviewed-on: https://go-review.googlesource.com/c/go/+/673535
TryBot-Bypass: Mark Freeman <mark@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Mark Freeman <mark@golang.org>
Explain that ServeMux.Handler doesn't populate the request with
matches.
Fixes#69623.
Change-Id: If625b3f8e8f4e54b05e1d9a86e8c471045e77763
Reviewed-on: https://go-review.googlesource.com/c/go/+/674095
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Chressie Himpel <chressie@google.com>
Reviewed-by: Sean Liao <sean@liao.dev>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
When a match involves a trailing-slash redirect, ServeMux.Handler now
returns the pattern that matched.
Fixes#73688.
Change-Id: I682d9cc9a3628bed8bf21139b98369ffa6c53792
Reviewed-on: https://go-review.googlesource.com/c/go/+/673815
Reviewed-by: Filippo Valsorda <filippo@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Damien Neil <dneil@google.com>
asancall and msancall are reachable from the signal handler, where we
are running on gsignal. Currently, these calls will use the g0 stack in
this case, but if the interrupted code was running on g0 this will
corrupt the stack and likely cause a crash.
As far as I know, racecall is not reachable from the signal handler, but
I have updated it as well for consistency.
This is the most straightforward fix, though it would be nice to
eventually migrate these wrappers to asmcgocall, which already handled
this case.
Fixes#71395.
Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-asan-clang15,gotip-linux-amd64-msan-clang15,gotip-linux-amd64-race
Change-Id: I6a6a636ccba826dd53e31c0e85b5d42fb1e98d12
Reviewed-on: https://go-review.googlesource.com/c/go/+/643875
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
The tests using testprog / testprogcgo are currently not covered on the
asan/msan/race builders because they don't build testprog with the
sanitizer flag.
Explicitly pass the flag if the test itself is built with the sanitizer.
There were a few tests that explicitly passed -race (even on non-race
builders). These tests will now only run on race builders.
For #71395.
Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-asan-clang15,gotip-linux-amd64-msan-clang15,gotip-linux-amd64-race
Change-Id: I6a6a636ce8271246316a80d426c0e4e2f6ab99c5
Reviewed-on: https://go-review.googlesource.com/c/go/+/643897
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
Previously, distpack filtered out tools from the packaged distribution
using a list of tools to remove. Instead follow mpratt's suggestion on
CL 666755 and instead filter out tools that are not on a list of tools
to keep. This will make it easier to tell which tools are actually in
the distribution.
For #71867
Change-Id: I8336465703ac820028c3381a0a743c457997e78a
Reviewed-on: https://go-review.googlesource.com/c/go/+/673696
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Michael Matloob <matloob@google.com>
This CL fixes a number of (all true positive) findings of vet's
copylock analyzer patched to treat the Bu{ff,uild}er types
as non-copyable after first use.
This does require imposing an additional indirection
between noder.writer and Encoder since the field is
embedded by value but its constructor now returns a pointer.
Updates golang/go#25907
Updates golang/go#47276
Change-Id: I0b4d77ac12bcecadf06a91709e695365da10766c
Reviewed-on: https://go-review.googlesource.com/c/go/+/635339
Reviewed-by: Robert Findley <rfindley@google.com>
Commit-Queue: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
This CL enables intrinsic support to emit the following prefetch
instructions for loong64 platform:
1.Prefetch - prefetches data from memory address to cache;
2.PrefetchStreamed - prefetches data from memory address, with a
hint that this data is being streamed.
Benchmarks picked from go/test/bench/garbage
Parameters tested with:
GOMAXPROCS=8
tree2 -heapsize=1000000000 -cpus=8
tree -n=18
parser
peano
Benchmarks Loongson-3A6000-HV @ 2500.00MHz:
| bench.old | bench.new |
| sec/op | sec/op vs base |
Tree2-8 1238.2µ ± 24% 999.9µ ± 453% ~ (p=0.089 n=10)
Tree-8 277.4m ± 1% 275.5m ± 1% ~ (p=0.063 n=10)
Parser-8 3.564 ± 0% 3.509 ± 1% -1.56% (p=0.000 n=10)
Peano-8 39.12m ± 2% 38.85m ± 2% ~ (p=0.353 n=10)
geomean 83.19m 78.28m -5.90%
Change-Id: I59e9aa4f609a106d4f70706e6d6d1fe6738ab72a
Reviewed-on: https://go-review.googlesource.com/c/go/+/671876
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Meidan Li <limeidan@loongson.cn>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: sophie zhao <zhaoxiaolin@loongson.cn>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Deeply nested parenthesized expressions could cause a stack
overflow during parsing. This change introduces a depth limit
(maxStackDepth) tracked in Tree.stackDepth to prevent this.
Additionally, this commit clarifies the security model in
the package documentation, noting that template authors
are trusted as text/template does not auto-escape.
Fixes#71201
Change-Id: Iab2c2ea6c193ceb44bb2bc7554f3fccf99a9542f
GitHub-Last-Rev: f4ebd1719f
GitHub-Pull-Request: golang/go#73670
Reviewed-on: https://go-review.googlesource.com/c/go/+/671755
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Sean Liao <sean@liao.dev>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Rob Pike <r@golang.org>
The "doc", "fix", and "covdata" tools invoked by the go command are not
needed for builds. Instead of invoking them directly using the installed
binary in the tool directory, use "go tool" to run them, building them
if needed. We can then stop distributing those tools in the
distribution.
covdata is used in tests and can form part of a cached test result, but
test results don't have the same requirements as build outputs to be
completely determined by the action id. We already don't include a
toolid for the covdata tool in the action id for a test run. The more
principled way to do things would be to load the covdata package,
create the actions to build it, and then depend on the output of
that action from the the test action and use that as the covdata tool.
For now, it's probably not worth the effort, but, in the future, if we
wanted to build a tool like cgo as needed, it would be best to build it
in the same action graph. That would introduce a whole bunch of complexity
because we'd need to build the tool in the host configuration, and all
the configuration parameters are global.
For #71867
Change-Id: Id9bbbb5c169296f66c072949f9da552424ecfa2f
Reviewed-on: https://go-review.googlesource.com/c/go/+/673119
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Michael Matloob <matloob@google.com>
This change removes some tools that are not used for builds, or
otherwise invoked by the go command (other than through "go tool"
itself) from the packaged distributions produced by distpack. When these
tools are missing, "go tool" will build and run them as needed.
Also update a case where we print a buildid commandline to specify
invoking buildid using "go tool" rather than the binary at it's install
location, because it may not exist there in packaged distributions
anymore.
The tools in this CL are the lowest hanging fruit. There are a few more
tools that aren't used by builds, but we'd have to get the go command to
run them using "go tool" rather than finding them in the tool install
directory.
For #71867
Change-Id: I217683bd549962a1add87405bf3fb1225e2333c5
Reviewed-on: https://go-review.googlesource.com/c/go/+/666755
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Michael Matloob <matloob@google.com>
The purpose of this change is to enable go get to be used when working
on a module that is usually built from a workspace and has unreleased
dependencies in that workspacae that have no requirements satisfying
them. These modules can't build in single module mode, and the
expectation is that a workspace will provide the unreleased
requirements.
Before this change, if go get was run in a module, and any of the
module's imports, that were not already satisfied using requirements,
could not be resolved from that module in single module mode, go get
would report an error. This could happen if, for example, the dependency
was unreleased, even privately, and couldn't be fetched using version
control or a proxy. go get would also do a check using
cmd/go/internal/modget.(*resolver).checkPackageProblems that, among
other things, any package patterns provided to go get (the pkgPattern
argument to checkPackageProblems) could properly load. When checking in
single-module mode, this would cause an error because imports in the
non-required workspace dependencies could not be resolved.
This change makes a couple of changes to address each of those problems.
First, while "go get" still uses the single module's module graph to
load packages and determine which imports are not satisfied by a module
in the build list (the set of modules the build is done with), it will
"cheat" and look up the set of modules that would be loaded in workspace
mode. It will not try to fetch modules to satisfy imports of packages in
those modules. (Alternatively, it could have tried to fetch modules to
satisfy the requirements, and allowed an error if it could not be found,
but we took the route of not doing the fetch to preserve the invariant
that the behavior wouldn't change if the network was down). The second,
and by far more complex, change is that the load that's done in
checkPackageProblems will be done in workspace mode rather than module
mode. While it is important that the requirements added by "go get" are
not determined using the workspace (with the necessary exception of the
skipped fetches) it is okay to use the workspace to load the modules,
as, if a go.work file is present, the go command would by default run
builds in workspace mode rather than single module mode. This more
relaxed check will allow get to succeed if a go list would succeed in
the workspace, even if it wouldn't from the single module.--
To avoid trying to satisfy imports that are in the other workspace
modules, we add a workspace field to the resolver that can be used to
check if an import is in the workspace. It reads the go.work file and
determines each of the modules' modroots. The hasPackage function will
call into modload logic using the new PkgIsInLocalModule function that
in turn calls into dirInModule to determine if the directory that would
contain the package sources exists in the module. We do that check in
cmd/go/internal/modget.(*resolver).loadPackages, which is used to
resolve modules to satisfy imports in the package graph supplied on the
command line. (Note that we do not skip resolving modules in the
functions that query for a package at a specific module version (such as
in "go get golang.org/x/tools/go/packages@latest), which are done in
(*resolver).queryPath. In that case, the user is explicitly requesting
to add a requirement on that package's module.)
The next step, checking for issues in the workspace mode, is more
complex because of two reasons. First, can't do all of
checkPackageProblems's work in a workspace, and second, that the module
loading state in the go command is global, so we have to manage the
global state in switching to workspace mode and back.
On the work that checkPackageProblems does: it broadly does three things:
first, a load of the packages specified to "go get", to make sure that
imports are satisfied and not ambiguous. Second, using that loaded
information, reporting any retracted or deprecated modules that are
relevant to the user and that they may want to take action on. And
third, checking that all the modules in the build list (the set of
module versions used in a load or build) have sums, and adding those
sums to the in-memory representation of the go.sum file that will be
written out at the end. When there's a workspace, the first two checks
need to be done in workspace mode so that we properly point out issues
in the build, but the sums need to be updated in module mode so that the
module's go.sum file is updated to reflect changes in go.mod.
To do the first two steps in workspace mode, we add a new
modload.EnterWorkspace function that will reset the global modload state
and load from the workspace, using the updated requirements that have
been calculated for the module. It returns a cleanup function that will
exit the workspace and reset to the previous global state. (We need the
previous global state because it holds the updated in memory
representations of go.mod and go.sum that will be written out by go get
if there are no errors.) We switch to workspace mode if there's a
relevant go.work file that would trigger a workspace load _and_ the
module go get is being run from belongs to that workspace (it wouldn't
make sense to use a workspace that the module itself didn't belong to).
We then switch back to module mode after the first two steps are
complete using the cleanup function. We have to be careful to finish
all the tasks checking for deprecations and retractions before to start
looking at the sums because they retraction and deprecation checking
tasks will depend on the global workspace state.
It's a bit unfortunate that much of the modload and modfetch state is
global. It's pretty gross doing the switch. It would be a lot of
work, but if we need to do a switch in a third instance other than for
go work sync and go get it might be worth doing the work to refactor
modload so that the state isn't global.
The EnterWorkspace function that does the switch to workspace mode (in
cmd/go/internal/modload/init.go) first saves the in memory
representation of the go.mod file (calculated using UpdateGoModFromReqs)
so that it can be applied in the workspace. It then uses the new
setState function to save the old state and reset to a clean state,
loads in workspace mode (using InitWorkfile to so that the go.work file
is used by LoadModFile), and then replaces the go.mod file
representation for the previous main module with the contents saved
earlier and reloads the requirements (holding the roots of the module
graph) to represent the updates. In workspace mode, the roots field of the
requirements is the same, but the reload is used to update the set of
direct requirements. rawGoModSummary is also update to use the in-
memory representation of the previous main module's go.mod file rather
than always reading it from disk so that it can take those updated
contents into account. (It previously didn't need to do this because
rawGoModSummary is primarily used to get module information for
dependency modules. The exception is in workspace mode but the same
logic worked because we didn't update workspace module's go.mod files in
a go command running in workspace mode). Finally, EnterWorkspace returns
a function that calls setState with the old state, to revert to the
previous state.
When we save the state of the modload package, we also need to save the
state of the modfetch package because it holds the state of the go.sum
file and the operation of looking up a value in the lookupCache or
downloadCache affects whether it appears in the go.sum file. lookupCache
and downloadCache are turned into pointers so that they can be saved in
the modfetchState.
Fixes#73654
Change-Id: I65cf835ec2293d4e3f66b91d3e77d3bb8d2f26d7
Reviewed-on: https://go-review.googlesource.com/c/go/+/669635
Reviewed-by: Michael Matloob <matloob@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Sam Thanawalla <samthanawalla@google.com>
Michael Anthony Knyszek
Julian Zhu
Shibi J M
Michael Matloob
Mateusz Poliwczak
Julian Zhu
Jake Bailey
khr@golang.org
thepudds
Vladislav Yarmak
Marc-Antoine Ruel
Sam Thanawalla
David Finkel
ICHINOSE Shogo
Michael Pratt
Mark Freeman
Jonathan Amsterdam
Alan Donovan
Guoqi Chen
limeidan
Ville Vesilehto