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

cmd/go: panic: runtime error: index out of range [-1] in collectDepsErrors #61816

Closed
findleyr opened this issue Aug 7, 2023 · 13 comments
Closed
Assignees
Labels
GoCommand cmd/go NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@findleyr
Copy link
Contributor

findleyr commented Aug 7, 2023

Related: #59905.

Just observed this panic while looking into golang/vscode-go#2923:

From the root directory of https://github.com/yaklang/yaklang:

> go list -e -compiled -test ./...
panic: runtime error: index out of range [-1]

goroutine 1 [running]:
cmd/go/internal/list.collectDepsErrors.func1(0xc0029afae8?, 0xc001d51008?)
        /home/rfindley/src/go/src/cmd/go/internal/list/list.go:962 +0x134
sort.insertionSort_func({0xc001d51210?, 0xc001bde8a0?}, 0x0, 0x2)
        /home/rfindley/src/go/src/sort/zsortfunc.go:12 +0xa7
sort.pdqsort_func({0xc001d51210?, 0xc001bde8a0?}, 0x7f2b023bc108?, 0x18?, 0xc000506000?)
        /home/rfindley/src/go/src/sort/zsortfunc.go:73 +0x31b
sort.Slice({0x9af040?, 0xc0029afae8?}, 0x2?)
        /home/rfindley/src/go/src/sort/slice.go:29 +0xf8
cmd/go/internal/list.collectDepsErrors(0xc002898600)
        /home/rfindley/src/go/src/cmd/go/internal/list/list.go:951 +0x2f7
cmd/go/internal/list.runList({0xb896b8?, 0xf2cfc0}, 0xef0a00?, {0xc000022230?, 0x1, 0x1})
        /home/rfindley/src/go/src/cmd/go/internal/list/list.go:807 +0x21c5
main.invoke(0xef0a00, {0xc0000221f0, 0x5, 0x5})
        /home/rfindley/src/go/src/cmd/go/main.go:268 +0x5c7
main.main()
        /home/rfindley/src/go/src/cmd/go/main.go:186 +0x786

Does not reproduce on 1.20.

@matloob @bcmills should this be a release blocker? It breaks gopls, and I'm not sure how many repositories it affects.

@findleyr
Copy link
Contributor Author

findleyr commented Aug 7, 2023

CC @golang/release for awareness, as it is the 11th hour (or 11:59pm, perhaps).

Given that others have not encountered this issue, this may be rare enough not to block the 1.21.0 release. But with that said, I'm concerned about shipping a known gopls breakage with 1.21.

@findleyr
Copy link
Contributor Author

findleyr commented Aug 7, 2023

Confirmed that this panic also occurs with go1.21rc4.

@bcmills
Copy link
Contributor

bcmills commented Aug 7, 2023

I think we had an omission in https://go.dev/cl/501978: looks like we missed the case wher len(stkj) == 0 and len(stki) != 0.

@findleyr findleyr added this to the Go1.21.1 milestone Aug 7, 2023
@findleyr
Copy link
Contributor Author

findleyr commented Aug 7, 2023

@gopherbot please consider this for backport to 1.21. It is a new Go command crash.

@gopherbot
Copy link

Backport issue(s) opened: #61818 (for 1.21).

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

@bcmills bcmills added NeedsFix The path to resolution is known, but the work has not been done. GoCommand cmd/go labels Aug 7, 2023
@bcmills bcmills modified the milestones: Go1.21.1, Go1.22 Aug 7, 2023
@matloob
Copy link
Contributor

matloob commented Aug 7, 2023

I have a fix cl. I wasn't able to reproduce using the yaklang repo, but I reproduced it in a test case.

@gopherbot
Copy link

Change https://go.dev/cl/516739 mentions this issue: cmd/go: fix missing case checking for empty slice

@bcmills bcmills removed their assignment Aug 7, 2023
@findleyr
Copy link
Contributor Author

findleyr commented Aug 8, 2023

I have a fix cl. I wasn't able to reproduce using the yaklang repo, but I reproduced it in a test case.

Interesting. For me, it took nothing more than checking out the repo and running go list -e -compiled -test ./... from the root directory, reproducing 100% of the time. I wonder if it is related to GOOS=linux?

@matloob
Copy link
Contributor

matloob commented Aug 8, 2023

It could be. I just tried it on linux and was able to repro.

@gopherbot
Copy link

Change https://go.dev/cl/519658 mentions this issue: [release-branch.go1.21] cmd/go: fix missing case checking for empty slice

gopherbot pushed a commit that referenced this issue Aug 15, 2023
…lice

When we were comparing the first element of import stacks when sorting
depserrors we checked if the first stack was non empty, but not the
second one. Do the check for both stacks.

Fixes #61818
Updates #61816
For #59905

Change-Id: Id5c11c2b1104eec93196a08c53372ee2ba97c701
Reviewed-on: https://go-review.googlesource.com/c/go/+/516739
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
(cherry picked from commit 58447d7)
Reviewed-on: https://go-review.googlesource.com/c/go/+/519658
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
@geitir
Copy link

geitir commented Aug 25, 2023

Hi is this fixed in release version of go1.21 or do we need to wait for a minor version?

@cagedmantis
Copy link
Contributor

@geitir This will be included in the next minor release.

@dreamerlzl
Copy link

Can confirm that this doesn't happen in the just released go 1.21.1 on my side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GoCommand cmd/go 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