Set eRegisterKindEHFrame register numbers for 32 bit ARM register contexts in minidumps

Stack unwinding was sometimes failing when trying to unwind stacks in 32 bit ARM. I discovered this was because the EH frame register numbers were not set. This patch fixes this issue and adds a unit test to verify this doesn't regress.

Differential Revision: https://reviews.llvm.org/D68088

llvm-svn: 374246

diff --git a/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_ARM.cpp b/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_ARM.cpp
index f2e456097dfcc228de4e37e5e9072743520db082..72dead07dcb481cd94389182ab7353a974622881 100644 (file)
--- a/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_ARM.cpp
+++ b/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_ARM.cpp
@@ -9,6 +9,7 @@
 #include "RegisterContextMinidump_ARM.h"
 
 #include "Utility/ARM_DWARF_Registers.h"
+#include "Utility/ARM_ehframe_Registers.h"
 #include "lldb/Utility/RegisterValue.h"
 #include "lldb/Utility/DataExtractor.h"
 #include "lldb/Utility/LLDBAssert.h"
@@ -29,14 +30,14 @@ using namespace minidump;
 #define DEF_R(i)                                                               \
   {                                                                            \
     "r" #i, nullptr, 4, OFFSET(r) + i * 4, eEncodingUint, eFormatHex,          \
-        {dwarf_r##i, dwarf_r##i, INV, INV, reg_r##i},                          \
+        {ehframe_r##i, dwarf_r##i, INV, INV, reg_r##i},                          \
         nullptr, nullptr, nullptr, 0    \
   }
 
 #define DEF_R_ARG(i, n)                                                        \
   {                                                                            \
     "r" #i, "arg" #n, 4, OFFSET(r) + i * 4, eEncodingUint, eFormatHex,         \
-        {dwarf_r##i, dwarf_r##i, LLDB_REGNUM_GENERIC_ARG1 + i, INV, reg_r##i}, \
+        {ehframe_r##i, dwarf_r##i, LLDB_REGNUM_GENERIC_ARG1 + i, INV, reg_r##i}, \
         nullptr, nullptr, nullptr, 0                                           \
   }
 
@@ -173,7 +174,7 @@ static RegisterInfo g_reg_info_apple_fp = {
     OFFSET(r) + 7 * 4,
     eEncodingUint,
     eFormatHex,
-    {INV, dwarf_r7, LLDB_REGNUM_GENERIC_FP, INV, reg_r7},
+    {ehframe_r7, dwarf_r7, LLDB_REGNUM_GENERIC_FP, INV, reg_r7},
     nullptr,
     nullptr,
     nullptr,
@@ -186,7 +187,7 @@ static RegisterInfo g_reg_info_fp = {
     OFFSET(r) + 11 * 4,
     eEncodingUint,
     eFormatHex,
-    {INV, dwarf_r11, LLDB_REGNUM_GENERIC_FP, INV, reg_r11},
+    {ehframe_r11, dwarf_r11, LLDB_REGNUM_GENERIC_FP, INV, reg_r11},
     nullptr,
     nullptr,
     nullptr,
@@ -213,7 +214,7 @@ static RegisterInfo g_reg_infos[] = {
      OFFSET(r) + 13 * 4,
      eEncodingUint,
      eFormatHex,
-     {INV, dwarf_sp, LLDB_REGNUM_GENERIC_SP, INV, reg_sp},
+     {ehframe_sp, dwarf_sp, LLDB_REGNUM_GENERIC_SP, INV, reg_sp},
      nullptr,
      nullptr,
      nullptr,
@@ -224,7 +225,7 @@ static RegisterInfo g_reg_infos[] = {
      OFFSET(r) + 14 * 4,
      eEncodingUint,
      eFormatHex,
-     {INV, dwarf_lr, LLDB_REGNUM_GENERIC_RA, INV, reg_lr},
+     {ehframe_lr, dwarf_lr, LLDB_REGNUM_GENERIC_RA, INV, reg_lr},
      nullptr,
      nullptr,
      nullptr,
@@ -235,7 +236,7 @@ static RegisterInfo g_reg_infos[] = {
      OFFSET(r) + 15 * 4,
      eEncodingUint,
      eFormatHex,
-     {INV, dwarf_pc, LLDB_REGNUM_GENERIC_PC, INV, reg_pc},
+     {ehframe_pc, dwarf_pc, LLDB_REGNUM_GENERIC_PC, INV, reg_pc},
      nullptr,
      nullptr,
      nullptr,
@@ -246,7 +247,7 @@ static RegisterInfo g_reg_infos[] = {
      OFFSET(cpsr),
      eEncodingUint,
      eFormatHex,
-     {INV, dwarf_cpsr, LLDB_REGNUM_GENERIC_FLAGS, INV, reg_cpsr},
+     {ehframe_cpsr, dwarf_cpsr, LLDB_REGNUM_GENERIC_FLAGS, INV, reg_cpsr},
      nullptr,
      nullptr,
      nullptr,
@@ -476,12 +477,22 @@ RegisterContextMinidump_ARM::RegisterContextMinidump_ARM(
   lldbassert(k_num_regs == k_num_reg_infos);
 }
 
-size_t RegisterContextMinidump_ARM::GetRegisterCount() { return k_num_regs; }
+size_t RegisterContextMinidump_ARM::GetRegisterCountStatic() {
+  return k_num_regs;
+}
+
+// Used for unit testing so we can verify register info is filled in for
+// all register flavors (DWARF, EH Frame, generic, etc).
+size_t RegisterContextMinidump_ARM::GetRegisterCount() {
+  return GetRegisterCountStatic();
+}
 
+// Used for unit testing so we can verify register info is filled in.
 const RegisterInfo *
-RegisterContextMinidump_ARM::GetRegisterInfoAtIndex(size_t reg) {
+RegisterContextMinidump_ARM::GetRegisterInfoAtIndexStatic(size_t reg,
+                                                          bool apple) {
   if (reg < k_num_reg_infos) {
-    if (m_apple) {
+    if (apple) {
       if (reg == reg_r7)
         return &g_reg_info_apple_fp;
     } else {
@@ -493,6 +504,11 @@ RegisterContextMinidump_ARM::GetRegisterInfoAtIndex(size_t reg) {
   return nullptr;
 }
 
+const RegisterInfo *
+RegisterContextMinidump_ARM::GetRegisterInfoAtIndex(size_t reg) {
+  return GetRegisterInfoAtIndexStatic(reg, m_apple);
+}
+
 size_t RegisterContextMinidump_ARM::GetRegisterSetCount() {
   return k_num_reg_sets;
 }

diff --git a/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_ARM.h b/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_ARM.h
index eff8cdfef00a00662b533d12f177b39e51752f7a..7af3b98a6fe7b77380dafd2a61ce25604958043f 100644 (file)
--- a/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_ARM.h
+++ b/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_ARM.h
@@ -38,6 +38,12 @@ public:
     // Do nothing... registers are always valid...
   }
 
+  // Used for unit testing.
+  static size_t GetRegisterCountStatic();
+  // Used for unit testing.
+  static const lldb_private::RegisterInfo *
+  GetRegisterInfoAtIndexStatic(size_t reg, bool apple);
+
   size_t GetRegisterCount() override;
 
   const lldb_private::RegisterInfo *GetRegisterInfoAtIndex(size_t reg) override;

diff --git a/lldb/unittests/Process/minidump/RegisterContextMinidumpTest.cpp b/lldb/unittests/Process/minidump/RegisterContextMinidumpTest.cpp
index 3d0f628f80f19e4e46eaab320970d8a3c5b47b96..f3cc8d6f76ad12f6a13d9ff002510294d704c8b9 100644 (file)
--- a/lldb/unittests/Process/minidump/RegisterContextMinidumpTest.cpp
+++ b/lldb/unittests/Process/minidump/RegisterContextMinidumpTest.cpp
@@ -10,7 +10,9 @@
 #include "Plugins/Process/Utility/RegisterContextLinux_x86_64.h"
 #include "Plugins/Process/minidump/RegisterContextMinidump_x86_32.h"
 #include "Plugins/Process/minidump/RegisterContextMinidump_x86_64.h"
+#include "Plugins/Process/minidump/RegisterContextMinidump_ARM.h"
 #include "lldb/Utility/DataBuffer.h"
+#include "llvm/ADT/StringRef.h"
 #include "gtest/gtest.h"
 
 using namespace lldb_private;
@@ -143,3 +145,56 @@ TEST(RegisterContextMinidump, ConvertMinidumpContext_x86_64) {
   EXPECT_EQ(Context.ds, reg64(*Buf, Info[lldb_ds_x86_64]));
   EXPECT_EQ(Context.es, reg64(*Buf, Info[lldb_es_x86_64]));
 }
+
+static void TestARMRegInfo(const lldb_private::RegisterInfo *info) {
+  // Make sure we have valid register numbers for eRegisterKindEHFrame and
+  // eRegisterKindDWARF for GPR registers r0-r15 so that we can unwind
+  // correctly when using this information.
+  llvm::StringRef name(info->name);
+  llvm::StringRef alt_name(info->alt_name);
+  if (name.startswith("r") || alt_name.startswith("r")) {
+    EXPECT_NE(info->kinds[lldb::eRegisterKindEHFrame], LLDB_INVALID_REGNUM);
+    EXPECT_NE(info->kinds[lldb::eRegisterKindDWARF], LLDB_INVALID_REGNUM);
+  }
+  // Verify generic register are set correctly
+  if (name == "r0")
+    EXPECT_EQ(info->kinds[lldb::eRegisterKindGeneric],
+              (uint32_t)LLDB_REGNUM_GENERIC_ARG1);
+  else if (name == "r1")
+    EXPECT_EQ(info->kinds[lldb::eRegisterKindGeneric],
+              (uint32_t)LLDB_REGNUM_GENERIC_ARG2);
+  else if (name == "r2")
+    EXPECT_EQ(info->kinds[lldb::eRegisterKindGeneric],
+              (uint32_t)LLDB_REGNUM_GENERIC_ARG3);
+  else if (name == "r3")
+    EXPECT_EQ(info->kinds[lldb::eRegisterKindGeneric],
+              (uint32_t)LLDB_REGNUM_GENERIC_ARG4);
+  else if (name == "sp")
+    EXPECT_EQ(info->kinds[lldb::eRegisterKindGeneric],
+              (uint32_t)LLDB_REGNUM_GENERIC_SP);
+  else if (name == "fp")
+    EXPECT_EQ(info->kinds[lldb::eRegisterKindGeneric],
+              (uint32_t)LLDB_REGNUM_GENERIC_FP);
+  else if (name == "lr")
+    EXPECT_EQ(info->kinds[lldb::eRegisterKindGeneric],
+              (uint32_t)LLDB_REGNUM_GENERIC_RA);
+  else if (name == "pc")
+    EXPECT_EQ(info->kinds[lldb::eRegisterKindGeneric],
+              (uint32_t)LLDB_REGNUM_GENERIC_PC);
+  else if (name == "cpsr")
+    EXPECT_EQ(info->kinds[lldb::eRegisterKindGeneric],
+              (uint32_t)LLDB_REGNUM_GENERIC_FLAGS);
+}
+
+TEST(RegisterContextMinidump, CheckRegisterContextMinidump_ARM) {
+  size_t num_regs = RegisterContextMinidump_ARM::GetRegisterCountStatic();
+  const lldb_private::RegisterInfo *reg_info;
+  for (size_t reg=0; reg<num_regs; ++reg) {
+    reg_info = RegisterContextMinidump_ARM::GetRegisterInfoAtIndexStatic(reg,
+                                                                         true);
+    TestARMRegInfo(reg_info);
+    reg_info = RegisterContextMinidump_ARM::GetRegisterInfoAtIndexStatic(reg,
+                                                                         false);
+    TestARMRegInfo(reg_info);
+  }
+}