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

x/tools/gopls: nil pointer on rename #32974

Closed
mykter opened this issue Jul 8, 2019 · 4 comments
Closed

x/tools/gopls: nil pointer on rename #32974

mykter opened this issue Jul 8, 2019 · 4 comments
Assignees
Labels
Milestone

Comments

@mykter
Copy link

@mykter mykter commented Jul 8, 2019

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

$ go version
go version go1.12.6 linux/amd64

$ gopls version
golang.org/x/tools/gopls v0.1.1
    golang.org/x/tools/gopls@v0.1.2-0.20190706070813-72ffa07ba3db h1:jEzcdhR4g/qmRjnefU4bjKYInYeXwZRsQqqcnUrVSxk=

Does this issue reproduce with the latest release?

Of gopls, yes

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

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/mmacnair/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/mmacnair/go"
GOPROXY=""
GORACE=""
GOROOT="/snap/go/3947"
GOTMPDIR=""
GOTOOLDIR="/snap/go/3947/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/mmacnair/pkg/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 -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build501054748=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Tried to rename a type defined at package level

What did you expect to see?

The type and all references to it be renamed

What did you see instead?

No changes, and a crash:

[Error - 11:44:32 AM] Request textDocument/rename failed.
  Message: method "textDocument/rename" did not reply
  Code: -32603 
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x40 pc=0x8d92b9]

goroutine 3907 [running]:
golang.org/x/tools/internal/lsp/source.(*renamer).checkInPackageBlock(0xc0005acc60, 0xb9a8a0, 0xc005fe2af0)
	/home/mmacnair/go/pkg/mod/golang.org/x/tools@v0.0.0-20190706070813-72ffa07ba3db/internal/lsp/source/rename_check.go:116 +0xf9
golang.org/x/tools/internal/lsp/source.(*renamer).check(0xc0005acc60, 0xb9a8a0, 0xc005fe2af0)
	/home/mmacnair/go/pkg/mod/golang.org/x/tools@v0.0.0-20190706070813-72ffa07ba3db/internal/lsp/source/rename_check.go:42 +0x39c
golang.org/x/tools/internal/lsp/source.(*IdentifierInfo).Rename(0xc004182410, 0xb8f900, 0xc0061f8de0, 0xc0057928a4, 0x4, 0x0, 0x0, 0x0)
	/home/mmacnair/go/pkg/mod/golang.org/x/tools@v0.0.0-20190706070813-72ffa07ba3db/internal/lsp/source/rename.go:76 +0x70b
golang.org/x/tools/internal/lsp.(*Server).rename(0xc000136d20, 0xb8f900, 0xc0061f8de0, 0xc0061f8e10, 0x0, 0x0, 0xc000542140)
	/home/mmacnair/go/pkg/mod/golang.org/x/tools@v0.0.0-20190706070813-72ffa07ba3db/internal/lsp/rename.go:34 +0x295
golang.org/x/tools/internal/lsp.(*Server).Rename(0xc000136d20, 0xb8f900, 0xc0061f8de0, 0xc0061f8e10, 0xc0061f8e10, 0x0, 0x0)
	/home/mmacnair/go/pkg/mod/golang.org/x/tools@v0.0.0-20190706070813-72ffa07ba3db/internal/lsp/server.go:244 +0x49
golang.org/x/tools/internal/lsp/protocol.serverHandler.func1(0xb8f900, 0xc0061f8de0, 0xc0060bd5c0)
	/home/mmacnair/go/pkg/mod/golang.org/x/tools@v0.0.0-20190706070813-72ffa07ba3db/internal/lsp/protocol/tsserver.go:415 +0x1291
golang.org/x/tools/internal/jsonrpc2.(*Conn).Run.func1(0xc00002acc0, 0xc0060bd5c0, 0xc00016e3f0, 0xb8f900, 0xc0061f8de0, 0xc00600a960, 0xc000b029f0)
	/home/mmacnair/go/pkg/mod/golang.org/x/tools@v0.0.0-20190706070813-72ffa07ba3db/internal/jsonrpc2/jsonrpc2.go:441 +0x117
created by golang.org/x/tools/internal/jsonrpc2.(*Conn).Run
	/home/mmacnair/go/pkg/mod/golang.org/x/tools@v0.0.0-20190706070813-72ffa07ba3db/internal/jsonrpc2/jsonrpc2.go:428 +0x6fc

Unfortunately the code isn't public and I don't have a minimal reproducer. I did do a quick test of adding a new package level type and variable definition, that weren't used anywhere else, and got the same result for each of them - the stack trace ends on the same line.

If this isn't enough info to diagnose, feel free to close it. I'm sure someone else will hit this at some point with an open source project!

@gopherbot gopherbot added this to the Unreleased milestone Jul 8, 2019
@gopherbot gopherbot added the gopls label Jul 8, 2019
@suzmue suzmue self-assigned this Jul 8, 2019
@suzmue

