Skip to content

Commit

Permalink
runtime: add race annotations in IncNonDefault
Browse files Browse the repository at this point in the history
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).

Fixes golang#64649

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>
  • Loading branch information
dr2chase authored and ezz-no committed Feb 17, 2024
1 parent 4117b20 commit d15e7c0
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 1 deletion.
31 changes: 31 additions & 0 deletions src/internal/godebug/godebug_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package godebug_test
import (
"fmt"
. "internal/godebug"
"internal/race"
"internal/testenv"
"os"
"os/exec"
Expand Down Expand Up @@ -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=^TestBisectTestCase$").CombinedOutput()
Expand Down
10 changes: 9 additions & 1 deletion src/runtime/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,15 @@ func (g *godebugInc) IncNonDefault() {
// *godebug.Setting.
inc = new(func())
*inc = (*newInc)(g.name)
g.inc.Store(inc)
if raceenabled {
racerelease(unsafe.Pointer(&g.inc))
}
if !g.inc.CompareAndSwap(nil, inc) {
inc = g.inc.Load()
}
}
if raceenabled {
raceacquire(unsafe.Pointer(&g.inc))
}
(*inc)()
}
Expand Down

0 comments on commit d15e7c0

Please sign in to comment.