Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update to Go 1.23.5 #112

Merged
merged 19 commits into from
Jan 28, 2025
Merged

Update to Go 1.23.5 #112

merged 19 commits into from
Jan 28, 2025

Conversation

mpminardi
Copy link
Member

No description provided.

dr2chase and others added 19 commits November 19, 2024 17:52
… f contain closure c?"

The old test relied on naming conventions.  The new test
uses an explicit parent pointer chain initialized when the
closures are created (in the same place that the names
used in the older fragile test were assigned).

Fixes golang#70198.

Change-Id: Ie834103c7096e4505faaff3bed1fc6e918a21211
Reviewed-on: https://go-review.googlesource.com/c/go/+/622656
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@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-on: https://go-review.googlesource.com/c/go/+/625535
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
This stops the test from failing with a known failure mode, and
creates time to look into what the next steps should be, if any.

For golang#69840
Fixes golang#70239

Change-Id: I060903d256ed65c5dfcd70ae76eb361cab63186f
Reviewed-on: https://go-review.googlesource.com/c/go/+/625197
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Eric Grosse <grosse@gmail.com>
(cherry picked from commit bea9b91)
Reviewed-on: https://go-review.googlesource.com/c/go/+/627575
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Bypass: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
syscall.SyscallN is implemented by runtime.syscall_syscalln, which makes
sure that the variadic argument doesn't escape.

There is no need to worry about the lifetime of the elements of the
variadic argument, as the compiler will keep them live until the
function returns.

For golang#70197
Fixes golang#70202

Change-Id: I12991f0be12062eea68f2b103fa0a794c1b527eb
Reviewed-on: https://go-review.googlesource.com/c/go/+/625297
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
(cherry picked from commit 7fff741)
Reviewed-on: https://go-review.googlesource.com/c/go/+/630196
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Quim Muntal <quimmuntal@gmail.com>
…ing mark termination

Currently it's possible for weak->strong conversions to create more GC
work during mark termination. When a weak->strong conversion happens
during the mark phase, we need to mark the newly-strong pointer, since
it may now be the only pointer to that object. In other words, the
object could be white.

But queueing new white objects creates GC work, and if this happens
during mark termination, we could end up violating mark termination
invariants. In the parlance of the mark termination algorithm, the
weak->strong conversion is a non-monotonic source of GC work, unlike the
write barriers (which will eventually only see black objects).

This change fixes the problem by forcing weak->strong conversions to
block during mark termination. We can do this efficiently by setting a
global flag before the ragged barrier that is checked at each
weak->strong conversion. If the flag is set, then the conversions block.
The ragged barrier ensures that all Ps have observed the flag and that
any weak->strong conversions which completed before the ragged barrier
have their newly-minted strong pointers visible in GC work queues if
necessary. We later unset the flag and wake all the blocked goroutines
during the mark termination STW.

There are a few subtleties that we need to account for. For one, it's
possible that a goroutine which blocked in a weak->strong conversion
wakes up only to find it's mark termination time again, so we need to
recheck the global flag on wake. We should also stay non-preemptible
while performing the check, so that if the check *does* appear as true,
it cannot switch back to false while we're actively trying to block. If
it switches to false while we try to block, then we'll be stuck in the
queue until the following GC.

All-in-all, this CL is more complicated than I would have liked, but
it's the only idea so far that is clearly correct to me at a high level.

This change adds a test which is somewhat invasive as it manipulates
mark termination, but hopefully that infrastructure will be useful for
debugging, fixing, and regression testing mark termination whenever we
do fix it.

For golang#69803.
Fixes golang#70323.

Change-Id: Ie314e6fd357c9e2a07a9be21f217f75f7aba8c4a
Reviewed-on: https://go-review.googlesource.com/c/go/+/623615
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
(cherry picked from commit 80d306d)
Reviewed-on: https://go-review.googlesource.com/c/go/+/627615
TryBot-Bypass: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
…getOrAddWeakHandle

getOrAddWeakHandle is very careful about keeping its input alive across
the operation, but not very careful about keeping the heap-allocated
handle it creates alive. In fact, there's a window in this function
where it is *only* visible via the special. Specifically, the window of
time between when the handle is stored in the special and when the
special actually becomes visible to the GC.

(If we fail to add the special because it already exists, that case is
fine. We don't even use the same handle value, but the one we obtain
from the attached GC-visible special, *and* we return that value, so it
remains live.)

For golang#70455.
Fixes golang#70469.

Change-Id: Iadaff0cfb93bcaf61ba2b05be7fa0519c481de82
Reviewed-on: https://go-review.googlesource.com/c/go/+/630316
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
…ows-386

The failures in golang#70288 are consistent with and strongly imply
stack corruption during fault handling, and debug prints show
that the Go code run during fault handling is running about
300 bytes above the bottom of the goroutine stack.
That should be okay, but that implies the DLL code that called
Go's handler was running near the bottom of the stack too,
and maybe it called other deeper things before or after the
Go handler and smashed the stack that way.

