misc/cgo/test: TestCrossPackageTests started to fail on release branches

[dmitshur]

The TestCrossPackageTests test is failing on the linux-386-longtest builder on the release-branch.go1.14 branch, from https://build.golang.org/?repo=&branch=release-branch.go1.14:

##### ../misc/cgo/test
--- FAIL: TestCrossPackageTests (0.84s)
    pkg_test.go:67: go test: exit status 2
        panic: runtime error: index out of range [2] with length 2
        
        goroutine 359 [running]:
        cmd/go/internal/work.(*Builder).cgo(0xad1db80, 0xb20db80, 0xb2947b0, 0x22, 0xb1199b0, 0x24, 0x0, 0x0, 0x0, 0x0, ...)
        	/workdir/go/src/cmd/go/internal/work/exec.go:2715 +0x3b42
        cmd/go/internal/work.(*Builder).build(0xad1db80, 0xb20db80, 0x0, 0x0)
        	/workdir/go/src/cmd/go/internal/work/exec.go:597 +0xdd0
        cmd/go/internal/work.(*Builder).Do.func2(0xb20db80)
        	/workdir/go/src/cmd/go/internal/work/exec.go:118 +0x7c
        cmd/go/internal/work.(*Builder).Do.func3(0xafd1370, 0xad1db80, 0xb40ab40)
        	/workdir/go/src/cmd/go/internal/work/exec.go:178 +0x63
        created by cmd/go/internal/work.(*Builder).Do
        	/workdir/go/src/cmd/go/internal/work/exec.go:165 +0x2dd
FAIL
exit status 1
FAIL	misc/cgo/test	2.727s

(Full log at https://build.golang.org/log/9e85634f5e633118d04a37b6ec4acc8af7a0ec06.)

It seems to have started after the most recent minor release, although there were some unrelated failures that may have masked when it first started. It doesn't seem to happen on the release-branch.go1.15 branch at https://build.golang.org/?repo=&branch=release-branch.go1.15. Update: It happened on release-branch.go1.15 branch too after CL 288112 was submitted.

However, it is happening on TryBots for both Go 1.15 and 1.14 release branches, see https://go-review.googlesource.com/c/go/+/288115/1#message-fc09f380d12c52f7cee4887d29ce110b4f7083b3 and https://go-review.googlesource.com/c/go/+/288112/2#message-74b2918d949d3bc7758e86c7a7a41db5e996b8de. (CC @neild.)

CC @rolandshoemaker, @ianlancetaylor, @jayconrod.

[jayconrod]

I took a quick look at this, but I wasn't able to reproduce it by running GO_TEST_SHORT=0 ./all.bash on a linux-386-longtest gomote on the release-branch.go1.14 branch.

The panic occurs in a place where we validate //go:cgo_ldflag pragmas in .go files generated by cgo. The the flags in the CGO_LDFLAGS environment variable as well as flags in #cgo LDFLAGS directives in source files should be written with these pragmas. We only need to validate the flags from #cgo LDFLAGS, since the user is in control of CGO_LDFLAGS.

The panic would occur if the flags from CGO_LDFLAGS weren't written as a consecutive sequence of //go:cgo_ldflag pragmas. I'm not sure why that would happen on this particular builder. If there's a way to manually reproduce this, we should be able to see what's going wrong with the generated files.

[ianlancetaylor]

I'm able to reproduce the problem about 50% of the line on a linux-386-longtest gomote by running, in the /workdir/go/misc/cgo/test directory, the command go test -test.run=TestCrossPackageTests -test.v.

At least in one occasion, the error occurs when the test, which itself invokes go test, is building a copy of the directory misc/cgo/test/testdata/issue9026. The variable flags, derived from the //go:cgo_ldflag comments in the generated files, is ["-g" "-O2"]. That is correct. The variable cgoLDFLAGS, which should come from the environment variable, is ["-g" "-O2" "-lm"]. That is not correct. I can't see any reason why it would contain "-lm".

The extraneous "-lm" may be coming from misc/cgo/test/testdata/issue8756.go or from misc/cgo/test/testdata/issue8756/issue8756.go, both of which have a line #cgo LFLAGS: -lm. I'm not yet sure which or how.

[ianlancetaylor]

The problem is the introduction of cgoEnvCache and the cgoEnv method. It causes sharing of slices across different executions, so the environment passed to cmd/cgo varies.

[ianlancetaylor]

It's fixed on tip by https://golang.org/cl/285873, which drops cgoEnvCache.

@jayconrod Since you already fixed this on tip, leaving for you to decide if we should backport that fix (which was for #43808 and only coincidentally fixes this bug). Let me know if I didn't explain the problem.

[jayconrod]

In that case, merging CL 285952 and CL 285954 will probably fix the issue. They revert the change that added cgoEnvCache. I opened backport CLs to fix a different regression.

[dmitshur]

Thank you for diagnosing this issue. CL 285952 and CL 285954 have been submitted now, which remove cgoEnvCache and so far this test has been passing as expected.

I believe this issue is resolved. We can reopen if we see this again, or if someone thinks there should be a test added to Go 1.16 or 1.17, if there's insufficient test coverage.