[lldb][NFC][MachO] Clean up LC_THREAD reading code, remove i386 corefile (#146480)

While fixing bugs in the x86_64 LC_THREAD parser in ObjectFileMachO, I
noticed that the other LC_THREAD parsers are all less clear than they
should be.

To recap, a Mach-O LC_THREAD load command has a byte size for the entire
payload. Within the payload, there will be one or more register sets
provided. A register set starts with a UInt32 "flavor", the type of
register set defined in the system headers, and a UInt32 "count", the
number of UInt32 words of memory for this register set. After one
register set, there may be additional sets. A parser can skip an unknown
register set flavor by using the count field to get to the next register
set. When the total byte size of the LC_THREAD load command has been
parsed, it is completed.

This patch fixes the riscv/arm/arm64 LC_THREAD parsers to use the total
byte size as the exit condition, and to skip past unrecognized register
sets, instead of stopping parsing.

Instead of fixing the i386 corefile support, I removed it. The last
macOS that supported 32-bit Intel code was macOS 10.14 in 2018. I also
removed i386 KDP support, 32-bit intel kernel debugging hasn't been
supported for even longer than that.

It would be preferable to do these things separately, but I couldn't
bring myself to update the i386 LC_THREAD parser, and it required very
few changes to remove this support entirely.

Author: jasonmolenda

lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp

@@ -11,7 +11,6 @@
 
 #include "Plugins/Process/Utility/RegisterContextDarwin_arm.h"
 #include "Plugins/Process/Utility/RegisterContextDarwin_arm64.h"
-#include "Plugins/Process/Utility/RegisterContextDarwin_i386.h"
 #include "Plugins/Process/Utility/RegisterContextDarwin_riscv32.h"
 #include "Plugins/Process/Utility/RegisterContextDarwin_x86_64.h"
 #include "lldb/Core/Debugger.h"
@@ -81,9 +80,6 @@
 #ifdef CPU_TYPE_ARM64_32
 #undef CPU_TYPE_ARM64_32
 #endif
-#ifdef CPU_TYPE_I386
-#undef CPU_TYPE_I386
-#endif
 #ifdef CPU_TYPE_X86_64
 #undef CPU_TYPE_X86_64
 #endif
@@ -358,122 +354,6 @@ class RegisterContextDarwin_x86_64_Mach : public RegisterContextDarwin_x86_64 {
   }
 };
 
