Skip to content
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

debug/gosym: Sym.PackageName provides the wrong result #66313

Open
Zxilly opened this issue Mar 14, 2024 · 10 comments · May be fixed by #66345
Open

debug/gosym: Sym.PackageName provides the wrong result #66313

Zxilly opened this issue Mar 14, 2024 · 10 comments · May be fixed by #66345
Assignees
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

@Zxilly
Copy link
Contributor

Zxilly commented Mar 14, 2024

Go version

go version go1.22.0 windows/amd64

Output of go env in your module/workspace:

set GO111MODULE=on
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\zxilly\AppData\Local\go-build
set GOENV=C:\Users\zxilly\AppData\Roaming\go\env
set GOEXE=
set GOEXPERIMENT=
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=C:\Users\zxilly\go\pkg\mod
set GONOPROXY=1
set GONOSUMDB=
set GOOS=linux
set GOPATH=C:\Users\zxilly\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=C:/Program Files/Go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLCHAIN=auto
set GOTOOLDIR=C:\Program Files\Go\pkg\tool\windows_amd64
set GOVCS=
set GOVERSION=go1.22.0
set GCCGO=gccgo
set GOAMD64=v1
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=0
set GOMOD=E:\Temp\gotmp\go.mod
set GOWORK=
set CGO_CFLAGS=-O2 -g
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-O2 -g
set CGO_FFLAGS=-O2 -g
set CGO_LDFLAGS=-O2 -g
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-fPIC -m64 -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=C:\Users\zxilly\AppData\Local\Temp\go-build2223077610=/tmp/go-build -gno-record-gcc-switches

What did you do?

In some edge cases, Sym.PackageName will provide incorrect results.
I'm sorry I couldn't provide a minimally reproducible sample. But you can find a sample at https://github.com/ZNotify/server/releases/download/test/analysis-server-linux.

I use the follow code to identify the wrong output.

package main

import (
	"debug/elf"
	"debug/gosym"
	"fmt"
	"strings"
)

func main() {
	f, err := elf.Open("analysis-server-linux")
	if err != nil {
		panic(err)
	}
	// read pclntab
	pclntab, err := f.Section(".gopclntab").Data()
	if err != nil {
		panic(err)
	}

	var textStart uint64
	symbols, err := f.Symbols()
	if err != nil {
		panic(err)
	}
	for _, sym := range symbols {
		if sym.Name == "runtime.text" {
			textStart = sym.Value
			break
		}
	}
	if textStart == 0 {
		panic("runtime.text not found")
	}

	// create gosym.Table
	ltable := gosym.NewLineTable(pclntab, textStart)
	table,err := gosym.NewTable(nil, ltable)
	if err != nil {
		panic(err)
	}

	for _, fn := range table.Funcs {
		pkgName := fn.PackageName()
		if strings.Contains(pkgName, "*") {
			fmt.Printf("FnName: %s\nPackageGot:%s\n", fn.Name, fn.PackageName())
		}
	}
}

What did you see happen?

With the script above, I get the follow output

FnName: ariga.io/atlas/sql/sqlclient.(*Tx).database/sql.grabConn
PackageGot:ariga.io/atlas/sql/sqlclient.(*Tx).database/sql
FnName: ariga.io/atlas/sql/sqlclient.(*Tx).database/sql.txCtx
PackageGot:ariga.io/atlas/sql/sqlclient.(*Tx).database/sql
FnName: github.com/ZNotify/server/app/api/common.(*Context).github.com/gin-gonic/gin.reset
PackageGot:github.com/ZNotify/server/app/api/common.(*Context).github.com/gin-gonic/gin

What did you expect to see?

The correct package for these names should be

ariga.io/atlas/sql/sqlclient
ariga.io/atlas/sql/sqlclient
github.com/ZNotify/server/app/api/common
@Zxilly
Copy link
Contributor Author

Zxilly commented Mar 14, 2024

I also test it with some other popular golang project like k8s.

For https://github.com/Zxilly/go-testdata/releases/download/kube/kubelet-linux-1.22-amd64, we got

