Should fix some asan build failures.
Change-Id: Ic0a816b56a1a278aa0ad541aea962f9fea7b10fc
Reviewed-on: https://go-review.googlesource.com/c/go/+/674696
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
All private keys need to go through a slow PCT in FIPS-140 mode.
ECDH and RSA keys have places to hide a precomputed value without
causing races, but Ed25519 and ECDSA keys might be constructed by the
application and then used with concurrent Sign calls.
For these, implement an equivalent to crypto/internal/boring/bcache
using weak.Pointer and runtime.AddCleanup.
fips140: latest
goos: linux
goarch: amd64
pkg: crypto/ed25519
cpu: AMD Ryzen 7 PRO 8700GE w/ Radeon 780M Graphics
│ 1a93e4a2cf │ 78a819ea78 │
│ sec/op │ sec/op vs base │
Signing-16 72.72µ ± 0% 16.93µ ± 1% -76.72% (p=0.002 n=6)
fips140: off
goos: linux
goarch: amd64
pkg: crypto/ed25519
cpu: AMD Ryzen 7 PRO 8700GE w/ Radeon 780M Graphics
│ 310bad31e5 │ 310bad31e5-dirty │
│ sec/op │ sec/op vs base │
Signing-16 17.18µ ± 1% 16.95µ ± 1% -1.36% (p=0.002 n=6)
fips140: latest
goos: linux
goarch: amd64
pkg: crypto/ecdsa
cpu: AMD Ryzen 7 PRO 8700GE w/ Radeon 780M Graphics
│ 1a93e4a2cf │ 78a819ea78 │
│ sec/op │ sec/op vs base │
Sign/P256-16 90.97µ ± 0% 21.04µ ± 0% -76.87% (p=0.002 n=6)
Sign/P384-16 701.6µ ± 1% 142.0µ ± 0% -79.75% (p=0.002 n=6)
Sign/P521-16 2943.5µ ± 1% 491.9µ ± 0% -83.29% (p=0.002 n=6)
fips140: off
goos: linux
goarch: amd64
pkg: crypto/ecdsa
cpu: AMD Ryzen 7 PRO 8700GE w/ Radeon 780M Graphics
│ 1a93e4a2cf │ 78a819ea78 │
│ sec/op │ sec/op vs base │
Sign/P256-16 21.27µ ± 0% 21.13µ ± 0% -0.65% (p=0.002 n=6)
Sign/P384-16 143.3µ ± 0% 142.4µ ± 0% -0.63% (p=0.009 n=6)
Sign/P521-16 525.3µ ± 0% 462.1µ ± 0% -12.04% (p=0.002 n=6)
This unavoidably introduces allocations in the very first use of Ed25519
private keys, but usually that's not in the hot path.
Change-Id: I6a6a465640a5dff64edd73ee5dda5f2ad1b476b9
Reviewed-on: https://go-review.googlesource.com/c/go/+/654096
Auto-Submit: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Replace the hchan.synctest bool with an hchan.bubble reference
to the synctest bubble that created the chan. I originally used
a bool to avoid increasing the size of hchan, but we have space
in hchan's current size class for another pointer.
This lets us detect one bubble operating on a chan created
in a different bubble.
For #67434
Change-Id: If6cf9ffcb372fe7fb3f8f4ef27b664848578ba5c
Reviewed-on: https://go-review.googlesource.com/c/go/+/674515
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Damien Neil <dneil@google.com>
Use a sync.OnceValue rather than a sync.WaitGroup to
coordinate access to encoderCache entries.
The OnceValue better expresses the intent of the code
(we want to initialize the cache entry only once).
However, the motivation for this change is to avoid
testing/synctest incorrectly reporting a deadlock
when multiple bubbles call Marshal at the same time.
Goroutines blocked on WaitGroup.Wait are "durably blocked",
causing confusion when a goroutine in one bubble Waits
for a goroutine in a different bubble. Goroutines blocked
on OnceValue are not durably blocked, avoiding the problem.
Fixes#73733
For #67434
Change-Id: I81cddda80af67cf5c280fd4327620bc37e7a6fe6
Reviewed-on: https://go-review.googlesource.com/c/go/+/673335
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Add a synctest.Test function, superseding the experimental
synctest.Run function. Promote the testing/synctest package
out of experimental status.
For #67434
For #73567
Change-Id: I3c5ba030860d90fe2ddb517a2f3536efd60181a9
Reviewed-on: https://go-review.googlesource.com/c/go/+/671961
Auto-Submit: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
This change switches the pkgsite command invoked to start a pkgsite
server from golang.org/x/pkgsite/cmd/pkgsite to
golang.org/x/pkgsite/cmd/internal/doc. The doc command is a simplified
version of cmd/pkgsite that changes some options to improve the user
experience. For example, it limits logging informational log messages,
doesn't always expect to find modules (for example if we're outside of a
module getting documentation for the standard library), and it takes the
address of the page to open in the browser (which simplifies waiting for
the server to start listening).
Fixes#68106
Change-Id: I667a49d03823242fa1aff333ecb1c0f198e92412
Reviewed-on: https://go-review.googlesource.com/c/go/+/674158
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Reviewed-by: David Chase <drchase@google.com>
cmd/doc uses go/build to get information about the packages it's
documenting. In some cases, go/build can return a build.Package that it
couldn't determine an import path for, in which case it sets the import
path to ".". This can happen for relative package paths in in a module:
for relative package paths we don't use the go command to get
information about the module and just open the source files directly
instead, and will be missing the import path. This is usually okay
because go doc doesn't need to print the import path of the package it's
documenting, but for go doc -http, we want to know the import path so we
can open the right page in the browser.
For #68106
Change-Id: Ifba92862ad01d8d63f531c2451f18db2b0d7a3e5
Reviewed-on: https://go-review.googlesource.com/c/go/+/674556
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Michael Matloob <matloob@google.com>
This fixes a bug where we start pkgsite for every requested object,
rather than the one that we would have printed the documentation for.
To make things simple, we'll run the logic that prints the
documentation, but with an io.Discard writer. Then we can tell if the
documentation was found based on the return values of those functions.
For #68106
Change-Id: Ibf2ab1720f381d7214fc9239b9c2e915c91f7f7b
Reviewed-on: https://go-review.googlesource.com/c/go/+/674555
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Matloob <matloob@google.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
This CL implements the TODO in combineStores to allow combining
stores of different sizes, as long as the total size aligns to
2, 4, 8.
Fixes#72832.
Change-Id: I6d1d471335da90d851ad8f3b5a0cf10bdcfa17c4
Reviewed-on: https://go-review.googlesource.com/c/go/+/661855
Reviewed-by: Keith Randall <khr@golang.org>
Auto-Submit: Junyang Shao <shaojunyang@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Like Sync, Ref[T] is also used to define things like StringRef.
Change-Id: I9e10234504ee4dd03907bb058a6f3ae7e6a287ca
Reviewed-on: https://go-review.googlesource.com/c/go/+/674157
Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Bypass: Mark Freeman <mark@golang.org>
Auto-Submit: Mark Freeman <mark@golang.org>
This implementation is zero-alloc when T is a concrete type,
allocates when val contains a method or when T is a interface
and Value was obtained for example through Elem(), in which case
it has to be allocated to avoid sharing the same memory.
goos: linux
goarch: amd64
pkg: reflect
cpu: AMD Ryzen 5 4600G with Radeon Graphics
│ /tmp/bench2 │
│ sec/op │
TypeAssert/TypeAssert[int](int)-12 2.725n ± 1%
TypeAssert/TypeAssert[uint8](int)-12 2.599n ± 1%
TypeAssert/TypeAssert[fmt.Stringer](reflect_test.testTypeWithMethod)-12 8.470n ± 0%
TypeAssert/TypeAssert[fmt.Stringer](*reflect_test.testTypeWithMethod)-12 8.460n ± 1%
TypeAssert/TypeAssert[interface_{}](int)-12 4.181n ± 1%
TypeAssert/TypeAssert[interface_{}](reflect_test.testTypeWithMethod)-12 4.178n ± 1%
TypeAssert/TypeAssert[time.Time](time.Time)-12 2.839n ± 0%
TypeAssert/TypeAssert[func()_string](func()_string)-12 151.1n ± 1%
geomean 6.645n
│ /tmp/bench2 │
│ B/op │
TypeAssert/TypeAssert[int](int)-12 0.000 ± 0%
TypeAssert/TypeAssert[uint8](int)-12 0.000 ± 0%
TypeAssert/TypeAssert[fmt.Stringer](reflect_test.testTypeWithMethod)-12 0.000 ± 0%
TypeAssert/TypeAssert[fmt.Stringer](*reflect_test.testTypeWithMethod)-12 0.000 ± 0%
TypeAssert/TypeAssert[interface_{}](int)-12 0.000 ± 0%
TypeAssert/TypeAssert[interface_{}](reflect_test.testTypeWithMethod)-12 0.000 ± 0%
TypeAssert/TypeAssert[time.Time](time.Time)-12 0.000 ± 0%
TypeAssert/TypeAssert[func()_string](func()_string)-12 72.00 ± 0%
geomean ¹
Fixes#62121
Change-Id: I0911c70c5966672c930d387438643f94a40441c4
GitHub-Last-Rev: ce89a53097
GitHub-Pull-Request: golang/go#71639
Reviewed-on: https://go-review.googlesource.com/c/go/+/648056
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
A chan created within a synctest bubble may not be
operated on from outside the bubble.
We panicked on send and receive, but not close.
Panic on close as well.
For #67434
Change-Id: I98d39e0cf7baa1a679aca1fb325453d69c535308
Reviewed-on: https://go-review.googlesource.com/c/go/+/671960
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Damien Neil <dneil@google.com>
For goroutines in a synctest bubble, include whether the goroutine
is "durably blocked" or not in the goroutine status.
Synctest categorizes goroutines in certain states as "durably"
blocked, where the goroutine is not merely idle but can only
be awoken by another goroutine in its bubble. To make it easier
for users to understand why a bubble is or is not idle,
print the state of each bubbled goroutine.
For example:
goroutine 36 [chan receive, synctest bubble 34, not durably blocked]:
goroutine 37 [chan receive (synctest), synctest bubble 34, durably blocked]:
Goroutine 36 is receiving from a channel created outside its bubble.
Goroutine 36 is receiving from a channel created inside its bubble.
For #67434
Change-Id: I006b656a9ce7eeb75b2be21e748440a5dd57ceb0
Reviewed-on: https://go-review.googlesource.com/c/go/+/670976
Auto-Submit: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
For #71661.
Change-Id: I74870de3f17a938bc9fd83ccc41e13e64b55b5e8
Reviewed-on: https://go-review.googlesource.com/c/go/+/671438
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
This change adds tracking for approximate finalizer and cleanup queue
lengths. These lengths are reported once every GC cycle as a single line
printed to stderr when GODEBUG=checkfinalizer>0.
This change lays the groundwork for runtime/metrics metrics to produce
the same values.
For #72948.
For #72950.
Change-Id: I081721238a0fc4c7e5bee2dbaba6cfb4120d1a33
Reviewed-on: https://go-review.googlesource.com/c/go/+/671437
Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
These were just enabled by https://go.dev/cl/643897, but freebsd
unfortunately doesn't seem to support cgo + race mode by default.
For #73788.
Cq-Include-Trybots: luci.golang.try:gotip-freebsd-amd64-race
Change-Id: I6a6a636c06176ca746548d0588283b1429d7c6d5
Reviewed-on: https://go-review.googlesource.com/c/go/+/674160
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>
This will be useful to show how the next CLs improve things.
Change-Id: I49a691295c1fe3c7455a67c7d19e5c03979f714a
Reviewed-on: https://go-review.googlesource.com/c/go/+/673015
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This change dumps a scan trace (each pointer marked and where it came
from) for the partial GC cycle performed by checkfinalizers mode when
checkfinalizers>1. This is useful for quickly understanding why certain
values are reachable without having to pull out tools like viewcore.
For #72949.
Change-Id: Ic583f80e9558cdfe1c667d27a1d975008dd39a9c
Reviewed-on: https://go-review.googlesource.com/c/go/+/662038
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This change adds support for identifying cleanups and finalizers
attached to tiny blocks to checkfinalizers mode. It also notes a subtle
pitfall, which is that the cleanup arg, if tiny-allocated, could end up
co-located with the object with the cleanup attached! Oops...
For #72949.
Change-Id: Icbe0112f7dcfc63f35c66cf713216796a70121ce
Reviewed-on: https://go-review.googlesource.com/c/go/+/662037
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
This change adds a new special kind called CheckFinalizer which is used
to annotate finalizers and cleanups with extra information about where
that cleanup or finalizer came from.
For #72949.
Change-Id: I3c1ace7bd580293961b7f0ea30345a6ce956d340
Reviewed-on: https://go-review.googlesource.com/c/go/+/662135
Reviewed-by: Carlos Amedee <carlos@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Windows API's TransmitFile function is limited to two concurrent
operations on workstation and client versions of Windows. This change
modifies the net.sendFile function to perform no work in such cases
so that TransmitFile is avoided.
Fixes#73746
Change-Id: Iba70d5d2758bf986e80c78254c8e9e10b39bb368
GitHub-Last-Rev: 315ddc0cd8
GitHub-Pull-Request: golang/go#73758
Reviewed-on: https://go-review.googlesource.com/c/go/+/673855
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Quim Muntal <quimmuntal@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
Before this change, when we tried to compute the set of packages in
'all', we'd add packages with invalid import paths to the set and try to
load them, which would fail. Instead, do not add them to the list of
packages to load in the second iteration of the loader. We'll still
return errors for invalid imports in the importing packages.
Change-Id: I682229011f555ed1d0c827f79100c1c43bf7f93a
Reviewed-on: https://go-review.googlesource.com/c/go/+/673655
Reviewed-by: Michael Matloob <matloob@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This new debug mode detects cleanup/finalizer leaks using checkmark
mode. It runs a partial GC using only specials as roots. If the GC can
find a path from one of these roots back to the object the special is
attached to, then the object might never be reclaimed. (The cycle could
be broken in the future, but it's almost certainly a bug.)
This debug mode is very barebones. It contains no type information and
no stack location for where the finalizer or cleanup was created.
For #72949.
Change-Id: Ibffd64c1380b51f281950e4cfe61f677385d42a5
Reviewed-on: https://go-review.googlesource.com/c/go/+/634599
Reviewed-by: Carlos Amedee <carlos@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
When an errorcheck test uses -m and instantiates an imported generic
function, the errors will include -m messages from the imported package
(since the new function has not previously been walked). These errors
cannot be matched since we can't write errors in files outside the test
input.
To fix this (and enable the other CLs in this stack), drop any unmatched
errors that occur in files outside those in the input set.
Change-Id: I2fcf0dd4693125d2e5823ea4437011730d8b1b1f
Reviewed-on: https://go-review.googlesource.com/c/go/+/672515
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
We want gcflags, which control builder type (e.g. noopt) to be used
for these tests also.
Should fix noopt and maybe other builders.
Change-Id: Iad34beab51714f0c38989ec0fc8778cf79087f72
Reviewed-on: https://go-review.googlesource.com/c/go/+/674455
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
This adds additional logging for the work that walk does to reduce
how often an interface conversion results in an allocation.
Also, as part of #71359, we will be updating how escape analysis and
walk handle basic literals, composite literals, and zero values,
so add some tests that uses this new logging.
By the end of our CL stack, we address all of these tests.
Updates #71359
Change-Id: I43fde8343d9aacaec1e05360417908014a86c8bd
Reviewed-on: https://go-review.googlesource.com/c/go/+/649076
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: David Chase <drchase@google.com>
Auto-Submit: Keith Randall <khr@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This change makes purego implementation of maphash.Comparable consistent
with the one in runtime and fixes hashing of channels.
Fixes#73657
Change-Id: If78a21d996f0c20c0224d4014e4a4177b09c3aa3
GitHub-Last-Rev: 2537216a1e
GitHub-Pull-Request: golang/go#73660
Reviewed-on: https://go-review.googlesource.com/c/go/+/671655
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: qiu laidongfeng2 <2645477756@qq.com>
This enable http.RoundTripper implementation to retry POST request (let's
say after a 500) after a 307/308 redirect.
Fixes#73439
Change-Id: I4365ff58b012c7f0d60e0317a08c98b1d48f657e
Reviewed-on: https://go-review.googlesource.com/c/go/+/666735
Reviewed-by: Sean Liao <sean@liao.dev>
Auto-Submit: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
CL 614257 refactored mallocgc but lost an optimization: if a span for a
large object is already backed by memory fresh from the OS (and thus
zeroed), we don't need to zero it. CL 614257 unconditionally zeroed
spans for large objects that contain pointers.
This change restores the optimization from before CL 614257, which seems
to matter in some real-world programs.
While we're here, let's also fix a hole with the garbage collector being
able to observe uninitialized memory of the large object is observed
by the conservative scanner before being published. The gory details are
in a comment in heapSetTypeLarge. In short, this change makes
span.largeType an atomic variable, such that the GC can only observe
initialized memory if span.largeType != nil.
Fixes#72991.
Change-Id: I2048aeb220ab363d252ffda7d980b8788e9674dc
Reviewed-on: https://go-review.googlesource.com/c/go/+/659956
Reviewed-by: Keith Randall <khr@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com>
Currently, it's possible for asynchronous preemption to observe a
partially initialized object. The sequence of events goes like this:
- The GC is in the mark phase.
- Thread T1 is allocating object O1.
- Thread T1 zeroes the allocation, runs the publication barrier, and
updates freeIndexForScan. It has not yet updated the mark bit on O1.
- Thread T2 is conservatively scanning some stack frame.
That stack frame has a dead pointer with the same address as O1.
- T2 picks up the pointer, checks isFree (which checks
freeIndexForScan without an import barrier), and sees that O1 is
allocated. It marks and queues O1.
- T2 then goes to scan O1, and observes uninitialized memory.
Although a publication barrier was executed, T2 did not have an import
barrier. T2 may thus observe T1's writes to zero the object out-of-order
with the write to freeIndexForScan.
Normally this would be impossible if T2 got a pointer to O1 from
somewhere written by T1. The publication barrier guarantees that if the
read side is data-dependent on the write side then we'd necessarily
observe all writes to O1 before T1 published it. However, T2 got the
pointer 'out of thin air' by scanning a stack frame with a dead pointer
on it.
One fix to this problem would be to add the import barrier in the
conservative scanner. We would then also need to put freeIndexForScan
behind the publication barrier, or make the write to freeIndexForScan
exactly that barrier.
However, there's a simpler way. We don't actually care if conservative
scanning observes a stale freeIndexForScan during the mark phase.
Newly-allocated memory is always marked at the point of allocation (the
allocate-black policy part of the GC's design). So it doesn't actually
matter that if the garbage collector scans that memory or not.
This change modifies the allocator to only update freeIndexForScan
outside the mark phase. This means freeIndexForScan is essentially
a snapshot of freeindex at the point the mark phase started. Because
there's no more race between conservative scanning and newly-allocated
objects, the complicated scenario above is no longer a possibility.
One thing we do have to be careful of is other callers of isFree.
Previously freeIndexForScan would always track freeindex, now it no
longer does. This change thus introduces isFreeOrNewlyAllocated which is
used by the conservative scanner, and uses freeIndexForScan. Meanwhile
isFree goes back to using freeindex like it used to. This change also
documents the requirement on isFree that the caller must have obtained
the pointer not 'out of thin air' but after the object was published.
isFree is not currently used anywhere particularly sensitive (heap dump
and checkmark mode, where the world is stopped in both cases) so using
freeindex is both conceptually simple and also safe.
Change-Id: If66b8c536b775971203fb4358c17d711c2944723
Reviewed-on: https://go-review.googlesource.com/c/go/+/672340
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
For modules that have already been indexed, we can skip ignored paths.
We already skip 'testdata' and '_' for this case so we can extend the
ignore directive for this case as well.
Updates: #42965
Change-Id: I076a242ba65c7b905b9dc65dcfb0a0247cbd68d6
Reviewed-on: https://go-review.googlesource.com/c/go/+/674076
Reviewed-by: Michael Matloob <matloob@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
Auto-Submit: Sam Thanawalla <samthanawalla@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Fast follow to golang.org/cl/636475 with a couple script tests that
build/runs a module that depends on a function inside a git repo using
sha256 hashes. (one with go get of a branch-name and the other
configuring go.mod directly)
Change-Id: Ief6c7efaf6d5c066dc54a3e4a63aad109f625abe
Reviewed-on: https://go-review.googlesource.com/c/go/+/672435
Reviewed-by: Michael Matloob <matloob@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
Auto-Submit: Sam Thanawalla <samthanawalla@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Sam Thanawalla <samthanawalla@google.com>
CL 518776 deleted the cmd/go/internal/modconv package and dropped the
ability to import dependency requirements from ~nine or so legacy
pre-module dependency configuration files. Part of the rationale from
Russ in 2023 for dropping that support was that "by now no one is
running into those configs anymore during 'go mod init'".
For two of those legacy file formats, Godeps.json and vendor.json, the
ability to import their listed dependencies was dropped in CL 518776,
but what remained for those two formats was the ability to guess the
resulting module name in the absence of a name being supplied to 'go mod
init'.
This could be explained by the fact that this smaller functionality for
guessing a module name was separate, did not rely on the deleted modconv
package, and instead only relied on simple JSON parsing.
The name guessing was helpful as part of the transition when module
support was initially released, but it was never perfect, including the
various third-party dependency managers did not all have the same naming
rules that were enforced by modules.
In short, it is very unlikely anyone is relying on this now, so we
delete it.
This CL was spawned from discussion in two related documentation CLs
(CL 662675 and CL 662695).
Updates #71537
Change-Id: I9e087aa296580239562a0ecee58913c5edc533ee
Reviewed-on: https://go-review.googlesource.com/c/go/+/664315
Reviewed-by: Michael Matloob <matloob@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Sam Thanawalla <samthanawalla@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Sam Thanawalla <samthanawalla@google.com>
Because that's what mallocgc did and some user code came to rely on it.
Fixes#73199
Change-Id: I45ca00d2ea448e6729ef9ac4cec3c1eb0ceccc89
Reviewed-on: https://go-review.googlesource.com/c/go/+/666116
Reviewed-by: t hepudds <thepudds1460@gmail.com>
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Keith Randall <khr@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
In regalloc, we allocate some values to registers before loop entry,
so that they don't need to be loaded (from spill locations) during
the loop.
But it is pointless if we've already regalloc'd the loop body.
Whatever restores we needed for the body are already generated.
It's not clear if this code is ever useful. No tests fail if I just
remove it. But at least this change is worthwhile. It doesn't help,
and it actively inserts more restores than we really need (mostly
because the desired register list is approximate - I have seen cases
where the loads implicated here end up being dead because the restores
hit the wrong registers and the edge shuffle pass knows it needs
the restores in different registers).
While we are here, might as well have layoutRegallocOrder return
the standard layout order instead of recomputing it.
Change-Id: Ia624d5121de59b6123492603695de50b272b277f
Reviewed-on: https://go-review.googlesource.com/c/go/+/672735
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: David Chase <drchase@google.com>
When appending, if the backing store doesn't escape and a
constant-sized backing store is big enough, use a constant-sized
stack-allocated backing store instead of allocating it from the heap.
cmd/go is <0.1% bigger.
As an example of how this helps, if you edit strings/strings.go:FieldsFunc
to replace
spans := make([]span, 0, 32)
with
var spans []span
then this CL removes the first 2 allocations that are part of the growth sequence:
│ base │ exp │
│ allocs/op │ allocs/op vs base │
FieldsFunc/ASCII/16-24 3.000 ± ∞ ¹ 2.000 ± ∞ ¹ -33.33% (p=0.008 n=5)
FieldsFunc/ASCII/256-24 7.000 ± ∞ ¹ 5.000 ± ∞ ¹ -28.57% (p=0.008 n=5)
FieldsFunc/ASCII/4096-24 11.000 ± ∞ ¹ 9.000 ± ∞ ¹ -18.18% (p=0.008 n=5)
FieldsFunc/ASCII/65536-24 18.00 ± ∞ ¹ 16.00 ± ∞ ¹ -11.11% (p=0.008 n=5)
FieldsFunc/ASCII/1048576-24 30.00 ± ∞ ¹ 28.00 ± ∞ ¹ -6.67% (p=0.008 n=5)
FieldsFunc/Mixed/16-24 2.000 ± ∞ ¹ 2.000 ± ∞ ¹ ~ (p=1.000 n=5)
FieldsFunc/Mixed/256-24 7.000 ± ∞ ¹ 5.000 ± ∞ ¹ -28.57% (p=0.008 n=5)
FieldsFunc/Mixed/4096-24 11.000 ± ∞ ¹ 9.000 ± ∞ ¹ -18.18% (p=0.008 n=5)
FieldsFunc/Mixed/65536-24 18.00 ± ∞ ¹ 16.00 ± ∞ ¹ -11.11% (p=0.008 n=5)
FieldsFunc/Mixed/1048576-24 30.00 ± ∞ ¹ 28.00 ± ∞ ¹ -6.67% (p=0.008 n=5)
(Of course, people have spotted and fixed a bunch of allocation sites
like this, but now we're ~automatically doing it everywhere going forward.)
No significant increases in frame sizes in cmd/go.
Change-Id: I301c4d9676667eacdae0058960321041d173751a
Reviewed-on: https://go-review.googlesource.com/c/go/+/664299
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Keith Randall <khr@golang.org>
-N+1 <= x % N <= N-1
This is useful for cases like:
func setBit(b []byte, i int) {
b[i/8] |= 1<<(i%8)
}
The shift does not need protection against larger-than-7 cases.
(It does still need protection against <0 cases.)
Change-Id: Idf83101386af538548bfeb6e2928cea855610ce2
Reviewed-on: https://go-review.googlesource.com/c/go/+/672995
Reviewed-by: Jorropo <jorropo.pgm@gmail.com>
Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Adding zero usually does not change the original value.
However, there is an exception with negative zero. (e.g. (-0) + (+0) = (+0))
This applies when x * y is negative and underflows.
Fixes#73757
Change-Id: Ib7b54bdacd1dcfe3d392802ea35cdb4e989f9371
GitHub-Last-Rev: 30d74883b2
GitHub-Pull-Request: golang/go#73759
Reviewed-on: https://go-review.googlesource.com/c/go/+/673856
Auto-Submit: Keith Randall <khr@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
This was just enabled in CL 643897. It seems to work fine on Linux, but
there are traceback issues on Darwin. We could disable just on Darwin,
but I'm not sure SIGSEGV inside of TSAN is something we care to support.
Fixes#73784.
Cq-Include-Trybots: luci.golang.try:gotip-darwin-arm64-race
Change-Id: I6a6a636cb15d7affaeb22c4c13d8f2a5c9bb31fd
Reviewed-on: https://go-review.googlesource.com/c/go/+/674276
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
Sync is used in the definition of primitives and documented by pkgbits.
It's not much help to also document it here.
Change-Id: I18bd0c7816f8249483550a1f0af7c76b9cfe09fb
Reviewed-on: https://go-review.googlesource.com/c/go/+/674156
Auto-Submit: Mark Freeman <mark@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Bypass: Mark Freeman <mark@golang.org>
ncpu is the total logical CPU count at startup. It is never updated. For
#73193, we will start using updated CPU counts for updated GOMAXPROCS,
making the ncpu name a bit ambiguous. Change to a less ambiguous name.
While we're at it, give the OS specific lookup functions a common name,
so it can be used outside of osinit later.
For #73193.
Change-Id: I6a6a636cf21cc60de36b211f3c374080849fc667
Reviewed-on: https://go-review.googlesource.com/c/go/+/672277
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
Moving to a smaller package allows its use in other internal/runtime
packages.
This isn't internal/strconvlite since it can't be used directly by
strconv.
For #73193.
Change-Id: I6a6a636c9c8b3f06b5fd6c07fe9dd5a7a37d1429
Reviewed-on: https://go-review.googlesource.com/c/go/+/672697
Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
The package section holds package stubs, which are a package
(path, name) pair and a series of declared imports.
Change-Id: If2a260c5e0a3522851be9808de46a3f128902002
Reviewed-on: https://go-review.googlesource.com/c/go/+/674175
Auto-Submit: Mark Freeman <mark@golang.org>
TryBot-Bypass: Mark Freeman <mark@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
khr@golang.org
Filippo Valsorda
Damien Neil
Michael Matloob
Junyang Shao
Mark Freeman
Mateusz Poliwczak
Michael Anthony Knyszek
Michael Pratt
Jake Bailey
MaineK00n
Julian Zhu
Shibi J M
Julian Zhu
thepudds
Vladislav Yarmak
Marc-Antoine Ruel
Sam Thanawalla
David Finkel
ICHINOSE Shogo