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: types.Func.typ=nil panic in missingMethod with gopls@master built with go@master #59848

Closed
timothy-king opened this issue Apr 20, 2023 · 13 comments
Closed

x/tools/gopls: types.Func.typ=nil panic in missingMethod with gopls@master built with go@master #59848

timothy-king opened this issue Apr 20, 2023 · 13 comments
Assignees
@griesemer @findleyr
Labels
gopls Issues related to the Go language server, gopls. NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone
Go1.21

Comments

@timothy-king
Copy link
Contributor

gopls version: v0.0.0-20230404161500-4d205d81b5a0 (devel go1.21-ac571a388dc Mon Apr 3 20:17:02 2023 0000)
gopls flags:
update flags: proxy
extension version: 0.38.0
go version: devel 1.21-ac571a388dc
environment: Visual Studio Code darwin
initialization error: undefined
issue timestamp: Thu, 20 Apr 2023 19:39:00 GMT
restart history:
Wed, 19 Apr 2023 21:24:47 GMT: activation (enabled: true)

ATTENTION: PLEASE PROVIDE THE DETAILS REQUESTED BELOW.

Describe what you observed.

panic: interface conversion: types.Type is nil, not *types.Signature

goroutine 2426 [running]:
go/types.(*Checker).handleBailout(0xc0030341e0, 0xc002ded5c8)
	  check.go:299  0x88
panic({0x100983da0%3F, 0xc00336b020%3F})
	  panic.go:913  0x21f
go/types.(*Checker).funcString(0xc0030341e0, 0xc0010e8cc0, 0x0)
	  lookup.go:483  0x1df
go/types.(*Checker).missingMethod(0xc0030341e0, {0x100d03458%3F, 0xc003ad0ec0}, {0x100d03430, 0xc000706cb0}, 0x1, 0x100bf7d00, 0xc00303dfa0)
	  lookup.go:432  0x46b
go/types.(*Checker).implements(0xc0030341e0, {0x100d03458%3F, 0xc003ad0ec0}, {0x100d03430%3F, 0xc000706cb0}, 0x0, 0xc00303dfa0)
	  instantiate.go:246  0x55d
go/types.(*operand).assignableTo(0xc0027d3a00, 0xc0030341e0, {0x100d03430%3F, 0xc000706cb0%3F}, 0xc002debfa0)
	  operand.go:285  0x89c
go/types.(*Checker).assignment(0xc0030341e0, 0xc0027d3a00, {0x100d03430, 0xc000706cb0}, {0x100aab8c9, 0x16})
	  assignments.go:89  0x7cc
go/types.(*Checker).indexedElts(0x100d034a8%3F, {0xc003ad07a0, 0x1, 0xc002dec488%3F}, {0x100d03430, 0xc000706cb0}, 0xffffffffffffffff)
	  index.go:453  0x14d
go/types.(*Checker).exprInternal(0xc0030341e0, {0x0%3F, 0x0%3F}, 0xc0027d3800, {0x100d059a0, 0xc0027d2180%3F}, {0x0%3F, 0x0%3F})
	  expr.go:1520  0xacd
go/types.(*Checker).rawExpr(0xc0030341e0, {0x0, 0x0}, 0xc0027d3800, {0x100d059a0%3F, 0xc0027d2180%3F}, {0x0%3F, 0x0%3F}, 0x0)
	  expr.go:1254  0x1ac
go/types.(*Checker).expr(0xc0030341e0%3F, {0x0%3F, 0x0%3F}, 0xc00256b800%3F, {0x100d059a0%3F, 0xc0027d2180%3F})
	  expr.go:1787  0x45
go/types.(*Checker).exprInternal(0xc0030341e0, {0x0%3F, 0x0%3F}, 0xc0027d3800, {0x100d059a0, 0xc0027d2200%3F}, {0x0%3F, 0x0%3F})
	  expr.go:1435  0x2459
go/types.(*Checker).rawExpr(0xc0030341e0, {0x0, 0x0}, 0xc0027d3800, {0x100d059a0%3F, 0xc0027d2200%3F}, {0x0%3F, 0x0%3F}, 0x0)
	  expr.go:1254  0x1ac
go/types.(*Checker).expr(0x20%3F, {0x0%3F, 0x0%3F}, 0x129295c88%3F, {0x100d059a0%3F, 0xc0027d2200%3F})
	  expr.go:1787  0x45
go/types.(*Checker).unary(0xc0030341e0, 0xc0027d3800, 0xc002a187c0)
	  expr.go:163  0x45
