cmd/go: "go list -f '{{.Stale}}'" stack overflow with cyclic imports

[jwebb]

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

go version go1.10.3 linux/amd64

Does this issue reproduce with the latest release?

Yes. We also saw this with 1.10.2.

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

GOARCH="amd64"
GOBIN=""
GOCACHE="/home/jwebb/.cache/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/jwebb/gocode"
GORACE=""
GOROOT="/home/jwebb/opt/go"
GOTMPDIR=""
GOTOOLDIR="/home/jwebb/opt/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
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-build138291515=/tmp/go-build -gno-record-gcc-switches"

What did you do?

This appears to be triggered by any cyclic import, but only with use of the {{.Stale}} template, e.g.:

#!/bin/bash

set -ex

export GOPATH="$PWD"
mkdir -p src/testorg/pkg{1,2}

cat > src/testorg/pkg1/pkg1.go <<EOF
package pkg1

import "testorg/pkg2"
EOF

cat > src/testorg/pkg2/pkg2.go <<EOF
package pkg2

import "testorg/pkg1"
EOF

go list -f '{{.Stale}}' testorg/...

What did you expect to see?

An error message about the cyclic import, but no crash.

What did you see instead?

A stack overflow:

can't load package: import cycle not allowed
package testorg/pkg1
	imports testorg/pkg2
	imports testorg/pkg1
runtime: goroutine stack exceeds 1000000000-byte limit
fatal error: stack overflow

runtime stack:
runtime.throw(0x8af01b, 0xe)
	/usr/local/go/src/runtime/panic.go:616 +0x81
runtime.newstack()
	/usr/local/go/src/runtime/stack.go:1054 +0x71f
runtime.morestack()
	/usr/local/go/src/runtime/asm_amd64.s:480 +0x89

goroutine 1 [running]:
fmt.(*fmt).pad(0xc421a22040, 0xc421a220a5, 0x7, 0x7)
	/usr/local/go/src/fmt/format.go:90 +0x3a9 fp=0xc440b16350 sp=0xc440b16348 pc=0x4bff39
fmt.(*fmt).fmt_integer(0xc421a22040, 0x1642c1, 0xa, 0x1, 0x8b0091, 0x11)
	/usr/local/go/src/fmt/format.go:307 +0x1db fp=0xc440b163a0 sp=0xc440b16350 pc=0x4c08fb
fmt.(*pp).fmtInteger(0xc421a22000, 0x1642c1, 0x6400000001)
	/usr/local/go/src/fmt/print.go:369 +0x1e5 fp=0xc440b163e0 sp=0xc440b163a0 pc=0x4c4105
fmt.(*pp).printArg(0xc421a22000, 0x80f6c0, 0xc4401dd5a8, 0x64)
	/usr/local/go/src/fmt/print.go:649 +0x837 fp=0xc440b16458 sp=0xc440b163e0 pc=0x4c6717
fmt.(*pp).doPrintf(0xc421a22000, 0x8aa476, 0x5, 0xc440b165d0, 0x1, 0x1)
	/usr/local/go/src/fmt/print.go:1099 +0x3fa fp=0xc440b16540 sp=0xc440b16458 pc=0x4c9ada
fmt.Sprintf(0x8aa476, 0x5, 0xc440b165d0, 0x1, 0x1, 0x20, 0x0)
	/usr/local/go/src/fmt/print.go:203 +0x66 fp=0xc440b16598 sp=0xc440b16540 pc=0x4c2c26
cmd/go/internal/work.(*Builder).NewObjdir(0xc420258460, 0x20, 0xc440b166b8)
	/usr/local/go/src/cmd/go/internal/work/action.go:257 +0x94 fp=0xc440b16610 sp=0xc440b16598 pc=0x5ea314
cmd/go/internal/work.(*Builder).CompileAction.func1(0x830b00)
	/usr/local/go/src/cmd/go/internal/work/action.go:335 +0x53 fp=0xc440b166b8 sp=0xc440b16610 pc=0x621073
cmd/go/internal/work.(*Builder).cacheAction(0xc420258460, 0x8aa494, 0x5, 0xc420086c00, 0xc440b16750, 0x412138)
	/usr/local/go/src/cmd/go/internal/work/action.go:300 +0x9f fp=0xc440b16720 sp=0xc440b166b8 pc=0x5eabef
