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.

Sign up for GitHub

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

Jump to bottom

x/tools/gopls: getImportPackage crash (PackagePath == "unsafe", id != "unsafe") #60890

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

x/tools/gopls: getImportPackage crash (PackagePath == "unsafe", id != "unsafe") #60890

rossb83 opened this issue Jun 18, 2023 · 17 comments
Assignees
@findleyr
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
gopls/v0.17.0

Comments

@rossb83
Copy link

rossb83 commented Jun 18, 2023 •
edited by adonovan

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>

Dups: WyBEVw 8IUuxg gGL_Og s38AiQ g1WTiA kaOGFA M1Fsyg eMTo4g 4jVSEw
@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
@findleyr
gopls/internal/lsp/cache: guard against "unsafe" by package path, not ID
9960b69 
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.

@adonovan adonovan reopened this Jan 2, 2024
@adonovan
Copy link
Member

This stack 8IUuxg 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.(*typeCheckBatch).typesConfig.func1:20
golang.org/x/tools/gopls/internal/lsp/cache.importerFunc.Import:0
go/types.(*Checker).importPackage:28
go/types.(*Checker).collectObjects.func1:13
go/types.(*Checker).walkDecl:9
go/types.(*Checker).walkDecls:?399
go/types.(*Checker).collectObjects:42
go/types.(*Checker).checkFiles:29
go/types.(*Checker).Files:?372
golang.org/x/tools/gopls/internal/lsp/cache.(*typeCheckBatch).checkPackageForImport:50
golang.org/x/tools/gopls/internal/lsp/cache.(*typeCheckBatch).getImportPackage:50
golang.org/x/tools/gopls/internal/lsp/cache.(*typeCheckBatch).typesConfig.func1:20
golang.org/x/tools/gopls/internal/lsp/cache.importerFunc.Import:0

golang.org/x/tools/gopls@v0.14.2 go1.22.0 darwin/arm64 vscode (1)

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

@adonovan
Copy link
Member

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.22.0 darwin/arm64 vscode (13)

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

@adonovan
Copy link
Member

adonovan commented Mar 9, 2024

This stack gGL_Og was reported by telemetry:

gopls/bug
golang.org/x/tools/gopls/internal/util/bug.report:+35
golang.org/x/tools/gopls/internal/util/bug.Reportf:+1
golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).getImportPackage:+43
golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).typesConfig.func1:+20
golang.org/x/tools/gopls/internal/cache.importerFunc.Import:+0
go/types.(*Checker).importPackage:+28
go/types.(*Checker).collectObjects.func1:+13
go/types.(*Checker).walkDecl:+9
go/types.(*Checker).walkDecls:=399
go/types.(*Checker).collectObjects:+42
go/types.(*Checker).checkFiles:+29
go/types.(*Checker).Files:=372
golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).checkPackageForImport:+50
golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).getImportPackage:+50
golang.org/x/tools/gopls/internal/cache.(*Snapshot).forEachPackageInternal.func1:+1
golang.org/x/sync/errgroup.(*Group).Go.func1:+3

golang.org/x/tools/gopls@v0.15.0 go1.22.0 darwin/arm64 vscode (1)
golang.org/x/tools/gopls@v0.15.1 go1.22.0 darwin/arm64 other,vscode (1)

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

@adonovan
Copy link
Member

adonovan commented Mar 9, 2024

This stack s38AiQ was reported by telemetry:

gopls/bug
golang.org/x/tools/gopls/internal/util/bug.report:+35
golang.org/x/tools/gopls/internal/util/bug.Reportf:+1
golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).getImportPackage:+43
golang.org/x/tools/gopls/internal/cache.(*Snapshot).forEachPackageInternal.func1:+1
golang.org/x/sync/errgroup.(*Group).Go.func1:+3
runtime.goexit:+0

golang.org/x/tools/gopls@v0.15.1 go1.22.0 darwin/arm64 other,vscode (6)

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

@adonovan
Copy link
Member

This stack g1WTiA was reported by telemetry:

gopls/bug
golang.org/x/tools/gopls/internal/util/bug.report:+35
golang.org/x/tools/gopls/internal/util/bug.Reportf:+1
golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).getImportPackage:+43
golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).typesConfig.func1:+20
golang.org/x/tools/gopls/internal/cache.importerFunc.Import:+0
go/types.(*Checker).importPackage:+28
go/types.(*Checker).collectObjects.func1:+13
go/types.(*Checker).walkDecl:+9
go/types.(*Checker).walkDecls:=391
go/types.(*Checker).collectObjects:+42
go/types.(*Checker).checkFiles:+32
go/types.(*Checker).Files:=341
golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).checkPackageForImport:+50
golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).getImportPackage:+50
golang.org/x/tools/gopls/internal/cache.(*Snapshot).forEachPackageInternal.func1:+1
golang.org/x/sync/errgroup.(*Group).Go.func1:+3

