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

Jump to bottom

cmd/go: panic in base.SetFromGOFLAGS #42013

Closed
tie opened this issue Oct 16, 2020 · 6 comments
Closed

cmd/go: panic in base.SetFromGOFLAGS #42013

tie opened this issue Oct 16, 2020 · 6 comments
Labels
FrozenDueToAge GoCommand cmd/go help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Backlog

Comments

@tie
Copy link
Contributor

tie commented Oct 16, 2020

What version of Go are you using (go version)?

go version devel +66e66e7113 Sun Sep 13 19:17:09 2020 +0000 darwin/amd64

What did you do?

GOFLAGS='=' gotip env
GOFLAGS='-trimpath =' gotip env
GOFLAGS='= -trimpath' gotip env

What did you expect to see?

go: parsing $GOFLAGS: non-flag "="

What did you see instead?

panic: runtime error: slice bounds out of range [1:0]

goroutine 1 [running]:
cmd/go/internal/base.SetFromGOFLAGS(0x195a938)
	/go/src/cmd/go/internal/base/goflags.go:101 +0x9a7
main.main()
	/go/src/cmd/go/main.go:188 +0x755
tie added a commit to tie/genji that referenced this issue Oct 16, 2020
@tie
Fix panic due to invalid GOFLAGS arguments
ae8bc12 
See golang/go#42013
tie added a commit to tie/genji that referenced this issue Oct 16, 2020
@tie
Fix panic due to invalid GOFLAGS arguments
4bd61ba 
See golang/go#42013
@bcmills bcmills added help wanted NeedsFix The path to resolution is known, but the work has not been done. labels Oct 16, 2020
@bcmills bcmills added this to the Backlog milestone Oct 16, 2020
@bcmills bcmills added the GoCommand cmd/go label Oct 16, 2020
@bcmills
Copy link
Contributor

bcmills commented Oct 16, 2020

Neat find! Thanks for the report.

@obeyda
Copy link
Contributor

obeyda commented Oct 16, 2020

Hello, can I help on this issue?

@bcmills
Copy link
Contributor

bcmills commented Oct 16, 2020

@obeyda, sure! Start with a new test case replicating the panic in src/cmd/go/testdata/script.

@gopherbot
Copy link

Change https://golang.org/cl/263098 mentions this issue: cmd/go: add flag name check for GOFLAGS entries

@obeyda
Copy link
Contributor

obeyda commented Oct 16, 2020

On second thought, I think it would make more sense to silently ignore the bad flag instead of showing an error, since this only happens in env and bug commands, and these commands are used when debugging...
see comment on: https://go.googlesource.com/go/+/refs/heads/master/src/cmd/go/internal/base/goflags.go#40

@bcmills
Copy link
Contributor

bcmills commented Oct 16, 2020

Yep. When hideErrors is false, InitGOFLAGS should report the invalid flags, so SetFromGOFLAGS can ignore invalid flags unconditionally. (Either the error has already been reported in InitGOFLAGS, or we don't care to report it at all.)

tie added a commit to tie/genji that referenced this issue Oct 19, 2020
@tie
Fix panic due to invalid GOFLAGS arguments
dd074fc 
See golang/go#42013
@golang golang locked and limited conversation to collaborators Oct 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge GoCommand cmd/go help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
4 participants
@bcmills @gopherbot @tie @obeyda