stackSystem is already 4096 bytes on amd64;
making it match that on 386 makes the flaky failures go away.
It's a little unsatisfying not to be able to say exactly what is
overflowing the stack, but the circumstantial evidence is
very strong that it's Windows.

For golang#70288.
Fixes golang#70475.

Change-Id: Ife89385873d5e5062a71629dbfee40825edefa49
Reviewed-on: https://go-review.googlesource.com/c/go/+/627375
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Russ Cox <rsc@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
(cherry picked from commit 7eeb0a1)
Reviewed-on: https://go-review.googlesource.com/c/go/+/632196
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Veronica Silina <veronicasilina@google.com>
Fix a regression that appeared in 1.23 when it comes to the stack traces
shown in the trace viewer. In 1.22 and earlier, the viewer was always
showing end stack traces. In 1.23 and later the viewer started to
exclusively show start stack traces.

Showing only the start stack traces made it impossible to see the last
stack trace produced by a goroutine. It also made it hard to understand
why a goroutine went off-cpu, as one had to hunt down the next running
slice of the same goroutine.

Emit end stack traces in addition to start stack traces to fix the
issue.

Fixes golang#70592

Change-Id: Ib22ea61388c1d94cdbc99fae2d207c4dce011a59
Reviewed-on: https://go-review.googlesource.com/c/go/+/631895
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com>
Reviewed-by: Nick Ripley <nick.ripley@datadoghq.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
(cherry picked from commit 6405e60)
Reviewed-on: https://go-review.googlesource.com/c/go/+/632075
Reviewed-by: Veronica Silina <veronicasilina@google.com>
Auto-Submit: Veronica Silina <veronicasilina@google.com>
Change-Id: I8d26e5231e868476949390ec900f0273c816d807
Reviewed-on: https://go-review.googlesource.com/c/go/+/633217
Reviewed-by: Veronica Silina <veronicasilina@google.com>
Auto-Submit: Gopher Robot <gobot@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Flips the pkgReader.enableAlias flag to true when reading unified IR.
This was disabled while resolving golang#66873. This resolves the TODO to
flip it back to true.

Fixes golang#70394
Fixes golang#70517
Updates golang#66873

Change-Id: Ifd52b0f9510d6bcf151de1c9a18d71ab548c14e4
Reviewed-on: https://go-review.googlesource.com/c/go/+/604099
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
(cherry picked from commit 209ed1a)
Reviewed-on: https://go-review.googlesource.com/c/go/+/631855
Commit-Queue: Tim King <taking@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
…t cgocallback

Currently, at a cgo callback where there is already a Go frame on
the stack (i.e. C->Go->C->Go), we require that at the inner Go
callback the SP is within the g0's stack bounds set by a previous
callback. This is to prevent that the C code switches stack while
having a Go frame on the stack, which we don't really support. But
this could also happen when we cannot get accurate stack bounds,
e.g. when pthread_getattr_np is not available. Since the stack
bounds are just estimates based on the current SP, if there are
multiple C->Go callbacks with various stack depth, it is possible
that the SP of a later callback falls out of a previous call's
estimate. This leads to runtime throw in a seemingly reasonable
program.

This CL changes it to save the old g0 stack bounds at cgocallback,
update the bounds, and restore the old bounds at return. So each
callback will get its own stack bounds based on the current SP,
and when it returns, the outer callback has the its old stack
bounds restored.

Also, at a cgo callback when there is no Go frame on the stack,
we currently always get new stack bounds. We do this because if
we can only get estimated bounds based on the SP, and the stack
depth varies a lot between two C->Go calls, the previous
estimates may be off and we fall out or nearly fall out of the
previous bounds. But this causes a performance problem: the
pthread API to get accurate stack bounds (pthread_getattr_np) is
very slow when called on the main thread. Getting the stack bounds
every time significantly slows down repeated C->Go calls on the
main thread.

This CL fixes it by "caching" the stack bounds if they are
accurate. I.e. at the second time Go calls into C, if the previous
stack bounds are accurate, and the current SP is in bounds, we can
be sure it is the same stack and we don't need to update the bounds.
This avoids the repeated calls to pthread_getattr_np. If we cannot
get the accurate bounds, we continue to update the stack bounds
based on the SP, and that operation is very cheap.

On a Linux/AMD64 machine with glibc:

name                     old time/op  new time/op  delta
CgoCallbackMainThread-8  96.4µs ± 3%   0.1µs ± 2%  -99.92%  (p=0.000 n=10+9)

Updates golang#68285.
Updates golang#68587.
Fixes golang#69988.

Change-Id: I3422badd5ad8ff63e1a733152d05fb7a44d5d435
Reviewed-on: https://go-review.googlesource.com/c/go/+/600296
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
(cherry picked from commit 76a8409)
Reviewed-on: https://go-review.googlesource.com/c/go/+/635775
…handle EOPNOTSUPP/ENOTSUP

This is not a cherry pick, because the code has changed on tip.

For golang#70763
Fixes golang#70789

Change-Id: If9fcfee17e86a746cf8c72293dc34f80b430f6e6
Reviewed-on: https://go-review.googlesource.com/c/go/+/635397
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
…pired certificates

