Skip to content

x/tools/deadcode: panic analyzing dynamic call #67915

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

Closed
ct16k opened this issue Jun 10, 2024 · 4 comments
Closed

x/tools/deadcode: panic analyzing dynamic call #67915

ct16k opened this issue Jun 10, 2024 · 4 comments
Assignees
Labels
gopls Issues related to the Go language server, gopls. NeedsFix The path to resolution is known, but the work has not been done. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@ct16k
Copy link

ct16k commented Jun 10, 2024

Go version

go version go1.22.4 darwin/arm64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='arm64'
GOBIN=''
GOCACHE='/tmp/Library/Caches/go-build'
GOENV='/tmp/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/tmp/go/pkg/mod'
GONOPROXY='example.com/*'
GONOSUMDB='*'
GOOS='darwin'
GOPATH='/tmp/go'
GOPRIVATE='example.com/*'
GOPROXY='https://example.com'
GOROOT='/usr/local/go'
GOSUMDB='off'
GOTMPDIR=''
GOTOOLCHAIN='local'
GOTOOLDIR='/usr/local/go/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.22.4'
GCCGO='gccgo'
AR='ar'
CC='clang'
CXX='clang++'
CGO_ENABLED='1'
GOMOD='/tmp/gotools/go.mod'
GOWORK=''
CGO_CFLAGS='-I/opt/homebrew/include/zookeeper'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/bk/ypqf1wr10w5gmzpct5zk8kgr0000gn/T/go-build1645728497=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

Running deadcode -whylive=somefunc . on some code produced a a panic:

$ deadcode -whylive=example.com/some.Func .
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x38 pc=0x100f9b770]

goroutine 1 [running]:
main.main()
        /tmp/gotools/cmd/deadcode/deadcode.go:263 +0x17f0

The fix is trivial:

--- a/cmd/deadcode/deadcode.go
+++ b/cmd/deadcode/deadcode.go
@@ -260,7 +260,7 @@ func main() {
                        edges = append(edges, jsonEdge{
                                Initial:  cond(len(edges) == 0, prettyName(edge.Caller.Func, true), ""),
                                Kind:     cond(isStaticCall(edge), "static", "dynamic"),
-                               Position: toJSONPosition(prog.Fset.Position(edge.Site.Pos())),
+                               Position: toJSONPosition(prog.Fset.Position(edge.Pos())),
                                Callee:   prettyName(edge.Callee.Func, true),
                        })
                }

as edge.Pos() accounts for edge.Site possibly being nil. Unfortunately I'm not able to come up with a simple reproducer. My code is along the lines of:

package main

func f1() {}

func f4(f func()) {
	f()
}

func main() {
	f2 := func() {
		f1()
	}

	f3 := func() func() {
		return func() {
			f2()
		}
	}

	f4(f3())
}

but this one passes successfully (no panics), so there is something else I'm missing from my actual code:

$ deadcode -filter= -whylive=example.com.f1 .
                   example.com.main
  static@L0020 --> example.com.f4
 dynamic@L0006 --> example.com.main$1
  static@L0011 --> example.com.f1

With the above patch applied, the tool runs successfully and the output ends with:

$ deadcode -whylive=example.com/some.Func .
...
 dynamic@L0000 --> example.com/some.otherFunc$1
  static@L0001 --> example.com/some.Func

(note line 0 for the dynamic call, which is where the tool previously panicked)

Unless somebody more knowledgeable beats me to it, I'll create a pull request when I'm able to come up with a simple test to reproduce the panic.

What did you see happen?

Tool panicked with nil pointer dereference.

What did you expect to see?

Tool running successfully, reporting the callstack to the given function.

@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Jun 10, 2024
@gopherbot gopherbot added this to the Unreleased milestone Jun 10, 2024
@ct16k ct16k changed the title x/tools/gopls: panic analyzing dynamic call x/tools/deadcode: panic analyzing dynamic call Jun 10, 2024
@rsc
Copy link
Contributor

rsc commented Jun 10, 2024

/cc @adonovan

@rsc rsc added the NeedsFix The path to resolution is known, but the work has not been done. label Jun 10, 2024
@adonovan adonovan self-assigned this Jun 10, 2024
@adonovan
Copy link
Member

Thanks for the bug report. A nil Site means a reflective call, such as in this test case:

xtools$ cat a.go
package main

import "reflect"

func main() {
	reflect.ValueOf(foo).Call(nil)
}

func foo() {
	println("hi")
}

xtools$ callgraph ./a.go | grep foo
...
(reflect.Value).Call	--static-0:0-->	command-line-arguments.foo
runtime.send	--dynamic-327:9-->	command-line-arguments.foo
runtime.recv	--dynamic-707:9-->	command-line-arguments.foo

xtools$ deadcode -whylive=command-line-arguments.foo ./a.go 
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x38 pc=0x1004a54bc]

goroutine 1 gp=0x140000021c0 m=11 mp=0x1400030e808 [running]:
panic({0x1005de180?, 0x100967f20?})
	/Users/adonovan/w/goroot/src/runtime/panic.go:778 +0x154 fp=0x14000035540 sp=0x14000035490 pc=0x1000ba4c4
runtime.panicmem(...)
	/Users/adonovan/w/goroot/src/runtime/panic.go:261
runtime.sigpanic()
	/Users/adonovan/w/goroot/src/runtime/signal_unix.go:900 +0x300 fp=0x140000355a0 sp=0x14000035540 pc=0x1000f36f0
main.main()
	/Users/adonovan/go/pkg/mod/golang.org/x/[email protected]/cmd/deadcode/deadcode.go:263 +0x17bc fp=0x14000035f40 sp=0x140000355b0 pc=0x1004a54bc
runtime.main()
	/Users/adonovan/w/goroot/src/runtime/proc.go:270 +0x288 fp=0x14000035fd0 sp=0x14000035f40 pc=0x1000be3b8
runtime.goexit({})
	/Users/adonovan/w/goroot/src/runtime/asm_arm64.s:1223 +0x4 fp=0x14000035fd0 sp=0x14000035fd0 pc=0x1000f7844

I'll prepare a fix.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/591615 mentions this issue: cmd/deadcode: fix nil panic in Edge.Site.Pos

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gopls Issues related to the Go language server, gopls. NeedsFix The path to resolution is known, but the work has not been done. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

5 participants