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).

Fixes #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>

diff --git a/src/internal/godebug/godebug_test.go b/src/internal/godebug/godebug_test.go
index ed8e93d453fd4feb70102417716bf9748fe3154c..1ed0a365ab9190f711f0976c49b5b02665555c67 100644 (file)
--- 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=^TestBisectTestCase$").CombinedOutput()

diff --git a/src/runtime/runtime.go b/src/runtime/runtime.go
index 0829a84e436cdc8412ec8a8d84d082adf94796d6..92cdfc310ec91cca0f666d2e964f09a8e4062a15 100644 (file)
--- a/src/runtime/runtime.go
+++ b/src/runtime/runtime.go
@@ -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)()
 }