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.

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

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

Closed
ComputerDruid opened this issue Aug 15, 2018 · 8 comments
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. release-blocker
Milestone

Comments

@ComputerDruid
Copy link
Contributor

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

go version go1.11rc1 linux/amd64

Does this issue reproduce with the latest release?

Doesn't seem to happen in go 1.10, but it does occur in 1.11rc1.

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

Used docker run -it golang:stretch-rc to grab a clean repro env.

GOARCH="amd64"
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"

What did you do?

The go compiler produces a non-deterministic small.a output when run on the following minimized test program:

package small

func A(arg interface{}) {
        _ = arg.(interface{ Func() int32 })
        _ = arg.(interface{ Func() int32 })
        // two is sufficient for nondeterminism, but let's add more to make it more likely
        _ = arg.(interface{ Func() int32 })
        _ = arg.(interface{ Func() int32 })
        _ = arg.(interface{ Func() int32 })
        _ = arg.(interface{ Func() int32 })
        _ = arg.(interface{ Func() int32 })
}

To demonstrate:

for i in {1..20}; do
  go tool compile -p small -linkobj small.a small.go &&
  sha1sum 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 nondeterministic output

5 0429fb2b8ff3754684ba5aa6a686ddf97459c6e2  small.a
3 0c9eb51706182c0e3fc15d79b6f0b8be9648ef94  small.a
5 1d7d836a2294347da9c458c158459888902ac5c2  small.a
1 2b4600b551a9c60d717fe593e8cb4765bcdbf0ce  small.a
1 429d2d81b645796682ca194287d45765c1703620  small.a
3 9b26abdd35e4094758423d392c69bf57e7a76bb3  small.a
2 ee0b1fb89dff9f5e26f33284e55e5826679e8f83  small.a

Notes

I did a fair amount of digging into the problem because it piqued my interest and have a patch that I believe fixes it. I'll upload that shortly.

The problem seems to be in the generated DWARF debug info containing a line number for the return variable "~r0" for the generated conversion function. It seems to non-deterministically be one of any of the lines that the conversion happens on.

@heschi heschi changed the title interface conversion non-deterministic build cmd/compile: duplicate anonymous interface declarations cause non-determinism Aug 15, 2018
@heschi
Copy link
Contributor

heschi commented Aug 15, 2018

Bisects to ca2f85f (cmd/compile: add indexed export format). I edited the title to reflect my best understanding but I might be a little wrong.

cc @mdempsky, @griesemer

@ComputerDruid
Copy link
Contributor Author

That commit (ca2f85f) adds a.Pos = t.Pos in repr.go which seems like a possible candidate for triggering the bug.

@gopherbot
Copy link

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

@ComputerDruid
Copy link
Contributor Author

https://golang.org/cl/129515 is my proposed fix (thanks @gopherbot)

@ALTree ALTree added this to the Go1.12 milestone Aug 16, 2018
@ALTree ALTree added the NeedsFix The path to resolution is known, but the work has not been done. label Aug 16, 2018
@josharian
Copy link
Contributor

This is a regression and reproducible builds are an important compiler feature. I think this should go into 1.11 if the fix is low risk.

@ALTree ALTree modified the milestones: Go1.12, Go1.11 Aug 16, 2018
@ALTree ALTree added NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. release-blocker and removed NeedsFix The path to resolution is known, but the work has not been done. labels Aug 16, 2018
@ALTree
Copy link
Member

ALTree commented Aug 16, 2018

(I've moved this to 1.11 - NeedsDecision to give it visibility for a possible 1.11 inclusion, as @josharian suggested).

@mdempsky
Copy link
Member

I think including the fix in 1.11 makes sense. @ComputerDruid 's patch seems safe and minimally invasive to me.

@gopherbot
Copy link

Change https://golang.org/cl/129680 mentions this issue: cmd/compile: add a test for reproducible build with anonymous interfaces

gopherbot pushed a commit that referenced this issue Aug 28, 2018
Duplicated anonymous interfaces caused nondeterministic build.
The fix is CL 129515. This CL adds a test.

Updates #27013.

Change-Id: I6b7e1bbfc943c22e8e6f32c145f7aebb567cef15
Reviewed-on: https://go-review.googlesource.com/129680
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@golang golang locked and limited conversation to collaborators Aug 17, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. release-blocker
Projects
None yet
Development

No branches or pull requests

6 participants