]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/compile: use src.NoXPos for entry-block constants
authorDavid Chase <drchase@google.com>
Thu, 30 Nov 2017 20:11:34 +0000 (15:11 -0500)
committerKeith Randall <khr@golang.org>
Fri, 1 Dec 2017 07:09:54 +0000 (07:09 +0000)
The ssa backend is aggressive about placing constants and
certain other values in the Entry block.  It's implausible
that the original line numbers for these constants makes
any sort of sense when it appears to a user stepping in a
debugger, and they're also not that useful in dumps since
entry-block instructions tend to be constants (i.e.,
unlikely to be the cause of a crash).

Therefore, use src.NoXPos for any values that are explicitly
inserted into a function's entry block.

Passes all tests, including ssa/debug_test.go with both
gdb and a fairly recent dlv.  Hand-verified that it solves
the reported problem; constructed a test that reproduced
a problem, and fixed it.

Modified test harness to allow injection of slightly more
interesting inputs.

Fixes #22558.

Change-Id: I4476927067846bc4366da7793d2375c111694c55
Reviewed-on: https://go-review.googlesource.com/81215
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
13 files changed:
src/cmd/compile/internal/gc/ssa.go
src/cmd/compile/internal/ssa/debug_test.go
src/cmd/compile/internal/ssa/func.go
src/cmd/compile/internal/ssa/testdata/hist.dbg-dlv.nexts
src/cmd/compile/internal/ssa/testdata/hist.dbg-gdb.nexts
src/cmd/compile/internal/ssa/testdata/hist.go
src/cmd/compile/internal/ssa/testdata/hist.opt-dlv.nexts
src/cmd/compile/internal/ssa/testdata/hist.opt-gdb.nexts
src/cmd/compile/internal/ssa/testdata/i22558.dbg-22558-dlv.nexts [new file with mode: 0644]
src/cmd/compile/internal/ssa/testdata/i22558.dbg-22558-gdb.nexts [new file with mode: 0644]
src/cmd/compile/internal/ssa/testdata/i22558.go [new file with mode: 0644]
src/cmd/compile/internal/ssa/testdata/i22600.dbg-race-gdb.nexts
src/cmd/compile/internal/ssa/testdata/i22600.go

