[lldb] [Process/FreeBSDRemote] Access GPR via reginfo offsets

[mgorny]

Summary

Read and write registers from m_gpr using offsets from RegisterInfo
rather than explicit switch-case. This eliminates a lot of redundant
code, and avoids mistakes such as type mismatches seen recently (wrt
segment registers). The same logic will be extended to other register
sets in the future.

Make m_gpr an uint8_t std::array to ease accesses. Ideally, we could
avoid including <machine/reg.h> entirely in the future and instead
get the correct GPR size from Utility/RegisterContextFreeBSD_* somehow.

While at it, modify register set logic to use an explicit enum with
llvm::Optional<>, making the code cleaner and at the same time enabling
compiler warnings for unhandled sets.

Since now we're fully relying on 'struct GPR' defined
in Utility/RegisterContextFreeBSD_* being entirely in sync with
the system structure, add unit tests to verify the field offsets
and sizes.

[labath]

Regarding the asserts, I'm thinking if it wouldn't be better to check the compare the resulting RegisterInfo structs instead of the raw struct layout. The reasons for that are:

• it will be checking the thing that we're actually relying on (the register info macros is pretty complex and can be messed up easily
• the register info artifacts (unlike the GPR structs) are exported from these files, so the checks for that could be placed into a unit test

What do you think?

[mgorny]

In D91216#2387909, @labath wrote:

Regarding the asserts, I'm thinking if it wouldn't be better to check the compare the resulting RegisterInfo structs instead of the raw struct layout. The reasons for that are:

• it will be checking the thing that we're actually relying on (the register info macros is pretty complex and can be messed up easily
• the register info artifacts (unlike the GPR structs) are exported from these files, so the checks for that could be placed into a unit test

What do you think?

I presume you mean verifying that byte_offsets and byte_sizes are written correctly to the struct? I suppose that makes sense.

I like the idea that static asserts are going to blow up during compilation already but I suppose it's not very important as I don't expect these structures to change.

[labath]

In D91216#2387956, @mgorny wrote:

In D91216#2387909, @labath wrote:

Regarding the asserts, I'm thinking if it wouldn't be better to check the compare the resulting RegisterInfo structs instead of the raw struct layout. The reasons for that are:

• it will be checking the thing that we're actually relying on (the register info macros is pretty complex and can be messed up easily
• the register info artifacts (unlike the GPR structs) are exported from these files, so the checks for that could be placed into a unit test

What do you think?

I presume you mean verifying that byte_offsets and byte_sizes are written correctly to the struct?

Yes, exactly.

I like the idea that static asserts are going to blow up during compilation already but I suppose it's not very important as I don't expect these structures to change.

That's true, but I've found that the inclusion of system headers (with all their acoompanying ifdefs) looks pretty bad...

[mgorny]

Updated to use unittests instead of static asserts.

[mgorny]

clang-format

[mgorny]

Add a helper ASSERT_REG macro.

[labath]

I like this. Some comments in the tests -- trying to reduce the amount of preprocessor magic..

lldb/unittests/Process/FreeBSD/CMakeLists.txt

1 ↗

