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: crash exporting unsafe #60890

Open
rossb83 opened this issue Jun 18, 2023 · 7 comments
Open

x/tools/gopls: crash exporting unsafe #60890

rossb83 opened this issue Jun 18, 2023 · 7 comments
Assignees
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

@rossb83
Copy link

rossb83 commented Jun 18, 2023

gopls version: v0.12.2 (go1.20.4)
gopls flags: -logfile /home/user/gopls.log -rpc.trace
update flags: off
extension version: 0.38.0-upstream-0.1.7-uber
go version: 1.20.4
environment: Visual Studio Code linux
initialization error: undefined
issue timestamp: Sun, 18 Jun 2023 04:57:07 GMT
restart history:
Sun, 18 Jun 2023 04:56:23 GMT: activation (enabled: true)

ATTENTION: PLEASE PROVIDE THE DETAILS REQUESTED BELOW.

Describe what you observed.

panic: cannot export package unsafe

goroutine 2285 [running]:
golang.org/x/tools/internal/gcimporter.iexportCommon.func1()
	  iexport.go:93  0x8a
panic({0xd82ea0, 0x114f800})
	  panic.go:884  0x213
golang.org/x/tools/internal/gcimporter.(*iexporter).pushDecl(0xc004730000, {0x11617f8, 0xc000037270})
	  iexport.go:395  0xf6
golang.org/x/tools/internal/gcimporter.iexportCommon({0x1151100, 0xc002cd1a10}, 0xc004f8aa40, 0x0, 0x1, 0x2, {0xc005ff5ca8, 0x1, 0xc002cd1980%3F})
	  iexport.go:124  0x93a
golang.org/x/tools/internal/gcimporter.IExportShallow(0xdc6300%3F, 0xc000036230)
	  iexport.go:43  0x75
golang.org/x/tools/gopls/internal/lsp/cache.(*typeCheckBatch).checkPackage.func1()
	  check.go:646  0x2b1
created by golang.org/x/tools/gopls/internal/lsp/cache.(*typeCheckBatch).checkPackage
	  check.go:638  0x305
[Error - 4:56:54 AM] 

OPTIONAL: If you would like to share more information, you can attach your complete gopls logs.

NOTE: THESE MAY CONTAIN SENSITIVE INFORMATION ABOUT YOUR CODEBASE.
DO NOT SHARE LOGS IF YOU ARE WORKING IN A PRIVATE REPOSITORY.

<OPTIONAL: ATTACH LOGS HERE>

@findleyr findleyr changed the title gopls: automated issue report (crash) x/tools/gopls: crash exporting unsafe Jun 20, 2023
@findleyr
Copy link
Contributor

Thank you for the report.

This is an interesting crash. There is code explicitly guarding against exporting unsafe in this control flow, but it checks only the package ID, not package path, which could theoretically misidentify a variant of the "unsafe" package.. However, there should not be any variants of the "unsafe" package. An exception to this is PGO variants, but these were explicitly avoided in #60456, and in any case you are using go1.20.

So, I think I can probably fix this, but I don't know how to repro.

Can you share anything about the code you were editing when you encountered this crash?

Do you have anything interesting in your settings.json?

@findleyr findleyr transferred this issue from golang/vscode-go Jun 20, 2023
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Jun 20, 2023
@findleyr findleyr added this to the gopls/v0.12.3 milestone Jun 20, 2023
@gopherbot gopherbot added the gopls Issues related to the Go language server, gopls. label Jun 20, 2023
@gopherbot gopherbot modified the milestones: gopls/v0.12.3, Unreleased Jun 20, 2023
@findleyr findleyr self-assigned this Jun 20, 2023
@gopherbot
Copy link

Change https://go.dev/cl/504555 mentions this issue: gopls/internal/lsp/cache: guard against "unsafe" by package path, not ID

@gopherbot
Copy link

Change https://go.dev/cl/504556 mentions this issue: internal/gcimporter: avoid a crash when exporting a struct with no pkg

@findleyr findleyr modified the milestones: Unreleased, gopls/v0.12.3 Jun 20, 2023
gopherbot pushed a commit to golang/tools that referenced this issue Jun 20, 2023
The crash in golang/go#60890 suggests that a user encountered a variant
of the unsafe package. I'm not sure how to reproduce this, but in any
case we should be checking package path, not ID, when guarding against
exporting "unsafe".

For golang/go#60890

Change-Id: Ib6c546b8f74ba513f5ee3df09b5ba29cea0c1b85
Reviewed-on: https://go-review.googlesource.com/c/tools/+/504555
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
@findleyr
Copy link
Contributor

Absent a repro, I'm not sure if this is fixed, but I will optimistically close this. We can reopen if we get another report.

@JamyDev
Copy link

JamyDev commented Jul 14, 2023

@findleyr this was reproable in 0.12.4 for us, but not in latest master branch. Would it be possible to get a 0.12.5 or 0.13 release out? we're currently stuck in 0.11 land, and I'd like to take advantage of the perf updates in 0.12

@findleyr
Copy link
Contributor

@JamyDev we're aiming for 12.5 soon -- either next week or the week after. You can of course also install gopls at master.

Thanks for confirming that it is fixed at master.

@adonovan
Copy link
Member

adonovan commented Jan 2, 2024

Reopening since we hit an "encountered unsafe as %s" assertion.

This stack WyBEVw was reported by telemetry:

gopls/bug
golang.org/x/tools/gopls/internal/bug.report:35
golang.org/x/tools/gopls/internal/bug.Reportf:1
golang.org/x/tools/gopls/internal/lsp/cache.(*typeCheckBatch).getImportPackage:43
golang.org/x/tools/gopls/internal/lsp/cache.(*snapshot).forEachPackageInternal.func1:1
golang.org/x/sync/errgroup.(*Group).Go.func1:3
runtime.goexit:0
golang.org/x/tools/gopls@v0.14.2 go1.21.1 darwin/amd64 (1)

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

No branches or pull requests

5 participants