[lldb] [Process/FreeBSD] Rework arm64 register access

[andrew]

Summary

To simplify future register access rework the FreeBSD arm64 register
read/write functions to be similar to on Linux.

This will simplify adding more register sets, e.g. for Pointer
Authentication.

[mgorny]

lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp

73–77

These helpers don't seem very helpful. Isn't it better to use switch on the value returned by GetRegisterInfo().GetRegisterSetFromRegisterIndex(reg)?

133

To be honest, it seems counterproductive to restore the duplication that I've removed before. Is there any real advantage to having two methods here, and calling them separately from inside the if instead of calling ReadRegisterSet(set) before the if?

[andrew]

lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp

73–77

Using a switch will be messy when adding pointer authentication or MTE registers as we need to call RegisterInfoPOSIX_arm64::IsPAuthReg [1] to check if a given register is a pointer authentication register, and similar for MTE. This is because they are dynamically added as needed.

[1] https://github.com/llvm/llvm-project/blob/76e47d4/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp#L392-L396

133

Because of the dynamic nature of the register sets we may not have a usable value to pass into ReadRegisterSet and any logic to decide which register set is being read in ReadRegisterSet would need to be repeated here and below when writing a register.

Calling the function ReadRegisterSet will also be confusing when I add PT_GETREGSET and PT_SETREGSET [1] to FreeBSD. This will be used to access the pointer authentication address masks on FreeBSD (and future registers, e.g. MTE & SVE).

See [2] for a WIP patch with how I plan to extend this for code & data address masks needed by pointer authentication.

[1] https://reviews.freebsd.org/D19831
[2] https://github.com/zxombie/llvm-project/commit/b801eca#diff-3d6a219398418eb59bf8104bda44a111f47d29081e076b4ce69336882e482cc2

[andrew]

Rebase

lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD.h

 //===-- NativeRegisterContextFreeBSD.h --------------------------*- C++ -*-===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
public:
   CreateHostNativeRegisterContextFreeBSD(const ArchSpec &target_arch,
                                          NativeThreadProtocol &native_thread);
   virtual llvm::Error
   CopyHardwareWatchpointsFrom(NativeRegisterContextFreeBSD &source) = 0;
 
 protected:
   virtual NativeProcessFreeBSD &GetProcess();
   virtual ::pid_t GetProcessPid();
+
+  virtual size_t GetGPRSize() const {
+    return GetRegisterInfoInterface().GetGPRSize();
+  }
 };
 
 } // namespace process_freebsd
 } // namespace lldb_private
 
 #endif // #ifndef lldb_NativeRegisterContextFreeBSD_h

lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.h

 //===-- NativeRegisterContextFreeBSD_arm64.h --------------------*- C++ -*-===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
public:
   llvm::Error
   CopyHardwareWatchpointsFrom(NativeRegisterContextFreeBSD &source) override;
 
 private:
   // Due to alignment, FreeBSD reg/fpreg are a few bytes larger than
   // LLDB's GPR/FPU structs.  However, all fields have matching offsets
   // and sizes, so we do not have to worry about these (and we have
   // a unittest to assert that).
-  std::array<uint8_t, sizeof(reg) + sizeof(fpreg)> m_reg_data;
+  std::array<uint8_t, sizeof(reg)> m_reg_data;
+  std::array<uint8_t, sizeof(fpreg)> m_fpreg_data;
 #ifdef LLDB_HAS_FREEBSD_WATCHPOINT
   dbreg m_dbreg;
   bool m_read_dbreg;
 #endif
 