go/types.(*Checker).exprInternal(0xc0030341e0, {0x0, 0x0}, 0xc0027d3800, {0x100d06030, 0xc002a187c0%3F}, {0x0%3F, 0x0%3F})
	  expr.go:1680  0x1605
go/types.(*Checker).rawExpr(0xc0030341e0, {0x0, 0x0}, 0xc0027d3800, {0x100d06030%3F, 0xc002a187c0%3F}, {0x0%3F, 0x0%3F}, 0x0)
	  expr.go:1254  0x1ac
go/types.(*Checker).expr(0x10000700b%3F, {0x0%3F, 0x0%3F}, 0x0%3F, {0x100d06030%3F, 0xc002a187c0%3F})
	  expr.go:1787  0x45
go/types.(*Checker).varDecl(0x10096d220%3F, 0xc0010e8a20, {0xc00086e758%3F, 0x1, 0x1}, {0x0, 0x0}, {0x100d06030, 0xc002a187c0})
	  decl.go:513  0x16c
go/types.(*Checker).objDecl(0xc0030341e0, {0x100d10198, 0xc0010e8a20}, 0x1%3F)
	  decl.go:194  0x95e
go/types.(*Checker).packageObjects(0xc0030341e0)
	  resolver.go:662  0x465
go/types.(*Checker).checkFiles(0xc0030341e0, {0xc002a19d00, 0x3, 0x4})
	  check.go:329  0x149
go/types.(*Checker).Files(...)
	  check.go:304
golang.org/x/tools/gopls/internal/lsp/cache.doTypeCheck({0x100d070a8, 0xc003539800}, 0xc0028c81e0, {{0xc00071a000, 0x3b}, {0xc00071a000, 0x3b}, {0xc000bfdec0, 0xc}, {0xc00375d2f0, ...}, ...})
	  check.go:1101  0x745
golang.org/x/tools/gopls/internal/lsp/cache.typeCheckImpl({0x100d070a8, 0xc003539650}, 0x1000100ad%3F, {{0xc00071a000, 0x3b}, {0xc00071a000, 0x3b}, {0xc000bfdec0, 0xc}, {0xc00375d2f0, ...}, ...})
	  check.go:968  0x205
golang.org/x/tools/gopls/internal/lsp/cache.(*typeCheckBatch).checkPackage(0xc002deded8%3F, {0x100d070a8, 0xc0028d2300}, 0xc0034c5420)
	  check.go:648  0x225
golang.org/x/tools/gopls/internal/lsp/cache.(*typeCheckBatch).handleSyntaxPackage(0xc0028c81e0, {0x100d070a8, 0xc0028d2300}, 0xc000cbb3e0%3F, {0xc00071a000, 0x3b})
	  check.go:532  0x425
golang.org/x/tools/gopls/internal/lsp/cache.(*snapshot).forEachPackageInternal.func2()
	  check.go:407  0x2b
golang.org/x/sync/errgroup.(*Group).Go.func1()
	  errgroup.go:75  0x5b
created by golang.org/x/sync/errgroup.(*Group).Go in goroutine 1998
	  errgroup.go:72  0x96
[Error - 12:38:45 PM]

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
Copy link
Contributor

Thanks @timothy-king!

This may be a type-checker regression, or related to the fact that we import invalid packages from export data. I'll investigate.

CC @adonovan @griesemer for awareness.

I'll transfer to the Go issue tracker.

@findleyr findleyr changed the title gopls: automated issue report (crash) x/tools/gopls: panic in missingMethod with gopls@master built with go@master Apr 26, 2023
@findleyr findleyr transferred this issue from golang/vscode-go Apr 26, 2023
@findleyr findleyr added gopls Issues related to the Go language server, gopls. and removed upstream-tools labels Apr 26, 2023
@gopherbot gopherbot added this to the Unreleased milestone Apr 26, 2023
@findleyr findleyr modified the milestones: Unreleased, gopls/v0.12.0 Apr 26, 2023
@findleyr findleyr added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker labels Apr 26, 2023
@findleyr findleyr self-assigned this Apr 26, 2023
@adonovan
Copy link
Member

adonovan commented Apr 26, 2023 •
edited

I wonder if this is dup of #59371, which was just fixed.

What exact version of GOROOT are you using? I can't relate the stack trace to the source. Specifically:

go/types.(*Checker).assignment(0xc0030341e0, 0xc0027d3a00, {0x100d03430, 0xc000706cb0}, {0x100aab8c9, 0x16})
	  assignments.go:89  0x7cc

