]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/compile: work with new bisect command
authorRuss Cox <rsc@golang.org>
Wed, 3 May 2023 05:08:10 +0000 (01:08 -0400)
committerRuss Cox <rsc@golang.org>
Fri, 5 May 2023 15:03:53 +0000 (15:03 +0000)
CL 491875 introduces a new bisect command, which we plan to
document for use by end users to debug semantic changes in
the compiler and in GODEBUGs.

This CL adapts the existing GOSSAHASH support, which bisect
is a revision of, to support the specific syntax and output used
by bisect as well.

A followup CL will remove the old GOSSAHASH syntax and output
once existing consumers of that interface have been updated.

Change-Id: I99c4af54bb82c91c74bd8b8282ded968e6316f56
Reviewed-on: https://go-review.googlesource.com/c/go/+/491895
Auto-Submit: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: David Chase <drchase@google.com>
src/cmd/compile/internal/base/hashdebug.go
src/cmd/compile/internal/base/hashdebug_test.go
src/cmd/compile/internal/loopvar/loopvar_test.go
src/cmd/compile/internal/ssa/fmahash_test.go
src/cmd/dist/buildtool.go

index 6276abe4fd27fd9651e4fb87b780ad6fbda9ecbd..64dad979f97f58280d2dfe2623bd26ceca50b8ed 100644 (file)
@@ -10,6 +10,7 @@ import (
        "cmd/internal/obj"
        "cmd/internal/src"
        "fmt"
+       "internal/bisect"
        "io"
        "os"
        "path/filepath"
@@ -40,7 +41,7 @@ type HashDebug struct {
        bytesTmp         bytes.Buffer
        matches          []hashAndMask // A hash matches if one of these matches.
        excludes         []hashAndMask // explicitly excluded hash suffixes
-       yes, no          bool
+       bisect           *bisect.Matcher
        fileSuffixOnly   bool // for Pos hashes, remove the directory prefix.
        inlineSuffixOnly bool // for Pos hashes, remove all but the most inline position.
 }
@@ -173,12 +174,12 @@ func NewHashDebug(ev, s string, file writeSyncer) *HashDebug {
        }
 
        hd := &HashDebug{name: ev, logfile: file}
-       switch s[0] {
-       case 'y', 'Y':
-               hd.yes = true
-               return hd
-       case 'n', 'N':
-               hd.no = true
+       if !strings.Contains(s, "/") {
+               m, err := bisect.New(s)
+               if err != nil {
+                       Fatalf("%s: %v", ev, err)
+               }
+               hd.bisect = m
                return hd
        }
        ss := strings.Split(s, "/")
@@ -292,23 +293,16 @@ func (d *HashDebug) DebugHashMatchParam(pkgAndName string, param uint64) bool {
        if d == nil {
                return true
        }
-       if d.no {
-               return false
-       }
-
-       if d.yes {
-               d.logDebugHashMatch(d.name, pkgAndName, "y", param)
-               return true
-       }
 
        hash := hashOf(pkgAndName, param)
-
-       // Return false for explicitly excluded hashes
-       if d.excluded(hash) {
-               return false
+       if d.bisect != nil {
+               if d.bisect.ShouldReport(hash) {
+                       d.logDebugHashMatch(d.name, pkgAndName, hash, param)
+               }
+               return d.bisect.ShouldEnable(hash)
        }
        if m := d.match(hash); m != nil {
-               d.logDebugHashMatch(m.name, pkgAndName, hashString(hash), param)
+               d.logDebugHashMatch(m.name, pkgAndName, hash, param)
                return true
        }
        return false
@@ -316,41 +310,39 @@ func (d *HashDebug) DebugHashMatchParam(pkgAndName string, param uint64) bool {
 
 // DebugHashMatchPos is similar to DebugHashMatchParam, but for hash computation
 // it uses the source position including all inlining information instead of
-// package name and path. The output trigger string is prefixed with "POS=" so
-// that tools processing the output can reliably tell the difference. The mutex
-// locking is also more frequent and more granular.
+// package name and path. The mutex locking is more frequent and more granular.
 // Note that the default answer for no environment variable (d == nil)
 // is "yes", do the thing.
 func (d *HashDebug) DebugHashMatchPos(pos src.XPos) bool {
        if d == nil {
                return true
        }
-       if d.no {
-               return false
-       }
        // Written this way to make inlining likely.
        return d.debugHashMatchPos(Ctxt, pos)
 }
 
 func (d *HashDebug) debugHashMatchPos(ctxt *obj.Link, pos src.XPos) bool {
+       // TODO: When we remove the old d.match code, we can use
+       // d.bisect.Hash instead of the locked buffer, and we can
+       // use d.bisect.Visible to decide whether to format a string.
        d.mu.Lock()
        defer d.mu.Unlock()
 
        b := d.bytesForPos(ctxt, pos)
-
-       if d.yes {
-               d.logDebugHashMatchLocked(d.name, string(b), "y", 0)
-               return true
-       }
-
        hash := hashOfBytes(b, 0)
+       if d.bisect != nil {
+               if d.bisect.ShouldReport(hash) {
+                       d.logDebugHashMatchLocked(d.name, string(b), hash, 0)
+               }
+               return d.bisect.ShouldEnable(hash)
+       }
 
        // Return false for explicitly excluded hashes
        if d.excluded(hash) {
                return false
        }
        if m := d.match(hash); m != nil {
-               d.logDebugHashMatchLocked(m.name, "POS="+string(b), hashString(hash), 0)
+               d.logDebugHashMatchLocked(m.name, string(b), hash, 0)
                return true
        }
        return false
@@ -381,13 +373,13 @@ func (d *HashDebug) bytesForPos(ctxt *obj.Link, pos src.XPos) []byte {
        return b.Bytes()
 }
 
-func (d *HashDebug) logDebugHashMatch(varname, name, hstr string, param uint64) {
+func (d *HashDebug) logDebugHashMatch(varname, name string, hash, param uint64) {
        d.mu.Lock()
        defer d.mu.Unlock()
-       d.logDebugHashMatchLocked(varname, name, hstr, param)
+       d.logDebugHashMatchLocked(varname, name, hash, param)
 }
 
-func (d *HashDebug) logDebugHashMatchLocked(varname, name, hstr string, param uint64) {
+func (d *HashDebug) logDebugHashMatchLocked(varname, name string, hash, param uint64) {
        file := d.logfile
        if file == nil {
                if tmpfile := os.Getenv("GSHS_LOGFILE"); tmpfile != "" {
@@ -403,6 +395,7 @@ func (d *HashDebug) logDebugHashMatchLocked(varname, name, hstr string, param ui
                }
                d.logfile = file
        }
+       hstr := hashString(hash)
        if len(hstr) > 24 {
                hstr = hstr[len(hstr)-24:]
        }
@@ -412,5 +405,11 @@ func (d *HashDebug) logDebugHashMatchLocked(varname, name, hstr string, param ui
        } else {
                fmt.Fprintf(file, "%s triggered %s:%d %s\n", varname, name, param, hstr)
        }
+       // Print new bisect version too.
+       if param == 0 {
+               fmt.Fprintf(file, "%s %s\n", name, bisect.Marker(hash))
+       } else {
+               fmt.Fprintf(file, "%s:%d %s\n", name, param, bisect.Marker(hash))
+       }
        file.Sync()
 }
index b74169f895a48dccef95115ea47721a070c297bc..0b83712532339ad5c2c8168e2dd038da760d5df6 100644 (file)
@@ -11,28 +11,22 @@ import (
 )
 
 func TestHashDebugGossahashY(t *testing.T) {
-       hd := NewHashDebug("GOSSAHASH", "y", nil)
+       hd := NewHashDebug("GOSSAHASH", "y", new(bufferWithSync))
        if hd == nil {
                t.Errorf("NewHashDebug should not return nil for GOSSASHASH=y")
        }
-       if !hd.yes {
-               t.Errorf("NewHashDebug should return hd.yes==true for GOSSASHASH=y")
-       }
-       if hd.no {
-               t.Errorf("NewHashDebug should not return hd.no==true for GOSSASHASH=y")
+       if !hd.DebugHashMatch("anything") {
+               t.Errorf("NewHashDebug should return yes for everything for GOSSASHASH=y")
        }
 }
 
 func TestHashDebugGossahashN(t *testing.T) {
-       hd := NewHashDebug("GOSSAHASH", "n", nil)
+       hd := NewHashDebug("GOSSAHASH", "n", new(bufferWithSync))
        if hd == nil {
                t.Errorf("NewHashDebug should not return nil for GOSSASHASH=n")
        }
-       if !hd.no {
-               t.Errorf("NewHashDebug should return hd.no==true GOSSASHASH=n")
-       }
-       if hd.yes {
-               t.Errorf("NewHashDebug should not return hd.yes==true for GOSSASHASH=n")
+       if hd.DebugHashMatch("anything") {
+               t.Errorf("NewHashDebug should return no for everything for GOSSASHASH=n")
        }
 }
 
@@ -73,6 +67,7 @@ func TestHashMatch(t *testing.T) {
                t.Errorf("GOSSAHASH=0011 should have matched for 'bar'")
        }
        wantPrefix(t, msg, "GOSSAHASH triggered bar ")
+       wantContains(t, msg, "\nbar [bisect-match ")
 }
 
 func TestHashMatchParam(t *testing.T) {
@@ -85,6 +80,7 @@ func TestHashMatchParam(t *testing.T) {
                t.Errorf("GOSSAHASH=1010 should have matched for 'bar', 1")
        }
        wantPrefix(t, msg, "GOSSAHASH triggered bar:1 ")
+       wantContains(t, msg, "\nbar:1 [bisect-match ")
 }
 
 func TestYMatch(t *testing.T) {
@@ -96,7 +92,8 @@ func TestYMatch(t *testing.T) {
        if !check {
                t.Errorf("GOSSAHASH=y should have matched for 'bar'")
        }
-       wantPrefix(t, msg, "GOSSAHASH triggered bar y")
+       wantPrefix(t, msg, "GOSSAHASH triggered bar 110101000010000100000011")
+       wantContains(t, msg, "\nbar [bisect-match ")
 }
 
 func TestNMatch(t *testing.T) {
@@ -108,9 +105,8 @@ func TestNMatch(t *testing.T) {
        if check {
                t.Errorf("GOSSAHASH=n should NOT have matched for 'bar'")
        }
-       if msg != "" {
-               t.Errorf("Message should have been empty, instead %s", msg)
-       }
+       wantPrefix(t, msg, "GOSSAHASH triggered bar 110101000010000100000011")
+       wantContains(t, msg, "\nbar [bisect-match ")
 }
 
 func TestHashNoMatch(t *testing.T) {
@@ -159,6 +155,12 @@ func (ws *bufferWithSync) String() string {
 
 func wantPrefix(t *testing.T, got, want string) {
        if !strings.HasPrefix(got, want) {
-               t.Errorf("Want %s, got %s", want, got)
+               t.Errorf("want prefix %q, got:\n%s", want, got)
+       }
+}
+
+func wantContains(t *testing.T, got, want string) {
+       if !strings.Contains(got, want) {
+               t.Errorf("want contains %q, got:\n%s", want, got)
        }
 }
index 729c240ef549e49cfed002d525bf5567af52a73e..22ff15ee8be622b7dd92a7507f39a804db80d98c 100644 (file)
@@ -193,7 +193,7 @@ func TestLoopVarHashes(t *testing.T) {
        m := f("000100000010011111101100")
        t.Logf(m)
 
-       mCount := strings.Count(m, "loopvarhash triggered POS=main.go:27:6")
+       mCount := strings.Count(m, "loopvarhash triggered main.go:27:6")
        otherCount := strings.Count(m, "loopvarhash")
        if mCount < 1 {
                t.Errorf("Did not see expected value of m compile")
index 7ebc8a48844f45f99c34ed34f04c07566eb9a6a0..dfa1aa1147ebf63d4419a5e594c2fbfa4e1b443b 100644 (file)
@@ -44,7 +44,7 @@ func TestFmaHash(t *testing.T) {
                t.Error(e)
        }
        s := string(b) // Looking for "GOFMAHASH triggered main.main:24"
-       re := "fmahash(0?) triggered POS=.*fma.go:29:..;.*fma.go:18:.."
+       re := "fmahash(0?) triggered .*fma.go:29:..;.*fma.go:18:.."
        match := regexp.MustCompile(re)
        if !match.MatchString(s) {
                t.Errorf("Expected to match '%s' with \n-----\n%s-----", re, s)
index 815f944fe26e3777c0154181768dca26d5ce6a59..bb36c076637b06f2e9540b2699a533eee00e7196 100644 (file)
@@ -63,6 +63,7 @@ var bootstrapDirs = []string{
        "go/constant",
        "internal/abi",
        "internal/coverage",
+       "internal/bisect",
        "internal/buildcfg",
        "internal/goarch",
        "internal/godebugs",