golang.org/x/tools/gopls@v0.15.1 go1.21.7 darwin/arm64 vscode (1)

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

@adonovan
Copy link
Member

This stack kaOGFA was reported by telemetry:

gopls/bug
golang.org/x/tools/gopls/internal/util/bug.report:+35
golang.org/x/tools/gopls/internal/util/bug.Reportf:+1
golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).getImportPackage:+43
golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).typesConfig.func1:+20
golang.org/x/tools/gopls/internal/cache.importerFunc.Import:+0
go/types.(*Checker).importPackage:+28
go/types.(*Checker).collectObjects.func1:+13
go/types.(*Checker).walkDecl:+9
go/types.(*Checker).walkDecls:=391
go/types.(*Checker).collectObjects:+42
go/types.(*Checker).checkFiles:+32
go/types.(*Checker).Files:=341
golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).checkPackageForImport:+50
golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).getImportPackage:+50
golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).typesConfig.func1:+20
golang.org/x/tools/gopls/internal/cache.importerFunc.Import:+0

golang.org/x/tools/gopls@v0.15.2 go1.21.8 darwin/arm64 other,vscode (1)

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

@adonovan
Copy link
Member

This stack M1Fsyg was reported by telemetry:

gopls/bug
golang.org/x/tools/gopls/internal/util/bug.report:+35
golang.org/x/tools/gopls/internal/util/bug.Reportf:+1
golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).getImportPackage:+43
golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).typesConfig.func1:+20
golang.org/x/tools/gopls/internal/cache.importerFunc.Import:+0
go/types.(*Checker).importPackage:+28
go/types.(*Checker).collectObjects.func1:+13
go/types.(*Checker).walkDecl:+9
go/types.(*Checker).walkDecls:=399
go/types.(*Checker).collectObjects:+42
go/types.(*Checker).checkFiles:+29
go/types.(*Checker).Files:=372
golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).checkPackageForImport:+50
golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).getImportPackage:+50
golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).typesConfig.func1:+20
golang.org/x/tools/gopls/internal/cache.importerFunc.Import:+0

golang.org/x/tools/gopls@v0.15.2 go1.22.0 darwin/arm64 other,vscode (1)

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

@adonovan adonovan changed the title x/tools/gopls: crash exporting unsafe x/tools/gopls: getImportPackage crash (PackagePath == "unsafe", id != "unsafe") Mar 21, 2024
@adonovan
Copy link
Member

This stack eMTo4g was reported by telemetry:

gopls/bug
golang.org/x/tools/gopls/internal/util/bug.report:+35
golang.org/x/tools/gopls/internal/util/bug.Reportf:+1
golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).getImportPackage:+43
golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).handleSyntaxPackage.func2:+1
golang.org/x/sync/errgroup.(*Group).Go.func1:+3
runtime.goexit:+0

golang.org/x/tools/gopls@v0.15.2 go1.22.0 darwin/arm64 vscode (2)

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

@adonovan
Copy link
Member

adonovan commented Apr 4, 2024

This stack 4jVSEw was reported by telemetry:

gopls/bug
golang.org/x/tools/gopls/internal/util/bug.report:+35
golang.org/x/tools/gopls/internal/util/bug.Reportf:+1
golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).getImportPackage:+43
golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).typesConfig.func1:+20
golang.org/x/tools/gopls/internal/cache.importerFunc.Import:+0
go/types.(*Checker).importPackage:+28
go/types.(*Checker).collectObjects.func1:+13
go/types.(*Checker).walkDecl:+9
go/types.(*Checker).walkDecls:=391
go/types.(*Checker).collectObjects:+42
go/types.(*Checker).checkFiles:+32
go/types.(*Checker).Files:=341
golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).checkPackageForImport:+50
golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).getImportPackage:+50
golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).handleSyntaxPackage.func2:+1
golang.org/x/sync/errgroup.(*Group).Go.func1:+3

golang.org/x/tools/gopls@v0.15.2 go1.21.3 darwin/arm64 other (2)

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

@gopherbot
Copy link

Change https://go.dev/cl/581175 mentions this issue: gopls/internal/cache: add more assertions for golang/go#60890

@adonovan adonovan modified the milestones: gopls/v0.12.3, gopls/v0.17.0 Apr 23, 2024
gopherbot pushed a commit to golang/tools that referenced this issue Apr 23, 2024
@adonovan
gopls/internal/cache: add more assertions for golang/go#60890
1523441 
This is one of our most frequent and oldest open crash bugs.

Updates golang/go#60890

Change-Id: I97bdf339ec355aaf23fb81ee8fed11b142d28409
Reviewed-on: https://go-review.googlesource.com/c/tools/+/581175
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@findleyr findleyr

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
Milestone
gopls/v0.17.0
Development

No branches or pull requests
5 participants
@JamyDev @rossb83 @adonovan @gopherbot @findleyr