diff options
author | David Chase <drchase@google.com> | 2023-12-14 14:20:12 -0500 |
---|---|---|
committer | Gopher Robot <gobot@golang.org> | 2024-01-04 21:37:29 +0000 |
commit | 8c6078adfbfc2ba5a2108dfddfafc3b69cce410e (patch) | |
tree | 89f6dd0ab66d61155cafb8717aa68e8bf0258f9a | |
parent | f6b203c8281cc84b9846a550aeee40ac395274ae (diff) | |
download | go-8c6078adfbfc2ba5a2108dfddfafc3b69cce410e.tar.gz go-8c6078adfbfc2ba5a2108dfddfafc3b69cce410e.zip |
[release-branch.go1.21] runtime: add race annotations in IncNonDefault
Also use CompareAndSwap to make the code actually less racy.
Added a test which will be meaningful when run under the race
detector (tested it -race with broken fix in runtime, it failed).
This backport incorporates the correction in CL 551856,
using racereleasemerge instead of racerelease.
Fixes #64757
Change-Id: I5972e08901d1adc8ba74858edad7eba91be1b0ce
Reviewed-on: https://go-review.googlesource.com/c/go/+/549796
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Mauri de Souza Meneguzzo <mauri870@gmail.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
(cherry picked from commit 3313bbb4055f38f53cd43c6c5782a229f445f230)
Reviewed-on: https://go-review.googlesource.com/c/go/+/550236
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
TryBot-Bypass: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
-rw-r--r-- | src/internal/godebug/godebug_test.go | 31 | ||||
-rw-r--r-- | src/runtime/runtime.go | 13 |
2 files changed, 40 insertions, 4 deletions
diff --git a/src/internal/godebug/godebug_test.go b/src/internal/godebug/godebug_test.go index 8e46283ada..65dd256e8e 100644 --- a/src/internal/godebug/godebug_test.go +++ b/src/internal/godebug/godebug_test.go @@ -7,6 +7,7 @@ package godebug_test import ( "fmt" . "internal/godebug" + "internal/race" "internal/testenv" "os" "os/exec" @@ -70,6 +71,36 @@ func TestMetrics(t *testing.T) { } } +// TestPanicNilRace checks for a race in the runtime caused by use of runtime +// atomics (not visible to usual race detection) to install the counter for +// non-default panic(nil) semantics. For #64649. +func TestPanicNilRace(t *testing.T) { + if !race.Enabled { + t.Skip("Skipping test intended for use with -race.") + } + if os.Getenv("GODEBUG") != "panicnil=1" { + cmd := testenv.CleanCmdEnv(testenv.Command(t, os.Args[0], "-test.run=^TestPanicNilRace$", "-test.v", "-test.parallel=2", "-test.count=1")) + cmd.Env = append(cmd.Env, "GODEBUG=panicnil=1") + out, err := cmd.CombinedOutput() + t.Logf("output:\n%s", out) + + if err != nil { + t.Errorf("Was not expecting a crash") + } + return + } + + test := func(t *testing.T) { + t.Parallel() + defer func() { + recover() + }() + panic(nil) + } + t.Run("One", test) + t.Run("Two", test) +} + func TestCmdBisect(t *testing.T) { testenv.MustHaveGoBuild(t) out, err := exec.Command("go", "run", "cmd/vendor/golang.org/x/tools/cmd/bisect", "GODEBUG=buggy=1#PATTERN", os.Args[0], "-test.run=BisectTestCase").CombinedOutput() diff --git a/src/runtime/runtime.go b/src/runtime/runtime.go index 0822d0e805..15119cf5df 100644 --- a/src/runtime/runtime.go +++ b/src/runtime/runtime.go @@ -101,12 +101,17 @@ func (g *godebugInc) IncNonDefault() { if newInc == nil { return } - // If other goroutines are racing here, no big deal. One will win, - // and all the inc functions will be using the same underlying - // *godebug.Setting. inc = new(func()) *inc = (*newInc)(g.name) - g.inc.Store(inc) + if raceenabled { + racereleasemerge(unsafe.Pointer(&g.inc)) + } + if !g.inc.CompareAndSwap(nil, inc) { + inc = g.inc.Load() + } + } + if raceenabled { + raceacquire(unsafe.Pointer(&g.inc)) } (*inc)() } |