Skip to content

Commit 7080b62

Browse files
authored
pkg/dwarf: do not insist stmt is same line as entry (#4186)
This invariant does not hold for optimized binaries. For the example program included in this patch, the disassembly of the function for an optimized build looks like the following: ``` TEXT main.ManyArgsWithNamedReturns(SB) /home/deparker/Code/delve/_fixtures/multinamedreturns.go multinamedreturns.go:24 0x4a9f40 4c894c2470 mov qword ptr [rsp+0x70], r9 multinamedreturns.go:24 0x4a9f45 4c899c2480000000 mov qword ptr [rsp+0x80], r11 => multinamedreturns.go:7 0x4a9f4d 488d1403 lea rdx, ptr [rbx+rax*1] multinamedreturns.go:7 0x4a9f51 4801ca add rdx, rcx multinamedreturns.go:7 0x4a9f54 4801fa add rdx, rdi multinamedreturns.go:7 0x4a9f57 4801f2 add rdx, rsi multinamedreturns.go:7 0x4a9f5a 4c01c2 add rdx, r8 multinamedreturns.go:7 0x4a9f5d 4c01ca add rdx, r9 multinamedreturns.go:7 0x4a9f60 4c01d2 add rdx, r10 ... ``` As you can see the entry is marked as line 24, whereas the first line we really want to stop at is line 7 in this build. Enforcing that we want to find a statement with the same line as the entry, this makes Delve stop at the end at the `ret` instruction: ``` multinamedreturns.go:22 0x4aa01b 488d1c11 lea rbx, ptr [rcx+rdx*1] multinamedreturns.go:22 0x4aa01f 48d1fb sar rbx, 0x1 multinamedreturns.go:24 0x4aa022 c3 ret ```
1 parent d3ee4ad commit 7080b62

File tree

4 files changed

+63
-11
lines changed

4 files changed

+63
-11
lines changed

_fixtures/multinamedreturns.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
package main
2+
3+
import "fmt"
4+
5+
//go:noinline
6+
func ManyArgsWithNamedReturns(a, b, c, d, e, f, g, h, i, j, k, l, m, n, o, p int) (sum int, product int) {
7+
sum = a + b + c + d + e + f + g + h
8+
product = 1
9+
10+
temp1 := i * j
11+
temp2 := k * l
12+
temp3 := m * n
13+
temp4 := o * p
14+
15+
sum += temp1 + temp2 + temp3 + temp4
16+
17+
product = (a + 1) * (b + 1) * (c + 1) * (d + 1)
18+
product += (e + f) * (g + h) * (i + j) * (k + l)
19+
product += (m + n) * (o + p)
20+
21+
sum = sum * 2
22+
product = product / 2
23+
24+
return sum, product
25+
}
26+
27+
func main() {
28+
sum, product := ManyArgsWithNamedReturns(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16)
29+
fmt.Println(sum, product)
30+
}

pkg/dwarf/line/state_machine.go

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -351,21 +351,16 @@ func (lineInfo *DebugLineInfo) PrologueEndPC(start, end uint64) (pc uint64, file
351351
}
352352
}
353353

354-
// FirstStmtForLine looks in the half open interval [start, end) for the
355-
// first PC address marked as stmt for the line at address 'start'.
356-
func (lineInfo *DebugLineInfo) FirstStmtForLine(start, end uint64) (pc uint64, file string, line int, ok bool) {
357-
first := true
354+
// FirstStmt looks in the half open interval [start, end) for the
355+
// first PC address marked as stmt.
356+
func (lineInfo *DebugLineInfo) FirstStmt(start, end uint64) (pc uint64, file string, line int, ok bool) {
358357
sm := lineInfo.stateMachineForEntry(start)
359358
for {
360359
if sm.valid {
361360
if sm.address >= end {
362361
return 0, "", 0, false
363362
}
364-
if first {
365-
first = false
366-
file, line = sm.file, sm.line
367-
}
368-
if sm.isStmt && sm.file == file && sm.line == line {
363+
if sm.isStmt {
369364
return sm.address, sm.file, sm.line, true
370365
}
371366
}

pkg/proc/bininfo.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ import (
3838
"github.com/go-delve/delve/pkg/logflags"
3939
"github.com/go-delve/delve/pkg/proc/debuginfod"
4040
"github.com/go-delve/delve/pkg/proc/evalop"
41-
"github.com/hashicorp/golang-lru/v2"
41+
lru "github.com/hashicorp/golang-lru/v2"
4242
)
4343

4444
const (
@@ -405,7 +405,7 @@ func FirstPCAfterPrologue(p Process, fn *Function, sameline bool) (uint64, error
405405
// Look for the first instruction with the stmt flag set, so that setting a
406406
// breakpoint with file:line and with the function name always result on
407407
// the same instruction being selected.
408-
if pc2, _, _, ok := fn.cu.lineInfo.FirstStmtForLine(fn.Entry, fn.End); ok {
408+
if pc2, _, _, ok := fn.cu.lineInfo.FirstStmt(fn.Entry, fn.End); ok {
409409
return pc2, nil
410410
}
411411
}

pkg/proc/proc_test.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -977,6 +977,33 @@ func TestBreakpointOnFunctionEntry(t *testing.T) {
977977
testseq2(t, "testprog", "main.main", []seqTest{{contContinue, 17}})
978978
}
979979

980+
func TestFirstStmtOptimizedBinaries(t *testing.T) {
981+
protest.AllowRecording(t)
982+
withTestProcessArgs("multinamedreturns", t, ".", []string{}, protest.EnableOptimization, func(p *proc.Target, grp *proc.TargetGroup, fixture protest.Fixture) {
983+
_ = setFunctionBreakpoint(p, t, "main.ManyArgsWithNamedReturns")
984+
985+
assertNoError(grp.Continue(), t, "Continue()")
986+
987+
pc := currentPC(p, t)
988+
989+
fn := p.BinInfo().PCToFunc(pc)
990+
if fn == nil {
991+
t.Fatal("Could not find function for current PC")
992+
}
993+
if fn.Name != "main.ManyArgsWithNamedReturns" {
994+
t.Fatalf("Expected to be in main.ManyArgsWithNamedReturns, got %s", fn.Name)
995+
}
996+
997+
_, f, l, _ := currentLocation(p, t)
998+
if !strings.Contains(f, "multinamedreturns.go") || l != 7 {
999+
t.Fatalf("Expected file name to be multinamedreturns.go and line number to be 7, got %s:%d", f, l)
1000+
}
1001+
if pc == fn.End-1 {
1002+
t.Fatalf("PC is at function end (0x%x), expected to be at function entry", pc)
1003+
}
1004+
})
1005+
}
1006+
9801007
func TestProcessReceivesSIGCHLD(t *testing.T) {
9811008
protest.AllowRecording(t)
9821009
withTestProcess("sigchldprog", t, func(p *proc.Target, grp *proc.TargetGroup, fixture protest.Fixture) {

0 commit comments

Comments
 (0)