-  Status ReadRegisterSet(uint32_t set);
-  Status WriteRegisterSet(uint32_t set);
+  void *GetGPRBuffer() { return &m_reg_data; }
+  size_t GetGPRBufferSize() { return sizeof(m_reg_data); }
+
+  void *GetFPRBuffer() { return &m_fpreg_data; }
+  size_t GetFPRSize() { return sizeof(m_fpreg_data); }
+
+  bool IsGPR(unsigned reg) const;
+  bool IsFPR(unsigned reg) const;
+
+  Status ReadGPR();
+  Status WriteGPR();
+
+  Status ReadFPR();
+  Status WriteFPR();
+
+  uint32_t CalculateFprOffset(const RegisterInfo *reg_info) const;
 
   llvm::Error ReadHardwareDebugInfo() override;
   llvm::Error WriteHardwareDebugRegs(DREGType hwbType) override;
 
   RegisterInfoPOSIX_arm64 &GetRegisterInfo() const;
 };
 
 } // namespace process_freebsd
 } // namespace lldb_private
 
 #endif // #ifndef lldb_NativeRegisterContextFreeBSD_arm64_h
 
 #endif // defined (__aarch64__)

lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp

 //===-- NativeRegisterContextFreeBSD_arm64.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
 //
 //===----------------------------------------------------------------------===//
 

 #include "Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h"
 
 // clang-format off
 #include <sys/param.h>
 #include <sys/ptrace.h>
 #include <sys/types.h>
 // clang-format on
 
