From 15f07dbf59e223ca3116eb57868632082657d3a6 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Wed, 1 Nov 2017 19:57:50 -0400 Subject: [PATCH] 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 TryBot-Result: Gobot Gobot Reviewed-by: David Crawshaw --- src/cmd/dist/build.go | 2 +- src/cmd/dist/deps.go | 1 + src/cmd/go/internal/cache/cache.go | 49 +++++++++++++++++++++++++ src/cmd/go/internal/cache/cache_test.go | 42 +++++++++++++++++++++ src/cmd/go/internal/cache/hash.go | 2 +- 5 files changed, 94 insertions(+), 2 deletions(-) diff --git a/src/cmd/dist/build.go b/src/cmd/dist/build.go index 04168d691c..03f11f0bfb 100644 --- a/src/cmd/dist/build.go +++ b/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) diff --git a/src/cmd/dist/deps.go b/src/cmd/dist/deps.go index a12cea5f93..55891d41d2 100644 --- a/src/cmd/dist/deps.go +++ b/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 }, diff --git a/src/cmd/go/internal/cache/cache.go b/src/cmd/go/internal/cache/cache.go index c1f6467a2e..93ead77cdd 100644 --- a/src/cmd/go/internal/cache/cache.go +++ b/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) } diff --git a/src/cmd/go/internal/cache/cache_test.go b/src/cmd/go/internal/cache/cache_test.go index 773698cbdf..55320139a5 100644 --- a/src/cmd/go/internal/cache/cache_test.go +++ b/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)) diff --git a/src/cmd/go/internal/cache/hash.go b/src/cmd/go/internal/cache/hash.go index b8896aa2f9..7440d5e89e 100644 --- a/src/cmd/go/internal/cache/hash.go +++ b/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 -- 2.48.1