Commit 591193b0 authored by David Chase's avatar David Chase

cmd/compile: enhance debug_test for infinite loops

ssa/debug_test.go already had a step limit; this exposes
it to individual tests, and it is then set low for the
infinite loop tests.

That however is not enough; in an infinite loop debuggers
see an unchanging line number, and therefore keep trying
until they see a different one.  To do this, the concept
of a "bogus" line number is introduced, and on output
single-instruction infinite loops are detected and a
hardware nop with correct line number is inserted into
the loop; the branch itself receives a bogus line number.

This breaks up the endless stream of same line number and
causes both gdb and delve to not hang; Delve complains
about the incorrect line number while gdb does
a sort of odd step-to-nowhere that then steps back
to the loop.  Since repeats are suppressed in the reference
file, a single line is shown there.

(The wrong line number mentioned in previous message
was an artifact of debug_test.go, not Delve, and is now
fixed.)

The bogus line number exposed in Delve is less than
wonderful, but compared to hanging, it is better.

Fixes #30664.

Change-Id: I30c927cf8869a84c6c9b84033ee44d7044aab552
Reviewed-on: https://go-review.googlesource.com/c/go/+/168477
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: default avatarKeith Randall <khr@golang.org>
parent 3089d189
...@@ -5315,6 +5315,12 @@ func genssa(f *ssa.Func, pp *Progs) { ...@@ -5315,6 +5315,12 @@ func genssa(f *ssa.Func, pp *Progs) {
} }
} }
} }
// If this is an empty infinite loop, stick a hardware NOP in there so that debuggers are less confused.
if s.bstart[b.ID] == s.pp.next && len(b.Succs) == 1 && b.Succs[0].Block() == b {
p := thearch.Ginsnop(s.pp)
p.Pos = p.Pos.WithIsStmt()
b.Pos = b.Pos.WithBogusLine() // Debuggers are not good about infinite loops, force a change in line number
}
// Emit control flow instructions for block // Emit control flow instructions for block
var next *ssa.Block var next *ssa.Block
if i < len(f.Blocks)-1 && Debug['N'] == 0 { if i < len(f.Blocks)-1 && Debug['N'] == 0 {
......
...@@ -156,33 +156,34 @@ func TestNexting(t *testing.T) { ...@@ -156,33 +156,34 @@ func TestNexting(t *testing.T) {
subTest(t, debugger+"-dbg-race", "i22600", dbgFlags, append(moreargs, "-race")...) subTest(t, debugger+"-dbg-race", "i22600", dbgFlags, append(moreargs, "-race")...)
optSubTest(t, debugger+"-opt", "hist", optFlags, moreargs...) optSubTest(t, debugger+"-opt", "hist", optFlags, 1000, moreargs...)
optSubTest(t, debugger+"-opt", "scopes", optFlags, moreargs...) optSubTest(t, debugger+"-opt", "scopes", optFlags, 1000, moreargs...)
optSubTest(t, debugger+"-opt", "infloop", optFlags, 10, moreargs...)
} }
// subTest creates a subtest that compiles basename.go with the specified gcflags and additional compiler arguments, // subTest creates a subtest that compiles basename.go with the specified gcflags and additional compiler arguments,
// then runs the debugger on the resulting binary, with any comment-specified actions matching tag triggered. // then runs the debugger on the resulting binary, with any comment-specified actions matching tag triggered.
func subTest(t *testing.T, tag string, basename string, gcflags string, moreargs ...string) { func subTest(t *testing.T, tag string, basename string, gcflags string, moreargs ...string) {
t.Run(tag+"-"+basename, func(t *testing.T) { t.Run(tag+"-"+basename, func(t *testing.T) {
testNexting(t, basename, tag, gcflags, moreargs...) testNexting(t, basename, tag, gcflags, 1000, moreargs...)
}) })
} }
// optSubTest is the same as subTest except that it skips the test if the runtime and libraries // optSubTest is the same as subTest except that it skips the test if the runtime and libraries
// were not compiled with optimization turned on. (The skip may not be necessary with Go 1.10 and later) // were not compiled with optimization turned on. (The skip may not be necessary with Go 1.10 and later)
func optSubTest(t *testing.T, tag string, basename string, gcflags string, moreargs ...string) { func optSubTest(t *testing.T, tag string, basename string, gcflags string, count int, moreargs ...string) {
// If optimized test is run with unoptimized libraries (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). // This occurs in the noopt builders (for example).
t.Run(tag+"-"+basename, func(t *testing.T) { t.Run(tag+"-"+basename, func(t *testing.T) {
if *force || optimizedLibs { if *force || optimizedLibs {
testNexting(t, basename, tag, gcflags, moreargs...) testNexting(t, basename, tag, gcflags, count, moreargs...)
} else { } else {
t.Skip("skipping for unoptimized stdlib/runtime") t.Skip("skipping for unoptimized stdlib/runtime")
} }
}) })
} }
func testNexting(t *testing.T, base, tag, gcflags string, moreArgs ...string) { func testNexting(t *testing.T, base, tag, gcflags string, count int, moreArgs ...string) {
// (1) In testdata, build sample.go into test-sample.<tag> // (1) In testdata, build sample.go into test-sample.<tag>
// (2) Run debugger gathering a history // (2) Run debugger gathering a history
// (3) Read expected history from testdata/sample.<tag>.nexts // (3) Read expected history from testdata/sample.<tag>.nexts
...@@ -219,7 +220,7 @@ func testNexting(t *testing.T, base, tag, gcflags string, moreArgs ...string) { ...@@ -219,7 +220,7 @@ func testNexting(t *testing.T, base, tag, gcflags string, moreArgs ...string) {
} else { } else {
dbg = newGdb(tag, exe) dbg = newGdb(tag, exe)
} }
h1 := runDbgr(dbg, 1000) h1 := runDbgr(dbg, count)
if *dryrun { if *dryrun {
fmt.Printf("# Tag for above is %s\n", dbg.tag()) fmt.Printf("# Tag for above is %s\n", dbg.tag())
return return
...@@ -261,6 +262,7 @@ func runDbgr(dbg dbgr, maxNext int) *nextHist { ...@@ -261,6 +262,7 @@ func runDbgr(dbg dbgr, maxNext int) *nextHist {
break break
} }
} }
dbg.quit()
h := dbg.hist() h := dbg.hist()
return h return h
} }
...@@ -298,7 +300,7 @@ func (t tstring) String() string { ...@@ -298,7 +300,7 @@ func (t tstring) String() string {
} }
type pos struct { type pos struct {
line uint16 line uint32
file uint8 // Artifact of plans to implement differencing instead of calling out to diff. file uint8 // Artifact of plans to implement differencing instead of calling out to diff.
} }
...@@ -386,7 +388,7 @@ func (h *nextHist) add(file, line, text string) bool { ...@@ -386,7 +388,7 @@ func (h *nextHist) add(file, line, text string) bool {
} }
} }
l := len(h.ps) l := len(h.ps)
p := pos{line: uint16(li), file: fi} p := pos{line: uint32(li), file: fi}
if l == 0 || *repeats || h.ps[l-1] != p { if l == 0 || *repeats || h.ps[l-1] != p {
h.ps = append(h.ps, p) h.ps = append(h.ps, p)
...@@ -721,6 +723,15 @@ func varsToPrint(line, lookfor string) []string { ...@@ -721,6 +723,15 @@ func varsToPrint(line, lookfor string) []string {
func (s *gdbState) quit() { func (s *gdbState) quit() {
response := s.ioState.writeRead("q\n") response := s.ioState.writeRead("q\n")
if strings.Contains(response.o, "Quit anyway? (y or n)") { if strings.Contains(response.o, "Quit anyway? (y or n)") {
defer func() {
if r := recover(); r != nil {
if s, ok := r.(string); !(ok && strings.Contains(s, "'Y\n'")) {
// Not the panic that was expected.
fmt.Printf("Expected a broken pipe panic, but saw the following panic instead")
panic(r)
}
}
}()
s.ioState.writeRead("Y\n") s.ioState.writeRead("Y\n")
} }
} }
......
./testdata/infloop.go
6: func test() {
8: go func() {}()
10: for {
1048575:
10: for {
1048575:
10: for {
1048575:
10: for {
1048575:
10: for {
src/cmd/compile/internal/ssa/testdata/infloop.go
6: func test() {
8: go func() {}()
10: for {
package main
var sink int
//go:noinline
func test() {
// This is for #30167, incorrect line numbers in an infinite loop
go func() {}()
for {
}
}
func main() {
test()
}
...@@ -304,7 +304,8 @@ type lico uint32 ...@@ -304,7 +304,8 @@ type lico uint32
// TODO: Prologue and epilogue are perhaps better handled as psuedoops for the assembler, // TODO: Prologue and epilogue are perhaps better handled as psuedoops for the assembler,
// because they have almost no interaction with other uses of the position. // because they have almost no interaction with other uses of the position.
const ( const (
lineBits, lineMax = 20, 1<<lineBits - 1 lineBits, lineMax = 20, 1<<lineBits - 2
bogusLine = 1<<lineBits - 1 // Not a line number; used to disruopt infinite loops
isStmtBits, isStmtMax = 2, 1<<isStmtBits - 1 isStmtBits, isStmtMax = 2, 1<<isStmtBits - 1
xlogueBits, xlogueMax = 2, 1<<xlogueBits - 1 xlogueBits, xlogueMax = 2, 1<<xlogueBits - 1
colBits, colMax = 32 - lineBits - xlogueBits - isStmtBits, 1<<colBits - 1 colBits, colMax = 32 - lineBits - xlogueBits - isStmtBits, 1<<colBits - 1
...@@ -355,6 +356,16 @@ const ( ...@@ -355,6 +356,16 @@ const (
PosEpilogueBegin PosEpilogueBegin
) )
func makeLicoRaw(line, col uint) lico {
return lico(line<<lineShift | col<<colShift)
}
// This is a not-position that will not be elided.
// Depending on the debugger (gdb or delve) it may or may not be displayed.
func makeBogusLico() lico {
return makeLicoRaw(bogusLine, 0).withIsStmt()
}
func makeLico(line, col uint) lico { func makeLico(line, col uint) lico {
if line > lineMax { if line > lineMax {
// cannot represent line, use max. line so we have some information // cannot represent line, use max. line so we have some information
...@@ -365,7 +376,7 @@ func makeLico(line, col uint) lico { ...@@ -365,7 +376,7 @@ func makeLico(line, col uint) lico {
col = colMax col = colMax
} }
// default is not-sure-if-statement // default is not-sure-if-statement
return lico(line<<lineShift | col<<colShift) return makeLicoRaw(line, col)
} }
func (x lico) Line() uint { return uint(x) >> lineShift } func (x lico) Line() uint { return uint(x) >> lineShift }
......
...@@ -60,6 +60,16 @@ func (p XPos) WithIsStmt() XPos { ...@@ -60,6 +60,16 @@ func (p XPos) WithIsStmt() XPos {
return p return p
} }
// WithBogusLine returns a bogus line that won't match any recorded for the source code.
// Its use is to disrupt the statements within an infinite loop so that the debugger
// will not itself loop infinitely waiting for the line number to change.
// gdb chooses not to display the bogus line; delve shows it with a complaint, but the
// alternative behavior is to hang.
func (p XPos) WithBogusLine() XPos {
p.lico = makeBogusLico()
return p
}
// WithXlogue returns the same location but marked with DWARF function prologue/epilogue // WithXlogue returns the same location but marked with DWARF function prologue/epilogue
func (p XPos) WithXlogue(x PosXlogue) XPos { func (p XPos) WithXlogue(x PosXlogue) XPos {
p.lico = p.lico.withXlogue(x) p.lico = p.lico.withXlogue(x)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment