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 <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>

3 files changed, 99 insertions, 7 deletions

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
+)