cmd/go: add cache verification mode

If GODEBUG=gocacheverify=1, then instead of using the cache to
avoid computations, the go command will do the computations and
double-check that they match any existing cache entries.
This is handled entirely in the cache implementation; there's no
complexity added to any of the cache usage sites.

(As of this CL there aren't any cache usage sites, but soon there will be.)

Also change GOCMDDEBUGHASH to the more usual GODEBUG=gocachehash=1.

Change-Id: I574f181e06b5299b1d9c6d402e40c57a0e064e74
Reviewed-on: https://go-review.googlesource.com/75294
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>

src/cmd/dist/build.go

@@ -1276,7 +1276,7 @@ func checkNotStale(goBinary string, targets ...string) {
 			"-f={{if .Stale}}\t{{.ImportPath}}: {{.StaleReason}}{{end}}",
 		}, targets...)...)
 	if out != "" {
-		os.Setenv("GOCMDDEBUGHASH", "1")
+		os.Setenv("GODEBUG", "gocachehash=1")
 		for _, target := range []string{"runtime/internal/sys", "cmd/dist", "cmd/link"} {
 			if strings.Contains(out, target) {
 				run(goroot, ShowOutput|CheckExit, goBinary, "list", "-f={{.ImportPath}} {{.Stale}}", target)

src/cmd/dist/deps.go

@@ -98,6 +98,7 @@ var builddeps = map[string][]string{
 		"path/filepath",        // cmd/go/internal/cache
 		"runtime",              // cmd/go/internal/cache
 		"strconv",              // cmd/go/internal/cache
+		"strings",              // cmd/go/internal/cache
 		"sync",                 // cmd/go/internal/cache
 	},
 

src/cmd/go/internal/cache/cache.go

@@ -16,6 +16,7 @@ import (
 	"os"
 	"path/filepath"
 	"strconv"
+	"strings"
 )
 
 // An ActionID is a cache action key, the hash of a complete description of a
@@ -74,11 +75,46 @@ const (
 	entrySize = 2 + 1 + hexSize + 1 + hexSize + 1 + 20 + 1
 )
 
+// verify controls whether to run the cache in verify mode.
+// In verify mode, the cache always returns errMissing from Get
+// but then double-checks in Put that the data being written
+// exactly matches any existing entry. This provides an easy
+// way to detect program behavior that would have been different
+// had the cache entry been returned from Get.
+//
+// verify is enabled by setting the environment variable
+// GODEBUG=gocacheverify=1.
+var verify = false
+
+func init() { initEnv() }
+
+func initEnv() {
+	verify = false
+	debugHash = false
+	debug := strings.Split(os.Getenv("GODEBUG"), ",")
+	for _, f := range debug {
+		if f == "gocacheverify=1" {
+			verify = true
+		}
+		if f == "gocachehash=1" {
+			debugHash = true
+		}
+	}
+}
+
 // Get looks up the action ID in the cache,
 // returning the corresponding output ID and file size, if any.
 // Note that finding an output ID does not guarantee that the
 // saved file for that output ID is still available.
 func (c *Cache) Get(id ActionID) (OutputID, int64, error) {
+	if verify {
+		return OutputID{}, 0, errMissing
+	}
+	return c.get(id)
+}
+
+// get is Get but does not respect verify mode, so that Put can use it.
+func (c *Cache) get(id ActionID) (OutputID, int64, error) {
 	missing := func() (OutputID, int64, error) {
 		// TODO: log miss
 		return OutputID{}, 0, errMissing
@@ -147,7 +183,20 @@ func (c *Cache) putIndexEntry(id ActionID, out OutputID, size int64) error {
 	// While not ideal, this is also not a correctness problem, so we
 	// don't make a big deal about it. In particular, we leave the action
 	// cache entries writable specifically so that they can be overwritten.
+	//
+	// Setting GODEBUG=gocacheverify=1 does make a big deal:
+	// in verify mode we are double-checking that the cache entries
+	// are entirely reproducible. As just noted, this may be unrealistic
+	// in some cases but the check is also useful for shaking out real bugs.
 	entry := []byte(fmt.Sprintf("v1 %x %x %20d\n", id, out, size))
+	if verify {
+		oldOut, oldSize, err := c.get(id)
+		if err == nil && (oldOut != out || oldSize != size) {
+			fmt.Fprintf(os.Stderr, "go: internal cache error: id=%x changed:\nold: %x %d\nnew: %x %d\n", id, out, size, oldOut, oldSize)
+			// panic to show stack trace, so we can see what code is generating this cache entry.
+			panic("cache verify failed")
+		}
+	}
 	return ioutil.WriteFile(c.fileName(id, "a"), entry, 0666)
 }
 

src/cmd/go/internal/cache/cache_test.go

@@ -12,6 +12,10 @@ import (
 	"testing"
 )
 
+func init() {
+	verify = false // even if GODEBUG is set
+}
+
 func TestBasic(t *testing.T) {
 	dir, err := ioutil.TempDir("", "cachetest-")
 	if err != nil {
@@ -99,6 +103,44 @@ func TestGrowth(t *testing.T) {
 	}
 }
 
+func TestVerifyPanic(t *testing.T) {
+	os.Setenv("GODEBUG", "gocacheverify=1")
+	initEnv()
+	defer func() {
+		os.Unsetenv("GODEBUG")
+		verify = false
+	}()
+
+	if !verify {
+		t.Fatal("initEnv did not set verify")
+	}
+
+	dir, err := ioutil.TempDir("", "cachetest-")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(dir)
+
+	c, err := Open(dir)
+	if err != nil {
+		t.Fatalf("Open: %v", err)
+	}
+
+	id := ActionID(dummyID(1))
+	if err := c.PutBytes(id, []byte("abc")); err != nil {
+		t.Fatal(err)
+	}
+
+	defer func() {
+		if err := recover(); err != nil {
+			t.Log(err)
+			return
+		}
+	}()
+	c.PutBytes(id, []byte("def"))
+	t.Fatal("mismatched Put did not panic in verify mode")
+}
+
 func dummyID(x int) [HashSize]byte {
 	var out [HashSize]byte
 	binary.LittleEndian.PutUint64(out[:], uint64(x))

src/cmd/go/internal/cache/hash.go

@@ -13,7 +13,7 @@ import (
 	"sync"
 )
 
-var debugHash = os.Getenv("GOCMDDEBUGHASH") == "1"
+var debugHash = false // set when GODEBUG=gocachehash=1
 
 // HashSize is the number of bytes in a hash.
 const HashSize = 32