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

go/types: assertion failure in recent range statement checking logic #68334

Closed
lmorg opened this issue Jul 5, 2024 · 16 comments
Closed

go/types: assertion failure in recent range statement checking logic #68334

lmorg opened this issue Jul 5, 2024 · 16 comments
Assignees
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@lmorg
Copy link

lmorg commented Jul 5, 2024

gopls version: v0.16.1/go1.22.5
gopls flags:
update flags: proxy
extension version: 0.41.4
environment: Visual Studio Code darwin
initialization error: undefined
issue timestamp: Fri, 05 Jul 2024 22:28:48 GMT
restart history:
Fri, 05 Jul 2024 21:41:59 GMT: activation (enabled: true)

ATTENTION: PLEASE PROVIDE THE DETAILS REQUESTED BELOW.

Describe what you observed.

panic:   stmt.go:932: assertion failed

goroutine 3607 [running]:
go/types.(*Checker).handleBailout(0x1400113ca00, 0x14005649878)
	  check.go:367  0x9c
panic({0x102d833a0%3F, 0x14000aee0e0%3F})
	  panic.go:770  0x124
go/types.assert(0x0%3F)
	  errors.go:28  0x60
go/types.(*Checker).rangeStmt(0x1400113ca00, 0x3, 0x140030696e0)
	  stmt.go:932  0xb58
go/types.(*Checker).stmt(0x1400113ca00, 0x3, {0x102f0ce80, 0x140030696e0})
	  stmt.go:827  0x874
go/types.(*Checker).stmtList(0x1400113ca00, 0x3, {0x140030c0480%3F, 0x102a04115%3F, 0x5%3F})
	  stmt.go:121  0x88
go/types.(*Checker).stmt(0x1400113ca00, 0x3, {0x102f0ce20, 0x140030b0fc0})
	  stmt.go:562  0x1974
go/types.(*Checker).rangeStmt(0x1400113ca00, 0x3, 0x14003069920)
	  stmt.go:993  0x570
go/types.(*Checker).stmt(0x1400113ca00, 0x0, {0x102f0ce80, 0x14003069920})
	  stmt.go:827  0x874
go/types.(*Checker).stmtList(0x1400113ca00, 0x0, {0x1400303f580%3F, 0x1020ec4e8%3F, 0x14005648668%3F})
	  stmt.go:121  0x88
go/types.(*Checker).funcBody(0x1400113ca00, 0x102f0a588%3F, {0x102a364cd%3F, 0x140000d4380%3F}, 0x14002ea4600, 0x140030b1050, {0x0%3F, 0x0%3F})
	  stmt.go:41  0x21c
go/types.(*Checker).exprInternal.func1()
	  expr.go:1089  0x44
go/types.(*Checker).processDelayed(0x1400113ca00, 0x92)
	  check.go:467  0x12c
go/types.(*Checker).stmt(0x1400113ca00, 0x0, {0x102f0cd00, 0x140030b4960})
	  stmt.go:832  0x1db0
go/types.(*Checker).stmtList(0x1400113ca00, 0x0, {0x140007e5d00%3F, 0x0%3F, 0x140056496d8%3F})
	  stmt.go:121  0x88
go/types.(*Checker).funcBody(0x1400113ca00, 0x102f0a588%3F, {0x140007843a0%3F, 0x140000d4380%3F}, 0x140028ff6c0, 0x140030b1080, {0x0%3F, 0x0%3F})
	  stmt.go:41  0x21c
go/types.(*Checker).funcDecl.func1()
	  decl.go:852  0x44
go/types.(*Checker).processDelayed(0x1400113ca00, 0x0)
	  check.go:467  0x12c
go/types.(*Checker).checkFiles(0x1400113ca00, {0x14003bb7300, 0xa, 0x10})
	  check.go:411  0x188
go/types.(*Checker).Files(...)
	  check.go:372
golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).checkPackage(0x1400546e8a0, {0x102f0e168, 0x14003234270}, 0x14003683c20)
	  check.go:1543  0x788
golang.org/x/tools/gopls/internal/cache.(*typeCheckBatch).handleSyntaxPackage(0x1400546e8a0, {0x102f0e168, 0x14003234270}, 0x0, {0x14000ab48d0, 0x2d})
	  check.go:568  0x534
golang.org/x/tools/gopls/internal/cache.(*Snapshot).forEachPackageInternal.func2()
	  check.go:418  0x34
golang.org/x/sync/errgroup.(*Group).Go.func1()
	  errgroup.go:78  0x58
created by golang.org/x/sync/errgroup.(*Group).Go in goroutine 151
	  errgroup.go:75  0x98
gopls stats -anon { "DirStats": { "Files": 1984, "TestdataFiles": 5, "GoFiles": 1083, "ModFiles": 1, "Dirs": 235 }, "GOARCH": "arm64", "GOOS": "darwin", "GOPACKAGESDRIVER": "", "GOPLSCACHE": "", "GoVersion": "go1.22.5", "GoplsVersion": "v0.16.1", "InitialWorkspaceLoadDuration": "1.169338542s", "MemStats": { "HeapAlloc": 60239376, "HeapInUse": 106414080, "TotalAlloc": 703540872 }, "WorkspaceStats": { "Files": { "Total": 2250, "Largest": 5748466, "Errs": 0 }, "Views": [ { "GoCommandVersion": "go1.22.5", "AllPackages": { "Packages": 742, "LargestPackage": 155, "CompiledGoFiles": 3962, "Modules": 26 }, "WorkspacePackages": { "Packages": 271, "LargestPackage": 63, "CompiledGoFiles": 1417, "Modules": 1 }, "Diagnostics": 0 } ] } }

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) go/types: assertion failure in recent range statement checking logic Jul 7, 2024
@findleyr findleyr transferred this issue from golang/vscode-go Jul 7, 2024
@findleyr
Copy link
Contributor

