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: SIGSEGV when semantic token provider is enabled and typing after return of a void function #65952

Closed
patrickpichler opened this issue Feb 27, 2024 · 5 comments
Labels
gopls/telemetry-wins gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@patrickpichler
Copy link

patrickpichler commented Feb 27, 2024

gopls version

v0.15.0

go env

GO111MODULE=''
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/patrickp/Library/Caches/go-build'
GOENV='/Users/patrickp/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/patrickp/.asdf/installs/golang/1.21.6/packages/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/patrickp/.asdf/installs/golang/1.21.6/packages'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/Users/patrickp/.asdf/installs/golang/1.21.6/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/Users/patrickp/.asdf/installs/golang/1.21.6/go/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.21.6'
GCCGO='gccgo'
AR='ar'
CC='clang'
CXX='clang++'
CGO_ENABLED='1'
GOMOD='/Users/patrickp/tmp/go-playground/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 arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/cp/jgzd29l5179c6nbllv2dx9dm0000gn/T/go-build4278098802=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

  • Ensure semantic token provider capability is enabled in LSP
  • Create a main.go file with the following content
func main() {
  return
}
  • Position cursor after return and start typing

What did you see happen?

LSP crashes with a SIGSEGV

goroutine 22695 [running]:
golang.org/x/tools/gopls/internal/golang.highlightFuncControlFlow({0x140004a6180, 0x5, 0x9?}, 0x14009dce4b0)
        /Users/patrickp/.asdf/installs/golang/1.21.6/packages/pkg/mod/golang.org/x/tools/gopls@v0.15.0/internal/golang/highlight.go:241 +0x350
golang.org/x/tools/gopls/internal/golang.highlightPath({0x140004a6180, 0x5, 0x8}, 0x140124e71d0?, 0x1400dedd7c0)
        /Users/patrickp/.asdf/installs/golang/1.21.6/packages/pkg/mod/golang.org/x/tools/gopls@v0.15.0/internal/golang/highlight.go:102 +0x274
golang.org/x/tools/gopls/internal/golang.Highlight({0x103726928?, 0x14009d02e10?}, 0x14001865440?, {0x103727be0, 0x14001865440}, {0x2760480?, 0x140?})
        /Users/patrickp/.asdf/installs/golang/1.21.6/packages/pkg/mod/golang.org/x/tools/gopls@v0.15.0/internal/golang/highlight.go:53 +0x208
golang.org/x/tools/gopls/internal/server.(*server).DocumentHighlight(0x14002760480?, {0x103726960, 0x14012c58870}, 0x14009d02bd0)
        /Users/patrickp/.asdf/installs/golang/1.21.6/packages/pkg/mod/golang.org/x/tools/gopls@v0.15.0/internal/server/highlight.go:32 +0x198
golang.org/x/tools/gopls/internal/protocol.serverDispatch({0x103726960, 0x14012c58870}, {0x103737d08, 0x14000f0da00}, 0x14009d02b70, {0x103726b20, 0x1400baf1000})
        /Users/patrickp/.asdf/installs/golang/1.21.6/packages/pkg/mod/golang.org/x/tools/gopls@v0.15.0/internal/protocol/tsserver.go:376 +0x4dd8
golang.org/x/tools/gopls/internal/lsprpc.(*streamServer).ServeStream.ServerHandler.func3({0x103726960, 0x14012c58870}, 0x14009d02b70, {0x103726b20, 0x1400baf1000})
        /Users/patrickp/.asdf/installs/golang/1.21.6/packages/pkg/mod/golang.org/x/tools/gopls@v0.15.0/internal/protocol/protocol.go:160 +0x74
golang.org/x/tools/gopls/internal/lsprpc.(*streamServer).ServeStream.handshaker.func4({0x103726960, 0x14012c58870}, 0x14009d02b70, {0x103726b20?, 0x1400baf1000?})
        /Users/patrickp/.asdf/installs/golang/1.21.6/packages/pkg/mod/golang.org/x/tools/gopls@v0.15.0/internal/lsprpc/lsprpc.go:509 +0x6d0
golang.org/x/tools/gopls/internal/protocol.Handlers.MustReplyHandler.func1({0x103726960, 0x14012c58870}, 0x1400b345308, {0x103726b20?, 0x1400baf1000?})
        /Users/patrickp/.asdf/installs/golang/1.21.6/packages/pkg/mod/golang.org/x/tools@v0.18.1-0.20240221145400-a220b3b5ba60/internal/jsonrpc2/handler.go:35 +0xdc
