Deeply nested parenthesized expressions could cause a stack
overflow during parsing. This change introduces a depth limit
(maxStackDepth) tracked in Tree.stackDepth to prevent this.
Additionally, this commit clarifies the security model in
the package documentation, noting that template authors
are trusted as text/template does not auto-escape.
Fixes#71201
Change-Id: Iab2c2ea6c193ceb44bb2bc7554f3fccf99a9542f
GitHub-Last-Rev: f4ebd1719f
GitHub-Pull-Request: golang/go#73670
Reviewed-on: https://go-review.googlesource.com/c/go/+/671755
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Sean Liao <sean@liao.dev>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Rob Pike <r@golang.org>
The "doc", "fix", and "covdata" tools invoked by the go command are not
needed for builds. Instead of invoking them directly using the installed
binary in the tool directory, use "go tool" to run them, building them
if needed. We can then stop distributing those tools in the
distribution.
covdata is used in tests and can form part of a cached test result, but
test results don't have the same requirements as build outputs to be
completely determined by the action id. We already don't include a
toolid for the covdata tool in the action id for a test run. The more
principled way to do things would be to load the covdata package,
create the actions to build it, and then depend on the output of
that action from the the test action and use that as the covdata tool.
For now, it's probably not worth the effort, but, in the future, if we
wanted to build a tool like cgo as needed, it would be best to build it
in the same action graph. That would introduce a whole bunch of complexity
because we'd need to build the tool in the host configuration, and all
the configuration parameters are global.
For #71867
Change-Id: Id9bbbb5c169296f66c072949f9da552424ecfa2f
Reviewed-on: https://go-review.googlesource.com/c/go/+/673119
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Michael Matloob <matloob@google.com>
This change removes some tools that are not used for builds, or
otherwise invoked by the go command (other than through "go tool"
itself) from the packaged distributions produced by distpack. When these
tools are missing, "go tool" will build and run them as needed.
Also update a case where we print a buildid commandline to specify
invoking buildid using "go tool" rather than the binary at it's install
location, because it may not exist there in packaged distributions
anymore.
The tools in this CL are the lowest hanging fruit. There are a few more
tools that aren't used by builds, but we'd have to get the go command to
run them using "go tool" rather than finding them in the tool install
directory.
For #71867
Change-Id: I217683bd549962a1add87405bf3fb1225e2333c5
Reviewed-on: https://go-review.googlesource.com/c/go/+/666755
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Michael Matloob <matloob@google.com>
The purpose of this change is to enable go get to be used when working
on a module that is usually built from a workspace and has unreleased
dependencies in that workspacae that have no requirements satisfying
them. These modules can't build in single module mode, and the
expectation is that a workspace will provide the unreleased
requirements.
Before this change, if go get was run in a module, and any of the
module's imports, that were not already satisfied using requirements,
could not be resolved from that module in single module mode, go get
would report an error. This could happen if, for example, the dependency
was unreleased, even privately, and couldn't be fetched using version
control or a proxy. go get would also do a check using
cmd/go/internal/modget.(*resolver).checkPackageProblems that, among
other things, any package patterns provided to go get (the pkgPattern
argument to checkPackageProblems) could properly load. When checking in
single-module mode, this would cause an error because imports in the
non-required workspace dependencies could not be resolved.
This change makes a couple of changes to address each of those problems.
First, while "go get" still uses the single module's module graph to
load packages and determine which imports are not satisfied by a module
in the build list (the set of modules the build is done with), it will
"cheat" and look up the set of modules that would be loaded in workspace
mode. It will not try to fetch modules to satisfy imports of packages in
those modules. (Alternatively, it could have tried to fetch modules to
satisfy the requirements, and allowed an error if it could not be found,
but we took the route of not doing the fetch to preserve the invariant
that the behavior wouldn't change if the network was down). The second,
and by far more complex, change is that the load that's done in
checkPackageProblems will be done in workspace mode rather than module
mode. While it is important that the requirements added by "go get" are
not determined using the workspace (with the necessary exception of the
skipped fetches) it is okay to use the workspace to load the modules,
as, if a go.work file is present, the go command would by default run
builds in workspace mode rather than single module mode. This more
relaxed check will allow get to succeed if a go list would succeed in
the workspace, even if it wouldn't from the single module.--
To avoid trying to satisfy imports that are in the other workspace
modules, we add a workspace field to the resolver that can be used to
check if an import is in the workspace. It reads the go.work file and
determines each of the modules' modroots. The hasPackage function will
call into modload logic using the new PkgIsInLocalModule function that
in turn calls into dirInModule to determine if the directory that would
contain the package sources exists in the module. We do that check in
cmd/go/internal/modget.(*resolver).loadPackages, which is used to
resolve modules to satisfy imports in the package graph supplied on the
command line. (Note that we do not skip resolving modules in the
functions that query for a package at a specific module version (such as
in "go get golang.org/x/tools/go/packages@latest), which are done in
(*resolver).queryPath. In that case, the user is explicitly requesting
to add a requirement on that package's module.)
The next step, checking for issues in the workspace mode, is more
complex because of two reasons. First, can't do all of
checkPackageProblems's work in a workspace, and second, that the module
loading state in the go command is global, so we have to manage the
global state in switching to workspace mode and back.
On the work that checkPackageProblems does: it broadly does three things:
first, a load of the packages specified to "go get", to make sure that
imports are satisfied and not ambiguous. Second, using that loaded
information, reporting any retracted or deprecated modules that are
relevant to the user and that they may want to take action on. And
third, checking that all the modules in the build list (the set of
module versions used in a load or build) have sums, and adding those
sums to the in-memory representation of the go.sum file that will be
written out at the end. When there's a workspace, the first two checks
need to be done in workspace mode so that we properly point out issues
in the build, but the sums need to be updated in module mode so that the
module's go.sum file is updated to reflect changes in go.mod.
To do the first two steps in workspace mode, we add a new
modload.EnterWorkspace function that will reset the global modload state
and load from the workspace, using the updated requirements that have
been calculated for the module. It returns a cleanup function that will
exit the workspace and reset to the previous global state. (We need the
previous global state because it holds the updated in memory
representations of go.mod and go.sum that will be written out by go get
if there are no errors.) We switch to workspace mode if there's a
relevant go.work file that would trigger a workspace load _and_ the
module go get is being run from belongs to that workspace (it wouldn't
make sense to use a workspace that the module itself didn't belong to).
We then switch back to module mode after the first two steps are
complete using the cleanup function. We have to be careful to finish
all the tasks checking for deprecations and retractions before to start
looking at the sums because they retraction and deprecation checking
tasks will depend on the global workspace state.
It's a bit unfortunate that much of the modload and modfetch state is
global. It's pretty gross doing the switch. It would be a lot of
work, but if we need to do a switch in a third instance other than for
go work sync and go get it might be worth doing the work to refactor
modload so that the state isn't global.
The EnterWorkspace function that does the switch to workspace mode (in
cmd/go/internal/modload/init.go) first saves the in memory
representation of the go.mod file (calculated using UpdateGoModFromReqs)
so that it can be applied in the workspace. It then uses the new
setState function to save the old state and reset to a clean state,
loads in workspace mode (using InitWorkfile to so that the go.work file
is used by LoadModFile), and then replaces the go.mod file
representation for the previous main module with the contents saved
earlier and reloads the requirements (holding the roots of the module
graph) to represent the updates. In workspace mode, the roots field of the
requirements is the same, but the reload is used to update the set of
direct requirements. rawGoModSummary is also update to use the in-
memory representation of the previous main module's go.mod file rather
than always reading it from disk so that it can take those updated
contents into account. (It previously didn't need to do this because
rawGoModSummary is primarily used to get module information for
dependency modules. The exception is in workspace mode but the same
logic worked because we didn't update workspace module's go.mod files in
a go command running in workspace mode). Finally, EnterWorkspace returns
a function that calls setState with the old state, to revert to the
previous state.
When we save the state of the modload package, we also need to save the
state of the modfetch package because it holds the state of the go.sum
file and the operation of looking up a value in the lookupCache or
downloadCache affects whether it appears in the go.sum file. lookupCache
and downloadCache are turned into pointers so that they can be saved in
the modfetchState.
Fixes#73654
Change-Id: I65cf835ec2293d4e3f66b91d3e77d3bb8d2f26d7
Reviewed-on: https://go-review.googlesource.com/c/go/+/669635
Reviewed-by: Michael Matloob <matloob@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Sam Thanawalla <samthanawalla@google.com>
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>
This updates cgo support for s390x changing from z196 to z13, as
z13 is the minimum machine level running on go for s390x.
Change-Id: I1a102294b2108c35ddb1428bf287ce83debaeac8
Reviewed-on: https://go-review.googlesource.com/c/go/+/666995
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>
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>
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>
before:
MOVV $n + $offset, Roff
PRELDX (Rbase)(Roff), $hint
after:
PRELDX offset(Rbase), $n, $hint
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>
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>
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>
It's hard to compare two different runs of a benchmark if they
are doing different amounts of work.
Change-Id: I5d6845f3d11bb10136f745e6207d5f683612276d
Reviewed-on: https://go-review.googlesource.com/c/go/+/672895
Reviewed-by: Junyang Shao <shaojunyang@google.com>
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
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>
OpMove is faster for small moves of fixed size.
For safety, we have to rewrite the Move rewrite rules a bit so that
all the loads are done before any stores happen.
Also use an 8-byte move instead of a 16-byte move if the tail is
at most 8 bytes.
Change-Id: I7f6c7496ac6d5eb2e0706fd59ca4b5d797c51101
Reviewed-on: https://go-review.googlesource.com/c/go/+/672997
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
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: e9aed02eb6
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>
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>
For #71661.
Change-Id: I802b0c36cac3bbd87b35ff216f06822e87fb7b5d
Reviewed-on: https://go-review.googlesource.com/c/go/+/671439
Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
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>
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: 6449f2973d
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>
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>
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
│ old.txt │ new.txt │
│ allocs/op │ allocs/op vs base │
StatNotExist-12 2.000 ± 0% 2.000 ± 0% ~ (p=1.000 n=10+7) ¹
Updates #72992.
Change-Id: Iaeb9596d0d18e5a5a1bd1970e296a3480501af78
Reviewed-on: https://go-review.googlesource.com/c/go/+/671458
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Jake Bailey <jacob.b.bailey@gmail.com>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
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>
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>
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>
On every arch except amd64, it is faster to do x&(x-1) than x^(1<<n).
Most archs need 3 instructions for the latter: MOV $1, R; SLL n, R;
ANDN R, x. Maybe 4 if there's no ANDN.
Most archs need only 2 instructions to do x&(x-1). It takes 3 on
x86/amd64 because NEG only works in place.
Only amd64 can do x^(1<<n) in a single instruction.
(We could on 386 also, but that's currently not implemented.)
Change-Id: I3b74b7a466ab972b20a25dbb21b572baf95c3467
Reviewed-on: https://go-review.googlesource.com/c/go/+/672956
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
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>
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>
Match standard Unix behavior: Symlinks are not followed when
O_CREATE|O_EXCL is passed to open.
Thanks to Junyoung Park and Dong-uk Kim of KAIST Hacking Lab
for discovering this issue.
Fixes#73702
Fixes CVE-2025-0913
Change-Id: Ieb46a6780c5e9a6090b09cd34290f04a8e3b0ca5
Reviewed-on: https://go-review.googlesource.com/c/go/+/672396
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>
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>
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>
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>
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: e6e202793c
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>
Replace the use of SetFinalizer with AddCleanup.
For #70907
Change-Id: I0cb2c2985eb9285e5f92be9dbcb9d77acc0f59c5
Reviewed-on: https://go-review.googlesource.com/c/go/+/671441
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
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>
This reverts commit 8d189f188e.
Reason for revert: failing test
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>
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>
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>
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>
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.
Co-authored-by: Aleks Fazlieva <britishrum@users.noreply.github.com>
Fixes#59928.
Change-Id: I29090b3d4f61f7334388b373ec18750d5637aafa
GitHub-Last-Rev: 18af0e1526
GitHub-Pull-Request: golang/go#71575
Reviewed-on: https://go-review.googlesource.com/c/go/+/646956
Reviewed-by: Arati <artichaut2023@gmail.com>
Auto-Submit: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
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.
[1]: 87aecf6017/riscv-elf.adoc?plain=1#L1448
Fixes#73516
Change-Id: I02ca90c100ba8a38733fe3b8b8403836b44a3dd1
GitHub-Last-Rev: d7842ceafb
GitHub-Pull-Request: golang/go#73592
Reviewed-on: https://go-review.googlesource.com/c/go/+/669675
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Meng Zhuo <mengzhuo1203@gmail.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Ville Vesilehto
Michael Matloob
Julian Zhu
Michael Anthony Knyszek
Srinivas Pokala
Sam Thanawalla
Guoqi Chen
qmuntal
Daniel McCarney
Keith Randall
Yongyue Sun
Jonathan Amsterdam
Nick Ripley
Zxilly
Xiaolin Zhao
kmvijay
Ian Alexander
qiulaidongfeng
Mark Freeman
Damien Neil
Julien Cretel
limeidan
suntala
Carlos Amedee
Roland Shoemaker
David Finkel
Sean Liao
xieyuschen
Radu Berinde
Yao Zi