Fix LLDB elf core dump register access for ARM/AArch64

[omjavaid]

Summary

This patch adds support to access AArch64 FP SIMD core dump registers and adds a test case to verify registers.

This patches fixes a bug where doing "register read --all" causes lldb to crash.

[labath]

Thanks for doing this. The patch is pretty straight-forward. I'd just like to get some clarification about the defensive read register checks.

lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm.cpp

46

Other elf core register contexts don't have this check. Why is it necessary here? Is that the register read --all bug ? If it is then isn't the proper solution to ensure we don't call this with a null register info ?

lldb/test/API/functionalities/postmortem/elf-core/aarch64-neon.c

15–57

I think it would be simpler, more obvious, and generate a smaller core file if you just used the inline assembly necessary to populate the relevant registers with necessary values (similar to the test/Shell/Register/* tests).

[omjavaid]

Updated diff after implementing core file using assembly code. Also removed the redundant register info is null condition.

LGTM?

[labath]

Thanks. LGTM, assuming the checked-in core files are of a "reasonable" (~ several tens of KB) size.

lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp

51

I guess all of these conditions should really be offset + reg_info->byte_size <= GetGPRSize()

lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm.cpp

 
 bool RegisterContextCorePOSIX_arm::WriteFPR() {
   assert(0);
   return false;
 }
 
 bool RegisterContextCorePOSIX_arm::ReadRegister(const RegisterInfo *reg_info,
                                                 RegisterValue &value) {
   lldb::offset_t offset = reg_info->byte_offset;
+  if (offset < GetGPRSize() && offset < m_gpr.GetByteSize()) {
     uint64_t v = m_gpr.GetMaxU64(&offset, reg_info->byte_size);
     if (offset == reg_info->byte_offset + reg_info->byte_size) {
       value = v;
       return true;
     }
+  }
   return false;
 }
 
 bool RegisterContextCorePOSIX_arm::ReadAllRegisterValues(
     lldb::DataBufferSP &data_sp) {
   return false;
 }
 

lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h

protected:
 
   bool WriteGPR() override;
 
   bool WriteFPR() override;
 
 private:
   lldb::DataBufferSP m_gpr_buffer;
   lldb_private::DataExtractor m_gpr;
+  lldb_private::DataExtractor m_fpregset;
 };
 
 #endif // LLDB_SOURCE_PLUGINS_PROCESS_ELF_CORE_REGISTERCONTEXTPOSIXCORE_ARM64_H

lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp

 RegisterContextCorePOSIX_arm64::RegisterContextCorePOSIX_arm64(
     Thread &thread, RegisterInfoInterface *register_info,
     const DataExtractor &gpregset, llvm::ArrayRef<CoreNote> notes)
     : RegisterContextPOSIX_arm64(thread, 0, register_info) {
   m_gpr_buffer = std::make_shared<DataBufferHeap>(gpregset.GetDataStart(),
                                                   gpregset.GetByteSize());
   m_gpr.SetData(m_gpr_buffer);
   m_gpr.SetByteOrder(gpregset.GetByteOrder());
+
+  m_fpregset = getRegset(
+      notes, register_info->GetTargetArchitecture().GetTriple(), FPR_Desc);
 }
 
 RegisterContextCorePOSIX_arm64::~RegisterContextCorePOSIX_arm64() {}
 
 bool RegisterContextCorePOSIX_arm64::ReadGPR() { return true; }
 
 bool RegisterContextCorePOSIX_arm64::ReadFPR() { return false; }
 
 bool RegisterContextCorePOSIX_arm64::WriteGPR() {
   assert(0);
   return false;
 }
 
 bool RegisterContextCorePOSIX_arm64::WriteFPR() {
   assert(0);
   return false;
 }
 
 bool RegisterContextCorePOSIX_arm64::ReadRegister(const RegisterInfo *reg_info,
                                                   RegisterValue &value) {
   lldb::offset_t offset = reg_info->byte_offset;
+  if (offset < GetGPRSize() && offset < m_gpr.GetByteSize()) {
     uint64_t v = m_gpr.GetMaxU64(&offset, reg_info->byte_size);
     if (offset == reg_info->byte_offset + reg_info->byte_size) {
       value = v;
       return true;
     }
+  }
+
+  const uint32_t reg = reg_info->kinds[lldb::eRegisterKindLLDB];
+  if (reg == LLDB_INVALID_REGNUM)
+    return false;
+
+  offset -= GetGPRSize();
+  if (IsFPR(reg) && offset < m_fpregset.GetByteSize()) {
+    Status error;
+    value.SetFromMemoryData(reg_info, m_fpregset.GetDataStart() + offset,
+                            reg_info->byte_size, lldb::eByteOrderLittle, error);
+    return error.Success();
+  }
+
   return false;
 }
 
 bool RegisterContextCorePOSIX_arm64::ReadAllRegisterValues(
     lldb::DataBufferSP &data_sp) {
   return false;
 }
 

lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py

 from lldbsuite.test import lldbutil
 
 
 class LinuxCoreTestCase(TestBase):
     NO_DEBUG_INFO_TESTCASE = True
 
     mydir = TestBase.compute_mydir(__file__)
 
+    _aarch64_pid = 37688
     _i386_pid = 32306
     _x86_64_pid = 32259
     _s390x_pid = 1045
     _mips64_n64_pid = 25619
     _mips64_n32_pid = 3670
     _mips_o32_pid = 3532
     _ppc64le_pid = 28147
 
+    _aarch64_regions = 4
     _i386_regions = 4
     _x86_64_regions = 5
     _s390x_regions = 2
     _mips_regions = 5
     _ppc64le_regions = 2
 
+
+    @skipIf(triple='^mips')
+    @skipIfLLVMTargetMissing("AArch64")
+    def test_aarch64(self):
+        """Test that lldb can read the process information from an aarch64 linux core file."""
+        self.do_test("linux-aarch64", self._aarch64_pid, self._aarch64_regions, "a.out")
+
     @skipIf(triple='^mips')
     @skipIfLLVMTargetMissing("X86")
     def test_i386(self):
         """Test that lldb can read the process information from an i386 linux core file."""
         self.do_test("linux-i386", self._i386_pid, self._i386_regions, "a.out")
 
     @skipIfLLVMTargetMissing("Mips")
     def test_mips_o32(self):
def test_x86_64_sysroot(self):
         # Check that we found executable from the sysroot
         mod_path = str(target.GetModuleAtIndex(0).GetFileSpec())
         self.assertEqual(mod_path, exe_inside)
         self.check_all(process, self._x86_64_pid, self._x86_64_regions, "a.out")
 
         self.dbg.DeleteTarget(target)
 
     @skipIf(triple='^mips')
+    @skipIfLLVMTargetMissing("AArch64")
+    def test_aarch64_regs(self):
+        # check 64 bit ARM core files
+        target = self.dbg.CreateTarget(None)
+        self.assertTrue(target, VALID_TARGET)
+        process = target.LoadCore("linux-aarch64-neon.core")
+
+        values = {}
+        values["x1"] = "0x000000000000002f"
+        values["w1"] = "0x0000002f"
+        values["fp"] = "0x0000007fc5dd7f20"
+        values["lr"] = "0x0000000000400180"
+        values["sp"] = "0x0000007fc5dd7f00"
+        values["pc"] = "0x000000000040014c"
+        values["v0"] = "{0x00 0x00 0x00 0x00 0x00 0x00 0xe0 0x3f 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}"
+        values["v1"] = "{0x00 0x00 0x00 0x00 0x00 0x00 0xf8 0x3f 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}"
+        values["v2"] = "{0x00 0x00 0x00 0x00 0x00 0x00 0x04 0x40 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}"
+        values["v3"] = "{0x00 0x00 0x00 0x00 0x00 0x00 0x0c 0x40 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}"
+        values["v4"] = "{0x00 0x00 0x90 0x40 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}"
+        values["v5"] = "{0x00 0x00 0xb0 0x40 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}"
+        values["v6"] = "{0x00 0x00 0xd0 0x40 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}"
+        values["v7"] = "{0x00 0x00 0xf0 0x40 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}"
+        values["v8"] = "{0x11 0x11 0x11 0x11 0x11 0x11 0x11 0x11 0x11 0x11 0x11 0x11 0x11 0x11 0x11 0x11}"
+        values["v27"] = "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}"
+        values["v28"] = "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}"
+        values["v31"] = "{0x30 0x30 0x30 0x30 0x30 0x30 0x30 0x30 0x30 0x30 0x30 0x30 0x30 0x30 0x30 0x30}"
+        values["s2"] = "0"
+        values["s3"] = "0"
+        values["s4"] = "4.5"
+        values["s5"] = "5.5"
+        values["s6"] = "6.5"
+        values["s7"] = "7.5"
+        values["s8"] = "1.14437e-28"
+        values["s30"] = "0"
+        values["s31"] = "6.40969e-10"
+        values["d0"] = "0.5"
+        values["d1"] = "1.5"
+        values["d2"] = "2.5"
+        values["d3"] = "3.5"
+        values["d4"] = "5.35161536149201e-315"
+        values["d5"] = "5.36197666906508e-315"
+        values["d6"] = "5.37233797663815e-315"
+        values["d7"] = "5.38269928421123e-315"
+        values["d8"] = "1.80107573659442e-226"
+        values["d30"] = "0"
+        values["d31"] = "1.39804328609529e-76"
+        values["fpsr"] = "0x00000000"
+        values["fpcr"] = "0x00000000"
+
+        for regname, value in values.items():
+            self.expect("register read {}".format(regname), substrs=["{} = {}".format(regname, value)])
+
+        self.expect("register read --all")
+
+    @skipIf(triple='^mips')
     @skipIfLLVMTargetMissing("ARM")
     def test_arm_core(self):
         # check 32 bit ARM core file
         target = self.dbg.CreateTarget(None)
         self.assertTrue(target, VALID_TARGET)
         process = target.LoadCore("linux-arm.core")
 
         values = {}
def test_arm_core(self):
         values["r12"] = "0x0000000c"
         values["sp"] = "0x0000000d"
         values["lr"] = "0x0000000e"
         values["pc"] = "0x0000000f"
         values["cpsr"] = "0x00000010"
         for regname, value in values.items():
             self.expect("register read {}".format(regname), substrs=["{} = {}".format(regname, value)])
 
+        self.expect("register read --all")
+
     def check_memory_regions(self, process, region_count):
         region_list = process.GetMemoryRegions()
         self.assertEqual(region_list.GetSize(), region_count)
 
         region = lldb.SBMemoryRegionInfo()
 
         # Check we have the right number of regions.
         self.assertEqual(region_list.GetSize(), region_count)

lldb/test/API/functionalities/postmortem/elf-core/aarch64-neon.c

+// compile with -march=armv8-a+sve on compatible aarch64 compiler
+// linux-aarch64-sve.core was generated by: aarch64-linux-gnu-gcc-8
+// commandline: -march=armv8-a+sve -nostdlib -static -g linux-aarch64-sve.c
+static void bar(char *boom) {
+  char F = 'b';
+  asm volatile("fmov     d0,  #0.5\n\t");
+  asm volatile("fmov     d1,  #1.5\n\t");
+  asm volatile("fmov     d2,  #2.5\n\t");
+  asm volatile("fmov     d3,  #3.5\n\t");
+  asm volatile("fmov     s4,  #4.5\n\t");
+  asm volatile("fmov     s5,  #5.5\n\t");
+  asm volatile("fmov     s6,  #6.5\n\t");
+  asm volatile("fmov     s7,  #7.5\n\t");
+  asm volatile("movi     v8.16b, #0x11\n\t");
+  asm volatile("movi     v31.16b, #0x30\n\t");
+
+  *boom = 47; // Frame bar
+}
+
+static void foo(char *boom, void (*boomer)(char *)) {
+  char F = 'f';
+  boomer(boom); // Frame foo
+}
+
+void _start(void) {
+  char F = '_';
+  foo(0, bar); // Frame _start
+}