[LLDB][Process/Utility] Introduce NativeRegisterContextDBReg class #118043
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Created using spr 1.3.5-bogner
|
@llvm/pr-subscribers-lldb Author: wanglei (wangleiat) ChangesThis patch adds support for setting/clearing hardware watchpoints and Refer to the following document for the hw break/watchpoint: Fix Failed Tests: Patch is 20.24 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/118043.diff 4 Files Affected:
diff --git a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_loongarch64.cpp b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_loongarch64.cpp
index f4d1bb297049da..1f73bd0467962d 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_loongarch64.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_loongarch64.cpp
@@ -11,7 +11,9 @@
#include "NativeRegisterContextLinux_loongarch64.h"
#include "lldb/Host/HostInfo.h"
+#include "lldb/Host/linux/Ptrace.h"
#include "lldb/Utility/DataBufferHeap.h"
+#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/Log.h"
#include "lldb/Utility/RegisterValue.h"
#include "lldb/Utility/Status.h"
@@ -32,6 +34,19 @@ using namespace lldb;
using namespace lldb_private;
using namespace lldb_private::process_linux;
+// CTRL_PLV3_ENABLE, used to enable breakpoint/watchpoint
+constexpr uint32_t g_enable_bit = 0x10;
+
+// Returns appropriate control register bits for the specified size
+// size encoded:
+// case 1 : 0b11
+// case 2 : 0b10
+// case 4 : 0b01
+// case 8 : 0b00
+static inline uint64_t GetSizeBits(int size) {
+ return (3 - llvm::Log2_32(size)) << 10;
+}
+
std::unique_ptr<NativeRegisterContextLinux>
NativeRegisterContextLinux::CreateHostNativeRegisterContextLinux(
const ArchSpec &target_arch, NativeThreadLinux &native_thread) {
@@ -61,6 +76,8 @@ NativeRegisterContextLinux_loongarch64::NativeRegisterContextLinux_loongarch64(
NativeRegisterContextLinux(native_thread) {
::memset(&m_fpr, 0, sizeof(m_fpr));
::memset(&m_gpr, 0, sizeof(m_gpr));
+ ::memset(&m_hwp_regs, 0, sizeof(m_hwp_regs));
+ ::memset(&m_hbp_regs, 0, sizeof(m_hbp_regs));
m_gpr_is_valid = false;
m_fpu_is_valid = false;
@@ -337,4 +354,471 @@ NativeRegisterContextLinux_loongarch64::GetExpeditedRegisters(
return expedited_reg_nums;
}
+uint32_t
+NativeRegisterContextLinux_loongarch64::NumSupportedHardwareBreakpoints() {
+ Log *log = GetLog(LLDBLog::Breakpoints);
+
+ // Read hardware breakpoint and watchpoint information.
+ Status error = ReadHardwareDebugInfo();
+
+ if (error.Fail()) {
+ LLDB_LOG(log, "failed to read debug registers");
+ return 0;
+ }
+
+ LLDB_LOG(log, "{0}", m_max_hbp_supported);
+ return m_max_hbp_supported;
+}
+
+uint32_t
+NativeRegisterContextLinux_loongarch64::SetHardwareBreakpoint(lldb::addr_t addr,
+ size_t size) {
+ Log *log = GetLog(LLDBLog::Breakpoints);
+ LLDB_LOG(log, "addr: {0:x}, size: {1:x}", addr, size);
+
+ // Read hardware breakpoint and watchpoint information.
+ Status error = ReadHardwareDebugInfo();
+ if (error.Fail()) {
+ LLDB_LOG(log, "unable to set breakpoint: failed to read debug registers");
+ return LLDB_INVALID_INDEX32;
+ }
+
+ uint32_t bp_index = 0;
+
+ // Check if size has a valid hardware breakpoint length.
+ if (size != 4)
+ return LLDB_INVALID_INDEX32; // Invalid size for a LoongArch hardware
+ // breakpoint
+
+ // Check 4-byte alignment for hardware breakpoint target address.
+ if (addr & 0x03)
+ return LLDB_INVALID_INDEX32; // Invalid address, should be 4-byte aligned.
+
+ // Iterate over stored breakpoints and find a free bp_index
+ bp_index = LLDB_INVALID_INDEX32;
+ for (uint32_t i = 0; i < m_max_hbp_supported; i++) {
+ if (!BreakpointIsEnabled(i))
+ bp_index = i; // Mark last free slot
+ else if (m_hbp_regs[i].address == addr)
+ return LLDB_INVALID_INDEX32; // We do not support duplicate breakpoints.
+ }
+
+ if (bp_index == LLDB_INVALID_INDEX32)
+ return LLDB_INVALID_INDEX32;
+
+ // Update breakpoint in local cache
+ m_hbp_regs[bp_index].address = addr;
+ m_hbp_regs[bp_index].control = g_enable_bit;
+
+ // PTRACE call to set corresponding hardware breakpoint register.
+ error = WriteHardwareDebugRegs(eDREGTypeBREAK);
+
+ if (error.Fail()) {
+ m_hbp_regs[bp_index].address = 0;
+ m_hbp_regs[bp_index].control = 0;
+
+ LLDB_LOG(log, "unable to set breakpoint: failed to write debug registers");
+ return LLDB_INVALID_INDEX32;
+ }
+
+ return bp_index;
+}
+bool NativeRegisterContextLinux_loongarch64::ClearHardwareBreakpoint(
+ uint32_t hw_idx) {
+ Log *log = GetLog(LLDBLog::Breakpoints);
+ LLDB_LOG(log, "hw_idx: {0}", hw_idx);
+
+ // Read hardware breakpoint and watchpoint information.
+ Status error = ReadHardwareDebugInfo();
+ if (error.Fail()) {
+ LLDB_LOG(log, "unable to clear breakpoint: failed to read debug registers");
+ return false;
+ }
+
+ if (hw_idx >= m_max_hbp_supported)
+ return false;
+
+ // Create a backup we can revert to in case of failure.
+ lldb::addr_t tempAddr = m_hbp_regs[hw_idx].address;
+ uint32_t tempControl = m_hbp_regs[hw_idx].control;
+
+ m_hbp_regs[hw_idx].control = 0;
+ m_hbp_regs[hw_idx].address = 0;
+
+ // PTRACE call to clear corresponding hardware breakpoint register.
+ error = WriteHardwareDebugRegs(eDREGTypeBREAK);
+
+ if (error.Fail()) {
+ m_hbp_regs[hw_idx].control = tempControl;
+ m_hbp_regs[hw_idx].address = tempAddr;
+
+ LLDB_LOG(log,
+ "unable to clear breakpoint: failed to write debug registers");
+ return false;
+ }
+
+ return true;
+}
+Status NativeRegisterContextLinux_loongarch64::GetHardwareBreakHitIndex(
+ uint32_t &bp_index, lldb::addr_t trap_addr) {
+ Log *log = GetLog(LLDBLog::Breakpoints);
+
+ LLDB_LOGF(log, "NativeRegisterContextLinux_loongarch64::%s()", __FUNCTION__);
+
+ lldb::addr_t break_addr;
+
+ for (bp_index = 0; bp_index < m_max_hbp_supported; ++bp_index) {
+ break_addr = m_hbp_regs[bp_index].address;
+
+ if (BreakpointIsEnabled(bp_index) && trap_addr == break_addr) {
+ m_hbp_regs[bp_index].hit_addr = trap_addr;
+ return Status();
+ }
+ }
+
+ bp_index = LLDB_INVALID_INDEX32;
+ return Status();
+}
+Status NativeRegisterContextLinux_loongarch64::ClearAllHardwareBreakpoints() {
+ Log *log = GetLog(LLDBLog::Breakpoints);
+
+ LLDB_LOGF(log, "NativeRegisterContextLinux_loongarch64::%s()", __FUNCTION__);
+
+ // Read hardware breakpoint and watchpoint information.
+ Status error = ReadHardwareDebugInfo();
+ if (error.Fail())
+ return error;
+
+ for (uint32_t i = 0; i < m_max_hbp_supported; i++) {
+ if (!BreakpointIsEnabled(i))
+ continue;
+ // Create a backup we can revert to in case of failure.
+ lldb::addr_t tempAddr = m_hbp_regs[i].address;
+ uint32_t tempControl = m_hbp_regs[i].control;
+
+ // Clear watchpoints in local cache
+ m_hbp_regs[i].control = 0;
+ m_hbp_regs[i].address = 0;
+
+ // Ptrace call to update hardware debug registers
+ error = WriteHardwareDebugRegs(eDREGTypeBREAK);
+
+ if (error.Fail()) {
+ m_hbp_regs[i].control = tempControl;
+ m_hbp_regs[i].address = tempAddr;
+
+ return error;
+ }
+ }
+
+ return Status();
+}
+bool NativeRegisterContextLinux_loongarch64::BreakpointIsEnabled(
+ uint32_t bp_index) {
+ Log *log = GetLog(LLDBLog::Breakpoints);
+ LLDB_LOG(log, "bp_index: {0}", bp_index);
+ return ((m_hbp_regs[bp_index].control & g_enable_bit) != 0);
+}
+
+uint32_t
+NativeRegisterContextLinux_loongarch64::NumSupportedHardwareWatchpoints() {
+ Log *log = GetLog(LLDBLog::Watchpoints);
+ Status error = ReadHardwareDebugInfo();
+ if (error.Fail()) {
+ LLDB_LOG(log, "failed to read debug registers");
+ return 0;
+ }
+
+ return m_max_hwp_supported;
+}
+
+uint32_t NativeRegisterContextLinux_loongarch64::SetHardwareWatchpoint(
+ lldb::addr_t addr, size_t size, uint32_t watch_flags) {
+ Log *log = GetLog(LLDBLog::Watchpoints);
+ LLDB_LOG(log, "addr: {0:x}, size: {1:x} watch_flags: {2:x}", addr, size,
+ watch_flags);
+
+ // Read hardware breakpoint and watchpoint information.
+ Status error = ReadHardwareDebugInfo();
+
+ if (error.Fail()) {
+ LLDB_LOG(log, "unable to set watchpoint: failed to read debug registers");
+ return LLDB_INVALID_INDEX32;
+ }
+
+ uint32_t control_value = 0, wp_index = 0;
+
+ // Check if we are setting watchpoint other than read/write/access Update
+ // watchpoint flag to match loongarch64 write-read bit configuration.
+ switch (watch_flags) {
+ case eWatchpointKindWrite:
+ watch_flags = 2;
+ break;
+ case eWatchpointKindRead:
+ watch_flags = 1;
+ break;
+ case (eWatchpointKindRead | eWatchpointKindWrite):
+ break;
+ default:
+ return LLDB_INVALID_INDEX32;
+ }
+
+ // Check if size has a valid hardware watchpoint length.
+ if (size != 1 && size != 2 && size != 4 && size != 8)
+ return LLDB_INVALID_INDEX32;
+
+ // Setup control value
+ control_value = g_enable_bit | GetSizeBits(size);
+ control_value |= watch_flags << 8;
+
+ // Iterate over stored watchpoints and find a free wp_index
+ wp_index = LLDB_INVALID_INDEX32;
+ for (uint32_t i = 0; i < m_max_hwp_supported; i++) {
+ if (!WatchpointIsEnabled(i)) {
+ wp_index = i; // Mark last free slot
+ } else if (m_hwp_regs[i].address == addr) {
+ return LLDB_INVALID_INDEX32; // We do not support duplicate watchpoints.
+ }
+ }
+
+ if (wp_index == LLDB_INVALID_INDEX32)
+ return LLDB_INVALID_INDEX32;
+
+ // Update watchpoint in local cache
+ m_hwp_regs[wp_index].address = addr;
+ m_hwp_regs[wp_index].control = control_value;
+
+ // PTRACE call to set corresponding watchpoint register.
+ error = WriteHardwareDebugRegs(eDREGTypeWATCH);
+
+ if (error.Fail()) {
+ m_hwp_regs[wp_index].address = 0;
+ m_hwp_regs[wp_index].control = 0;
+
+ LLDB_LOG(log, "unable to set watchpoint: failed to write debug registers");
+ return LLDB_INVALID_INDEX32;
+ }
+
+ return wp_index;
+}
+
+bool NativeRegisterContextLinux_loongarch64::ClearHardwareWatchpoint(
+ uint32_t wp_index) {
+ Log *log = GetLog(LLDBLog::Watchpoints);
+ LLDB_LOG(log, "wp_index: {0}", wp_index);
+
+ // Read hardware breakpoint and watchpoint information.
+ Status error = ReadHardwareDebugInfo();
+
+ if (error.Fail()) {
+ LLDB_LOG(log, "unable to clear watchpoint: failed to read debug registers");
+ return false;
+ }
+
+ if (wp_index >= m_max_hwp_supported)
+ return false;
+
+ // Create a backup we can revert to in case of failure.
+ lldb::addr_t tempAddr = m_hwp_regs[wp_index].address;
+ uint32_t tempControl = m_hwp_regs[wp_index].control;
+
+ // Update watchpoint in local cache
+ m_hwp_regs[wp_index].control = 0;
+ m_hwp_regs[wp_index].address = 0;
+
+ // Ptrace call to update hardware debug registers
+ error = WriteHardwareDebugRegs(eDREGTypeWATCH);
+
+ if (error.Fail()) {
+ m_hwp_regs[wp_index].control = tempControl;
+ m_hwp_regs[wp_index].address = tempAddr;
+
+ LLDB_LOG(log, "unable to clear watchpoint: failed to read debug registers");
+ return false;
+ }
+
+ return true;
+}
+
+Status NativeRegisterContextLinux_loongarch64::ClearAllHardwareWatchpoints() {
+ // Read hardware breakpoint and watchpoint information.
+ Status error = ReadHardwareDebugInfo();
+ if (error.Fail())
+ return error;
+
+ for (uint32_t i = 0; i < m_max_hwp_supported; i++) {
+ if (!WatchpointIsEnabled(i))
+ continue;
+ // Create a backup we can revert to in case of failure.
+ lldb::addr_t tempAddr = m_hwp_regs[i].address;
+ uint32_t tempControl = m_hwp_regs[i].control;
+
+ // Clear watchpoints in local cache
+ m_hwp_regs[i].control = 0;
+ m_hwp_regs[i].address = 0;
+
+ // Ptrace call to update hardware debug registers
+ error = WriteHardwareDebugRegs(eDREGTypeWATCH);
+
+ if (error.Fail()) {
+ m_hwp_regs[i].control = tempControl;
+ m_hwp_regs[i].address = tempAddr;
+
+ return error;
+ }
+ }
+
+ return Status();
+}
+
+uint32_t
+NativeRegisterContextLinux_loongarch64::GetWatchpointSize(uint32_t wp_index) {
+ Log *log = GetLog(LLDBLog::Watchpoints);
+ LLDB_LOG(log, "wp_index: {0}", wp_index);
+
+ switch ((m_hwp_regs[wp_index].control >> 10) & 0x3) {
+ case 0x0:
+ return 8;
+ case 0x1:
+ return 4;
+ case 0x2:
+ return 2;
+ case 0x3:
+ return 1;
+ default:
+ return 0;
+ }
+}
+
+bool NativeRegisterContextLinux_loongarch64::WatchpointIsEnabled(
+ uint32_t wp_index) {
+ Log *log = GetLog(LLDBLog::Watchpoints);
+ LLDB_LOG(log, "wp_index: {0}", wp_index);
+ return ((m_hwp_regs[wp_index].control & g_enable_bit) != 0);
+}
+
+Status NativeRegisterContextLinux_loongarch64::GetWatchpointHitIndex(
+ uint32_t &wp_index, lldb::addr_t trap_addr) {
+ Log *log = GetLog(LLDBLog::Watchpoints);
+ LLDB_LOG(log, "wp_index: {0}, trap_addr: {1:x}", wp_index, trap_addr);
+
+ // Read hardware breakpoint and watchpoint information.
+ Status error = ReadHardwareDebugInfo();
+ if (error.Fail())
+ return error;
+
+ uint32_t watch_size;
+ lldb::addr_t watch_addr;
+
+ for (wp_index = 0; wp_index < m_max_hwp_supported; ++wp_index) {
+ watch_size = GetWatchpointSize(wp_index);
+ watch_addr = m_hwp_regs[wp_index].address;
+
+ if (WatchpointIsEnabled(wp_index) && trap_addr >= watch_addr &&
+ trap_addr <= watch_addr + watch_size) {
+ m_hwp_regs[wp_index].hit_addr = trap_addr;
+ return Status();
+ }
+ }
+
+ wp_index = LLDB_INVALID_INDEX32;
+ return Status();
+}
+
+lldb::addr_t NativeRegisterContextLinux_loongarch64::GetWatchpointAddress(
+ uint32_t wp_index) {
+ Log *log = GetLog(LLDBLog::Watchpoints);
+ LLDB_LOG(log, "wp_index: {0}", wp_index);
+
+ if (wp_index >= m_max_hwp_supported)
+ return LLDB_INVALID_ADDRESS;
+
+ if (WatchpointIsEnabled(wp_index))
+ return m_hwp_regs[wp_index].address;
+
+ return LLDB_INVALID_ADDRESS;
+}
+
+lldb::addr_t NativeRegisterContextLinux_loongarch64::GetWatchpointHitAddress(
+ uint32_t wp_index) {
+ Log *log = GetLog(LLDBLog::Watchpoints);
+ LLDB_LOG(log, "wp_index: {0}", wp_index);
+
+ if (wp_index >= m_max_hwp_supported)
+ return LLDB_INVALID_ADDRESS;
+
+ if (WatchpointIsEnabled(wp_index))
+ return m_hwp_regs[wp_index].hit_addr;
+
+ return LLDB_INVALID_ADDRESS;
+}
+
+Status NativeRegisterContextLinux_loongarch64::ReadHardwareDebugInfo() {
+ if (!m_refresh_hwdebug_info)
+ return Status();
+
+ ::pid_t tid = m_thread.GetID();
+
+ int regset = NT_LOONGARCH_HW_WATCH;
+ struct iovec ioVec;
+ struct user_watch_state dreg_state;
+ Status error;
+
+ ioVec.iov_base = &dreg_state;
+ ioVec.iov_len = sizeof(dreg_state);
+ error = NativeProcessLinux::PtraceWrapper(PTRACE_GETREGSET, tid, ®set,
+ &ioVec, ioVec.iov_len);
+ if (error.Fail())
+ return error;
+
+ m_max_hwp_supported = dreg_state.dbg_info & 0x3f;
+
+ regset = NT_LOONGARCH_HW_BREAK;
+ error = NativeProcessLinux::PtraceWrapper(PTRACE_GETREGSET, tid, ®set,
+ &ioVec, ioVec.iov_len);
+ if (error.Fail())
+ return error;
+
+ m_max_hbp_supported = dreg_state.dbg_info & 0x3f;
+
+ m_refresh_hwdebug_info = false;
+
+ return error;
+}
+
+Status NativeRegisterContextLinux_loongarch64::WriteHardwareDebugRegs(
+ DREGType hwbType) {
+ struct iovec ioVec;
+ struct user_watch_state dreg_state;
+ int regset;
+
+ memset(&dreg_state, 0, sizeof(dreg_state));
+ ioVec.iov_base = &dreg_state;
+
+ switch (hwbType) {
+ case eDREGTypeWATCH:
+ regset = NT_LOONGARCH_HW_WATCH;
+ ioVec.iov_len = sizeof(dreg_state.dbg_info) +
+ (sizeof(dreg_state.dbg_regs[0]) * m_max_hwp_supported);
+
+ for (uint32_t i = 0; i < m_max_hwp_supported; i++) {
+ dreg_state.dbg_regs[i].addr = m_hwp_regs[i].address;
+ dreg_state.dbg_regs[i].ctrl = m_hwp_regs[i].control;
+ }
+ break;
+ case eDREGTypeBREAK:
+ regset = NT_LOONGARCH_HW_BREAK;
+ ioVec.iov_len = sizeof(dreg_state.dbg_info) +
+ (sizeof(dreg_state.dbg_regs[0]) * m_max_hbp_supported);
+
+ for (uint32_t i = 0; i < m_max_hbp_supported; i++) {
+ dreg_state.dbg_regs[i].addr = m_hbp_regs[i].address;
+ dreg_state.dbg_regs[i].ctrl = m_hbp_regs[i].control;
+ }
+ break;
+ }
+
+ return NativeProcessLinux::PtraceWrapper(PTRACE_SETREGSET, m_thread.GetID(),
+ ®set, &ioVec, ioVec.iov_len);
+}
#endif // defined(__loongarch__) && __loongarch_grlen == 64
diff --git a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_loongarch64.h b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_loongarch64.h
index 0a6084ff4206db..6599677924d41d 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_loongarch64.h
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_loongarch64.h
@@ -51,6 +51,42 @@ class NativeRegisterContextLinux_loongarch64
bool RegisterOffsetIsDynamic() const override { return true; }
+ // Hardware breakpoints management functions
+
+ uint32_t NumSupportedHardwareBreakpoints() override;
+
+ uint32_t SetHardwareBreakpoint(lldb::addr_t addr, size_t size) override;
+
+ bool ClearHardwareBreakpoint(uint32_t hw_idx) override;
+
+ Status ClearAllHardwareBreakpoints() override;
+
+ Status GetHardwareBreakHitIndex(uint32_t &bp_index,
+ lldb::addr_t trap_addr) override;
+
+ bool BreakpointIsEnabled(uint32_t bp_index);
+
+ // Hardware watchpoints management functions
+ uint32_t NumSupportedHardwareWatchpoints() override;
+
+ uint32_t SetHardwareWatchpoint(lldb::addr_t addr, size_t size,
+ uint32_t watch_flags) override;
+
+ bool ClearHardwareWatchpoint(uint32_t hw_index) override;
+
+ Status ClearAllHardwareWatchpoints() override;
+
+ Status GetWatchpointHitIndex(uint32_t &wp_index,
+ lldb::addr_t trap_addr) override;
+
+ lldb::addr_t GetWatchpointHitAddress(uint32_t wp_index) override;
+
+ lldb::addr_t GetWatchpointAddress(uint32_t wp_index) override;
+
+ uint32_t GetWatchpointSize(uint32_t wp_index);
+
+ bool WatchpointIsEnabled(uint32_t wp_index);
+
protected:
Status ReadGPR() override;
@@ -83,6 +119,30 @@ class NativeRegisterContextLinux_loongarch64
uint32_t CalculateFprOffset(const RegisterInfo *reg_info) const;
const RegisterInfoPOSIX_loongarch64 &GetRegisterInfo() const;
+
+ // Debug register type select
+ enum DREGType { eDREGTypeWATCH = 0, eDREGTypeBREAK };
+
+ Status ReadHardwareDebugInfo();
+ Status WriteHardwareDebugRegs(DREGType hwbType);
+
+ // Debug register info for hardware watchpoints management.
+ struct DREG {
+ lldb::addr_t address; // Breakpoint/watchpoint address value.
+ lldb::addr_t hit_addr; // Address at which last watchpoint trigger
+ // exception occurred.
+ uint32_t control; // Breakpoint/watchpoint control value.
+ };
+
+ std::array<struct DREG, 14> m_hbp_regs; // hardware breakpoints
+ std::array<struct DREG, 14> m_hwp_regs; // hardware watchpoints
+
+ // Refer to:
+ // https://loongson.github.io/LoongArch-Documentation/LoongArch-Vol1-EN.html#control-and-status-registers-related-to-watchpoints
+ // 14 is just a maximum value, query hardware for actual watchpoint count.
+ uint32_t m_max_hwp_supported = 14;
+ uint32_t m_max_hbp_supported = 14;
+ bool m_refresh_hwdebug_info = true;
};
} // namespace process_linux
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
index 324db3db7eb4c7..c2fe05cad566ef 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -231,7 +231,8 @@ GDBRemoteCommunicationServerCommon::Handle_qHostInfo(
host_arch.GetMachine() == llvm::Triple::aarch64_32 ||
host_arch.GetMachine() == llvm::Triple::aarch64_be ||
host_arch.GetMachine() == llvm::Triple::arm ||
- host_arch.GetMachine() == llvm::Triple::armeb || host_arch.IsMIPS())
+ host_arch.GetMachine() == llvm::Triple::armeb || host_arch.IsMIPS() ||
+ host_arch.GetTriple().isLoongArch())
response.Printf("watchpoint_exceptions_received:before;");
else
response.Printf("watchpoint_exceptions_received:after;");
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index db33525978a16a..68485a40a3fcce 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -2517,7 +2517,8 @@ bool Process::GetWatchpointReportedAfter() {
llvm::Triple triple = arch.GetTriple();
if (triple.isMIPS() || triple.isPPC64() ...
[truncated]
|
There was a problem hiding this comment.
I started to comment on some style issues here then realised that this code is adapted from lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_arm64.cpp and I think about 90% of it is the same.
I would prefer that you extract the generic parts of the code into a generic NativeRegisterContextDBReg then have Arm64 and LoongArch specialise that.
It's one more level of classes in an already deep hierarchy but in this case I think the overlap is large enough to justify it.
This also solves little things like the name of g_enable_bit being too generic for a global in a file that does things other than breakpoints. If it's in a file specific to breakpoints, the name is ok.
GetWatchpointSize is an example of something that will need to be per architecture but
ClearHardwareBreakpoint is the same for both so it can go in a base class.
|
And I don't see that LoongArch is doing anything unusual, so I think this is just a refactoring exercise. It is native code so if you don't have an AArch64 machine to build on, I can test the PR for you. |
|
We could move all this back into NativeRegisterContextLinux too but A: let's take one step at a time and B: it may be that some architecture does not fit the pattern that AArch64 and LoongArch do. So in the interests of you getting this done and us having less churn, stick to making a NativeRegisterContextDBReg class. One of these days maybe we'll hoist it further up. |
SixWeining
reviewed
Dec 3, 2024
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_loongarch64.cpp
Outdated
Show resolved
Hide resolved
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_loongarch64.cpp
Outdated
Show resolved
Hide resolved
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_loongarch64.cpp
Outdated
Show resolved
Hide resolved
Thank you for your suggestion. I will divide it into two patches:
|
Created using spr 1.3.5-bogner
wangleiat
changed the title
There was a problem hiding this comment.
I was looking at the Arm (32 bit) code and it's also very similar, so if I find the time I'll migrate this to this base class too.
Edit: Though 32/64 probably makes that annoying, anyway, point is, these patterns are clearly more common than we thought.
See if you can move the use of pac_bits, then I'll test this for you on AArch64.
| @@ -234,9 +234,9 @@ class NativeRegisterContextLinux_arm64 | |||
|
|
|||
| size_t GetFPMRBufferSize() { return sizeof(m_fpmr_reg); } | |||
|
|
|||
| llvm::Error ReadHardwareDebugInfo() override; | |||
| Status ReadHardwareDebugInfo() override; | |||
There was a problem hiding this comment.
@labath I seem to remember you saying we want to move to or away from Status, if I didn't imagine that, which is it?
There was a problem hiding this comment.
@wangleiat can you explain why this switched from Status to llvm::Error? Was it that one copy of the code used Status and the other llvm::Erorr?
There was a problem hiding this comment.
Because I noticed that the function calling it returns a Status (virtual Status ClearAllHardwareWatchpoints() in file lldb/include/lldb/Host/common/NativeRegisterContext.h), and other architectures, such as ARM (not AArch64) and PPC, also return a Status.
Should consistency be maintained here? If llvm::Error is required, I will revert it. I also made a mistake—the definition in this file NativeRegisterContextFreeBSD_arm64.h hasn't been updated yet. I apologize for my carelessness.
lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_arm64.cpp
Outdated
Show resolved
Hide resolved
Created using spr 1.3.5-bogner
lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_arm64.cpp
Outdated
Show resolved
Hide resolved
lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_arm64.h
Outdated
Show resolved
Hide resolved
|
And now I see that x86 has a The way it stores registers is quite different though, so I don't think:
So let's ignore that class for now. LoongArch and AArch64 is enough to be dealing with. |
lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg.cpp
Outdated
Show resolved
Hide resolved
lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg.cpp
Outdated
Show resolved
Hide resolved
lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_arm64.cpp
Outdated
Show resolved
Hide resolved
lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg.h
Outdated
Show resolved
Hide resolved
lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_arm64.cpp
Outdated
Show resolved
Hide resolved
|
Something is up with watchpoints: I'm digging into it now. |
|
The test failures were caused by: Which is actually trying to change size for AArch64, but it's passed by copy so it wasn't being updated in the caller. See my comments on those lines. With that fixed, the test suite passes. If you address that, I'll test this again. If it's all working then you can address @SixWeining 's style suggestions from the original PR, and I'll have a few of my own. |
Created using spr 1.3.5-bogner
Thanks very much for your comments. |
There was a problem hiding this comment.
This passed all tests. Some remaining cosmetic issues but otherwise looks good.
We could go more into the style issues here but
- This code was like that before, you just moved it, so it's not actually a regression.
- I don't want to get into refactoring and break tests again. If we want to do that we can do it in small pieces later.
Going to look again at the follow up PR as well.
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
Outdated
Show resolved
Hide resolved
| @@ -234,9 +234,9 @@ class NativeRegisterContextLinux_arm64 | |||
|
|
|||
| size_t GetFPMRBufferSize() { return sizeof(m_fpmr_reg); } | |||
|
|
|||
| llvm::Error ReadHardwareDebugInfo() override; | |||
| Status ReadHardwareDebugInfo() override; | |||
There was a problem hiding this comment.
@wangleiat can you explain why this switched from Status to llvm::Error? Was it that one copy of the code used Status and the other llvm::Erorr?
lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg.cpp
Outdated
Show resolved
Hide resolved
lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg.cpp
Outdated
Show resolved
Hide resolved
Do you mean submitting the subsequent changes in a new PR? Since the |
Created using spr 1.3.5-bogner
|
Hi @DavidSpickett, the following are the main changes: Thank you very much for your review. |
DavidSpickett
approved these changes
Dec 11, 2024
wangleiat
deleted the
users/wangleiat/spr/lldbprocesslinux-introduce-loongarch64-hw-breakwatchpoint-support
branch
wangleiat
added a commit
that referenced
this pull request
Dec 13, 2024
This patch adds support for setting/clearing hardware watchpoints and breakpoints on LoongArch 64-bit hardware. Refer to the following document for the hw break/watchpoint: https://loongson.github.io/LoongArch-Documentation/LoongArch-Vol1-EN.html#control-and-status-registers-related-to-watchpoints Fix Failed Tests: lldb-shell :: Subprocess/clone-follow-child-wp.test lldb-shell :: Subprocess/clone-follow-parent-wp.test lldb-shell :: Subprocess/fork-follow-child-wp.test lldb-shell :: Subprocess/fork-follow-parent-wp.test lldb-shell :: Subprocess/vfork-follow-child-wp.test lldb-shell :: Subprocess/vfork-follow-parent-wp.test lldb-shell :: Watchpoint/ExpressionLanguage.test Depends on: #118043 Reviewed By: SixWeining Pull Request: #118770
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Since the setup of debug registers for AArch64 and LoongArch is similar,
we extracted the shared logic from Class:
NativeRegisterContextDBReg_arm64into a new Class:
NativeRegisterContextDBReg.This will simplify the subsequent implementation of hardware breakpoints
and watchpoints on LoongArch.