[lldb] [Process/FreeBSDRemote] Access debug registers via offsets

[mgorny]

Summary

Use offset-based method to access x86 debug registers. This also
involves adding a test for the correctness of these offsets, and making
GetDR() method of NativeRegisterContextWatchpoint_x86 public to avoid
duplicate code.

[mgorny]

Update following the changes in GPR patch.

[mgorny]

Rebased.

[mgorny]

Fix native_i386_dbregs on i386.

[labath]

lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp

699–711

return GetDR(0)->byte_offset?

[mgorny]

lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp

699–711

Is there a point in changing this given that it gets replaced by D91411?

[labath]

lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp

699–711

nah, that's fine

lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h

private:
   enum {
     YMMXSaveSet,
     MaxXSaveSet = YMMXSaveSet,
   };
 
   // Private member variables.
   std::array<uint8_t, sizeof(struct reg)> m_gpr;
   std::array<uint8_t, 512> m_fpr; // FXSAVE
-  struct dbreg m_dbr;
+  std::array<uint8_t, sizeof(struct dbreg)> m_dbr;
   std::vector<uint8_t> m_xsave;
   std::array<uint32_t, MaxXSaveSet + 1> m_xsave_offsets;
 
   llvm::Optional<enum RegSetKind> GetSetForNativeRegNum(int reg_num) const;
-  int GetDR(int num) const;
 
   Status ReadRegisterSet(uint32_t set);
   Status WriteRegisterSet(uint32_t set);
 
   size_t GetFPROffset() const;
+  size_t GetDBROffset() const;
 };
 
 } // namespace process_freebsd
 } // namespace lldb_private
 
 #endif // #ifndef lldb_NativeRegisterContextFreeBSD_x86_64_h
 
 #endif // defined(__x86_64__)

lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp

if (HostInfo::GetArchitecture().GetAddressByteSize() == 4) {
     // x86_64 register context.
     return new RegisterContextFreeBSD_x86_64(target_arch);
   }
 }
 
 NativeRegisterContextFreeBSD_x86_64::NativeRegisterContextFreeBSD_x86_64(
     const ArchSpec &target_arch, NativeThreadProtocol &native_thread)
     : NativeRegisterContextRegisterInfo(
-          native_thread, CreateRegisterInfoInterface(target_arch)),
-      m_dbr() {
+          native_thread, CreateRegisterInfoInterface(target_arch)) {
   assert(m_gpr.size() == GetRegisterInfoInterface().GetGPRSize());
 }
 
 // CONSIDER after local and llgs debugging are merged, register set support can
 // be moved into a base x86-64 class with IsRegisterSetAvailable made virtual.
 uint32_t NativeRegisterContextFreeBSD_x86_64::GetRegisterSetCount() const {
   uint32_t sets = 0;
   for (uint32_t set_index = 0; set_index < k_num_register_sets; ++set_index) {
static constexpr int RegNumX86ToX86_64(int regnum) {
   case lldb_bnd1_i386:
   case lldb_bnd2_i386:
   case lldb_bnd3_i386:
     return lldb_bnd0_x86_64 + regnum - lldb_bnd0_i386;
   case lldb_bndcfgu_i386:
     return lldb_bndcfgu_x86_64;
   case lldb_bndstatus_i386:
     return lldb_bndstatus_x86_64;
-  case lldb_dr0_i386:
-  case lldb_dr1_i386:
-  case lldb_dr2_i386:
-  case lldb_dr3_i386:
-  case lldb_dr4_i386:
-  case lldb_dr5_i386:
-  case lldb_dr6_i386:
-  case lldb_dr7_i386:
-    return lldb_dr0_x86_64 + regnum - lldb_dr0_i386;
   default:
     llvm_unreachable("Unhandled i386 register.");
   }
 }
 
 llvm::Optional<enum NativeRegisterContextFreeBSD_x86_64::RegSetKind>
 NativeRegisterContextFreeBSD_x86_64::GetSetForNativeRegNum(int reg_num) const {
   switch (GetRegisterInfoInterface().GetTargetArchitecture().GetMachine()) {
#if defined(__x86_64__)
     return NativeProcessFreeBSD::PtraceWrapper(PT_GETFPREGS, m_thread.GetID(),
                                                m_fpr.data());
 #else
     return NativeProcessFreeBSD::PtraceWrapper(PT_GETXMMREGS, m_thread.GetID(),
                                                m_fpr.data());
 #endif
   case DBRegSet:
     return NativeProcessFreeBSD::PtraceWrapper(PT_GETDBREGS, m_thread.GetID(),
-                                               &m_dbr);
+                                               m_dbr.data());
   case XSaveRegSet: {
     struct ptrace_xstate_info info;
     Status ret = NativeProcessFreeBSD::PtraceWrapper(
         PT_GETXSTATE_INFO, GetProcessPid(), &info, sizeof(info));
     if (!ret.Success())
       return ret;
 
     assert(info.xsave_mask & XFEATURE_ENABLED_X87);
#if defined(__x86_64__)
     return NativeProcessFreeBSD::PtraceWrapper(PT_SETFPREGS, m_thread.GetID(),
                                                m_fpr.data());
 #else
     return NativeProcessFreeBSD::PtraceWrapper(PT_SETXMMREGS, m_thread.GetID(),
                                                m_fpr.data());
 #endif
   case DBRegSet:
     return NativeProcessFreeBSD::PtraceWrapper(PT_SETDBREGS, m_thread.GetID(),
-                                               &m_dbr);
+                                               m_dbr.data());
   case XSaveRegSet:
     // ReadRegisterSet() must always be called before WriteRegisterSet().
     assert(m_xsave.size() > 0);
     return NativeProcessFreeBSD::PtraceWrapper(PT_SETXSTATE, GetProcessPid(),
                                                m_xsave.data(), m_xsave.size());
   }
   llvm_unreachable("NativeRegisterContextFreeBSD_x86_64::WriteRegisterSet");
 }
NativeRegisterContextFreeBSD_x86_64::ReadRegister(const RegisterInfo *reg_info,
   case GPRegSet:
     reg_value.SetBytes(m_gpr.data() + reg_info->byte_offset,
                        reg_info->byte_size, endian::InlHostByteOrder());
     return error;
   case FPRegSet:
     reg_value.SetBytes(m_fpr.data() + reg_info->byte_offset - GetFPROffset(),
                        reg_info->byte_size, endian::InlHostByteOrder());
     return error;
-  case XSaveRegSet:
   case DBRegSet:
+    reg_value.SetBytes(m_dbr.data() + reg_info->byte_offset - GetDBROffset(),
+                       reg_info->byte_size, endian::InlHostByteOrder());
+    return error;
+  case XSaveRegSet:
     // legacy logic
     break;
   }
 
   switch (GetRegisterInfoInterface().GetTargetArchitecture().GetMachine()) {
   case llvm::Triple::x86_64:
     break;
   case llvm::Triple::x86:
if (offset == LLDB_INVALID_XSAVE_OFFSET) {
       YMMReg ymm =
           XStateToYMM(reinterpret_cast<void *>(&fpreg->sv_xmm[reg_index]),
                       reinterpret_cast<void *>(&ymmreg[reg_index]));
       reg_value.SetBytes(ymm.bytes, reg_info->byte_size,
                          endian::InlHostByteOrder());
     }
     break;
   }
-  case lldb_dr0_x86_64:
-  case lldb_dr1_x86_64:
-  case lldb_dr2_x86_64:
-  case lldb_dr3_x86_64:
-  case lldb_dr4_x86_64:
-  case lldb_dr5_x86_64:
-  case lldb_dr6_x86_64:
-  case lldb_dr7_x86_64:
-    reg_value = (uint64_t)m_dbr.dr[reg - lldb_dr0_x86_64];
-    break;
   default:
     llvm_unreachable("Reading unknown/unsupported register");
   }
 
   return error;
 }
 
 Status NativeRegisterContextFreeBSD_x86_64::WriteRegister(
Status NativeRegisterContextFreeBSD_x86_64::WriteRegister(
   case GPRegSet:
     ::memcpy(m_gpr.data() + reg_info->byte_offset, reg_value.GetBytes(),
              reg_value.GetByteSize());
     return WriteRegisterSet(set);
   case FPRegSet:
     ::memcpy(m_fpr.data() + reg_info->byte_offset - GetFPROffset(),
              reg_value.GetBytes(), reg_value.GetByteSize());
     return WriteRegisterSet(set);
-  case XSaveRegSet:
   case DBRegSet:
+    ::memcpy(m_dbr.data() + reg_info->byte_offset - GetDBROffset(),
+             reg_value.GetBytes(), reg_value.GetByteSize());
+    return WriteRegisterSet(set);
+  case XSaveRegSet:
     // legacy logic
     break;
   }
 
   switch (GetRegisterInfoInterface().GetTargetArchitecture().GetMachine()) {
   case llvm::Triple::x86_64:
     break;
   case llvm::Triple::x86:
if (offset == LLDB_INVALID_XSAVE_OFFSET) {
 
       YMMReg ymm;
       ::memcpy(ymm.bytes, reg_value.GetBytes(), reg_value.GetByteSize());
       YMMToXState(ymm, reinterpret_cast<void *>(&fpreg->sv_xmm[reg_index]),
                   reinterpret_cast<void *>(&ymmreg[reg_index]));
     }
     break;
   }
-  case lldb_dr0_x86_64:
-  case lldb_dr1_x86_64:
-  case lldb_dr2_x86_64:
-  case lldb_dr3_x86_64:
-  case lldb_dr4_x86_64:
-  case lldb_dr5_x86_64:
-  case lldb_dr6_x86_64:
-  case lldb_dr7_x86_64:
-    m_dbr.dr[reg - lldb_dr0_x86_64] = reg_value.GetAsUInt64();
-    break;
   default:
     llvm_unreachable("Reading unknown/unsupported register");
   }
 
   return WriteRegisterSet(set);
 }
 
 Status NativeRegisterContextFreeBSD_x86_64::ReadAllRegisterValues(
Status NativeRegisterContextFreeBSD_x86_64::WriteAllRegisterValues(
   error = WriteRegisterSet(GPRegSet);
   if (error.Fail())
     return error;
   src += GetRegisterInfoInterface().GetGPRSize();
 
   return error;
 }
 
-int NativeRegisterContextFreeBSD_x86_64::GetDR(int num) const {
-  assert(num >= 0 && num <= 7);
-  switch (GetRegisterInfoInterface().GetTargetArchitecture().GetMachine()) {
-  case llvm::Triple::x86:
-    return lldb_dr0_i386 + num;
-  case llvm::Triple::x86_64:
-    return lldb_dr0_x86_64 + num;
-  default:
-    llvm_unreachable("Unhandled target architecture.");
-  }
-}
-
 llvm::Error NativeRegisterContextFreeBSD_x86_64::CopyHardwareWatchpointsFrom(
     NativeRegisterContextFreeBSD &source) {
   auto &r_source = static_cast<NativeRegisterContextFreeBSD_x86_64 &>(source);
-  Status res = r_source.ReadRegisterSet(DBRegSet);
+  // NB: This implicitly reads the whole dbreg set.
+  RegisterValue dr7;
+  Status res = r_source.ReadRegister(GetDR(7), dr7);
   if (!res.Fail()) {
     // copy dbregs only if any watchpoints were set
-    if ((r_source.m_dbr.dr[7] & 0xFF) == 0)
+    if ((dr7.GetAsUInt64() & 0xFF) == 0)
       return llvm::Error::success();
 
     m_dbr = r_source.m_dbr;
     res = WriteRegisterSet(DBRegSet);
   }
   return res.ToError();
 }
 
 size_t NativeRegisterContextFreeBSD_x86_64::GetFPROffset() const {
   uint32_t regno;
   switch (GetRegisterInfoInterface().GetTargetArchitecture().GetMachine()) {
   case llvm::Triple::x86:
     regno = lldb_fctrl_i386;
     break;
   case llvm::Triple::x86_64:
     regno = lldb_fctrl_x86_64;
     break;
   default:
     llvm_unreachable("Unhandled target architecture.");
   }
 
   return GetRegisterInfoInterface().GetRegisterInfo()[regno].byte_offset;
 }
 
+size_t NativeRegisterContextFreeBSD_x86_64::GetDBROffset() const {
+  uint32_t regno;
+  switch (GetRegisterInfoInterface().GetTargetArchitecture().GetMachine()) {
+  case llvm::Triple::x86:
+    regno = lldb_dr0_i386;
+    break;
+  case llvm::Triple::x86_64:
+    regno = lldb_dr0_x86_64;
+    break;
+  default:
+    llvm_unreachable("Unhandled target architecture.");
+  }
+
+  return GetRegisterInfoInterface().GetRegisterInfo()[regno].byte_offset;
+}
+
 #endif // defined(__x86_64__)

lldb/source/Plugins/Process/Utility/NativeRegisterContextWatchpoint_x86.h

public:
 
   uint32_t SetHardwareWatchpoint(lldb::addr_t addr, size_t size,
                                  uint32_t watch_flags) override;
 
   lldb::addr_t GetWatchpointAddress(uint32_t wp_index) override;
 
   uint32_t NumSupportedHardwareWatchpoints() override;
 
-private:
   const RegisterInfo *GetDR(int num) const;
 };
 
 } // namespace lldb_private
 
 #endif // #ifndef lldb_NativeRegisterContextWatchpoint_x86_h

