From 1bb3f4ed2b045f0b10d0a66820681568c9b6377e Mon Sep 17 00:00:00 2001 From: chressie Date: Fri, 6 Feb 2026 15:05:22 +0000 Subject: [PATCH] runtime: add race instrumentation for moduleToTypelinksLock From Cherry Mui on the CL review: The reason why we need race instrumentation is tricky. The runtime package is not compiled with race instrumentation, therefore the race detector doesn't know about synchronizations within the runtime, and usually it doesn't care, as it also doesn't know about the concurrent data accesses in the runtime. However, in this case the concurrent access is in a map. Map accesses are always instrumented https://cs.opensource.google/go/go/+/master:src/internal/runtime/maps/runtime_fast64.go;l=25, even if the map is defined in the runtime. So the race detector does see the concurrent access. But it doesn't see the synchronization. So this CL adds them. Go maps are not common in the runtime. But I recall an issue like this did occur. Maybe we could make the compiler/runtime not to instrument map accesses for maps defined in the runtime (which would be a more involved change). --- The reproducer in src/runtime/testdata/testprog/typelinksrace.go was provided by Cherry Mui. Change-Id: I03b27c51b7278ee4b2939a1a0665169966999b33 Reviewed-on: https://go-review.googlesource.com/c/go/+/742740 LUCI-TryBot-Result: Go LUCI Reviewed-by: Cherry Mui --- .../testdata/testprog/typelinksrace.go | 37 +++++++++++++++++++ src/runtime/type.go | 9 +++++ src/runtime/typelinksrace_test.go | 17 +++++++++ 3 files changed, 63 insertions(+) create mode 100644 src/runtime/testdata/testprog/typelinksrace.go create mode 100644 src/runtime/typelinksrace_test.go diff --git a/src/runtime/testdata/testprog/typelinksrace.go b/src/runtime/testdata/testprog/typelinksrace.go new file mode 100644 index 0000000000..b4678b1780 --- /dev/null +++ b/src/runtime/testdata/testprog/typelinksrace.go @@ -0,0 +1,37 @@ +// Copyright 2026 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +import ( + "fmt" + "reflect" +) + + +func init() { + register("TypelinksRace", TypelinksRace) +} + +const N = 2 + +type T int + +// just needs some exotic type that the compiler doesn't build its pointer type +var t = reflect.TypeOf([5]T{}) + +var ch = make(chan int, N) + +func TypelinksRace() { + for range N { + go func() { + _ = reflect.PointerTo(t) + ch <- 1 + }() + } + for range N { + <-ch + } + fmt.Println("OK") +} diff --git a/src/runtime/type.go b/src/runtime/type.go index a8205c2404..ae8da3bc29 100644 --- a/src/runtime/type.go +++ b/src/runtime/type.go @@ -507,8 +507,14 @@ var ( // This slice is constructed as needed. func moduleTypelinks(md *moduledata) []*_type { lock(&moduleToTypelinksLock) + if raceenabled { + raceacquire(unsafe.Pointer(&moduleToTypelinksLock)) + } if typelinks, ok := moduleToTypelinks[md]; ok { + if raceenabled { + racerelease(unsafe.Pointer(&moduleToTypelinksLock)) + } unlock(&moduleToTypelinksLock) return typelinks } @@ -551,6 +557,9 @@ func moduleTypelinks(md *moduledata) []*_type { } moduleToTypelinks[md] = ret + if raceenabled { + racerelease(unsafe.Pointer(&moduleToTypelinksLock)) + } unlock(&moduleToTypelinksLock) return ret } diff --git a/src/runtime/typelinksrace_test.go b/src/runtime/typelinksrace_test.go new file mode 100644 index 0000000000..7bd436e77a --- /dev/null +++ b/src/runtime/typelinksrace_test.go @@ -0,0 +1,17 @@ +// Copyright 2026 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package runtime_test + +import ( + "testing" +) + +func TestTypelinksRace(t *testing.T) { + output := runTestProg(t, "testprog", "TypelinksRace") + want := "OK\n" + if output != want { + t.Fatalf("want %s, got %s\n", want, output) + } +} -- 2.52.0