golang/go

runtime/pprof: incorrect function names for generics functions

korniltsev opened this issue · 10 comments

korniltsev commented

Go version

go version devel go1.22-de5b418bea Sat Dec 2 03:15:03 2023 +0000 linux/amd64

What operating system and processor architecture are you using (go env)?

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/korniltsev/.cache/go-build'
GOENV='/home/korniltsev/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/korniltsev/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/korniltsev/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/home/korniltsev/github/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/home/korniltsev/github/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='devel go1.22-de5b418bea Sat Dec 2 03:15:03 2023 +0000'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/home/korniltsev/github/go/src/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build3732277415=/tmp/go-build -gno-record-gcc-switches'

What did you do?

I wrote a test

func genericAllocFunc[T any](n int) []T {
	return make([]T, int(n))
}

func profileToString(p *profile.Profile) []string {
	var res []string
	for _, s := range p.Sample {
		var funcs []string
		for i := range s.Location {
			loc := s.Location[len(s.Location)-1-i]
			for _, line := range loc.Line {
				funcs = append(funcs, line.Function.Name)
			}
		}
		res = append(res, fmt.Sprintf("%s %v", strings.Join(funcs, ";"), s.Value))
	}
	return res
}

func TestGenericsHashKeyInPprofBuilder(t *testing.T) {
	functionInt := reflect.ValueOf(genericAllocFunc[int]).Pointer()
	functionUint64 := reflect.ValueOf(genericAllocFunc[uint64]).Pointer()
	rate := int64(524288)
	rec := []runtime.MemProfileRecord{
		{
			AllocBytes: 128, AllocObjects: 1,
			FreeBytes: 0, FreeObjects: 0,
			Stack0: [32]uintptr{functionInt},
		},
		{
			AllocBytes: 239, AllocObjects: 1,
			FreeBytes: 0, FreeObjects: 0,
			Stack0: [32]uintptr{functionUint64},
		},
	}

	var buf bytes.Buffer
	if err := writeHeapProto(&buf, rec, rate, ""); err != nil {
		t.Fatalf("writing profile: %v", err)
	}

	p, err := profile.Parse(&buf)
	if err != nil {
		t.Fatalf("profile.Parse: %v", err)
	}

	actual := profileToString(p)
	expected := []string{
		"runtime/pprof.genericAllocFunc[int] [4096 524352 4096 524352]",
		"runtime/pprof.genericAllocFunc[uint64] [2194 524407 2194 524407]",
	}
	if !reflect.DeepEqual(actual, expected) {
		t.Errorf("profile = %v\nwant = %v", actual, expected)
	}
}

What did you expect to see?

I expected the test to pass

What did you see instead?

The test fails.

I suspect the reason is because profileBuilder is using Frame->Function as key for checking if we already emited a function. However for generics functions it has dots there [...], so sometime for different functions with different generics types, the profileBuilder uses wrong functions.
https://github.com/golang/go/blob/de5b418bea70aaf27de1f47e9b5813940d1e15a4/src/runtime/pprof/proto.go#L614C39-L614C39

gopherbot commented

Change https://go.dev/cl/546815 mentions this issue: runtime/pprof: fix generics function names

korniltsev commented

Also this line could be affected in similar way, but Im not 100% sure yet.

go/src/runtime/pprof/proto.go

Line 564 in de5b418

if last.Function == newFrame.Function { // maybe recursion.

cherrymui commented

Thanks for the report. Could you reproduce with a memory profile taken by the pprof package, instead of manually constructing the records? The code above creates records corresponding to generic functions that are usually wrappers and not shown in profiles. Using an actually profile makes the test more realistic. Thanks.

korniltsev commented

@cherrymui Thanks. Yes I found this problem with real-world memory profile dump. I've updated the test in CL to dump the profile with pprof package.

prattmic commented

Thanks for the report and fix. I think this warrants a backport.

@gopherbot Please backport to 1.21. This can create invalid profiles when using generics with no practical workaround.

gopherbot commented

Backport issue(s) opened: #64609 (for 1.21).

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

prattmic commented

Also this line could be affected in similar way, but Im not 100% sure yet.

go/src/runtime/pprof/proto.go

Line 564 in de5b418

if last.Function == newFrame.Function { // maybe recursion.

I haven't thought about this closely, but I suspect it would be affected.

korniltsev commented

Also this line could be affected in similar way, but Im not 100% sure yet.

go/src/runtime/pprof/proto.go

Line 564 in de5b418

if last.Function == newFrame.Function { // maybe recursion.

I haven't thought about this closely, but I suspect it would be affected.

I think I reproduced it https://gist.github.com/korniltsev/f6c639c68c7c40e1ead5db6012abed92
it is not reliable though (and I dont know how to make it reliable), for example it does not work if I run the test while debugging within my ide

prattmic commented

for example it does not work if I run the test while debugging within my ide

Perhaps your IDE disables optimizations/inlining (-N -l), which would impact the way stack traces are represented.

gopherbot commented

Change https://go.dev/cl/549535 mentions this issue: [release-branch.go1.21] runtime/pprof: fix generics function names