Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cmd/compile: duplicate anonymous interface declarations cause non-determinism #30202

Closed
jellevandenhooff opened this issue Feb 13, 2019 · 11 comments
Closed

cmd/compile: duplicate anonymous interface declarations cause non-determinism #30202

jellevandenhooff opened this issue Feb 13, 2019 · 11 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone
Go1.13

Comments

@jellevandenhooff
Copy link
Contributor

jellevandenhooff commented Feb 13, 2019 •
edited

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

$ go 1.11.5

Does this issue reproduce with the latest release?

This happens on both go 1.11.5 and go 1.12rc1

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

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/jelle/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/jelle/hack/backend/go"
GOPROXY=""
GORACE=""
GOROOT="/Users/jelle/hack/backend/opt/go1.11.5"
GOTMPDIR=""
GOTOOLDIR="/Users/jelle/hack/backend/opt/go1.11.5/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/7n/4n_h9yg109b8kkpk1974y86h0000gp/T/go-build173276175=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Similar to #27013 but with the addition of concurrency, the go compiler produces a non-deterministic small.a output when running on the following minimized test program.

package small

func A(x interface {
        X() int
}) int {
        return x.X()
}

func B(x interface {
        X() int
}) int {
        return x.X()
}

To demonstrate:

for i in {1..20}; do
  go tool compile -c 2 -p small -linkobj small.a small.go &&
  md5 small.a
done | sort | uniq -c

What did you expect to see?

The go compiler produces the same output each time it is run, since it receives the same inputs.

The little script above should only see one hash from the 20 small.a files.

What did you see instead?

The go compiler produces non-deterministic output

  12 MD5 (small.a) = bc3a55c714f25041576fe53690c16732
   8 MD5 (small.a) = fae691f5253d0dcb5991b1689c3c6006

Notes

This issue is related to #27013. The fix for that issue (https://go-review.googlesource.com/c/go/+/129515/) does not work for concurrent builds. It imposes an ordering
based on invocation order of addsignat in src/cmd/compile/internal/gc.go, but that invocation order is non-deterministic.

This problems occurs in the wild in https://github.com/gogo/protobuf/blob/master/types/duration_gogo.go.
@jellevandenhooff jellevandenhooff mentioned this issue Feb 13, 2019
Closed
@gopherbot
Copy link

Change https://golang.org/cl/162240 mentions this issue: cmd/compile: make duplicate anonymous interface output deterministic

@odeke-em
Copy link
Member

Thank you for reporting this issue @jellevandenhooff!

I shall page some folks about it @josharian @mdempsky @randall77

@odeke-em odeke-em added the NeedsFix The path to resolution is known, but the work has not been done. label Feb 13, 2019
@odeke-em odeke-em added this to the Go1.13 milestone Feb 13, 2019
@bradfitz bradfitz added release-blocker CherryPickCandidate Used during the release process for point releases labels Feb 13, 2019
@jellevandenhooff
Copy link
Contributor Author

I have a bit of a dream as a follow-up... It would be fantastic if there was some job scraping Github for go packages, build them a couple times, and find packages exhibiting non-deterministic builds.

@josharian
Copy link
Contributor

I did that in Go 1.9, as part of thoroughly testing concurrent compilation. :)

@cuonglm
Copy link
Member

cuonglm commented Mar 4, 2019

@josharian @mdempsky @randall77 sorry to bother you in this issue, but related to #29612

Should 2 anonymous interface in different package go to the same bucket in itable?

@randall77
Copy link
Contributor

@Gnouc I'm not sure entirely what you are asking.

Are you talking about the runtime itabTable in runtime/iface.go? I'm going to assume you are.

That table is a hash table. The general rule is that two equal keys must map to the same bucket. Different keys should map to different buckets, but they don't have to. Different keys mapping to the same bucket make the code slower, but not wrong.

So ideally two otherwise identical anonymous interfaces in different packages should map to different buckets, but they don't have to. Looking at the code, it seems like they would map to the same bucket, as we use a hash of the string representation of the interface type. Not ideal, but this case probably doesn't happen often enough to warrant doing anything about it.

@cuonglm
Copy link
Member

cuonglm commented Mar 4, 2019

@randall77 Thanks for the information. Can you take a look at #29612 when you have time.

@bradfitz bradfitz removed the CherryPickCandidate Used during the release process for point releases label Mar 13, 2019
@josharian josharian mentioned this issue May 11, 2019
Closed
@josharian
Copy link
Contributor

Bisecting #31975, which is a duplicate of this issue, yielded https://go-review.googlesource.com/106796, which was the introduction of the indexed export format. Not a surprise, in retrospect, but good to confirm.

Of note: https://go-review.googlesource.com/c/go/+/170037 added anonymous interfaces to the standard library (internal/oserror), so this now directly causes toolstash-check to have spurious failures.

@andybons
Copy link
Member

@randall77 any updates on this?

@josharian
Copy link
Contributor

@andybons https://golang.org/cl/162240 needs finishing up

@gopherbot
Copy link

Change https://golang.org/cl/183852 mentions this issue: cmd/compile: make duplicate anonymous interface output deterministic

@golang golang locked and limited conversation to collaborators Jul 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Projects
None yet
Milestone
Go1.13
Development

Successfully merging a pull request may close this issue.

cmd/compile: make duplicate anonymous interface output deterministic jellevandenhooff/go
8 participants
@bradfitz @josharian @jellevandenhooff @andybons @cuonglm @odeke-em @randall77 @gopherbot