lldb/unittests/Process/Utility/RegisterContextFreeBSDTests.cpp

EXPECT_THAT(GetRegParams(reg_ctx, lldb_##regname),                           \
               ::testing::Pair(offset + base_offset, size))
 
 #if defined(__x86_64__)
 
 #define ASSERT_GPR_X86_64(regname)                                             \
   EXPECT_THAT(                                                                 \
       GetRegParams(reg_ctx, lldb_##regname##_x86_64),                          \
       ::testing::Pair(offsetof(reg, r_##regname), sizeof(reg::r_##regname)))
+#define ASSERT_DBR_X86_64(num)                                                 \
+  ASSERT_OFF(dr##num##_x86_64, offsetof(dbreg, dr[num]), sizeof(dbreg::dr[num]))
 
 TEST(RegisterContextFreeBSDTest, x86_64) {
   ArchSpec arch{"x86_64-unknown-freebsd12.2"};
   RegisterContextFreeBSD_x86_64 reg_ctx{arch};
 
   ASSERT_GPR_X86_64(r15);
   ASSERT_GPR_X86_64(r14);
   ASSERT_GPR_X86_64(r13);
TEST(RegisterContextFreeBSDTest, x86_64) {
   ASSERT_OFF(xmm8_x86_64, 0x120, 16);
   ASSERT_OFF(xmm9_x86_64, 0x130, 16);
   ASSERT_OFF(xmm10_x86_64, 0x140, 16);
   ASSERT_OFF(xmm11_x86_64, 0x150, 16);
   ASSERT_OFF(xmm12_x86_64, 0x160, 16);
   ASSERT_OFF(xmm13_x86_64, 0x170, 16);
   ASSERT_OFF(xmm14_x86_64, 0x180, 16);
   ASSERT_OFF(xmm15_x86_64, 0x190, 16);
+
+  base_offset = reg_ctx.GetRegisterInfo()[lldb_dr0_x86_64].byte_offset;
+  ASSERT_DBR_X86_64(0);
+  ASSERT_DBR_X86_64(1);
+  ASSERT_DBR_X86_64(2);
+  ASSERT_DBR_X86_64(3);
+  ASSERT_DBR_X86_64(4);
+  ASSERT_DBR_X86_64(5);
+  ASSERT_DBR_X86_64(6);
+  ASSERT_DBR_X86_64(7);
 }
 #endif
 
 #if defined(__i386__) || defined(__x86_64__)
 
 #define ASSERT_GPR_I386(regname)                                               \
   EXPECT_THAT(GetRegParams(reg_ctx, lldb_##regname##_i386),                    \
               ::testing::Pair(offsetof(native_i386_regs, r_##regname),         \
                               sizeof(native_i386_regs::r_##regname)))
+#define ASSERT_DBR_I386(num)                                                   \
+  ASSERT_OFF(dr##num##_i386, offsetof(dbreg32, dr[num]),                       \
+             sizeof(dbreg32::dr[num]))
 
 TEST(RegisterContextFreeBSDTest, i386) {
   ArchSpec arch{"i686-unknown-freebsd12.2"};
   RegisterContextFreeBSD_i386 reg_ctx{arch};
 
 #if defined(__i386__)
   using native_i386_regs = ::reg;
 #else
#endif
   ASSERT_OFF(xmm0_i386, 0xA0, 16);
   ASSERT_OFF(xmm1_i386, 0xB0, 16);
   ASSERT_OFF(xmm2_i386, 0xC0, 16);
   ASSERT_OFF(xmm3_i386, 0xD0, 16);
   ASSERT_OFF(xmm4_i386, 0xE0, 16);
   ASSERT_OFF(xmm5_i386, 0xF0, 16);
   ASSERT_OFF(xmm6_i386, 0x100, 16);
   ASSERT_OFF(xmm7_i386, 0x110, 16);
+
+  base_offset = reg_ctx.GetRegisterInfo()[lldb_dr0_i386].byte_offset;
+  ASSERT_DBR_I386(0);
+  ASSERT_DBR_I386(1);
+  ASSERT_DBR_I386(2);
+  ASSERT_DBR_I386(3);
+  ASSERT_DBR_I386(4);
+  ASSERT_DBR_I386(5);
+  ASSERT_DBR_I386(6);
+  ASSERT_DBR_I386(7);
 }
 #endif