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/cgo: deterministically iterate over C.xxx references? #8487

Closed
mdempsky opened this issue Aug 6, 2014 · 5 comments
Closed

cmd/cgo: deterministically iterate over C.xxx references? #8487

mdempsky opened this issue Aug 6, 2014 · 5 comments
Labels
FrozenDueToAge
Milestone
Go1.4

Comments

@mdempsky
Copy link
Contributor

mdempsky commented Aug 6, 2014 

A handful of recent cmd/cgo issues were only intermittent failures because they subtly
depended quirks in GCC's DWARF output, which in turn subtly depended upon the order of
C.xxx references output by cgo (which in turn is random because of a "range"
iteration over the f.Name map in (*Package).guessKinds(*File)).  Making it deterministic
seems easy enough to do (just add an appropriate call to sort.Sort somewhere in
cmd/cgo/gcc.go), but it's not obvious to me whether that's desirable or not.

On one hand, the random ordering seems to have helped fuzz test debug/dwarf and cmd/cgo
a bit and perhaps expose bugs we wouldn't have otherwise been aware of, but it also
means builds might spuriously fail because of latent bugs rather than deterministically
fail depending on the input.
@ianlancetaylor
Copy link
Member

Comment 1:

Labels changed: added repo-main, release-go1.4.

Status changed to Accepted.

@minux
Copy link
Member

minux commented Aug 7, 2014

Comment 2:

i agree it's better to fail deterministically.
however, could we create a flag (or environment
variable, or other means) that make cgo switch
back to the random ordering behavior?
we can enable that at least for misc/cgo/test
to catch more bugs.

@davecheney
Copy link
Contributor

Comment 3:

I think this is a good compromise, if that flag was available via the go build tool so
that we could ask reporters of cgo issues to try it and see if it alters the symptoms
they see. 
In any case these additional requests should be secondary to fix cgo to use a
deterministic ordering.

@gopherbot
Copy link
Contributor

Comment 4:

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

@ianlancetaylor
Copy link
Member

Comment 5:

This issue was closed by revision 03e6a88.

Status changed to Fixed.

@rsc rsc added this to the Go1.4 milestone Apr 14, 2015
@rsc rsc removed the release-go1.4 label Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 25, 2016
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jun 25, 2018
@mdempsky @wheatman
cmd/cgo: iterate over names in deterministic order
e6c994d 
This makes GCC behavior (and cgo build failures) deterministic.

Fixes golang#8487.

Ran this shell command on linux/amd64 (Ubuntu 12.04) before and
after this change:

    for x in `seq 100`; do
      go tool cgo -debug-gcc=true issue8441.go 2>&1 | md5sum
    done | sort | uniq -c

Before:
    67 2cdcb8c7c4e290f7d9009abc581b83dd  -
    10 9a55390df94f7cec6d810f3e20590789  -
    10 acfad22140d43d9b9517bbc5dfc3c0df  -
    13 c337f8fee2304b3a8e3158a4362d8698  -

After:
    100 785c316cbcbcd50896695050e2fa23c1  -

LGTM=minux, iant
R=golang-codereviews, bradfitz, minux, iant
CC=golang-codereviews
https://golang.org/cl/126990043
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jul 9, 2018
@mdempsky @wheatman
cmd/cgo: iterate over names in deterministic order
04c192f 
This makes GCC behavior (and cgo build failures) deterministic.

Fixes golang#8487.

Ran this shell command on linux/amd64 (Ubuntu 12.04) before and
after this change:

    for x in `seq 100`; do
      go tool cgo -debug-gcc=true issue8441.go 2>&1 | md5sum
    done | sort | uniq -c

Before:
    67 2cdcb8c7c4e290f7d9009abc581b83dd  -
    10 9a55390df94f7cec6d810f3e20590789  -
    10 acfad22140d43d9b9517bbc5dfc3c0df  -
    13 c337f8fee2304b3a8e3158a4362d8698  -

After:
    100 785c316cbcbcd50896695050e2fa23c1  -

LGTM=minux, iant
R=golang-codereviews, bradfitz, minux, iant
CC=golang-codereviews
https://golang.org/cl/126990043
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.4
Development

No branches or pull requests
6 participants
@davecheney @mdempsky @rsc @minux @ianlancetaylor @gopherbot