[Lldb-commits] [lldb] e94c609 - [lldb][Mach-O] Fix several bugs in x86_64 Mach-O corefile (#146460)

[via lldb-commits]

Author: Jason Molenda
Date: 2025-06-30T21:27:53-07:00
New Revision: e94c6091c93a72e9b81cf8d6394495d043b409d4

URL: https://github.com/llvm/llvm-project/commit/e94c6091c93a72e9b81cf8d6394495d043b409d4
DIFF: https://github.com/llvm/llvm-project/commit/e94c6091c93a72e9b81cf8d6394495d043b409d4.diff

LOG: [lldb][Mach-O] Fix several bugs in x86_64 Mach-O corefile (#146460)

reading, and one bug in the new RegisterContextUnifiedCore class.

The PR I landed a few days ago to allow Mach-O corefiles to augment
their registers with additional per-thread registers in metadata exposed
a few bugs in the x86_64 corefile reader when running under different CI
environments. It also showed a bug in my RegisterContextUnifiedCore
class where I wasn't properly handling lookups of unknown registers
(e.g. the LLDB_GENERIC_RA when debugging an intel target).

The Mach-O x86_64 corefile support would say that it had fpu & exc
registers available in every corefile, regardless of whether they were
actually present. It would only read the bytes for the first register
flavor in the LC_THREAD, the GPRs, but it read them incorrectly, so
sometimes you got more register context than you'd expect. The LC_THREAD
register context specifies a flavor and the number of uint32_t words;
the ObjectFileMachO method would read that number of uint64_t's,
exceeding the GPR register space, but it was followed by FPU and then
EXC register space so it didn't crash. If you had a corefile with GPR
and EXC register bytes, it would be written into the GPR and then FPU
register areas, with zeroes filling out the rest of the context.

Added: 
    

Modified: 
    lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
    lldb/source/Plugins/Process/mach-core/RegisterContextUnifiedCore.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index 4394caf7f31e9..70f954cd5413f 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -184,46 +184,32 @@ class RegisterContextDarwin_x86_64_Mach : public RegisterContextDarwin_x86_64 {
     SetError(GPRRegSet, Read, -1);
     SetError(FPURegSet, Read, -1);
     SetError(EXCRegSet, Read, -1);
-    bool done = false;
 
-    while (!done) {
+    while (offset < data.GetByteSize()) {
       int flavor = data.GetU32(&offset);
       if (flavor == 0)
-        done = true;
-      else {
-        uint32_t i;
-        uint32_t count = data.GetU32(&offset);
-        switch (flavor) {
-        case GPRRegSet:
-          for (i = 0; i < count; ++i)
-            (&gpr.rax)[i] = data.GetU64(&offset);
-          SetError(GPRRegSet, Read, 0);
-          done = true;
-
-          break;
-        case FPURegSet:
-          // TODO: fill in FPU regs....
-          // SetError (FPURegSet, Read, -1);
-          done = true;
-
-          break;
-        case EXCRegSet:
-          exc.trapno = data.GetU32(&offset);
-          exc.err = data.GetU32(&offset);
-          exc.faultvaddr = data.GetU64(&offset);
-          SetError(EXCRegSet, Read, 0);
-          done = true;
-          break;
-        case 7:
-        case 8:
-        case 9:
-          // fancy flavors that encapsulate of the above flavors...
-          break;
-
-        default:
-          done = true;
-          break;
-        }
+        break;
+      uint32_t count = data.GetU32(&offset);
+      switch (flavor) {
+      case GPRRegSet: {
+        uint32_t *gpr_data = reinterpret_cast<uint32_t *>(&gpr.rax);
+        for (uint32_t i = 0; i < count && offset < data.GetByteSize(); ++i)
+          gpr_data[i] = data.GetU32(&offset);
+        SetError(GPRRegSet, Read, 0);
+      } break;
+      case FPURegSet:
+        // TODO: fill in FPU regs....
+        SetError(FPURegSet, Read, -1);
+        break;
+      case EXCRegSet:
+        exc.trapno = data.GetU32(&offset);
+        exc.err = data.GetU32(&offset);
+        exc.faultvaddr = data.GetU64(&offset);
+        SetError(EXCRegSet, Read, 0);
+        break;
+      default:
+        offset += count * 4;
+        break;
       }
     }
   }
@@ -353,11 +339,11 @@ class RegisterContextDarwin_x86_64_Mach : public RegisterContextDarwin_x86_64 {
   }
 
 protected:
-  int DoReadGPR(lldb::tid_t tid, int flavor, GPR &gpr) override { return 0; }
+  int DoReadGPR(lldb::tid_t tid, int flavor, GPR &gpr) override { return -1; }
 
-  int DoReadFPU(lldb::tid_t tid, int flavor, FPU &fpu) override { return 0; }
+  int DoReadFPU(lldb::tid_t tid, int flavor, FPU &fpu) override { return -1; }
 
-  int DoReadEXC(lldb::tid_t tid, int flavor, EXC &exc) override { return 0; }
+  int DoReadEXC(lldb::tid_t tid, int flavor, EXC &exc) override { return -1; }
 
   int DoWriteGPR(lldb::tid_t tid, int flavor, const GPR &gpr) override {
     return 0;

diff  --git a/lldb/source/Plugins/Process/mach-core/RegisterContextUnifiedCore.cpp b/lldb/source/Plugins/Process/mach-core/RegisterContextUnifiedCore.cpp
index 16568f788c202..449f85bc3f52a 100644
--- a/lldb/source/Plugins/Process/mach-core/RegisterContextUnifiedCore.cpp
+++ b/lldb/source/Plugins/Process/mach-core/RegisterContextUnifiedCore.cpp
@@ -262,7 +262,9 @@ size_t RegisterContextUnifiedCore::GetRegisterCount() {
 
 const RegisterInfo *
 RegisterContextUnifiedCore::GetRegisterInfoAtIndex(size_t reg) {
-  return &m_register_infos[reg];
+  if (reg < m_register_infos.size())
+    return &m_register_infos[reg];
+  return nullptr;
 }
 
 size_t RegisterContextUnifiedCore::GetRegisterSetCount() {
@@ -270,7 +272,9 @@ size_t RegisterContextUnifiedCore::GetRegisterSetCount() {
 }
 
 const RegisterSet *RegisterContextUnifiedCore::GetRegisterSet(size_t set) {
-  return &m_register_sets[set];
+  if (set < m_register_sets.size())
+    return &m_register_sets[set];
+  return nullptr;
 }
 
 bool RegisterContextUnifiedCore::ReadRegister(