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: using overlay in GOPACKAGESDRIVER fails or panics #33727

Open
kylelemons opened this issue Aug 20, 2019 · 4 comments

Comments

@kylelemons
Copy link
Contributor

commented Aug 20, 2019

I can't seem to figure out how to prepare the results from my GOPACKAGESDRIVER for gopls to correctly use them. I am using overlay to simultaneously inject my own new packages and to zero out (i.e. replace with package pkgname) any files that are not in my overlay to try to get autocompletion working for TinyGo-based Go programs (which have their own "machine" "runtime" etc packages). In the course of this, I accidentally managed to get gopls to crash. While the input is almost certainly incorrect in some way, it seems like gopls would prefer not tp allow malformed input (from either Go source or GOPACKAGESDRIVERs) to cause it to panic.

What did you do?

I am working on trying to get a go/packages driver working for TinyGo. My package driver performs overlays before calling out to packages.Load. If I do this, then I see errors about not being able to open files under $GOROOT that aren't actually there, they're in an overlay. So, I decided to translate the GoFiles, CompiledGoFiles, etc back to the original files on their way back to gopls. This only made things even more interesting :)

What did you expect to see?

Gopls should parse the remapped files (maybe), or at least report an error message if some information is not available.

What did you see instead?

gopls crashes with a stack trace:

panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xc0000005 code=0x0 addr=0x0 pc=0x674aa3]

goroutine 2200 [running]:
go/types.(*StdSizes).Sizeof(0x0, 0xbbd840, 0xfedbe0, 0xc000d5bcc0)
	c:/go/src/go/types/sizes.go:154 +0x1c3
golang.org/x/tools/go/analysis/passes/atomicalign.run(0xc003c8a960, 0xc0017a7f00, 0x0, 0xc003ca22b8, 0x1)
	C:/Users/kyle/dev/go/src/golang.org/x/tools/go/analysis/passes/atomicalign/atomicalign.go:30 +0x69
golang.org/x/tools/internal/lsp/source.(*Action).execOnce(0xc0027d03c0, 0xbc6a80, 0xc003c89900, 0xc0001dcb00, 0xc000d5bec8, 0x10)
	C:/Users/kyle/dev/go/src/golang.org/x/tools/internal/lsp/source/analysis.go:174 +0x812
golang.org/x/tools/internal/lsp/source.(*Action).exec.func1()
	C:/Users/kyle/dev/go/src/golang.org/x/tools/internal/lsp/source/analysis.go:108 +0x55
sync.(*Once).Do(0xc0027d03c0, 0xc000d5bf08)
	c:/go/src/sync/once.go:44 +0xba
golang.org/x/tools/internal/lsp/source.(*Action).exec(0xc0027d03c0, 0xbc6a80, 0xc003c89900, 0xc0001dcb00, 0xbc7380, 0xc000d5bf90)
	C:/Users/kyle/dev/go/src/golang.org/x/tools/internal/lsp/source/analysis.go:107 +0x92
golang.org/x/tools/internal/lsp/source.execAll.func1(0xc000000008, 0xb0d268)
	C:/Users/kyle/dev/go/src/golang.org/x/tools/internal/lsp/source/analysis.go:99 +0x4f
golang.org/x/sync/errgroup.(*Group).Go.func1(0xc003c7f8f0, 0xc003ca67c0)
	C:/Users/kyle/dev/go/src/golang.org/x/sync/errgroup/errgroup.go:57 +0x5e
created by golang.org/x/sync/errgroup.(*Group).Go
	C:/Users/kyle/dev/go/src/golang.org/x/sync/errgroup/errgroup.go:54 +0x6d

This corresponds to the if statement below in analysis.go:

