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

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?

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.

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

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.

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.

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

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.

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.

@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.)