Skip to content

x/tools/gopls: Rename: crash due to Info.Defs[id]=nil bug in checkStructField #70968

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
Closed
adonovan opened this issue Dec 23, 2024 · 8 comments
Closed

x/tools/gopls: Rename: crash due to Info.Defs[id]=nil bug in checkStructField #70968

adonovan opened this issue Dec 23, 2024 · 8 comments
Assignees
@adonovan
Labels
gopls/telemetry-wins gopls Issues related to the Go language server, gopls. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
gopls/v0.19.0

Comments

@adonovan
Copy link
Member

adonovan commented Dec 23, 2024 •
edited
Loading 

#!stacks
"runtime.sigpanic" && "checkStructField:+36"

Issue created by stacks.

named := r.pkg.TypesInfo().Defs[spec.Name].Type()

This stack Zepg4g was reported by telemetry:

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

Dups: _A3zlQ
@adonovan adonovan added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository. gopls/telemetry-wins labels Dec 23, 2024
@gopherbot gopherbot added this to the Unreleased milestone Dec 23, 2024
@gabyhelp
Copy link

Related Issues

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

@adonovan
Copy link
Member Author

This has essentially the same root cause as #69362.

@findleyr
Copy link
Member

Thanks. Let's put this in v0.18.0 for now.

Since it is in rename (and likely in the context of a broken build), I think it is not too disruptive to leave it until the v0.18.0 release. Can re-evaluate if we see this frequently.

Of course, we still have no idea how this scenario arises...

@findleyr findleyr modified the milestones: Unreleased, gopls/v0.18.0 Dec 23, 2024
@gabyhelp gabyhelp mentioned this issue Dec 27, 2024
Closed
@adonovan
Copy link
Member Author

adonovan commented Jan 3, 2025

This stack _A3zlQ was reported by telemetry:

golang.org/x/tools/gopls@v0.17.1 go1.23.4 darwin/amd64 other,vscode (1)
golang.org/x/tools/gopls@v0.17.1 go1.23.3 darwin/arm64 vscode (1)

@gabyhelp gabyhelp mentioned this issue Jan 6, 2025
Open
@gabyhelp gabyhelp mentioned this issue Jan 14, 2025
Open
@gabyhelp gabyhelp mentioned this issue Jan 29, 2025
Closed
@adonovan
Copy link
Member Author

Yay, repro:

type T int
type T struct { f int }  // rename f to g
[stderr]  goroutine 37661 gp=0x1400809b6c0 m=19 mp=0x14018440008 [running]:
[stderr]  panic({0x102f88fc0?, 0x103817890?})
[stderr]  	/Users/adonovan/w/goroot/src/runtime/panic.go:806 +0x154 fp=0x14000040fe0 sp=0x14000040f30 pc=0x10220e0e4
[stderr]  runtime.panicmem(...)
[stderr]  	/Users/adonovan/w/goroot/src/runtime/panic.go:262
[stderr]  runtime.sigpanic()
[stderr]  	/Users/adonovan/w/goroot/src/runtime/signal_unix.go:925 +0x300 fp=0x14000041040 sp=0x14000040fe0 pc=0x1022107c0
[stderr]  golang.org/x/tools/gopls/internal/golang.(*renamer).checkStructField(0x1400fb295e0, 0x14014261bc0)
[stderr]  	/Users/adonovan/w/xtools/gopls/internal/golang/rename_check.go:475 +0x2c4 fp=0x14000041120 sp=0x14000041050 pc=0x102a922b4
[stderr]  golang.org/x/tools/gopls/internal/golang.(*renamer).check(0x1400fb295e0, {0x103102378, 0x14014261bc0})
[stderr]  golang.org/x/tools/gopls/internal/golang.(*renamer).check(0x1400fb295e0, {0x103102378, 0x14014261bc0})
[stderr]  	/Users/adonovan/w/xtools/gopls/internal/golang/rename_check.go:106 +0x10c fp=0x140000411d0 sp=0x14000041120 pc=0x102a902fc
[stderr]  golang.org/x/tools/gopls/internal/golang.renameObjects({0x102e7e478, 0x1}, 0x14011332f60, {0x140154ffe40, 0x1, 0x1?})
[stderr]  	/Users/adonovan/w/xtools/gopls/internal/golang/rename.go:1173 +0x23c fp=0x14000041300 sp=0x140000411d0 pc=0x102a8d7ac
[stderr]  golang.org/x/tools/gopls/internal/golang.renameOrdinary({0x1030f3e38, 0x1400fce2450}, 0x1400ca89560, {0x1030f6c20, 0x1401463f4a0}, {0x298a868?, 0x1?}, {0x102e7e478, 0x1})
[stderr]  	/Users/adonovan/w/xtools/gopls/internal/golang/rename.go:574 +0x908 fp=0x14000041500 sp=0x14000041300 pc=0x102a8ac38
[stderr]  golang.org/x/tools/gopls/internal/golang.Rename({0x1030f3e38?, 0x1400fce22a0?}, 0x1400ca89560, {0x1030f6c20, 0x1401463f4a0}, {0x230d80c?, 0x1?}, {0x102e7e478, 0x1})
[stderr]  	/Users/adonovan/w/xtools/gopls/internal/golang/rename.go:419 +0x138 fp=0x140000416b0 sp=0x14000041500 pc=0x102a89d58
[stderr]  golang.org/x/tools/gopls/internal/server.(*server).Rename(0x14000032d80, {0x1030f3e38, 0x14006ad0f60}, 0x1401538e140)

@adonovan
Copy link
Member Author

We really need to write down all the invariants of go/ast and go/types, including:

  • Defs[id] may be absent, even for an id on the LHS of a declaration, if that identifier is an illegal redeclaration.

This was referenced Feb 3, 2025
Open
Open
@findleyr findleyr modified the milestones: gopls/v0.18.0, gopls/v0.19.0 Feb 11, 2025
This was referenced Feb 11, 2025
Open
Closed
Open
@gabyhelp gabyhelp mentioned this issue Feb 24, 2025
Open
@gabyhelp gabyhelp mentioned this issue Mar 18, 2025
Open
@gabyhelp gabyhelp mentioned this issue Mar 31, 2025
Open
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/663895 mentions this issue: go/types: document that Defs[id] may be nil in ill-typed code

@adonovan adonovan self-assigned this Apr 8, 2025
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/663915 mentions this issue: gopls/internal/golang: Rename: fix crash in ill-typed redeclaration

gopherbot pushed a commit that referenced this issue Apr 8, 2025
@adonovan @gopherbot
go/types: document that Defs[id] may be missing in ill-typed code
8433412 
Updates #70968

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

@adonovan adonovan

Labels
gopls/telemetry-wins gopls Issues related to the Go language server, gopls. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
gopls/v0.19.0
Development

No branches or pull requests
4 participants
@adonovan @gopherbot @findleyr @gabyhelp