golang.org/x/tools/gopls/internal/protocol.Handlers.AsyncHandler.func2.2()
        /Users/patrickp/.asdf/installs/golang/1.21.6/packages/pkg/mod/golang.org/x/tools@v0.18.1-0.20240221145400-a220b3b5ba60/internal/jsonrpc2/handler.go:103 +0x90
created by golang.org/x/tools/gopls/internal/protocol.Handlers.AsyncHandler.func2 in goroutine 92
        /Users/patrickp/.asdf/installs/golang/1.21.6/packages/pkg/mod/golang.org/x/tools@v0.18.1-0.20240221145400-a220b3b5ba60/internal/jsonrpc2/handler.go:100 +0x1d0

What did you expect to see?

LSP not crashing

Editor and settings

No response

Logs

No response

Duplicates:

This stack aeuFyQ was reported by telemetry:

highlightFuncControlFlow:+93 is

for _, field := range funcType.Results.List {
crash/crash
runtime.gopanic:+69
runtime.panicmem:=261
runtime.sigpanic:+19
golang.org/x/tools/gopls/internal/golang.highlightFuncControlFlow:+93
golang.org/x/tools/gopls/internal/golang.highlightPath:+32
golang.org/x/tools/gopls/internal/golang.Highlight:+31
golang.org/x/tools/gopls/internal/server.(*server).DocumentHighlight:+14
golang.org/x/tools/gopls/internal/protocol.serverDispatch:+280
golang.org/x/tools/gopls/internal/lsprpc.(*streamServer).ServeStream.ServerHandler.func3:+5
golang.org/x/tools/gopls/internal/lsprpc.(*streamServer).ServeStream.handshaker.func4:+52
golang.org/x/tools/gopls/internal/protocol.Handlers.MustReplyHandler.func1:+2
golang.org/x/tools/gopls/internal/protocol.Handlers.AsyncHandler.func2.2:+3
runtime.goexit:+0
golang.org/x/tools/gopls@v0.15.0 devel darwin/arm64 vscode (2)

Issue created by golang.org/x/tools/gopls/internal/telemetry/cmd/stacks.

Also: oF1UWw

@patrickpichler patrickpichler added gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository. labels Feb 27, 2024
@gopherbot gopherbot added this to the Unreleased milestone Feb 27, 2024
patrickpichler added a commit to patrickpichler/go-tools that referenced this issue Feb 27, 2024
The result of `funcType` will be `nil`, if a function does not return
any values. This caused an `SIGSEGV` before.

fixes golang/go#65952
@gopherbot
Copy link

Change https://go.dev/cl/567275 mentions this issue: gopls: add non nil if check around function result highlight

patrickpichler pushed a commit to patrickpichler/go-tools that referenced this issue Feb 27, 2024
The result of `funcType` will be `nil`, if a function does not return
any values. This caused an `SIGSEGV` before.

fixes golang/go#65952
patrickpichler pushed a commit to patrickpichler/go-tools that referenced this issue Feb 27, 2024
The result of funcType will be nil, if a function does not return
any values. This caused an SIGSEGV before.

Fixes golang/go#65952
patrickpichler pushed a commit to patrickpichler/go-tools that referenced this issue Feb 27, 2024
The result of funcType will be nil, if a function does not return
any values. This caused an SIGSEGV before.

Fixes golang/go#65952
patrickpichler pushed a commit to patrickpichler/go-tools that referenced this issue Feb 27, 2024
The result of funcType will be nil, if a function does not return
any values. This caused an SIGSEGV before.

Fixes golang/go#65952
@gopherbot
Copy link

Change https://go.dev/cl/567256 mentions this issue: gopls/internal/golang: fix NPE when in control flow highlighting

@gopherbot
Copy link

Change https://go.dev/cl/567415 mentions this issue: [gopls-release-branch.0.15] gopls: add non nil if check around function result highlight

@findleyr findleyr modified the milestones: Unreleased, gopls/v0.15.1 Feb 27, 2024
gopherbot pushed a commit to golang/tools that referenced this issue Feb 27, 2024
…on result highlight

The result of funcType will be nil, if a function does not return any
values. This caused an SIGSEGV before.

Fixes golang/go#65952

Change-Id: Ibf4ac3070744f42033504220f05b35a78c97d992
GitHub-Last-Rev: 74182b2
GitHub-Pull-Request: #480
Reviewed-on: https://go-review.googlesource.com/c/tools/+/567275
Reviewed-by: Mauri de Souza Meneguzzo <mauri870@gmail.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
(cherry picked from commit c1f340a)
Reviewed-on: https://go-review.googlesource.com/c/tools/+/567415
Reviewed-by: Alan Donovan <adonovan@google.com>
gopherbot pushed a commit to golang/tools that referenced this issue Feb 27, 2024
Add a test for the fix in golang/go#65952: a nil pointer exception when
highlighting a return value in a function returning no results.

Also, merge tests related to control flow highlighting, since it is
convenient to be able to run them together, and since there is
nontrivial overhead to tiny tests.

Updates golang/go#65952

Change-Id: Ibf8c7c6f0f4feed6dc7a283736bc038600a0bf04
Reviewed-on: https://go-review.googlesource.com/c/tools/+/567256
Reviewed-by: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Robert Findley <rfindley@google.com>
@gopherbot
Copy link

Change https://go.dev/cl/567417 mentions this issue: [gopls-release-branch.0.15] gopls/internal/test: add test for NPE in control flow highlighting

gopherbot pushed a commit to golang/tools that referenced this issue Feb 27, 2024
…control flow highlighting

Add a test for the fix in golang/go#65952: a nil pointer exception when
highlighting a return value in a function returning no results.

Also, merge tests related to control flow highlighting, since it is
convenient to be able to run them together, and since there is
nontrivial overhead to tiny tests.

Updates golang/go#65952

Change-Id: Ibf8c7c6f0f4feed6dc7a283736bc038600a0bf04
Reviewed-on: https://go-review.googlesource.com/c/tools/+/567256
Reviewed-by: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Robert Findley <rfindley@google.com>
(cherry picked from commit fc70354)
Reviewed-on: https://go-review.googlesource.com/c/tools/+/567417
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Mar 3, 2024
0.15.1

This release fixes golang/go#65952, a crash in document highlighting when the cursor is in a return value for a function that has no results, such as the following example:

func f() { // <-- no results
   return 0| // <-- cursor at '|'
}

Thanks very much to @patrickpichler who both reported and fixed this bug!

We're hopeful that once Go 1.23 is released, the opt-in automated crash reporting added in gopls v0.15.0 will increase the likelihood that these types of crashes are caught before they are released.

0.15.0

This release introduces "zero config" gopls workspaces, which is a set of heuristics allowing gopls to Do The Right Thing when you open a Go file. We believe this addresses two of the largest pain points we hear about from our users: difficulty configuring multi-module repositories, and working on multiple GOOS/GOARCH combinations. However, this is a large change to the way gopls models your workspace, and the dynamic loading/unloading of builds may be surprising in some cases. Your feedback on this new feature is greatly appreciated. See below for more details.

New Features

Simpler workspace configuration and improved build tag support
Preview refactoring edits
Analysis & diagnostics
Automated crash reporting (off by default)
Housekeeping

and Bug Fixes.
@adonovan
Copy link
Member

adonovan commented Mar 6, 2024

This stack oF1UWw was reported by telemetry:

crash/crash
runtime.gopanic:+69
runtime.panicmem:=261
runtime.sigpanic:+19
golang.org/x/tools/gopls/internal/golang.highlightFuncControlFlow:+93
golang.org/x/tools/gopls/internal/golang.highlightPath:+27
golang.org/x/tools/gopls/internal/golang.Highlight:+31
golang.org/x/tools/gopls/internal/server.(*server).DocumentHighlight:+14
golang.org/x/tools/gopls/internal/protocol.serverDispatch:+280
golang.org/x/tools/gopls/internal/lsprpc.(*streamServer).ServeStream.ServerHandler.func3:+5
golang.org/x/tools/gopls/internal/lsprpc.(*streamServer).ServeStream.handshaker.func4:+52
golang.org/x/tools/gopls/internal/protocol.Handlers.MustReplyHandler.func1:+2
golang.org/x/tools/gopls/internal/protocol.Handlers.AsyncHandler.func2.2:+3
runtime.goexit:+0
golang.org/x/tools/gopls@v0.15.0 devel linux/amd64 coc.nvim (3)

Issue created by golang.org/x/tools/gopls/internal/telemetry/cmd/stacks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gopls/telemetry-wins gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants