[lldb][Process/FreeBSD] Add riscv64 support#180549
Conversation
Signed-off-by: Minsoo Choo <minsoochoo0122@proton.me>
|
I'll notify here once testing is done. |
|
@llvm/pr-subscribers-lldb @llvm/pr-subscribers-backend-risc-v Author: Minsoo Choo (mchoo7) ChangesLLDB needs to maintain separate structure for FreeBSD's riscv register structure. The translation is done through Patch is 23.45 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/180549.diff 6 Files Affected:
diff --git a/lldb/docs/index.rst b/lldb/docs/index.rst
index 10683c7593b01..53a37cb462a1a 100644
--- a/lldb/docs/index.rst
+++ b/lldb/docs/index.rst
@@ -73,7 +73,7 @@ are welcome:
* iOS, tvOS, and watchOS simulator debugging on i386, x86_64 and AArch64
* iOS, tvOS, and watchOS device debugging on ARM and AArch64
* Linux user-space debugging for i386, x86_64, ARM, AArch64, PPC64le, s390x
-* FreeBSD user-space debugging for i386, x86_64, ARM, AArch64, PPC
+* FreeBSD user-space debugging for i386, x86_64, ARM, AArch64, PPC, RISCV64
* FreeBSD kernel debugging for i386, x86_64, AArch64
* NetBSD user-space debugging for i386 and x86_64
* Windows user-space debugging for i386, x86_64, ARM and AArch64 (*)
diff --git a/lldb/source/Plugins/Process/FreeBSD/CMakeLists.txt b/lldb/source/Plugins/Process/FreeBSD/CMakeLists.txt
index 8574df58b4ada..b7b4b969dbfe0 100644
--- a/lldb/source/Plugins/Process/FreeBSD/CMakeLists.txt
+++ b/lldb/source/Plugins/Process/FreeBSD/CMakeLists.txt
@@ -4,6 +4,7 @@ add_lldb_library(lldbPluginProcessFreeBSD
NativeRegisterContextFreeBSD_arm.cpp
NativeRegisterContextFreeBSD_arm64.cpp
NativeRegisterContextFreeBSD_powerpc.cpp
+ NativeRegisterContextFreeBSD_riscv64.cpp
NativeRegisterContextFreeBSD_x86_64.cpp
NativeThreadFreeBSD.cpp
diff --git a/lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_riscv64.cpp b/lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_riscv64.cpp
new file mode 100644
index 0000000000000..3464d0a377bd5
--- /dev/null
+++ b/lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_riscv64.cpp
@@ -0,0 +1,559 @@
+//===-- NativeRegisterContextFreeBSD_riscv64.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(__riscv) && __riscv_xlen == 64
+
+#include "NativeRegisterContextFreeBSD_riscv64.h"
+
+#include "lldb/Utility/DataBufferHeap.h"
+#include "lldb/Utility/RegisterValue.h"
+#include "lldb/Utility/Status.h"
+
+#include "Plugins/Process/FreeBSD/NativeProcessFreeBSD.h"
+#include "Plugins/Process/Utility/RegisterInfoPOSIX_riscv64.h"
+
+// clang-format off
+#include <sys/param.h>
+#include <sys/ptrace.h>
+#include <sys/types.h>
+// clang-format on
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace lldb_private::process_freebsd;
+
+// Translation between RegisterInfoPosix_riscv64 and reg.h
+// https://github.com/freebsd/freebsd-src/blob/main/sys/riscv/include/reg.h:
+//
+// struct reg {
+// __uint64_t ra; /* return address */
+// __uint64_t sp; /* stack pointer */
+// __uint64_t gp; /* global pointer */
+// __uint64_t tp; /* thread pointer */
+// __uint64_t t[7]; /* temporaries */
+// __uint64_t s[12]; /* saved registers */
+// __uint64_t a[8]; /* function arguments */
+// __uint64_t sepc; /* exception program counter */
+// __uint64_t sstatus; /* status register */
+// };
+//
+// struct fpreg {
+// __uint64_t fp_x[32][2]; /* Floating point registers */
+// __uint64_t fp_fcsr; /* Floating point control reg */
+// };
+//
+// struct dbreg {
+// int dummy;
+// };
+
+// ============================================================================
+// Static Conversion Functions between FreeBSD and POSIX
+// ============================================================================
+
+void NativeRegisterContextFreeBSD_riscv64::FreeBSDToPOSIXGPR(
+ const struct reg &freebsd_gpr, RegisterInfoPOSIX_riscv64::GPR &posix_gpr) {
+ posix_gpr.gpr[gpr_pc_riscv64] = freebsd_gpr.sepc; // x0/pc
+ posix_gpr.gpr[gpr_ra_riscv64] = freebsd_gpr.ra; // x1/ra
+ posix_gpr.gpr[gpr_sp_riscv64] = freebsd_gpr.sp; // x2/sp
+ posix_gpr.gpr[gpr_gp_riscv64] = freebsd_gpr.gp; // x3/gp
+ posix_gpr.gpr[gpr_tp_riscv64] = freebsd_gpr.tp; // x4/tp
+
+ // x5-x7: t0-t2
+ posix_gpr.gpr[gpr_t0_riscv64] = freebsd_gpr.t[0];
+ posix_gpr.gpr[gpr_t1_riscv64] = freebsd_gpr.t[1];
+ posix_gpr.gpr[gpr_t2_riscv64] = freebsd_gpr.t[2];
+
+ // x8-x9: s0-s1 (s0 is also fp)
+ posix_gpr.gpr[gpr_s0_riscv64] = freebsd_gpr.s[0];
+ posix_gpr.gpr[gpr_s1_riscv64] = freebsd_gpr.s[1];
+
+ // x10-x17: a0-a7
+ for (int i = 0; i < 8; i++)
+ posix_gpr.gpr[gpr_a0_riscv64 + i] = freebsd_gpr.a[i];
+
+ // x18-x27: s2-s11
+ for (int i = 0; i < 10; i++)
+ posix_gpr.gpr[gpr_s2_riscv64 + i] = freebsd_gpr.s[2 + i];
+
+ // x28-x31: t3-t6
+ posix_gpr.gpr[gpr_t3_riscv64] = freebsd_gpr.t[3];
+ posix_gpr.gpr[gpr_t4_riscv64] = freebsd_gpr.t[4];
+ posix_gpr.gpr[gpr_t5_riscv64] = freebsd_gpr.t[5];
+ posix_gpr.gpr[gpr_t6_riscv64] = freebsd_gpr.t[6];
+}
+
+void NativeRegisterContextFreeBSD_riscv64::POSIXToFreeBSDGPR(
+ const RegisterInfoPOSIX_riscv64::GPR &posix_gpr, struct reg &freebsd_gpr) {
+ freebsd_gpr.sepc = posix_gpr.gpr[gpr_pc_riscv64]; // x0/pc
+ freebsd_gpr.ra = posix_gpr.gpr[gpr_ra_riscv64]; // x1/ra
+ freebsd_gpr.sp = posix_gpr.gpr[gpr_sp_riscv64]; // x2/sp
+ freebsd_gpr.gp = posix_gpr.gpr[gpr_gp_riscv64]; // x3/gp
+ freebsd_gpr.tp = posix_gpr.gpr[gpr_tp_riscv64]; // x4/tp
+
+ // x5-x7: t0-t2
+ freebsd_gpr.t[0] = posix_gpr.gpr[gpr_t0_riscv64];
+ freebsd_gpr.t[1] = posix_gpr.gpr[gpr_t1_riscv64];
+ freebsd_gpr.t[2] = posix_gpr.gpr[gpr_t2_riscv64];
+
+ // x8-x9: s0-s1
+ freebsd_gpr.s[0] = posix_gpr.gpr[gpr_s0_riscv64];
+ freebsd_gpr.s[1] = posix_gpr.gpr[gpr_s1_riscv64];
+
+ // x10-x17: a0-a7
+ for (int i = 0; i < 8; i++)
+ freebsd_gpr.a[i] = posix_gpr.gpr[gpr_a0_riscv64 + i];
+
+ // x18-x27: s2-s11
+ for (int i = 0; i < 10; i++)
+ freebsd_gpr.s[2 + i] = posix_gpr.gpr[gpr_s2_riscv64 + i];
+
+ // x28-x31: t3-t6
+ freebsd_gpr.t[3] = posix_gpr.gpr[gpr_t3_riscv64];
+ freebsd_gpr.t[4] = posix_gpr.gpr[gpr_t4_riscv64];
+ freebsd_gpr.t[5] = posix_gpr.gpr[gpr_t5_riscv64];
+ freebsd_gpr.t[6] = posix_gpr.gpr[gpr_t6_riscv64];
+}
+
+void NativeRegisterContextFreeBSD_riscv64::FreeBSDToPOSIXFPR(
+ const struct fpreg &freebsd_fpr,
+ RegisterInfoPOSIX_riscv64::FPR &posix_fpr) {
+ // FreeBSD stores FP registers as 128-bit (fp_x[32][2])
+ // POSIX expects 64-bit (fpr[32])
+ // We only use the lower 64 bits (D extension, double precision)
+ for (int i = 0; i < 32; i++)
+ posix_fpr.fpr[i] = freebsd_fpr.fp_x[i][0];
+
+ // FCSR: FreeBSD has 64-bit, POSIX expects 32-bit
+ posix_fpr.fcsr = static_cast<uint32_t>(freebsd_fpr.fp_fcsr);
+}
+
+void NativeRegisterContextFreeBSD_riscv64::POSIXToFreeBSDFPR(
+ const RegisterInfoPOSIX_riscv64::FPR &posix_fpr,
+ struct fpreg &freebsd_fpr) {
+ // POSIX has 64-bit FP registers, FreeBSD expects 128-bit
+ for (int i = 0; i < 32; i++) {
+ freebsd_fpr.fp_x[i][0] = posix_fpr.fpr[i]; // Lower 64 bits
+ freebsd_fpr.fp_x[i][1] = 0; // Upper 64 bits (unused for D)
+ }
+
+ // FCSR: POSIX has 32-bit, FreeBSD expects 64-bit
+ freebsd_fpr.fp_fcsr = static_cast<uint64_t>(posix_fpr.fcsr);
+}
+
+// ============================================================================
+// Constructor and Setup
+// ============================================================================
+
+NativeRegisterContextFreeBSD *
+NativeRegisterContextFreeBSD::CreateHostNativeRegisterContextFreeBSD(
+ const ArchSpec &target_arch, NativeThreadFreeBSD &native_thread) {
+ return new NativeRegisterContextFreeBSD_riscv64(target_arch, native_thread);
+}
+
+NativeRegisterContextFreeBSD_riscv64::NativeRegisterContextFreeBSD_riscv64(
+ const ArchSpec &target_arch, NativeThreadFreeBSD &native_thread)
+ : NativeRegisterContextFreeBSD(native_thread), m_gpr(), m_fpr(),
+ m_gpr_is_valid(false), m_fpr_is_valid(false) {
+ m_register_info_interface_up =
+ std::make_unique<RegisterInfoPOSIX_riscv64>(target_arch, 0);
+
+ ::memset(&m_gpr, 0, sizeof(m_gpr));
+ ::memset(&m_fpr, 0, sizeof(m_fpr));
+}
+
+RegisterInfoPOSIX_riscv64 &
+NativeRegisterContextFreeBSD_riscv64::GetRegisterInfo() const {
+ return static_cast<RegisterInfoPOSIX_riscv64 &>(
+ *m_register_info_interface_up);
+}
+
+// ============================================================================
+// Register Set Information
+// ============================================================================
+
+uint32_t NativeRegisterContextFreeBSD_riscv64::GetRegisterSetCount() const {
+ return GetRegisterInfo().GetRegisterSetCount();
+}
+
+const RegisterSet *
+NativeRegisterContextFreeBSD_riscv64::GetRegisterSet(uint32_t set_index) const {
+ return GetRegisterInfo().GetRegisterSet(set_index);
+}
+
+uint32_t NativeRegisterContextFreeBSD_riscv64::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;
+}
+
+void NativeRegisterContextFreeBSD_riscv64::InvalidateAllRegisters() {
+ m_gpr_is_valid = false;
+ m_fpr_is_valid = false;
+}
+
+// ============================================================================
+// Ptrace Wrappers
+// ============================================================================
+
+Status NativeRegisterContextFreeBSD_riscv64::ReadGPR() {
+ if (m_gpr_is_valid)
+ return Status();
+
+ Status error =
+ NativeProcessFreeBSD::PtraceWrapper(PT_GETREGS, m_thread.GetID(), &m_gpr);
+
+ if (error.Success())
+ m_gpr_is_valid = true;
+
+ return error;
+}
+
+Status NativeRegisterContextFreeBSD_riscv64::WriteGPR() {
+ Status error =
+ NativeProcessFreeBSD::PtraceWrapper(PT_SETREGS, m_thread.GetID(), &m_gpr);
+
+ if (error.Success())
+ m_gpr_is_valid = true;
+
+ return error;
+}
+
+Status NativeRegisterContextFreeBSD_riscv64::ReadFPR() {
+ if (m_fpr_is_valid)
+ return Status();
+
+ Status error = NativeProcessFreeBSD::PtraceWrapper(PT_GETFPREGS,
+ m_thread.GetID(), &m_fpr);
+
+ if (error.Success())
+ m_fpr_is_valid = true;
+
+ return error;
+}
+
+Status NativeRegisterContextFreeBSD_riscv64::WriteFPR() {
+ Status error = NativeProcessFreeBSD::PtraceWrapper(PT_SETFPREGS,
+ m_thread.GetID(), &m_fpr);
+
+ if (error.Success())
+ m_fpr_is_valid = true;
+
+ return error;
+}
+
+// ============================================================================
+// Single Register Access Helpers
+// ============================================================================
+
+Status
+NativeRegisterContextFreeBSD_riscv64::GetGPRValue(uint32_t reg_index,
+ uint64_t &value) const {
+ // Map LLDB register index to FreeBSD struct reg field
+ switch (reg_index) {
+ case gpr_pc_riscv64:
+ value = m_gpr.sepc;
+ return Status();
+ case gpr_ra_riscv64:
+ value = m_gpr.ra;
+ return Status();
+ case gpr_sp_riscv64:
+ value = m_gpr.sp;
+ return Status();
+ case gpr_gp_riscv64:
+ value = m_gpr.gp;
+ return Status();
+ case gpr_tp_riscv64:
+ value = m_gpr.tp;
+ return Status();
+
+ // t0-t6
+ case gpr_t0_riscv64:
+ case gpr_t1_riscv64:
+ case gpr_t2_riscv64:
+ value = m_gpr.t[reg_index - gpr_t0_riscv64];
+ return Status();
+ case gpr_t3_riscv64:
+ case gpr_t4_riscv64:
+ case gpr_t5_riscv64:
+ case gpr_t6_riscv64:
+ value = m_gpr.t[reg_index - gpr_t3_riscv64 + 3];
+ return Status();
+
+ // s0-s11
+ case gpr_s0_riscv64:
+ case gpr_s1_riscv64:
+ value = m_gpr.s[reg_index - gpr_s0_riscv64];
+ return Status();
+ case gpr_s2_riscv64:
+ case gpr_s3_riscv64:
+ case gpr_s4_riscv64:
+ case gpr_s5_riscv64:
+ case gpr_s6_riscv64:
+ case gpr_s7_riscv64:
+ case gpr_s8_riscv64:
+ case gpr_s9_riscv64:
+ case gpr_s10_riscv64:
+ case gpr_s11_riscv64:
+ value = m_gpr.s[reg_index - gpr_s2_riscv64 + 2];
+ return Status();
+
+ // a0-a7
+ case gpr_a0_riscv64:
+ case gpr_a1_riscv64:
+ case gpr_a2_riscv64:
+ case gpr_a3_riscv64:
+ case gpr_a4_riscv64:
+ case gpr_a5_riscv64:
+ case gpr_a6_riscv64:
+ case gpr_a7_riscv64:
+ value = m_gpr.a[reg_index - gpr_a0_riscv64];
+ return Status();
+
+ default:
+ return Status::FromErrorStringWithFormat("invalid GPR register index: %u",
+ reg_index);
+ }
+}
+
+Status NativeRegisterContextFreeBSD_riscv64::SetGPRValue(uint32_t reg_index,
+ uint64_t value) {
+ switch (reg_index) {
+ case gpr_pc_riscv64:
+ m_gpr.sepc = value;
+ return Status();
+ case gpr_ra_riscv64:
+ m_gpr.ra = value;
+ return Status();
+ case gpr_sp_riscv64:
+ m_gpr.sp = value;
+ return Status();
+ case gpr_gp_riscv64:
+ m_gpr.gp = value;
+ return Status();
+ case gpr_tp_riscv64:
+ m_gpr.tp = value;
+ return Status();
+
+ case gpr_t0_riscv64:
+ case gpr_t1_riscv64:
+ case gpr_t2_riscv64:
+ m_gpr.t[reg_index - gpr_t0_riscv64] = value;
+ return Status();
+ case gpr_t3_riscv64:
+ case gpr_t4_riscv64:
+ case gpr_t5_riscv64:
+ case gpr_t6_riscv64:
+ m_gpr.t[reg_index - gpr_t3_riscv64 + 3] = value;
+ return Status();
+
+ case gpr_s0_riscv64:
+ case gpr_s1_riscv64:
+ m_gpr.s[reg_index - gpr_s0_riscv64] = value;
+ return Status();
+ case gpr_s2_riscv64:
+ case gpr_s3_riscv64:
+ case gpr_s4_riscv64:
+ case gpr_s5_riscv64:
+ case gpr_s6_riscv64:
+ case gpr_s7_riscv64:
+ case gpr_s8_riscv64:
+ case gpr_s9_riscv64:
+ case gpr_s10_riscv64:
+ case gpr_s11_riscv64:
+ m_gpr.s[reg_index - gpr_s2_riscv64 + 2] = value;
+ return Status();
+
+ case gpr_a0_riscv64:
+ case gpr_a1_riscv64:
+ case gpr_a2_riscv64:
+ case gpr_a3_riscv64:
+ case gpr_a4_riscv64:
+ case gpr_a5_riscv64:
+ case gpr_a6_riscv64:
+ case gpr_a7_riscv64:
+ m_gpr.a[reg_index - gpr_a0_riscv64] = value;
+ return Status();
+
+ default:
+ return Status::FromErrorStringWithFormat("invalid GPR register index: %u",
+ reg_index);
+ }
+}
+
+// ============================================================================
+// Single Register Read/Write
+// ============================================================================
+
+Status
+NativeRegisterContextFreeBSD_riscv64::ReadRegister(const RegisterInfo *reg_info,
+ RegisterValue ®_value) {
+ if (!reg_info)
+ return Status::FromErrorString("reg_info NULL");
+
+ const uint32_t reg = reg_info->kinds[lldb::eRegisterKindLLDB];
+
+ if (reg == LLDB_INVALID_REGNUM)
+ return Status::FromErrorStringWithFormat(
+ "no lldb regnum for %s", reg_info->name ? reg_info->name : "<unknown>");
+
+ if (GetRegisterInfo().IsGPR(reg)) {
+ Status error = ReadGPR();
+ if (error.Fail())
+ return error;
+
+ uint64_t value;
+ error = GetGPRValue(reg, value);
+ if (error.Fail())
+ return error;
+
+ reg_value = value;
+ return Status();
+ }
+
+ if (GetRegisterInfo().IsFPR(reg)) {
+ Status error = ReadFPR();
+ if (error.Fail())
+ return error;
+
+ uint32_t fpr_index =
+ reg - GetRegisterInfo().GetRegisterInfo()[reg].kinds[eRegisterKindLLDB];
+
+ if (fpr_index < 32) {
+ reg_value = m_fpr.fp_x[fpr_index][0]; // Lower 64 bits
+ } else if (fpr_index == 32) {
+ reg_value = static_cast<uint32_t>(m_fpr.fp_fcsr);
+ } else {
+ return Status::FromErrorString("invalid FPR index");
+ }
+
+ return Status();
+ }
+
+ return Status::FromErrorString("unsupported register type");
+}
+
+Status NativeRegisterContextFreeBSD_riscv64::WriteRegister(
+ const RegisterInfo *reg_info, const RegisterValue ®_value) {
+ if (!reg_info)
+ return Status::FromErrorString("reg_info NULL");
+
+ const uint32_t reg = reg_info->kinds[lldb::eRegisterKindLLDB];
+
+ if (reg == LLDB_INVALID_REGNUM)
+ return Status::FromErrorStringWithFormat(
+ "no lldb regnum for %s", reg_info->name ? reg_info->name : "<unknown>");
+
+ if (GetRegisterInfo().IsGPR(reg)) {
+ Status error = ReadGPR(); // Read first to preserve other registers
+ if (error.Fail())
+ return error;
+
+ error = SetGPRValue(reg, reg_value.GetAsUInt64());
+ if (error.Fail())
+ return error;
+
+ return WriteGPR();
+ }
+
+ if (GetRegisterInfo().IsFPR(reg)) {
+ Status error = ReadFPR();
+ if (error.Fail())
+ return error;
+
+ uint32_t fpr_index =
+ reg - GetRegisterInfo().GetRegisterInfo()[reg].kinds[eRegisterKindLLDB];
+
+ if (fpr_index < 32) {
+ m_fpr.fp_x[fpr_index][0] = reg_value.GetAsUInt64();
+ m_fpr.fp_x[fpr_index][1] = 0;
+ } else if (fpr_index == 32) {
+ m_fpr.fp_fcsr = reg_value.GetAsUInt32();
+ } else {
+ return Status::FromErrorString("invalid FPR index");
+ }
+
+ return WriteFPR();
+ }
+
+ return Status::FromErrorString("unsupported register type");
+}
+
+// ============================================================================
+// Bulk Register Read/Write (using conversion functions)
+// ============================================================================
+
+Status NativeRegisterContextFreeBSD_riscv64::ReadAllRegisterValues(
+ lldb::WritableDataBufferSP &data_sp) {
+ // Read from kernel
+ Status error = ReadGPR();
+ if (error.Fail())
+ return error;
+
+ error = ReadFPR();
+ if (error.Fail())
+ return error;
+
+ // Allocate buffer for POSIX format
+ const size_t total_size = sizeof(RegisterInfoPOSIX_riscv64::GPR) +
+ sizeof(RegisterInfoPOSIX_riscv64::FPR);
+ data_sp = std::make_shared<DataBufferHeap>(total_size, 0);
+ if (!data_sp || !data_sp->GetBytes())
+ return Status::FromErrorString("failed to allocate data buffer");
+
+ // Get pointers to GPR and FPR sections of buffer
+ auto *gpr_dst =
+ reinterpret_cast<RegisterInfoPOSIX_riscv64::GPR *>(data_sp->GetBytes());
+ auto *fpr_dst = reinterpret_cast<RegisterInfoPOSIX_riscv64::FPR *>(
+ data_sp->GetBytes() + sizeof(RegisterInfoPOSIX_riscv64::GPR));
+
+ // Convert FreeBSD format to POSIX format
+ FreeBSDToPOSIXGPR(m_gpr, *gpr_dst);
+ FreeBSDToPOSIXFPR(m_fpr, *fpr_dst);
+
+ return Status();
+}
+
+Status NativeRegisterContextFreeBSD_riscv64::WriteAllRegisterValues(
+ const lldb::DataBufferSP &data_sp) {
+ if (!data_sp)
+ return Status::FromErrorString("invalid data_sp provided");
+
+ const size_t expected_size = sizeof(RegisterInfoPOSIX_riscv64::GPR) +
+ sizeof(RegisterInfoPOSIX_riscv64::FPR);
+
+ if (data_sp->GetByteSize() != expected_size) {
+ return Status::FromErrorStringWithFormat(
+ "data_sp size mismatch, expected %zu, actual %" PRIu64, expected_size,
+ data_sp->GetByteSize());
+ }
+
+ const uint8_t *src = data_sp->GetBytes();
+ if (!src)
+ return Status::FromErrorString("DataBuffer::GetBytes() returned null");
+
+ // Get pointers to GPR and FPR sections of buffer
+ const auto *gpr_src =
+ reinterpret_cast<const RegisterInfoPOSIX_riscv64::GPR *>(src);
+ const auto *fpr_src =
+ reinterpret_cast<const RegisterInfoPOSIX_riscv64::FPR *>(
+ src + sizeof(RegisterInfoPOSIX_riscv64::GPR));
+
+ // Convert POSIX format to FreeBSD format
+ POSIXToFreeBSDGPR(*gpr_src, m_gpr);
+ POSIXToFreeBSDFPR(*fpr_src, m_fpr);
+
+ // Write to kernel
+ Status error = WriteGPR();
+ if (error.Fail())
+ return error;
+
+ return WriteFPR();
+}
+
+#endif // defined(__riscv) && __riscv_xlen == 64
diff --git a/lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_riscv64.h b/lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_riscv64.h
new file mode 100644
index 0000000000000..cd8d72cc60773
--- /dev/null
+++ b/lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_riscv64.h
@@ -0,0 +1,81 @@
+//===-- NativeRegisterContextFreeBSD_riscv64.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
+//
+//===----------------------------------------------------------------------===//
+
+#if defined(__riscv) && __riscv_xlen == 64
+
+#ifndef lldb_NativeRegisterContextFreeBSD_riscv64_h
+#define lldb_NativeRegisterContextFreeBSD_riscv64_h
+
+// clang-format off
+#include <sys/types.h>
+#include...
[truncated]
|
|
I have question for knowledgable LLDB contributors: what are |
|
I looked at x86. RegisterContext_x86.h defines a bunch of stuff, for example a type to represent some special register kinds. This is then included in RegisterContextPOSIX_x86.h and RegisterContextWindows_x86_64.cpp. This is not the case for RISCV where RegisterContextDarwin_riscv32.h and RegisterContextPOSIX_riscv32.h seem to do their own thing. I'd get it working and then go looking for overlaps between them all. It might be small enough that it's not worth a refactor. |
DavidSpickett
left a comment
DavidSpickett
left a comment
There was a problem hiding this comment.
I can't be the approver on this, I can only comment on the use of existing LLDB infrastructure and not on architecture specific details. So you will need approval from a FreeBSD person, especially as this is a new port.
I am suspicious of all the conversion going on. I await your explanation on that.
For you to claim support, we need to see that you are able to run check-lldb and have reasonable results. Ideally there would be no failures, and no major features skipped for reasons under LLDB's control. E.g. lack of hardware breakpoint support is fine, as long as it's because FreeBSD has not implemented and/or stabailised it yet.
The definition of "major" is up to reviewers but you can also present your own assessment of the results of course.
If you cannot present good enough results, that does not mean this PR cannot be accepted, but we will need to see more PRs to fix the problems before the platform can be claimed to be "supported". We have a few experimentally supported platforms, it's not unusual.
Where the bar lies for this PR to be accepted on its own, that lies with the other reviewers.
lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_riscv64.h
Show resolved
Hide resolved
lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_riscv64.cpp
Outdated
Show resolved
Hide resolved
lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_riscv64.cpp
Outdated
Show resolved
Hide resolved
lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_riscv64.cpp
Outdated
Show resolved
Hide resolved
lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_riscv64.cpp
Outdated
Show resolved
Hide resolved
lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_riscv64.cpp
Outdated
Show resolved
Hide resolved
| @@ -97,6 +97,8 @@ ThreadElfCore::CreateRegisterContextForFrame(StackFrame *frame) { | |||
| case llvm::Triple::ppc64le: | |||
| reg_interface = new RegisterContextFreeBSD_powerpc64(arch); | |||
| break; | |||
| case llvm::Triple::riscv64: | |||
| break; | |||
There was a problem hiding this comment.
Is this making something work, or making it compile?
I see there is a lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_riscv64.cpp file, which could be being picked up. I suppose that's why you wanted the conversions.
But at a glance, I don't see why this is not following the pattern of the others.
There was a problem hiding this comment.
arm/arm64 follows this. Other have their own RegisterContextFreeBSD_* implementation. As noted above, should I fall back to FreeBSD's own implementation of RegisterContext? I think I cannot follow arm/arm64's example here.
There was a problem hiding this comment.
For arm64, and probably arm too, the relevant structs have the same layout on FreeBSD, so presumably there's a common implementation that can be used. For riscv64, FreeBSD's struct is annoying jumbled (grouped by ABI name / purpose, not register index, which gets confusing), whereas Linux's is in register index order (though, curiously, as individual per-register fields, no arrays).
There was a problem hiding this comment.
And I guess it's too late to change reg.h...
There was a problem hiding this comment.
Yes.
As far as I can tell, the conversion functions exist only for ReadAllRegisterValues and WriteAllRegisterValues.
NativeRegisterContextFreeBSD_riscv64::ReadAllRegisterValues calls FreeBSDToPOSIXGPR and FreeBSDToPOSIXFPR. NativeRegisterContextFreeBSD_riscv64::WriteAllRegisterValues calls POSIXToFreeBSDGPR and POSIXToFreeBSDFPR.
These return, and use, a DataBufferSP. ReadAllRegisterValues is called by GDBRemoteCommunicationServerLLGS::Handle_QSaveRegisterState which inserts the SP into a map m_saved_registers_map. All it does with this is pull the SP back out when asked to write all register values.
So I think that you could use the FreeBSD format here because no one tries to look inside the saved data. The FreeBSD plugin can return its own format, and will get back data in its own format.
If I'm correct, it's still worth adding a note in the header next to these two functions that the data is in a format you wouldn't expect. Just in case we later try to start poking into these saved register sets (which sounds like a bad idea anyway).
lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_riscv64.cpp
Outdated
Show resolved
Hide resolved
lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_riscv64.cpp
Show resolved
Hide resolved
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
mchoo7
force-pushed
the
riscv64
branch
4 times, most recently
from
a156f42 to
fda00e1
Compare
I was (and still am) confused of RegisterContextFreeBSD being used in other architectures but arm/arm64, so I created conversion logic between RegisterInfoPOSIX and FreeBSD's |
Me too tbf, no criticism from me on that.
Yeah that's a fine thing to do. And you put your specific concern in a comment for the reviewers to see - so many people forget to do this.
I'm not sure it is, it just needs justifying. So what I recommend is to look at one of the architectures that does not need this conversion and see what layers it has. Perhaps it would have needed this conversion if they'd taken your approach, perhaps they don't need the conversion because FreeBSD and Linux (and Darwin? seems unlikely) line up. If the other architectures also tackled this problem, but by adding more specialised classes, I think I'd prefer the extra compile time over the extra runtime work of conversion. But anyway let's see what you discover. If you do need help with Linux AArch64 details, let me know.
Might be one of many ways of being correct, let's see what the evidence shows us.
Leave the PR as is. It's clear to you, me, and anyone else who reads this what state it's in. You can set it to draft if you like. Sometimes it is good to show new approaches as a separate PR though. Helps reviewers to be able to have the 2 side by side. |
Signed-off-by: Minsoo Choo <minsoochoo0122@proton.me>
| @@ -182,6 +180,7 @@ Changes to LLDB | |||
| * Support for FreeBSD on MIPS64 has been removed. | |||
| * The minimum assumed version of FreeBSD is now 14. The effect of which is that watchpoints are | |||
| assumed to be supported. | |||
| * Userspace debugging support for RISCV64 on FreeBSD has been added. | |||
There was a problem hiding this comment.
"FreeBSD on RISCV64"
4 participants
LLDB needs to maintain separate structure for FreeBSD's riscv register structure. The translation is done through
FreeBSDToPOSIXGPR,FreeBSDToPOSIXFPR,POSIXToFreeBSDGPR, andPOSIXToFreeBSDFPR.