Michael Anthony Knyszek [Thu, 1 Feb 2024 05:32:03 +0000 (05:32 +0000)]
[release-branch.go1.22] runtime: traceAcquire and traceRelease across all P steals
Currently there are a few places where a P can get stolen where the
runtime doesn't traceAcquire and traceRelease across the steal itself.
What can happen then is the following scenario:
- Thread 1 enters a syscall and writes an event about it.
- Thread 2 steals Thread 1's P.
- Thread 1 exits the syscall and writes one or more events about it.
- Tracing ends (trace.gen is set to 0).
- Thread 2 checks to see if it should write an event for the P it just
stole, sees that tracing is disabled, and doesn't.
This results in broken traces, because there's a missing ProcSteal
event. The parser always waits for a ProcSteal to advance a
GoSyscallEndBlocked event, and in this case, it never comes.
Fixes #65181.
Change-Id: I437629499bb7669bf7fe2fc6fc4f64c53002916b
Reviewed-on: https://go-review.googlesource.com/c/go/+/560235 Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
(cherry picked from commit c9d88ea2aa628cae224335c49f256e13adfce337)
Reviewed-on: https://go-review.googlesource.com/c/go/+/559958
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Michael Anthony Knyszek [Thu, 1 Feb 2024 05:10:32 +0000 (05:10 +0000)]
[release-branch.go1.22] runtime: clear trace map without write barriers
Currently the trace map is cleared with an assignment, but this ends up
invoking write barriers. Theoretically, write barriers could try to
write a trace event and eventually try to acquire the same lock. The
static lock ranking expresses this constraint.
This change replaces the assignment with a call to memclrNoHeapPointer
to clear the map, removing the write barriers.
Note that technically this problem is purely theoretical. The way the
trace maps are used today is such that reset is only ever called when
the tracer is no longer writing events that could emit data into a map.
Furthermore, reset is never called from an event-writing context.
Therefore another way to resolve this is to simply not hold the trace
map lock over the reset operation. However, this makes the trace map
implementation less robust because it needs to be used in a very
specific way. Furthermore, the rest of the trace map code avoids write
barriers already since its internal structures are all notinheap, so
it's actually more consistent to just avoid write barriers in the reset
method.
Fixes #56554.
Change-Id: Icd86472e75e25161b2c10c1c8aaae2c2fed4f67f
Reviewed-on: https://go-review.googlesource.com/c/go/+/560216 Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
(cherry picked from commit 829f2ce3ba33a09a7975139a0a33d462bb3114db)
Reviewed-on: https://go-review.googlesource.com/c/go/+/559957
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Michael Anthony Knyszek [Thu, 21 Dec 2023 20:26:38 +0000 (20:26 +0000)]
[release-branch.go1.22] runtime: fix trace EvGoStop Gosched reason to match function
Currently the stop reason for runtime.Gosched is labeled
"runtime.GoSched" which doesn't actually match the function name. Fix
the label to match the function name.
This change doesn't regenerate the internal/trace/v2 tests, because
regenerating the tests breaks summarization tests in internal/trace that
rely on very specific details in the example traces that aren't
guaranteed. Also, go122-gc-trace.test isn't generated at all, as it
turns out. I'll fix this all up in a follow-up CL. For now, just replace
runtime.GoSched with runtime.Gosched in the traces so we don't have a
problem later if a test wants to look for that string.
This change does regenerate the cmd/trace/v2 test, but it turns out the
cmd/trace/v2 tests are way too strict about network unblock events, and
3 usually pop up instead of 1 or 2, which is what the test expects.
AFAICT this looks plausible to me, so just lift the restriction on
"up to 2" events entirely.
Change-Id: Id7350132be19119c743c259f2f5250903bf41a04
Reviewed-on: https://go-review.googlesource.com/c/go/+/552275
TryBot-Bypass: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
(cherry picked from commit 287f791845ac0311012814f4419d5e043c212d17)
Reviewed-on: https://go-review.googlesource.com/c/go/+/560555
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Michael Anthony Knyszek [Thu, 1 Feb 2024 04:47:22 +0000 (04:47 +0000)]
[release-branch.go1.22] runtime: model wakeableSleep.lock in the race detector
Currently the flight recorder tests are failing in race mode because the
race detector doesn't see s.lock, leading to false positives. This has
also appeared in the trace tests. Model the lock in the race detector.
Fixes #65207.
Fixes #65283.
Change-Id: I1e9a5c9606536f55fdfc46b5f8443e9c7213c23d
Reviewed-on: https://go-review.googlesource.com/c/go/+/560215
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Pratt <mpratt@google.com>
(cherry picked from commit 0b12e3d81cdba8a5676d6d61970d3dc5cb1462ac)
Reviewed-on: https://go-review.googlesource.com/c/go/+/559956
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Russ Cox [Wed, 24 Jan 2024 21:29:20 +0000 (16:29 -0500)]
[release-branch.go1.22] go/version: fix package to accept go1.21.0-bigcorp
The proposal discussion made clear that suffixes should be accepted,
so that people who use custom VERSION files can still pass runtime.Version()
to this code. But we forgot to do that in the CL. Do that.
Note that cmd/go also strips space- and tab-prefixed suffixes,
but go.dev/doc/toolchain only mentions dash, so this code only
strips dash.
Fixes #65061.
Change-Id: I6a427b78f964eb41c024890dae30223beaef13eb
Cq-Include-Trybots: luci.golang.try:go1.22-linux-amd64-longtest,go1.22-windows-amd64-longtest
Reviewed-on: https://go-review.googlesource.com/c/go/+/559796
TryBot-Bypass: Russ Cox <rsc@golang.org> Reviewed-by: Mauri de Souza Meneguzzo <mauri870@gmail.com> Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/559802
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This reverts CL 514235. Also reverts CL 518056 which is a followup
fix.
Reason for revert: Proposal #50102 defined an interface that is
too specific to UNIX-y systems and also didn't make much sense.
The proposal is un-accepted, and we'll revisit in Go 1.23.
Fixes #65245.
Updates #50102.
Change-Id: I41ba0ee286c1d893e6564a337e5d76418d19435d
Reviewed-on: https://go-review.googlesource.com/c/go/+/558295 Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
(cherry picked from commit 5000b5168037d26a796da46a07088e834c3af1a0)
Reviewed-on: https://go-review.googlesource.com/c/go/+/558296
Michael Anthony Knyszek [Thu, 11 Jan 2024 21:55:04 +0000 (21:55 +0000)]
[release-branch.go1.22] runtime: make a much better effort to emit CPU profile in a trace
Currently the new execution tracer's handling of CPU profile samples is
very best-effort. The same CPU profile buffer is used across
generations, leading to a high probability that CPU samples will bleed
across generations. Also, because the CPU profile buffer (not the trace
buffer the samples get written into) isn't guaranteed to be flushed when
we close out a generation, nor when tracing stops. This has led to test
failures, but can more generally just lead to lost samples.
In general, lost samples are considered OK. The CPU profile buffer is
only read from every 100 ms, so if it fills up too much before then, old
samples will get overwritten. The tests already account for this, and in
that sense the CPU profile samples are already best-effort. But with
actual CPU profiles, this is really the only condition under which
samples are dropped.
This CL aims to align CPU profiles better with traces by eliminating
all best-effort parts of the implementation aside from the possibility
of dropped samples from a full buffer.
To achieve this, this CL adds a second CPU profile buffer and has the
SIGPROF handler pick which CPU profile buffer to use based on the
generation, much like every other part of the tracer. The SIGPROF
handler then reads the trace generation, but not before ensuring it
can't change: it grabs its own thread's trace seqlock. It's possible
that a SIGPROF signal lands while this seqlock is already held by the
thread. Luckily this is detectable and the SIGPROF handler can simply
elide the locking if this happens (the tracer will already wait until
all threads exit their seqlock critical section).
Now that there are two CPU profile buffers written to, the read side
needs to change. Instead of calling traceAcquire/traceRelease for every
single CPU sample event, the trace CPU profile reader goroutine holds
this conceptual lock over the entirety of flushing a buffer. This means
it can pick the CPU profile buffer for the current generation to flush.
With all this machinery in place, we're now at a point where all CPU
profile samples get divided into either the previous generation or the
current generation. This is good, since it means that we're able to
emit profile samples into the correct generation, avoiding surprises in
the final trace. All that's missing is to flush the CPU profile buffer
from the previous generation, once the runtime has moved on from that
generation. That is, when the generation counter updates, there may yet
be CPU profile samples sitting in the last generation's buffer. So,
traceCPUFlush now first flushes the CPU profile buffer, followed by any
trace buffers containing CPU profile samples.
The end result of all this is that no sample gets left behind unless it
gets overwritten in the CPU profile buffer in the first place. CPU
profile samples in the trace will now also get attributed to the right
generation, since the SIGPROF handler now participates in the tracer's
synchronization across trace generations.
Fixes #55317.
Change-Id: I47719fad164c544eef0bb12f99c8f3c15358e344
Reviewed-on: https://go-review.googlesource.com/c/go/+/555495
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>
(cherry picked from commit f5e475edafd4186c51aadf2e7fdf164eb365379f)
Reviewed-on: https://go-review.googlesource.com/c/go/+/557838 Reviewed-by: Cherry Mui <cherryyz@google.com>
Michael Anthony Knyszek [Mon, 22 Jan 2024 16:34:41 +0000 (16:34 +0000)]
[release-branch.go1.22] runtime: use the correct M ID for syscalling goroutines in traces
Earlier in the development of the new tracer, m.id was used as a the
canonical ID for threads. Later, we switched to m.procid because it
matches the underlying OS resource. However, in that switch, we missed a
spot.
The tracer catches and emits statuses for goroutines that have remained
in either waiting or syscall across a whole generation, and emits a
thread ID for the latter set. The ID being used here, however, was m.id
instead of m.procid, like the rest of the tracer.
This CL also adds a regression test. In order to make the regression
test actually catch the failure, we also have to make the parser a
little less lenient about GoStatus events with GoSyscall: if this isn't
the first generation, then we should've seen the goroutine bound to an
M already when its status is getting emitted for its context. If we emit
the wrong ID, then we'll catch the issue when we emit the right ID when
the goroutine exits the syscall.
Fixes #65196.
Change-Id: I78b64fbea65308de5e1291c478a082a732a8bf9f
Reviewed-on: https://go-review.googlesource.com/c/go/+/557456 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>
(cherry picked from commit c46966653f6144e20f8b9bccb96e7a7f1d32aeb9)
Reviewed-on: https://go-review.googlesource.com/c/go/+/557436
The motivation is the same as in the commit message of CL 511317.
For #61422.
Change-Id: I0e86cf35ec3501a931d6d7fffb0c83f3e57106e1
Reviewed-on: https://go-review.googlesource.com/c/go/+/557515
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Bryan C. Mills [Wed, 17 Jan 2024 20:00:40 +0000 (15:00 -0500)]
cmd/go/internal/modfetch: set protocol.version=2 for shallow git fetches
This works around an apparent bug in the Git HTTP backend, introduced
in Git 2.21, that causes responses for the version 1 protocol to
provide incomplete tags.
For Git commands older than 2.18, this configuration flag is ignored.
(Note that Git 2.29 and above already use protocol version 2 by
default.)
Damien Neil [Wed, 17 Jan 2024 19:59:10 +0000 (11:59 -0800)]
doc/go1.22: document net/netip changes
For #61422
Change-Id: Ide818366b035eada4ba04b70b4741fb1891585d6
Reviewed-on: https://go-review.googlesource.com/c/go/+/556396
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Carlos Amedee <carlos@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Auto-Submit: Damien Neil <dneil@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Cuong Manh Le [Mon, 23 Oct 2023 03:56:57 +0000 (10:56 +0700)]
runtime: document GODEBUG panicnil values
Updates #25448
Change-Id: Ia1b7a376f5175f67e14ad4bd065d6e8ad5250d38
Reviewed-on: https://go-review.googlesource.com/c/go/+/536955 Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Carlos Amedee <carlos@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Keith Randall <khr@golang.org>
Robert Griesemer [Wed, 17 Jan 2024 00:59:31 +0000 (16:59 -0800)]
cmd/compile: call types2.Unalias to be ready for GODEBUG=gotypesalias=1
types2.Unalias is not needed if we know we have a core or underlying
type. Also, types of declared functions (signatures) cannot be aliases
(this includes tuples).
Fixes #65125.
Change-Id: I1faa26b66f6c646719e830dd661136fae86f3775
Reviewed-on: https://go-review.googlesource.com/c/go/+/556036
Run-TryBot: Robert Griesemer <gri@google.com> Reviewed-by: Mauri de Souza Meneguzzo <mauri870@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Robert Griesemer <gri@google.com> Reviewed-by: Robert Griesemer <gri@google.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Dominik Honnef [Wed, 27 Dec 2023 21:01:19 +0000 (22:01 +0100)]
internal/trace/v2: avoid several panics for malformed traces
This addresses some panics (out of bounds slice accesses and nil pointer
dereferences) when parsing malformed data. These were found via light
fuzzing, not by any rigorous means, and more potential panics probably
exist.
Fixes #64878.
Fixes #64879.
Change-Id: I4085788ba7dc91fec62e4abd88f50777577db42f
Reviewed-on: https://go-review.googlesource.com/c/go/+/552995
Auto-Submit: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Most libraries don't consider N secret, but it's arguably useful for
privacy applications. However, E should generally be fixed, and there is
a lot of performance to be gained by using variable-time exponentiation.
Filippo Valsorda [Tue, 26 Dec 2023 22:21:34 +0000 (23:21 +0100)]
crypto/rsa: use E = 65537 in benchmarks
Now, this is embarrassing. For the whole Go 1.20 and Go 1.21 cycles, we
based RSA public key operation (verification and decryption) benchmarks
on the keys in rsa_test.go, which had E = 3. Most keys in use, including
all those generated by GenerateKey, have E = 65537. This significantly
skewed even relative benchmarks, because the new constant-time
algorithms would incur a larger slowdown for larger exponents.
I noticed this only because I got a production profile for an
application that does a lot of RSA verifications, saw ExpShort show up,
made ExpShort faster, and the crypto/rsa profiles didn't move.
We were measuring the wrong thing, and the slowdown was worse than we
thought. My apologies.
(If E had not been parametrized, it would have avoided issues like this
one, too. Grumble. https://words.filippo.io/parameters/#fn9)
Keith Randall [Thu, 11 Jan 2024 20:52:16 +0000 (12:52 -0800)]
runtime: ensure we free unrolled GC bitmaps
CL 555355 has a bug in it - the GC program flag was also used to decide
when to free the unrolled bitmap. After that CL, we just don't free any
unrolled bitmaps, leading to a memory leak.
Use a separate flag to track types that need to be freed when their
corresponding object is freed.
Change-Id: I841b65492561f5b5e1853875fbd8e8a872205a84
Reviewed-on: https://go-review.googlesource.com/c/go/+/555416
Auto-Submit: Keith Randall <khr@google.com> 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>
Robert Griesemer [Wed, 10 Jan 2024 23:49:33 +0000 (15:49 -0800)]
go/types, types2: don't lose position info of interface embeddings
Accurate position information for embedded types in interfaces is
crucial to identify the corresponding source file, and with that
the Go language version associated with that file. (The position
information is also important for proper error messages.)
Before this CL, the position information for embedded types was
discarded after type set computation, in the assumption that it
was not needed anymore. However, substitutions that update the
interface may lead to repeated type set computations which then
won't have the correct position information.
This CL does preserve the position information for embedded
types until the end of type checking (cleanup phase), and also
copy the position information during a substitution of the
interface.
The respective bug (#64759) doesn't seem to appear in 1.22 (most
likely because it's hidden by some of the changes made with respect
to the file version logic), but the existing code is still wrong.
The backport of this code to 1.21 and 1.20 fixes the issue in those
releases.
For #64759.
Change-Id: I80f4004c9d79cb02eac6739c324c477706615102
Reviewed-on: https://go-review.googlesource.com/c/go/+/555296
Run-TryBot: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Robert Findley <rfindley@google.com> Reviewed-by: Robert Griesemer <gri@google.com>
Bryan C. Mills [Mon, 8 Jan 2024 22:20:14 +0000 (17:20 -0500)]
os: relax tests and add examples for UserCacheDir and UserConfigDir
Per https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html:
“If, when attempting to write a file, the destination directory is
non-existent an attempt should be made to create it with permission
0700. […] The application should be prepared to handle the case where
the file could not be written […]. In such case it may choose to
present an error message to the user.”
In certain CI environments, these directories have well-defined
locations but do not exist and cannot be created. In that case,
we now choose to log and return from the test without failing it.
To prevent the functions from falling back to being entirely untested,
we still fail the test (and “present an error message to the user”) if
either function returns an empty string without an error, or returns a
path that refers to a non-directory or results in an error other than
ErrNotExist.
In addition, since the tests themselves no longer create subdirectories,
we add examples illustrating the suggested pattern of usage.
Than McIntosh [Tue, 9 Jan 2024 15:06:15 +0000 (15:06 +0000)]
cmd/compile: use hashed symbol name for go.shape types if too long
Shape-based stenciling in the Go compiler's generic instantiation
phase looks up shape types using the underlying type of a given target
type. This has a beneficial effect in most cases (e.g. we can use the
same shape type for two different named types whose underlying type is
"int"), but causes some problems when the underlying type is a very
large structure. The link string for the underlying type of a large
imported struct can be extremely long, since the link string
essentially enumerates the full package path for every field type;
this can produce a "go.shape.struct { ... " symbol name that is
absurdly long.
This patch switches the compiler to use a hash of the underlying type
link string instead of the string itself, which should continue to
provide commoning but keep symbol name lengths reasonable for shape
types based on large imported structs.
Fixes #65030.
Change-Id: I87d602626c43172beb99c186b8ef72327b8227a2
Reviewed-on: https://go-review.googlesource.com/c/go/+/554975
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Than McIntosh <thanm@google.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Michael Pratt [Tue, 9 Jan 2024 18:13:45 +0000 (13:13 -0500)]
runtime: replace rwmutexR/W with per-rwmutex lock rank
CL 549536 intended to decouple the internal implementation of rwmutex
from the semantic meaning of an rwmutex read/write lock in the static
lock ranking.
Unfortunately, it was not thought through well enough. The internals
were represented with the rwmutexR and rwmutexW lock ranks. The idea was
that the internal lock ranks need not model the higher-level ordering,
since those have separate rankings. That is incorrect; rwmutexW is held
for the duration of a write lock, so it must be ranked before any lock
taken while any write lock is held, which is precisely what we were
trying to avoid.
This is visible in violations like:
0 : execW 11 0x0
1 : rwmutexW 51 0x111d9c8
2 : fin 30 0x111d3a0
fatal error: lock ordering problem
execW < fin is modeled, but rwmutexW < fin is missing.
Fix this by eliminating the rwmutexR/W lock ranks shared across
different types of rwmutex. Instead require users to define an
additional "internal" lock rank to represent the implementation details
of rwmutex.rLock. We can avoid an additional "internal" lock rank for
rwmutex.wLock because the existing writeRank has the same semantics for
semantic and internal locking. i.e., writeRank is held for the duration
of a write lock, which is exactly how rwmutex.wLock is used, so we can
use writeRank directly on wLock.
For #64722.
Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-staticlockranking
Change-Id: Ia572de188a46ba8fe054ae28537648beaa16b12c
Reviewed-on: https://go-review.googlesource.com/c/go/+/555055
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Roger Peppe [Fri, 5 Jan 2024 12:47:56 +0000 (12:47 +0000)]
io/fs,path/filepath: fix typo in SkipAll/SkipDir doc
Also make the reference into a doc link.
Change-Id: Ib112307a65b65c8f963abf60aa92cb1942de940c
Reviewed-on: https://go-review.googlesource.com/c/go/+/554295 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Dmitri Shuralyov [Mon, 8 Jan 2024 20:05:45 +0000 (15:05 -0500)]
[release-branch.go1.22] all: merge master (8eaa793) into release-branch.go1.22
Merge List:
+ 2024-01-08 8eaa7935db net: clarify maxListenerBacklog windows implementation
+ 2024-01-08 759849187f log/slog: clarify SetDefault behavior
+ 2024-01-08 10a66d6816 sync: use map[any]any instead of map[interface{}]interface{} in the godoc
+ 2024-01-08 881869dde0 cmd/compile: handle defined iter func type correctly
+ 2024-01-05 1ae729e6d3 doc: s/adjustements/adjustments
+ 2024-01-05 8088b6db23 slices: explicitly discard results of some functions
+ 2024-01-05 c0693f648a cmd/go: run cover tool before swig
+ 2024-01-04 8db131082d github: switch seen/expected order in issue forms
+ 2024-01-04 ead47b0ab3 net/http: respond with 400 Bad Request for empty hex number of chunk length
+ 2024-01-04 1e07c144c3 net/http/cgi: in TestCopyError, check for a Handler.ServeHTTP goroutine instead of a running PID
+ 2024-01-04 15dcdeb5aa cmd/api: fix panic on exported basic type aliases
+ 2024-01-03 6db1102605 pagetrace: fix build when experiment is on
+ 2024-01-03 7d1b82dbf1 net/http: make Request.Clone create fresh copies for matches and otherValues
+ 2024-01-03 aa0a6ad1db doc/go1.22: add links to go/types symbols
+ 2024-01-03 c95fe91d07 runtime: correct scavengeIndex.sysGrow min index handling
+ 2023-12-30 b25f5558c6 all: replace outdated links
+ 2023-12-30 3233542e85 reflect: fix typo in type.go
+ 2023-12-27 988b718f41 doc: fix typo in example in spec
+ 2023-12-27 26ba75fe59 doc: document new iteration variable semantics in spec
+ 2023-12-27 1dddd83c49 doc: document version at which new language features were introduced in spec
+ 2023-12-26 36a2463e7c lib/time: update to 2023d/2023d
+ 2023-12-21 2184a39477 runtime/metrics: godoc link fixes
+ 2023-12-21 9c01ecce48 doc: fix misspelling in go1.22 release notes
+ 2023-12-21 0b56804084 runtime: use racereleasemerge for godebugInc
+ 2023-12-21 f6509cf5cd cmd/compile: handle constant-folding of an out-of-range jump table index
+ 2023-12-20 adec22b9f7 doc/go1.22: document changes to vet loopclosure analyzer
+ 2023-12-20 a2a2c5b947 doc: update unsafe.Pointer rule in spec
+ 2023-12-19 35222eeb78 doc: add html/template release note
Mauri de Souza Meneguzzo [Sat, 2 Dec 2023 15:11:36 +0000 (15:11 +0000)]
net: clarify maxListenerBacklog windows implementation
The previous TODO comments were somewhat ambiguous. This aims to
provide a clearer understanding of the behavior on Windows.
Windows does not offer a way to peek at the current backlog length, this
is explicitly stated in the winapi for `listen`.
When set to `syscall.SOMAXCONN`, the OS dynamically adjusts the
backlog to a maximum reasonable value. It goes as far as the dotnet
runtime itself introducing a new version of `listen` that does not accept a
backlog parameter to help eliminate the confusion when comparing the
behavior with UNIXes.
The docs also mention that `SOMAXCONN_HINT(N)` can be used, and that
it clips the final computed value between (200, 65535), which suggests
windows might use a `uint16` to back this number. Either way it does not
matter since windows will adjust this value anyway, so I removed the
wrapping TODO as well.
See https://learn.microsoft.com/en-us/windows/win32/api/winsock2/nf-winsock2-listen
Change-Id: I7b2e7cb547467c4bfc572ef0477a58de8c772521
GitHub-Last-Rev: 34e74abffe8792c8709c73db4d7a5fa05f64b1d0
GitHub-Pull-Request: golang/go#63549
Reviewed-on: https://go-review.googlesource.com/c/go/+/535475 Reviewed-by: Damien Neil <dneil@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Than McIntosh [Thu, 21 Dec 2023 13:21:59 +0000 (08:21 -0500)]
cmd/go: run cover tool before swig
When building a package, run the cover tool on the collected go/cgo
source files before invoking swig (if swig files are present), as
opposed to running swig and then cover. Running swig adds new Go files
to the "cgo" list, and we want to avoid running those newly generated
files through the cover tool.
Fixes #64661.
Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-longtest
Change-Id: I32b6dad5c39fcf5e656c40fb3b44220c69320889
Reviewed-on: https://go-review.googlesource.com/c/go/+/552095
Auto-Submit: Than McIntosh <thanm@google.com> Reviewed-by: Bryan Mills <bcmills@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Bryan C. Mills [Thu, 4 Jan 2024 16:41:54 +0000 (11:41 -0500)]
net/http/cgi: in TestCopyError, check for a Handler.ServeHTTP goroutine instead of a running PID
Previously, the test could fail spuriously if the CGI process's PID
happened to be reused in between checks. That sort of reuse is highly
unlikely on platforms that cycle through the PID space sequentially
(such as Linux), but plausible on platforms that use randomized PIDs
(such as OpenBSD).
Also unskip the test on Windows, since it no longer relies on being
able to send signal 0 to an arbitrary PID.
Also change the expected failure mode of the test to a timeout instead
of a call to t.Fatalf, so that on failure we get a useful goroutine
dump for debugging instead of a non-actionable failure message.
Fixes #57369 (maybe).
Change-Id: Ib7e3fff556450b48cb5e6ea120fdf4d53547479b
Reviewed-on: https://go-review.googlesource.com/c/go/+/554075
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Bryan Mills <bcmills@google.com> Reviewed-by: Damien Neil <dneil@google.com>
Devon H. O'Dell [Thu, 4 Jan 2024 16:49:17 +0000 (11:49 -0500)]
cmd/api: fix panic on exported basic type aliases
The order of emitting named type and type aliases in the `Walker`'s
`emitType` function is inverted. When the type alias references a basic
type, this causes a panic as the type assertion on `*types.Named` fails.
This change reorders the logic such that type aliases are emitted prior
to this type assertion.
Jes Cok [Wed, 3 Jan 2024 04:18:15 +0000 (04:18 +0000)]
net/http: make Request.Clone create fresh copies for matches and otherValues
This change fixes Request.Clone to correctly work with SetPathValue
by creating fresh copies for matches and otherValues so that
SetPathValue for cloned requests doesn't pollute the original request.
While here, also added a doc for Request.SetPathValue.
Fixes #64911
Change-Id: I2831b38e135935dfaea2b939bb9db554c75b65ef
GitHub-Last-Rev: 1981db16475a49fe8d4b874a6bceec64d28a1332
GitHub-Pull-Request: golang/go#64913
Reviewed-on: https://go-review.googlesource.com/c/go/+/553375 Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
Run-TryBot: Jes Cok <xigua67damn@gmail.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Jonathan Amsterdam <jba@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Joel Sing [Fri, 29 Dec 2023 11:56:58 +0000 (22:56 +1100)]
runtime: correct scavengeIndex.sysGrow min index handling
The backing store for the scavengeIndex chunks slice is allocated on demand
as page allocation occurs. When pageAlloc.grow is called, a range is
allocated from a reserved region, before scavengeIndex.grow is called
to ensure that the chunks needed to manage this new range have a valid
backing store. The valid region for chunks is recorded as the index min
and index max. Any changes need to take the existing valid range into
consideration and ensure that a contiguous valid range is maintained.
However, a bug in the min index handling can currently lead to an existing
part of the chunk slice backing store being zeroed via remapping. Initially,
there is no backing store allocated and both min and max are zero. As soon
as an allocation occurs max will be non-zero, however it is still valid for
min to be zero depending on the base addresses of the page allocations. A
sequence like the following will trigger the bug:
1. A page allocation occurs requiring chunks [0, 512) (after rounding) - a
sysMap occurs for the backing store, min is set to 0 and max is set
to 512.
2. A page allocation occurs requiring chunks [512, 1024) - another sysMap
occurs for this part of the backing store, max is set to 1024, however
min is incorrectly set to 512, since haveMin == 0 (validly).
3. Another page allocation occurs requiring chunks [0, 512) - since min is
currently 512 a sysMap occurs for the already mapped and inuse part
of the backing store from [0, 512), zeroing the chunk data.
Correct this by only updating min when either haveMax == 0 (the
uninitialised case) or when needMin < haveMin (the case where the new
backing store range is actually below the current allocation). Remove
the unnecessary haveMax == 0 check for updating max, as the existing
needMax > haveMax case already covers this.
Fixes #63385
Change-Id: I9deed74c4ffa187c98286fe7110e5d735e81f35f
Reviewed-on: https://go-review.googlesource.com/c/go/+/553135 Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Joel Sing <joel@sing.id.au>
Robert Griesemer [Wed, 27 Dec 2023 05:17:04 +0000 (21:17 -0800)]
doc: fix typo in example in spec
Follow-up on CL 551095.
For #56010.
Change-Id: I8913d6ca96c419c81683e88c6286b05ae1323416
Reviewed-on: https://go-review.googlesource.com/c/go/+/552915
TryBot-Bypass: Robert Griesemer <gri@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Robert Griesemer [Tue, 19 Dec 2023 01:09:18 +0000 (17:09 -0800)]
doc: document new iteration variable semantics in spec
For #56010.
Change-Id: Icca987a03d80587dd0e901f596ff7788584893ed
Reviewed-on: https://go-review.googlesource.com/c/go/+/551095 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Bypass: Robert Griesemer <gri@google.com> Reviewed-by: Rob Pike <r@golang.org> Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Cherry Mui [Wed, 20 Dec 2023 19:33:46 +0000 (14:33 -0500)]
runtime: use racereleasemerge for godebugInc
CL 549796 adds race annotations to godebugInc. It uses racerelease
to model a CompareAndSwap. However, a CompareAndSwap is
essentially a load and a store. Modeling it as just racerelease
makes it not synchronized with other racerelease, i.e. other CAS.
For the following execution
thread A B
load, got nil
load, got nil
set *inc
set *inc
racerelease
CAS success
racerelease
CAS fail
load
raceacquire
use *inc (from A)
On thread B, the raceacquire synchronizes with the previous
racerelease, which is not synchronized with racerelease on thread
A, so it doesn't know that the use of *inc on thread B is after
the set on thread A, and will report a race.
Change it to use racereleasemerge, which synchronizes with
previous racerelease and racereleasemerge. So in the case above it
knows thread B's CAS is after thread A's.
Also remove stale comment that was more relevant when the code
used atomic store, where CL 549796 changed to CAS.
Updates #64649.
Change-Id: I17671090a19c0699fcb4e6481e2abd98ef2e5542
Reviewed-on: https://go-review.googlesource.com/c/go/+/551856 Reviewed-by: Mauri de Souza Meneguzzo <mauri870@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: David Chase <drchase@google.com>
Tim King [Tue, 19 Dec 2023 18:08:00 +0000 (10:08 -0800)]
doc/go1.22: document changes to vet loopclosure analyzer
cmd/vet no longer reports loopclosure bugs within files built at
GoVersion>=1.22.
For #61422.
Change-Id: I6f29373bb236822ece4e7ae35914859538b8d57b
Reviewed-on: https://go-review.googlesource.com/c/go/+/551376
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Tim King <taking@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Robert Griesemer [Tue, 19 Dec 2023 21:32:07 +0000 (13:32 -0800)]
doc: update unsafe.Pointer rule in spec
The valid conversions consider the core types of operands, not just
their underlying type.
This also explains the valid arguments for unsafe.Slice which are
explained in terms of unsafe.Pointer conversions.
unsafe.SliceData simply refers to "slice argument" and we use
similar terminology elsewhere in the spec to denote values that
have a core type of slice (or any other type for that matter).
Leaving alone for now.
Fixes #64452.
Change-Id: I0eed3abbc0606f22358835e5d434f026fe0909c8
Reviewed-on: https://go-review.googlesource.com/c/go/+/551379
Auto-Submit: Robert Griesemer <gri@google.com> Reviewed-by: Robert Griesemer <gri@google.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Bypass: Robert Griesemer <gri@google.com>
Than McIntosh [Tue, 19 Dec 2023 15:57:35 +0000 (10:57 -0500)]
[release-branch.go1.22] all: merge master (0324250) into release-branch.go1.22
Merge List:
+ 2023-12-19 03242506de doc: comment out remaining TODOs in Go 1.22 relnotes (for now)
+ 2023-12-19 9dd1cde9ac doc/go1.22,cmd/go: document that 'go mod init' no longer imports from other vendoring tools
+ 2023-12-19 22284c34f2 doc/go1.22: document removal of 'go get' support in GOPATH mode
+ 2023-12-19 339177aa31 doc: typo fix for net/http.ServeMux
+ 2023-12-19 52dbffeac8 cmd/go/internal/toolchain: revert "make a best effort to parse 'go run' and 'go install' flags"
Than McIntosh [Tue, 19 Dec 2023 15:32:52 +0000 (10:32 -0500)]
doc: comment out remaining TODOs in Go 1.22 relnotes (for now)
This patch comments out the remaining "TODO" items in the Go 1.22
release notes temporarily, so as to have RC1 go out with the notes
in a clean (TODO-less) state.
Change-Id: I88f5fef75860fc78b8077dce704ae71c6194a6a9
Reviewed-on: https://go-review.googlesource.com/c/go/+/551257 Reviewed-by: Carlos Amedee <carlos@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Than McIntosh <thanm@google.com>
Russ Cox [Tue, 19 Dec 2023 14:21:45 +0000 (09:21 -0500)]
cmd/go/internal/toolchain: revert "make a best effort to parse 'go run' and 'go install' flags"
This caused other problems, and for the purposes of the Go 1.22
release, we can just roll back to the Go 1.21 behavior and then
decide in January what the correct path forward is.
Robert Griesemer [Tue, 19 Dec 2023 02:13:30 +0000 (18:13 -0800)]
cmd/compile: remove interfacecycles debug flag
Per the discussion on the issue, since no problems related to this
appeared since Go 1.20, remove the ability to disable the check for
anonymous interface cycles permanently.
Adjust various tests accordingly.
For #56103.
Change-Id: Ica2b28752dca08934bbbc163a9b062ae1eb2a834
Reviewed-on: https://go-review.googlesource.com/c/go/+/550896
Run-TryBot: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com> Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Than McIntosh [Mon, 18 Dec 2023 21:11:13 +0000 (21:11 +0000)]
Revert "cmd/cgo/internal/testsanitizers: fix msan test failing with clang >= 16"
This reverts commit https://go.dev/cl/c/go/+/549297
Reason for revert: breaks clang builder
Change-Id: I2321dec9bc1fc20dfafa8a984303b0b5710f8aac
Reviewed-on: https://go-review.googlesource.com/c/go/+/550779
Auto-Submit: Than McIntosh <thanm@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Mauri de Souza Meneguzzo <mauri870@gmail.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Tim King [Mon, 18 Dec 2023 20:01:43 +0000 (12:01 -0800)]
cmd: go get golang.org/x/tools@83bceaf2 and revendor
go get golang.org/x/tools@83bceaf2 # CL 550395
go mod tidy
go mod vendor
Fixes #64786
Change-Id: Ia9879975eb3e8e4130ded7b2d8ba1277b5811eec
Reviewed-on: https://go-review.googlesource.com/c/go/+/550895
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Tim King <taking@google.com> Reviewed-by: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Irakli Safareli [Mon, 18 Dec 2023 08:53:03 +0000 (08:53 +0000)]
builtin: mention PanicNilError in comments of recover
As of CL 461956 `recover` will not return `nil` if `panic` is called with `nil`. I have updated comments of `recover` to account for this change.
Change-Id: Ibd0b27fe9b89fb29349b62ad34e762239a1d165b
GitHub-Last-Rev: c773abb75c8cd8e08c3470f064a3205573156fea
GitHub-Pull-Request: golang/go#64393
Reviewed-on: https://go-review.googlesource.com/c/go/+/544975 Reviewed-by: Mauri de Souza Meneguzzo <mauri870@gmail.com> Reviewed-by: Than McIntosh <thanm@google.com> 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>
Bryan C. Mills [Tue, 12 Dec 2023 21:26:45 +0000 (16:26 -0500)]
internal/syscall/windows: fix the signature of SetFileInformationByHandle
Also fix its call site in internal/poll to pass the length of the
actual buffer instead of an unrelated variable, and update the
definition of FILE_BASIC_INFO to match the documented field types
and add padding that is empirically needed on the 386 architecture.
Passing a pointer to a Go-allocated buffer as type uintptr violates
the unsafe.Pointer conversion rules, which allow such a conversion
only in the call expression itself for a call to syscall.Syscall or
equivalent. That can allow the buffer to be corrupted arbitrarily if
the Go runtime happens to garbage-collect it while the call to
SetFileInformationByHandle is in progress.
The Microsoft documentation for SetFileInformationByHandle specifies
its third argument type as LPVOID, which corresponds to Go's
unsafe.Pointer, not uintptr.
Fixes #58933 (maybe).
Change-Id: If577b57adea9922f5fcca55e46030c703d8f035c
Cq-Include-Trybots: luci.golang.try:gotip-windows-amd64-longtest
Reviewed-on: https://go-review.googlesource.com/c/go/+/549256
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Than McIntosh <thanm@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com> Reviewed-by: Quim Muntal <quimmuntal@gmail.com> Reviewed-by: Alex Brainman <alex.brainman@gmail.com>