+#define REG_CONTEXT_SIZE (GetGPRSize() + GetFPRSize())
+
 using namespace lldb;
 using namespace lldb_private;
 using namespace lldb_private::process_freebsd;
 
 NativeRegisterContextFreeBSD *
 NativeRegisterContextFreeBSD::CreateHostNativeRegisterContextFreeBSD(
     const ArchSpec &target_arch, NativeThreadProtocol &native_thread) {
   return new NativeRegisterContextFreeBSD_arm64(target_arch, native_thread);

 
 uint32_t NativeRegisterContextFreeBSD_arm64::GetUserRegisterCount() const {
   uint32_t count = 0;
   for (uint32_t set_index = 0; set_index < GetRegisterSetCount(); ++set_index)
     count += GetRegisterSet(set_index)->num_registers;
   return count;
 }
 
-Status NativeRegisterContextFreeBSD_arm64::ReadRegisterSet(uint32_t set) {
-  switch (set) {
-  case RegisterInfoPOSIX_arm64::GPRegSet:
-    return NativeProcessFreeBSD::PtraceWrapper(PT_GETREGS, m_thread.GetID(),
-                                               m_reg_data.data());
-  case RegisterInfoPOSIX_arm64::FPRegSet:
-    return NativeProcessFreeBSD::PtraceWrapper(
-        PT_GETFPREGS, m_thread.GetID(),
-        m_reg_data.data() + sizeof(RegisterInfoPOSIX_arm64::GPR));
-  }
-  llvm_unreachable("NativeRegisterContextFreeBSD_arm64::ReadRegisterSet");
+bool NativeRegisterContextFreeBSD_arm64::IsGPR(unsigned reg) const {
+  if (GetRegisterInfo().GetRegisterSetFromRegisterIndex(reg) ==
+      RegisterInfoPOSIX_arm64::GPRegSet)
+    return true;
+  return false;
+}
+
+bool NativeRegisterContextFreeBSD_arm64::IsFPR(unsigned reg) const {
+  if (GetRegisterInfo().GetRegisterSetFromRegisterIndex(reg) ==
+      RegisterInfoPOSIX_arm64::FPRegSet)
+    return true;
+  return false;
+}
+
+Status NativeRegisterContextFreeBSD_arm64::ReadGPR() {
+  return NativeProcessFreeBSD::PtraceWrapper(
+      PT_GETREGS, m_thread.GetID(), m_reg_data.data());
+}
+
+Status NativeRegisterContextFreeBSD_arm64::WriteGPR() {
+      PT_SETREGS, m_thread.GetID(), m_reg_data.data());
+
+Status NativeRegisterContextFreeBSD_arm64::ReadFPR() {
+  return NativeProcessFreeBSD::PtraceWrapper(
+      PT_GETFPREGS, m_thread.GetID(), m_fpreg_data.data());
 }
 
-Status NativeRegisterContextFreeBSD_arm64::WriteRegisterSet(uint32_t set) {
-  switch (set) {
-  case RegisterInfoPOSIX_arm64::GPRegSet:
-    return NativeProcessFreeBSD::PtraceWrapper(PT_SETREGS, m_thread.GetID(),
-                                               m_reg_data.data());
-  case RegisterInfoPOSIX_arm64::FPRegSet:
-    return NativeProcessFreeBSD::PtraceWrapper(
-        PT_SETFPREGS, m_thread.GetID(),
-        m_reg_data.data() + sizeof(RegisterInfoPOSIX_arm64::GPR));
-  }
-  llvm_unreachable("NativeRegisterContextFreeBSD_arm64::WriteRegisterSet");
+Status NativeRegisterContextFreeBSD_arm64::WriteFPR() {
+      PT_SETFPREGS, m_thread.GetID(), m_fpreg_data.data());
+
+uint32_t NativeRegisterContextFreeBSD_arm64::CalculateFprOffset(
+    const RegisterInfo *reg_info) const {
+  return reg_info->byte_offset - GetGPRSize();
 }
 
 Status
 NativeRegisterContextFreeBSD_arm64::ReadRegister(const RegisterInfo *reg_info,
                                                  RegisterValue &reg_value) {
   Status error;
 
   if (!reg_info) {
     error.SetErrorString("reg_info NULL");
     return error;
   }
 
   const uint32_t reg = reg_info->kinds[lldb::eRegisterKindLLDB];
 
   if (reg == LLDB_INVALID_REGNUM)
     return Status("no lldb regnum for %s", reg_info && reg_info->name
                                                ? reg_info->name
                                                : "<unknown register>");
 
-  uint32_t set = GetRegisterInfo().GetRegisterSetFromRegisterIndex(reg);
-  error = ReadRegisterSet(set);
+  uint8_t *src;
+  uint32_t offset = LLDB_INVALID_INDEX32;
+
+  if (IsGPR(reg)) {
+    error = ReadGPR();
+    if (error.Fail())
+      return error;
+
+    offset = reg_info->byte_offset;
+    assert(offset < GetGPRSize());
+    src = (uint8_t *)GetGPRBuffer() + offset;
+  } else if (IsFPR(reg)) {
+    error = ReadFPR();
     if (error.Fail())
       return error;
 
-  assert(reg_info->byte_offset + reg_info->byte_size <= m_reg_data.size());
-  reg_value.SetBytes(m_reg_data.data() + reg_info->byte_offset,
-                     reg_info->byte_size, endian::InlHostByteOrder());
+    offset = CalculateFprOffset(reg_info);
+    assert(offset < GetFPRSize());
+    src = (uint8_t *)GetFPRBuffer() + offset;
+  } else
+    return Status("Failed to read register value");
+
+  reg_value.SetFromMemoryData(reg_info, src, reg_info->byte_size,
+                              endian::InlHostByteOrder(), error);
   return error;
 }
 
 Status NativeRegisterContextFreeBSD_arm64::WriteRegister(
     const RegisterInfo *reg_info, const RegisterValue &reg_value) {
   Status error;
 
   if (!reg_info)
     return Status("reg_info NULL");
 
   const uint32_t reg = reg_info->kinds[lldb::eRegisterKindLLDB];
 
   if (reg == LLDB_INVALID_REGNUM)
     return Status("no lldb regnum for %s", reg_info && reg_info->name
                                                ? reg_info->name
                                                : "<unknown register>");
 
-  uint32_t set = GetRegisterInfo().GetRegisterSetFromRegisterIndex(reg);
-  error = ReadRegisterSet(set);
+  uint8_t *dst;
+  uint32_t offset = LLDB_INVALID_INDEX32;
+
+  if (IsGPR(reg)) {
+    error = ReadGPR();
     if (error.Fail())
       return error;
 
-  assert(reg_info->byte_offset + reg_info->byte_size <= m_reg_data.size());
-  ::memcpy(m_reg_data.data() + reg_info->byte_offset, reg_value.GetBytes(),
-           reg_info->byte_size);
+    assert(reg_info->byte_offset < GetGPRSize());
+    dst = (uint8_t *)GetGPRBuffer() + reg_info->byte_offset;
+    ::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size);
+
+    return WriteGPR();
+  } else if (IsFPR(reg)) {
+    error = ReadFPR();
+    if (error.Fail())
+      return error;
+
+    offset = CalculateFprOffset(reg_info);
+    assert(offset < GetFPRSize());
+    dst = (uint8_t *)GetFPRBuffer() + offset;
+    ::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size);
+
+    return WriteFPR();
+  }
 
-  return WriteRegisterSet(set);
+  return Status("Failed to write register value");
 }
 
 Status NativeRegisterContextFreeBSD_arm64::ReadAllRegisterValues(
     lldb::DataBufferSP &data_sp) {
   Status error;
 
-  error = ReadRegisterSet(RegisterInfoPOSIX_arm64::GPRegSet);
+  data_sp.reset(new DataBufferHeap(REG_CONTEXT_SIZE, 0));
+
+  error = ReadGPR();
   if (error.Fail())
     return error;
 
-  error = ReadRegisterSet(RegisterInfoPOSIX_arm64::FPRegSet);
+  error = ReadFPR();
   if (error.Fail())
     return error;
 
-  data_sp.reset(new DataBufferHeap(m_reg_data.size(), 0));
   uint8_t *dst = data_sp->GetBytes();
-  ::memcpy(dst, m_reg_data.data(), m_reg_data.size());
+  ::memcpy(dst, GetGPRBuffer(), GetGPRSize());
+  dst += GetGPRSize();
+  ::memcpy(dst, GetFPRBuffer(), GetFPRSize());
 
   return error;
 }
 
 Status NativeRegisterContextFreeBSD_arm64::WriteAllRegisterValues(
     const lldb::DataBufferSP &data_sp) {
   Status error;
 
   if (!data_sp) {
     error.SetErrorStringWithFormat(
         "NativeRegisterContextFreeBSD_arm64::%s invalid data_sp provided",
         __FUNCTION__);
     return error;
   }
 
-  if (data_sp->GetByteSize() != m_reg_data.size()) {
+  if (data_sp->GetByteSize() != REG_CONTEXT_SIZE) {
     error.SetErrorStringWithFormat(
         "NativeRegisterContextFreeBSD_arm64::%s data_sp contained mismatched "
         "data size, expected %" PRIu64 ", actual %" PRIu64,
         __FUNCTION__, m_reg_data.size(), data_sp->GetByteSize());
     return error;
   }
 
   uint8_t *src = data_sp->GetBytes();
   if (src == nullptr) {
     error.SetErrorStringWithFormat("NativeRegisterContextFreeBSD_arm64::%s "
                                    "DataBuffer::GetBytes() returned a null "
                                    "pointer",
                                    __FUNCTION__);
     return error;
   }
-  ::memcpy(m_reg_data.data(), src, m_reg_data.size());
+  ::memcpy(GetGPRBuffer(), src, GetRegisterInfoInterface().GetGPRSize());
 
-  error = WriteRegisterSet(RegisterInfoPOSIX_arm64::GPRegSet);
+  error = WriteGPR();
   if (error.Fail())
     return error;
 
-  return WriteRegisterSet(RegisterInfoPOSIX_arm64::FPRegSet);
+  src += GetRegisterInfoInterface().GetGPRSize();
+  ::memcpy(GetFPRBuffer(), src, GetFPRSize());
+
+  error = WriteFPR();
+  if (error.Fail())
+    return error;
+
+  return error;
 }
 
 llvm::Error NativeRegisterContextFreeBSD_arm64::CopyHardwareWatchpointsFrom(
     NativeRegisterContextFreeBSD &source) {
 #ifdef LLDB_HAS_FREEBSD_WATCHPOINT
   auto &r_source = static_cast<NativeRegisterContextFreeBSD_arm64 &>(source);
   llvm::Error error = r_source.ReadHardwareDebugInfo();
   if (error)