Skip to content

cmd/compile: inconsistency with whether wrapper shows in traceback in race mode when method is tail call optimized #73915

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

Open
amusman opened this issue May 29, 2025 · 1 comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@amusman
Copy link
Contributor

amusman commented May 29, 2025

Go version

go version go1.25-devel_dbaa2d3e65 Wed May 28 20:35:09 2025 -0700 linux/amd64

Output of go env in your module/workspace:

AR='ar'
CC='gcc'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_ENABLED='1'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
CXX='g++'
GCCGO='gccgo'
GO111MODULE=''
GOAMD64='v1'
GOARCH='amd64'
GOAUTH='netrc'
GOBIN=''
GOCACHE='/home/amusman/.cache/go-build'
GOCACHEPROG=''
GODEBUG=''
GOENV='/home/amusman/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build1830851168=/tmp/go-build -gno-record-gcc-switches'
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMOD='/dev/null'
GOMODCACHE='/home/amusman/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/amusman/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/home/amusman/ws/gs'
GOSUMDB='sum.golang.org'
GOTELEMETRY='local'
GOTELEMETRYDIR='/home/amusman/.config/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/home/amusman/ws/gs/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.25-devel_dbaa2d3e65 Wed May 28 20:35:09 2025 -0700'
GOWORK=''
PKG_CONFIG='pkg-config'

What did you do?

Consider the following two examples in the next section. They are supposed to cause a race which may be detected using instrumentation with -race option and has a wrapper for the Base type's method Method (generated for the Derived). The wrapper is present as an inner frame of stack trace we expect to get for the race. We prevent inlining of Method into its wrapper in this example using go:noinline.

What did you see happen?

For the first example (where receiver is value) the wrapper is visible in the output:

$ cat racewrapval.go
package main

import (
        "fmt"
        "sync"
)

type Base struct {
        value *int
        mu    sync.Mutex
}

//go:noinline
func (b Base) Method() {
        b.mu.Lock()
        defer b.mu.Unlock()
        fmt.Println("Base.Method called with value:", *b.value)
}

type Derived struct {
        Base
}

type Methoder interface {
        Method()
}

var gm Methoder

func main() {
        v := 42
        d := &Derived{Base{value: &v}}
        gm = d

        // Create a data race to trigger race detector
        var wg sync.WaitGroup
        wg.Add(2)
        go func() {
                defer wg.Done()
                gm.Method() // This should use a tail call in the wrapper
        }()
        go func() {
                defer wg.Done()
                // Access the same field without locking to create a race
                v = 100
        }()
        wg.Wait()
}

$ go build -gcflags=-d=tailcall=1 -a -race -o racewrapval racewrapval.go
$ ./racewrapval

==================
WARNING: DATA RACE
Read at 0x00c000292028 by goroutine 8:
  main.Base.Method()
      /home/amusman/test/go_racy_tail/1/racewrapval.go:17 +0xf0
  main.(*Derived).Method()
      <autogenerated>:1 +0x58
  main.main.func1()
      /home/amusman/test/go_racy_tail/1/racewrapval.go:40 +0x7c

Previous write at 0x00c000292028 by goroutine 9:
  main.main.func2()
      /home/amusman/test/go_racy_tail/1/racewrapval.go:45 +0x68

Goroutine 8 (running) created at:
  main.main()
      /home/amusman/test/go_racy_tail/1/racewrapval.go:38 +0x158

Goroutine 9 (finished) created at:
  main.main()
      /home/amusman/test/go_racy_tail/1/racewrapval.go:42 +0x1f0
==================
Base.Method called with value: 100
Found 1 data race(s)

Second example: if we change the Method to have pointer receiver (the only change compared to racewrapval.go above), the call trace with the -race option does not include the wrapper:

$ cat racewrapptr.go
package main

import (
        "fmt"
        "sync"
)

type Base struct {
        value *int
        mu    sync.Mutex
}

//go:noinline
func (b *Base) Method() {
        b.mu.Lock()
        defer b.mu.Unlock()
        fmt.Println("Base.Method called with value:", *b.value)
}

type Derived struct {
        Base
}

type Methoder interface {
        Method()
}

var gm Methoder

func main() {
        v := 42
        d := &Derived{Base{value: &v}}
        gm = d

        // Create a data race to trigger race detector
        var wg sync.WaitGroup
        wg.Add(2)
        go func() {
                defer wg.Done()
                gm.Method() // This should use a tail call in the wrapper
        }()
        go func() {
                defer wg.Done()
                // Access the same field without locking to create a race
                v = 100
        }()
        wg.Wait()
}

$ go build -gcflags=-d=tailcall=1 -a -race -o racewrapptr racewrapptr.go
# command-line-arguments
./racewrapptr.go:20:6: tail call emitted for the method (*Base).Method wrapper
$ ./racewrapptr
==================
WARNING: DATA RACE
Read at 0x00c000222028 by goroutine 8:
  main.(*Base).Method()
      /home/amusman/test/go_racy_tail/1/racewrapptr.go:17 +0xb0
  main.main.func1()
      /home/amusman/test/go_racy_tail/1/racewrapptr.go:40 +0x7c

Previous write at 0x00c000222028 by goroutine 9:
  main.main.func2()
      /home/amusman/test/go_racy_tail/1/racewrapptr.go:45 +0x68

Goroutine 8 (running) created at:
  main.main()
      /home/amusman/test/go_racy_tail/1/racewrapptr.go:38 +0x158

Goroutine 9 (finished) created at:
  main.main()
      /home/amusman/test/go_racy_tail/1/racewrapptr.go:42 +0x1f0
==================
Base.Method called with value: 100
Found 1 data race(s)

In the second example, the instrumentation calls to runtime.racefuncenter/runtime.racefuncexit are removed by opt because there is no call in the wrapper - there is a tail call instead. Also, after the tail call there is no place to add call to racefuncexit.

What did you expect to see?

I wonder, do we expect consistency between pointer and non-pointer method wrappers in showing/not showing them in the trace with -race option. If they better to keep consistent, which way of handling wrappers in such trace would be preferred: (1) with wrapper's inner frames included like it works in the first example or (2) omit the inner frames from wrapper in trace, like it works in the second example and in general trace ( see #73747 ).
We could get consistent (1) behavior disabling tail call when the instrumentation is enabled: in this case there would be remaining call in the wrapper which prevents optimizing away calls to runtime.racefuncenter/runtime.racefuncexit. On the other hand, if we want to always omit them (2), perhaps we need to avoid inserting this part of instrumentation in ssagen for the wrappers. But if we inlined into the wrapper then such instrumentation seems still needed.

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label May 29, 2025
@mknyszek mknyszek changed the title cmd/compile: Should we avoid using tail call when instrumentation is enabled cmd/compile: inconsistency with whether wrapper shows in traceback in race mode when method is tail call optimized May 29, 2025
@mknyszek mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 29, 2025
@mknyszek mknyszek added this to the Go1.25 milestone May 29, 2025
@mknyszek
Copy link
Contributor

CC @cherrymui @golang/compiler

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

3 participants