FnName: github.com/google/cel-go/parser/gen.(*CELLexer).github.com/antlr/antlr4/runtime/Go/antlr/v4.reset
PackageGot:github.com/google/cel-go/parser/gen.(*CELLexer).github.com/antlr/antlr4/runtime/Go/antlr/v4
FnName: github.com/google/cel-go/parser/gen.(*CELParser).github.com/antlr/antlr4/runtime/Go/antlr/v4.reset
PackageGot:github.com/google/cel-go/parser/gen.(*CELParser).github.com/antlr/antlr4/runtime/Go/antlr/v4
FnName: github.com/google/cel-go/parser.(*recoveryLimitErrorStrategy).github.com/antlr/antlr4/runtime/Go/antlr/v4.reset
PackageGot:github.com/google/cel-go/parser.(*recoveryLimitErrorStrategy).github.com/antlr/antlr4/runtime/Go/antlr/v4
FnName: github.com/gogo/protobuf/protoc-gen-gogo/descriptor.(*FieldOptions).github.com/gogo/protobuf/proto.extensionsRead
PackageGot:github.com/gogo/protobuf/protoc-gen-gogo/descriptor.(*FieldOptions).github.com/gogo/protobuf/proto
FnName: github.com/gogo/protobuf/protoc-gen-gogo/descriptor.(*FieldOptions).github.com/gogo/protobuf/proto.extensionsWrite
PackageGot:github.com/gogo/protobuf/protoc-gen-gogo/descriptor.(*FieldOptions).github.com/gogo/protobuf/proto
FnName: github.com/gogo/protobuf/protoc-gen-gogo/descriptor.(*EnumValueOptions).github.com/gogo/protobuf/proto.extensionsRead
PackageGot:github.com/gogo/protobuf/protoc-gen-gogo/descriptor.(*EnumValueOptions).github.com/gogo/protobuf/proto
FnName: github.com/gogo/protobuf/protoc-gen-gogo/descriptor.(*EnumValueOptions).github.com/gogo/protobuf/proto.extensionsWrite
PackageGot:github.com/gogo/protobuf/protoc-gen-gogo/descriptor.(*EnumValueOptions).github.com/gogo/protobuf/proto
FnName: github.com/gogo/protobuf/protoc-gen-gogo/descriptor.(*EnumOptions).github.com/gogo/protobuf/proto.extensionsRead
PackageGot:github.com/gogo/protobuf/protoc-gen-gogo/descriptor.(*EnumOptions).github.com/gogo/protobuf/proto
FnName: github.com/gogo/protobuf/protoc-gen-gogo/descriptor.(*EnumOptions).github.com/gogo/protobuf/proto.extensionsWrite
PackageGot:github.com/gogo/protobuf/protoc-gen-gogo/descriptor.(*EnumOptions).github.com/gogo/protobuf/proto
FnName: github.com/gogo/protobuf/protoc-gen-gogo/descriptor.(*ExtensionRangeOptions).github.com/gogo/protobuf/proto.extensionsRead
PackageGot:github.com/gogo/protobuf/protoc-gen-gogo/descriptor.(*ExtensionRangeOptions).github.com/gogo/protobuf/proto
FnName: github.com/gogo/protobuf/protoc-gen-gogo/descriptor.(*ExtensionRangeOptions).github.com/gogo/protobuf/proto.extensionsWrite
PackageGot:github.com/gogo/protobuf/protoc-gen-gogo/descriptor.(*ExtensionRangeOptions).github.com/gogo/protobuf/proto
FnName: github.com/gogo/protobuf/protoc-gen-gogo/descriptor.(*OneofOptions).github.com/gogo/protobuf/proto.extensionsRead
PackageGot:github.com/gogo/protobuf/protoc-gen-gogo/descriptor.(*OneofOptions).github.com/gogo/protobuf/proto
FnName: github.com/gogo/protobuf/protoc-gen-gogo/descriptor.(*OneofOptions).github.com/gogo/protobuf/proto.extensionsWrite
PackageGot:github.com/gogo/protobuf/protoc-gen-gogo/descriptor.(*OneofOptions).github.com/gogo/protobuf/proto
FnName: github.com/gogo/protobuf/protoc-gen-gogo/descriptor.(*MessageOptions).github.com/gogo/protobuf/proto.extensionsRead
PackageGot:github.com/gogo/protobuf/protoc-gen-gogo/descriptor.(*MessageOptions).github.com/gogo/protobuf/proto
FnName: github.com/gogo/protobuf/protoc-gen-gogo/descriptor.(*MessageOptions).github.com/gogo/protobuf/proto.extensionsWrite
PackageGot:github.com/gogo/protobuf/protoc-gen-gogo/descriptor.(*MessageOptions).github.com/gogo/protobuf/proto
FnName: github.com/gogo/protobuf/protoc-gen-gogo/descriptor.(*MethodOptions).github.com/gogo/protobuf/proto.extensionsRead
PackageGot:github.com/gogo/protobuf/protoc-gen-gogo/descriptor.(*MethodOptions).github.com/gogo/protobuf/proto
FnName: github.com/gogo/protobuf/protoc-gen-gogo/descriptor.(*MethodOptions).github.com/gogo/protobuf/proto.extensionsWrite
PackageGot:github.com/gogo/protobuf/protoc-gen-gogo/descriptor.(*MethodOptions).github.com/gogo/protobuf/proto
FnName: github.com/gogo/protobuf/protoc-gen-gogo/descriptor.(*ServiceOptions).github.com/gogo/protobuf/proto.extensionsRead
PackageGot:github.com/gogo/protobuf/protoc-gen-gogo/descriptor.(*ServiceOptions).github.com/gogo/protobuf/proto
FnName: github.com/gogo/protobuf/protoc-gen-gogo/descriptor.(*ServiceOptions).github.com/gogo/protobuf/proto.extensionsWrite
PackageGot:github.com/gogo/protobuf/protoc-gen-gogo/descriptor.(*ServiceOptions).github.com/gogo/protobuf/proto
FnName: github.com/gogo/protobuf/protoc-gen-gogo/descriptor.(*FileOptions).github.com/gogo/protobuf/proto.extensionsRead
PackageGot:github.com/gogo/protobuf/protoc-gen-gogo/descriptor.(*FileOptions).github.com/gogo/protobuf/proto
FnName: github.com/gogo/protobuf/protoc-gen-gogo/descriptor.(*FileOptions).github.com/gogo/protobuf/proto.extensionsWrite
PackageGot:github.com/gogo/protobuf/protoc-gen-gogo/descriptor.(*FileOptions).github.com/gogo/protobuf/proto

