From adb1746c6019cd2ac40e37bb00d71647245268a2 Mon Sep 17 00:00:00 2001 From: aarzilli Date: Mon, 30 Dec 2019 10:42:20 +0100 Subject: [PATCH] proc: fix inlined stack reading for midstack inlined calls Due to a bug in the Go compiler midstack inlined calls do not report their ranges correctly. We can't check if an address is in the range of a DIE by simply looking at that DIE's range, we should also recursively check the DIE's children's ranges. Also fixes the way stacktraces of midstack inlined calls are reported (they used to be inverted, with the deepest inlined stack frame reported last). Fixes #1795 --- _fixtures/issue1795.go | 14 +++++ pkg/dwarf/reader/reader.go | 110 +++++++++++++++++++++++-------------- pkg/proc/bininfo.go | 2 +- pkg/proc/proc_test.go | 50 ++++++++++++++++- 4 files changed, 133 insertions(+), 43 deletions(-) create mode 100644 _fixtures/issue1795.go diff --git a/_fixtures/issue1795.go b/_fixtures/issue1795.go new file mode 100644 index 00000000..b7e0956f --- /dev/null +++ b/_fixtures/issue1795.go @@ -0,0 +1,14 @@ +package main + +import ( + "fmt" + "regexp" + "runtime" +) + +func main() { + r := regexp.MustCompile("ab") + runtime.Breakpoint() + out := r.MatchString("blah") + fmt.Printf("%v\n", out) +} diff --git a/pkg/dwarf/reader/reader.go b/pkg/dwarf/reader/reader.go index 4f19b08f..fb2ef5fa 100644 --- a/pkg/dwarf/reader/reader.go +++ b/pkg/dwarf/reader/reader.go @@ -371,63 +371,93 @@ func LoadAbstractOrigin(entry *dwarf.Entry, aordr *dwarf.Reader) (Entry, dwarf.O type InlineStackReader struct { dwarf *dwarf.Data reader *dwarf.Reader - entry *dwarf.Entry - depth int pc uint64 + entry *dwarf.Entry err error + + // stack contains the list of DIEs that will be returned by Next. + stack []*dwarf.Entry } // InlineStack returns an InlineStackReader for the specified function and // PC address. // If pc is 0 then all inlined calls will be returned. -func InlineStack(dwarf *dwarf.Data, fnoff dwarf.Offset, pc RelAddr) *InlineStackReader { - reader := dwarf.Reader() +func InlineStack(dw *dwarf.Data, fnoff dwarf.Offset, pc RelAddr) *InlineStackReader { + reader := dw.Reader() reader.Seek(fnoff) - return &InlineStackReader{dwarf: dwarf, reader: reader, entry: nil, depth: 0, pc: uint64(pc)} + r := &InlineStackReader{dwarf: dw, reader: reader, pc: uint64(pc)} + r.precalcStack(nil) + return r +} + +// precalcStack precalculates the inlined call stack for irdr.pc. +// If irdr.pc == 0 then all inlined calls will be saved in irdr.stack. +// Otherwise an inlined call will be saved in irdr.stack if its range, or +// the range of one of its child entries contains irdr.pc. +// The recursive calculation of range inclusion is necessary because +// sometimes when doing midstack inlining the Go compiler emits the toplevel +// inlined call with ranges that do not cover the inlining of nested inlined +// calls. +// For example if a function A calls B which calls C and both the calls to +// B and C are inlined the DW_AT_inlined_subroutine entry for A might have +// ranges that do not cover the ranges of the inlined call to C. +// This is probably a violation of the DWARF standard (it's unclear) but we +// might as well support it as best as possible anyway. +func (irdr *InlineStackReader) precalcStack(rentry *dwarf.Entry) bool { + var contains bool + +childLoop: + for { + if irdr.err != nil { + return contains + } + e2, err := irdr.reader.Next() + if e2 == nil || err != nil { + break + } + + switch e2.Tag { + case 0: + break childLoop + case dwarf.TagLexDwarfBlock, dwarf.TagSubprogram, dwarf.TagInlinedSubroutine: + if irdr.precalcStack(e2) { + contains = true + } + default: + irdr.reader.SkipChildren() + } + } + + if rentry != nil && rentry.Tag == dwarf.TagInlinedSubroutine { + if !contains { + if irdr.pc != 0 { + contains, irdr.err = entryRangesContains(irdr.dwarf, rentry, irdr.pc) + } else { + contains = true + } + } + if contains { + irdr.stack = append(irdr.stack, rentry) + } + } + return contains } // Next reads next inlined call in the stack, returns false if there aren't any. +// Next reads the inlined stack of calls backwards, starting with the +// deepest inlined call and moving back out, like a normal stacktrace works. func (irdr *InlineStackReader) Next() bool { if irdr.err != nil { return false } - for { - irdr.entry, irdr.err = irdr.reader.Next() - if irdr.entry == nil || irdr.err != nil { - return false - } - - switch irdr.entry.Tag { - case 0: - irdr.depth-- - if irdr.depth == 0 { - return false - } - - case dwarf.TagLexDwarfBlock, dwarf.TagSubprogram, dwarf.TagInlinedSubroutine: - var recur bool - if irdr.pc != 0 { - recur, irdr.err = entryRangesContains(irdr.dwarf, irdr.entry, irdr.pc) - } else { - recur = true - } - if recur { - irdr.depth++ - if irdr.entry.Tag == dwarf.TagInlinedSubroutine { - return true - } - } else { - if irdr.depth == 0 { - return false - } - irdr.reader.SkipChildren() - } - - default: - irdr.reader.SkipChildren() - } + if len(irdr.stack) == 0 { + return false } + + irdr.entry = irdr.stack[0] + irdr.stack = irdr.stack[1:] + return true } // Entry returns the DIE for the current inlined call. diff --git a/pkg/proc/bininfo.go b/pkg/proc/bininfo.go index 732505aa..73a5d452 100644 --- a/pkg/proc/bininfo.go +++ b/pkg/proc/bininfo.go @@ -440,7 +440,7 @@ func (bi *BinaryInfo) PCToInlineFunc(pc uint64) *Function { fn := bi.PCToFunc(pc) irdr := reader.InlineStack(fn.cu.image.dwarf, fn.offset, reader.ToRelAddr(pc, fn.cu.image.StaticBase)) var inlineFnEntry *dwarf.Entry - for irdr.Next() { + if irdr.Next() { inlineFnEntry = irdr.Entry() } diff --git a/pkg/proc/proc_test.go b/pkg/proc/proc_test.go index 4f9282a1..92a134ae 100644 --- a/pkg/proc/proc_test.go +++ b/pkg/proc/proc_test.go @@ -3177,6 +3177,9 @@ func logStacktrace(t *testing.T, bi *proc.BinaryInfo, frames []proc.Stackframe) if frames[j].Current.Fn != nil { name = frames[j].Current.Fn.Name } + if frames[j].Call.Fn != nil && frames[j].Current.Fn != frames[j].Call.Fn { + name = fmt.Sprintf("%s inlined in %s", frames[j].Call.Fn.Name, frames[j].Current.Fn.Name) + } t.Logf("\t%#x %#x %#x %s at %s:%d\n", frames[j].Call.PC, frames[j].FrameOffset(), frames[j].FramePointerOffset(), name, filepath.Base(frames[j].Call.File), frames[j].Call.Line) if frames[j].TopmostDefer != nil { @@ -3613,8 +3616,10 @@ func checkFrame(frame proc.Stackframe, fnname, file string, line int, inlined bo if frame.Call.Fn == nil || frame.Call.Fn.Name != fnname { return fmt.Errorf("wrong function name: %s", fnname) } - if frame.Call.File != file || frame.Call.Line != line { - return fmt.Errorf("wrong file:line %s:%d", frame.Call.File, frame.Call.Line) + if file != "" { + if frame.Call.File != file || frame.Call.Line != line { + return fmt.Errorf("wrong file:line %s:%d", frame.Call.File, frame.Call.Line) + } } if frame.Inlined != inlined { if inlined { @@ -4546,3 +4551,44 @@ func TestListPackagesBuildInfo(t *testing.T) { } }) } + +func TestIssue1795(t *testing.T) { + // When doing midstack inlining the Go compiler sometimes (always?) emits + // the toplevel inlined call with ranges that do not cover the inlining of + // other nested inlined calls. + // For example if a function A calls B which calls C and both the calls to + // B and C are inlined the DW_AT_inlined_subroutine entry for A might have + // ranges that do not cover the ranges of the inlined call to C. + // This is probably a violation of the DWARF standard (it's unclear) but we + // might as well support it as best as possible anyway. + if !goversion.VersionAfterOrEqual(runtime.Version(), 1, 13) { + t.Skip("Test not relevant to Go < 1.13") + } + withTestProcessArgs("issue1795", t, ".", []string{}, protest.EnableInlining|protest.EnableOptimization, func(p proc.Process, fixture protest.Fixture) { + assertNoError(proc.Continue(p), t, "Continue()") + assertLineNumber(p, t, 12, "wrong line number after Continue,") + assertNoError(proc.Next(p), t, "Next()") + assertLineNumber(p, t, 13, "wrong line number after Next,") + }) + withTestProcessArgs("issue1795", t, ".", []string{}, protest.EnableInlining|protest.EnableOptimization, func(p proc.Process, fixture protest.Fixture) { + setFunctionBreakpoint(p, t, "regexp.(*Regexp).doExecute") + assertNoError(proc.Continue(p), t, "Continue()") + assertLineNumber(p, t, 12, "wrong line number after Continue (1),") + assertNoError(proc.Continue(p), t, "Continue()") + frames, err := proc.ThreadStacktrace(p.CurrentThread(), 40) + assertNoError(err, t, "ThreadStacktrace()") + logStacktrace(t, p.BinInfo(), frames) + if err := checkFrame(frames[0], "regexp.(*Regexp).doExecute", "", 0, false); err != nil { + t.Errorf("Wrong frame 0: %v", err) + } + if err := checkFrame(frames[1], "regexp.(*Regexp).doMatch", "", 0, true); err != nil { + t.Errorf("Wrong frame 1: %v", err) + } + if err := checkFrame(frames[2], "regexp.(*Regexp).MatchString", "", 0, true); err != nil { + t.Errorf("Wrong frame 2: %v", err) + } + if err := checkFrame(frames[3], "main.main", fixture.Source, 12, false); err != nil { + t.Errorf("Wrong frame 3: %v", err) + } + }) +}