cmd/go: list confuses imports

[rsc]

go list std cmd should show information about the standard math/rand package, but it seems to only show information for math/rand compiled for cmd/compile, or at least with dependencies compiled for cmd/compile.

This bug appears to have been introduced between Go 1.20 and Go 1.21.0. I have not bisected further.

% go list -f '{{.Dir}} {{.Imports}}' std |grep /src/math/rand
/Users/rsc/go/src/math/rand [internal/godebug math sync sync/atomic unsafe]
/Users/rsc/go/src/math/rand/v2 [errors internal/chacha8rand math math/bits unsafe]
% go list -f '{{.Dir}} {{.Imports}}' std cmd |grep /src/math/rand 
/Users/rsc/go/src/math/rand [internal/godebug [cmd/compile] math [cmd/compile] sync [cmd/compile] sync/atomic [cmd/compile] unsafe [cmd/compile]]
/Users/rsc/go/src/math/rand/v2 [errors internal/chacha8rand math math/bits unsafe]
% 

[adonovan]

@findleyr I wonder whether this is the source of a number of the metadata inconsistency problems we've been seeing in gopls.

[timothy-king]

I also wonder whether this is related to the various ssa panics that go away when compiling with 1.22 (#65608). That could be explained by a go list inconsistency for a package.

[findleyr]

@adonovan while it does look like there's a bug in cmd/go, I believe for this specific case we won't encounter that bug in go/packages as we disabled PGO in go list in https://go.dev/issue/60456.

[aclements]

It does seem quite likely this is related to PGO.

[aclements]

git checkout go1.21.0 && git qbisect --from go1.20 --setup ./make.bash zsh -c "go list -f '{{.Dir}} {{.Imports}}' std cmd |grep /src/math/rand | grep 'math sync'" says 45b74d2, which confirms PGO, but otherwise isn't very helpful.

[thanm]

Wondering what is happening with this bug (during weekly release checkin meeting). Any updates?

[dmitshur]

@matloob Are there updates on this? This is currently a blocker for Go 1.23 RC 1. Is this important enough to be considered a release blocker for Go 1.23?

[matloob]

No updates yet. I plan to look at this next week.

[cagedmantis]

@matloob Is there is status update on this issue? This came up in the release meeting.

[matloob]

Not yet. I plan to start looking tomorrow.

[matloob]

So it looks like what's happening is that while when we split off the pgo variant of a package, while we clone the Internal.Imports slice so that the actual import links between packages are different, we don't do the same thing for the (non-internal) Imports field. See

go/src/cmd/go/internal/load/pkg.go

Line 2948 in 1c92457

p1.Internal.Imports = slices.Clone(p.Internal.Imports)

for where the split is done.

The split code is does the following

	p1 := new(Package)
	*p1 = *p
	p1.Internal.Imports = slices.Clone(p.Internal.Imports)
	p1.Internal.ForMain = pmain.ImportPath

where p is the original package. So the slices point to the same arrays and other pointers in p at the time of the split are shared.

So the pgo and non pgo variants share the same (PackagePublic) Imports slice. When the list code updates the package public Imports, it then ends up modifying the original too.

[gopherbot]

Change https://go.dev/cl/588275 mentions this issue: cmd/go/internal/load: clone pgo variant's PackagePublic.Imports in split