[lldb][RISCV] Support optionally disabled FPR for riscv64 (#104547)

The PR adds the support optionally enabled/disabled FP-registers to LLDB
`RegisterInfoPOSIX_riscv64`. This situation might take place for RISC-V
builds having no FP-registers, like RV64IMAC or RV64IMACV.

To aim this, patch adds `opt_regsets` flags mechanism. It re-works
RegisterInfo class to work with flexibly allocated (depending on
`opt_regsets` flag) `m_register_sets` and `m_register_infos` vectors
instead of statically defined structures. The registration of regsets is
being arranged by `m_per_regset_regnum_range` map.

The patch flows are spread to `NativeRegisterContextLinux_riscv64` and
`RegisterContextCorePOSIX_riscv64` classes, that were tested on:
 - x86_64 host working with coredumps
- RV64GC and RV64IMAC targets working with coredumps and natively in
run-time with binaries

`EmulateInstructionRISCV` is out of scope of this patch, and its
behavior did not change, using maximum set of registers.

According testcase built for RV64IMAC (no-FPR) was added to
`TestLinuxCore.py`.

Author: AlexeyMerzlyakov

lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp

@@ -1748,10 +1748,10 @@ EmulateInstructionRISCV::GetRegisterInfo(RegisterKind reg_kind,
     }
   }
 
-  const RegisterInfo *array =
-      RegisterInfoPOSIX_riscv64::GetRegisterInfoPtr(m_arch);
-  const uint32_t length =
-      RegisterInfoPOSIX_riscv64::GetRegisterInfoCount(m_arch);
+  RegisterInfoPOSIX_riscv64 reg_info(m_arch,
+                                     RegisterInfoPOSIX_riscv64::eRegsetMaskAll);
+  const RegisterInfo *array = reg_info.GetRegisterInfo();
+  const uint32_t length = reg_info.GetRegisterCount();
 
   if (reg_index >= length || reg_kind != eRegisterKindLLDB)
     return {};

lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_riscv64.cpp

@@ -23,12 +23,11 @@
 
 // System includes - They have to be included after framework includes because
 // they define some macros which collide with variable names in other modules
+#include <sys/ptrace.h>
 #include <sys/uio.h>
 // NT_PRSTATUS and NT_FPREGSET definition
 #include <elf.h>
 
-#define REG_CONTEXT_SIZE (GetGPRSize() + GetFPRSize())
-
 using namespace lldb;
 using namespace lldb_private;
 using namespace lldb_private::process_linux;
