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: build panics when passed =" as build path #49137

Open
noerw opened this issue Oct 24, 2021 · 6 comments · May be fixed by #50089
Open

cmd/go: build panics when passed =" as build path #49137

noerw opened this issue Oct 24, 2021 · 6 comments · May be fixed by #50089
Labels
GoCommand cmd/go help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Backlog

Comments

@noerw
Copy link

noerw commented Oct 24, 2021 •
edited

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

$ go version
1.17.1 linux/amd64

Does this issue reproduce with the latest release?

yes (1.17.2)

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/asdf/.cache/go-build"
GOENV="/home/asdf/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/norwin/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/norwin/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.17.1"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/asdf/asdf/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build3283573966=/tmp/go-build -gno-record-gcc-switches"

What did you do?

run either of these commands in any go project dir

go build '' -ldflags='-X "main.Tags="'
go install '' -ldflags='-X "main.Tags="'

What did you expect to see?

The compilation should succeed, as it does when removing the ''.

-go build '' -ldflags='-X "main.Tags="'
+go build -ldflags='-X "main.Tags="'

What did you see instead?

panic: path "-ldflags=-X \"main.Tags=\"" not in error "invalid import path \"-ldflags=-X \\\"main.Tags=\\\"\"" [recovered]
	panic: path "-ldflags=-X \"main.Tags=\"" not in error "invalid import path \"-ldflags=-X \\\"main.Tags=\\\"\""

goroutine 1 [running]:
cmd/go/internal/load.(*preload).flush(0xc0001f62c0)
	/usr/lib/go/src/cmd/go/internal/load/pkg.go:1041 +0x78
panic({0x8f9d60, 0xc000684a60})
	/usr/lib/go/src/runtime/panic.go:1038 +0x215
cmd/go/internal/load.ImportErrorf({0x7ffca7ec715d, 0x18}, {0x99f456, 0x18}, {0xc00021f6b0, 0xc00021fb58, 0x0})
	/usr/lib/go/src/cmd/go/internal/load/pkg.go:500 +0x19b
cmd/go/internal/load.(*Package).load(0xc0006dc000, {0xa70560, 0xc000122000}, {0x34, 0xc0, 0x2}, {0x7ffca7ec715d, 0x0}, 0xc00021fb58, {0x0, ...}, ...)
	/usr/lib/go/src/cmd/go/internal/load/pkg.go:1846 +0x17dc
cmd/go/internal/load.loadImport({0xa70560, 0xc000122000}, {0x60, 0x5, 0xa7}, 0xc0001f62c0, {0x7ffca7ec715d, 0x18}, {0xc00002c034, 0x24}, ...)
	/usr/lib/go/src/cmd/go/internal/load/pkg.go:718 +0x4dd
cmd/go/internal/load.PackagesAndErrors({0xa70560, 0xc000122000}, {0x2d, 0x0, 0x0}, {0xc000134030, 0x2, 0x2})
	/usr/lib/go/src/cmd/go/internal/load/pkg.go:2468 +0x809
cmd/go/internal/work.runBuild({0xa70560, 0xc000122000}, 0xc000150420, {0xc000134030, 0x2, 0x2})
	/usr/lib/go/src/cmd/go/internal/work/build.go:371 +0xa5
main.invoke(0xd804e0, {0xc000134010, 0x4, 0x4})
	/usr/lib/go/src/cmd/go/main.go:216 +0x2f6
main.main()
	/usr/lib/go/src/cmd/go/main.go:173 +0x78e
@seankhliao
Copy link
Member

Note the invocation won't succeed even if the panic is fixed, '' passes an argument of an empty string and go requires flags to come before other arguments.

More minimal panic:

$ go build '="'
panic: path "=\"" not in error "invalid import path \"=\\\"\"" [recovered]
	panic: path "=\"" not in error "invalid import path \"=\\\"\""

cc @bcmills @matloob

@seankhliao seankhliao changed the title go build crashes given bad invocation cmd/go: build panics when passed =" as build path Oct 24, 2021
@seankhliao seankhliao added GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Oct 24, 2021
@bcmills bcmills added help wanted NeedsFix The path to resolution is known, but the work has not been done. labels Oct 25, 2021
@bcmills bcmills added this to the Backlog milestone Oct 25, 2021
@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Oct 25, 2021
@SilverRainZ
Copy link
Contributor

Hello, I am trying to fix this.

Once double quoted string appears in the import path, build panics. for example: go build '"a"' also crash:

$ go build '"a"'
panic: path "\"a\"" not in error "invalid import path \"\\\"a\\\"\"" [recovered]
	panic: path "\"a\"" not in error "invalid import path \"\\\"a\\\"\""

goroutine 1 [running]:
cmd/go/internal/load.(*preload).flush(0xc0001196c0)
	/usr/local/Cellar/go/1.17/libexec/src/cmd/go/internal/load/pkg.go:1041 +0x78
panic({0x14f7e80, 0xc000119770})
	/usr/local/Cellar/go/1.17/libexec/src/runtime/panic.go:1038 +0x215
