From: Josh Bleecher Snyder Date: Sun, 7 May 2017 06:19:41 +0000 (-0700) Subject: cmd/compile: make builds reproducible in presence of **byte and **int8 X-Git-Tag: go1.9beta1~300 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=6f2ee0f3dfe941e2b0cfe3f5b775727e29b1fdf0;p=gostls13.git cmd/compile: make builds reproducible in presence of **byte and **int8 CL 39915 introduced sorting of signats by ShortString for reproducible builds. But ShortString treats types byte and uint8 identically; same for rune and uint32. CL 39915 attempted to compensate for this by only adding the underlying type (uint8) to signats in addsignat. This only works for byte and uint8. For e.g. *byte and *uint, both get added, and their sort order is random, leading to non-reproducible builds. One fix would be to add yet another type printing mode that doesn't eliminate byte and rune, and use it for sorting signats. But the formatting routines are complicated enough as it is. Instead, just sort first by ShortString and then by String. We can't just use String, because ShortString makes distinctions that String doesn't. ShortString is really preferred here; String is serving only as a backstop for handling of bytes and runes. The long series of types in the test helps increase the odds of failure, allowing a smaller number of iterations in the test. On my machine, a full test takes 700ms. Passes toolstash-check. Updates #19961 Fixes #20272 name old alloc/op new alloc/op delta Template 37.9MB ± 0% 37.9MB ± 0% +0.12% (p=0.032 n=5+5) Unicode 28.9MB ± 0% 28.9MB ± 0% ~ (p=0.841 n=5+5) GoTypes 110MB ± 0% 110MB ± 0% ~ (p=0.841 n=5+5) Compiler 463MB ± 0% 463MB ± 0% ~ (p=0.056 n=5+5) SSA 1.11GB ± 0% 1.11GB ± 0% +0.02% (p=0.016 n=5+5) Flate 24.7MB ± 0% 24.8MB ± 0% +0.14% (p=0.032 n=5+5) GoParser 31.1MB ± 0% 31.1MB ± 0% ~ (p=0.421 n=5+5) Reflect 73.9MB ± 0% 73.9MB ± 0% ~ (p=1.000 n=5+5) Tar 25.8MB ± 0% 25.8MB ± 0% +0.15% (p=0.016 n=5+5) XML 41.2MB ± 0% 41.2MB ± 0% ~ (p=0.310 n=5+5) [Geo mean] 72.0MB 72.0MB +0.07% name old allocs/op new allocs/op delta Template 384k ± 0% 385k ± 1% ~ (p=0.056 n=5+5) Unicode 343k ± 0% 344k ± 0% ~ (p=0.548 n=5+5) GoTypes 1.16M ± 0% 1.16M ± 0% ~ (p=0.421 n=5+5) Compiler 4.43M ± 0% 4.44M ± 0% +0.26% (p=0.032 n=5+5) SSA 9.86M ± 0% 9.87M ± 0% +0.10% (p=0.032 n=5+5) Flate 237k ± 1% 238k ± 0% +0.49% (p=0.032 n=5+5) GoParser 319k ± 1% 320k ± 1% ~ (p=0.151 n=5+5) Reflect 957k ± 0% 957k ± 0% ~ (p=1.000 n=5+5) Tar 251k ± 0% 252k ± 1% +0.49% (p=0.016 n=5+5) XML 399k ± 0% 401k ± 1% ~ (p=0.310 n=5+5) [Geo mean] 739k 741k +0.26% Change-Id: Ic27995a8d374d012b8aca14546b1df9d28d30df7 Reviewed-on: https://go-review.googlesource.com/42955 Run-TryBot: Josh Bleecher Snyder TryBot-Result: Gobot Gobot Reviewed-by: Robert Griesemer --- diff --git a/src/cmd/compile/internal/gc/reflect.go b/src/cmd/compile/internal/gc/reflect.go index 1e165fd465..786517af1a 100644 --- a/src/cmd/compile/internal/gc/reflect.go +++ b/src/cmd/compile/internal/gc/reflect.go @@ -1412,7 +1412,7 @@ func itabsym(it *obj.LSym, offset int64) *obj.LSym { } func addsignat(t *types.Type) { - signatlist[formalType(t)] = true + signatlist[t] = true } func addsignats(dcls []*Node) { @@ -1432,7 +1432,7 @@ func dumpsignats() { signats = signats[:0] // Transfer entries to a slice and sort, for reproducible builds. for t := range signatlist { - signats = append(signats, typeAndStr{t: t, s: typesymname(t)}) + signats = append(signats, typeAndStr{t: t, short: typesymname(t), regular: t.String()}) delete(signatlist, t) } sort.Sort(typesByString(signats)) @@ -1542,15 +1542,25 @@ func dumpbasictypes() { } type typeAndStr struct { - t *types.Type - s string + t *types.Type + short string + regular string } type typesByString []typeAndStr -func (a typesByString) Len() int { return len(a) } -func (a typesByString) Less(i, j int) bool { return a[i].s < a[j].s } -func (a typesByString) Swap(i, j int) { a[i], a[j] = a[j], a[i] } +func (a typesByString) Len() int { return len(a) } +func (a typesByString) Less(i, j int) bool { + if a[i].short != a[j].short { + return a[i].short < a[j].short + } + // When the only difference between the types is whether + // they refer to byte or uint8, such as **byte vs **uint8, + // the types' ShortStrings can be identical. + // To preserve deterministic sort ordering, sort these by String(). + return a[i].regular < a[j].regular +} +func (a typesByString) Swap(i, j int) { a[i], a[j] = a[j], a[i] } func dalgsym(t *types.Type) *obj.LSym { var lsym *obj.LSym diff --git a/src/cmd/compile/internal/gc/reproduciblebuilds_test.go b/src/cmd/compile/internal/gc/reproduciblebuilds_test.go new file mode 100644 index 0000000000..b5f318e761 --- /dev/null +++ b/src/cmd/compile/internal/gc/reproduciblebuilds_test.go @@ -0,0 +1,48 @@ +// Copyright 2017 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package gc_test + +import ( + "bytes" + "internal/testenv" + "io/ioutil" + "os" + "os/exec" + "path/filepath" + "testing" +) + +func TestReproducibleBuilds(t *testing.T) { + testenv.MustHaveGoBuild(t) + iters := 10 + if testing.Short() { + iters = 4 + } + t.Parallel() + var want []byte + tmp, err := ioutil.TempFile("", "") + if err != nil { + t.Fatalf("temp file creation failed: %v", err) + } + defer os.Remove(tmp.Name()) + defer tmp.Close() + for i := 0; i < iters; i++ { + out, err := exec.Command(testenv.GoToolPath(t), "tool", "compile", "-o", tmp.Name(), filepath.Join("testdata", "reproducible", "issue20272.go")).CombinedOutput() + if err != nil { + t.Fatalf("failed to compile: %v\n%s", err, out) + } + obj, err := ioutil.ReadFile(tmp.Name()) + if err != nil { + t.Fatalf("failed to read object file: %v", err) + } + if i == 0 { + want = obj + } else { + if !bytes.Equal(want, obj) { + t.Fatalf("builds produced different output after %d iters (%d bytes vs %d bytes)", i, len(want), len(obj)) + } + } + } +} diff --git a/src/cmd/compile/internal/gc/testdata/reproducible/issue20272.go b/src/cmd/compile/internal/gc/testdata/reproducible/issue20272.go new file mode 100644 index 0000000000..3db0b8a357 --- /dev/null +++ b/src/cmd/compile/internal/gc/testdata/reproducible/issue20272.go @@ -0,0 +1,34 @@ +// Copyright 2017 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package p + +var ( + i0 uint8 + b0 byte + + i1 *uint8 + b1 *byte + + i2 **uint8 + b2 **byte + + i3 ***uint8 + b3 ***byte + + i4 ****uint8 + b4 ****byte + + i5 *****uint8 + b5 *****byte + + i6 ******uint8 + b6 ******byte + + i7 *******uint8 + b7 *******byte + + i8 ********uint8 + b8 ********byte +)