Merge "Cleanup the libbacktrace interface a bit."

8 files changed, 37 insertions, 29 deletions

diff --git a/libbacktrace/Backtrace.cpp b/libbacktrace/Backtrace.cpp
index e46d353..81f5e32 100644
--- a/libbacktrace/Backtrace.cpp
+++ b/libbacktrace/Backtrace.cpp
@@ -84,10 +84,8 @@ std::string Backtrace::FormatFrameData(size_t frame_num) {
 }
 
 std::string Backtrace::FormatFrameData(const backtrace_frame_data_t* frame) {
-  uintptr_t relative_pc;
   std::string map_name;
   if (BacktraceMap::IsValid(frame->map)) {
-    relative_pc = BacktraceMap::GetRelativePc(frame->map, frame->pc);
     if (!frame->map.name.empty()) {
       map_name = frame->map.name.c_str();
       if (map_name[0] == '[' && map_name[map_name.size() - 1] == ']') {
@@ -99,10 +97,9 @@ std::string Backtrace::FormatFrameData(const backtrace_frame_data_t* frame) {
     }
   } else {
     map_name = "<unknown>";
-    relative_pc = frame->pc;
   }
 
-  std::string line(StringPrintf("#%02zu pc %" PRIPTR "  ", frame->num, relative_pc));
+  std::string line(StringPrintf("#%02zu pc %" PRIPTR "  ", frame->num, frame->rel_pc));
   line += map_name;
   // Special handling for non-zero offset maps, we need to print that
   // information.
diff --git a/libbacktrace/UnwindCurrent.cpp b/libbacktrace/UnwindCurrent.cpp
index 3c509e6..2c87fa8 100644
--- a/libbacktrace/UnwindCurrent.cpp
+++ b/libbacktrace/UnwindCurrent.cpp
@@ -133,6 +133,11 @@ bool UnwindCurrent::UnwindFromContext(size_t num_ignore_frames, ucontext_t* ucon
           backtrace_frame_data_t* prev = &frames_.at(num_frames-1);
           prev->stack_size = frame->sp - prev->sp;
         }
+        if (BacktraceMap::IsValid(frame->map)) {
+          frame->rel_pc = frame->pc - frame->map.start + frame->map.load_bias;
+        } else {
+          frame->rel_pc = frame->pc;
+        }
         num_frames++;
       } else {
         num_ignore_frames--;
diff --git a/libbacktrace/UnwindMap.cpp b/libbacktrace/UnwindMap.cpp
index af79562..0b8232b 100644
--- a/libbacktrace/UnwindMap.cpp
+++ b/libbacktrace/UnwindMap.cpp
@@ -57,7 +57,7 @@ bool UnwindMapRemote::GenerateMap() {
     map.start = unw_map.start;
     map.end = unw_map.end;
     map.offset = unw_map.offset;
-    map.load_base = unw_map.load_base;
+    map.load_bias = unw_map.load_base;
     map.flags = unw_map.flags;
     map.name = unw_map.path;
 
@@ -106,7 +106,7 @@ bool UnwindMapLocal::GenerateMap() {
       map.start = unw_map.start;
       map.end = unw_map.end;
       map.offset = unw_map.offset;
-      map.load_base = unw_map.load_base;
+      map.load_bias = unw_map.load_base;
       map.flags = unw_map.flags;
       map.name = unw_map.path;
 
diff --git a/libbacktrace/UnwindPtrace.cpp b/libbacktrace/UnwindPtrace.cpp
index 42ac1bc..87282ef 100644
--- a/libbacktrace/UnwindPtrace.cpp
+++ b/libbacktrace/UnwindPtrace.cpp
@@ -135,6 +135,11 @@ bool UnwindPtrace::Unwind(size_t num_ignore_frames, ucontext_t* ucontext) {
       }
 
       FillInMap(frame->pc, &frame->map);
+      if (BacktraceMap::IsValid(frame->map)) {
+        frame->rel_pc = frame->pc - frame->map.start + frame->map.load_bias;
+      } else {
+        frame->rel_pc = frame->pc;
+      }
 
       frame->func_name = GetFunctionName(frame->pc, &frame->func_offset, &frame->map);
 
diff --git a/libbacktrace/backtrace_offline_test.cpp b/libbacktrace/backtrace_offline_test.cpp
index 465b3f9..16b1d79 100644
--- a/libbacktrace/backtrace_offline_test.cpp
+++ b/libbacktrace/backtrace_offline_test.cpp
@@ -167,9 +167,9 @@ TEST(libbacktrace, DISABLED_generate_offline_testdata) {
   // 2. Dump maps
   for (auto it = map->begin(); it != map->end(); ++it) {
     testdata += android::base::StringPrintf(
-        "map: start: %" PRIxPTR " end: %" PRIxPTR " offset: %" PRIxPTR
-        " load_base: %" PRIxPTR " flags: %d name: %s\n",
-        it->start, it->end, it->offset, it->load_base, it->flags, it->name.c_str());
+        "map: start: %" PRIxPTR " end: %" PRIxPTR " offset: %" PRIxPTR " load_bias: %" PRIxPTR
+        " flags: %d name: %s\n",
+        it->start, it->end, it->offset, it->load_bias, it->flags, it->name.c_str());
   }
   // 3. Dump registers
   testdata += android::base::StringPrintf("registers: %zu ", sizeof(arg.unw_context));
@@ -246,9 +246,9 @@ bool ReadOfflineTestData(const std::string offline_testdata_path, OfflineTestDat
       backtrace_map_t& map = testdata->maps.back();
       int pos;
       sscanf(line.c_str(),
-             "map: start: %" SCNxPTR " end: %" SCNxPTR " offset: %" SCNxPTR
-             " load_base: %" SCNxPTR " flags: %d name: %n",
-             &map.start, &map.end, &map.offset, &map.load_base, &map.flags, &pos);
+             "map: start: %" SCNxPTR " end: %" SCNxPTR " offset: %" SCNxPTR " load_bias: %" SCNxPTR
+             " flags: %d name: %n",
+             &map.start, &map.end, &map.offset, &map.load_bias, &map.flags, &pos);
       map.name = android::base::Trim(line.substr(pos));
     } else if (android::base::StartsWith(line, "registers:")) {
       size_t size;
@@ -392,8 +392,8 @@ TEST(libbacktrace, offline_unwind_mix_eh_frame_and_arm_exidx) {
   // The last frame is outside of libart.so
   ASSERT_EQ(testdata.symbols.size() + 1, backtrace->NumFrames());
   for (size_t i = 0; i + 1 < backtrace->NumFrames(); ++i) {
-    uintptr_t vaddr_in_file = backtrace->GetFrame(i)->pc - testdata.maps[0].start +
-        testdata.maps[0].load_base;
+    uintptr_t vaddr_in_file =
+        backtrace->GetFrame(i)->pc - testdata.maps[0].start + testdata.maps[0].load_bias;
     std::string name = FunctionNameForAddress(vaddr_in_file, testdata.symbols);
     ASSERT_EQ(name, testdata.symbols[i].name);
   }
diff --git a/libbacktrace/backtrace_test.cpp b/libbacktrace/backtrace_test.cpp
index fb463b0..1ec6a45 100644
--- a/libbacktrace/backtrace_test.cpp
+++ b/libbacktrace/backtrace_test.cpp
@@ -784,6 +784,7 @@ TEST(libbacktrace, format_test) {
   backtrace_frame_data_t frame;
   frame.num = 1;
   frame.pc = 2;
+  frame.rel_pc = 2;
   frame.sp = 0;
   frame.stack_size = 0;
   frame.func_offset = 0;
@@ -799,9 +800,10 @@ TEST(libbacktrace, format_test) {
 
   // Check map name empty, but exists.
   frame.pc = 0xb0020;
+  frame.rel_pc = 0x20;
   frame.map.start = 0xb0000;
   frame.map.end = 0xbffff;
-  frame.map.load_base = 0;
+  frame.map.load_bias = 0;
 #if defined(__LP64__)
   EXPECT_EQ("#01 pc 0000000000000020  <anonymous:00000000000b0000>",
 #else
@@ -813,7 +815,7 @@ TEST(libbacktrace, format_test) {
   frame.pc = 0xc0020;
   frame.map.start = 0xc0000;
   frame.map.end = 0xcffff;
-  frame.map.load_base = 0;
+  frame.map.load_bias = 0;
   frame.map.name = "[anon:thread signal stack]";
 #if defined(__LP64__)
   EXPECT_EQ("#01 pc 0000000000000020  [anon:thread signal stack:00000000000c0000]",
@@ -824,6 +826,7 @@ TEST(libbacktrace, format_test) {
 
   // Check relative pc is set and map name is set.
   frame.pc = 0x12345679;
+  frame.rel_pc = 0x12345678;
   frame.map.name = "MapFake";
   frame.map.start =  1;
   frame.map.end =  1;
@@ -852,9 +855,10 @@ TEST(libbacktrace, format_test) {
 #endif
             backtrace->FormatFrameData(&frame));
 
-  // Check func_name is set, func offset is non-zero, and load_base is non-zero.
+  // Check func_name is set, func offset is non-zero, and load_bias is non-zero.
+  frame.rel_pc = 0x123456dc;
   frame.func_offset = 645;
-  frame.map.load_base = 100;
+  frame.map.load_bias = 100;
 #if defined(__LP64__)
   EXPECT_EQ("#01 pc 00000000123456dc  MapFake (ProcFake+645)",
 #else
@@ -1737,9 +1741,13 @@ static void UnwindThroughSignal(bool use_action) {
   FinishRemoteProcess(pid);
 }
 
-TEST(libbacktrace, unwind_remote_through_signal_using_handler) { UnwindThroughSignal(false); }
+TEST(libbacktrace, unwind_remote_through_signal_using_handler) {
+  UnwindThroughSignal(false);
+}
 
-TEST(libbacktrace, unwind_remote_through_signal_using_action) { UnwindThroughSignal(true); }
+TEST(libbacktrace, unwind_remote_through_signal_using_action) {
+  UnwindThroughSignal(true);
+}
 
 #if defined(ENABLE_PSS_TESTS)
 #include "GetPss.h"
diff --git a/libbacktrace/include/backtrace/Backtrace.h b/libbacktrace/include/backtrace/Backtrace.h
index 4f73a65..b919e81 100644
--- a/libbacktrace/include/backtrace/Backtrace.h
+++ b/libbacktrace/include/backtrace/Backtrace.h
@@ -55,6 +55,7 @@ enum BacktraceUnwindError : uint32_t {
 struct backtrace_frame_data_t {
   size_t num;             // The current fame number.
   uintptr_t pc;           // The absolute pc.
+  uintptr_t rel_pc;       // The relative pc.
   uintptr_t sp;           // The top of the stack.
   size_t stack_size;      // The size of the stack, zero indicate an unknown stack size.
   backtrace_map_t map;    // The map associated with the given pc.
diff --git a/libbacktrace/include/backtrace/BacktraceMap.h b/libbacktrace/include/backtrace/BacktraceMap.h
index 8ab0dfa..02a50f7 100644
--- a/libbacktrace/include/backtrace/BacktraceMap.h
+++ b/libbacktrace/include/backtrace/BacktraceMap.h
@@ -41,7 +41,7 @@ struct backtrace_map_t {
   uintptr_t start = 0;
   uintptr_t end = 0;
   uintptr_t offset = 0;
-  uintptr_t load_base = 0;
+  uintptr_t load_bias = 0;
   int flags = 0;
   std::string name;
 };
@@ -95,14 +95,6 @@ public:
     return map.end > 0;
   }
 
-  static uintptr_t GetRelativePc(const backtrace_map_t& map, uintptr_t pc) {
-    if (IsValid(map)) {
-      return pc - map.start + map.load_base;
-    } else {
-      return pc;
-    }
-  }
-
 protected:
   BacktraceMap(pid_t pid);