this frame currently contains a plain return statement (in a function that doesn't use defer), and rolling back recent commits don't make it line up either:

goroot$ git co master
goroot$ nl -ba ./src/go/types/assignments.go | grep -w 89
    89			return
goroot$ git log ./src/go/types/assignments.go | grep ^commit | head -n 2
commit 522eace4ca496626bcb3b65f9dcb6e8db2673fed
commit 969ab34e4629bdda410c8468d4f45a08e4fec9f8
goroot$ git co 522eace4ca496626bcb3b65f9dcb6e8db2673fed^
goroot$ nl -ba ./src/go/types/assignments.go | grep -w 89
    89			return
goroot$ git co 969ab34e4629bdda410c8468d4f45a08e4fec9f8^
goroot$ nl -ba ./src/go/types/assignments.go | grep -w 89
    89		}
goroot$

[EDIT: never mind, I see that it's go@ ac571a3, and it does line up]

@adonovan adonovan changed the title x/tools/gopls: panic in missingMethod with gopls@master built with go@master x/tools/gopls: types.Func.typ=nil panic in missingMethod with gopls@master built with go@master May 10, 2023
@adonovan
Copy link
Member

The panic is caused by Func.typ=nil in funcString when missingMethod tries to print the wrongName reason for the failed assignment to a nontrivial interface type. The syntax must be something like var _ = MyStruct{field: I{x}} where I is an interface type and x has a method of the wrong name or receiver pointerness. But that doesn't explain why the type of the I.f or x.f method is nil, nor which one it is. (Perhaps the program counter might answer the "which" question.) I don't think it can be due to a cycle because a type decl can't depend on a var decl.

@griesemer
Copy link
Contributor

wrongName can only happen in one case where a function f is found with a folded-case name that matches.

@adonovan
Copy link
Member

adonovan commented May 10, 2023 •
edited

wrongName can only happen in one case where a function f is found with a folded-case name that matches.

Or a receiver type that lacks a pointer, I think.

@findleyr
Copy link
Contributor

findleyr commented May 10, 2023 •
edited

From looking at this, I think go/types.Checker cannot produce a Func without a Signature type. All Funcs are produced via NewFunc, which is only called from Checker.interfaceType (with a provably non-nil signature), or from collectObjects (which will be provably given a non-nil signature by Checker.FuncDecl).

I could be mistaken, but I think the Func must be created by the importer.

@findleyr
Copy link
Contributor

I could be mistaken, but I think the Func must be created by the importer.

Hah, well I should have looked at the importer before making this assertion. I must be mistaken: it is even easier to see that the indexed importer cannot produce a Func without a signature (NewFunc is only called with the result of importReader.signature). Now I'm very confused.

@griesemer griesemer self-assigned this May 11, 2023
@findleyr
Copy link
Contributor

@griesemer and I both have no ideas how to proceed; let's move this to the v0.13.0 milestone, as I don't think we'll be able to figure it out without more data.

@findleyr findleyr modified the milestones: gopls/v0.12.0, gopls/v0.13.0 May 11, 2023
@findleyr findleyr modified the milestones: gopls/v0.13.0, gopls/v0.12.0 May 11, 2023
@findleyr
Copy link
Contributor

We had two google-internal reproducers of this crash, not using gopls, albeit with slightly different call stacks (but still in initVar). @griesemer and @adonovan I CCed you on those internal issues.

Therefore, I think this is a go/types regression related to the recent refactoring of assignability. Moving into the go1.21 milestone.

@findleyr findleyr modified the milestones: gopls/v0.12.0, Go1.21 May 12, 2023
@findleyr
Copy link
Contributor

Tentatively marking this as a release blocker. There was a new internal google toolchain release to our internal infrastructure yesterday, and two crashes overnight suggests to me that this is a frequent enough crash pattern to block the release.

@findleyr
Copy link
Contributor

Oh. This shouldn't have been this complicated. missingMethod logic was refactored in https://go.dev/cl/472637 and subsequent.

It looks like there is a way to miss the objDecl call, here:
https://cs.opensource.google/go/go/+/master:src/go/types/lookup.go;l=391;drc=804d786a30c664df63ef1e0c95d44772063afae1

Note that in the wrongName case, we skip the objDecl call.

Now to construct a reproducer.

@findleyr
Copy link
Contributor

Simple repro:

type T struct{}
type I interface{M()}
var _ I = T{}
func (T) m()

@gopherbot
Copy link

Change https://go.dev/cl/494615 mentions this issue: go/types, types2: be sure to type-check wrong methods in missingMethod

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels May 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@griesemer griesemer

@findleyr findleyr

Labels
gopls Issues related to the Go language server, gopls. NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Projects
None yet
Milestone
Go1.21
Development

No branches or pull requests
6 participants
@timothy-king @dmitshur @adonovan @griesemer @gopherbot @findleyr