Updates golang#71077
Fixes golang#71104

Change-Id: I6a6a465685f3bd50a5bb35a160f87b59b74fa6af
Reviewed-on: https://go-review.googlesource.com/c/go/+/639655
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Joel Sing <joel@sing.id.au>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/640315
Reviewed-by: Filippo Valsorda <filippo@golang.org>
…in injectglist

Currently injectglist emits all the trace events before actually calling
casgstatus on each goroutine. This is a problem, since tracing can
observe an inconsistent state (gstatus does not match tracer's 'emitted
an event' state).

This change fixes the problem by having injectglist do what every other
scheduler function does, and that's wrap each call to casgstatus in
traceAcquire/traceRelease.

For golang#70883.
Fixes golang#71147.

Change-Id: I857e96cec01688013597e8efc0c4c3d0b72d3a70
Reviewed-on: https://go-review.googlesource.com/c/go/+/638558
Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
(cherry picked from commit f025d19)
Reviewed-on: https://go-review.googlesource.com/c/go/+/641378
Auto-Submit: Michael Pratt <mpratt@google.com>
… URIs

When checking URI constraints, use netip.ParseAddr, which understands
zones, unlike net.ParseIP which chokes on them. This prevents zone IDs
from mistakenly satisfying URI constraints.

Thanks to Juho Forsén of Mattermost for reporting this issue.

For golang#71156
Fixes golang#71208
Fixes CVE-2024-45341

Change-Id: Iecac2529f3605382d257996e0fb6d6983547e400
Reviewed-on: https://go-internal-review.googlesource.com/c/go/+/1700
Reviewed-by: Tatiana Bradley <tatianabradley@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
(cherry picked from commit 22ca55d396ba801e6ae9b2bd67a059fcb30562fd)
Reviewed-on: https://go-internal-review.googlesource.com/c/go/+/1762
Reviewed-by: Roland Shoemaker <bracewell@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/643103
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>
…eated redirects

When an HTTP redirect changes the host of a request, we drop
sensitive headers such as Authorization from the redirected request.
Fix a bug where a chain of redirects could result in sensitive
headers being sent to the wrong host:

  1. request to a.tld with Authorization header
  2. a.tld redirects to b.tld
  3. request to b.tld with no Authorization header
  4. b.tld redirects to b.tld
  3. request to b.tld with Authorization header restored

Thanks to Kyle Seely for reporting this issue.

For golang#70530
Fixes #golang#71211
Fixes CVE-2024-45336

Reviewed-on: https://go-internal-review.googlesource.com/c/go/+/1641
Reviewed-by: Roland Shoemaker <bracewell@google.com>
Reviewed-by: Tatiana Bradley <tatianabradley@google.com>
Commit-Queue: Roland Shoemaker <bracewell@google.com>
Change-Id: I326544358de71ff892d9e9fe338252a5dd04001f
Reviewed-on: https://go-internal-review.googlesource.com/c/go/+/1764
Reviewed-on: https://go-review.googlesource.com/c/go/+/643104
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>
Change-Id: I849328cf93adf24c223f103f5b834635970ea652
Reviewed-on: https://go-review.googlesource.com/c/go/+/643137
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Gopher Robot <gobot@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
We can't coalesce a non-WB store with a subsequent Move, as the
result of the store might be the source of the move.

There's a simple codegen test. Not sure how we might do a real test,
as all the repro's I've come up with are very expensive and unreliable.

Fixes golang#71230

Change-Id: If18bf181a266b9b90964e2591cd2e61a7168371c
Reviewed-on: https://go-review.googlesource.com/c/go/+/642197
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>
Reviewed-on: https://go-review.googlesource.com/c/go/+/642498
… to send all tags in shallow fetch

Newer git versions (at least git 2.47.1) do not send all the matching tags
for a shallow fetch of a specific hash anymore. The go command assumes
that git servers do this. Since that assumption is broken, use the local
copy of the remote refs list to augment the tags sent by the server.
This makes the cmd/go/internal/modfetch tests pass again with newer git.

For golang#71261
Fixes golang#71263

Change-Id: I9fd4f3fd7beeb68a522938599f8f3acd887d0b26
Reviewed-on: https://go-review.googlesource.com/c/go/+/642437
Reviewed-by: Michael Matloob <matloob@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Russ Cox <rsc@golang.org>
(cherry picked from commit bd80d89)
Reviewed-on: https://go-review.googlesource.com/c/go/+/642696
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
@mpminardi mpminardi self-assigned this Jan 28, 2025
@mpminardi mpminardi changed the title Update go1.23.5 Update to Go 1.23.5 Jan 28, 2025
@mpminardi mpminardi marked this pull request as ready for review January 28, 2025 18:11
@mpminardi mpminardi merged commit 9193e3c into tailscale.go1.23 Jan 28, 2025
5 checks passed
@mpminardi mpminardi deleted the update-go1.23.5 branch January 28, 2025 19:02
@bradfitz bradfitz restored the update-go1.23.5 branch January 28, 2025 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.