cmd/dist: bootstrap of tip fails with GOEXPERIMENT=newinliner and added VERSION file

[thanm]

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

$ go version
go version devel go1.22-e14b96cb51 Tue Nov 14 16:45:59 2023 +0000 linux/amd64

Does this issue reproduce with the latest release?

No.

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

linux/amd64

What did you do?

Reported by Heschi: if you take a tip (1.22) Go repo and drop a VERSION file into it of the following form:

go1.22beta1
time 2023-11-14T23:00:34Z

then run GOEXPERIMENT=newinliner bash make.bash the bootstrap build fails (works ok if no VERSION file).

Symptom:

 Building Go bootstrap cmd/go (go_bootstrap) using Go toolchain1.
 Building Go toolchain2 using go_bootstrap and Go toolchain1.
 Building Go toolchain3 using go_bootstrap and Go toolchain2.
 Building packages and commands for linux/amd64.
 # crypto/cipher
 crypto/cipher/ofb.go:24:6: internal compiler error: panic: unexpected decoding error: EOF
 Please file a bug report including a short program that triggers the error.
 https://go.dev/issue/new
 # internal/coverage/cmerge
 internal/coverage/defs.go:198:6: internal compiler error: panic: unexpected decoding error: EOF
 Please file a bug report including a short program that triggers the error.
 https://go.dev/issue/new
 # internal/coverage/decodemeta
 internal/coverage/defs.go:198:6: internal compiler error: panic: unexpected decoding error: EOF
 Please file a bug report including a short program that triggers the error.
 https://go.dev/issue/new
 # internal/coverage/encodemeta
 internal/coverage/defs.go:198:6: internal compiler error: panic: unexpected decoding error: EOF
 Please file a bug report including a short program that triggers the error.

I've done a little poking around and it is indeed related to the goexperiment ; stack trace at the point of the failure appears to indicate that the compiler is trying to read the "function properties" string for an inlinable function from the object file and encountering an early EOF -- this would seem to point to some sort of mixup in object file flavors? Not clear. Representative trace at point of failure:

# crypto/rc4
goroutine 1 [running]:
runtime/debug.Stack()
	runtime/debug/stack.go:24 +0x5e
runtime/debug.PrintStack()
	runtime/debug/stack.go:16 +0x13
internal/pkgbits.(*Decoder).checkErr(0xc0004fd2f8?, {0xf4f300, 0x145e750})
	internal/pkgbits/decoder.go:246 +0x2c
internal/pkgbits.(*Decoder).rawUvarint(...)
	internal/pkgbits/decoder.go:253
internal/pkgbits.(*Decoder).Uint64(0xc000534dc0)
	internal/pkgbits/decoder.go:379 +0xa5
internal/pkgbits.(*Decoder).Len(...)
	internal/pkgbits/decoder.go:383
internal/pkgbits.(*Decoder).Reloc(0xc000534dc0, 0x0)
	internal/pkgbits/decoder.go:407 +0x8d
internal/pkgbits.(*Decoder).String(0xc000534dc0)
	internal/pkgbits/decoder.go:414 +0x89
cmd/compile/internal/noder.(*reader).funcExt(0xc000534dc0, 0xc0005151e0, 0x0)
	cmd/compile/internal/noder/reader.go:1107 +0x44c
cmd/compile/internal/noder.(*pkgReader).objIdx(0xc0004dc000, 0x3, {0x0?, 0x0, 0x0}, {0x1556f20?, 0x0, 0x0}, 0x0)
	cmd/compile/internal/noder/reader.go:768 +0xdcd
cmd/compile/internal/noder.(*pkgReader).objIdx(0xc0004dc540, 0x3, {0x0?, 0x0, 0x0}, {0x1556f20?, 0x0, 0x0}, 0x0)
	cmd/compile/internal/noder/reader.go:678 +0x4da
cmd/compile/internal/noder.(*pkgReader).objInstIdx(...)
	cmd/compile/internal/noder/reader.go:660
cmd/compile/internal/noder.(*reader).obj(0xc0005343c0)
	cmd/compile/internal/noder/reader.go:632 +0x88
cmd/compile/internal/noder.(*reader).expr(0xc0005343c0)
	cmd/compile/internal/noder/reader.go:2037 +0x4c5
cmd/compile/internal/noder.(*reader).expr(0xc0005343c0)
	cmd/compile/internal/noder/reader.go:2363 +0x26da
cmd/compile/internal/noder.(*reader).expr(0xc0005343c0)
	cmd/compile/internal/noder/reader.go:2299 +0x19fd
cmd/compile/internal/noder.(*reader).expr(0xc0005343c0)
	cmd/compile/internal/noder/reader.go:2363 +0x26da
cmd/compile/internal/noder.(*reader).ifStmt(0xc0005343c0)
	cmd/compile/internal/noder/reader.go:1798 +0x7f
cmd/compile/internal/noder.(*reader).stmt1(0xc0005343c0, 0xc000530700?, 0xc0004ff600)
	cmd/compile/internal/noder/reader.go:1668 +0x711
cmd/compile/internal/noder.(*reader).stmts(0xc0005343c0)
	cmd/compile/internal/noder/reader.go:1581 +0xa5
cmd/compile/internal/noder.(*reader).funcBody.func1()
	cmd/compile/internal/noder/reader.go:1234 +0x66
cmd/compile/internal/ir.WithFunc(0x394?, 0x31?)
	cmd/compile/internal/ir/func.go:387 +0x95
cmd/compile/internal/noder.(*reader).funcBody(0xc0005343c0, 0xc0004f7200)
	cmd/compile/internal/noder/reader.go:1223 +0xe6
