Merge "libbacktraceoffline: choose correct debug section for a given address." am: 5c3a707542

am: c458193f7f

Change-Id: I0c705f6cbc04f90713cc834d68d85a672982bafd

libbacktrace/BacktraceOffline.cpp

@@ -21,6 +21,7 @@ extern "C" {
 #include <dwarf.h>
 }
 
+#include <pthread.h>
 #include <stdint.h>
 #include <stdio.h>
 #include <string.h>
@@ -282,39 +283,14 @@ bool BacktraceOffline::FindProcInfo(unw_addr_space_t addr_space, uint64_t ip,
 
   // vaddr in the elf file.
   uint64_t ip_vaddr = ip - map.start + debug_frame->min_vaddr;
-  if (debug_frame->has_arm_exidx) {
-    auto& func_vaddrs = debug_frame->arm_exidx.func_vaddr_array;
-    if (ip_vaddr >= func_vaddrs[0] && ip_vaddr < debug_frame->text_end_vaddr) {
-      // Use binary search to find the correct function.
-      auto it = std::upper_bound(func_vaddrs.begin(), func_vaddrs.end(),
-                                 static_cast<uint32_t>(ip_vaddr));
-      if (it != func_vaddrs.begin()) {
-        --it;
-        // Found the exidx entry.
-        size_t index = it - func_vaddrs.begin();
-
-        proc_info->format = UNW_INFO_FORMAT_ARM_EXIDX;
-        proc_info->unwind_info = reinterpret_cast<void*>(
-            static_cast<uintptr_t>(index * sizeof(ArmIdxEntry) +
-                                   debug_frame->arm_exidx.exidx_vaddr +
-                                   debug_frame->min_vaddr));
-
-        // Prepare arm_exidx space and arm_extab space.
-        arm_exidx_space_.start = debug_frame->min_vaddr + debug_frame->arm_exidx.exidx_vaddr;
-        arm_exidx_space_.end = arm_exidx_space_.start +
-            debug_frame->arm_exidx.exidx_data.size() * sizeof(ArmIdxEntry);
-        arm_exidx_space_.data = reinterpret_cast<const uint8_t*>(
-            debug_frame->arm_exidx.exidx_data.data());
-
-        arm_extab_space_.start = debug_frame->min_vaddr + debug_frame->arm_exidx.extab_vaddr;
-        arm_extab_space_.end = arm_extab_space_.start +
-            debug_frame->arm_exidx.extab_data.size();
-        arm_extab_space_.data = debug_frame->arm_exidx.extab_data.data();
-        return true;
-      }
-    }
-  }
 
+  // The unwind info can come from .ARM.exidx or .eh_frame, or .debug_frame/.gnu_debugdata.
+  // First check .eh_frame/.debug_frame, then check .ARM.exidx. Because .eh_frame/.debug_frame has
+  // function range for each entry, by matching ip address with the function range, we know exactly
+  // whether the ip address hits an entry. But .ARM.exidx doesn't have function range for each
+  // entry, it thinks that an ip address hits an entry when (entry.addr <= ip < next_entry.addr).
+  // To prevent ip addresses hit in .eh_frame/.debug_frame being regarded as addresses hit in
+  // .ARM.exidx, we need to check .eh_frame/.debug_frame first.
   if (debug_frame->has_eh_frame) {
     if (ip_vaddr >= debug_frame->eh_frame.min_func_vaddr &&
         ip_vaddr < debug_frame->text_end_vaddr) {
@@ -323,7 +299,6 @@ bool BacktraceOffline::FindProcInfo(unw_addr_space_t addr_space, uint64_t ip,
       eh_frame_hdr_space_.end =
           eh_frame_hdr_space_.start + debug_frame->eh_frame.hdr_data.size();
       eh_frame_hdr_space_.data = debug_frame->eh_frame.hdr_data.data();
-
       eh_frame_space_.start = ip - ip_vaddr + debug_frame->eh_frame.vaddr;
       eh_frame_space_.end = eh_frame_space_.start + debug_frame->eh_frame.data.size();
       eh_frame_space_.data = debug_frame->eh_frame.data.data();
@@ -345,7 +320,6 @@ bool BacktraceOffline::FindProcInfo(unw_addr_space_t addr_space, uint64_t ip,
       }
     }
   }
-
   if (debug_frame->has_debug_frame || debug_frame->has_gnu_debugdata) {
     unw_dyn_info_t di;
     unw_word_t segbase = map.start - map.offset;
@@ -359,6 +333,40 @@ bool BacktraceOffline::FindProcInfo(unw_addr_space_t addr_space, uint64_t ip,
       }
     }
   }
+
+  if (debug_frame->has_arm_exidx) {
+    auto& func_vaddrs = debug_frame->arm_exidx.func_vaddr_array;
+    if (ip_vaddr >= func_vaddrs[0] && ip_vaddr < debug_frame->text_end_vaddr) {
+      // Use binary search to find the correct function.
+      auto it = std::upper_bound(func_vaddrs.begin(), func_vaddrs.end(),
+                                 static_cast<uint32_t>(ip_vaddr));
+      if (it != func_vaddrs.begin()) {
+        --it;
+        // Found the exidx entry.
+        size_t index = it - func_vaddrs.begin();
+        proc_info->start_ip = *it;
+        proc_info->format = UNW_INFO_FORMAT_ARM_EXIDX;
+        proc_info->unwind_info = reinterpret_cast<void*>(
+            static_cast<uintptr_t>(index * sizeof(ArmIdxEntry) +
+                                   debug_frame->arm_exidx.exidx_vaddr +
+                                   debug_frame->min_vaddr));
+        eh_frame_hdr_space_.Clear();
+        eh_frame_space_.Clear();
+        // Prepare arm_exidx space and arm_extab space.
+        arm_exidx_space_.start = debug_frame->min_vaddr + debug_frame->arm_exidx.exidx_vaddr;
+        arm_exidx_space_.end = arm_exidx_space_.start +
+            debug_frame->arm_exidx.exidx_data.size() * sizeof(ArmIdxEntry);
+        arm_exidx_space_.data = reinterpret_cast<const uint8_t*>(
+            debug_frame->arm_exidx.exidx_data.data());
+
+        arm_extab_space_.start = debug_frame->min_vaddr + debug_frame->arm_exidx.extab_vaddr;
+        arm_extab_space_.end = arm_extab_space_.start +
+            debug_frame->arm_exidx.extab_data.size();
+        arm_extab_space_.data = debug_frame->arm_exidx.extab_data.data();
+        return true;
+      }
+    }
+  }
   return false;
 }
 