@@ -38,7 +37,21 @@ NativeRegisterContextLinux::CreateHostNativeRegisterContextLinux(
     const ArchSpec &target_arch, NativeThreadLinux &native_thread) {
   switch (target_arch.GetMachine()) {
   case llvm::Triple::riscv64: {
-    Flags opt_regsets;
+    Flags opt_regsets(RegisterInfoPOSIX_riscv64::eRegsetMaskDefault);
+
+    RegisterInfoPOSIX_riscv64::FPR fpr;
+    struct iovec ioVec;
+    ioVec.iov_base = &fpr;
+    ioVec.iov_len = sizeof(fpr);
+    unsigned int regset = NT_FPREGSET;
+
+    if (NativeProcessLinux::PtraceWrapper(PTRACE_GETREGSET,
+                                          native_thread.GetID(), &regset,
+                                          &ioVec, sizeof(fpr))
+            .Success()) {
+      opt_regsets.Set(RegisterInfoPOSIX_riscv64::eRegsetMaskFP);
+    }
+
     auto register_info_up =
         std::make_unique<RegisterInfoPOSIX_riscv64>(target_arch, opt_regsets);
     return std::make_unique<NativeRegisterContextLinux_riscv64>(
@@ -194,20 +207,23 @@ Status NativeRegisterContextLinux_riscv64::ReadAllRegisterValues(
     lldb::WritableDataBufferSP &data_sp) {
   Status error;
 
-  data_sp.reset(new DataBufferHeap(REG_CONTEXT_SIZE, 0));
+  data_sp.reset(new DataBufferHeap(GetRegContextSize(), 0));
 
   error = ReadGPR();
   if (error.Fail())
     return error;
 
-  error = ReadFPR();
-  if (error.Fail())
-    return error;
+  if (GetRegisterInfo().IsFPPresent()) {
+    error = ReadFPR();
+    if (error.Fail())
+      return error;
+  }
 
   uint8_t *dst = const_cast<uint8_t *>(data_sp->GetBytes());
   ::memcpy(dst, GetGPRBuffer(), GetGPRSize());
   dst += GetGPRSize();
-  ::memcpy(dst, GetFPRBuffer(), GetFPRSize());
+  if (GetRegisterInfo().IsFPPresent())
+    ::memcpy(dst, GetFPRBuffer(), GetFPRSize());
 
   return error;
 }
@@ -223,11 +239,11 @@ Status NativeRegisterContextLinux_riscv64::WriteAllRegisterValues(
     return error;
   }
 
-  if (data_sp->GetByteSize() != REG_CONTEXT_SIZE) {
+  if (data_sp->GetByteSize() != GetRegContextSize()) {
     error = Status::FromErrorStringWithFormat(
         "NativeRegisterContextLinux_riscv64::%s data_sp contained mismatched "
         "data size, expected %" PRIu64 ", actual %" PRIu64,
-        __FUNCTION__, REG_CONTEXT_SIZE, data_sp->GetByteSize());
+        __FUNCTION__, GetRegContextSize(), data_sp->GetByteSize());
     return error;
   }
 
@@ -247,23 +263,32 @@ Status NativeRegisterContextLinux_riscv64::WriteAllRegisterValues(
     return error;
 
   src += GetRegisterInfoInterface().GetGPRSize();
-  ::memcpy(GetFPRBuffer(), src, GetFPRSize());
 
-  error = WriteFPR();
-  if (error.Fail())
-    return error;
+  if (GetRegisterInfo().IsFPPresent()) {
+    ::memcpy(GetFPRBuffer(), src, GetFPRSize());
+
+    error = WriteFPR();
+    if (error.Fail())
+      return error;
+  }
 
   return error;
 }
 
+size_t NativeRegisterContextLinux_riscv64::GetRegContextSize() {
+  size_t size = GetGPRSize();
+  if (GetRegisterInfo().IsFPPresent())
+    size += GetFPRSize();
+  return size;
+}
+
 bool NativeRegisterContextLinux_riscv64::IsGPR(unsigned reg) const {
   return GetRegisterInfo().GetRegisterSetFromRegisterIndex(reg) ==
          RegisterInfoPOSIX_riscv64::GPRegSet;
 }
 
 bool NativeRegisterContextLinux_riscv64::IsFPR(unsigned reg) const {
-  return GetRegisterInfo().GetRegisterSetFromRegisterIndex(reg) ==
-         RegisterInfoPOSIX_riscv64::FPRegSet;
+  return GetRegisterInfo().IsFPReg(reg);
 }
 
 Status NativeRegisterContextLinux_riscv64::ReadGPR() {

lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_riscv64.h

@@ -75,6 +75,8 @@ class NativeRegisterContextLinux_riscv64 : public NativeRegisterContextLinux {
 
   RegisterInfoPOSIX_riscv64::FPR m_fpr;
 
+  size_t GetRegContextSize();
+
   bool IsGPR(unsigned reg) const;
 
   bool IsFPR(unsigned reg) const;

lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_riscv64.cpp

@@ -77,6 +77,5 @@ bool RegisterContextPOSIX_riscv64::IsGPR(unsigned int reg) {
 }
 
 bool RegisterContextPOSIX_riscv64::IsFPR(unsigned int reg) {
-  return m_register_info_up->GetRegisterSetFromRegisterIndex(reg) ==
-         RegisterInfoPOSIX_riscv64::FPRegSet;
+  return m_register_info_up->IsFPReg(reg);
 }

lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_riscv64.cpp

@@ -18,42 +18,15 @@
 #define GPR_OFFSET(idx) ((idx)*8 + 0)
 #define FPR_OFFSET(idx) ((idx)*8 + sizeof(RegisterInfoPOSIX_riscv64::GPR))
 
-#define REG_CONTEXT_SIZE                                                       \
-  (sizeof(RegisterInfoPOSIX_riscv64::GPR) +                                    \
-   sizeof(RegisterInfoPOSIX_riscv64::FPR))
-
 #define DECLARE_REGISTER_INFOS_RISCV64_STRUCT
 #include "RegisterInfos_riscv64.h"
 #undef DECLARE_REGISTER_INFOS_RISCV64_STRUCT
 
-const lldb_private::RegisterInfo *RegisterInfoPOSIX_riscv64::GetRegisterInfoPtr(
-    const lldb_private::ArchSpec &target_arch) {
-  switch (target_arch.GetMachine()) {
-  case llvm::Triple::riscv64:
-    return g_register_infos_riscv64_le;
-  default:
-    assert(false && "Unhandled target architecture.");
-    return nullptr;
-  }
-}
-
-uint32_t RegisterInfoPOSIX_riscv64::GetRegisterInfoCount(
-    const lldb_private::ArchSpec &target_arch) {
-  switch (target_arch.GetMachine()) {
-  case llvm::Triple::riscv64:
-    return static_cast<uint32_t>(sizeof(g_register_infos_riscv64_le) /
-                                 sizeof(g_register_infos_riscv64_le[0]));
-  default:
-    assert(false && "Unhandled target architecture.");
-    return 0;
-  }
-}
-
 // Number of register sets provided by this context.
 enum {
   k_num_gpr_registers = gpr_last_riscv - gpr_first_riscv + 1,
   k_num_fpr_registers = fpr_last_riscv - fpr_first_riscv + 1,
-  k_num_register_sets = 2
+  k_num_register_sets_default = 1
 };
 
 // RISC-V64 general purpose registers.
@@ -73,38 +46,69 @@ static_assert(((sizeof g_gpr_regnums_riscv64 /
                1) == k_num_gpr_registers,
               "g_gpr_regnums_riscv64 has wrong number of register infos");
 
-// RISC-V64 floating point registers.
-static const uint32_t g_fpr_regnums_riscv64[] = {
-    fpr_f0_riscv,   fpr_f1_riscv,       fpr_f2_riscv,  fpr_f3_riscv,
-    fpr_f4_riscv,   fpr_f5_riscv,       fpr_f6_riscv,  fpr_f7_riscv,
-    fpr_f8_riscv,   fpr_f9_riscv,       fpr_f10_riscv, fpr_f11_riscv,
-    fpr_f12_riscv,  fpr_f13_riscv,      fpr_f14_riscv, fpr_f15_riscv,
-    fpr_f16_riscv,  fpr_f17_riscv,      fpr_f18_riscv, fpr_f19_riscv,
-    fpr_f20_riscv,  fpr_f21_riscv,      fpr_f22_riscv, fpr_f23_riscv,
-    fpr_f24_riscv,  fpr_f25_riscv,      fpr_f26_riscv, fpr_f27_riscv,
-    fpr_f28_riscv,  fpr_f29_riscv,      fpr_f30_riscv, fpr_f31_riscv,
-    fpr_fcsr_riscv, LLDB_INVALID_REGNUM};
-
-static_assert(((sizeof g_fpr_regnums_riscv64 /
-                sizeof g_fpr_regnums_riscv64[0]) -
-               1) == k_num_fpr_registers,
-              "g_fpr_regnums_riscv64 has wrong number of register infos");
-
 // Register sets for RISC-V64.
-static const lldb_private::RegisterSet g_reg_sets_riscv64[k_num_register_sets] =
-    {{"General Purpose Registers", "gpr", k_num_gpr_registers,
-      g_gpr_regnums_riscv64},
-     {"Floating Point Registers", "fpr", k_num_fpr_registers,
-      g_fpr_regnums_riscv64}};
+static const lldb_private::RegisterSet g_reg_set_gpr_riscv64 = {
+    "General Purpose Registers", "gpr", k_num_gpr_registers,
+    g_gpr_regnums_riscv64};
+static const lldb_private::RegisterSet g_reg_set_fpr_riscv64 = {
+    "Floating Point Registers", "fpr", k_num_fpr_registers, nullptr};
 
 RegisterInfoPOSIX_riscv64::RegisterInfoPOSIX_riscv64(
-    const lldb_private::ArchSpec &target_arch, lldb_private::Flags flags)
+    const lldb_private::ArchSpec &target_arch, lldb_private::Flags opt_regsets)
     : lldb_private::RegisterInfoAndSetInterface(target_arch),
-      m_register_info_p(GetRegisterInfoPtr(target_arch)),
-      m_register_info_count(GetRegisterInfoCount(target_arch)) {}
+      m_opt_regsets(opt_regsets) {
+  switch (target_arch.GetMachine()) {
+  case llvm::Triple::riscv64: {
+    // By-default considering RISC-V has only GPR.
+    // Other register sets could be enabled optionally by opt_regsets.
+    AddRegSetGP();
+
+    if (m_opt_regsets.AnySet(eRegsetMaskFP))
+      AddRegSetFP();
+
+    break;
+  }
+  default:
+    assert(false && "Unhandled target architecture.");
+  }
+}
+
+void RegisterInfoPOSIX_riscv64::AddRegSetGP() {
+  m_register_infos.resize(k_num_gpr_registers);
+  memcpy(&m_register_infos[0], g_register_infos_riscv64_gpr,
+         sizeof(g_register_infos_riscv64_gpr));
+  m_register_sets.push_back(g_reg_set_gpr_riscv64);
+
+  m_per_regset_regnum_range[GPRegSet] =
+      std::make_pair(gpr_first_riscv, m_register_infos.size());
+}
+
+void RegisterInfoPOSIX_riscv64::AddRegSetFP() {
+  const uint32_t register_info_count = m_register_infos.size();
+  const uint32_t register_set_count = m_register_sets.size();
+
+  // Filling m_register_infos.
+  // For FPR case we do not need to correct register offsets and kinds
+  // while for other further cases (like VPR), register offset/kind
+  // should be started counting from the last one in previously added
+  // regset. This is needed for the case e.g. when architecture has GPR + VPR
+  // sets only.
+  m_register_infos.resize(register_info_count + k_num_fpr_registers);
+  memcpy(&m_register_infos[register_info_count], g_register_infos_riscv64_fpr,
+         sizeof(g_register_infos_riscv64_fpr));
+
+  // Filling m_register_sets with enabled register set
+  for (uint32_t i = 0; i < k_num_fpr_registers; i++)
+    m_fp_regnum_collection.push_back(register_info_count + i);
+  m_register_sets.push_back(g_reg_set_fpr_riscv64);
+  m_register_sets.back().registers = m_fp_regnum_collection.data();
+
+  m_per_regset_regnum_range[register_set_count] =
+      std::make_pair(register_info_count, m_register_infos.size());
+}
 
 uint32_t RegisterInfoPOSIX_riscv64::GetRegisterCount() const {
-  return m_register_info_count;
+  return m_register_infos.size();
 }
 
 size_t RegisterInfoPOSIX_riscv64::GetGPRSize() const {
@@ -117,26 +121,30 @@ size_t RegisterInfoPOSIX_riscv64::GetFPRSize() const {
 
 const lldb_private::RegisterInfo *
 RegisterInfoPOSIX_riscv64::GetRegisterInfo() const {
-  return m_register_info_p;
+  return m_register_infos.data();
 }
 
 size_t RegisterInfoPOSIX_riscv64::GetRegisterSetCount() const {
-  return k_num_register_sets;
+  return m_register_sets.size();
 }
 
 size_t RegisterInfoPOSIX_riscv64::GetRegisterSetFromRegisterIndex(
     uint32_t reg_index) const {
-  // coverity[unsigned_compare]
-  if (reg_index >= gpr_first_riscv && reg_index <= gpr_last_riscv)
-    return GPRegSet;
-  if (reg_index >= fpr_first_riscv && reg_index <= fpr_last_riscv)
-    return FPRegSet;
+  for (const auto &regset_range : m_per_regset_regnum_range) {
+    if (reg_index >= regset_range.second.first &&
+        reg_index < regset_range.second.second)
+      return regset_range.first;
+  }
   return LLDB_INVALID_REGNUM;
 }
 
+bool RegisterInfoPOSIX_riscv64::IsFPReg(unsigned reg) const {
+  return llvm::is_contained(m_fp_regnum_collection, reg);
+}
+
 const lldb_private::RegisterSet *
 RegisterInfoPOSIX_riscv64::GetRegisterSet(size_t set_index) const {
   if (set_index < GetRegisterSetCount())
-    return &g_reg_sets_riscv64[set_index];
+    return &m_register_sets[set_index];
   return nullptr;
 }

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

@@ -11,19 +11,21 @@
 
 #include "RegisterInfoAndSetInterface.h"
 #include "lldb/Target/RegisterContext.h"
+#include "lldb/Utility/Flags.h"
 #include "lldb/lldb-private.h"
 #include <map>
 
 class RegisterInfoPOSIX_riscv64
     : public lldb_private::RegisterInfoAndSetInterface {
 public:
-  static const lldb_private::RegisterInfo *
-  GetRegisterInfoPtr(const lldb_private::ArchSpec &target_arch);
-  static uint32_t
-  GetRegisterInfoCount(const lldb_private::ArchSpec &target_arch);
+  enum { GPRegSet = 0 };
 
-public:
-  enum { GPRegSet = 0, FPRegSet };
+  // RISC-V64 register set mask value
+  enum {
+    eRegsetMaskDefault = 0,
+    eRegsetMaskFP = 1,
+    eRegsetMaskAll = -1,
+  };
 
   struct GPR {
     // note: gpr[0] is pc, not x0
@@ -41,7 +43,11 @@ class RegisterInfoPOSIX_riscv64
   };
 
   RegisterInfoPOSIX_riscv64(const lldb_private::ArchSpec &target_arch,
-                            lldb_private::Flags flags);
+                            lldb_private::Flags opt_regsets);
+
+  void AddRegSetGP();
+
+  void AddRegSetFP();
 
   size_t GetGPRSize() const override;
 
@@ -58,9 +64,23 @@ class RegisterInfoPOSIX_riscv64
 
   size_t GetRegisterSetFromRegisterIndex(uint32_t reg_index) const override;
 
+  bool IsFPPresent() const { return m_opt_regsets.AnySet(eRegsetMaskFP); }
+
+  bool IsFPReg(unsigned reg) const;
+
 private:
-  const lldb_private::RegisterInfo *m_register_info_p;
-  uint32_t m_register_info_count;
+  std::vector<lldb_private::RegisterInfo> m_register_infos;
+
+  std::vector<lldb_private::RegisterSet> m_register_sets;
+
+  // Contains pair of [start, end] register numbers of a register set with start
+  // and end included.
+  std::map<uint32_t, std::pair<uint32_t, uint32_t>> m_per_regset_regnum_range;
+
+  // Register collections to be stored as reference for m_register_sets items
+  std::vector<uint32_t> m_fp_regnum_collection;
+
+  lldb_private::Flags m_opt_regsets;
 };
 
 #endif