cmd/go/internal/work.(*Builder).CompileAction(0xc420258460, 0x1, 0x1, 0xc420086c00, 0xc43445b1ff)
	/usr/local/go/src/cmd/go/internal/work/action.go:330 +0xc0 fp=0xc440b16780 sp=0xc440b16720 pc=0x5eae10
cmd/go/internal/work.(*Builder).CompileAction.func1(0x830b00)
	/usr/local/go/src/cmd/go/internal/work/action.go:339 +0x16e fp=0xc440b16828 sp=0xc440b16780 pc=0x62118e
cmd/go/internal/work.(*Builder).cacheAction(0xc420258460, 0x8aa494, 0x5, 0xc420086800, 0xc440b168c0, 0x412138)
	/usr/local/go/src/cmd/go/internal/work/action.go:300 +0x9f fp=0xc440b16890 sp=0xc440b16828 pc=0x5eabef

etc.

[iwdgo]

This issue cannot be reproduced on go1.12.2.

$ go version
go version go1.12.2 windows/amd64
$ ls $GOPATH/src
github.com/  golang.org/  google.golang.org/  rsc.io/  testorg/
$ go list -f '{{ .Stale }}' testorg/...
can't load package: import cycle not allowed
package testorg/pkg1
        imports testorg/pkg2
        imports testorg/pkg1
true
$ ls -R $GOPATH/src/testorg/
/c/Users/***/Documents/Google/src/testorg/:
pkg1/  pkg2/

/c/Users/***/Documents/Google/src/testorg/pkg1:
pkg1.go

/c/Users/***/Documents/Google/src/testorg/pkg2:
pkg2.go

The description above is placing files in $GOPATH/src/src but this is not creating an issue either as src is appended to GOPATH. Issue seems solved.

[bcmills]

We got a report via Slack of what seems to be the same crash on Go 1.15.7, with the following cycle in the stack trace:

go/src/cmd/go/internal/work/action.go

Line 371 in e8e7fac

a = f()

go/src/cmd/go/internal/work/action.go

Line 404 in e8e7fac

a := b.cacheAction("build", p, func() *Action {

go/src/cmd/go/internal/work/action.go

Line 414 in e8e7fac

a.Deps = append(a.Deps, b.CompileAction(depMode, depMode, p1))

Like the original report — and like #43279 — the top of the stack at the time of the crash was in the fmt package via (*Builder).NewObjdir. It seems that this bug is still present.

[bcmills]

I'm intrigued by the condition that is supposed to guard against import cycles here:

go/src/cmd/go/internal/work/action.go

Line 412 in e8e7fac

if p.Error == nil || !p.Error.IsImportCycle {

Perhaps p.Error is starting out as an import cycle error, butt somehow being overwritten with some error for which p.Error.IsImportCycle is false?

CC @jayconrod @matloob

[rolandshoemaker]

I ran into this when analyzing some gVisor code. In particular the package gvisor.dev/gvisor/pkg/sync will trigger the issue. It appears to be caused by two packages being defined in the same directory (here sync and seqatomic), causing p.Error to be populated with a MultiplePackageError, while also containing an import loop (seqatomic imports sync).

Because p.Error != nil and p.Error.IsImportCycle != true we get stuck in the CompileAction loop. This is obviously a very weird situation, and I'm honestly not really sure what the correct solution is. It seems like probably reusePackage should either be setting IsImportCycle when it detects a loop, and p.Error is already set, or it should stomp the existing p.Error and overwrite with the import loop error, since that is likely to be the more useful error, but my familiarity with this area of the codebase is pretty limited, so there might be a good reason not to do that.

[jayconrod]

Reproduced at gvisor.dev/gvisor@ab488702a by running go list -compiled -e.

Thank you!

[gopherbot]

Change https://golang.org/cl/301289 mentions this issue: cmd/go/internal/load: always set IsImportCycle when in a cycle

[rolandshoemaker]

Threw together a quick CL with what I imagine is the simplest solution to this problem, feel free to ignore if you think there is a better approach.

[stamblerre]

@gopherbot please consider this for backport to 1.16 and 1.15

[gopherbot]

Backport issue(s) opened: #47347 (for 1.15), #47348 (for 1.16).

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

[gopherbot]

Change https://golang.org/cl/336649 mentions this issue: [release-branch.go1.16] cmd/go/internal/load: always set IsImportCycle when in a cycle

[gopherbot]

Change https://golang.org/cl/336669 mentions this issue: [release-branch.go1.15] cmd/go/internal/load: always set IsImportCycle when in a cycle