Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: golang/go
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: master@{1day}
Choose a base ref
...
head repository: golang/go
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: master
Choose a head ref
  • 3 commits
  • 12 files changed
  • 2 contributors

Commits on Jun 18, 2025

  1. runtime: prevent mutual deadlock between GC stopTheWorld and suspendG 

    Almost everywhere we stop the world we casGToWaitingForGC to prevent
mutual deadlock with the GC trying to scan our stack. This historically
was only necessary if we weren't stopping the world to change the GC
phase, because what we were worried about was mutual deadlock with mark
workers' use of suspendG. And, they were the only users of suspendG.

In Go 1.22 this changed. The execution tracer began using suspendG, too.
This leads to the possibility of mutual deadlock between the execution
tracer and a goroutine trying to start or end the GC mark phase. The fix
is simple: make the stop-the-world calls for the GC also call
casGToWaitingForGC. This way, suspendG is guaranteed to make progress in
this circumstance, and once it completes, the stop-the-world can
complete as well.

We can take this a step further, though, and move casGToWaitingForGC
into stopTheWorldWithSema, since there's no longer really a place we can
afford to skip this detail.

While we're here, rename casGToWaitingForGC to casGToWaitingForSuspendG,
since the GC is now not the only potential source of mutual deadlock.

Fixes #72740.

Change-Id: I5e3739a463ef3e8173ad33c531e696e46260692f
Reviewed-on: https://go-review.googlesource.com/c/go/+/681501
Reviewed-by: Carlos Amedee <[email protected]>
Auto-Submit: Michael Knyszek <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Cherry Mui <[email protected]>
    @mknyszek @gopherbot
    mknyszek authored and gopherbot committed Jun 18, 2025
    Configuration menu
    Copy the full SHA
    c6ac736 View commit details
    Browse the repository at this point in the history

  2. runtime: set mspan limit field early and eagerly 

    Currently the mspan limit field is set after allocSpan returns, *after*
the span has already been published to the GC (including the
conservative scanner). But the limit field is load-bearing, because it's
checked to filter out invalid pointers. A stale limit value could cause
a crash by having the conservative scanner access allocBits out of
bounds.

Fix this by setting the mspan limit field before publishing the span.
For large objects and arena chunks, we adjust the limit down after
allocSpan because we don't have access to the true object's size from
allocSpan. However this is safe, since we first initialize the limit to
something definitely safe (the actual span bounds) and only adjust it
down after. Adjusting it down has the benefit of more precise debug
output, but the window in which it's imprecise is also fine because a
single object (logically, with arena chunks) occupies the whole span, so
the 'invalid' part of the memory will just safely point back to that
object. We can't do this for smaller objects because the limit will
include space that does *not* contain any valid objects.

Fixes #74288.

Change-Id: I0a22e5b9bccc1bfdf51d2b73ea7130f1b99c0c7c
Reviewed-on: https://go-review.googlesource.com/c/go/+/682655
Reviewed-by: Keith Randall <[email protected]>
Auto-Submit: Michael Knyszek <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Keith Randall <[email protected]>
    @mknyszek @gopherbot
    mknyszek authored and gopherbot committed Jun 18, 2025
    Configuration menu
    Copy the full SHA
    4c75672 View commit details
    Browse the repository at this point in the history

  3. cmd/go/internal/fips140: ignore GOEXPERIMENT on error 

    During toolchain selection, the GOEXPERIMENT value may not be valid for
the current version (but it is valid for the selected version). In this
case, cfg.ExperimentErr is set and cfg.Experiment is nil.

Normally cmd/go main exits when ExperimentErr is set, so Experiment is
~never nil. But that is skipped during toolchain selection, and
fips140.Init is used during toolchain selection.

Fixes #74111.

Change-Id: I6a6a636c65ee5831feaf3d29993a60613bbec6f2
Reviewed-on: https://go-review.googlesource.com/c/go/+/680976
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Michael Matloob <[email protected]>
Reviewed-by: Junyang Shao <[email protected]>
Auto-Submit: Michael Pratt <[email protected]>
    @prattmic @gopherbot
    prattmic authored and gopherbot committed Jun 18, 2025
    Configuration menu
    Copy the full SHA
    8552bcf View commit details
    Browse the repository at this point in the history
Loading