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: non-reproducible builds for some cgo code #20272

Closed
josharian opened this issue May 7, 2017 · 5 comments

Comments

Projects
None yet
2 participants
@josharian
Copy link
Contributor

commented May 7, 2017

Reproduce:

Grab the three cgo-generated files from https://gist.github.com/josharian/88c23ec7c4c7c45ac87a539ee18ede5d. These were generated from a slightly minimized, old copy of package github.com/mattn/go-gtk/glib. Run go tool compile -o a.o *.go on them multiple times, and check that the output file is identical.

Want:

Exactly one canonical output for these inputs.

Have:

Two distinct outputs, one object file that is 240250 bytes and one that is 240092. The difference appears to be in the namedata (type information) symbols that are emitted.

@josharian josharian added this to the Go1.9 milestone May 7, 2017

@josharian josharian self-assigned this May 7, 2017

@josharian

This comment has been minimized.

Copy link
Contributor Author

commented May 7, 2017

In particular, diffing the -S output, the larger object file also contains:

26091a26092,26102
> type..namedata.***uint8. SRODATA dupok size=11
> 	0x0000 00 00 08 2a 2a 2a 75 69 6e 74 38                 ...***uint8
> type.***uint8 SRODATA dupok size=56
> 	0x0000 08 00 00 00 00 00 00 00 08 00 00 00 00 00 00 00  ................
> 	0x0010 a1 e3 63 f9 00 08 08 36 00 00 00 00 00 00 00 00  ..c....6........
> 	0x0020 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 	0x0030 00 00 00 00 00 00 00 00                          ........
> 	rel 24+8 t=1 runtime.algarray+80
> 	rel 32+8 t=1 runtime.gcbits.01+0
> 	rel 40+4 t=5 type..namedata.***uint8.+0
> 	rel 48+8 t=1 type.**uint8+0
26101a26113
> 	rel 44+4 t=6 type.***uint8+0
@josharian

This comment has been minimized.

Copy link
Contributor Author

commented May 7, 2017

Broken, it appears, by CL 39915.

@josharian

This comment has been minimized.

Copy link
Contributor Author

commented May 7, 2017

Diagnosis: CL 39915 sorts signats by ShortName for reproducibility. But ShortName treats types byte and uint8 identically. CL 39915 attempted to compensate for this by only adding the underlying type (uint8) to signats in addsignat: signatlist[formalType(t)] = true.

However, though this works for byte and uint8, it does not work for *byte and *uint8, or any other composite types including byte or rune. In the Go files above, we end up with both **uint8 and **byte in signatlist. Their sort order is random, leading to different output.

@josharian

This comment has been minimized.

Copy link
Contributor Author

commented May 7, 2017

The simplest fix appears to be to sort both by t.ShortName() and then by t.String(). This always generates the smaller output file, thanks to the fact that byte and rune get sorted before uint8 and uint32, regardless of how buried in a complicated type structure they are. It's ugly and awkward, but simple, and has very little performance impact. Writing a test for this will require some doing, so the CL will have to wait for Monday.

@gopherbot

This comment has been minimized.

Copy link

commented May 9, 2017

CL https://golang.org/cl/42955 mentions this issue.

@gopherbot gopherbot closed this in 6f2ee0f May 9, 2017

@golang golang locked and limited conversation to collaborators May 9, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.