@@ -560,7 +568,7 @@ DebugFrameInfo* BacktraceOffline::GetDebugFrameInFile(const std::string& filenam
   }
   DebugFrameInfo* debug_frame = ReadDebugFrameFromFile(filename);
   if (cache_file_) {
-      g_debug_frames.emplace(filename, std::unique_ptr<DebugFrameInfo>(debug_frame));
+    g_debug_frames.emplace(filename, std::unique_ptr<DebugFrameInfo>(debug_frame));
   }
   return debug_frame;
 }

libbacktrace/backtrace_offline_test.cpp

@@ -220,25 +220,7 @@ static std::string GetArch() {
 #endif
 }
 
-static void BacktraceOfflineTest(const std::string& testlib_name) {
-  const std::string arch = GetArch();
-  if (arch.empty()) {
-    GTEST_LOG_(INFO) << "This test does nothing on current arch.";
-    return;
-  }
-  const std::string offline_testdata_path = "testdata/" + arch + "/offline_testdata";
-  std::string testdata;
-  ASSERT_TRUE(android::base::ReadFileToString(offline_testdata_path, &testdata));
-
-  const std::string testlib_path = "testdata/" + arch + "/" + testlib_name;
-  struct stat st;
-  if (stat(testlib_path.c_str(), &st) == -1) {
-    GTEST_LOG_(INFO) << "This test is skipped as " << testlib_path << " doesn't exist.";
-    return;
-  }
-
-  // Parse offline_testdata.
-  std::vector<std::string> lines = android::base::Split(testdata, "\n");
+struct OfflineTestData {
   int pid;
   int tid;
   std::vector<backtrace_map_t> maps;
@@ -246,63 +228,93 @@ static void BacktraceOfflineTest(const std::string& testlib_name) {
   backtrace_stackinfo_t stack_info;
   std::vector<uint8_t> stack;
   std::vector<FunctionSymbol> symbols;
+};
+
+bool ReadOfflineTestData(const std::string offline_testdata_path, OfflineTestData* testdata) {
+  std::string s;
+  if (!android::base::ReadFileToString(offline_testdata_path, &s)) {
+    return false;
+  }
+  // Parse offline_testdata.
+  std::vector<std::string> lines = android::base::Split(s, "\n");
+  memset(&testdata->unw_context, 0, sizeof(testdata->unw_context));
   for (const auto& line : lines) {
     if (android::base::StartsWith(line, "pid:")) {
-      sscanf(line.c_str(), "pid: %d tid: %d", &pid, &tid);
+      sscanf(line.c_str(), "pid: %d tid: %d", &testdata->pid, &testdata->tid);
     } else if (android::base::StartsWith(line, "map:")) {
-      maps.resize(maps.size() + 1);
+      testdata->maps.resize(testdata->maps.size() + 1);
+      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",
-             &maps.back().start, &maps.back().end, &maps.back().offset,
-             &maps.back().load_base, &maps.back().flags, &pos);
-      maps.back().name = android::base::Trim(line.substr(pos));
+             &map.start, &map.end, &map.offset, &map.load_base, &map.flags, &pos);
+      map.name = android::base::Trim(line.substr(pos));
     } else if (android::base::StartsWith(line, "registers:")) {
       size_t size;
       int pos;
       sscanf(line.c_str(), "registers: %zu %n", &size, &pos);
-      ASSERT_EQ(sizeof(unw_context), size);
-      HexStringToRawData(&line[pos], &unw_context, size);
+      if (sizeof(testdata->unw_context) != size) {
+        return false;
+      }
+      HexStringToRawData(&line[pos], &testdata->unw_context, size);
     } else if (android::base::StartsWith(line, "stack:")) {
       size_t size;
       int pos;
       sscanf(line.c_str(),
              "stack: start: %" SCNx64 " end: %" SCNx64 " size: %zu %n",
-             &stack_info.start, &stack_info.end, &size, &pos);
-      stack.resize(size);
-      HexStringToRawData(&line[pos], &stack[0], size);
-      stack_info.data = stack.data();
+             &testdata->stack_info.start, &testdata->stack_info.end, &size, &pos);
+      testdata->stack.resize(size);
+      HexStringToRawData(&line[pos], &testdata->stack[0], size);
+      testdata->stack_info.data = testdata->stack.data();
     } else if (android::base::StartsWith(line, "function:")) {
-      symbols.resize(symbols.size() + 1);
+      testdata->symbols.resize(testdata->symbols.size() + 1);
+      FunctionSymbol& symbol = testdata->symbols.back();
       int pos;
       sscanf(line.c_str(),
              "function: start: %" SCNxPTR " end: %" SCNxPTR " name: %n",
-             &symbols.back().start, &symbols.back().end,
-             &pos);
-      symbols.back().name = line.substr(pos);
+             &symbol.start, &symbol.end, &pos);
+      symbol.name = line.substr(pos);
     }
   }