I suspect that something triggers a special rule at link time that adds an extra module name to the embedded receiver for identification purposes, but I can't find the exact code that adds the corresponding package name to the receiver, so I can't confirm that this is the actual behaviour occurring, and therefore can't write a patch that fixes the bug.

I would appreciate any help from anyone familiar with the linker codebase.

@cherrymui
Copy link
Member

It seems to me we now are adding package paths to some of the methods (maybe just unexported methods? embedded methods?), which is unexpected to me. I think it probably occurs in the compiler, not the linker.

@cherrymui cherrymui added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. compiler/runtime Issues related to the Go compiler and/or runtime. labels Mar 14, 2024
@cherrymui cherrymui added this to the Backlog milestone Mar 14, 2024
@Zxilly
Copy link
Contributor Author

Zxilly commented Mar 15, 2024

I agree with you, funcnametab was created by compiler and linker can do nothing about it.

Another point of note is that when I look at the source file for these functions using table.PCToLine, it shows them as <autogenerated>, which suggests that they were inserted by the compiler and are not part of the code

@cherrymui
Copy link
Member

Yeah, for embedded methods, the compiler creates wrapper methods on the outer type.

@Zxilly
Copy link
Contributor Author

Zxilly commented Mar 15, 2024

That's my guess too, but I can't reproduce it on a smaller example, do you have one of those?
I'm reading the code in gc about LSym, looking for specific rules for generating names.

@Zxilly
Copy link
Contributor Author

Zxilly commented Mar 15, 2024

If possible, I think we may need to change the rules for generating the symbol name.

Because things like this also happen with non-pointer receivers. For something like github.com/gogo/protobuf/protoc-gen-gogo/descriptor.FileOptions.github.com/gogo/protobuf/proto.extensionsWrite, there's no way to get the real package path since dot was allowed in the package path like gopkg.in/yaml.v3

@gopherbot
Copy link

Change https://go.dev/cl/572076 mentions this issue: cmd/compile/internal/ir: wrap the method package with parentheses

@cherrymui
Copy link
Member

cc @golang/compiler

@mdempsky
Copy link
Member

We package-prefix unexported promoted methods that were declared in another package. Something like this should produce prefixed method name:

package a
type T struct{}
func (T) m() {}

package b
import "path/to/a"
type U struct { a.T } // will produce a method named "path/to/b.U.path/to/a.m"

@Zxilly
Copy link
Contributor Author

Zxilly commented Mar 22, 2024

I was wondering if this issue might be fixed in go1.23

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
Status: In Progress
Development

Successfully merging a pull request may close this issue.

4 participants