golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
121.48k stars 17.4k forks source link

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

Open thesilentg opened 4 months ago

thesilentg commented 4 months ago

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.

thesilentg commented 4 months ago

@adonovan Given your closeness to the likely similar prior issue

adonovan commented 4 months ago

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))
thesilentg commented 4 months ago

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 commented 4 months ago

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