]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/go: add cache verification mode
authorRuss Cox <rsc@golang.org>
Wed, 1 Nov 2017 23:57:50 +0000 (19:57 -0400)
committerRuss Cox <rsc@golang.org>
Thu, 2 Nov 2017 04:06:53 +0000 (04:06 +0000)
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
src/cmd/dist/deps.go
src/cmd/go/internal/cache/cache.go
src/cmd/go/internal/cache/cache_test.go
src/cmd/go/internal/cache/hash.go

index 04168d691c189b58aca64d96d57ed7e9e1329b29..03f11f0bfb9bbdc3b2a89e50841718daa41d78fa 100644 (file)
@@ -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)
index a12cea5f93fe5c9ed01b39ad58bd38bd1cd7f21f..55891d41d2f1535434df49711b0b4e176eea3cc0 100644 (file)
@@ -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
        },
 
index c1f6467a2e6b1159aebd99b6f1ebd136ab5ccc32..93ead77cdd990d76aa5b46e737fa61a72721ed37 100644 (file)
@@ -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)
 }
 
index 773698cbdfb86f171c8e0b825434056d1526e9e9..55320139a51cda25ac02bcaa87d5a94aafaba7fe 100644 (file)
@@ -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))
index b8896aa2f95a1b019baa7bdcdd8215ec59596a2d..7440d5e89efd57aba3fa9ee552054d09c62212cb 100644 (file)
@@ -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