[symbolize] Handle backtrace addresses correctly
This change handles the issue where the reported addresses are return addresses
and not the caller address. Because subtracting 1 will on x86_64 and AArch64
cause the address to be in the middle (or start on x86_64 in rare cases) this
is always a valid technique.
Test: Just symbolized a real crash with it
Change-Id: I3838b9e929763e6a9a04a2650297951839f6acd1
diff --git a/symbolize/demuxer_test.go b/symbolize/demuxer_test.go
index c5808ea..ae6ee90 100644
--- a/symbolize/demuxer_test.go
+++ b/symbolize/demuxer_test.go
@@ -250,8 +250,8 @@
msg := "{{{module:1:libc.so:elf:4fcb712aa6387724a9f465a32cd8c14b}}}\n" +
"{{{mmap:0x12345000:0xcf6bc:load:1:rx:0x0}}}\n" +
"Backtrace:\n" +
- "{{{bt:0:0x12388680}}}\n" +
- "{{{bt:1:0x123879c0}}}\n"
+ "{{{bt:0:0x12388681}}}\n" +
+ "{{{bt:1:0x123879c1}}}\n"
// start sending InputLines to the demuxer
ctx := context.Background()
@@ -296,10 +296,10 @@
"[131.503] 1234.5678> {{{mmap:0x23456000:0x83c80:load:09:rx:0x80000}}}\n" +
"[131.503] 1234.5678> {{{mmap:0x34567000:0x1000:load:3:rx:0x0}}}\n" +
"[131.604] 1234.5678> Backtrace:\n" +
- "[131.604] 1234.5678> {{{bt:0:0x12388680}}}\n" +
- "[131.604] 1234.5678> {{{bt:1:0x23457000}}}\n" +
- "[131.604] 1234.5678> {{{bt:2:0x123879c0}}}\n" +
- "[131.705] 1234.5678> {{{bt:3:0x34567042}}}\n"
+ "[131.604] 1234.5678> {{{bt:0:0x12388681}}}\n" +
+ "[131.604] 1234.5678> {{{bt:1:0x23457001}}}\n" +
+ "[131.604] 1234.5678> {{{bt:2:0x123879c1}}}\n" +
+ "[131.705] 1234.5678> {{{bt:3:0x34567043}}}\n"
// start sending InputLines to the demuxer
ctx := context.Background()
diff --git a/symbolize/filter.go b/symbolize/filter.go
index 2d6fcaa..bcdf8b6 100644
--- a/symbolize/filter.go
+++ b/symbolize/filter.go
@@ -225,7 +225,26 @@
}
func (f *filterVisitor) VisitBt(elem *BacktraceElement) {
- info, err := f.filter.findInfoForAddress(elem.vaddr)
+
+ // From //zircon/docs/symbolizer_markup.md:
+ // In frames after frame zero, this code location identifies a call site.
+ // Some emitters may subtract one byte or one instruction length from the
+ // actual return address for the call site, with the intent that the address
+ // logged can be translated directly to a source location for the call site
+ // and not for the apparent return site thereafter (which can be confusing).
+ // It‘s recommended that emitters not do this, so that each frame’s code
+ // location is the exact return address given to its callee and e.g. could be
+ // highlighted in instruction-level disassembly. The symbolizing filter can do
+ // the adjustment to the address it translates into a source location. Assuming
+ // that a call instruction is longer than one byte on all supported machines,
+ // applying the "subtract one byte" adjustment a second time still results in an
+ // address somewhere in the call instruction, so a little sloppiness here does
+ // no harm.
+ vaddr := elem.vaddr
+ if vaddr != 0 {
+ vaddr -= 1
+ }
+ info, err := f.filter.findInfoForAddress(vaddr)
if err != nil {
// Don't be noisy about missing objects.
if _, ok := err.(*missingObjError); !ok {
diff --git a/symbolize/filter_test.go b/symbolize/filter_test.go
index 889fdc5..d20926b 100644
--- a/symbolize/filter_test.go
+++ b/symbolize/filter_test.go
@@ -126,7 +126,7 @@
func TestBacktrace(t *testing.T) {
parseLine := GetLineParser()
- line := parseLine("Error at {{{bt:0:0x12389987}}}")
+ line := parseLine("Error at {{{bt:0:0x12389988}}}")
if line == nil {
t.Error("got", nil, "expected", "not nil")
@@ -160,7 +160,7 @@
expectedJson := []byte(`[
{"type": "text", "text": "Error at "},
- {"type": "bt", "vaddr": 305699207, "num": 0, "locs":[
+ {"type": "bt", "vaddr": 305699208, "num": 0, "locs":[
{"line": 64, "function": "duffcopy", "file": "duff.h"},
{"line": 76, "function": "memcpy", "file": "memcpy.c"}
]}