(On Diff #304448)

I'd put this in unittests/Process/Utility, to match the location of the code being tested.

8–9 ↗

(On Diff #304448)

Drop this, and include the file as "Plugins/Process/Utility/..."

lldb/unittests/Process/FreeBSD/RegisterContextFreeBSDTests.cpp

16 ↗

(On Diff #304448)

These don't really have to be ifdefed, do they?

26–31 ↗

(On Diff #304448)

pair<size_t(?), size_t> GetRegParams(const RegisterContext &ctx, unsigned reg) {
  const RegisterInfo &info = reg_ctx.GetRegisterInfo()[reg];
  return {info.byte_offset, info.byte_size};
}

35 ↗

(On Diff #304448)

#define ASSERT_GPR_X86_64(regname)
  ASSERT_THAT(GetRegParams(reg_ctx, regname##_x86_64, testing::Pair(offsetof(reg, r_##regname), sizeof(reg::r_##regname))

72–74 ↗

(On Diff #304448)

Inside the test function:

#ifdef __i386__
using native_i386_regs = ::reg32;
#else
using native_i386_regs = ::reg;
#endif

[mgorny]

lldb/unittests/Process/FreeBSD/RegisterContextFreeBSDTests.cpp

26–31 ↗

(On Diff #304448)

Fun fact: RegisterContextFreeBSD_* is not a RegisterContext& but a RegisterInfoInterface& ;-).

[mgorny]

Implemented all the requested changes.

[mgorny]

clang-format

[labath]

lldb/unittests/Process/FreeBSD/RegisterContextFreeBSDTests.cpp

26–31 ↗

(On Diff #304448)

Yeah, tell me about it...

lldb/unittests/Process/Utility/CMakeLists.txt

2–3

Rename to ProcessUtilityTests -- all files in this folder should go into a single binary. We have more tests coming in D87442.

4

It seems we have some files ending in Tests, but the prevalent convention is just Test

lldb/unittests/Process/Utility/RegisterContextFreeBSDTests.cpp

38 ↗

(On Diff #304859)

I guess the version is not really relevant for this..

[labath]

lldb/unittests/Process/Utility/RegisterContextFreeBSDTests.cpp

32 ↗

(On Diff #304859)

Also, please rename this to EXPECT_GRP_X86_64. ASSERT_ is for macros that terminate test when they fail.

[mgorny]

Implemented requested changes.

[mgorny]

Create the unit test unconditionally, guard the code with __FreeBSD__ define.

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

 
 // clang-format off
 #include <sys/param.h>
 #include <sys/ptrace.h>
 #include <sys/types.h>
 #include <machine/reg.h>
 // clang-format on
 
+#include <array>
+
 #include "Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD.h"
 #include "Plugins/Process/Utility/RegisterContext_x86.h"
 #include "Plugins/Process/Utility/NativeRegisterContextWatchpoint_x86.h"
 #include "Plugins/Process/Utility/lldb-x86-register-enums.h"
 
 #define LLDB_INVALID_XSAVE_OFFSET UINT32_MAX
 
 namespace lldb_private {
public:
 
   Status WriteAllRegisterValues(const lldb::DataBufferSP &data_sp) override;
 
   llvm::Error
   CopyHardwareWatchpointsFrom(NativeRegisterContextFreeBSD &source) override;
 
 private:
   // Private member types.
-  enum { GPRegSet, FPRegSet, XSaveRegSet, DBRegSet };
+  enum RegSetKind {
+    GPRegSet,
+    FPRegSet,
+    XSaveRegSet,
+    DBRegSet,
+  };
   enum {
     YMMXSaveSet,
     MaxXSaveSet = YMMXSaveSet,
   };
 
   // Private member variables.
-  struct reg m_gpr;
+  std::array<uint8_t, sizeof(struct reg)> m_gpr;
 #if defined(__x86_64__)
   struct fpreg m_fpr;
 #else
   struct xmmreg m_fpr;
 #endif
   struct dbreg m_dbr;
   std::vector<uint8_t> m_xsave;
   std::array<uint32_t, MaxXSaveSet + 1> m_xsave_offsets;
 
-  int GetSetForNativeRegNum(int reg_num) const;
+  llvm::Optional<enum RegSetKind> GetSetForNativeRegNum(int reg_num) const;
   int GetDR(int num) const;
 
   Status ReadRegisterSet(uint32_t set);
   Status WriteRegisterSet(uint32_t set);
 };
 
 } // 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) {
     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_gpr(), m_fpr(), m_dbr() {}
+      m_fpr(), m_dbr() {
+  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) {
-    if (GetSetForNativeRegNum(set_index) != -1)
+    if (GetSetForNativeRegNum(set_index))
       ++sets;
   }
 
   return sets;
 }
 
 const RegisterSet *
 NativeRegisterContextFreeBSD_x86_64::GetRegisterSet(uint32_t set_index) const {
   switch (GetRegisterInfoInterface().GetTargetArchitecture().GetMachine()) {
   case llvm::Triple::x86:
     return &g_reg_sets_i386[set_index];
   case llvm::Triple::x86_64:
     return &g_reg_sets_x86_64[set_index];
   default:
     llvm_unreachable("Unhandled target architecture.");
   }
 }
 
 static constexpr int RegNumX86ToX86_64(int regnum) {
   switch (regnum) {
-  case lldb_eax_i386:
-    return lldb_rax_x86_64;
-  case lldb_ebx_i386:
-    return lldb_rbx_x86_64;
-  case lldb_ecx_i386:
-    return lldb_rcx_x86_64;
-  case lldb_edx_i386:
-    return lldb_rdx_x86_64;
-  case lldb_edi_i386:
-    return lldb_rdi_x86_64;
-  case lldb_esi_i386:
-    return lldb_rsi_x86_64;
-  case lldb_ebp_i386:
-    return lldb_rbp_x86_64;
-  case lldb_esp_i386:
-    return lldb_rsp_x86_64;
-  case lldb_eip_i386:
-    return lldb_rip_x86_64;
-  case lldb_eflags_i386:
-    return lldb_rflags_x86_64;
-  case lldb_cs_i386:
-    return lldb_cs_x86_64;
-  case lldb_fs_i386:
-    return lldb_fs_x86_64;
-  case lldb_gs_i386:
-    return lldb_gs_x86_64;
-  case lldb_ss_i386:
-    return lldb_ss_x86_64;
-  case lldb_ds_i386:
-    return lldb_ds_x86_64;
-  case lldb_es_i386:
-    return lldb_es_x86_64;
   case lldb_fctrl_i386:
     return lldb_fctrl_x86_64;
   case lldb_fstat_i386:
     return lldb_fstat_x86_64;
   case lldb_ftag_i386:
     return lldb_ftag_x86_64;
   case lldb_fop_i386:
     return lldb_fop_x86_64;
static constexpr int RegNumX86ToX86_64(int regnum) {
   case lldb_dr6_i386:
   case lldb_dr7_i386:
     return lldb_dr0_x86_64 + regnum - lldb_dr0_i386;
   default:
     llvm_unreachable("Unhandled i386 register.");
   }
 }
 
-int NativeRegisterContextFreeBSD_x86_64::GetSetForNativeRegNum(
-    int reg_num) const {
-
+llvm::Optional<enum NativeRegisterContextFreeBSD_x86_64::RegSetKind>
+NativeRegisterContextFreeBSD_x86_64::GetSetForNativeRegNum(int reg_num) const {
   switch (GetRegisterInfoInterface().GetTargetArchitecture().GetMachine()) {
   case llvm::Triple::x86:
     if (reg_num >= k_first_gpr_i386 && reg_num <= k_last_gpr_i386)
       return GPRegSet;
     if (reg_num >= k_first_fpr_i386 && reg_num <= k_last_fpr_i386)
       return FPRegSet;
     if (reg_num >= k_first_avx_i386 && reg_num <= k_last_avx_i386)
       return XSaveRegSet; // AVX
     if (reg_num >= k_first_mpxr_i386 && reg_num <= k_last_mpxr_i386)
-      return -1; // MPXR
+      return llvm::None; // MPXR
     if (reg_num >= k_first_mpxc_i386 && reg_num <= k_last_mpxc_i386)
-      return -1; // MPXC
+      return llvm::None; // MPXC
     if (reg_num >= k_first_dbr_i386 && reg_num <= k_last_dbr_i386)
       return DBRegSet; // DBR
     break;
   case llvm::Triple::x86_64:
     if (reg_num >= k_first_gpr_x86_64 && reg_num <= k_last_gpr_x86_64)
       return GPRegSet;
     if (reg_num >= k_first_fpr_x86_64 && reg_num <= k_last_fpr_x86_64)
       return FPRegSet;
     if (reg_num >= k_first_avx_x86_64 && reg_num <= k_last_avx_x86_64)
       return XSaveRegSet; // AVX
     if (reg_num >= k_first_mpxr_x86_64 && reg_num <= k_last_mpxr_x86_64)
-      return -1; // MPXR
+      return llvm::None; // MPXR
     if (reg_num >= k_first_mpxc_x86_64 && reg_num <= k_last_mpxc_x86_64)
-      return -1; // MPXC
+      return llvm::None; // MPXC
     if (reg_num >= k_first_dbr_x86_64 && reg_num <= k_last_dbr_x86_64)
       return DBRegSet; // DBR
     break;
   default:
     llvm_unreachable("Unhandled target architecture.");
   }
 
   llvm_unreachable("Register does not belong to any register set");
 }
 
 Status NativeRegisterContextFreeBSD_x86_64::ReadRegisterSet(uint32_t set) {
   switch (set) {
   case GPRegSet:
     return NativeProcessFreeBSD::PtraceWrapper(PT_GETREGS, m_thread.GetID(),
-                                               &m_gpr);
+                                               m_gpr.data());
   case FPRegSet:
 #if defined(__x86_64__)
     return NativeProcessFreeBSD::PtraceWrapper(PT_GETFPREGS, m_thread.GetID(),
                                                &m_fpr);
 #else
     return NativeProcessFreeBSD::PtraceWrapper(PT_GETXMMREGS, m_thread.GetID(),
                                                &m_fpr);
 #endif
#endif
   }
   llvm_unreachable("NativeRegisterContextFreeBSD_x86_64::ReadRegisterSet");
 }
 
 Status NativeRegisterContextFreeBSD_x86_64::WriteRegisterSet(uint32_t set) {
   switch (set) {
   case GPRegSet:
     return NativeProcessFreeBSD::PtraceWrapper(PT_SETREGS, m_thread.GetID(),
-                                               &m_gpr);
+                                               m_gpr.data());
   case FPRegSet:
 #if defined(__x86_64__)
     return NativeProcessFreeBSD::PtraceWrapper(PT_SETFPREGS, m_thread.GetID(),
                                                &m_fpr);
 #else
     return NativeProcessFreeBSD::PtraceWrapper(PT_SETXMMREGS, m_thread.GetID(),
                                                &m_fpr);
 #endif
if (reg == LLDB_INVALID_REGNUM) {
     // This is likely an internal register for lldb use only and should not be
     // directly queried.
     error.SetErrorStringWithFormat("register \"%s\" is an internal-only lldb "
                                    "register, cannot read directly",
                                    reg_info->name);
     return error;
   }
 
-  int set = GetSetForNativeRegNum(reg);
-  if (set == -1) {
+  llvm::Optional<enum RegSetKind> opt_set = GetSetForNativeRegNum(reg);
+  if (!opt_set) {
     // This is likely an internal register for lldb use only and should not be
     // directly queried.
     error.SetErrorStringWithFormat("register \"%s\" is in unrecognized set",
                                    reg_info->name);
     return error;
   }
 
+  enum RegSetKind set = opt_set.getValue();
+  error = ReadRegisterSet(set);
+  if (error.Fail())
+    return error;
+
+  switch (set) {
+  case GPRegSet:
+    reg_value.SetBytes(m_gpr.data() + reg_info->byte_offset,
+                       reg_info->byte_size, endian::InlHostByteOrder());
+    return error;
+  case FPRegSet:
+  case XSaveRegSet:
+  case DBRegSet:
+    // legacy logic
+    break;
+  }
+
   switch (GetRegisterInfoInterface().GetTargetArchitecture().GetMachine()) {
   case llvm::Triple::x86_64:
     break;
   case llvm::Triple::x86:
     reg = RegNumX86ToX86_64(reg);
     break;
   default:
     llvm_unreachable("Unhandled target architecture.");
   }
 
-  error = ReadRegisterSet(set);
-  if (error.Fail())
-    return error;
-
   switch (reg) {
 #if defined(__x86_64__)
-  case lldb_rax_x86_64:
-    reg_value = (uint64_t)m_gpr.r_rax;
-    break;
-  case lldb_rbx_x86_64:
-    reg_value = (uint64_t)m_gpr.r_rbx;
-    break;
-  case lldb_rcx_x86_64:
-    reg_value = (uint64_t)m_gpr.r_rcx;
-    break;
-  case lldb_rdx_x86_64:
-    reg_value = (uint64_t)m_gpr.r_rdx;
-    break;
-  case lldb_rdi_x86_64:
-    reg_value = (uint64_t)m_gpr.r_rdi;
-    break;
-  case lldb_rsi_x86_64:
-    reg_value = (uint64_t)m_gpr.r_rsi;
-    break;
-  case lldb_rbp_x86_64:
-    reg_value = (uint64_t)m_gpr.r_rbp;
-    break;
-  case lldb_rsp_x86_64:
-    reg_value = (uint64_t)m_gpr.r_rsp;
-    break;
-  case lldb_r8_x86_64:
-    reg_value = (uint64_t)m_gpr.r_r8;
-    break;
-  case lldb_r9_x86_64:
-    reg_value = (uint64_t)m_gpr.r_r9;
-    break;
-  case lldb_r10_x86_64:
-    reg_value = (uint64_t)m_gpr.r_r10;
-    break;
-  case lldb_r11_x86_64:
-    reg_value = (uint64_t)m_gpr.r_r11;
-    break;
-  case lldb_r12_x86_64:
-    reg_value = (uint64_t)m_gpr.r_r12;
-    break;
-  case lldb_r13_x86_64:
-    reg_value = (uint64_t)m_gpr.r_r13;
-    break;
-  case lldb_r14_x86_64:
-    reg_value = (uint64_t)m_gpr.r_r14;
-    break;
-  case lldb_r15_x86_64:
-    reg_value = (uint64_t)m_gpr.r_r15;
-    break;
-  case lldb_rip_x86_64:
-    reg_value = (uint64_t)m_gpr.r_rip;
-    break;
-  case lldb_rflags_x86_64:
-    reg_value = (uint64_t)m_gpr.r_rflags;
-    break;
-  case lldb_cs_x86_64:
-    reg_value = (uint64_t)m_gpr.r_cs;
-    break;
-  case lldb_fs_x86_64:
-    reg_value = (uint16_t)m_gpr.r_fs;
-    break;
-  case lldb_gs_x86_64:
-    reg_value = (uint16_t)m_gpr.r_gs;
-    break;
-  case lldb_ss_x86_64:
-    reg_value = (uint64_t)m_gpr.r_ss;
-    break;
-  case lldb_ds_x86_64:
-    reg_value = (uint16_t)m_gpr.r_ds;
-    break;
-  case lldb_es_x86_64:
-    reg_value = (uint16_t)m_gpr.r_es;
-    break;
-#else
-  case lldb_rax_x86_64:
-    reg_value = (uint32_t)m_gpr.r_eax;
-    break;
-  case lldb_rbx_x86_64:
-    reg_value = (uint32_t)m_gpr.r_ebx;
-    break;
-  case lldb_rcx_x86_64:
-    reg_value = (uint32_t)m_gpr.r_ecx;
-    break;
-  case lldb_rdx_x86_64:
-    reg_value = (uint32_t)m_gpr.r_edx;
-    break;
-  case lldb_rdi_x86_64:
-    reg_value = (uint32_t)m_gpr.r_edi;
-    break;
-  case lldb_rsi_x86_64:
-    reg_value = (uint32_t)m_gpr.r_esi;
-    break;
-  case lldb_rsp_x86_64:
-    reg_value = (uint32_t)m_gpr.r_esp;
-    break;
-  case lldb_rbp_x86_64:
-    reg_value = (uint32_t)m_gpr.r_ebp;
-    break;
-  case lldb_rip_x86_64:
-    reg_value = (uint32_t)m_gpr.r_eip;
-    break;
-  case lldb_rflags_x86_64:
-    reg_value = (uint32_t)m_gpr.r_eflags;
-    break;
-  case lldb_cs_x86_64:
-    reg_value = (uint32_t)m_gpr.r_cs;
-    break;
-  case lldb_fs_x86_64:
-    reg_value = (uint16_t)m_gpr.r_fs;
-    break;
-  case lldb_gs_x86_64:
-    reg_value = (uint16_t)m_gpr.r_gs;
-    break;
-  case lldb_ss_x86_64:
-    reg_value = (uint32_t)m_gpr.r_ss;
-    break;
-  case lldb_ds_x86_64:
-    reg_value = (uint16_t)m_gpr.r_ds;
-    break;
-  case lldb_es_x86_64:
-    reg_value = (uint16_t)m_gpr.r_es;
-    break;
-#endif
-#if defined(__x86_64__)
 // the 32-bit field carries more detail, so we don't have to reinvent
 // the wheel
 #define FPR_ENV(x) ((struct envxmm32 *)m_fpr.fpr_env)->x
 #else
 #define FPR_ENV(x) ((struct envxmm *)m_fpr.xmm_env)->x
 #endif
   case lldb_fctrl_x86_64:
     reg_value = (uint16_t)FPR_ENV(en_cw);
if (reg == LLDB_INVALID_REGNUM) {
     // This is likely an internal register for lldb use only and should not be
     // directly queried.
     error.SetErrorStringWithFormat("register \"%s\" is an internal-only lldb "
                                    "register, cannot read directly",
                                    reg_info->name);
     return error;
   }
 
-  int set = GetSetForNativeRegNum(reg);
-  if (set == -1) {
+  llvm::Optional<enum RegSetKind> opt_set = GetSetForNativeRegNum(reg);
+  if (!opt_set) {
     // This is likely an internal register for lldb use only and should not be
     // directly queried.
     error.SetErrorStringWithFormat("register \"%s\" is in unrecognized set",
                                    reg_info->name);
     return error;
   }
 
+  enum RegSetKind set = opt_set.getValue();
+  error = ReadRegisterSet(set);
+  if (error.Fail())
+    return error;
+
+  switch (set) {
+  case GPRegSet:
+    ::memcpy(m_gpr.data() + reg_info->byte_offset, reg_value.GetBytes(),
+             reg_value.GetByteSize());
+    return WriteRegisterSet(set);
+  case FPRegSet:
+  case XSaveRegSet:
+  case DBRegSet:
+    // legacy logic
+    break;
+  }
+
   switch (GetRegisterInfoInterface().GetTargetArchitecture().GetMachine()) {
   case llvm::Triple::x86_64:
     break;
   case llvm::Triple::x86:
     reg = RegNumX86ToX86_64(reg);
     break;
   default:
     llvm_unreachable("Unhandled target architecture.");
   }
 
-  error = ReadRegisterSet(set);
-  if (error.Fail())
-    return error;
-
   switch (reg) {
-#if defined(__x86_64__)
-  case lldb_rax_x86_64:
-    m_gpr.r_rax = reg_value.GetAsUInt64();
-    break;
-  case lldb_rbx_x86_64:
-    m_gpr.r_rbx = reg_value.GetAsUInt64();
-    break;
-  case lldb_rcx_x86_64:
-    m_gpr.r_rcx = reg_value.GetAsUInt64();
-    break;
-  case lldb_rdx_x86_64:
-    m_gpr.r_rdx = reg_value.GetAsUInt64();
-    break;
-  case lldb_rdi_x86_64:
-    m_gpr.r_rdi = reg_value.GetAsUInt64();
-    break;
-  case lldb_rsi_x86_64:
-    m_gpr.r_rsi = reg_value.GetAsUInt64();
-    break;
-  case lldb_rbp_x86_64:
-    m_gpr.r_rbp = reg_value.GetAsUInt64();
-    break;
-  case lldb_rsp_x86_64:
-    m_gpr.r_rsp = reg_value.GetAsUInt64();
-    break;
-  case lldb_r8_x86_64:
-    m_gpr.r_r8 = reg_value.GetAsUInt64();
-    break;
-  case lldb_r9_x86_64:
-    m_gpr.r_r9 = reg_value.GetAsUInt64();
-    break;
-  case lldb_r10_x86_64:
-    m_gpr.r_r10 = reg_value.GetAsUInt64();
-    break;
-  case lldb_r11_x86_64:
-    m_gpr.r_r11 = reg_value.GetAsUInt64();
-    break;
-  case lldb_r12_x86_64:
-    m_gpr.r_r12 = reg_value.GetAsUInt64();
-    break;
-  case lldb_r13_x86_64:
-    m_gpr.r_r13 = reg_value.GetAsUInt64();
-    break;
-  case lldb_r14_x86_64:
-    m_gpr.r_r14 = reg_value.GetAsUInt64();
-    break;
-  case lldb_r15_x86_64:
-    m_gpr.r_r15 = reg_value.GetAsUInt64();
-    break;
-  case lldb_rip_x86_64:
-    m_gpr.r_rip = reg_value.GetAsUInt64();
-    break;
-  case lldb_rflags_x86_64:
-    m_gpr.r_rflags = reg_value.GetAsUInt64();
-    break;
-  case lldb_cs_x86_64:
-    m_gpr.r_cs = reg_value.GetAsUInt64();
-    break;
-  case lldb_fs_x86_64:
-    m_gpr.r_fs = reg_value.GetAsUInt16();
-    break;
-  case lldb_gs_x86_64:
-    m_gpr.r_gs = reg_value.GetAsUInt16();
-    break;
-  case lldb_ss_x86_64:
-    m_gpr.r_ss = reg_value.GetAsUInt64();
-    break;
-  case lldb_ds_x86_64:
-    m_gpr.r_ds = reg_value.GetAsUInt16();
-    break;
-  case lldb_es_x86_64:
-    m_gpr.r_es = reg_value.GetAsUInt16();
-    break;
-#else
-  case lldb_rax_x86_64:
-    m_gpr.r_eax = reg_value.GetAsUInt32();
-    break;
-  case lldb_rbx_x86_64:
-    m_gpr.r_ebx = reg_value.GetAsUInt32();
-    break;
-  case lldb_rcx_x86_64:
-    m_gpr.r_ecx = reg_value.GetAsUInt32();
-    break;
-  case lldb_rdx_x86_64:
-    m_gpr.r_edx = reg_value.GetAsUInt32();
-    break;
-  case lldb_rdi_x86_64:
-    m_gpr.r_edi = reg_value.GetAsUInt32();
-    break;
-  case lldb_rsi_x86_64:
-    m_gpr.r_esi = reg_value.GetAsUInt32();
-    break;
-  case lldb_rsp_x86_64:
-    m_gpr.r_esp = reg_value.GetAsUInt32();
-    break;
-  case lldb_rbp_x86_64:
-    m_gpr.r_ebp = reg_value.GetAsUInt32();
-    break;
-  case lldb_rip_x86_64:
-    m_gpr.r_eip = reg_value.GetAsUInt32();
-    break;
-  case lldb_rflags_x86_64:
-    m_gpr.r_eflags = reg_value.GetAsUInt32();
-    break;
-  case lldb_cs_x86_64:
-    m_gpr.r_cs = reg_value.GetAsUInt32();
-    break;
-  case lldb_fs_x86_64:
-    m_gpr.r_fs = reg_value.GetAsUInt16();
-    break;
-  case lldb_gs_x86_64:
-    m_gpr.r_gs = reg_value.GetAsUInt16();
-    break;
-  case lldb_ss_x86_64:
-    m_gpr.r_ss = reg_value.GetAsUInt32();
-    break;
-  case lldb_ds_x86_64:
-    m_gpr.r_ds = reg_value.GetAsUInt16();
-    break;
-  case lldb_es_x86_64:
-    m_gpr.r_es = reg_value.GetAsUInt16();
-    break;
-#endif
   case lldb_fctrl_x86_64:
     FPR_ENV(en_cw) = reg_value.GetAsUInt16();
     break;
   case lldb_fstat_x86_64:
     FPR_ENV(en_sw) = reg_value.GetAsUInt16();
     break;
   case lldb_ftag_x86_64:
     FPR_ENV(en_tw) = reg_value.GetAsUInt16();
Status NativeRegisterContextFreeBSD_x86_64::ReadAllRegisterValues(
   Status error;
 
   data_sp.reset(new DataBufferHeap(REG_CONTEXT_SIZE, 0));
   error = ReadRegisterSet(GPRegSet);
   if (error.Fail())
     return error;
 
   uint8_t *dst = data_sp->GetBytes();
-  ::memcpy(dst, &m_gpr, GetRegisterInfoInterface().GetGPRSize());
+  ::memcpy(dst, m_gpr.data(), GetRegisterInfoInterface().GetGPRSize());
   dst += GetRegisterInfoInterface().GetGPRSize();
 
   return error;
 }
 
 Status NativeRegisterContextFreeBSD_x86_64::WriteAllRegisterValues(
     const lldb::DataBufferSP &data_sp) {
   Status error;
Status NativeRegisterContextFreeBSD_x86_64::WriteAllRegisterValues(
   uint8_t *src = data_sp->GetBytes();
   if (src == nullptr) {
     error.SetErrorStringWithFormat("NativeRegisterContextFreeBSD_x86_64::%s "
                                    "DataBuffer::GetBytes() returned a null "
                                    "pointer",
                                    __FUNCTION__);
     return error;
   }
-  ::memcpy(&m_gpr, src, GetRegisterInfoInterface().GetGPRSize());
+  ::memcpy(m_gpr.data(), src, GetRegisterInfoInterface().GetGPRSize());
 
   error = WriteRegisterSet(GPRegSet);
   if (error.Fail())
     return error;
   src += GetRegisterInfoInterface().GetGPRSize();
 
   return error;
 }

lldb/unittests/Process/CMakeLists.txt

 add_subdirectory(gdb-remote)
 if (CMAKE_SYSTEM_NAME MATCHES "Linux|Android")
   add_subdirectory(Linux)
   add_subdirectory(POSIX)
 endif()
+add_subdirectory(Utility)
 add_subdirectory(minidump)
 
 add_lldb_unittest(ProcessEventDataTests
   ProcessEventDataTest.cpp
 
   LINK_LIBS
       lldbCore
       lldbHost
       lldbTarget
       lldbSymbol
       lldbUtility
       lldbUtilityHelpers
       lldbInterpreter
       lldbPluginPlatformMacOSX
   )

lldb/unittests/Process/Utility/CMakeLists.txt

+add_lldb_unittest(ProcessUtilityTests
+  RegisterContextFreeBSDTest.cpp
+
+  LINK_LIBS
+    lldbPluginProcessUtility)

lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp

+//===-- RegisterContextFreeBSDTests.cpp -----------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#if defined(__FreeBSD__)
+
+// clang-format off
+#include <sys/types.h>
+#include <machine/reg.h>
+// clang-format on
+
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+#include "Plugins/Process/Utility/lldb-x86-register-enums.h"
+#include "Plugins/Process/Utility/RegisterContextFreeBSD_i386.h"
+#include "Plugins/Process/Utility/RegisterContextFreeBSD_x86_64.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+std::pair<size_t, size_t> GetRegParams(RegisterInfoInterface &ctx,
+                                       uint32_t reg) {
+  const RegisterInfo &info = ctx.GetRegisterInfo()[reg];
+  return {info.byte_offset, info.byte_size};
+}
+
+#if defined(__x86_64__)
+
+#define EXPECT_GPR_X86_64(regname)                                             \
+  EXPECT_THAT(                                                                 \
+      GetRegParams(reg_ctx, lldb_##regname##_x86_64),                          \
+      ::testing::Pair(offsetof(reg, r_##regname), sizeof(reg::r_##regname)))
+
+TEST(RegisterContextFreeBSDTest, x86_64) {
+  ArchSpec arch{"x86_64-unknown-freebsd"};
+  RegisterContextFreeBSD_x86_64 reg_ctx{arch};
+
+  EXPECT_GPR_X86_64(r15);
+  EXPECT_GPR_X86_64(r14);
+  EXPECT_GPR_X86_64(r13);
+  EXPECT_GPR_X86_64(r12);
+  EXPECT_GPR_X86_64(r11);
+  EXPECT_GPR_X86_64(r10);
+  EXPECT_GPR_X86_64(r9);
+  EXPECT_GPR_X86_64(r8);
+  EXPECT_GPR_X86_64(rdi);
+  EXPECT_GPR_X86_64(rsi);
+  EXPECT_GPR_X86_64(rbp);
+  EXPECT_GPR_X86_64(rbx);
+  EXPECT_GPR_X86_64(rdx);
+  EXPECT_GPR_X86_64(rcx);
+  EXPECT_GPR_X86_64(rax);
+  EXPECT_GPR_X86_64(fs);
+  EXPECT_GPR_X86_64(gs);
+  EXPECT_GPR_X86_64(es);
+  EXPECT_GPR_X86_64(ds);
+  EXPECT_GPR_X86_64(rip);
+  EXPECT_GPR_X86_64(cs);
+  EXPECT_GPR_X86_64(rflags);
+  EXPECT_GPR_X86_64(rsp);
+  EXPECT_GPR_X86_64(ss);
+}
+
+#endif // defined(__x86_64__)
+
+#if defined(__i386__) || defined(__x86_64__)
+
+#define EXPECT_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)))
+
+TEST(RegisterContextFreeBSDTest, i386) {
+  ArchSpec arch{"i686-unknown-freebsd"};
+  RegisterContextFreeBSD_i386 reg_ctx{arch};
+
+#if defined(__i386__)
+  using native_i386_regs = ::reg;
+#else
+  using native_i386_regs = ::reg32;
+#endif
+
+  EXPECT_GPR_I386(fs);
+  EXPECT_GPR_I386(es);
+  EXPECT_GPR_I386(ds);
+  EXPECT_GPR_I386(edi);
+  EXPECT_GPR_I386(esi);
+  EXPECT_GPR_I386(ebp);
+  EXPECT_GPR_I386(ebx);
+  EXPECT_GPR_I386(edx);
+  EXPECT_GPR_I386(ecx);
+  EXPECT_GPR_I386(eax);
+  EXPECT_GPR_I386(eip);
+  EXPECT_GPR_I386(cs);
+  EXPECT_GPR_I386(eflags);
+  EXPECT_GPR_I386(esp);
+  EXPECT_GPR_I386(ss);
+  EXPECT_GPR_I386(gs);
+}
+
+#endif // defined(__i386__) || defined(__x86_64__)
+
+#endif // defined(__FreeBSD__)