func run(pass *analysis.Pass) (interface{}, error) {
	if 8*pass.TypesSizes.Sizeof(types.Typ[types.Uintptr]) == 64 {
		return nil, nil // 64-bit platform
	}

I checked the results that are getting sent back, and they do set TypesSizes:

{ "WordSize": 4, "MaxAlign": 4 }

If I forcibly return from that analysis, I get a different failure further down the line:

[Trace - 12:07:16 AM] Sending notification 'textDocument/documentSymbol' in 3633ms.
Params: {"textDocument":{"uri":"file:///c%3A/Users/kyle/dev/tinygo/tests/gameboy-advance/registeroffsets/registeroffsets.go"}}


panic: interface conversion: types.Object is nil, not *types.Func

goroutine 2323 [running]:
golang.org/x/tools/go/analysis/passes/ctrlflow.run.func1(0xbbd6c0, 0xc00210c7b0)
	C:/Users/kyle/dev/xtools/go/analysis/passes/ctrlflow/ctrlflow.go:105 +0x39f
golang.org/x/tools/go/ast/inspector.(*Inspector).Preorder(0xc001896660, 0xc000d59c70, 0x2, 0x2, 0xc000d59c90)
	C:/Users/kyle/dev/xtools/go/ast/inspector/inspector.go:77 +0xa6
golang.org/x/tools/go/analysis/passes/ctrlflow.run(0xc00336a6e0, 0xc0020ed000, 0x0, 0xc00343e000, 0x223)
	C:/Users/kyle/dev/xtools/go/analysis/passes/ctrlflow/ctrlflow.go:102 +0x1a8
golang.org/x/tools/internal/lsp/source.(*Action).execOnce(0xc00327f080, 0xbc7080, 0xc0016fab80, 0xc00002b940, 0xc000d59ec8, 0x10)
	C:/Users/kyle/dev/xtools/internal/lsp/source/analysis.go:174 +0x812
golang.org/x/tools/internal/lsp/source.(*Action).exec.func1()
	C:/Users/kyle/dev/xtools/internal/lsp/source/analysis.go:108 +0x55
sync.(*Once).Do(0xc00327f080, 0xc000d59f08)
	c:/go/src/sync/once.go:44 +0xba
golang.org/x/tools/internal/lsp/source.(*Action).exec(0xc00327f080, 0xbc7080, 0xc0016fab80, 0xc00002b940, 0xc0019acb80, 0xc000d59f90)
	C:/Users/kyle/dev/xtools/internal/lsp/source/analysis.go:107 +0x92
golang.org/x/tools/internal/lsp/source.execAll.func1(0x8, 0xb0db68)
	C:/Users/kyle/dev/xtools/internal/lsp/source/analysis.go:99 +0x4f
golang.org/x/sync/errgroup.(*Group).Go.func1(0xc0007d69f0, 0xc001b69140)
	C:/Users/kyle/dev/go/pkg/mod/golang.org/x/sync@v0.0.0-20190423024810-112230192c58/errgroup/errgroup.go:57 +0x5e
created by golang.org/x/sync/errgroup.(*Group).Go
	C:/Users/kyle/dev/go/pkg/mod/golang.org/x/sync@v0.0.0-20190423024810-112230192c58/errgroup/errgroup.go:54 +0x6d

Build info

version v0.1.3, built in $GOPATH mode

Go info

go version go1.12.7 windows/amd64

set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\kyle\AppData\Local\go-build
set GOEXE=.exe
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOOS=windows
set GOPATH=C:\Users\kyle\dev\go
set GOPROXY=
set GORACE=
set GOROOT=c:\go
set GOTMPDIR=
set GOTOOLDIR=c:\go\pkg\tool\windows_amd64
set GCCGO=gccgo
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=C:\Users\kyle\dev\tinygo\go.mod
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=C:\Users\kyle\AppData\Local\Temp\go-build437939018=/tmp/go-build -gno-record-gcc-switches
@gopherbot

This comment has been minimized.

Copy link

commented Aug 20, 2019

Thank you for filing a gopls issue! Please take a look at the Troubleshooting section of the gopls Wiki page, and make sure that you have provided all of the relevant information here.

@gopherbot gopherbot added the gopls label Aug 20, 2019

@stamblerre

This comment has been minimized.

Copy link
Contributor

commented Aug 20, 2019

I agree that we should probably handle nil pointers in go/analysis a bit better, as these do seem to happen fairly often.

However, I'm not sure on how to help with the overlay issue - is there a way we can reproduce this to help you investigate? I'm not surprised that gopls would crash with a replaced standard library, but if you could report the issues you saw with the files not being found, that would be helpful as well.

@gopherbot

This comment has been minimized.

Copy link

commented Aug 20, 2019

Change https://golang.org/cl/190908 mentions this issue: go/analysis: handle common nil pointers

@odeke-em odeke-em changed the title gopls: using overlay in GOPACKAGESDRIVER fails or panics x/tools/gopls: using overlay in GOPACKAGESDRIVER fails or panics Aug 21, 2019

@gopherbot gopherbot added this to the Unreleased milestone Aug 21, 2019

gopherbot pushed a commit to golang/tools that referenced this issue Aug 24, 2019
go/analysis: handle common nil pointers
Updates golang/go#33727, golang/go#33689

Change-Id: Ie32ac4efc9fe0d7b08fcff3feb44b28d83df942d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/190908
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
@kylelemons

This comment has been minimized.

Copy link
Contributor Author

commented Aug 26, 2019

I've added an example of this as kylelemons/gopls33727 -- though along the way, it looks like the panic that I'm getting might not actually relate to the overlay. If you comment out the overlay environment variable and replace MaxInt with something that exists like Max, I still see the same panic.

EDIT: If I patch analysis.go like this, my demo actually works:

        file = file:///C:/Users/kyle/dev/gopls33727/gopath/packagepath/main.go
C:\Users\kyle\dev\gopls33727\overlay\src\math\intmath.go:6:6-12: defined here as func math.MaxInt(a int, b int) int

@gopherbot gopherbot added the Tools label Sep 12, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.