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

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

Comments

@mdempsky
Copy link
Member

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

This comment has been minimized.

Copy link
Contributor

commented Aug 6, 2014

Comment 1:

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

Status changed to Accepted.

@minux

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Contributor

commented Aug 7, 2014

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

This comment has been minimized.

Copy link

commented Aug 10, 2014

Comment 4:

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

This comment has been minimized.

Copy link
Contributor

commented Aug 11, 2014

Comment 5:

This issue was closed by revision 03e6a88.

Status changed to Fixed.

@mdempsky mdempsky added fixed labels Aug 11, 2014

@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 added a commit to wheatman/go-akaros that referenced this issue Jun 25, 2018
cmd/cgo: iterate over names in deterministic order
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 added a commit to wheatman/go-akaros that referenced this issue Jul 9, 2018
cmd/cgo: iterate over names in deterministic order
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.
Projects
None yet
6 participants
You can’t perform that action at this time.