-class RegisterContextDarwin_i386_Mach : public RegisterContextDarwin_i386 {
-public:
-  RegisterContextDarwin_i386_Mach(lldb_private::Thread &thread,
-                                  const DataExtractor &data)
-      : RegisterContextDarwin_i386(thread, 0) {
-    SetRegisterDataFrom_LC_THREAD(data);
-  }
-
-  void InvalidateAllRegisters() override {
-    // Do nothing... registers are always valid...
-  }
-
-  void SetRegisterDataFrom_LC_THREAD(const DataExtractor &data) {
-    lldb::offset_t offset = 0;
-    SetError(GPRRegSet, Read, -1);
-    SetError(FPURegSet, Read, -1);
-    SetError(EXCRegSet, Read, -1);
-    bool done = false;
-
-    while (!done) {
-      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.eax)[i] = data.GetU32(&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.GetU32(&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;
-        }
-      }
-    }
-  }
-
-  static bool Create_LC_THREAD(Thread *thread, Stream &data) {
-    RegisterContextSP reg_ctx_sp(thread->GetRegisterContext());
-    if (reg_ctx_sp) {
-      RegisterContext *reg_ctx = reg_ctx_sp.get();
-
-      data.PutHex32(GPRRegSet); // Flavor
-      data.PutHex32(GPRWordCount);
-      PrintRegisterValue(reg_ctx, "eax", nullptr, 4, data);
-      PrintRegisterValue(reg_ctx, "ebx", nullptr, 4, data);
-      PrintRegisterValue(reg_ctx, "ecx", nullptr, 4, data);
-      PrintRegisterValue(reg_ctx, "edx", nullptr, 4, data);
-      PrintRegisterValue(reg_ctx, "edi", nullptr, 4, data);
-      PrintRegisterValue(reg_ctx, "esi", nullptr, 4, data);
-      PrintRegisterValue(reg_ctx, "ebp", nullptr, 4, data);
-      PrintRegisterValue(reg_ctx, "esp", nullptr, 4, data);
-      PrintRegisterValue(reg_ctx, "ss", nullptr, 4, data);
-      PrintRegisterValue(reg_ctx, "eflags", nullptr, 4, data);
-      PrintRegisterValue(reg_ctx, "eip", nullptr, 4, data);
-      PrintRegisterValue(reg_ctx, "cs", nullptr, 4, data);
-      PrintRegisterValue(reg_ctx, "ds", nullptr, 4, data);
-      PrintRegisterValue(reg_ctx, "es", nullptr, 4, data);
-      PrintRegisterValue(reg_ctx, "fs", nullptr, 4, data);
-      PrintRegisterValue(reg_ctx, "gs", nullptr, 4, data);
-
-      // Write out the EXC registers
-      data.PutHex32(EXCRegSet);
-      data.PutHex32(EXCWordCount);
-      PrintRegisterValue(reg_ctx, "trapno", nullptr, 4, data);
-      PrintRegisterValue(reg_ctx, "err", nullptr, 4, data);
-      PrintRegisterValue(reg_ctx, "faultvaddr", nullptr, 4, data);
-      return true;
-    }
-    return false;
-  }
-
-protected:
-  int DoReadGPR(lldb::tid_t tid, int flavor, GPR &gpr) override { return 0; }
-
-  int DoReadFPU(lldb::tid_t tid, int flavor, FPU &fpu) override { return 0; }
-
-  int DoReadEXC(lldb::tid_t tid, int flavor, EXC &exc) override { return 0; }
-
-  int DoWriteGPR(lldb::tid_t tid, int flavor, const GPR &gpr) override {
-    return 0;
-  }
-
-  int DoWriteFPU(lldb::tid_t tid, int flavor, const FPU &fpu) override {
-    return 0;
-  }
-
-  int DoWriteEXC(lldb::tid_t tid, int flavor, const EXC &exc) override {
-    return 0;
-  }
-};
-
 class RegisterContextDarwin_arm_Mach : public RegisterContextDarwin_arm {
 public:
   RegisterContextDarwin_arm_Mach(lldb_private::Thread &thread,
@@ -491,12 +371,11 @@ class RegisterContextDarwin_arm_Mach : public RegisterContextDarwin_arm {
     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);
       uint32_t count = data.GetU32(&offset);
-      lldb::offset_t next_thread_state = offset + (count * 4);
+      offset_t next_thread_state = offset + (count * 4);
       switch (flavor) {
       case GPRAltRegSet:
       case GPRRegSet: {
@@ -510,9 +389,7 @@ class RegisterContextDarwin_arm_Mach : public RegisterContextDarwin_arm {
 
           SetError(GPRRegSet, Read, 0);
         }
-      }
-        offset = next_thread_state;
-        break;
+      } break;
 
       case FPURegSet: {
         uint8_t *fpu_reg_buf = (uint8_t *)&fpu.floats;
@@ -522,12 +399,8 @@ class RegisterContextDarwin_arm_Mach : public RegisterContextDarwin_arm {
           offset += fpu_reg_buf_size;
           fpu.fpscr = data.GetU32(&offset);
           SetError(FPURegSet, Read, 0);
-        } else {
-          done = true;
         }
-      }
-        offset = next_thread_state;
-        break;
+      } break;
 
       case EXCRegSet:
         if (count == 3) {
@@ -536,14 +409,11 @@ class RegisterContextDarwin_arm_Mach : public RegisterContextDarwin_arm {
           exc.far = data.GetU32(&offset);
           SetError(EXCRegSet, Read, 0);
         }
-        done = true;
-        offset = next_thread_state;
         break;
 
-      // Unknown register set flavor, stop trying to parse.
       default:
-        done = true;
       }
+      offset = next_thread_state;
     }
   }
 
@@ -626,11 +496,10 @@ class RegisterContextDarwin_arm64_Mach : public RegisterContextDarwin_arm64 {
     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);
       uint32_t count = data.GetU32(&offset);
-      lldb::offset_t next_thread_state = offset + (count * 4);
+      offset_t next_thread_state = offset + (count * 4);
       switch (flavor) {
       case GPRRegSet:
         // x0-x29 + fp + lr + sp + pc (== 33 64-bit registers) plus cpsr (1
@@ -645,7 +514,6 @@ class RegisterContextDarwin_arm64_Mach : public RegisterContextDarwin_arm64 {
           gpr.cpsr = data.GetU32(&offset);
           SetError(GPRRegSet, Read, 0);
         }
-        offset = next_thread_state;
         break;
       case FPURegSet: {
         uint8_t *fpu_reg_buf = (uint8_t *)&fpu.v[0];
@@ -654,25 +522,19 @@ class RegisterContextDarwin_arm64_Mach : public RegisterContextDarwin_arm64 {
             data.ExtractBytes(offset, fpu_reg_buf_size, eByteOrderLittle,
                               fpu_reg_buf) == fpu_reg_buf_size) {
           SetError(FPURegSet, Read, 0);
-        } else {
-          done = true;
         }
-      }
-        offset = next_thread_state;
-        break;
+      } break;
       case EXCRegSet:
         if (count == 4) {
           exc.far = data.GetU64(&offset);
           exc.esr = data.GetU32(&offset);
           exc.exception = data.GetU32(&offset);
           SetError(EXCRegSet, Read, 0);
         }
