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/go/callgraph/vta: panics in analysis.Analyzer #50670

Closed
knsh14 opened this issue Jan 18, 2022 · 6 comments
Closed

x/tools/go/callgraph/vta: panics in analysis.Analyzer #50670

knsh14 opened this issue Jan 18, 2022 · 6 comments
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
Unreleased

Comments

@knsh14
Copy link

knsh14 commented Jan 18, 2022

What version of Go are you using (go version)?

$ go version
go version go1.17.5 darwin/amd64

Does this issue reproduce with the latest release?

yes

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/knsh14/Library/Caches/go-build"
GOENV="/Users/knsh14/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/knsh14/go/pkg/mod"
GONOPROXY="github.com/kouzoh/*"
GONOSUMDB="github.com/kouzoh/*"
GOOS="darwin"
GOPATH="/Users/knsh14/go"
GOPRIVATE="github.com/kouzoh/*"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/Cellar/go/1.17.5/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.17.5/libexec/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.17.5"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/knsh14/go/src/github.com/knsh14/vtasample/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/45/s50k6fxs1lqb8vk3b8rd0gcw0000gp/T/go-build2766226786=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

https://github.com/knsh14/vtasample/blob/master/vtasample.go
using vta package to build call graph in golang.org/x/tools/go/analysis.Analyzer.Run

What did you expect to see?

build callgraph or return error

What did you see instead?

test panics with panic: runtime error: index out of range [0] with length 0
at this line https://cs.opensource.google/go/x/tools/+/master:go/callgraph/vta/graph.go;l=588
@bcmills bcmills changed the title golang.org/x/tools/go/callgraph/vta : panics when call in analysis.Analyzer x/tools/go/callgraph/vta: panics in analysis.Analyzer Jan 18, 2022
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Jan 18, 2022
@gopherbot gopherbot added this to the Unreleased milestone Jan 18, 2022
@bcmills
Copy link
Contributor

bcmills commented Jan 18, 2022

(CC @zpavlinovic @timothy-king)

@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 18, 2022
@zpavlinovic
Copy link
Contributor

zpavlinovic commented Jan 18, 2022 •
edited

It seems some functions passed to VTA have an empty body/missing parameters, which can happen when VTA is used within analysis.Analyzer. I will look into that and make a fix/workaround so that panic goes away.

A comment on the code in vtasample.go. Maybe this is just an illustrative example, but VTA typically accepts allfns instead of each function individually, if the intention is to build a call graph for s.Pkg.Prog. Another thing I would point out is that run works on package level, so running this on a realistic program would build the same call graph for each package (since we call VTA on allfns which is defined over the whole program s.Pkg.Prog). VTA is inherently a whole-program analysis.

@zpavlinovic zpavlinovic self-assigned this Jan 18, 2022
@zpavlinovic zpavlinovic added the Analysis Issues related to static analysis (vet, x/tools/go/analysis) label Jan 18, 2022
@timothy-king
Copy link
Contributor

Another thing I would point out is that run works on package level, so running this on a realistic program would build the same call graph for each package (since we call VTA on allfns which is defined over the whole program s.Pkg.Prog). VTA is inherently a whole-program analysis.

IIUC this is not quite right. The result of x/tools/go/analysis/passes/buildssa is a new package with its directly imported packages stubbed out [+ a bit of extra stuff]. The current package+the stubs form the program accessible from the buildssa Result. This enables modular analysis of ssa without access to transitively imported packages.

A big implication of this is that (1) type sets for CHA cannot be complete, and (2) type flows for VTA will only contain the current package. Calling an imported function q.Id from package p where package q; func Id(i interface{}) interface{} { return i} then the body of q.Id will be empty. [If IIUC] vta will not include a type flow into the return value in this case.

@knsh14
Copy link
Author

knsh14 commented Jan 19, 2022

thanks for comments! I wanted to investigate which function causes panic. that is why I separate and build callgraph of each functions.

and more general situation to cause this panic is calling other package's exported function.

@gopherbot
Copy link

Change https://golang.org/cl/380795 mentions this issue: go/callgraph/vta: avoid panic on missing function definitions

gopherbot pushed a commit to golang/tools that referenced this issue Jan 27, 2022
@zpavlinovic
go/callgraph/vta: avoid panic on missing function definitions
0f0bbfd 
For golang/go#50670

Change-Id: I86d4aee70584cf255f0972e56726b29555120e6d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/380795
Reviewed-by: Guodong Li <guodongli@google.com>
Run-TryBot: Zvonimir Pavlinovic <zpavlinovic@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Trust: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Tim King <taking@google.com>
@zpavlinovic
Copy link
Contributor

The fix should now be in. Closing this one.

@golang golang locked and limited conversation to collaborators Jun 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. 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
5 participants
@timothy-king @zpavlinovic @knsh14 @bcmills @gopherbot