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

Weird panic in garble test #522

Closed
0x1a8510f2 opened this issue Apr 15, 2022 · 5 comments · Fixed by #527
Closed

Weird panic in garble test #522

0x1a8510f2 opened this issue Apr 15, 2022 · 5 comments · Fixed by #527
Milestone

Comments

@0x1a8510f2
Copy link

What version of Garble and Go are you using?

$ garble version
mvdan.cc/garble v0.6.1-0.20220407152631-c8e0abf9c9bd h1:uJh/qbuqVyzoIaFQgA3fbFeOnyfXMmhix9zpxrfgSt8=

Build settings:
       -compiler gc
     CGO_ENABLED 1
          GOARCH amd64
            GOOS linux
         GOAMD64 v1

$ go version
go version go1.18.1 linux/amd64

What environment are you running Garble on?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/[REDACTED]/.cache/go-build"
GOENV="/home/[REDACTED]/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/[REDACTED]/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/[REDACTED]/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib64/go/1.18"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib64/go/1.18/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.18.1"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="[REDACTED]"
GOWORK=""
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-build2003152507=/tmp/go-build -gno-record-gcc-switches"

What did you do?

While attempting to test a package with garble test, garble panics while go test succeeds. Please see build logs below (note: the environment is different to that detailed above but the result is the same):

Package test log (successful) with regular Go command: https://ci.0x1a8510f2.space/0x1a8510f2/wraith/111/1/2
Package test log (failed) with garble command: https://ci.0x1a8510f2.space/0x1a8510f2/wraith/110/1/2
File causing issues: https://git.0x1a8510f2.space/0x1a8510f2/wraith/src/branch/indev/wraith/libwraith/Shm_test.go
Relevant garble code:

garble/shared.go

Lines 386 to 391 in 1a0b028

for _, dep := range curPkg.Deps {
if dep == pkg.ImportPath {
return pkg, nil
}
}
return nil, fmt.Errorf("refusing to list non-dependency package: %s", path)

What did you expect to see?

Successful test as with Go command

What did you see instead?

> garble test ./...
?   	git.0x1a8510f2.space/0x1a8510f2/wraith/wraith	[no test files]
# git.0x1a8510f2.space/0x1a8510f2/wraith/wraith/libwraith [git.0x1a8510f2.space/0x1a8510f2/wraith/wraith/libwraith.test]
panic: refusing to list non-dependency package: git.0x1a8510f2.space/0x1a8510f2/wraith/wraith/libwraith

goroutine 1 [running]:
main.processImportCfg({0xc00012db00?, 0xc00028a980?, 0x6?})
	/home/[REDACTED]/go/pkg/mod/mvdan.cc/garble@v0.6.1-0.20220407152631-c8e0abf9c9bd/main.go:975 +0x532
main.transformCompile({0xc000003a20?, 0x17?, 0x18?})
	/home/[REDACTED]/go/pkg/mod/mvdan.cc/garble@v0.6.1-0.20220407152631-c8e0abf9c9bd/main.go:736 +0x330
main.mainErr({0xc000003a10, 0x18, 0x19})
	/home/[REDACTED]/go/pkg/mod/mvdan.cc/garble@v0.6.1-0.20220407152631-c8e0abf9c9bd/main.go:416 +0x528
main.main1()
	/home/[REDACTED]/go/pkg/mod/mvdan.cc/garble@v0.6.1-0.20220407152631-c8e0abf9c9bd/main.go:236 +0x16e
main.main()
	/home/[REDACTED]/go/pkg/mod/mvdan.cc/garble@v0.6.1-0.20220407152631-c8e0abf9c9bd/main.go:133 +0x19
FAIL	git.0x1a8510f2.space/0x1a8510f2/wraith/wraith/libwraith [build failed]
?   	git.0x1a8510f2.space/0x1a8510f2/wraith/wraith/stdmod	[no test files]
FAIL
exit status 2
@mvdan
Copy link
Member

mvdan commented Apr 22, 2022

Could you please share full repro steps, e.g. including git clone, git checkout, cd, garble test etc?

@0x1a8510f2
Copy link
Author

Sure!

git clone https://git.0x1a8510f2.space/0x1a8510f2/wraith
cd wraith/wraith
git checkout 26de8979cfc0b6e6213a07d7507ce467c0a762e4
go test ./...
garble test ./...

The garble invocation panics while go succeeds.

@mvdan
Copy link
Member

mvdan commented Apr 22, 2022

Thanks, I can reproduce.

mvdan added a commit to mvdan/garble-fork that referenced this issue Apr 22, 2022
The added test case reproduces the failure if we uncomment the added
"continue" line in processImportCfg:

	# test/bar/exporttest [test/bar/exporttest.test]
	panic: refusing to list non-dependency package: test/bar/exporttest

	goroutine 1 [running]:
	mvdan.cc/garble.processImportCfg({0xc000166780?, 0xc0001f4a70?, 0x2?})
		/home/mvdan/src/garble/main.go:983 +0x58b
	mvdan.cc/garble.transformCompile({0xc000124020?, 0x11?, 0x12?})
		/home/mvdan/src/garble/main.go:736 +0x338

It seems like a quirk of cmd/go that it includes a redundant packagefile
line in this particular edge case, but it's generally harmless for "go
build". For "garble build" it's also harmless in principle, but in
practice we had sanity checks that got upset by the unexpected line.

For now, notice the edge case and ignore it.

Fixes burrowers#522.
@mvdan
Copy link
Member

mvdan commented Apr 22, 2022

Can you confirm that the fix above works for you?

@lu4p lu4p closed this as completed in #527 Apr 22, 2022
lu4p pushed a commit that referenced this issue Apr 22, 2022
The added test case reproduces the failure if we uncomment the added
"continue" line in processImportCfg:

	# test/bar/exporttest [test/bar/exporttest.test]
	panic: refusing to list non-dependency package: test/bar/exporttest

	goroutine 1 [running]:
	mvdan.cc/garble.processImportCfg({0xc000166780?, 0xc0001f4a70?, 0x2?})
		/home/mvdan/src/garble/main.go:983 +0x58b
	mvdan.cc/garble.transformCompile({0xc000124020?, 0x11?, 0x12?})
		/home/mvdan/src/garble/main.go:736 +0x338

It seems like a quirk of cmd/go that it includes a redundant packagefile
line in this particular edge case, but it's generally harmless for "go
build". For "garble build" it's also harmless in principle, but in
practice we had sanity checks that got upset by the unexpected line.

For now, notice the edge case and ignore it.

Fixes #522.
@0x1a8510f2
Copy link
Author

0x1a8510f2 commented Apr 23, 2022

@mvdan Can indeed confirm: https://ci.0x1a8510f2.space/0x1a8510f2/wraith/112/1/2
Thanks for the quick fix!

@mvdan mvdan added this to the v0.6.0 milestone May 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants