Keith Randall [Wed, 9 Apr 2025 21:38:03 +0000 (14:38 -0700)]
cmd/compile: allocate backing store for append on the stack
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:
(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>
ICHINOSE Shogo [Sun, 18 May 2025 17:28:09 +0000 (17:28 +0000)]
math: fix portable FMA implementation when x*y ~ 0, x*y < 0 and z = 0
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: 30d74883b21667fc9439d9d14932b7edb3e72cd5
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>
Michael Pratt [Mon, 19 May 2025 20:20:47 +0000 (16:20 -0400)]
runtime: disable TestSegv in race mode
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>
Michael Pratt [Tue, 13 May 2025 17:12:47 +0000 (13:12 -0400)]
runtime: rename ncpu to numCPUStartup
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>
Mark Freeman [Fri, 16 May 2025 14:42:06 +0000 (10:42 -0400)]
cmd/compile/internal/noder: document the PosBase section
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>
Change-Id: Id55698800fd70f3cb2ff48393584456b87208921
Reviewed-on: https://go-review.googlesource.com/c/go/+/673556
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: Cherry Mui <cherryyz@google.com>
Michael Pratt [Wed, 22 Jan 2025 19:13:33 +0000 (14:13 -0500)]
runtime: check for gsignal in asancall/msancall/racecall
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>
Michael Pratt [Wed, 22 Jan 2025 22:18:19 +0000 (17:18 -0500)]
runtime: pass through -asan/-msan/-race to testprog tests
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>
Michael Matloob [Fri, 16 May 2025 23:53:21 +0000 (19:53 -0400)]
cmd/distpack: use positive list of tools to keep
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>
Alan Donovan [Wed, 11 Dec 2024 20:42:06 +0000 (15:42 -0500)]
std: pass bytes.Buffer and strings.Builder by pointer
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>
Guoqi Chen [Mon, 12 May 2025 03:01:44 +0000 (11:01 +0800)]
cmd/compile: add prefetch intrinsic support on loong64
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
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: f4ebd1719ff966ae3c6516e3fb935dfea2f5362e
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>
Michael Matloob [Thu, 15 May 2025 19:30:57 +0000 (15:30 -0400)]
cmd/distpack: remove more tools from packaged distribution
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>
Michael Matloob [Thu, 6 Feb 2025 21:11:50 +0000 (16:11 -0500)]
cmd/distpack: remove some tools from packaged distribution
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>
Change-Id: I35c1f264d216c214ea3f56252a9ddab8ea850fa6
Reviewed-on: https://go-review.googlesource.com/c/go/+/673555 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>
Michael Matloob [Thu, 1 May 2025 02:17:21 +0000 (22:17 -0400)]
cmd/go/internal/modget: check in workspace mode if go.work present
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 [Fri, 16 May 2025 04:42:38 +0000 (04:42 +0000)]
sync: set GOMAXPROCS to 1 in TestPoolGC
This test expects to be able to drain a Pool using only Get. This isn't
actually possible in the general case, since a pooled value could get
stuck in some P's private slot. However, if GOMAXPROCS=1, there's only 1
P we could be running on, so getting stuck becomes impossible.
This test isn't checking any concurrent properties of Pool, so this is
fine. Just set GOMAXPROCS=1 for this one particular test.
Fixes #73728.
Change-Id: I9053e28118060650f2cd7d0d58f5a86d630b36f7
Reviewed-on: https://go-review.googlesource.com/c/go/+/673375
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Sam Thanawalla [Wed, 8 Jan 2025 18:30:50 +0000 (18:30 +0000)]
cmd/go: add global ignore mechanism
This CL adds the ignore directive which enables users to tell the Go
Command to skip traversing into a given directory.
This behaves similar to how '_' or 'testdata' are currently treated.
This mainly has benefits for go list and go mod tidy.
This does not affect what is packed into a module.
Fixes: #42965
Change-Id: I232e27c1a065bb6eb2d210dbddad0208426a1fdd
Reviewed-on: https://go-review.googlesource.com/c/go/+/643355
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Matloob <matloob@golang.org> Reviewed-by: Michael Matloob <matloob@google.com>
Michael Anthony Knyszek [Sat, 10 May 2025 18:53:14 +0000 (18:53 +0000)]
runtime: prevent cleanup goroutines from missing work
Currently, there's a window of time where each cleanup goroutine has
committed to going to sleep (immediately after full.pop() == nil) but
hasn't yet marked itself as asleep (state.sleep()). If new work arrives
in this window, it might get missed. This is what we see in #73642, and
I can reproduce it with stress2.
Side-note: even if the work gets missed by the existing sleeping
goroutines, needg is incremented. So in theory a new goroutine will
handle the work. Right now that doesn't happen in tests like the one
running in #73642, where there might never be another call to AddCleanup
to create the additional goroutine. Also, if we've hit the maximum on
cleanup goroutines and all of them are in this window simultaneously, we
can still end up missing work, it's just more rare. So this is still a
problem even if we choose to just be more aggressive about creating new
cleanup goroutines.
This change fixes the problem and also aims to make the cleanup
wake/sleep code clearer. The way this change fixes this problem is to
have cleanup goroutines re-check the work list before going to sleep,
but after having already marked themselves as sleeping. This way, if new
work comes in before the cleanup goroutine marks itself as going to
sleep, we can rely on the re-check to pick up that work. If new work
comes after the goroutine marks itself as going to sleep and after the
re-check, we can rely on the scheduler noticing that the goroutine is
asleep and waking it up. If work comes in between a goroutine marking
itself as sleeping and the re-check, then the re-check will catch that
piece of work. However, the scheduler might now get a false signal that
the goroutine is asleep and try to wake it up. This is OK. The sleeping
signal is now mutated and double-checked under the queue lock, so the
scheduler will grab the lock, may notice there are no sleeping
goroutines, and go on its way. This may cause spurious lock acquisitions
but it should be very rare. The window between a cleanup goroutine
marking itself as going to sleep and re-checking the work list is a
handful of instructions at most.
This seems subtle but overall it's a simplification of the code. We
rely more on the lock, which is easier to reason about, and we track two
separate atomic variables instead of the merged cleanupSleepState: the
length of the full list, and the number of cleanup goroutines that are
asleep. The former is now the primary way to acquire work. Cleanup
goroutines must decrement the length successfully to obtain an item off
the full list. The number of cleanup goroutines asleep, meanwhile, is
now only updated with the queue lock held. It can be checked without the
lock held, and the invariant to make that safe is simple: it must always
be an overestimate of the number of sleeping cleanup goroutines.
The changes here do change some other behaviors.
First, since we're tracking the length of the full list instead of the
abstract concept of a wake-up, the waker can't consume wake-ups anymore.
This means that cleanup goroutines may be created more aggressively. If
two threads in the scheduler see that there are goroutines that are
asleep, only one will win the race, but the other will observe zero
asleep goroutines but potentially many work units available. This will
cause it to signal many goroutines to be created. This is OK since we
have a cap on the number of cleanup goroutines, and the race should be
relatively rare.
Second, because cleanup goroutines can now fail to go to sleep if any
units of work come in, they might spend more time contended on the lock.
For example, if we have N cleanup goroutines and work comes in at *just*
the wrong rate, in the worst case we'll have each of G goroutines loop
N times for N blocks, resulting in O(G*N) thread time to handle each
block in the worst case. To paint a picture, imagine each goroutine
trying to go to sleep, fail because a new block of work came in, and
only one goroutine will get that block. Then once that goroutine is
done, we all try again, fail because a new block of work came in, and so
on and so forth. This case is unlikely, though, and probably not worth
worrying about until it actually becomes a problem. (A similar problem
exists with parking (and exists before this change, too) but at least in
that case each goroutine parks, so it doesn't block the thread.)
Fixes #73642.
Change-Id: I6bbe1b789e7eb7e8168e56da425a6450fbad9625
Reviewed-on: https://go-review.googlesource.com/c/go/+/671676
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Pratt <mpratt@google.com>
This instruction is supported in CL 671875, but is not actually used
Change-Id: I943d488ea6dc77781cd796ef480a89fede666bab
Reviewed-on: https://go-review.googlesource.com/c/go/+/673155 Reviewed-by: Meidan Li <limeidan@loongson.cn> Reviewed-by: sophie zhao <zhaoxiaolin@loongson.cn> Reviewed-by: 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>
qmuntal [Thu, 15 May 2025 06:05:52 +0000 (08:05 +0200)]
os: set FILE_FLAG_BACKUP_SEMANTICS when opening without I/O access
FILE_FLAG_BACKUP_SEMANTICS is necessary to open directories on Windows,
and to enable backup applications do extended operations on files if
they hold the SE_BACKUP_NAME and SE_RESTORE_NAME privileges.
os.OpenFile currently sets FILE_FLAG_BACKUP_SEMANTICS for all supported
cases except when the file is opened with O_WRONLY | O_RDWR (that is,
access mode 3). This access mode doesn't correspond to any of the
standard POSIX access modes, but some OSes special case it to mean
different things. For example, on Linux, O_WRONLY | O_RDWR means check
for read and write permission on the file and return a file descriptor
that can't be used for reading or writing.
On Windows, os.OpenFile has historically mapped O_WRONLY | O_RDWR to a
0 access mode, which Windows internally interprets as
FILE_READ_ATTRIBUTES. Additionally, it doesn't prepare the file for I/O,
given that the read attributes permission doesn't allow reading or
writing (not that this is similar to what happens on Linux). This
makes opening the file around 50% faster, and one can still use the
handle to stat it, so some projects have been using this behavior
to open files without I/O access.
This CL updates os.OpenFile so that directories can also be opened
without I/O access. This effectively closes #23312, as all the remaining
cases where we don't set FILE_FLAG_BACKUP_SEMANTICS imply opening
with O_WRONLY or O_RDWR, and that's not allowed by Unix's open.
Closes #23312.
Change-Id: I77c4f55e1ca377789aef75bd8a9bce2b7499f91d
Reviewed-on: https://go-review.googlesource.com/c/go/+/673035 Reviewed-by: Damien Neil <dneil@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
The BoGo IgnoreClientVersionOrder test checks that a client that sends
a supported_versions extension with the list [TLS 1.2, TLS 1.3] ends up
negotiating TLS 1.3.
However, the crypto/tls module treats this list as being in client
preference order, and so negotiates TLS 1.2, failing the test.
Our behaviour appears to be the correct handling based on RFC 8446
§4.2.1 where it says:
The extension contains a list of supported versions in preference
order, with the most preferred version first.
This commit updates the reason we skip this test to cite the RFC instead
of saying it's something to be fixed.
Updates #72006
Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5ca
Reviewed-on: https://go-review.googlesource.com/c/go/+/671415 Reviewed-by: Roland Shoemaker <roland@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Daniel McCarney <daniel@binaryparadox.net>
Keith Randall [Wed, 14 May 2025 23:00:25 +0000 (16:00 -0700)]
cmd/compile: allow load-op merging in additional situations
x += *p
We want to do this with a single load+add operation on amd64.
The tricky part is that we don't want to combine if there are
other uses of x after this instruction.
Implement a simple detector that seems to capture a common situation -
x += *p is in a loop, and the other use of x is after loop exit.
In that case, it does not hurt to do the load+add combo.
Change-Id: I466174cce212e78bde83f908cc1f2752b560c49c
Reviewed-on: https://go-review.googlesource.com/c/go/+/672957 Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Keith Randall [Wed, 14 May 2025 22:57:58 +0000 (15:57 -0700)]
cmd/compile: schedule induction variable increments late
for ..; ..; i++ {
...
}
We want to schedule the i++ late in the block, so that all other
uses of i in the block are scheduled first. That way, i++ can
happen in place in a register instead of requiring a temporary register.
Change-Id: Id777407c7e67a5ddbd8e58251099b0488138c0df
Reviewed-on: https://go-review.googlesource.com/c/go/+/672998
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Keith Randall <khr@google.com>
Yongyue Sun [Thu, 15 May 2025 13:53:23 +0000 (13:53 +0000)]
cmd/compile: create LSym for closures with type conversion
Follow-up to #54959 with another failing case.
The linker needs FuncInfo metadata for all inlined functions. CL 436240 explicitly creates LSym for direct closure calls to ensure we keep the FuncInfo metadata.
However, CL 436240 won't work if the direct closure call is wrapped by a no-effect type conversion, even if that closure could be inlined.
This commit should fix such case.
Fixes #73716
Change-Id: Icda6024da54c8d933f87300e691334c080344695
GitHub-Last-Rev: e9aed02eb6ef343e4ed2d8a79f6426abf917ab0e
GitHub-Pull-Request: golang/go#73718
Reviewed-on: https://go-review.googlesource.com/c/go/+/672855 Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Jonathan Amsterdam [Thu, 15 May 2025 15:16:34 +0000 (11:16 -0400)]
testing: fix panic in t.Log
If a testing.TB is no longer on the stack, t.Log would panic because
its outputWriter is nil. Check for nil and drop the write, which
is the previous behavior.
Change-Id: Ifde97997a3aa26ae604ac9c218588c1980110cbf
Reviewed-on: https://go-review.googlesource.com/c/go/+/673215
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Alan Donovan <adonovan@google.com>
Auto-Submit: Jonathan Amsterdam <jba@google.com>
Nick Ripley [Thu, 15 May 2025 11:01:14 +0000 (07:01 -0400)]
runtime/pprof: include PCs for deduplication in TestMutexBlockFullAggregation
TestMutexBlockFullAggregation aggregates stacks by function, file, and
line number. But there can be multiple function calls on the same line,
giving us different sequences of PCs. This causes the test to spuriously
fail in some cases. Include PCs in the stacks for this test.
Also pick up a small "range over int" modernize suggestion while we're
looking at the test.
Fixes #73641
Change-Id: I50489e19fcf920e27b9eebd9d4b35feb89981cbc
Reviewed-on: https://go-review.googlesource.com/c/go/+/673115 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>
Zxilly [Thu, 15 May 2025 10:50:40 +0000 (10:50 +0000)]
cmd/link: fix outdated output mmap check
Outbuf.View used to perform a mmap check by default
and return an error if the check failed,
this behavior has been changed so that now
the View never returns any error,
so the usage needs to be modified accordingly.
Change-Id: I76ffcda5476847f6fed59856a5a5161734f47562
GitHub-Last-Rev: 6449f2973d28c3b4a5c9e289c38dfcc38f83b3d9
GitHub-Pull-Request: golang/go#73730
Reviewed-on: https://go-review.googlesource.com/c/go/+/673095 Reviewed-by: Keith Randall <khr@golang.org>
Auto-Submit: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
qmuntal [Tue, 13 May 2025 12:28:01 +0000 (14:28 +0200)]
net: avoid windows hang in TestCloseWrite
On Windows, reading from a socket at the same time as the other
end is closing it will occasionally hang. This is a Windows issue, not
a Go issue, similar to what happens in macOS (see #49352).
Work around this condition by adding a brief sleep before the read.
Fixes #73140.
Change-Id: I24e457a577e507d0d69924af6ffa1aa24c4aaaa6
Reviewed-on: https://go-review.googlesource.com/c/go/+/671457 Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Alan Donovan <adonovan@google.com> Reviewed-by: Alan Donovan <adonovan@google.com>
Commit-Queue: Alan Donovan <adonovan@google.com> Reviewed-by: Damien Neil <dneil@google.com>
qmuntal [Tue, 13 May 2025 15:26:06 +0000 (17:26 +0200)]
os: don't fallback to the Stat slow path if file doesn't exist on Windows
os.Stat and os.Lstat first try stating the file without opening it. If
that fails, then they open the file and try again, operations that tends
to be slow. There is no point in trying the slow path if the file
doesn't exist, we should just return an error immediately.
This CL makes stating a non-existent file on Windows 50% faster:
goos: windows
goarch: amd64
pkg: os
cpu: Intel(R) Core(TM) i7-10850H CPU @ 2.70GHz
│ old.txt │ new.txt │
│ sec/op │ sec/op vs base │
StatNotExist-12 43.65µ ± 15% 20.02µ ± 10% -54.14% (p=0.000 n=10+7)
│ old.txt │ new.txt │
│ B/op │ B/op vs base │
StatNotExist-12 224.0 ± 0% 224.0 ± 0% ~ (p=1.000 n=10+7) ¹
¹ all samples are equal
qmuntal [Tue, 13 May 2025 11:31:22 +0000 (13:31 +0200)]
net: use closesocket when closing socket os.File's on Windows
The WSASocket documentation states that the returned socket must be
closed by calling closesocket instead of CloseHandle. The different
File methods on the net package return an os.File that is not aware
that it should use closesocket. Ideally, os.NewFile should detect that
the passed handle is a socket and use the appropriate close function,
but there is no reliable way to detect that a handle is a socket on
Windows (see CL 671455).
To work around this, we add a hidden function to the os package that
can be used to return an os.File that uses closesocket. This approach
is the same as used on Unix, which also uses a hidden function for other
purposes.
While here, fix a potential issue with FileConn, which was using File.Fd
rather than File.SyscallConn to get the handle. This could result in the
File being closed and garbage collected before the syscall was made.
Fixes #73683.
Change-Id: I179405f34c63cbbd555d8119e0f77157c670eb3e
Reviewed-on: https://go-review.googlesource.com/c/go/+/672195 Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Michael Anthony Knyszek [Sat, 10 May 2025 16:14:25 +0000 (16:14 +0000)]
sync: use blockUntilCleanupQueueEmpty instead of busy-looping in tests
testPool currently does the old-style busy loop to wait until cleanups
have executed. Clean this up by using the linkname'd
blockUntilCleanupQueueEmpty.
For #73642.
Change-Id: Ie0c2614db858a984f25b33a805dc52948069eb52
Reviewed-on: https://go-review.googlesource.com/c/go/+/671675 Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Michael Anthony Knyszek [Tue, 25 Feb 2025 22:50:10 +0000 (22:50 +0000)]
runtime: help the race detector detect possible concurrent cleanups
This change makes it so that cleanup goroutines, in race mode, create a
fake race context and switch to it, emulating cleanups running on new
goroutines. This helps in catching races between cleanups that might run
concurrently.
Change-Id: I4c4e33054313798d4ac4e5d91ff2487ea3eb4b16
Reviewed-on: https://go-review.googlesource.com/c/go/+/652635
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Xiaolin Zhao [Wed, 14 May 2025 06:35:41 +0000 (14:35 +0800)]
cmd/compile: fold negation into addition/subtraction on loong64
This change also avoid double negation, and add loong64 codegen for arithmetic tests.
Reduce the number of go toolchain instructions on loong64 as follows.
Change-Id: I0ac50135de2e69fcf802be31e5175d666c93ad4c
Reviewed-on: https://go-review.googlesource.com/c/go/+/667817 Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Keith Randall <khr@golang.org> 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>
Change-Id: I0f8450743e2f7e736c5ff96a316a8b5d98b27222
Reviewed-on: https://go-review.googlesource.com/c/go/+/662475 Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Keith Randall <khr@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Ian Alexander [Tue, 6 May 2025 19:16:03 +0000 (15:16 -0400)]
cmd/go/internal/pkg: fail on bad filenames
Unhidden filenames with forbidden characters in subdirectories now
correctly fail the build instead of silently being skipped.
Previously this behavior would only trigger on files in the root of
the embedded directory.
Fixes #54003
Change-Id: I52967d90d6b7929e4ae474b78d3f88732bdd3ac4
Reviewed-on: https://go-review.googlesource.com/c/go/+/670615 Reviewed-by: Michael Matloob <matloob@google.com> Reviewed-by: Michael Matloob <matloob@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
qiulaidongfeng [Sat, 10 May 2025 17:09:16 +0000 (01:09 +0800)]
cmd/go: fix not print GCCGO when it's not the default
Fixes #69994
Change-Id: I2a23e5998b7421fd5ae0fdb68303d3244361b341
Reviewed-on: https://go-review.googlesource.com/c/go/+/671635 Reviewed-by: Michael Matloob <matloob@golang.org> Reviewed-by: Michael Matloob <matloob@google.com> Reviewed-by: Sean Liao <sean@liao.dev>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
To understand this change, we begin with a short description of the UIR
file format.
Every file is a header followed by a series of sections. Each section
has a kind, which determines the type of elements it contains. An
element is just a collection of one or more primitives, as defined by
package pkgbits.
Strings have their own section. Elements in the string section contain
only string primitives. To use a string, elements in other sections
encode a reference to the string section.
To illustrate, consider a simple file which exports nothing at all.
package p
In the meta section, there is an element representing a package stub.
In that package stub, a string ("p") represents both the path and name
of the package. Again, these are encoded as references.
To manage references, every element begins with a reference table.
Instead of writing the bytes for "p" directly, the package stub encodes
an index in this reference table. At that index, a pair of numbers is
stored, indicating:
1. which section
2. which element index within the section
Effectively, elements always use *2* layers of indirection; first to the
reference table, then to the bytes themselves.
With some minor hand-waving, an encoding for the above package is given
below, with (S)ections, (E)lements and (P)rimitives denoted.
+ Header
| + Section Ends // each section has 1 element
| | + 1 // String is elements [0, 1)
| | + 2 // Meta is elements [1, 2)
| + Element Ends
| | + 1 // "p" is bytes [0, 1)
| | + 6 // stub is bytes [1, 6)
+ Payload
| + (S) String
| | + (E) String
| | | + (P) String { byte } 0x70 // "p"
| + (S) Meta
| | + (E) Package Stub
| | | + Reference Table
| | | | + (P) Entry Count uvarint 1 // there is a single entry
| | | | + (P) 0th Section uvarint 0 // to String, 0th section
| | | | + (P) 0th Index uvarint 0 // to 0th element in String
| | | + Internals
| | | | + (P) Path uvarint 0 // 0th entry in table
| | | | + (P) Name uvarint 0 // 0th entry in table
Note that string elements do not have reference tables like other
elements. They behave more like a primitive.
As this is a bit complicated and getting into details of the UIR file
format, we omit some details in the documentation here. The structure
will become clearer as we continue documenting.
Change-Id: I12a5ce9a34251c5358a20f2f2c4d0f9bd497f4d0
Reviewed-on: https://go-review.googlesource.com/c/go/+/671997 Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Mark Freeman <mark@golang.org>
TryBot-Bypass: Mark Freeman <mark@golang.org>
Julien Cretel [Tue, 13 May 2025 11:11:13 +0000 (11:11 +0000)]
bytes, strings: speed up Split{,After}Seq
CL 669735 brought a welcome performance boost to splitSeq; however, it rendered explodeSeq ineligible for inlining and failed to update that function's doc comment.
This CL inlines the call to explodeSeq in splitSeq, thereby unlocking a further speedup in the case of an empty separator, and removes function explodeSeq altogether.
Ian Alexander [Tue, 13 May 2025 16:41:36 +0000 (12:41 -0400)]
cmd/internal/script: fix copying directory when symlink fails
The change fixes `linkOrCopy` to work on systems wihtout symlinks,
when copying directories. This was originally noticed on Windows
systems when the user did not have admin privs.
Fixes #73692
Change-Id: I8ca66d65e99433ad38e70314abfabafd43794b79
Reviewed-on: https://go-review.googlesource.com/c/go/+/672275 Reviewed-by: Michael Matloob <matloob@golang.org> Reviewed-by: Michael Matloob <matloob@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
khr@golang.org [Wed, 7 May 2025 21:34:30 +0000 (14:34 -0700)]
runtime: increase freebsd/amd64 pointer size from 48 to 57 bits
Because freebsd is now enabling la57 by default.
Fixes #49405
Change-Id: I30f7bac8b8a9baa85e0c097e06072c19ad474e5a
Reviewed-on: https://go-review.googlesource.com/c/go/+/670715
Auto-Submit: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Xiaolin Zhao [Tue, 13 May 2025 02:50:51 +0000 (10:50 +0800)]
cmd/intarnal/obj: add new assembly format for VANDV and VANDB on loong64
In order to make it easier to write in assembly and to be consistent
with the usage of general instructions, a new assembly format is
added for the instructions VANDV and VANDB.
It also works for instructions XVAND{V,B}, [X]V{OR,XOR,NOR,ANDN,ORN}V
and [X]V{OR,XOR,NOR}B.
Change-Id: Ia75d607ac918950e58840ec627aaf0be45d837fe
Reviewed-on: https://go-review.googlesource.com/c/go/+/671316 Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
suntala [Tue, 13 May 2025 21:30:35 +0000 (21:30 +0000)]
testing: add Output
Output is a method on T, B and F. It provides an io.Writer that writes
to the same test output stream as TB.Log. The new output writer is
used to refactor the implementation of Log. It maintains the formatting
provided by Log while making call site information optional.
Additionally, it provides buffering of log messages. This fixes and
expands on
https://go-review.googlesource.com/c/go/+/646956.
For #59928.
Change-Id: I08179c35a681f601cf125c0f4aeb648bc10c7a9f
GitHub-Last-Rev: e6e202793c9bc471493187e0556a3a1e7305ff82
GitHub-Pull-Request: golang/go#73703
Reviewed-on: https://go-review.googlesource.com/c/go/+/672395
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Alan Donovan <adonovan@google.com> Reviewed-by: Alan Donovan <adonovan@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Jonathan Amsterdam <jba@google.com>
Carlos Amedee [Wed, 23 Apr 2025 16:30:52 +0000 (12:30 -0400)]
reflect: use runtime.AddCleanup instead of runtime.SetFinalizer
Replace a usage of runtime.SetFinalizer with runtime.AddCleanup in
the TestCallReturnsEmpty test. There is an additional use of
SetFinalizer in the reflect package which depends on object
resurrection and needs further refactoring to replace.
Updates #70907
Change-Id: I4c0e56c35745a225776bd611d026945efdaf96f5
Reviewed-on: https://go-review.googlesource.com/c/go/+/667595 Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Change-Id: I951087eaef7818697acf87e3206003bcc8a81ee2
Reviewed-on: https://go-review.googlesource.com/c/go/+/672335
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Jonathan Amsterdam <jba@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Roland Shoemaker [Tue, 6 May 2025 16:27:10 +0000 (09:27 -0700)]
crypto/x509: decouple key usage and policy validation
Disabling key usage validation (by passing ExtKeyUsageAny)
unintentionally disabled policy validation. This change decouples these
two checks, preventing the user from unintentionally disabling policy
validation.
Thanks to Krzysztof Skrzętnicki (@Tener) of Teleport for reporting this
issue.
Fixes #73612
Fixes CVE-2025-22874
Change-Id: Iec8f080a8879a3dd44cb3da30352fa3e7f539d40
Reviewed-on: https://go-review.googlesource.com/c/go/+/670375 Reviewed-by: Daniel McCarney <daniel@binaryparadox.net> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Ian Stapleton Cordasco <graffatcolmingov@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
David Finkel [Sat, 14 Dec 2024 01:36:30 +0000 (20:36 -0500)]
cmd/go: add support for git sha256 hashes
Git's supported SHA 256 object hashes since 2.29[1] in 2021, and Gitlab
now has experimental support for sha256 repos.
Take rsc@'s suggestion of checking the of the length of the hashes from
git ls-remote to determine whether a git repo is using sha256 hashes and
decide whether to pass --object-format=sha256 to git init.
Unfortunately, just passing --object-format=sha256 wasn't quite enough,
though. We also need to decide whether the hash-length is 64 hex bytes
or 40 hex bytes when resolving refs to decide whether we've been passed
a full commit-hash. To that end, we use
git config extensions.objectformat to decide whether the (now guaranteed
local) repo is using sha256 hashes and hence 64-hex-byte strings.
[1]: lost experimental status in 2.42 from Aug 2023
(https://lore.kernel.org/git/xmqqr0nwp8mv.fsf@gitster.g/)
For: #68359
Change-Id: I47f480ab8334128c5d17570fe76722367d0d8ed8
Reviewed-on: https://go-review.googlesource.com/c/go/+/636475
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> Reviewed-by: Michael Matloob <matloob@golang.org> Reviewed-by: Michael Matloob <matloob@google.com> Reviewed-by: David Finkel <david.finkel@gmail.com>
xieyuschen [Mon, 14 Oct 2024 11:11:53 +0000 (19:11 +0800)]
cmd/go: support -json flag in go version
It supports features described in the issue:
* add -json flag for 'go version -m' to print json encoding of
runtime/debug.BuildSetting to standard output.
* report an error when specifying -json flag without -m.
* print build settings on seperated line for each binary
Fixes #69712
Change-Id: I79cba2109f80f7459252d197a74959694c4eea1f
Reviewed-on: https://go-review.googlesource.com/c/go/+/619955 Reviewed-by: Sam Thanawalla <samthanawalla@google.com> Reviewed-by: Junyang Shao <shaojunyang@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Radu Berinde [Tue, 13 May 2025 00:45:25 +0000 (00:45 +0000)]
crypto: limit md5 or sha256 blocks processed at once in assembly
This change limits the amount of data that can be hashed at once - the
assembly routines are not preemptible and can result in large latency
outliers when part of a larger system.
suntala [Mon, 12 May 2025 19:23:41 +0000 (19:23 +0000)]
testing: add Output
Output is a method on T, B and F. It provides an io.Writer that writes
to the same test output stream as TB.Log. The new output writer is
used to refactor the implementation of Log. It maintains the formatting
provided by Log while making call site information optional.
Additionally, it provides buffering of log messages.
Yao Zi [Fri, 9 May 2025 15:09:39 +0000 (15:09 +0000)]
cmd/link: ignore mapping symbols on riscv64
Specified in RISC-V ELF psABI[1], mapping symbols are symbols starting
with "$d" or "$x" with STT_NOTYPE, STB_LOCAL and zero sizes, indicating
boundaries between code and data in the same section.
Let's simply ignore them as they're only markers instead of real symbols.
This fixes linking errors like
sym#63 ("$d"): ignoring symbol in section 4 (".riscv.attributes") (type 0)
when using CGO together with Clang and internal linker, which are caused
by unnecessary (but technically correct) mapping symbols created by LLVM
for various sections.
Carlos Amedee [Wed, 30 Apr 2025 19:17:45 +0000 (15:17 -0400)]
runtime: only poll network from one P at a time in findRunnable
This change reintroduces CL 564197. It was reverted due to a failing
benchmark. That failure has been resolved.
For #65064
Change-Id: Ic88841d2bc24c2717ad324873f0f52699f21dc66
Reviewed-on: https://go-review.googlesource.com/c/go/+/669235
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Mauri de Souza Meneguzzo <mauri870@gmail.com> Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Change-Id: Ie81d22ebaf4153388a7e9d8fa0f618a0ae7a1c9f
Reviewed-on: https://go-review.googlesource.com/c/go/+/671875 Reviewed-by: sophie zhao <zhaoxiaolin@loongson.cn> Reviewed-by: Meidan Li <limeidan@loongson.cn> Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Mark Freeman [Fri, 9 May 2025 20:49:15 +0000 (16:49 -0400)]
internal/pkgbits: write a formal grammar for UIR primitives
This complements the grammar being developed in package noder. It
is unclear how to discuss references in their current state, as
they require knowledge of sections, elements, etc.
Perhaps the references here should refer to indices on the byte
array. This would allow a stronger separation of pkgbits and noder.
Change-Id: Ic0e5ac9c07f0a0b92d6ffd4d4e26dbe5dcf89e57
Reviewed-on: https://go-review.googlesource.com/c/go/+/671440 Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Bypass: Mark Freeman <mark@golang.org>
Auto-Submit: Mark Freeman <mark@golang.org>
Sean Liao [Sat, 10 May 2025 11:18:32 +0000 (12:18 +0100)]
text/template: clone options when cloning templates
Fixes #43022
Change-Id: I727b86ea0ebfff06f82c909457479c2afb9106dc
Reviewed-on: https://go-review.googlesource.com/c/go/+/671615 Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Rob Pike <r@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
qmuntal [Fri, 9 May 2025 14:55:00 +0000 (16:55 +0200)]
os: remove NewFile socket detection on Windows
NewFile was recently updated (in CL 668195) to detect whether the
handle is a socket or not. This special case is not really necessary,
given that socket handles can be used as if they were normal file
handles on all functions supported by os.File (see https://learn.microsoft.com/en-us/windows/win32/winsock/socket-handles-2).
Not only is not necessary, but is can also be problematic, as there is
no way to reliably detect whether a handle is a socket or not. For
example, the test failure reported in #73630 is caused by a named pipe
wrongly detected as a socket.
This aligns with the Unix NewFile behavior of returning an os.File that
identifies itself as a file handle even if it is a socket. This makes
os.File.Close to always return os.ErrClosed in case of multiple calls
rather than sometimes returning "use of closed network connection".
Updates #10350.
Fixes #73630.
Change-Id: Ia8329783d5c8ef6dac34ef69ed1ce9d2a9862e11
Reviewed-on: https://go-review.googlesource.com/c/go/+/671455
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Damien Neil <dneil@google.com>
qmuntal [Fri, 9 May 2025 15:42:48 +0000 (17:42 +0200)]
cmd/link: use >4GB base address for 64-bit PE binaries
Windows prefers 64-bit binaries to be loaded at an address above 4GB.
Having a preferred base address below this boundary triggers a
compatibility mode in Address Space Layout Randomization (ASLR) on
recent versions of Windows that reduces the number of locations to which
ASLR may relocate the binary.
The Go internal linker was using a smaller base address due to an issue
with how dynamic cgo symbols were relocated, which has been fixed in
this CL.
Fixes #73561.
Cq-Include-Trybots: luci.golang.try:gotip-windows-amd64-longtest
Change-Id: Ia8cb35d57d921d9be706a8975fa085af7996f124
Reviewed-on: https://go-review.googlesource.com/c/go/+/671515 Reviewed-by: 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>
t-katsumura [Sat, 10 May 2025 23:43:40 +0000 (08:43 +0900)]
net/http: add missing ServeTLS on the comment of http.Server.Shutdown
A sentinel error http.ErrServerClosed is returned after Server.Shutdown
and Server.Close but it is not documented on the Server.Shutdown while
other methods such as Server.Serve are documented on it.
Change-Id: Id82886d9d6a1474a514d62e9169b35f3579a9eee
Reviewed-on: https://go-review.googlesource.com/c/go/+/671695 Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Sean Liao <sean@liao.dev> Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Sean Liao <sean@liao.dev>
runtime: add goschedIfBusy to bgsweep to prevent livelock after inlining
gcMarkTermination() ensures that all caches are flushed before continuing the GC cycle, thus preempting all goroutines.
However, inlining calls to lock() in bgsweep makes it non-preemptible for most of the time, leading to livelock.
This change adds explicit preemption to avoid this.
Fixes #73499.
Change-Id: I4abf0d658f3d7a03ad588469cd013a0639de0c8a
Reviewed-on: https://go-review.googlesource.com/c/go/+/668795
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Change-Id: I4d7e759968779cf8470826b8662b9f2018e663bc
Reviewed-on: https://go-review.googlesource.com/c/go/+/666275
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Keith Randall <khr@google.com>
Auto-Submit: Keith Randall <khr@google.com>
Tobias Klauser [Thu, 8 May 2025 12:13:16 +0000 (14:13 +0200)]
bytes, strings: rename parameters in ExampleCut{Pre,Suf}fix
The old parameter name sep was probably copied from ExampleCut. Change
the parameter names to prefix and suffix, respectivly to make the
examples a bit more readable.
Change-Id: Ie14b0050c2fafe3301c5368efd548a1629a7545f
Reviewed-on: https://go-review.googlesource.com/c/go/+/670955
Auto-Submit: Tobias Klauser <tobias.klauser@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Sean Liao <sean@liao.dev> Reviewed-by: Robert Griesemer <gri@google.com>
Daniel McCarney [Thu, 8 May 2025 21:33:15 +0000 (17:33 -0400)]
crypto/tls: handle client hello version too high
If the client hello legacy version is >= TLS 1.3, and no
supported_versions extension is sent, negotiate TLS 1.2 or lower when
supported.
On the topic of supported version negotiation RFC 8446 4.2.1 indicates
TLS 1.3 implementations MUST send a supported_versions extension with
a list of their supported protocol versions. The crypto/tls package
enforces this when the client hello legacy version indicates TLS 1.3
(0x0304), aborting the handshake with an alertMissingExtension alert if
no supported_versions were received.
However, section 4.2.1 indicates different behaviour should be used when
the extension is not present and TLS 1.2 or prior are supported:
If this extension is not present, servers which are compliant with
this specification and which also support TLS 1.2 MUST negotiate
TLS 1.2 or prior as specified in [RFC5246], even if
ClientHello.legacy_version is 0x0304 or later.
This commit updates the client hello processing logic to allow this
behaviour. If no supported_versions extension was received we ignore the
legacy version being >= TLS 1.3 and instead negotiate a lower supported
version if the server configuration allows.
This fix in turn allows enabling the BoGo ClientHelloVersionTooHigh,
MinorVersionTolerance, and MajorVersionTolerance tests.
Updates #72006
Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5c9
Reviewed-on: https://go-review.googlesource.com/c/go/+/671235 Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Roland Shoemaker <roland@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Previously for protocol versions older than TLS 1.3 our server handshake
implementation sent an alertBadCertificate alert in the case where the
server TLS config indicates a client cert is required and none was
received.
This commit updates the relevant logic to instead send
alertHandshakeFailure in these circumstances.
For TLS 1.2, RFC 5246 §7.4.6 unambiguously describes this as the correct
alert:
If the client does not send any certificates, the
server MAY at its discretion either continue the handshake without
client authentication, or respond with a fatal handshake_failure
alert.
The TLS 1.1 and 1.0 specs also describe using this alert (RFC 4346 §7.4.6
and RFC 2246 §7.4.6) both say:
If client authentication is required by the server for the handshake
to continue, it may respond with a fatal handshake failure alert.
Making this correction also allows enabling the
RequireAnyClientCertificate-TLS1* bogo tests.
Updates #72006
Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5c8
Reviewed-on: https://go-review.googlesource.com/c/go/+/671195
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Roland Shoemaker <roland@golang.org>
Daniel McCarney [Tue, 29 Apr 2025 21:41:53 +0000 (17:41 -0400)]
crypto/tls: enable more large record bogo tests
Previously a handful of large record tests were in the bogo config
ignore list. The ignored tests were failing because they used
insecure ciphersuites that aren't enabled by default.
This commit adds the non-default insecure ciphersuites to the bogo
TLS configuration and re-enables the tests. Doing this uncovered
a handful of unrelated tests that needed to be fixed, each handled
before this commit.
Updates #72006
Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5c7
Reviewed-on: https://go-review.googlesource.com/c/go/+/669158 Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Roland Shoemaker <roland@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Daniel McCarney [Wed, 30 Apr 2025 14:07:10 +0000 (10:07 -0400)]
crypto/tls: skip BadRSAClientKeyExchange-[4,5]
These two bogo tests mutate the version number used for the premaster
secret calculation for a client RSA key exchange, with the expectation
the server rejects the handshake.
Per the comment in the end of rsaKeyAgreement.processClientKeyExchange
we explicitly choose *not* to verify the version number.
This commit adds the two version number tests to the ignore list. They
coincidentally happen to produced the expected failure because they use
a non-default ciphersuite. When we add this ciphersuite to the client
config for the bogo test they will start to fail unless ignored.
Updates #72006
Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5c6
Reviewed-on: https://go-review.googlesource.com/c/go/+/669175
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Roland Shoemaker <roland@golang.org>
Daniel McCarney [Tue, 29 Apr 2025 21:39:08 +0000 (17:39 -0400)]
crypto/tls: err for unsupported point format configs
If a client or server explicitly offers point formats, and the point
formats don't include the uncompressed format, then error. This matches
BoringSSL and Rustls behaviour and allows enabling the
PointFormat-Client-MissingUncompressed bogo test.
Updates #72006
Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5c5
Reviewed-on: https://go-review.googlesource.com/c/go/+/669157 Reviewed-by: Roland Shoemaker <roland@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Unlike in earlier TLS versions, in TLS 1.3 when processing a server
hello the legacy_compression_method MUST have the value 0. It is no
longer a parameter that offers a choice of compression method.
With this in mind, it seems more appropriate to return a decode error
when we encounter a non-zero compression method in a server hello
message. We haven't found a parameter value we reject, we've found
a message that doesn't decode according to its specification.
Making this change also aligns with BoringSSL and allows enabling the
TLS13-HRR-InvalidCompressionMethod bogo test.
Updates #72006
Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5c4
Reviewed-on: https://go-review.googlesource.com/c/go/+/669156 Reviewed-by: Roland Shoemaker <roland@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Daniel McCarney [Tue, 29 Apr 2025 18:33:15 +0000 (14:33 -0400)]
crypto/tls: use illegal param alert for bad compression
Previously if the clientHandshakeState for the TLS 1.2 client code
encountered a server helo message that contained a compression method
other than compressionNone, we would emit an unexpected message alert.
Instead, it seems more appropriate to return an illegal parameter alert.
The server hello message _was_ expected, it just contained a bad
parameter option.
Making this change also allows enabling the InvalidCompressionMethod
bogo test.
Updates #72006
Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5c3
Reviewed-on: https://go-review.googlesource.com/c/go/+/669155 Reviewed-by: Roland Shoemaker <roland@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Michael Matloob [Thu, 6 Feb 2025 21:11:50 +0000 (16:11 -0500)]
cmd/go: change go tool to build tools missing from GOROOT/pkg/tool
If a tool in cmd is not installed in $GOROOT/pkg/tool/${GOOS}_${GOARCH},
go tool will build (if it's not cached) and run it in a similar way
(with some changes) to how tools declared with tool directives are built
and run.
The main change in how builtin tools are run as compared to mod tools is
that they are built "in host mode" using the running go command's GOOS
and GOARCH. The "-exec" flag is also ignored and we don't add GOROOT/bin
to the PATH.
A ForceHost function has been added to the cfg package to force the
configuration to runtime.GOOS/runtime.GOARCH. It has to recompute the
BuildContext because it's normally determined at init time but we're
changing it after we realize we're running a builtin tool. (Detecting
that we're running a builtin tool at init time would mean replicating
the cmd line parsing logic so recomputing BuildContext sounds like the
smaller change.)
For #71867
Change-Id: I3b2edf2cb985c1dcf5f845fbf39b7dc11dea4df7
Reviewed-on: https://go-review.googlesource.com/c/go/+/666476
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>
Mark Freeman [Wed, 7 May 2025 20:49:02 +0000 (16:49 -0400)]
cmd/compile/internal/noder: begin documenting meta section
Meta is the most fundamental section. To flesh this out, we discuss references. Primitives are briefly mentioned by pointing to pkgbits,
where they will be defined using a similar grammar.
Change-Id: I7abd899f38fad4cc5caf87ebfc7aa1b1985b17d4
Reviewed-on: https://go-review.googlesource.com/c/go/+/671176
Auto-Submit: Mark Freeman <mark@golang.org>
TryBot-Bypass: Mark Freeman <mark@golang.org> Reviewed-by: Robert Griesemer <gri@google.com>