+  return true;
+}
+
+static void BacktraceOfflineTest(const std::string& testlib_name) {
+  const std::string arch = GetArch();
+  if (arch.empty()) {
+    GTEST_LOG_(INFO) << "This test does nothing on current arch.";
+    return;
+  }
+  const std::string testlib_path = "testdata/" + arch + "/" + testlib_name;
+  struct stat st;
+  if (stat(testlib_path.c_str(), &st) == -1) {
+    GTEST_LOG_(INFO) << "This test is skipped as " << testlib_path << " doesn't exist.";
+    return;
+  }
+
+  const std::string offline_testdata_path = "testdata/" + arch + "/offline_testdata";
+  OfflineTestData testdata;
+  ASSERT_TRUE(ReadOfflineTestData(offline_testdata_path, &testdata));
 
   // Fix path of libbacktrace_testlib.so.
-  for (auto& map : maps) {
+  for (auto& map : testdata.maps) {
     if (map.name.find("libbacktrace_test.so") != std::string::npos) {
       map.name = testlib_path;
     }
   }
 
   // Do offline backtrace.
-  std::unique_ptr<BacktraceMap> map(BacktraceMap::Create(pid, maps));
+  std::unique_ptr<BacktraceMap> map(BacktraceMap::Create(testdata.pid, testdata.maps));
   ASSERT_TRUE(map != nullptr);
 
   std::unique_ptr<Backtrace> backtrace(
-      Backtrace::CreateOffline(pid, tid, map.get(), stack_info));
+      Backtrace::CreateOffline(testdata.pid, testdata.tid, map.get(), testdata.stack_info));
   ASSERT_TRUE(backtrace != nullptr);
 
-  ucontext_t ucontext = GetUContextFromUnwContext(unw_context);
+  ucontext_t ucontext = GetUContextFromUnwContext(testdata.unw_context);
   ASSERT_TRUE(backtrace->Unwind(0, &ucontext));
 
-
   // Collect pc values of the call stack frames.
   std::vector<uintptr_t> pc_values;
   for (size_t i = 0; i < backtrace->NumFrames(); ++i) {
@@ -311,17 +323,20 @@ static void BacktraceOfflineTest(const std::string& testlib_name) {
 
   size_t test_one_index = 0;
   for (size_t i = 0; i < pc_values.size(); ++i) {
-    if (FunctionNameForAddress(pc_values[i], symbols) == "test_level_one") {
+    if (FunctionNameForAddress(pc_values[i], testdata.symbols) == "test_level_one") {
       test_one_index = i;
       break;
     }
   }
 
   ASSERT_GE(test_one_index, 3u);
-  ASSERT_EQ("test_level_one", FunctionNameForAddress(pc_values[test_one_index], symbols));
-  ASSERT_EQ("test_level_two", FunctionNameForAddress(pc_values[test_one_index - 1], symbols));
-  ASSERT_EQ("test_level_three", FunctionNameForAddress(pc_values[test_one_index - 2], symbols));
-  ASSERT_EQ("test_level_four", FunctionNameForAddress(pc_values[test_one_index - 3], symbols));
+  ASSERT_EQ("test_level_one", FunctionNameForAddress(pc_values[test_one_index], testdata.symbols));
+  ASSERT_EQ("test_level_two", FunctionNameForAddress(pc_values[test_one_index - 1],
+                                                     testdata.symbols));
+  ASSERT_EQ("test_level_three", FunctionNameForAddress(pc_values[test_one_index - 2],
+                                                       testdata.symbols));
+  ASSERT_EQ("test_level_four", FunctionNameForAddress(pc_values[test_one_index - 3],
+                                                      testdata.symbols));
 }
 
 TEST(libbacktrace, offline_eh_frame) {
@@ -339,3 +354,47 @@ TEST(libbacktrace, offline_gnu_debugdata) {
 TEST(libbacktrace, offline_arm_exidx) {
   BacktraceOfflineTest("libbacktrace_test_arm_exidx.so");
 }
+
+// This test tests the situation that ranges of functions covered by .eh_frame and .ARM.exidx
+// overlap with each other, which appears in /system/lib/libart.so.
+TEST(libbacktrace, offline_unwind_mix_eh_frame_and_arm_exidx) {
+  const std::string arch = GetArch();
+  if (arch.empty() || arch != "arm") {
+    GTEST_LOG_(INFO) << "This test does nothing on current arch.";
+    return;
+  }
+  const std::string testlib_path = "testdata/" + arch + "/libart.so";
+  struct stat st;
+  ASSERT_EQ(0, stat(testlib_path.c_str(), &st)) << "can't find testlib " << testlib_path;
+
+  const std::string offline_testdata_path = "testdata/" + arch + "/offline_testdata_for_libart";
+  OfflineTestData testdata;
+  ASSERT_TRUE(ReadOfflineTestData(offline_testdata_path, &testdata));
+
+  // Fix path of /system/lib/libart.so.
+  for (auto& map : testdata.maps) {
+    if (map.name.find("libart.so") != std::string::npos) {
+      map.name = testlib_path;
+    }
+  }
+
+  // Do offline backtrace.
+  std::unique_ptr<BacktraceMap> map(BacktraceMap::Create(testdata.pid, testdata.maps));
+  ASSERT_TRUE(map != nullptr);
+
+  std::unique_ptr<Backtrace> backtrace(
+      Backtrace::CreateOffline(testdata.pid, testdata.tid, map.get(), testdata.stack_info));
+  ASSERT_TRUE(backtrace != nullptr);
+
+  ucontext_t ucontext = GetUContextFromUnwContext(testdata.unw_context);
+  ASSERT_TRUE(backtrace->Unwind(0, &ucontext));
+
+  // 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;
+    std::string name = FunctionNameForAddress(vaddr_in_file, testdata.symbols);
+    ASSERT_EQ(name, testdata.symbols[i].name);
+  }
+}