Skip to content

runtime: -race data race map traceback report incorrect functions #73191

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
prattmic opened this issue Apr 7, 2025 · 5 comments
Closed

runtime: -race data race map traceback report incorrect functions #73191

prattmic opened this issue Apr 7, 2025 · 5 comments
Labels
BugReport Issues describing a possible bug in the Go implementation. compiler/runtime Issues related to the Go compiler and/or runtime. FixPending Issues that have a fix which has not yet been reviewed or submitted. NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@prattmic
Copy link
Member

prattmic commented Apr 7, 2025

The fast variant swissmap functions report the wrong PC to TSAN in their custom instrumentation, which results in incorrect stack traces when races are found and GOTRACEBACK=system or higher (by default the runtime frame is hidden).

package main

func main() {
        m := make(map[int]int)
        go func() {
                for {
                        _ = m[1]
                }
        }()
        go func() {
                for {
                        m[1] = 1
                }
        }()
        select {}
}
$ GOTRACEBACK=system go run -race main.go
==================
WARNING: DATA RACE
Read at 0x00c000034030 by goroutine 6:
  runtime.mapaccess1()
      /usr/lib/google-golang/src/internal/runtime/maps/runtime_swiss.go:43 +0x0
  main.main.func1()
      /tmp/racemap/main.go:7 +0x3a

Previous write at 0x00c000034030 by goroutine 7:
  runtime.mapassign()
      /usr/lib/google-golang/src/internal/runtime/maps/runtime_swiss.go:191 +0x0
  main.main.func2()
      /tmp/racemap/main.go:12 +0x3a

Goroutine 6 (running) created at:
  main.main()
      /tmp/racemap/main.go:5 +0x7c

Goroutine 7 (running) created at:
  main.main()
      /tmp/racemap/main.go:10 +0xc6
==================
fatal error: concurrent map read and map write

goroutine 5 gp=0xc000003dc0 m=5 mp=0xc000088008 [running]:
runtime.fatal({0x4bbb40, 0x21})
        /usr/lib/google-golang/src/runtime/panic.go:1134 +0x5c fp=0xc000049760 sp=0xc000049730 pc=0x46949c
internal/runtime/maps.fatal({0x4bbb40?, 0x4a1939?})
        /usr/lib/google-golang/src/runtime/panic.go:1067 +0x18 fp=0xc000049780 sp=0xc000049760 pc=0x49ad38
runtime.mapaccess1_fast64(0x4ad660, 0xc000034030, 0x1)
        /usr/lib/google-golang/src/internal/runtime/maps/runtime_fast64_swiss.go:29 +0xfe fp=0xc0000497b0 sp=0xc000049780 pc=0x43b61e
main.main.func1()
        /tmp/racemap/main.go:7 +0x3b fp=0xc0000497e0 sp=0xc0000497b0 pc=0x4a3b7b
runtime.goexit({})
        /usr/lib/google-golang/src/runtime/asm_amd64.s:1700 +0x1 fp=0xc0000497e8 sp=0xc0000497e0 pc=0x4a02a1
created by main.main in goroutine 1
        /tmp/racemap/main.go:5 +0x7d
...

In the DATA RACE report, the read should say runtime.mapaccess1_fast64, not runtime.mapaccess1 and the write should say runtime.mapassign_fast64, not runtime.mapassign.

Note that the always-enabled concurrent map read/write throw got the stack correct because it is just a normal throw, but that isn't guaranteed to catch a race when TSAN does.

Thanks @mateusz834 for finding this.

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Apr 7, 2025
@prattmic prattmic added the NeedsFix The path to resolution is known, but the work has not been done. label Apr 7, 2025
@prattmic prattmic added this to the Go1.25 milestone Apr 7, 2025
@prattmic
Copy link
Member Author

prattmic commented Apr 7, 2025

@gopherbot Please backport to 1.24. This is a minor issue. DATA RACE reports for races in fast-variant maps (int or string keys) will report the wrong map function (e.g., mapassign vs mapassign_fast32). This is a reporting issue only that may cause confusion for readers, as it seems to be an "impossible" call that doesn't appear in the assembly. The most confusing one is map delete, which will report as map assign. This only affects GOTRACEBACK=system or higher because we otherwise hide the runtime function anyway.

@gopherbot
Copy link
Contributor

Backport issue(s) opened: #73192 (for 1.24).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

@gabyhelp gabyhelp added the BugReport Issues describing a possible bug in the Go implementation. label Apr 7, 2025
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/663175 mentions this issue: internal/runtime/maps: pass proper func PC to race.WritePC/race.ReadPC

@dmitshur dmitshur added the FixPending Issues that have a fix which has not yet been reviewed or submitted. label Apr 7, 2025
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/663777 mentions this issue: [release-branch.go1.24] internal/runtime/maps: pass proper func PC to race.WritePC/race.ReadPC

gopherbot pushed a commit that referenced this issue Apr 28, 2025
… race.WritePC/race.ReadPC

Fixes #73192
For #73191

Change-Id: I0f8a5a19faa745943a98476c7caf4c97ccdce184
Reviewed-on: https://go-review.googlesource.com/c/go/+/663175
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Michael Pratt <[email protected]>
Auto-Submit: Michael Pratt <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
(cherry picked from commit 14b15a2)
Reviewed-on: https://go-review.googlesource.com/c/go/+/663777
Reviewed-by: Carlos Amedee <[email protected]>
Auto-Submit: Junyang Shao <[email protected]>
Reviewed-by: Junyang Shao <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BugReport Issues describing a possible bug in the Go implementation. compiler/runtime Issues related to the Go compiler and/or runtime. FixPending Issues that have a fix which has not yet been reviewed or submitted. NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants