qiulaidongfeng [Tue, 3 Jun 2025 15:01:27 +0000 (23:01 +0800)]
cmd/compile: better error message when import embed package
Fixes #73955
Change-Id: I7cf3ab4c70dc2e2765b54b88ae8cfc77a3073344
Reviewed-on: https://go-review.googlesource.com/c/go/+/678355
Auto-Submit: Robert Griesemer <gri@google.com> Reviewed-by: Carlos Amedee <carlos@golang.org> Reviewed-by: Robert Griesemer <gri@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Michael Matloob [Fri, 30 May 2025 16:53:42 +0000 (12:53 -0400)]
cmd/dist: don't install tools that won't be shipped in distribution
We shouldn't be installing these tools because we will remove them in
distpack. Installing the tools will also prevent us from testing what
happens when the tools are missing.
The changes below this on the stack, CL 677775 (cmd/doc: build cmd/doc
directly into the go command) and CL 677636 (cmd/go/internal/cfg: fix
GOROOT setting when forcing host config) are needed for this change to
pass tests. The doc change is being done so we preserve the properties
in the tests that doc can be invoked without doing a build. It's not
strictly necessary (we could just remove the tests) but it's nice to
have. The GOROOT setting is a significant bug in switching the
configuration to host mode: the value of GOROOT wasn't being reset,
which caused issues for go commands built with trimpath, because
runtime.GOROOT wouldn't have the correct goroot value.
For #71867
Change-Id: I4181711ba117066b7d62d7d013ad4b186871cfb7
Reviewed-on: https://go-review.googlesource.com/c/go/+/677558 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> Reviewed-by: Michael Matloob <matloob@google.com>
Michael Matloob [Fri, 30 May 2025 22:20:05 +0000 (18:20 -0400)]
cmd/doc: build cmd/doc directly into the go command
There are a couple of places where our tests expect that 'go doc'
doesn't need to do a build. Invoke the cmd/doc code directly by the go
command instead of starting the doc tool in a separate process so we can
preserve that property.
This change moves most of the doc code into the package
cmd/internal/doc, and exposes a Main function from that function that's
called both by the cmd/doc package, and by go doc.
This change makes couple of additional changes to intergrate doc into
the go command:
The counter.Open call and the increment of invocations counter are only
needed by cmd/doc. The go command will open the counters file and
increment a counter for the doc subcommand.
We add a cmd_go_bootstrap tagged variant of the file that defines go doc
so that we don't end up linking net into the bootstrap version of the go
command. We don't need doc in that version of the command.
We create a new flagSet rather than using flag.CommandLine because when
running as part of the go command, the flags to "go doc" won't be the top
level flags.
We change TestGoListTest in go_test.go to use gofmt instead of doc as an
example of a main package in cmd with an in-package test.
For #71867
Change-Id: I3e3df83e5fa266559606fdc086b461165e09f037
Reviewed-on: https://go-review.googlesource.com/c/go/+/677775
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>
Michael Pratt [Tue, 3 Jun 2025 17:08:06 +0000 (13:08 -0400)]
go/token: remove unreachable code
Reported by go vet.
Change-Id: I6a6a636c79923fafd8c649c583383cdf455c6ce2
Reviewed-on: https://go-review.googlesource.com/c/go/+/678317 Reviewed-by: Robert Findley <rfindley@google.com> Reviewed-by: Alan Donovan <adonovan@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Michael Matloob [Fri, 30 May 2025 20:16:27 +0000 (16:16 -0400)]
cmd/go/internal/cfg: fix GOROOT setting when forcing host config
We manage the state using a bunch of global config, so we need to make
sure we're doing things in the right order. In this case, the SetGOROOT
function was being called in init, setting the GOROOT on the global
Context, but when we reset the context in ForceHost we lost the goroot
configuration. We need to call SetGOROOT in ForceHost to re-set the
GOROOT on the new context.
This was uncovered by CL 677558 because a go command that was built with
trimpath would try to use its runtime.GOROOT(), which wouldn't be valid
in trimpath mode. Setting GOROOT properly with SetGOROOT will use the
value from findGOROOT, assuming GOROOT isn't set in the environment,
and findGOROOT will try to determine GOROOT using the path of the go
command executable.
For #71867
Change-Id: I731b6c5d859b4504fc128b29ab904e3a2886ff3c
Reviewed-on: https://go-review.googlesource.com/c/go/+/677636
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Matloob <matloob@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Keith Randall [Mon, 2 Jun 2025 23:24:07 +0000 (16:24 -0700)]
runtime: additional memmove benchmarks
For testing out duffcopy changes.
Change-Id: I93b4a52d75418a6e31aae5ad99f95d1870812b69
Reviewed-on: https://go-review.googlesource.com/c/go/+/678215 Reviewed-by: David Chase <drchase@google.com>
Auto-Submit: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Keith Randall <khr@google.com>
Alan Donovan [Fri, 23 May 2025 02:06:13 +0000 (22:06 -0400)]
go/token: FileSet: hold Files in a balanced tree
This CL changes the representation of FileSet from a slice
to a tree, specifically an AVL tree keyed by the File's
base-end range. This makes a sequence of insertions using
AddExistingFiles much more efficient: creating a FileSet
of size n by a sequence of calls costs O(n log n), whereas
before it was O(n^2 log n) because of the repeated sorting.
The AVL tree is based on Russ' github.com/rsc/omap,
simplified for clarity and to reduce unnecessary dynamism.
We use an AVL tree as it is more strongly balanced than an
RB tree, optimising lookups at the expense of insertions.
The CL includes a basic unit test of the tree using
operations on pseudorandom values.
Benchmarks of Position lookups actually improve because
the tree avoids BinarySearchFunc's dynamic dispatch to cmp,
and the benchmark of AddExistingFiles is about 1000x (!) faster:
Damien Neil [Thu, 29 May 2025 18:48:06 +0000 (11:48 -0700)]
runtime: randomize order of timers at the same instant in bubbles
In synctest bubbles, fire timers scheduled for the same instant
in a randomized order.
Pending timers are added to a heap ordered by the timer's wakeup time.
Add a per-timer random value, set when the timer is added to a heap,
to break ties between timers scheduled for the same instant.
Only inject this randomness in synctest bubbles. We could do so
for all timers at the cost of one cheaprand call per timer,
but given that it's effectively impossible to create two timers
scheduled for the same instant outside of a fake-time environment,
don't bother.
Fixes #73876
For #73850
Change-Id: Ie96c86a816f548d4c31e4e014bf9293639155bd4
Reviewed-on: https://go-review.googlesource.com/c/go/+/677276
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Damien Neil <dneil@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Alan Donovan [Mon, 2 Jun 2025 15:27:08 +0000 (11:27 -0400)]
slices,sort: explicitly discard results in benchmarks
The unusedresult analyzer will report failure to use the results
of these pure functions.
Updates #73950
Change-Id: I783cb92ad913105afd46c782bedf6234410c645d
Reviewed-on: https://go-review.googlesource.com/c/go/+/677995 Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Commit-Queue: Alan Donovan <adonovan@google.com> Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Carlos Amedee [Fri, 30 May 2025 20:05:04 +0000 (16:05 -0400)]
internal/trace: expose the go version read by the reader
This change adds a function to expose the version set by the trace
reader after reading the trace header (in tests). The trace validator
needs to be able to determine what version of the trace it needs to
validate against. Clock snapshot checks have been disabled for
Windows and WASM.
For #63185
Change-Id: Ia3d63e6ed7a5ecd87e63292b84cc417d982aaa5a
Reviewed-on: https://go-review.googlesource.com/c/go/+/677695
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Carlos Amedee <carlos@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Michael Matloob [Fri, 30 May 2025 19:23:36 +0000 (15:23 -0400)]
cmd/distpack: add test case for pack tool being excluded
For #71867
Change-Id: Ic4c6304b9a6b35c45bf35342523930924c68545a
Reviewed-on: https://go-review.googlesource.com/c/go/+/677635
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Matloob <matloob@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Michael Matloob [Fri, 30 May 2025 16:22:23 +0000 (12:22 -0400)]
cmd/distpack: don't keep the pack tool
This was an oversight: the pack tool isn't actually used in builds.
For #71867
Change-Id: Ib1f1cce0b574cf1d2c1002b2f2ab9ef9d750d0fb
Reviewed-on: https://go-review.googlesource.com/c/go/+/677557 Reviewed-by: Michael Matloob <matloob@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Pratt <mpratt@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Cherry Mui [Thu, 29 May 2025 19:42:23 +0000 (15:42 -0400)]
cmd/compile: update default PGO profile
Since last time the default.pgo profile is collected, there has
been a lot of development in the compiler. It's time to refresh
the compiler's PGO profile.
Profile collected by running the cmd/compile/profile.sh script on
the gotip-linux-amd64_c3h88-perf_vs_release gomote.
This String method can potentially recurse infinitely, since %#x will
apparently call String if the method exists. This isn't well documented,
but cmd/vet will be updated soon to check this (when we update the
vendored x/tools dependency) so cut off the recursion by converting to
the underlying type first.
Change-Id: Ia6fc046c9eb56a5dd6a33772afd23da443a06116
Reviewed-on: https://go-review.googlesource.com/c/go/+/677261
Auto-Submit: Michael Knyszek <mknyszek@google.com> Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Damien Neil [Thu, 22 May 2025 18:14:53 +0000 (11:14 -0700)]
runtime, testing/synctest: breaking bubble isolation with Cond is fatal
sync.Cond.Wait is durably blocking. Waking a goroutine out of Cond.Wait
from outside its bubble panics.
Make this panic a fatal panic, since it leaves the notifyList in an
inconsistent state. We could do some work to make this a recoverable
panic, but the complexity doesn't seem worth the outcome.
For #67434
Change-Id: I88874c1519c2e5c0063175297a9b120cedabcd07
Reviewed-on: https://go-review.googlesource.com/c/go/+/675617
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Damien Neil <dneil@google.com>
Mark Freeman [Thu, 22 May 2025 15:06:23 +0000 (11:06 -0400)]
internal/pkgbits: explain the rationale for reference tables
The primary benefit of reference tables is to the linker, though they
are also reasonably compact as compared to absolute element indices. It
is worth also checking if reference table structure is similarly
exploited past the IR linking stage.
Ideally, the reference table definition would live in / near the linker.
As it stands, it's a bit hard to infer the purpose of the reference
tables when looking at pkgbits in isolation.
Change-Id: I496aca5a4edcf28e66fa7863ddfa4d825e1b2e89
Reviewed-on: https://go-review.googlesource.com/c/go/+/675596
Auto-Submit: Mark Freeman <mark@golang.org> Reviewed-by: Robert Griesemer <gri@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Michael Pratt [Wed, 28 May 2025 20:01:33 +0000 (16:01 -0400)]
runtime: guarantee no GOMAXPROCS update syscalls after GOMAXPROCS call
We already guarantee that no automatic updates to GOMAXPROCS occur after
a GOMAXPROCS call returns. This is easily achieved by having the update
goroutine double-check that updates are still allowed during STW before
committing the new value.
However, it is possible for sysmon to concurrently run defaultGOMAXPROCS
to compute a new GOMAXPROCS value after GOMAXPROCS returns. This new
value will be discarded later, but we'll still perform the system calls
necessary to compute the new value.
Normally this distinction doesn't matter, but if you want to sandbox a
Go program, then you may want to disable GOMAXPROCS updates to reduce
the system call footprint. A call to GOMAXPROCS will disable updates,
but without a guarantee on when sysmon will observe the change it is
somewhat fragile.
Add explicit synchronization between GOMAXPROCS and sysmon to guarantee
that sysmon won't run defaultGOMAXPROCS after GOMAXPROCS returns.
The synchronization is a bit complex because we can't hold a mutex
across STW, nor take a semaphore from sysmon, but the result isn't too
bad.
One oddity is that sched.customGOMAXPROCS and gomaxprocs are no longer
updated in lockstep (even though both are protected by sched.lock), but
I don't believe anything should depend on that.
For #73193.
Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-staticlockranking
Change-Id: I6a6a636cff243a9b69ac1b5d2f98925648e60236
Reviewed-on: https://go-review.googlesource.com/c/go/+/677037
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Damien Neil [Fri, 23 May 2025 00:18:12 +0000 (17:18 -0700)]
internal/synctest: speed up TestWeak
Run TestWeak for fewer iterations. Five is enough reproduce #73817,
which was the motivation for this test. runtime.GC is ridiculously
slow on wasm, and not especially fast anywhere else.
Change-Id: Ieb5235d064b123cbc22a306425e385c273b54493
Reviewed-on: https://go-review.googlesource.com/c/go/+/675716 Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Damien Neil [Tue, 20 May 2025 22:56:43 +0000 (15:56 -0700)]
runtime, internal/synctest, sync: associate WaitGroups with bubbles
Add support to internal/synctest for managing associations between
arbitrary pointers and synctest bubbles. (Implemented internally to
the runtime package by attaching a special to the pointer.)
Associate WaitGroups with bubbles.
Since WaitGroups don't have a constructor,
perform the association when Add is called.
All Add calls must be made from within the same bubble,
or outside any bubble.
When a bubbled goroutine calls WaitGroup.Wait,
the wait is durably blocking iff the WaitGroup is associated
with the current bubble.
Change-Id: I77e2701e734ac2fa2b32b28d5b0c853b7b2825c9
Reviewed-on: https://go-review.googlesource.com/c/go/+/676656 Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Damien Neil <dneil@google.com>
Michael Pratt [Wed, 28 May 2025 18:34:52 +0000 (14:34 -0400)]
runtime: increment updatemaxprocs metric only when disabled
The updatemaxprocs metric logic is currently backwards. We only
increment the metric when we update GOMAXPROCS, but that only occurs if
updatemaxprocs is enabled.
Instead, the metric is supposed to increment when updatemaxprocs is
disabled and there would be different behavior if it were enabled.
Theoretically we should run the entire update system in a dry run mode,
and only bail out right before committing updates. But that is an awful
lot of effort for a feature that is disabled. Plus some users (like
sandboxes) want to completely disable the update syscalls
(sched_getaffinity and pread64). If we still do dry run updates then we
need an additional GODEBUG for completely disabling functionality.
This CL also avoids starting the update goroutine at all if disabled,
since it isn't needed.
For #73193.
Change-Id: I6a6a636ceec8fced44e36cb27dcb1b4ba51fce33
Reviewed-on: https://go-review.googlesource.com/c/go/+/677036 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>
Alan Donovan [Wed, 21 May 2025 15:21:31 +0000 (11:21 -0400)]
go/doc: NewFromFiles: fix panic on Files with SkipObjectResolution
This CL fixes a panic in NewFromFiles when it is provided files
produced by the parser in SkipObjectResolution mode, which skips
the step of connecting ast.Idents to (deprecated) ast.Objects.
Instead of calling ast.NewPackage, which performs a number of
unnecessary steps, we just construct the ast.Package directly.
Fixes #66290
Change-Id: Id55bd30d8afb9d396c3901070e7607c5a22030d5
Reviewed-on: https://go-review.googlesource.com/c/go/+/675036 Reviewed-by: Robert Griesemer <gri@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Keith Randall [Thu, 29 May 2025 00:09:05 +0000 (17:09 -0700)]
cmd/compile: do nil check before calling duff functions, on arm64 and amd64
On these platforms, we set up a frame pointer record below
the current stack pointer, so when we're in duffcopy or duffzero,
we get a reasonable traceback. See #73753.
But because this frame pointer record is below SP, it is vulnerable.
Anything that adds a new stack frame to the stack might clobber it.
Which actually happens in #73748 on amd64. I have not yet come across
a repro on arm64, but might as well be safe here.
The only real situation this could happen is when duffzero or duffcopy
is passed a nil pointer. So we can just avoid the problem by doing the
nil check outside duffzero/duffcopy. That way we never add a frame
below duffzero/duffcopy. (Most other ways to get a new frame below the
current one, like async preempt or debugger-generated calls, don't
apply to duffzero/duffcopy because they are runtime functions; we're
not allowed to preempt there.)
Longer term, we should stop putting stuff below SP. #73753 will
include that as part of its remit. But that's not for 1.25, so we'll
do the simple thing for 1.25 for this issue.
Fixes #73748
Change-Id: I913c49ee46dcaee8fb439415a4531f7b59d0f612
Reviewed-on: https://go-review.googlesource.com/c/go/+/676916
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Keith Randall <khr@google.com>
Mark Freeman [Wed, 28 May 2025 15:10:53 +0000 (11:10 -0400)]
go/types, types2: dump position stack for non-bailout panics
We make sure to dump to stderr since that's where the panic information
ends up. Long traces get truncated with a "..." in the middle. We pick
an arbitrary limit of 10 positions, but this could be changed.
For #51603
Change-Id: I02326a93181e94e1c48afc05684240540c2c90ba
Reviewed-on: https://go-review.googlesource.com/c/go/+/676815 Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Mark Freeman <mark@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Cherry Mui [Wed, 21 May 2025 18:32:21 +0000 (14:32 -0400)]
cmd/link: allow linkname reference to a TEXT symbol regardless of size
In CL 660696, we made the linker to choose the symbol of the
larger size in case there are multiple contentless declarations of
the same symbol. We also made it emit an error in the case that
there are a contentless declaration of a larger size and a
definition with content of a smaller size. In this case, we should
choose the definition with content, but the code accesses it
through the declaration of the larger size could fall into the
next symbol, potentially causing data corruption. So we disallowed
it.
There is one spcial case, though, that some code uses a linknamed
variable declaration to reference a function in assembly, in order
to take its address. The variable is often declared as uintptr.
The function symbol is the definition, which could sometimes be
shorter. This would trigger the error case above, causing existing
code failing to build.
This CL allows it as a special case. It is still not safe to
access the variable's content. But it is actually okay to just
take its address, which the existing code often do.
Fixes #73617.
Change-Id: I467381bc5f6baa16caee6752a0a824c7185422f6
Reviewed-on: https://go-review.googlesource.com/c/go/+/676636 Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Michael Anthony Knyszek [Wed, 28 May 2025 17:39:14 +0000 (17:39 +0000)]
doc/next: tweak runtime release notes
- Add section headings to make the section easier to read.
- Reorder features to better reflect their impact and importance.
- Tweak some awkward wording here and there.
Change-Id: If72c526f4b3a26a7a4584d6c59857db02c0c1338
Reviewed-on: https://go-review.googlesource.com/c/go/+/676818 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>
Dmitri Shuralyov [Wed, 28 May 2025 15:23:36 +0000 (11:23 -0400)]
lib/time: update to 2025b/2025b
Commit generated by update.bash.
For #22487.
Change-Id: If4132dc12296b23b85a221bffdb1b854d0332010
Reviewed-on: https://go-review.googlesource.com/c/go/+/676855 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Carlos Amedee <carlos@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Damien Neil [Tue, 27 May 2025 21:16:17 +0000 (14:16 -0700)]
os: don't follow symlinks on Windows when O_CREATE|O_EXCL and read-only
Fix a bug in CL 672396, where we add FILE_FLAG_OPEN_REPARSE_POINT to
the attributes passed to CreateFile, but then overwrite the attributes
with FILE_ATTRIBUTE_READONLY when opening a file with a read-only
permissions mode.
For #73702
Change-Id: I6c10bf470054592bafa031732585fc3155c61341
Reviewed-on: https://go-review.googlesource.com/c/go/+/676655
Auto-Submit: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Alan Donovan <adonovan@google.com>
Damien Neil [Wed, 21 May 2025 22:08:08 +0000 (15:08 -0700)]
runtime, testing/synctest: verify cleanups/finalizers run outside bubbles
Cleanup functions and finalizers must not run in a synctest bubble.
If they did, a function run by the GC at an unpredictable time
could unblock a bubble that synctest believes is durably
blocked.
Add a test verifying that cleanups and finalizers are always
run by non-bubbled goroutines. (This is already the case because
we never add system goroutines to a bubble.)
For #67434
Change-Id: I5a48db2b26f9712c3b0dc1f425d99814031a2fc1
Reviewed-on: https://go-review.googlesource.com/c/go/+/675257 Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Damien Neil <dneil@google.com>
thepudds [Tue, 27 May 2025 17:32:36 +0000 (13:32 -0400)]
cmd/compile/internal/walk: use original type for composite literals in addrTemp
When creating a new *ir.Name or *ir.LinksymOffsetExpr to represent
a composite literal stored in the read-only data section, we should
use the original type of the expression that was found via
ir.ReassignOracle.StaticValue. (This is needed because the StaticValue
method can traverse through OCONVNOP operations to find its final
result.)
Otherwise, the compilation may succeed, but the linker might erroneously
conclude that a type is not used and prune an itab when it should not,
leading to a call at execution-time to runtime.unreachableMethod, which
throws "fatal error: unreachable method called. linker bug?".
The tests exercise both the case of a zero value struct literal that
can be represented by the read-only runtime.zeroVal, which was the case
of the simplified example from #73888, and also modifies that example to
test the non zero value struct literal case.
This CL makes two similar changes for those two cases. We can get either
of the tests we are adding to fail independently if we only make
a single corresponding change.
Fixes #73888
Updates #71359
Change-Id: Ifd91f445cc168ab895cc27f7964a6557d5cc32e5
Reviewed-on: https://go-review.googlesource.com/c/go/+/676517 Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com>
Auto-Submit: Keith Randall <khr@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Carlos Amedee [Fri, 23 May 2025 20:41:51 +0000 (16:41 -0400)]
runtime/trace: add a trace validation test for different trace orders
This adds a test which validates the traces generated by the execution
tracer and the flight recorder depending on the order where they are
stopped and started. This test uncovered that under certain
circumstances, the traces which were produced would possibly be
missing the trace header. All traces have the trace headers included
now. Clock snapshot checks have been disabled for Windows and WASM.
Change-Id: I5be719d228300469891fc56817fbce4ba5453fff
Reviewed-on: https://go-review.googlesource.com/c/go/+/675975
Auto-Submit: Carlos Amedee <carlos@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Michael Matloob [Fri, 23 May 2025 17:19:44 +0000 (13:19 -0400)]
cmd/go: fix get with the new 'work' pattern
Before this change, go get didn't have support for the work pattern. The
work pattern is new in Go 1.25 and evaluates to the packages in the work
(also called main) modules. 'go get work' would cause a panic because
'work' would be incorrectly considered a path pattern and then queryPath
would would try to query a metapackage pattern (resulting in the
internal error panic). This change properly supports the work pattern in
go get.
It's pretty simple: First, we need to seprate the work pattern from the
other patterns. Then in performWorkQueries, which maps queries to the
modules that satisfy them, we return the single main module because by
definition the work pattern is the set of packages in the work modules,
and go get always runs in single module mode. (The exception is when the
work module contains no packages, in which case we report a warning, and
return no candidates because nothing is needed to resolve nothing).
The rest of the work is already done by loading the packages matching
the query and finding missing imports in the call to
findAndUpgradeImports in runGet.
Change-Id: I3c4610878b3d930a1d106cc59d9a0be194d966cd
Reviewed-on: https://go-review.googlesource.com/c/go/+/675895 Reviewed-by: Michael Matloob <matloob@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Sean Liao [Tue, 27 May 2025 18:11:28 +0000 (19:11 +0100)]
log/slog: fix longtests with empty source
Tests broken by CL 674875
Updates #73808
Change-Id: I7ad93e4a8ba1977d136f99b9d4963fa8a9c159ff
Reviewed-on: https://go-review.googlesource.com/c/go/+/676595
Auto-Submit: Michael Knyszek <mknyszek@google.com>
TryBot-Bypass: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
David Chase [Tue, 27 May 2025 18:21:11 +0000 (14:21 -0400)]
doc/next: add small header to TODO
Change-Id: I91c03f455fff8e4078f3297ea357cd1e1dd09f66
Reviewed-on: https://go-review.googlesource.com/c/go/+/676536 Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: David Chase <drchase@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This CL adds a benchmark of FileSet.Position, the lookup
operation, and the new AddExistingFiles. It is evident
that its behavior is quadratic in important cases:
Michael Pratt [Tue, 27 May 2025 14:37:50 +0000 (10:37 -0400)]
runtime: skip nil Ps in allp during cleanup flush
cleanupQueue.Flush is reachable from mallocgc via sweepAssist. Normally
allp will continue all valid Ps, but procresize itself increases the
size of allp and then allocates new Ps to place in allp. If we get
perfectly unlucky, the new(p) allocations will complete sweeping and
cleanupQueue.Flush will dereference a nil pointer from allp. Avoid this
by skipping nil Ps.
I've looked through every other use of allp and none of them appear to
be reachable from procresize.
Change-Id: I6a6a636cab49ef268eb8fcd9ff9a96790d9c5685
Reviewed-on: https://go-review.googlesource.com/c/go/+/676515
Auto-Submit: Michael Pratt <mpratt@google.com> Reviewed-by: Carlos Amedee <carlos@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
jiahua wang [Tue, 20 May 2025 05:40:18 +0000 (13:40 +0800)]
errors: add joinError Unwrap example
Change-Id: Id7489247e9bdd413f82fdf5a70197856c47abfb5
Reviewed-on: https://go-review.googlesource.com/c/go/+/674336 Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Sean Liao <sean@liao.dev>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: David Chase <drchase@google.com>
Auto-Submit: Sean Liao <sean@liao.dev>
cuishuang [Sun, 18 May 2025 08:28:13 +0000 (16:28 +0800)]
io/fs: add examples for Glob,ReadFile and ValidPath
Change-Id: I8451179bc0fa88b7e60afbc6fd9e06a22a94f3aa
Reviewed-on: https://go-review.googlesource.com/c/go/+/673835 Reviewed-by: Sean Liao <sean@liao.dev>
Auto-Submit: Sean Liao <sean@liao.dev> Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: David Chase <drchase@google.com>
Antonio Pitasi [Wed, 21 May 2025 09:58:13 +0000 (11:58 +0200)]
log/slog: make TextHandler discard empty Source
Fixes #73808
Change-Id: Ica4b7a63eebbf0fff41d68f4de928f9da90c8ada
Reviewed-on: https://go-review.googlesource.com/c/go/+/674875 Reviewed-by: Jonathan Amsterdam <jba@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Sean Liao <sean@liao.dev>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
David Chase [Fri, 23 May 2025 18:49:13 +0000 (14:49 -0400)]
doc/next: add crudely processed todos
This is the output of relnote -goroot=... todo,
with each todo in a comment, followed by summary
text from the issue and perhaps the CL, lightly
processed into markdown.
For #71661.
Change-Id: I855c4c4ee02491b5b6113822baf69dbafb4e54ab
Reviewed-on: https://go-review.googlesource.com/c/go/+/675877 Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Carlos Amedee [Thu, 22 May 2025 17:33:28 +0000 (13:33 -0400)]
runtime/trace: fix flaky test for SetMinAge
This change fixes the flaky test which expects setting SetMinAge to a
small ammount. It expects two sync events but should realistically
expect up to 3.
Change-Id: Ibd02fe55ebca99eb880025eb968fcebae9cb09c9
Reviewed-on: https://go-review.googlesource.com/c/go/+/675597 Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Keith Randall [Thu, 22 May 2025 22:17:37 +0000 (15:17 -0700)]
doc: fix TBD mark
Change-Id: I2133e3c62b4de0cec08eeb120d593c644643a62c
Reviewed-on: https://go-review.googlesource.com/c/go/+/675755 Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Keith Randall [Wed, 16 Apr 2025 20:50:44 +0000 (13:50 -0700)]
doc: mention stack allocation of variable-sized make calls
Also mention the bisect tool and flag used to track down
incorrect uses.
Change-Id: Id36a236e1bb2733b8611b22a5b16916e7d9f5522
Reviewed-on: https://go-review.googlesource.com/c/go/+/666075 Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> 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>
Michael Anthony Knyszek [Thu, 22 May 2025 00:29:14 +0000 (00:29 +0000)]
runtime: don't spin looking for a tiny alloc address with asan or race
CL 674655 modified the checkfinalizers test to spin looking for an
appropriate address to trip the detector, but this doesn't work with
ASAN or in race mode, which both disable the tiny allocator.
Fixes #73834.
Change-Id: I27416da1f29cd953271698551e9ce9724484c683
Reviewed-on: https://go-review.googlesource.com/c/go/+/675395
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Cuong Manh Le [Fri, 10 Jan 2025 05:49:59 +0000 (12:49 +0700)]
cmd/compile: do not shapify when reading reshaping expr
Fixes #71184
Change-Id: I22e7ae5203311e86a90502bfe155b0597007887d
Reviewed-on: https://go-review.googlesource.com/c/go/+/641955
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: David Chase <drchase@google.com>
cmd/compile: fix ICE with recursive alias type parameter
CL 585399 fixed an initialization loop during IR contruction that
involving alias type, by avoiding publishing alias declarations until
the RHS type expression has been constructed.
There's an assertion to ensure that the alias's type must be the same
during the initialization. However, that assertion is too strict, since
we may construct different instances of the same type, if the type is an
instantination of generic type.
To fix this, we could use types.IdenticalStrict to ensure that these
types matching exactly.
Updates #66873.
Updates #73309.
Change-Id: I2559bed37e21615854333fb1057d7349406e6a1b
Reviewed-on: https://go-review.googlesource.com/c/go/+/668175 Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Keith Randall <khr@golang.org>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Keith Randall <khr@google.com>
Michael Matloob [Thu, 22 May 2025 16:40:51 +0000 (12:40 -0400)]
cmd/doc: properly set GOPROXY to avoid deprecation checks
This change fixes a bug that was introduced in CL 675155. Instead of
doing the two step download and run with GOPROXY=off, do the run with
GOPROXY=<download cache>:$GOPROXY, so that we use the previously
downloaded version of pkgsite as the latest.
Fixes #73833
Change-Id: I8803426498ab026602805d6448a130eb11458c99
Reviewed-on: https://go-review.googlesource.com/c/go/+/675576 Reviewed-by: Jonathan Amsterdam <jba@google.com> Reviewed-by: Michael Matloob <matloob@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Cuong Manh Le [Thu, 22 May 2025 11:06:27 +0000 (18:06 +0700)]
cmd/compile: fix ICE when transforming loopvar
When transforming for loop variables, the compiler does roughly
following steps:
(1) prebody = {z := z' for z in leaked}
...
(4) init' = (init : s/z/z' for z in leaked)
However, the definition of z is not updated to `z := z'` statement,
causing ReassignOracle incorrectly use the new init statement with z'
instead of z, trigger the ICE.
Fixing this by updating the correct/new definition statement for z
during the prebody initialization.
Fixes #73823
Change-Id: Ice2a6741be7478506c58f4000f591d5582029136
Reviewed-on: https://go-review.googlesource.com/c/go/+/675475
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
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>
Jake Bailey [Tue, 13 May 2025 03:39:54 +0000 (20:39 -0700)]
cmd/compile/internal/ssa: eliminate string copies for calls to unique.Make
unique.Make always copies strings passed into it, so it's safe to not
copy byte slices converted to strings either. Handle this just like map
accesses with string(b) as keys.
This CL only handles unique.Make(string(b)), not nested cases like
unique.Make([2]string{string(b1), string(b2)}); this could be done in a
followup CL but the map lookup code in walk is sufficiently different
than the call handling code that I didn't attempt it. (SSA is much
easier).
Fixes #71926
Change-Id: Ic2f82f2f91963d563b4ddb1282bd49fc40da8b85
Reviewed-on: https://go-review.googlesource.com/c/go/+/672135 Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Cherry Mui [Wed, 21 May 2025 18:33:13 +0000 (14:33 -0400)]
cmd/compile, unique: model data flow of non-string pointers
Currently, hash/maphash.Comparable escapes its parameter if it
contains non-string pointers, but does not escape strings or types
that contain strings but no other pointers. This is achieved by a
compiler intrinsic.
unique.Make does something similar: it stores its parameter to a
central map, with strings cloned. So from the escape analysis's
perspective, the non-string pointers are passed through, whereas
string pointers are not. We currently cannot model this type of
type-dependent data flow directly in Go. So we do this with a
compiler intrinsic. In fact, we can unify this and the intrinsic
above.
Tests are from Jake Bailey's CL 671955 (thanks!).
Fixes #73680.
Change-Id: Ia6a78e09dee39f8d9198a16758e4b5322ee2c56a
Reviewed-on: https://go-review.googlesource.com/c/go/+/675156
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Jake Bailey <jacob.b.bailey@gmail.com>
thepudds [Wed, 21 May 2025 20:50:17 +0000 (16:50 -0400)]
cmd/compile/internal/escape: make escape analysis -m=2 logs more accessible
This was the first CL in a series of CLs aimed at reducing
how often interface arguments escape for the print functions in fmt.
This CL makes some small improvements to the escape analysis logging.
Here is a sample snippet of the current -m=2 logs:
./print.go:587:7: parameter p leaks to {heap} with derefs=0:
./print.go:587:7: flow: p = p:
./print.go:587:7: from (*pp).printArg(p, err, 'v') (call parameter) at ./print.go:613:13
./print.go:587:7: flow: p = p:
./print.go:587:7: from (*pp).handleMethods(p, verb) (call parameter) at ./print.go:749:22
[..]
If we attempt to tease apart some reasons why the -m=2 logs can be
challenging to understand for the uninitiated:
- The "flow" lines are very useful, but contain more-or-less abstracted
pseudocode. The "from" lines most often use actual code. When first
looking at the logs, that distinction might not be apparent, which can
result in looking back to the original code to hunt for pseudocode
that doesn't exist there. (The log example shows 'p = p', but there is
no 'p = p' in the original source).
- Escape analysis can be most interesting with inlining, but that can
result in seeing overlapping short variable names (e.g., p, b, v...).
- The directionality of the "flow" lines might not be obvious,
including whether they build top-to-bottom or bottom-to-top.
- The use of '{' and '}' in the -m=2 logs somewhat intersects with Go
literals (e.g., if the log says "{temp}", an initial thought might
be that represents some temp inside of some Go literal).
- And of course, escape analysis itself is subtle.
This CL:
- Adds the function name to the first -m=2 line to provide more context
and reduce how often the reader needs to lookup line numbers.
- Uses the Unicode left arrow '←' rather than '=' on the flow lines
to make it clearer that these lines are abstracted away from the
original Go code and to help the directionality jump out.
In the future, we can consider changing "{heap}", "{temp}",
"{storage for foo}" to something else, but we leave them as is for now.
Two examples with the modifications:
./f1.go:3:9: parameter inptr leaks to outptr for func1 with derefs=0:
./f1.go:3:9: flow: localptr ← inptr:
./f1.go:3:9: from localptr := inptr (assign) at ./f1.go:4:11
./f1.go:3:9: flow: outptr ← localptr:
./f1.go:3:9: from return localptr (return) at ./f1.go:5:2
./b.go:14:20: []byte{...} escapes to heap in byteOrderExample:
./b.go:14:20: flow: b ← &{storage for []byte{...}}:
./b.go:14:20: from []byte{...} (spill) at ./byteorder.go:14:20
./b.go:14:20: from b := []byte{...} (assign) at ./byteorder.go:14:11
./b.go:14:20: flow: <heap> ← b:
./b.go:14:20: from byteOrder.Uint32(b) (call parameter) at ./byteorder.go:15:32
These changes only affect the -m=2 output and leave the -m=1 output
as is.
Updates #8618
Updates #62653
Change-Id: Ic082a371c3d3fa0d8fd8bfbe4d64ec3e1e53c173
Reviewed-on: https://go-review.googlesource.com/c/go/+/524937 Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: David Chase <drchase@google.com>
Carlos Amedee [Wed, 14 May 2025 20:13:24 +0000 (16:13 -0400)]
runtime/trace: add the flight recorder
This change adds the flight recorder to the trace package.
Flight recording is a technique in which trace data is kept
in a circular buffer and can be flushed upon request. The
implementation will be added in follow-up CLs.
The flight recorder has already been implemented inside of the
golang.org/x/exp/trace package. This copies the current implementation
and modifies it to work within the runtime/trace package.
The changes include:
This adds the ability for multiple consumers (both the execution
tracer and the flight recorder) to subscribe to tracing events. This
change allows us to add multiple consumers without making major
modifications to the runtime. Future optimizations are planned
for this functionality.
This removes the use of byte readers from the process that
parses and processes the trace batches.
This modifies the flight recorder to not parse out the trace
clock frequency, since that requires knowledge of the format that's
unfortunate to encode in yet another place. Right now, the trace clock
frequency is considered stable for the lifetime of the program, so just
grab it directly from the runtime.
This change adds an in-band end-of-generation signal to the internal
implementation of runtime.ReadTrace. The internal implementation is
exported via linkname to runtime/trace, so the flight recorder can
identify exactly when a generation has ended. This signal is also useful
for ensuring that subscribers to runtime trace data always see complete
generations, by starting or stopping data streaming only at generation
boundaries.
For #63185
Change-Id: I5c15345981a6bbe9764a3d623448237e983c64ec
Reviewed-on: https://go-review.googlesource.com/c/go/+/673116
Auto-Submit: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Change-Id: I1b8cf3939b433e1765682196b8fc1aa07d37f895
Reviewed-on: https://go-review.googlesource.com/c/go/+/673476
Auto-Submit: Keith Randall <khr@google.com> Reviewed-by: Keith Randall <khr@google.com>
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>
Roland Shoemaker [Wed, 21 May 2025 18:43:35 +0000 (11:43 -0700)]
crypto/sha3: un-interleave EOR instructions
Move two EOR instructions out of blocks of RAX and
BCAX instructions. This appears to get a teeny
performance improvement, and matches what the
Linux kernel implementation does.
Daniel McCarney [Sat, 17 May 2025 15:17:21 +0000 (11:17 -0400)]
crypto/tls: use decode alert for handshake msg unmarshal err
Previously if instances of the handshakeMessage interface returned false
from unmarshal(), indicating an umarshalling error, the crypto/tls
package would emit an unexpected_message alert. This commit changes to
use a decode_error alert for this condition instead.
The usage-pattern of the handshakeMessage interface is that we switch on
the message type, invoke a specific concrete handshakeMessage type's
unmarshal function, and then return it to the caller on success. At this
point the caller looks at the message type and can determine if the
message was unexpected or not. If it was unexpected, the call-sites emit
the correct error for that case. Only the caller knows the current
protocol state and allowed message types, not the generic handshake
decoding logic.
With the above in mind, if we find that within the unmarshal logic for
a specific message type that the data we have in hand doesn't match the
protocol syntax we should emit a decode_error. An unexpected_message
error isn't appropriate because we don't yet know if the message is
unexpected or not, only that the message can't be decoded based on the
spec's syntax for the type the message claimed to be.
Notably one unit test, TestQUICPostHandshakeKeyUpdate, had to have its
test data adjusted because it was previously not testing the right
thing: it was double-encoding the type & length prefix data for a key
update message and expecting the QUIC logic to reject it as an
inappropriate post-handshake message. In reality it was being rejected
sooner as an invalid key update message from the double-encoding and
this was masked by the previous alert for this condition matching the
expected alert.
Finally, changing our alert allows enabling a handful of BoGo tests
related to duplicate extensions of the form
"DuplicateExtension[Server|Client]-TLS-[TLS1|TLS11|TLS12|TLS13]". One
test remains skipped (DuplicateExtensionClient-TLS-TLS13), as it
requires additional follow-up.
Updates #72006
Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5d1
Reviewed-on: https://go-review.googlesource.com/c/go/+/673738 Reviewed-by: Roland Shoemaker <roland@golang.org> Reviewed-by: David Chase <drchase@google.com>
Auto-Submit: Daniel McCarney <daniel@binaryparadox.net>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Filippo Valsorda <filippo@golang.org>
Filippo Valsorda [Sat, 15 Mar 2025 14:12:39 +0000 (15:12 +0100)]
crypto/tls: disable SHA-1 signature algorithms in TLS 1.2
This implements RFC 9155 by removing support for SHA-1 algorithms:
- we don't advertise them in ClientHello and CertificateRequest
(where supportedSignatureAlgorithms is used directly)
- we don't select them in our ServerKeyExchange and CertificateVerify
(where supportedSignatureAlgorithms filters signatureSchemesForCertificate)
- we reject them in the peer's ServerKeyExchange and CertificateVerify
(where we check against the algorithms we advertised in ClientHello
and CertificateRequest)
Fixes #72883
Change-Id: I6a6a4656e2aafd2c38cdd32090d3d8a9a8047818
Reviewed-on: https://go-review.googlesource.com/c/go/+/658216
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Filippo Valsorda <filippo@golang.org> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Roland Shoemaker <roland@golang.org> Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
Roland Shoemaker [Thu, 8 May 2025 23:27:36 +0000 (16:27 -0700)]
crypto: add a test for disallowed instructions
WORD and BYTE usage in crypto assembly cores is an anti-pattern which
makes extremely sensitive code significantly harder to understand, and
can result in unexpected behavior.
Because of this, we've decided to ban their usage in the crypto/ tree
(as part of the cryptography assembly policy).
This test walks the crypto/ tree looking for assembly files (those with
the filetype .s) and look for lines that match the regular rexpression
"(^|;)\s(BYTE|WORD)\s".
Change-Id: I60b5283e05e8588fa53273904a9611a411741f72
Reviewed-on: https://go-review.googlesource.com/c/go/+/671099 Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
Auto-Submit: Roland Shoemaker <roland@golang.org>
Filippo Valsorda [Sat, 15 Mar 2025 11:12:22 +0000 (12:12 +0100)]
crypto/tls: don't advertise TLS 1.2-only sigAlgs in TLS 1.3
If a ClientHello only supports TLS 1.3, or if a CertificateRequest is
sent after selecting TLS 1.3, we should not advertise TLS 1.2-only
signature_algorithms like PKCS#1 v1.5 or SHA-1.
However, since crypto/x509 still supports PKCS#1 v1.5, and a direct
CertPool match might not care about the signature in the certificate at
all, start sending a separate signature_algorithms_cert extension to
indicate support for PKCS#1 v1.5 and SHA-1 in certificates.
We were already correctly rejecting these algorithms if the peer
selected them in a TLS 1.3 connection.
Updates #72883
Change-Id: I6a6a4656ab60e1b7fb20fdedc32604dc156953ae
Reviewed-on: https://go-review.googlesource.com/c/go/+/658215 Reviewed-by: Roland Shoemaker <roland@golang.org> Reviewed-by: David Chase <drchase@google.com>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
Dmitri Shuralyov [Wed, 21 May 2025 17:13:48 +0000 (13:13 -0400)]
cmd/link/internal/ld: use 12.0.0 OS/SDK versions for macOS linking
Go 1.25 will require macOS 12 Monterey or later, so macOS 11 will be
unsupported. The comment here suggests using a supported macOS version,
and that it can be the most recent one.
For now, make a minimal change of going from 11.0.0 to 12.0.0 so that
the chosen version is a supported one (although not the most recent).
However, it looks like even in CL 460476 (where the comment was added)
we were staying with the macOS version that matched Go's oldest, so we
might not have have recent experience with going beyond that. Update
the comment accordingly.
For #69839.
Change-Id: I90908971b0d5a8235ce77dc6bc9649e86008270a
Cq-Include-Trybots: luci.golang.try:gotip-darwin-amd64-longtest,gotip-darwin-arm64-longtest,gotip-darwin-amd64_12,gotip-darwin-amd64_14,gotip-darwin-arm64_12,gotip-darwin-arm64_15
Reviewed-on: https://go-review.googlesource.com/c/go/+/675095
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>