We encountered a new type of "no such process" error on loong64, it's like this
"Couldn't get NT_PRSTATUS registers: No such process.", I checked the source code
of gdb, NT_PRSTATUS is not fixed, it may be another name, so I use regular
expression here to match possible cases.
Updates #50838Fixes#74389
Change-Id: I3e3f7455b2dc6b8aa10c084f24f6a2a114790855
Reviewed-on: https://go-review.googlesource.com/c/go/+/684195
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: abner chenc <chenguoqi@loongson.cn>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
On Windows, GOMODCACHE almost never starts with a slash, and
"go doc -http" constructs a GOPROXY URL by doing "file://" + GOMODCACHE,
resulting in an invalid file URI.
For example, if GOMODCACHE is "C:\foo", then the file URI should be
"file:///C:/foo", but it becomes "file://C:/foo" instead, where "C:" is
understood as a host name, not a drive letter.
Fixes#74137.
Change-Id: I23e776e0f649a0062e01d1a4a6ea8268ba467331
Reviewed-on: https://go-review.googlesource.com/c/go/+/684575
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Michael Matloob <matloob@google.com>
The NeedmDeadlock test program currently has a 5-second timeout,
which is sort of arbitrary. It is long enough in regular mode
(which usually takes 0.0X seconds), but not quite so for
configurations like ASAN. Instead of using an arbitrary timeout,
just use the test's deadline. The test program is invoked with
testenv.Command, which will send it a SIGQUIT before the deadline
expires.
Fixes#56420 (at least for the asan builder).
Change-Id: I0b13651cb07241401837ca2e60eaa1b83275b093
Reviewed-on: https://go-review.googlesource.com/c/go/+/684697
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
In the Go language a type assertion of a nil interface value
will always report false:
var err error
v, ok := err.(error) // always reports (nil, false)
Consequently, assertion on a reflect.Value.Interface()
will also report false:
var err error
rv := ValueOf(&err).Elem()
v, ok := rv.Interface().(error) // reports (nil, false)
However, prior to this change, a TypeAssert would report true:
var err error
rv := ValueOf(&err).Elem()
v, ok := TypeAssert[error](rv) // reports (nil, true)
when it should report false.
This fixes TypeAssert to match the Go language by
pushing the typ != v.typ check to the very end after
we have validated that neither v nor T are interface kinds.
Fixes#74404
Change-Id: Ie14d5cf18c8370c3e27ce4bdf4570c89519d8a16
Reviewed-on: https://go-review.googlesource.com/c/go/+/684675
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Joseph Tsai <joetsai@digital-static.net>
Reviewed-by: Mateusz Poliwczak <mpoliwczak34@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
On Windows, the process might not have read permission on the parent
directory, but still can delete files in it. This change allows
RemoveAll to open the parent directory with minimal permissions, which
is sufficient for deleting child files.
Fixes#74134.
Cq-Include-Trybots: luci.golang.try:gotip-windows-amd64-longtest,gotip-windows-arm64
Change-Id: I5d5c5977caaebf6e0f93fb2313b0ceb346f70e05
Reviewed-on: https://go-review.googlesource.com/c/go/+/684515
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Damien Neil <dneil@google.com>
Add a section to the package doc which details the security
considerations of using encoding/json, in particular with respect to
parser misalignment issues.
Additionally, clarify previously ambiguous statement in the Unmarshal
doc about how case is used when matching keys in objects, and add a note
about how duplicate keys are handled.
Fixes#14750
Change-Id: I66f9b845efd98c86a684d7333b3aa8a456564922
Reviewed-on: https://go-review.googlesource.com/c/go/+/684315
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Joseph Tsai <joetsai@digital-static.net>
Auto-Submit: Roland Shoemaker <roland@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
If a goroutine is synchronously preempted, then taking a
frame-pointer-based stack trace at that preemption will skip PC of the
caller of the function which called into morestack. This happens because
the frame pointer is pushed to the stack after the preamble, leaving the
stack in an odd state for frame pointer unwinding.
Deal with this by marking a goroutine as synchronously preempted and
using that signal to load the missing PC from the stack. On LR platforms
this is available in gp.sched.lr. On non-LR platforms like x86, it's at
gp.sched.sp, because there are no args, no locals, and no frame pointer
pushed to the SP yet.
For #68090.
Change-Id: I73a1206d8b84eecb8a96dbe727195da30088f288
Reviewed-on: https://go-review.googlesource.com/c/go/+/684435
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Nick Ripley <nick.ripley@datadoghq.com>
The existing js/wasm implementation of RoundTrip calls abort() on the
fetch() call when the context is canceled but does not wait for for the
resulting promise to be rejected. The result is the failure callback for the
promise will be called at some later point in time when the promise
rejection is handled. In some case this callback may be called after the Go
program has exited resulting in "Go program has already exited" errors.
Fixes#57098
Change-Id: Ia37fd22cb9f667dbb0805ff5db0ceb8fdba7246b
Reviewed-on: https://go-review.googlesource.com/c/go/+/680937
Reviewed-by: 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>
Currently, if there is a BSS reference and a DATA symbol
definition with the same name, we pick the DATA symbol, as it
contains the right content. In this case, if the BSS reference
has a larger size, we error out, because it is not safe to access
a smaller symbol as if it has a larger size.
Sometimes code declares a global variable in Go and defines it
in assembly with content. They are expected to be of the same
size. However, in ASAN mode, we insert a red zone for the variable
on the Go side, making it have a larger size, whereas the assembly
symbol is unchanged. This causes the Go reference (BSS) has a
larger size than the assembly definition (DATA). It results in an
error currently. This code is valid and safe, so we should permit
that.
We support this case by increasing the symbol size to match the
larger size (of the BSS side). The symbol content (from the DATA
side) is kept. In some sense, we merge the two symbols. When
loading symbols, it is not easy to change its size (as the object
file may be mapped read-only), so we add it to a fixup list, and
fix it up later after all Go symbols are loaded. This is a very
rare case, so the list should not be long.
We could limit this to just ASAN mode. But it seems okay to allow
this in general. As long as the symbol has the larger size, it is
safe to access it with the larger size.
Fixes#74314.
Change-Id: I3ee6e46161d8f59500e2b81befea11e563355a57
Reviewed-on: https://go-review.googlesource.com/c/go/+/684236
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
CL 649035 and CL 649079 updated escape analysis to rewrite
certain operands in OMAKE and OCONVIFACE nodes from non-constant
expressions to basic literals that evaluate to the same value.
However, when doing that rewriting, we need to evaluate any
side effects prior to replacing the expression, which is what
this CL now does.
Issue #74379 reported a problem with OCONVIFACE nodes due to CL 649079.
CL 649035 has essentially the same issue with OMAKE nodes. To illustrate
that, we add a test for the OMAKE case in fixedbugs/issue74379b.go,
which fails without this change. To avoid introducing an unnecessary
temporary for OMAKE nodes, we also conditionalize the main work of
CL 649035 on whether the OMAKE operand is already an OLITERAL.
CL 649555 and CL 649078 were related changes that created read-only
global storage for composite literals used in an interface conversion.
This CL adds a test in fixedbugs/issue74379c.go to illustrate
that they do not have the same problem.
Updates #71359Fixes#74379
Change-Id: I6645575ef34f1fe2b0241a22dc205875d66b7ada
Reviewed-on: https://go-review.googlesource.com/c/go/+/684116
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Keith Randall <khr@google.com>
WARNING: This commit contains a breaking change.
This is permissible since jsontext is experimental and
not subject to the Go 1 compatibility agreement.
Existing callers of UnusedBuffer should use AvailableBuffer instead.
Updates #71497
Change-Id: Ib080caf306d545a8fb038e57f0817b18dd0f91cf
Reviewed-on: https://go-review.googlesource.com/c/go/+/683897
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
Reviewed-by: Damien Neil <dneil@google.com>
Auto-Submit: Joseph Tsai <joetsai@digital-static.net>
This follows the precedent set by:
bufio.Writer.AvailableBuffer
bytes.Buffer.AvailableBuffer
both with methods that return a zero-length buffer that
is intended to only be used with a following Write call.
This keeps the older UnusedBuffer method around so that
at least one commit that has both methods for migration purposes.
Updates #71497
Change-Id: I3815f593e09f645280ae5ad9cbdd63a6c147123b
Reviewed-on: https://go-review.googlesource.com/c/go/+/683896
Reviewed-by: Damien Neil <dneil@google.com>
TryBot-Bypass: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
The hugo binary gets slower, potentially dramatically so, with
GOEXPERIMENT=greenteagc. The root cause is page mapping churn. The Green
Tea code introduced a new implicit nil check on value in a
freshly-allocated span to clear some new heap metadata. This nil check
would read the fresh memory, causing Linux to back that virtual address
space with an RO page. This would then be almost immediately written to,
causing Linux to possibly flush the TLB and find memory to replace that
read-only page (likely deduplicated as just the zero page).
This CL fixes the issue by replacing the implicit nil check, which is a
memory read expected to fault if it's truly nil, with an explicit one.
The explicit nil check is a branch, and thus makes no reads to memory.
The result is that the hugo binary no longer gets slower.
No regression test because it doesn't seem possible without access to OS
internals, like Linux tracepoints. We briefly experimented with RSS
metrics, but they're inconsistent. Some system RSS metrics count the
deduplicated zero page, while others (like those produced by
/proc/self/smaps) do not.
Instead, we'll add a new benchmark to our benchmark suite, separately.
For #73581.
Fixes#74375.
Change-Id: I708321c14749a94ccff55072663012eba18b3b91
Reviewed-on: https://go-review.googlesource.com/c/go/+/684015
Reviewed-by: Keith Randall <khr@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>
Reviewed-by: Keith Randall <khr@google.com>
When an application calls runtime.GOMAXPROCS(runtime.GOMAXPROCS(0)), the
runtime does not need to change the actual GOMAXPROCS value (via STW).
However, this call must still transition from "automatic" to "custom"
GOMAXPROCS state, thus disabling background updates.
Thus this case shouldn't return quite as early as it currently does.
Change-Id: I6a6a636c42f73996532bd9f7beb95e933256c9e7
Reviewed-on: https://go-review.googlesource.com/c/go/+/683815
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
I missed one in the previous CL.
Change-Id: I448a871523d7fb8f429b4482839d7f101ea003b6
Reviewed-on: https://go-review.googlesource.com/c/go/+/681497
Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Add a test that would have detected the regression in #74303: interface
method fields should have a recorded type.
For #74303
Change-Id: Ide5df51cd71c38809c364bb4f95950163ecefb66
Reviewed-on: https://go-review.googlesource.com/c/go/+/683595
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Auto-Submit: Robert Findley <rfindley@google.com>
There is a non-public zstd decoder in the stdlib (CL 473356) and
also zstd compressed testdata already present.
Delete testdata/code.json.gz and
instead use internal/jsontest/testdata/golang_source.json.zst,
which has exactly the same content:
$ cat internal/jsontest/testdata/golang_source.json.zst | zstd -d | sha1sum
3f70b6fd429f4aba3e8e1c3e5a294c8f2e219a6e -
$ cat testdata/code.json.gz | zstd -d | sha1sum
3f70b6fd429f4aba3e8e1c3e5a294c8f2e219a6e -
This will reduce the size of the final Go release by 118KB.
Updates #71845
Change-Id: I6da2df27bd260befc0a44c6bc0255365be0a5b0f
Reviewed-on: https://go-review.googlesource.com/c/go/+/525516
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Bypass: Damien Neil <dneil@google.com>
Based on the discussion in #71631, it is hotly contested
whether the default JSON representation for a Go time.Duration
should be the time.Duration.String format or
a particular profile of ISO 8601.
Regardless of the default, it seems clear that we should
at least support ISO 8601 if specified via a format flag.
Note that this CL does not alter the default representation.
Unfortunately, ISO 8601 is a large and evolving standard
with many optional extensions and optional restrictions.
Thus, the term "ISO 8601 duration" unfortunately does not
resolve to a particular grammar, nor one that is stable.
However, there is precedence that we can follow in this matter.
JSON finds its heritage in JavaScript and
JavaScript is adding a Temporal.Duration type whose default
JSON representation is ISO 8601.
There is a well-specified grammar for their particular
profile of ISO 8601, which is documented at:
https://tc39.es/proposal-temporal/#prod-Duration
This particular CL adds support for ISO 8601 according to
the exact same grammar that JavaScript uses.
While Temporal.Duration is technically still a proposal,
it is already in stage 3 of the TC39 proposal process
(i.e., "no changes to the proposal are expected"
and "has been recommended for implementation")
and therefore close to final adoption.
One major concern with ISO 8601 is that it supports
nominal date units like years, months, weeks, and days
that do not have an accurate meaning without being
anchored to a particular point in time and place on Earth.
Fortunately, JavaScript (by default) avoids producing
Temporal.Duration values with nominal units unless
arithmetic in JavaScript explicitly sets a largestUnits
value that is larger than "hours". In the Go implementation,
we support syntactically parsing the full ISO 8601 grammar
(according to JavaScript), but semantically report an error if
nominal units are present. This ensures that ISO 8601 durations
remain accurate so long as they only use the accurate units
of hours, minutes, or seconds.
Updates #71631
Change-Id: I983593662f2150461ebc486a5acfeb72f0286939
Reviewed-on: https://go-review.googlesource.com/c/go/+/682403
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Damien Neil <dneil@google.com>
Auto-Submit: Joseph Tsai <joetsai@digital-static.net>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
In this blog:
https://blog.trailofbits.com/2025/06/17/unexpected-security-footguns-in-gos-parsers/
the concern was raised that whenever "-" is combined with other options,
the "-" is intepreted as as a name, rather than an ignored field,
which may go contrary to user expectation.
Static analysis demonstrates that there are ~2k instances of `json:"-,omitempty"
in the wild, where almost all of them intended for the field to be ignored.
To prevent this footgun, reject any tags that has "-," as a prefix
and warn the user to choose one of the reasonable alternatives.
The documentation of json/v2 already suggests `json:"'-'"`
as the recommended way to explicitly specify dash as the name.
See Example_fieldNames for example usages of the single-quoted literal.
Update the v1 json documentation to suggest the same thing.
Updates #71497
Change-Id: I7687b6eecdf82a5d894d057c78a4a90af4f5a6e4
Reviewed-on: https://go-review.googlesource.com/c/go/+/683175
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
Reviewed-by: Damien Neil <dneil@google.com>
Auto-Submit: Joseph Tsai <joetsai@digital-static.net>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
The default representation of a time.Duration is still undecided.
In order to keep the future open, report an error on a time.Duration
without an explicit format flag provided.
Updates #71631
Change-Id: I08248404ff6551723851417c8188a13f53c61937
Reviewed-on: https://go-review.googlesource.com/c/go/+/682455
Auto-Submit: Joseph Tsai <joetsai@digital-static.net>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Damien Neil <dneil@google.com>
This also updates wasip1_wasm to use a 8MiB stack, which is
the same stack size as what is used by go_js_wasm_exec.
The increase of stack size is necessary because the jsonv2
tests exercise that the jsonv2 and jsontext packages support
a hard limit of a maximum JSON nesting depth of 10000.
However, even with a depth limit of 10000, this still exceeds
the previously specified maximum stack size of 1 MiB.
For use of JSON with untrusted inputs in WASM,
we really need to support #56733 as there is no right answer
for the default max depth limit to use since the max wasm
stack size is determined on a per-system basis.
Updates #71845
Change-Id: I3b32c58cc9f594a5c59bb3e4b20f5e86d85d8209
Reviewed-on: https://go-review.googlesource.com/c/go/+/683575
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
The gc-stress test is useful for trying to exercise GC-related trace
events by producing a lot of them in many different situations.
Unfortunately this test is flaky, because allocating in a loop can
easily out-run the GC when it's trying to preempt the allocating
goroutine.
It's been a long standing problem that a program that allocates in a
loop can outrun a GC. The problem isn't the GC persay, it's consistently
correlated with a high STW time (likely a high 'stopping' time, not a
'stopped' time), suggesting that in the window of time when the garbage
collector is trying to stop all goroutines, they continue to allocate.
This should probably be fixed in general, but for now, let's focus on
this flaky test.
This CL changes the gc-stress test to (1) set a memory limit and (2) do
more work in between allocations. (2) is really what makes things less
flaky, but (2) unfortunately also means the GC is less exercised. That's
where (1) comes in. By setting a low memory limit, we increase GC
activity (in particular, assist activity). The memory limit also helps
prevent the heap from totally blowing up due to the heap goal inflating
from floating garbage, but it's not perfect.
After this change, under stress2, this test exceeds a heap size of 500
MiB only 1 in 5000 runs on my 64-vCPU VM. Before this change, it got
that big about 1/4th of the time.
Fixes#74052.
Change-Id: I49233c914c8b65b1d593d3953891fddda6685aec
Reviewed-on: https://go-review.googlesource.com/c/go/+/683515
Reviewed-by: Carlos Amedee <carlos@golang.org>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
RFC 8259, section 2 uses the term "begin-array" amd "begin-object"
rather than "start array" or "start object".
Be consistent in our documentation.
Change-Id: I172eb354c5e64b84c74bd662b1e581424e719a32
Reviewed-on: https://go-review.googlesource.com/c/go/+/683155
Auto-Submit: Joseph Tsai <joetsai@digital-static.net>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
sysReserveAlignedSbrk locks memlock at entry, but it is not
unlocked at one of the return path. Add the missing unlock.
Fixes#74339.
Change-Id: Ib641bc348aca41494ec410e2c4eb9857f3362484
Reviewed-on: https://go-review.googlesource.com/c/go/+/683295
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
This reverts commit 4ac729283c.
Reason for revert: changes semantics of types.Info.TypeOf; introduces new inconsistency around FieldList types.
For #74303
Change-Id: Ib99558c95f1b615fa9a02b028500ed230c8bb185
Reviewed-on: https://go-review.googlesource.com/c/go/+/683297
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
For #62640.
For #61394.
This is a copy of https://go-review.googlesource.com/c/go/+/528402,
which has stalled in review and the owner is no longer working on Go.
Change-Id: Ic7a1ae65c70d4857ab1061ccae1a926bf5c4ff55
Reviewed-on: https://go-review.googlesource.com/c/go/+/681235
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
During toolchain selection, the GOEXPERIMENT value may not be valid for
the current version (but it is valid for the selected version). In this
case, cfg.ExperimentErr is set and cfg.Experiment is nil.
Normally cmd/go main exits when ExperimentErr is set, so Experiment is
~never nil. But that is skipped during toolchain selection, and
fips140.Init is used during toolchain selection.
Fixes#74111.
Change-Id: I6a6a636c65ee5831feaf3d29993a60613bbec6f2
Reviewed-on: https://go-review.googlesource.com/c/go/+/680976
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
Currently the mspan limit field is set after allocSpan returns, *after*
the span has already been published to the GC (including the
conservative scanner). But the limit field is load-bearing, because it's
checked to filter out invalid pointers. A stale limit value could cause
a crash by having the conservative scanner access allocBits out of
bounds.
Fix this by setting the mspan limit field before publishing the span.
For large objects and arena chunks, we adjust the limit down after
allocSpan because we don't have access to the true object's size from
allocSpan. However this is safe, since we first initialize the limit to
something definitely safe (the actual span bounds) and only adjust it
down after. Adjusting it down has the benefit of more precise debug
output, but the window in which it's imprecise is also fine because a
single object (logically, with arena chunks) occupies the whole span, so
the 'invalid' part of the memory will just safely point back to that
object. We can't do this for smaller objects because the limit will
include space that does *not* contain any valid objects.
Fixes#74288.
Change-Id: I0a22e5b9bccc1bfdf51d2b73ea7130f1b99c0c7c
Reviewed-on: https://go-review.googlesource.com/c/go/+/682655
Reviewed-by: Keith Randall <khr@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@golang.org>
Almost everywhere we stop the world we casGToWaitingForGC to prevent
mutual deadlock with the GC trying to scan our stack. This historically
was only necessary if we weren't stopping the world to change the GC
phase, because what we were worried about was mutual deadlock with mark
workers' use of suspendG. And, they were the only users of suspendG.
In Go 1.22 this changed. The execution tracer began using suspendG, too.
This leads to the possibility of mutual deadlock between the execution
tracer and a goroutine trying to start or end the GC mark phase. The fix
is simple: make the stop-the-world calls for the GC also call
casGToWaitingForGC. This way, suspendG is guaranteed to make progress in
this circumstance, and once it completes, the stop-the-world can
complete as well.
We can take this a step further, though, and move casGToWaitingForGC
into stopTheWorldWithSema, since there's no longer really a place we can
afford to skip this detail.
While we're here, rename casGToWaitingForGC to casGToWaitingForSuspendG,
since the GC is now not the only potential source of mutual deadlock.
Fixes#72740.
Change-Id: I5e3739a463ef3e8173ad33c531e696e46260692f
Reviewed-on: https://go-review.googlesource.com/c/go/+/681501
Reviewed-by: Carlos Amedee <carlos@golang.org>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Our attempt to evenly distribute tests across shards struggles a bit
because certain long-running targets are very difficult to distinguish
in ResultDB, namely racebench and the test directory tests. These are
the only tests where the JSON output from dist omits the variant from
the package, making it impossible to distinguish them in the test result
data. My current suspicion is that this is preventing the load balancing
from being effective for the race builders in particular, though I worry
the longtest builders have a similar situation with the test directory
tests.
For #65814.
Change-Id: I5804c2af092ff9aa4a3f0f6897b4a57c4628f837
Reviewed-on: https://go-review.googlesource.com/c/go/+/681955
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Bypass: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Copied current (presumably correct) comment text from reflect package.
Change-Id: I19582b3675fbcb96a925002498d24ad2b7bc6178
Reviewed-on: https://go-review.googlesource.com/c/go/+/681935
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
Issue #74045 describes a scenario in which gopark is inlined into
readTrace, such that there are no preemption points. This is only a
problem because readTrace spins if trace.shutdown is set, through
traceReaderAvailable. However, trace.shutdown is almost certainly
overkill for traceReaderAvailable. The first condition, checking whether
the reader gen and the flushed gen match, should be sufficient to ensure
the reader wakes up and finishes flushing all buffers. The first
condition is also safe because it guarantees progress. In the case of
shutdown, all the trace work that will be flushed has been flushed, and
so the trace reader will exit into a regular goroutine context when
it's finished. If not shutting down, then the trace reader will release
doneSema, increase readerGen, and then the gopark unlockf will let it
block until new work actually comes in.
Fixes#74045.
Change-Id: Id9b15c277cb731618488771bd484577341b68675
Reviewed-on: https://go-review.googlesource.com/c/go/+/680738
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Nick Ripley <nick.ripley@datadoghq.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
A previous change renamed Value.Uint64 to Value.ToUint64 to accomodate
string values. The method for a string value is then Value.ToString,
while the method for a debug string (for example, for fmt) is just
called String, as per fmt.Stringer.
This change follows a request from Dominik Honnef, maintainer of
gotraceui, to make Value follow the conventions of the reflect package.
The Value type there has a method String which fulfills both purposes:
getting the string for a String Value, and as fmt.Stringer. It's
not exactly pretty, but it does make sense to just stick to convention.
Change-Id: I55b364be88088d2121527bffc833ef03dbdb9764
Reviewed-on: https://go-review.googlesource.com/c/go/+/680978
Reviewed-by: Florian Lehner <lehner.florian86@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
It was added in CL 650256, and then the use in the unique package
was removed in CL 650697.
Change-Id: Id95f5dff7e11a2dc3eb544fda2586a305d3d91ab
Reviewed-on: https://go-review.googlesource.com/c/go/+/681476
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
In Go 1.25 we added a number of new linknames for standard library
internal uses. Add them to the linker's blocklist to keep them
internal.
Change-Id: I5b6051a669b7ff132a1d2c05deefbbf74701c5d5
Reviewed-on: https://go-review.googlesource.com/c/go/+/681475
Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This change replaces a few user-visible mentions of golang.org and
godoc.org with go.dev and pkg.go.dev, respectively. Non-user-visible
mentions (e.g. in test scripts) were left untouched.
Change-Id: I5d828edcd618b6c55243d0dfcadc6fa1ce9422ce
Reviewed-on: https://go-review.googlesource.com/c/go/+/681255
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
This change switches from using testenv.Command to
testenv.CommandContext which is a little bit friendlier. It also
switches away from using 'go run' to 'go build' and running the
resulting binary explicitly. This helps eliminate any questions about
signal handling and propagation.
For #72740.
Change-Id: Ife8010da89a7bc439e061fe0c9c6b1f5620d90f1
Reviewed-on: https://go-review.googlesource.com/c/go/+/680977
Reviewed-by: Carlos Amedee <carlos@golang.org>
TryBot-Bypass: Michael Knyszek <mknyszek@google.com>
A few methods that were not implemented on Windows are implemented
in CL 668195.
Change-Id: I35423792a5af00f29fcd24e56a6dfcf013669371
Reviewed-on: https://go-review.googlesource.com/c/go/+/680180
Reviewed-by: Quim Muntal <quimmuntal@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Change-Id: I7b2c391749e0113e006f37b2ac1ebfe3ee0a4e0e
Reviewed-on: https://go-review.googlesource.com/c/go/+/680715
TryBot-Bypass: Damien Neil <dneil@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Joseph Tsai <joetsai@digital-static.net>
Auto-Submit: Damien Neil <dneil@google.com>