-        offset = next_thread_state;
         break;
       default:
-        done = true;
-        break;
       }
+      offset = next_thread_state;
     }
   }
 
@@ -775,11 +637,10 @@ class RegisterContextDarwin_riscv32_Mach
     SetError(FPURegSet, Read, -1);
     SetError(EXCRegSet, Read, -1);
     SetError(CSRRegSet, Read, -1);
-    bool done = false;
-    while (!done) {
+    while (offset < data.GetByteSize()) {
       int flavor = data.GetU32(&offset);
       uint32_t count = data.GetU32(&offset);
-      lldb::offset_t next_thread_state = offset + (count * 4);
+      offset_t next_thread_state = offset + (count * 4);
       switch (flavor) {
       case GPRRegSet:
         // x0-x31 + pc
@@ -789,7 +650,6 @@ class RegisterContextDarwin_riscv32_Mach
           gpr.pc = data.GetU32(&offset);
           SetError(GPRRegSet, Read, 0);
         }
-        offset = next_thread_state;
         break;
       case FPURegSet: {
         // f0-f31 + fcsr
@@ -799,22 +659,18 @@ class RegisterContextDarwin_riscv32_Mach
           fpr.fcsr = data.GetU32(&offset);
           SetError(FPURegSet, Read, 0);
         }
-      }
-        offset = next_thread_state;
-        break;
+      } break;
       case EXCRegSet:
         if (count == 3) {
           exc.exception = data.GetU32(&offset);
           exc.fsr = data.GetU32(&offset);
           exc.far = data.GetU32(&offset);
           SetError(EXCRegSet, Read, 0);
         }
-        offset = next_thread_state;
         break;
       default:
-        done = true;
-        break;
       }
+      offset = next_thread_state;
     }
   }
 
@@ -5408,16 +5264,6 @@ lldb_private::Address ObjectFileMachO::GetEntryPointAddress() {
               done = true;
             }
             break;
-          case llvm::MachO::CPU_TYPE_I386:
-            if (flavor ==
-                1) // x86_THREAD_STATE32 from mach/i386/thread_status.h
-            {
-              offset += 40; // This is the offset of eip in the GPR thread state
-                            // data structure.
-              start_address = m_data.GetU32(&offset);
-              done = true;
-            }
-            break;
           case llvm::MachO::CPU_TYPE_X86_64:
             if (flavor ==
                 4) // x86_THREAD_STATE64 from mach/i386/thread_status.h
@@ -5897,11 +5743,6 @@ ObjectFileMachO::GetThreadContextAtIndex(uint32_t idx,
             std::make_shared<RegisterContextDarwin_arm_Mach>(thread, data);
         break;
 
-      case llvm::MachO::CPU_TYPE_I386:
-        reg_ctx_sp =
-            std::make_shared<RegisterContextDarwin_i386_Mach>(thread, data);
-        break;
-
       case llvm::MachO::CPU_TYPE_X86_64:
         reg_ctx_sp =
             std::make_shared<RegisterContextDarwin_x86_64_Mach>(thread, data);
@@ -6769,11 +6610,6 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp,
                   thread_sp.get(), LC_THREAD_datas[thread_idx]);
               break;
 
-            case llvm::MachO::CPU_TYPE_I386:
-              RegisterContextDarwin_i386_Mach::Create_LC_THREAD(
-                  thread_sp.get(), LC_THREAD_datas[thread_idx]);
-              break;
-
             case llvm::MachO::CPU_TYPE_X86_64:
               RegisterContextDarwin_x86_64_Mach::Create_LC_THREAD(
                   thread_sp.get(), LC_THREAD_datas[thread_idx]);

lldb/source/Plugins/Process/MacOSX-Kernel/CMakeLists.txt

@@ -12,7 +12,6 @@ add_lldb_library(lldbPluginProcessMacOSXKernel PLUGIN
   ProcessKDPLog.cpp
   RegisterContextKDP_arm.cpp
   RegisterContextKDP_arm64.cpp
-  RegisterContextKDP_i386.cpp
   RegisterContextKDP_x86_64.cpp
   ThreadKDP.cpp