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

[clayborg]

Summary

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.

lldb/source/Plugins/Process/minidump/RegisterContextMinidump_ARM.h

RegisterContextMinidump_ARM(lldb_private::Thread &thread,
                               const DataExtractor &data, bool apple);
 
   ~RegisterContextMinidump_ARM() override = default;
 
   void InvalidateAllRegisters() override {
     // 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;
 
   size_t GetRegisterSetCount() override;
 
   const lldb_private::RegisterSet *GetRegisterSet(size_t set) override;
 

lldb/source/Plugins/Process/minidump/RegisterContextMinidump_ARM.cpp

 //===-- RegisterContextMinidump_ARM.cpp -------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #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"
 #include "lldb/lldb-enumerations.h"
 
 // C includes
 #include <assert.h>
 
 // C++ includes
 
 using namespace lldb;
 using namespace lldb_private;
 using namespace minidump;
 
 #define INV LLDB_INVALID_REGNUM
 #define OFFSET(r) (offsetof(RegisterContextMinidump_ARM::Context, r))
 
 #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                                           \
   }
 
 #define DEF_D(i)                                                               \
   {                                                                            \
     "d" #i, nullptr, 8, OFFSET(d) + i * 8, eEncodingVector,                    \
         eFormatVectorOfUInt8, {dwarf_d##i, dwarf_d##i, INV, INV, reg_d##i},    \
         nullptr, nullptr, nullptr, 0    \

 
 static RegisterInfo g_reg_info_apple_fp = {
     "fp",
     "r7",
     4,
     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,
     0};
 
 static RegisterInfo g_reg_info_fp = {
     "fp",
     "r11",
     4,
     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,
     0};
 
 // Register info definitions for this register context
 static RegisterInfo g_reg_infos[] = {
     DEF_R_ARG(0, 1),
static RegisterInfo g_reg_infos[] = {
     DEF_R(11),
     DEF_R(12),
     {"sp",
      "r13",
      4,
      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,
      0},
     {"lr",
      "r14",
      4,
      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,
      0},
     {"pc",
      "r15",
      4,
      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,
      0},
     {"cpsr",
      "psr",
      4,
      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,
      0},
     {"fpscr",
      nullptr,
      8,
      OFFSET(fpscr),
for (unsigned i = 0; i < llvm::array_lengthof(m_regs.r); ++i)
     m_regs.r[i] = data.GetU32(&offset);
   m_regs.cpsr = data.GetU32(&offset);
   m_regs.fpscr = data.GetU64(&offset);
   for (unsigned i = 0; i < llvm::array_lengthof(m_regs.d); ++i)
     m_regs.d[i] = data.GetU64(&offset);
   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 {
       if (reg == reg_r11)
         return &g_reg_info_fp;
     }
     return &g_reg_infos[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;
 }
 
 const RegisterSet *RegisterContextMinidump_ARM::GetRegisterSet(size_t set) {
   if (set < k_num_reg_sets)
     return &g_reg_sets[set];
   return nullptr;

lldb/unittests/Process/minidump/RegisterContextMinidumpTest.cpp

 //===-- RegisterContextMinidumpTest.cpp -------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "Plugins/Process/Utility/RegisterContextLinux_i386.h"
 #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;
 using namespace lldb_private::minidump;
 
 static uint32_t reg32(const DataBuffer &Buf, const RegisterInfo &Info) {
   return *reinterpret_cast<const uint32_t *>(Buf.GetBytes() + Info.byte_offset);
 }
TEST(RegisterContextMinidump, ConvertMinidumpContext_x86_64) {
   EXPECT_EQ(Context.eflags, reg64(*Buf, Info[lldb_rflags_x86_64]));
   EXPECT_EQ(Context.cs, reg64(*Buf, Info[lldb_cs_x86_64]));
   EXPECT_EQ(Context.fs, reg64(*Buf, Info[lldb_fs_x86_64]));
   EXPECT_EQ(Context.gs, reg64(*Buf, Info[lldb_gs_x86_64]));
   EXPECT_EQ(Context.ss, reg64(*Buf, Info[lldb_ss_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);
+  }
+}