This comment has been minimized.

Copy link
Contributor

@suzmue suzmue commented Jul 8, 2019

@mykter Are both the use and the type definition in the same package? Could you also post the line declaring the package level type and a variable definition? Thanks!

@suzmue

This comment has been minimized.

Copy link
Contributor

@suzmue suzmue commented Jul 9, 2019

Do you have any test files? If so, https://go-review.googlesource.com/c/tools/+/185277 should fix it.

The following produces a similar trace:

Rename x
hello.go

package hello

var x int

hello_test.go

package hello

error:

[Error - 7:45:50 PM] Request textDocument/rename failed.
  Message: method "textDocument/rename" did not reply
  Code: -32603 
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x40 pc=0x14d8f69]

goroutine 1823 [running]:
golang.org/x/tools/internal/lsp/source.(*renamer).checkInPackageBlock(0xc001f60840, 0x179acc0, 0xc001d15270)
	/Users/suzmue/go/pkg/mod/golang.org/x/tools@v0.0.0-20190708203411-c8855242db9c/internal/lsp/source/rename_check.go:116 +0xf9
golang.org/x/tools/internal/lsp/source.(*renamer).check(0xc001f60840, 0x179acc0, 0xc001d15270)
	/Users/suzmue/go/pkg/mod/golang.org/x/tools@v0.0.0-20190708203411-c8855242db9c/internal/lsp/source/rename_check.go:42 +0x39c
golang.org/x/tools/internal/lsp/source.(*IdentifierInfo).Rename(0xc0015eb1e0, 0x178fb80, 0xc001ac4d20, 0xc003e5b1c8, 0x2, 0x0, 0x0, 0x0)
	/Users/suzmue/go/pkg/mod/golang.org/x/tools@v0.0.0-20190708203411-c8855242db9c/internal/lsp/source/rename.go:76 +0x70b
golang.org/x/tools/internal/lsp.(*Server).rename(0xc00022a000, 0x178fb80, 0xc001ac4d20, 0xc0018d14a0, 0x0, 0x0, 0xc001eebb80)
	/Users/suzmue/go/pkg/mod/golang.org/x/tools@v0.0.0-20190708203411-c8855242db9c/internal/lsp/rename.go:34 +0x295
golang.org/x/tools/internal/lsp.(*Server).Rename(0xc00022a000, 0x178fb80, 0xc001ac4d20, 0xc0018d14a0, 0xc0018d14a0, 0x0, 0x0)
	/Users/suzmue/go/pkg/mod/golang.org/x/tools@v0.0.0-20190708203411-c8855242db9c/internal/lsp/server.go:244 +0x49
golang.org/x/tools/internal/lsp/protocol.serverHandler.func1(0x178fb80, 0xc001ac4d20, 0xc002593f20)
	/Users/suzmue/go/pkg/mod/golang.org/x/tools@v0.0.0-20190708203411-c8855242db9c/internal/lsp/protocol/tsserver.go:415 +0x1291
golang.org/x/tools/internal/jsonrpc2.(*Conn).Run.func1(0xc003e4a7e0, 0xc002593f20, 0xc00022c000, 0x178fb80, 0xc001ac4d20, 0xc001d15b80, 0xc000e21da0)
	/Users/suzmue/go/pkg/mod/golang.org/x/tools@v0.0.0-20190708203411-c8855242db9c/internal/jsonrpc2/jsonrpc2.go:441 +0x117
created by golang.org/x/tools/internal/jsonrpc2.(*Conn).Run
	/Users/suzmue/go/pkg/mod/golang.org/x/tools@v0.0.0-20190708203411-c8855242db9c/internal/jsonrpc2/jsonrpc2.go:428 +0x6fc
@mykter

This comment has been minimized.

Copy link
Author

@mykter mykter commented Jul 9, 2019

I do have tests for that package, yes - looks like you've found it!
Thanks.

@stamblerre stamblerre changed the title x/tools/cmd/gopls: Crash on rename x/tools/cmd/gopls: nil pointer on rename Jul 9, 2019
@stamblerre stamblerre changed the title x/tools/cmd/gopls: nil pointer on rename x/tools/gopls: nil pointer on rename Jul 9, 2019
@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Jul 9, 2019

Change https://golang.org/cl/185277 mentions this issue: internal/lsp: rename identifiers in test packages

movie-travel-code added a commit to movie-travel-code/tools that referenced this issue Jul 11, 2019
Support renaming of identifiers in test packages. The packages for
all of the references must be checked and the changes need to be
deduped, since both a package and its test package contain some of the
same files.

Fixes golang/go#32974

Change-Id: Ie51e19716faae77ce7e5254eeb3956faa42c2a09
Reviewed-on: https://go-review.googlesource.com/c/tools/+/185277
Run-TryBot: Suzy Mueller <suzmue@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.