cmd/compile/internal/noder.pkgReaderIndex.funcBody({0xc0004dc540, 0x1, 0xc0004e2c30, 0xc0005013c0, 0x0}, 0xc0004f7200)
	cmd/compile/internal/noder/reader.go:1211 +0xf2
cmd/compile/internal/noder.readBodies(0xc0002100c0, 0x0)
	cmd/compile/internal/noder/unified.go:242 +0x16a
cmd/compile/internal/noder.unified(...)
	cmd/compile/internal/noder/unified.go:182
cmd/compile/internal/noder.LoadPackage({0xc0001c6100?, 0x1, 0x2})
	cmd/compile/internal/noder/noder.go:77 +0x5cb
cmd/compile/internal/gc.Main(0xe23cd0)
	cmd/compile/internal/gc/main.go:197 +0xbbd
main.main()
	cmd/compile/main.go:57 +0xf9
crypto/rc4/rc4.go:31:6: internal compiler error: panic: unexpected decoding error: EOF

[gopherbot]

Change https://go.dev/cl/542995 mentions this issue: cmd/dist: clear GOCACHE when building with newinliner

[thanm]

OK, I think I have worked this out.

cmd/dist is indeed a maze of twisty little passages.

Here's what I think is happening: in the build part of cmd/dist, at this line here we have finished building toolchain1 and are about to build toolchain2:

https://go.googlesource.com/go/+/ff19f8e7636f0b5797f3b65cee69f41fb650b965/src/cmd/dist/build.go#1467

This code sets GOEXPERIMENT=newinliner, then kicks off the build of toolchain2:

goInstall(toolenv(), goBootstrap, append([]string{"-pgo=off"}, toolchain...)...)

The problem here is that at the point when we start building dependent packages needed for the toolchain, the installed compiler is not producing GOEXPERIMENT=newinliner objects, but the GOEXPERIMENT env variable is set. Thus any object produced at this point and entered into the cache will be bad (it will advertise itself as a newinliner object, but will not be one).

I hacked up internal/pkgbits/decoder.go to show the name of the input file it is reading at the point where it hits the fatal error. It points to this object:

# vendor/golang.org/x/crypto/internal/poly1305
=-= pkgPath=crypto/subtle
=-= objpath=/ssd2/go.master/pkg/obj/go-build/47/47fc3132c7124dc248bc85c9dc51aba8942337da3ffc2e57a5b20ae60520c7b8-d

which is coming out of the go-build cache. I looked through the "go build -x" output for the toolchain2 build, and the "crypto/subtle" is built and copied into the cache before the new GOEXPERIMENT=newinliner compiler is installed.

I can think of a couple of ways to attack this. If we don't want to change the dist command at all, then we could change the Go command to compute hash keys not just with the GOEXPERIMENT setting but also the GOEXPERIMENT values baked into each of the components tools (compile, link, etc). Not sure this a great idea; seems like it would slow things down in the common case just to handle the weird corner cases where we're in the mid-experiment change.

The other alternative would be to do some sort of cache cleaning in "dist" along the lines of what is in the CL that Cuong sent. Seems to me that the best place for the clean would be immediately after the toolchain2 build (if GOEXPERIMENT is set).

Thoughts?

[thanm]

Another possible fix: change the compiler to always read/write the func.Inl.Properties strings, but just write an empty string if the new inliner is turned off. This would result in a tiny increase in object file size for the nonewinliner case, but would make the object files compatible.

[gopherbot]

Change https://go.dev/cl/543215 mentions this issue: cmd/compile/internal/noder: fix compatibility issue with fn.Inl.Properties

[mdempsky]

The problem here is that at the point when we start building dependent packages needed for the toolchain, the installed compiler is not producing GOEXPERIMENT=newinliner objects, but the GOEXPERIMENT env variable is set.

Why is this happening? If we set GOEXPERIMENT=newinliner, then the newinliner should be enabled.

This seems like a cmd/go issue.

[thanm]

Why is this happening? If we set GOEXPERIMENT=newinliner, then the newinliner should be enabled.

My (somewhat hazy) recollection is that for Go experiments that control what the compiler does, the value of the experiment is baked into the cmd/compile binary at build time (happy to be corrected if this is not right BTW). Hence if you install a GOEXPERIMENT=newinliner compiler, then run

$ GOEXPERIMENT=nonewinliner go build my/program

you'll actually still be getting objects built with GOEXPERIMENT=newinliner.

[gopherbot]

Change https://go.dev/cl/543196 mentions this issue: cmd/dist: clean build cache after toolchain2 if non-empty goexperiment

[mdempsky]

My (somewhat hazy) recollection is that for Go experiments that control what the compiler does, the value of the experiment is baked into the cmd/compile binary at build time (happy to be corrected if this is not right BTW).

The GOEXPERIMENT environment variable is supposed to work the same as GOOS and GOARCH. That is, if you set it while running go build, then it should determine what experiments are used for the target binary, independent of what experiments were used for the compiler itself.

[bcmills]

Why is this happening? If we set GOEXPERIMENT=newinliner, then the newinliner should be enabled.

This seems like a cmd/go issue.

I looked several times and couldn't find any place where cmd/go would be modifying the GOEXPERIMENT environment variable. I expect that it is being passed through unchanged from the environment..?

So I think the likely explanations are that either cmd/go is producing the wrong cache key for the object file (inserting a GOEXPERIMENT in the cache key where it is not enabled, or omitting it when it is actually needed), or cmd/compile is not correctly applying the GOEXPERIMENT settings from its environment.

[mdempsky]

@bcmills Thanks for looking. I agree that I think it's actually a cmd/compile issue. (See my latest comment at https://go.dev/cl/543215 for details; tldr, we're using internal/goexperiment where we should use internal/buildcfg.)