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.

Sign up for GitHub

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

Jump to bottom

x/tools/cmd/callgraph: calls from unreachable unexported methods not reported in callgraph #66251

Open
thesilentg opened this issue Mar 11, 2024 · 4 comments
Open

x/tools/cmd/callgraph: calls from unreachable unexported methods not reported in callgraph #66251

thesilentg opened this issue Mar 11, 2024 · 4 comments
Assignees
@adonovan
Labels
Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
Unreleased

Comments

@thesilentg
Copy link

Go version

go version go1.22.0 darwin/amd64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN='/Users/aggnolek/go/bin'
GOCACHE='/Users/aggnolek/Library/Caches/go-build'
GOENV='/Users/aggnolek/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/aggnolek/go/pkg/mod'
GONOPROXY='*'
GONOSUMDB='*'
GOOS='darwin'
GOPATH='/Users/aggnolek/go'
GOPRIVATE='*'
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/Users/aggnolek/sdk/go1.22.0'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/Users/aggnolek/sdk/go1.22.0/pkg/tool/darwin_amd64'
GOVCS=''
GOVERSION='go1.22.0'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='cc'
CXX='c++'
CGO_ENABLED='1'
GOMOD='/Users/aggnolek/gorepos/aggnolek/scratchpad/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 -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/x7/2f8ynt3954s4y78yt_54v9fr0000gs/T/go-build886972198=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

go.mod

module scratchpad

go 1.21

example2/main.go

package main

func called() {}

type unexported struct{}

func (u unexported) Func1() {
	called()
}

type Exported struct{}

func (e Exported) Func1() {
	called()
}

func main() {
	
}

Ran callgraph -algo={algo} ./example2 for algo in [static, cha, rta, vta]

What did you see happen?

callgraph -algo=static ./example2

(scratchpad/example2.Exported).Func1	--static-14:8-->	scratchpad/example2.called

callgraph -algo=cha ./example2

(scratchpad/example2.Exported).Func1	--static-14:8-->	scratchpad/example2.called

callgraph -algo=rta ./example2

{Empty Output}

callgraph -algo=vta ./example2

(scratchpad/example2.Exported).Func1	--static-14:8-->	scratchpad/example2.called

What did you expect to see?

callgraph -algo=static ./example2

(scratchpad/example2.Exported).Func1	--static-14:8-->	scratchpad/example2.called
(scratchpad/example2.unexported).Func1	--static-8:8-->	scratchpad/example2.called

callgraph -algo=cha ./example2

(scratchpad/example2.Exported).Func1	--static-14:8-->	scratchpad/example2.called
(scratchpad/example2.unexported).Func1	--static-8:8-->	scratchpad/example2.called

callgraph -algo=rta ./example2

{Empty Output}

This is because rta only includes reachable funcs by design

callgraph -algo=vta ./example2

(scratchpad/example2.Exported).Func1	--static-14:8-->	scratchpad/example2.called
(scratchpad/example2.unexported).Func1	--static-8:8-->	scratchpad/example2.called

Note that the link from example2.unexported).Func1 to called is present when example2.unexported).Func1 is forced to be reachable. For example:

example2/main.go

func main() {
	unexported{}.Func1()
}

callgraph -algo=static ./example2

(scratchpad/example2.unexported).Func1	--static-8:8-->	scratchpad/example2.called
(scratchpad/example2.Exported).Func1	--static-14:8-->	scratchpad/example2.called
scratchpad/example2.main	--static-18:20-->	(scratchpad/example2.unexported).Func1

callgraph -algo=cha ./example2

(scratchpad/example2.unexported).Func1	--static-8:8-->	scratchpad/example2.called
(scratchpad/example2.Exported).Func1	--static-14:8-->	scratchpad/example2.called
scratchpad/example2.main	--static-18:20-->	(scratchpad/example2.unexported).Func1

callgraph -algo=rta ./example2

(scratchpad/example2.unexported).Func1	--static-8:8-->	scratchpad/example2.called
scratchpad/example2.main	--static-18:20-->	(scratchpad/example2.unexported).Func1

This is because rta only includes reachable funcs by design

callgraph -algo=vta ./example2

(scratchpad/example2.unexported).Func1	--static-8:8-->	scratchpad/example2.called
(scratchpad/example2.Exported).Func1	--static-14:8-->	scratchpad/example2.called
scratchpad/example2.main	--static-18:20-->	(scratchpad/example2.unexported).Func1

This likely has to do with the same usage of ssautil.AllFunctions (at least for vta) that resulted in this bug for the deadcode command.
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Mar 11, 2024
@gopherbot gopherbot added this to the Unreleased milestone Mar 11, 2024
@thesilentg
Copy link
Author

@adonovan Given your closeness to the likely similar prior issue

@adonovan
Copy link
Member

Thanks. I expect a patch along these lines should be effective:

        case "vta":
-               cg = vta.CallGraph(ssautil.AllFunctions(prog), cha.CallGraph(prog))
+               // Gather all source-level functions as entry points.
+               sourceFuncs := make(map[*ssa.Function]bool)
+               packages.Visit(initial, nil, func(p *packages.Package) {
+                       for _, file := range p.Syntax {
+                               for _, decl := range file.Decls {
+                                       if decl, ok := decl.(*ast.FuncDecl); ok {
+                                               obj := p.TypesInfo.Defs[decl.Name].(*types.Func)
+                                               fn := prog.FuncValue(obj)
+                                               sourceFuncs[fn] = true
+                                       }
+                               }
+                       }
+               })
+
+               cg = vta.CallGraph(sourceFuncs, cha.CallGraph(prog))

@adonovan adonovan self-assigned this Mar 11, 2024
@thesilentg
Copy link
Author

Agree with respect to rta, although based on my understanding both static and cha will require separate fixes as those don't take in sourceFuncs.

@thesilentg
Copy link
Author

Based on my early testing, I believe you'll also need to filter out nil results for fn, as prog.FuncValue returns nil for interface methods and those nils cause panics within vta.CallGraph

@samuknet samuknet mentioned this issue Mar 20, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@adonovan adonovan

Labels
Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
3 participants
@thesilentg @adonovan @gopherbot