findleyr commented Jul 7, 2024

@griesemer this recent panic appears to occur in logic that was recently back-ported to 1.22: https://go.dev/cl/580937.

The assertion failure is here:
https://cs.opensource.google/go/go/+/refs/tags/go1.22.5:src/go/types/stmt.go;l=932;drc=11b861e45936eb7911e30304163a14553f53a5d1

assert(i == 0) // at most one iteration variable (rhs[1] == nil for rangeOverInt)

@findleyr findleyr added this to the Go1.23 milestone Jul 7, 2024
@findleyr
Copy link
Contributor

findleyr commented Jul 7, 2024

@lmorg if you happen to recall any details about the range statement you were editing at the time of this panic, that would be helpful, but in any case we should be able to find the logical flaw (I'll note that this assertion was discussed in code review; we must have missed something).

@griesemer tentatively assigning to you for further investigation. I can also take a look, but will first be busy with #68311. The bug may be easy to spot--I did not attempt.

@findleyr
Copy link
Contributor

findleyr commented Jul 8, 2024

Actually, I think I'll be able to get to this sooner than @griesemer.

CC @timothy-king for awareness: Tim, I'll ask you to review! :)

@findleyr findleyr assigned findleyr and unassigned griesemer Jul 8, 2024
@lmorg
Copy link
Author

lmorg commented Jul 8, 2024

@findleyr I can't say for 100% certain but looking at the commit times on the project I was working on, and corrolating that to the report time of this issue, I'd guess it was this one:

https://github.com/lmorg/murex/blob/37b8ce0c539eefce7c6deadb28f9bc2d51a5e39a/builtins/core/modules/cmd-list.go#L152

@timothy-king
Copy link
Contributor

timothy-king commented Jul 8, 2024

edit: Nope. That wasn't it.

gopls sets types.Config.Error to a non-nil function. If check.conf.Error != nil, then

		case v == nil && sValue != nil:
			check.softErrorf(sValue, InvalidIterVar, "range over %s permits only one iteration variable", &x)

won't stop execution in (*types.Checker).rangeStmt. This might be how assert(i == 0) is getting triggered.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/597356 mentions this issue: go/types: fix range over int invariant

@findleyr
Copy link
Contributor

@gopherbot please backport this issue to 1.22. It is a regression in the latest patch release and causes crashes in gopls.

@gopherbot
Copy link
Contributor

Backport issue(s) opened: #68370 (for 1.22).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

@findleyr
Copy link
Contributor

Not technically a release blocker since this is already released in 1.22.5, but we should fix for 1.23 and backport.

Thanks @timothy-king for investigating.

@lmorg
Copy link
Author

lmorg commented Jul 10, 2024

Thanks both. I appreciate there wasn't much to go on for this particular issue (sorry about that)

@findleyr
Copy link
Contributor

@lmorg your time to file the issue is greatly appreciated. No need to apologize.

gopherbot pushed a commit that referenced this issue Jul 12, 2024
Fixes an assertion failure in Checker.rangeStmt that range over int
only has a key type and no value type. When allowVersion failed,
rangeKeyVal returns Typ[Invalid] for the value instead of nil. When
Config.Error != nil, rangeStmt proceeded. The check for rhs[1]==nil
was not enough to catch this case. It must also check rhs[1]==

Updates #68334

Change-Id: Iffa1b2f7b6a94570ec50b8c6603e727a45ba3357
Reviewed-on: https://go-review.googlesource.com/c/go/+/597356
Reviewed-by: Robert Findley <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/598055 mentions this issue: go/types: fix assertion failure when range over int is not permitted

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/598055 mentions this issue: [release-branch.go1.22] go/types: fix assertion failure when range over int is not permitted

@seankhliao seankhliao added the NeedsFix The path to resolution is known, but the work has not been done. label Jul 13, 2024
@findleyr
Copy link
Contributor

Closing as I believe this is fixed (in go@master). The backport issue will be closed when 1.22.6 is released.

Thanks @timothy-king for the fix.

gopherbot pushed a commit that referenced this issue Jul 16, 2024
…er int is not permitted

Fixes an assertion failure in Checker.rangeStmt that range over int
only has a key type and no value type. When allowVersion failed,
rangeKeyVal returns Typ[Invalid] for the value instead of nil. When
Config.Error != nil, rangeStmt proceeded. The check for rhs[1]==nil
was not enough to catch this case. It must also check rhs[1]==

Fixes #68334
Fixes #68370

Change-Id: Iffa1b2f7b6a94570ec50b8c6603e727a45ba3357
Reviewed-on: https://go-review.googlesource.com/c/go/+/597356
Reviewed-by: Robert Findley <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
(cherry picked from commit 4e77872)
Reviewed-on: https://go-review.googlesource.com/c/go/+/598055
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

7 participants