cmd/go/internal/load.ImportErrorf({0x7ffeefbff889, 0x3}, {0x159e971, 0x3}, {0xc0001d36b0, 0xc0001d3b58, 0x0})
	/usr/local/Cellar/go/1.17/libexec/src/cmd/go/internal/load/pkg.go:500 +0x19b
cmd/go/internal/load.(*Package).load(0xc000184b00, {0x166f9c0, 0xc000122000}, {0x24, 0x80, 0x2}, {0x7ffeefbff889, 0x0}, 0xc0001d3b58, {0x0, ...}, ...)
	/usr/local/Cellar/go/1.17/libexec/src/cmd/go/internal/load/pkg.go:1846 +0x17dc
cmd/go/internal/load.loadImport({0x166f9c0, 0xc000122000}, {0xc0, 0xf9, 0x66}, 0xc0001196c0, {0x7ffeefbff889, 0x3}, {0xc000028024, 0x17}, ...)
	/usr/local/Cellar/go/1.17/libexec/src/cmd/go/internal/load/pkg.go:718 +0x4dd
cmd/go/internal/load.PackagesAndErrors({0x166f9c0, 0xc000122000}, {0x2d, 0x0, 0x0}, {0xc00011c1a0, 0x1, 0x1})
	/usr/local/Cellar/go/1.17/libexec/src/cmd/go/internal/load/pkg.go:2468 +0x809
cmd/go/internal/work.runBuild({0x166f9c0, 0xc000122000}, 0xc000134480, {0xc00011c1a0, 0x1, 0x1})
	/usr/local/Cellar/go/1.17/libexec/src/cmd/go/internal/work/build.go:371 +0xa5
main.invoke(0x1988580, {0xc00011c190, 0x2, 0x2})
	/usr/local/Cellar/go/1.17/libexec/src/cmd/go/main.go:216 +0x2f6
main.main()
	/usr/local/Cellar/go/1.17/libexec/src/cmd/go/main.go:173 +0x78e

$ go version
go version go1.17 darwin/amd64

In "cmd/go/internal/load/pkg.go", ImportErrorf checks whether the path (importError.importPath) is contained in err (importError.err):

// func ImportErrorf
	if errStr := err.Error(); !strings.Contains(errStr, path) {
		panic(fmt.Sprintf("path %q not in error %q", path, errStr))
	}

err is created by stmt like fmt.Errorf("invalid import path %q", importPath), so the import path in error message is escaped. If import path contians any char that can be escaped by %q, strings.Contains returns false.

IMO, this check is quite unreasonable, do we really need it？

@gopherbot
Copy link

Change https://golang.org/cl/358815 mentions this issue: cmd/go: make escape consistent in ImportErrorf

SilverRainZ added a commit to SilverRainZ/go that referenced this issue Dec 10, 2021
@SilverRainZ
cmd/go/internal/load: prevent set ImportPathError when the err is alr…
1985479 
…eady *module.InvalidPathError

1. The call of setError is unnecessary bacause setError do nothing when
p.Error is not nil.
2. The call of ImportErrorf may panic in this situation
(double quotes appear in the import path).

Fixes golang#49137
SilverRainZ added a commit to SilverRainZ/go that referenced this issue Dec 10, 2021
@SilverRainZ
cmd/go/internal/load: prevent calling ImportErrorf when the err is *m…
54bed3e 
…odule.InvalidPathError

1. The call of setError is unnecessary bacause setError do nothing when
p.Error is not nil.
2. The call of ImportErrorf may panic in this situation
(double quotes appear in the import path).

Fixes golang#49137
@SilverRainZ SilverRainZ linked a pull request Dec 10, 2021 that will close this issue
Open
SilverRainZ added a commit to SilverRainZ/go that referenced this issue Dec 14, 2021
@SilverRainZ
cmd/go/internal/load: prevent calling ImportErrorf when the err is *m…
df89974 
…odule.InvalidPathError

1. The call of setError is unnecessary bacause setError do nothing when
p.Error is not nil.
2. The call of ImportErrorf may panic in this situation
(double quotes appear in the import path).

Fixes golang#49137
@gopherbot
Copy link

Change https://golang.org/cl/372398 mentions this issue: cmd/go/internal/load: prevent calling ImportErrorf when the err is *module.InvalidPathError

SilverRainZ added a commit to SilverRainZ/go that referenced this issue Mar 24, 2022
@SilverRainZ
cmd/go/internal/load: prevent calling ImportErrorf when the err is *m…
0e49fb4 
…odule.InvalidPathError

1. The call of setError is unnecessary bacause setError do nothing when
p.Error is not nil.
2. The call of ImportErrorf may panic in this situation
(double quotes appear in the import path).

Fixes golang#49137
@srinivas-pokala
Copy link
Contributor

srinivas-pokala commented Nov 8, 2022 •
edited

@bcmills , Is the above CL(https://golang.org/cl/372398) working well for the Issue or else is still there is someother changes required for this.

@gopherbot
Copy link

Change https://go.dev/cl/452715 mentions this issue: cmd/go/internal/load: bypass calling ImportErrorf function

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
GoCommand cmd/go help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Backlog
6 participants
@bcmills @noerw @SilverRainZ @gopherbot @seankhliao @srinivas-pokala