index dfa2d081d17aea314399bf2dced88d9528d02004..36dd1a4be4f7567f0e68ba58664f1ae2d7555ac5 100644 (file)
@@ -493,27 +493,27 @@ func (s *state) entryNewValue0(op ssa.Op, t *types.Type) *ssa.Value {
 
 // entryNewValue0A adds a new value with no arguments and an aux value to the entry block.
 func (s *state) entryNewValue0A(op ssa.Op, t *types.Type, aux interface{}) *ssa.Value {
-       return s.f.Entry.NewValue0A(s.peekPos(), op, t, aux)
+       return s.f.Entry.NewValue0A(src.NoXPos, op, t, aux)
 }
 
 // entryNewValue1 adds a new value with one argument to the entry block.
 func (s *state) entryNewValue1(op ssa.Op, t *types.Type, arg *ssa.Value) *ssa.Value {
-       return s.f.Entry.NewValue1(s.peekPos(), op, t, arg)
+       return s.f.Entry.NewValue1(src.NoXPos, op, t, arg)
 }
 
 // entryNewValue1 adds a new value with one argument and an auxint value to the entry block.
 func (s *state) entryNewValue1I(op ssa.Op, t *types.Type, auxint int64, arg *ssa.Value) *ssa.Value {
-       return s.f.Entry.NewValue1I(s.peekPos(), op, t, auxint, arg)
+       return s.f.Entry.NewValue1I(src.NoXPos, op, t, auxint, arg)
 }
 
 // entryNewValue1A adds a new value with one argument and an aux value to the entry block.
 func (s *state) entryNewValue1A(op ssa.Op, t *types.Type, aux interface{}, arg *ssa.Value) *ssa.Value {
-       return s.f.Entry.NewValue1A(s.peekPos(), op, t, aux, arg)
+       return s.f.Entry.NewValue1A(src.NoXPos, op, t, aux, arg)
 }
 
 // entryNewValue2 adds a new value with two arguments to the entry block.
 func (s *state) entryNewValue2(op ssa.Op, t *types.Type, arg0, arg1 *ssa.Value) *ssa.Value {
-       return s.f.Entry.NewValue2(s.peekPos(), op, t, arg0, arg1)
+       return s.f.Entry.NewValue2(src.NoXPos, op, t, arg0, arg1)
 }
 
 // const* routines add a new const value to the entry block.
index 0e349025122a94bf16fc377b6e41d409ca3861e0..2eb4f73ee262e3da77ef7050d900feeab4d86141 100644 (file)
@@ -135,6 +135,9 @@ func TestNexting(t *testing.T) {
        t.Run("dbg-race-"+debugger, func(t *testing.T) {
                testNexting(t, "i22600", "dbg-race", "-N -l", "-race")
        })
+       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.
                // This occurs in the noopt builders (for example).
@@ -501,7 +504,7 @@ func (s *delveState) stepnext(ss string) bool {
 func (s *delveState) start() {
        if *dryrun {
                fmt.Printf("%s\n", asCommandLine("", s.cmd))
-               fmt.Printf("b main.main\n")
+               fmt.Printf("b main.test\n")
                fmt.Printf("c\n")
                return
        }
@@ -511,7 +514,7 @@ func (s *delveState) start() {
                panic(fmt.Sprintf("There was an error [start] running '%s', %v\n", line, err))
        }
        s.ioState.readExpecting(-1, 5000, "Type 'help' for list of commands.")
-       expect("Breakpoint [0-9]+ set at ", s.ioState.writeReadExpect("b main.main\n", "[(]dlv[)] "))
+       expect("Breakpoint [0-9]+ set at ", s.ioState.writeReadExpect("b main.test\n", "[(]dlv[)] "))
        s.stepnext("c")
 }
 
@@ -555,7 +558,7 @@ func (s *gdbState) start() {
        }
        if *dryrun {
                fmt.Printf("%s\n", asCommandLine("", s.cmd))
-               fmt.Printf("tbreak main.main\n")
+               fmt.Printf("tbreak main.test\n")
                fmt.Printf("%s\n", run)
                return
        }
@@ -565,7 +568,7 @@ func (s *gdbState) start() {
                panic(fmt.Sprintf("There was an error [start] running '%s', %v\n", line, err))
        }
        s.ioState.readExpecting(-1, -1, "[(]gdb[)] ")
-       x := s.ioState.writeReadExpect("b main.main\n", "[(]gdb[)] ")
+       x := s.ioState.writeReadExpect("b main.test\n", "[(]gdb[)] ")
        expect("Breakpoint [0-9]+ at", x)
        s.stepnext(run)
 }
index 01966adb0ff9e051a60c9277d54e52d0127f1d09..62550df0ccf0b055579e6373b4ce757c2b408d2c 100644 (file)
@@ -407,6 +407,7 @@ func (b *Block) NewValue4(pos src.XPos, op Op, t *types.Type, arg0, arg1, arg2,
 
 // constVal returns a constant value for c.
 func (f *Func) constVal(pos src.XPos, op Op, t *types.Type, c int64, setAuxInt bool) *Value {
+       // TODO remove unused pos parameter, both here and in *func.ConstXXX callers.
        if f.constants == nil {
                f.constants = make(map[int64][]*Value)
        }
@@ -421,9 +422,9 @@ func (f *Func) constVal(pos src.XPos, op Op, t *types.Type, c int64, setAuxInt b
        }
        var v *Value
        if setAuxInt {
-               v = f.Entry.NewValue0I(pos, op, t, c)
+               v = f.Entry.NewValue0I(src.NoXPos, op, t, c)
        } else {
-               v = f.Entry.NewValue0(pos, op, t)
+               v = f.Entry.NewValue0(src.NoXPos, op, t)
        }
        f.constants[c] = append(vv, v)
        return v
index f4fe2af161904338ab072808fc667d49d98b5635..ec79b77de277c8dc9742a62a15c99e2cf139b1de 100644 (file)
@@ -1,12 +1,12 @@
   ./testdata/hist.go
-55:    func main() {
+55:    func test() {
 57:            l := line{point{1 + zero, 2 + zero}, point{3 + zero, 4 + zero}}
 58:            tinycall()                // this forces l etc to stack
 59:            dx := l.end.x - l.begin.x //gdb-dbg=(l.begin.x,l.end.y)//gdb-opt=(l,dx/O,dy/O)
 60:            dy := l.end.y - l.begin.y //gdb-opt=(dx,dy/O)
 61:            sink = dx + dy            //gdb-opt=(dx,dy)
-63:            hist := make([]int, 7)                                //gdb-opt=(sink,dx/O,dy/O)
-64:            var reader io.Reader = strings.NewReader(cannedInput) //gdb-dbg=(hist/A,cannedInput/A)
+63:            hist := make([]int, 7)                                //gdb-opt=(dx/O,dy/O) // TODO sink is missing if this code is in 'test' instead of 'main'
+64:            var reader io.Reader = strings.NewReader(cannedInput) //gdb-dbg=(hist/A) // TODO cannedInput/A is missing if this code is in 'test' instead of 'main'
 65:            if len(os.Args) > 1 {
 73:            scanner := bufio.NewScanner(reader)
 74:            for scanner.Scan() { //gdb-opt=(scanner/A)
@@ -96,3 +96,4 @@
 87:                    if a == 0 { //gdb-opt=(a,n,t)
 88:                            continue
 86:            for i, a := range hist {
+98:    }
index abd4535ca5e42f67f72f76282e745600d06baf84..fe000147bde4f07bc243aefd55703aab13606bcf 100644 (file)
@@ -1,5 +1,5 @@
   src/cmd/compile/internal/ssa/testdata/hist.go
-55:    func main() {
+55:    func test() {
 57:            l := line{point{1 + zero, 2 + zero}, point{3 + zero, 4 + zero}}
 58:            tinycall()                // this forces l etc to stack
 59:            dx := l.end.x - l.begin.x //gdb-dbg=(l.begin.x,l.end.y)//gdb-opt=(l,dx/O,dy/O)
@@ -7,10 +7,9 @@ l.begin.x = 1
 l.end.y = 4
 60:            dy := l.end.y - l.begin.y //gdb-opt=(dx,dy/O)
 61:            sink = dx + dy            //gdb-opt=(dx,dy)
-63:            hist := make([]int, 7)                                //gdb-opt=(sink,dx/O,dy/O)
-64:            var reader io.Reader = strings.NewReader(cannedInput) //gdb-dbg=(hist/A,cannedInput/A)
+63:            hist := make([]int, 7)                                //gdb-opt=(dx/O,dy/O) // TODO sink is missing if this code is in 'test' instead of 'main'
+64:            var reader io.Reader = strings.NewReader(cannedInput) //gdb-dbg=(hist/A) // TODO cannedInput/A is missing if this code is in 'test' instead of 'main'
 hist =  []int = {0, 0, 0, 0, 0, 0, 0}
-cannedInput = "1\n1\n1\n2\n2\n2\n4\n4\n5\n"
 65:            if len(os.Args) > 1 {
 73:            scanner := bufio.NewScanner(reader)
 74:            for scanner.Scan() { //gdb-opt=(scanner/A)
@@ -121,3 +120,4 @@ t = 22
 87:                    if a == 0 { //gdb-opt=(a,n,t)
 88:                            continue
 86:            for i, a := range hist {
+98:    }
index 7d1d06b47dabc5480fda3a9e18f07557d87fc780..8a0cc2728081abee0f54c47e2414a42c0fac5cba 100644 (file)
@@ -52,7 +52,7 @@ var cannedInput string = `1
 5
 `
 
-func main() {
+func test() {
        // For #19868
        l := line{point{1 + zero, 2 + zero}, point{3 + zero, 4 + zero}}
        tinycall()                // this forces l etc to stack
@@ -60,8 +60,8 @@ func main() {
        dy := l.end.y - l.begin.y //gdb-opt=(dx,dy/O)
        sink = dx + dy            //gdb-opt=(dx,dy)
        // For #21098
-       hist := make([]int, 7)                                //gdb-opt=(sink,dx/O,dy/O)
-       var reader io.Reader = strings.NewReader(cannedInput) //gdb-dbg=(hist/A,cannedInput/A)
+       hist := make([]int, 7)                                //gdb-opt=(dx/O,dy/O) // TODO sink is missing if this code is in 'test' instead of 'main'
+       var reader io.Reader = strings.NewReader(cannedInput) //gdb-dbg=(hist/A) // TODO cannedInput/A is missing if this code is in 'test' instead of 'main'
        if len(os.Args) > 1 {
                var err error
                reader, err = os.Open(os.Args[1])
@@ -91,5 +91,8 @@ func main() {
                n += a
                fmt.Fprintf(os.Stderr, "%d\t%d\t%d\t%d\t%d\n", i, a, n, i*a, t) //gdb-dbg=(n,i,t)
        }
+}
 
+func main() {
+       test()
 }
index 7aab219552036439014a46eb2ed0d772f1bbefe7..b98e3c6e65e7c3307c51d24095e71ff0757c724d 100644 (file)
@@ -1,13 +1,13 @@
   ./testdata/hist.go
-55:    func main() {
+55:    func test() {
 57:            l := line{point{1 + zero, 2 + zero}, point{3 + zero, 4 + zero}}
 58:            tinycall()                // this forces l etc to stack
 57:            l := line{point{1 + zero, 2 + zero}, point{3 + zero, 4 + zero}}
 59:            dx := l.end.x - l.begin.x //gdb-dbg=(l.begin.x,l.end.y)//gdb-opt=(l,dx/O,dy/O)
 60:            dy := l.end.y - l.begin.y //gdb-opt=(dx,dy/O)
 61:            sink = dx + dy            //gdb-opt=(dx,dy)
-63:            hist := make([]int, 7)                                //gdb-opt=(sink,dx/O,dy/O)
-64:            var reader io.Reader = strings.NewReader(cannedInput) //gdb-dbg=(hist/A,cannedInput/A)
+63:            hist := make([]int, 7)                                //gdb-opt=(dx/O,dy/O) // TODO sink is missing if this code is in 'test' instead of 'main'
+64:            var reader io.Reader = strings.NewReader(cannedInput) //gdb-dbg=(hist/A) // TODO cannedInput/A is missing if this code is in 'test' instead of 'main'
 65:            if len(os.Args) > 1 {
 74:            for scanner.Scan() { //gdb-opt=(scanner/A)
 76:                    i, err := strconv.ParseInt(s, 10, 64)
 92:                    fmt.Fprintf(os.Stderr, "%d\t%d\t%d\t%d\t%d\n", i, a, n, i*a, t) //gdb-dbg=(n,i,t)
 87:                    if a == 0 { //gdb-opt=(a,n,t)
 88:                            continue
+98:    }
index f6c6a3c9bec8bafbf5c9bd16e1232c24e7a78609..e4dc2808694a5225b8518bc02126e9940d7c32ae 100644 (file)
@@ -1,5 +1,5 @@
   src/cmd/compile/internal/ssa/testdata/hist.go
-55:    func main() {
+55:    func test() {
 57:            l := line{point{1 + zero, 2 + zero}, point{3 + zero, 4 + zero}}
 58:            tinycall()                // this forces l etc to stack
 57:            l := line{point{1 + zero, 2 + zero}, point{3 + zero, 4 + zero}}
@@ -13,11 +13,10 @@ dy = <Optimized out, as expected>
 61:            sink = dx + dy            //gdb-opt=(dx,dy)
 dx = 2
 dy = 2
-63:            hist := make([]int, 7)                                //gdb-opt=(sink,dx/O,dy/O)
-sink = 4
+63:            hist := make([]int, 7)                                //gdb-opt=(dx/O,dy/O) // TODO sink is missing if this code is in 'test' instead of 'main'
 dx = <Optimized out, as expected>
 dy = <Optimized out, as expected>
-64:            var reader io.Reader = strings.NewReader(cannedInput) //gdb-dbg=(hist/A,cannedInput/A)
+64:            var reader io.Reader = strings.NewReader(cannedInput) //gdb-dbg=(hist/A) // TODO cannedInput/A is missing if this code is in 'test' instead of 'main'
 65:            if len(os.Args) > 1 {
 73:            scanner := bufio.NewScanner(reader)
 74:            for scanner.Scan() { //gdb-opt=(scanner/A)
@@ -180,3 +179,4 @@ a = 0
 n = 9
 t = 22
 88:                            continue
+98:    }
diff --git a/src/cmd/compile/internal/ssa/testdata/i22558.dbg-22558-dlv.nexts b/src/cmd/compile/internal/ssa/testdata/i22558.dbg-22558-dlv.nexts
new file mode 100644 (file)
index 0000000..3c33fe0
--- /dev/null
@@ -0,0 +1,11 @@
+  ./testdata/i22558.go
+19:    func test(t *thing, u *thing) {
+20:            if t.next != nil {
+23:            fmt.Fprintf(os.Stderr, "%s\n", t.name)
+24:            u.self = u
+25:            t.self = t
+26:            t.next = u
+27:            for _, p := range t.stuff {
+28:                    if isFoo(t, p) {
+29:                            return
+43:    }
diff --git a/src/cmd/compile/internal/ssa/testdata/i22558.dbg-22558-gdb.nexts b/src/cmd/compile/internal/ssa/testdata/i22558.dbg-22558-gdb.nexts
new file mode 100644 (file)
index 0000000..b88a227
--- /dev/null
@@ -0,0 +1,11 @@
+  src/cmd/compile/internal/ssa/testdata/i22558.go
+19:    func test(t *thing, u *thing) {
+20:            if t.next != nil {
+23:            fmt.Fprintf(os.Stderr, "%s\n", t.name)
+24:            u.self = u
+25:            t.self = t
+26:            t.next = u
+27:            for _, p := range t.stuff {
+28:                    if isFoo(t, p) {
+29:                            return
+43:    }
diff --git a/src/cmd/compile/internal/ssa/testdata/i22558.go b/src/cmd/compile/internal/ssa/testdata/i22558.go
new file mode 100644 (file)
index 0000000..a62e11e
--- /dev/null
@@ -0,0 +1,43 @@
+package main
+
+import (
+       "fmt"
+       "os"
+)
+
+type big struct {
+       pile [768]int8
+}
+
+type thing struct {
+       name  string
+       next  *thing
+       self  *thing
+       stuff []big
+}
+
+func test(t *thing, u *thing) {
+       if t.next != nil {
+               return
+       }
+       fmt.Fprintf(os.Stderr, "%s\n", t.name)
+       u.self = u
+       t.self = t
+       t.next = u
+       for _, p := range t.stuff {
+               if isFoo(t, p) {
+                       return
+               }
+       }
+}
+
+//go:noinline
+func isFoo(t *thing, b big) bool {
+       return true
+}
+
+func main() {
+       t := &thing{name: "t", self: nil, next: nil, stuff: make([]big, 1)}
+       u := thing{name: "u", self: t, next: t, stuff: make([]big, 1)}
+       test(t, &u)
+}
index 681167d3af1329b5b28f52e64509219dc1b0068a..bfffec4a5dc5a6e20564283fa37966c9bc11f23c 100644 (file)
@@ -1,6 +1,7 @@
   src/cmd/compile/internal/ssa/testdata/i22600.go
-8:     func main() {
+8:     func test() {
 9:             pwd, err := os.Getwd()
 10:            if err != nil {
 14:            fmt.Println(pwd)
 15:    }
+19:    }
index 8cecd14a4bab11201b5550f17a27e5ea4d30c00a..f7a7ade37467653d1f0fb461e66f6cdc6e309ce1 100644 (file)
@@ -5,7 +5,7 @@ import (
        "os"
 )
 
-func main() {
+func test() {
        pwd, err := os.Getwd()
        if err != nil {
                fmt.Println(err)
@@ -13,3 +13,7 @@ func main() {
        }
        fmt.Println(pwd)
 }
+
+func main() {
+       test()
+}