cmd/compile: fails to compile very long files starting go1.17

[sywhang]

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

My dev laptop is m1 Mac:

$ go version
go version go1.17.1 darwin/arm64

But we also verified this fails on linux/amd64 as well.

Does this issue reproduce with the latest release?

Yes, including the go1.18 beta.

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

go env Output

$ go env
GO111MODULE=""
GOARCH="arm64"
GOBIN="/Users/sungyoon/go/bin/"
GOCACHE="/Users/sungyoon/Library/Caches/go-build"
GOENV="/Users/sungyoon/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="arm64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/sungyoon/go118/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/sungyoon/go118/go/"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/Users/sungyoon/go118/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/Users/sungyoon/go118/go/pkg/tool/darwin_arm64"
GOVCS=""
GOVERSION="go1.17.1"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/l1/s1n3wjrj7hn5fwgwdn70kzww0000gn/T/go-build3915184577=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Some teams here at Uber were using go-bindata to embed binaries into Go file, which resulted in a very lengthy file (>1 mil lines of Go).

In Go 1.17 these files started seeing build failures, with the following compiler error:

./generated2.go:1048574:3: internal compiler error: non-monotonic scope positions
        /Users/sungyoon/repro/generated2.go:1048574:42: previous scope position

Please file a bug report including a short program that triggers the error.
https://go.dev/issue/new

What did you expect to see?

The file being compiled.

What did you see instead?

The compiler error above.

Additional Details

We dug into the source code and found out that this happens due to an overflow bug in cmd/compile. Specifically, the newly added dwarfgen package does not account for the case where XPos can be non-monotonic increment between two characters due to Lico using only 20 bits to represent the line number:

https://github.com/golang/go/blob/3d7f83612390d913e7e8bb4ffa3dc69c41b3078d/src/cmd/internal/src/pos.go#L313
// A lico is a compact encoding of a LIne and COlumn number.
type lico uint32

// Layout constants: 20 bits for line, 8 bits for column, 2 for isStmt, 2 for pro/epilogue
// (If this is too tight, we can either make lico 64b wide,
// or we can introduce a tiered encoding where we remove column
// information as line numbers grow bigger; similar to what gcc
// does.)
// The bitfield order is chosen to make IsStmt be the least significant
// part of a position; its use is to communicate statement edges through
// instruction scrambling in code generation, not to impose an order.
// TODO: Prologue and epilogue are perhaps better handled as pseudo-ops for the assembler,
// because they have almost no interaction with other uses of the position.

We are unblocked by changing these files to not do this by using go:embed (or opting to not generate DWARF symbols), but I do feel that 20 bits is a bit too limiting since it's possible for code-generated files to reach this length.

We tried a local fix as suggested in the comment - by changing lico to uint64 and changing the layout as 10 bits reserved (empty), 42 bits for line, 8 bits for column, 2 for isStmt, 2 for pro/epilogue and verified that the fix works.

We're happy to submit this fix but I'm posting an issue here before submitting it to discuss potential alternatives. Thanks!

[ALTree]

cc @griesemer for decision.

[randall77]

Another fix is to ensure that when we saturate the line field, we always use column 1. That way incrementing a position never goes backwards.

[randall77]

Does this work?

--- a/src/cmd/internal/src/pos.go
+++ b/src/cmd/internal/src/pos.go
@@ -389,9 +389,10 @@ func makeBogusLico() lico {
 }
 
 func makeLico(line, col uint) lico {
-       if line > lineMax {
+       if line >= lineMax {
                // cannot represent line, use max. line so we have some information
                line = lineMax
+               col = 1
        }
        if col > colMax {
                // cannot represent column, use max. column so we have some information

[ianlancetaylor]

CC @thanm because DWARF.

[abhinav]

@randall77 just checked; that patch also fixes the issue.

[gopherbot]

Change https://go.dev/cl/385795 mentions this issue: cmd/compile: drop column info when line number saturates

[thanm]

@sywhang thanks for the report.

Just for my curiosity, what is the advantage of using go-bindata as opposed to Go's "go:embed" feature, e.g. https://pkg.go.dev/embed?

[abhinav]

Just for my curiosity, what is the advantage of using go-bindata as opposed to Go's "go:embed" feature, e.g. https://pkg.go.dev/embed?

There is no advantage to using go-bindata over go:embed that we're aware of.
This was just one of those cases that had not yet migrated from go-bindata to go:embed.

[sywhang]

@thanm there isn't. This existed prior to go:embed feature.

[shawndx]

My apologies for commenting a closed issue, we ran into the error in practice when compiling a large auto-generated file, feasible to make a backport? Thanks.

@gopherbot please consider this for backport to 1.17, it's a compiler error.