cmd/compile: clean up debug_test.go

Exercise of preparing a how-to document motivated me to
clean up some of the stupider wonkier bits.  Since this
does not run for test -short, expect no change for trybots,
did pass testing with OSX gdb and a refreshed copy of Delve.

Change-Id: I58edd10599b172c4787ff5f110db078f6c2c81c5
Reviewed-on: https://go-review.googlesource.com/83957
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>

diff --git a/src/cmd/compile/internal/ssa/debug_test.go b/src/cmd/compile/internal/ssa/debug_test.go
index 2eb4f73ee262e3da77ef7050d900feeab4d86141..6e9a8fb06d31ba946554737078eba3c3d828a719 100644 (file)
--- a/src/cmd/compile/internal/ssa/debug_test.go
+++ b/src/cmd/compile/internal/ssa/debug_test.go
@@ -87,6 +87,10 @@ var debugger = "gdb" // For naming files, etc.
 // go test debug_test.go -args -u -d
 
 func TestNexting(t *testing.T) {
+       gogcflags := os.Getenv("GO_GCFLAGS")
+       // optimizedLibs usually means "not running in a noopt test builder".
+       optimizedLibs := (!strings.Contains(gogcflags, "-N") && !strings.Contains(gogcflags, "-l"))
+
        skipReasons := "" // Many possible skip reasons, list all that apply
        if testing.Short() {
                skipReasons = "not run in short mode; "
@@ -100,7 +104,7 @@ func TestNexting(t *testing.T) {
                // Various architectures tend to differ slightly sometimes, and keeping them
                // all in sync is a pain for people who don't have them all at hand,
                // so limit testing to amd64 (for now)
-               skipReasons += "not run unless linux-amd64 or -d or -f; "
+               skipReasons += "not run unless linux-amd64 or -d (delve) or -f (force); "
        }
 
        if *useDelve {
@@ -115,6 +119,7 @@ func TestNexting(t *testing.T) {
                        if runtime.GOOS != "darwin" {
                                skipReasons += "not run because gdb not on path; "
                        } else {
+                               // On Darwin, MacPorts installs gdb as "ggdb".
                                _, err = exec.LookPath("ggdb")
                                if err != nil {
                                        skipReasons += "not run because gdb (and also ggdb) not on path; "
@@ -129,6 +134,12 @@ func TestNexting(t *testing.T) {
                t.Skip(skipReasons[:len(skipReasons)-2])
        }
 
+       optFlags := "-dwarflocationlists"
+       if !*useDelve && !*inlines {
+               // For gdb (default), disable inlining so that a compiler test does not depend on library code.
+               optFlags += " -l"
+       }
+
        t.Run("dbg-"+debugger, func(t *testing.T) {
                testNexting(t, "hist", "dbg", "-N -l")
        })
@@ -138,18 +149,14 @@ func TestNexting(t *testing.T) {
        t.Run("dbg-22558-"+debugger, func(t *testing.T) {
                testNexting(t, "i22558", "dbg-22558", "-N -l")
        })
+
        t.Run("opt-"+debugger, func(t *testing.T) {
-               // If this is test is run with a runtime compiled with -N -l, it is very likely to fail.
+               // If optimized test is run with unoptimized libraries (compiled with -N -l), it is very likely to fail.
                // This occurs in the noopt builders (for example).
-               if gogcflags := os.Getenv("GO_GCFLAGS"); *force || (!strings.Contains(gogcflags, "-N") && !strings.Contains(gogcflags, "-l")) {
-                       if *useDelve || *inlines {
-                               testNexting(t, "hist", "opt", "-dwarflocationlists")
-                       } else {
-                               // For gdb, disable inlining so that a compiler test does not depend on library code.
-                               testNexting(t, "hist", "opt", "-l -dwarflocationlists")
-                       }
+               if *force || optimizedLibs {
+                       testNexting(t, "hist", "opt", optFlags)
                } else {
-                       t.Skip("skipping for unoptimized runtime")
+                       t.Skip("skipping for unoptimized stdlib/runtime")
                }
        })
 }
@@ -183,16 +190,17 @@ func testNexting(t *testing.T, base, tag, gcflags string, moreArgs ...string) {
 
        runGo(t, "", runGoArgs...)
 
-       var h1 *nextHist
        nextlog := logbase + "-" + debugger + ".nexts"
        tmplog := tmpbase + "-" + debugger + ".nexts"
+       var dbg dbgr
        if *useDelve {
-               h1 = dlvTest(tag, exe, 1000)
+               dbg = newDelve(tag, exe)
        } else {
-               h1 = gdbTest(tag, exe, 1000)
+               dbg = newGdb(tag, exe)
        }
+       h1 := runDbgr(dbg, 1000)
        if *dryrun {
-               fmt.Printf("# Tag for above is %s\n", tag)
+               fmt.Printf("# Tag for above is %s\n", dbg.tag())
                return
        }
        if *update {
@@ -219,27 +227,10 @@ type dbgr interface {
        stepnext(s string) bool // step or next, possible with parameter, gets line etc.  returns true for success, false for unsure response
        quit()
        hist() *nextHist
+       tag() string
 }
 
-// gdbTest runs the debugger test with gdb and returns the history
-func gdbTest(tag, executable string, maxNext int, args ...string) *nextHist {
-       dbg := newGdb(tag, executable, args...)
-       dbg.start()
-       if *dryrun {
-               return nil
-       }
-       for i := 0; i < maxNext; i++ {
-               if !dbg.stepnext("n") {
-                       break
-               }
-       }
-       h := dbg.hist()
-       return h
-}
-
-// dlvTest runs the debugger test with dlv and returns the history
-func dlvTest(tag, executable string, maxNext int, args ...string) *nextHist {
-       dbg := newDelve(tag, executable, args...)
+func runDbgr(dbg dbgr, maxNext int) *nextHist {
        dbg.start()
        if *dryrun {
                return nil
@@ -445,8 +436,8 @@ func canonFileName(f string) string {
 /* Delve */
 
 type delveState struct {
-       cmd *exec.Cmd
-       tag string
+       cmd  *exec.Cmd
+       tagg string
        *ioState
        atLineRe         *regexp.Regexp // "\n =>"
        funcFileLinePCre *regexp.Regexp // "^> ([^ ]+) ([^:]+):([0-9]+) .*[(]PC: (0x[a-z0-9]+)"
@@ -462,7 +453,7 @@ func newDelve(tag, executable string, args ...string) dbgr {
                cmd.Args = append(cmd.Args, "--")
                cmd.Args = append(cmd.Args, args...)
        }
-       s := &delveState{tag: tag, cmd: cmd}
+       s := &delveState{tagg: tag, cmd: cmd}
        // HAHA Delve has control characters embedded to change the color of the => and the line number
        // that would be '(\\x1b\\[[0-9;]+m)?' OR TERM=dumb
        s.atLineRe = regexp.MustCompile("\n=>[[:space:]]+[0-9]+:(.*)")
@@ -471,6 +462,10 @@ func newDelve(tag, executable string, args ...string) dbgr {
        return s
 }
 
+func (s *delveState) tag() string {
+       return "dlv-" + s.tagg
+}
+
 func (s *delveState) stepnext(ss string) bool {
        x := s.ioState.writeReadExpect(ss+"\n", "[(]dlv[)] ")
        excerpts := s.atLineRe.FindStringSubmatch(x.o)
@@ -526,7 +521,7 @@ func (s *delveState) quit() {
 
 type gdbState struct {
        cmd  *exec.Cmd
-       tag  string
+       tagg string
        args []string
        *ioState
        atLineRe         *regexp.Regexp
@@ -540,7 +535,7 @@ func newGdb(tag, executable string, args ...string) dbgr {
        // Turn off shell, necessary for Darwin apparently
        cmd := exec.Command(gdb, "-ex", "set startup-with-shell off", executable)
        cmd.Env = replaceEnv(cmd.Env, "TERM", "dumb")
-       s := &gdbState{tag: tag, cmd: cmd, args: args}
+       s := &gdbState{tagg: tag, cmd: cmd, args: args}
        s.atLineRe = regexp.MustCompile("(^|\n)([0-9]+)(.*)")
        s.funcFileLinePCre = regexp.MustCompile(
                "([^ ]+) [(][^)]*[)][ \\t\\n]+at ([^:]+):([0-9]+)")
@@ -551,6 +546,10 @@ func newGdb(tag, executable string, args ...string) dbgr {
        return s
 }
 
+func (s *gdbState) tag() string {
+       return "gdb-" + s.tagg
+}
+
 func (s *gdbState) start() {
        run := "run"
        for _, a := range s.args {
@@ -614,7 +613,7 @@ func (s *gdbState) stepnext(ss string) bool {
                return true
        }
        // Look for //gdb-<tag>=(v1,v2,v3) and print v1, v2, v3
-       vars := varsToPrint(excerpt, "//gdb-"+s.tag+"=(")
+       vars := varsToPrint(excerpt, "//"+s.tag()+"=(")
        for _, v := range vars {
                slashIndex := strings.Index(v, "/")
                substitutions := ""