[lldb][ARM] Port Arm Linux to use NativeRegisterContextDBReg #152284
Merged
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
Which is also used by AArch64 and LoongArch. To do this I had to make some adjustments to NativeRegisterContextDBReg: * New method AdjustBreakpoint. This is like AdjustWatchpoint, but only Arm needs to adjust breakpoints. For Arm this method removes the bottom address bits according to the mode. * MakeWatchControlValue now takes the address too. Arm uses this to generate the byte mask. To test this, I ran the test suite as usual on Arm and found no new failures, then ran it again with all test programs compiled with `-mthumb`. This means the binaries will be entirely Thumb code. Finally I tested it on AArch64, but this is mostly a build test, as I did not run the entire test suite compiled to AArch32 mode. Prior to this change, these tests failed with `-mthumb`: ``` Failed Tests (14): lldb-api :: commands/process/reverse-continue/TestReverseContinue.py lldb-api :: functionalities/breakpoint/breakpoint_by_line_and_column/TestBreakpointByLineAndColumn.py lldb-api :: functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py lldb-api :: functionalities/reverse-execution/TestReverseContinueBreakpoints.py lldb-api :: functionalities/reverse-execution/TestReverseContinueWatchpoints.py lldb-api :: functionalities/tail_call_frames/disambiguate_call_site/TestDisambiguateCallSite.py lldb-api :: functionalities/tail_call_frames/disambiguate_paths_to_common_sink/TestDisambiguatePathsToCommonSink.py lldb-api :: functionalities/tail_call_frames/disambiguate_tail_call_seq/TestDisambiguateTailCallSeq.py lldb-api :: functionalities/tail_call_frames/inlining_and_tail_calls/TestInliningAndTailCalls.py lldb-api :: functionalities/tail_call_frames/sbapi_support/TestTailCallFrameSBAPI.py lldb-api :: functionalities/tail_call_frames/thread_step_out_message/TestArtificialFrameStepOutMessage.py lldb-api :: functionalities/thread/jump/TestThreadJump.py lldb-api :: lang/c/vla/TestVLA.py lldb-api :: linux/sepdebugsymlink/TestTargetSymbolsSepDebugSymlink.py ``` After this change, no new failures occured. So I am confident it is correct. Looking into those failures, it's a few things: * Something in the reverse execution wrapper that I can't figure out, but is likely nothing to do with the real breakpoints. * The inability to tail call when building thumb code, because the call goes via. a veneer that might do a mode switch. * Assumptions about source locations being in speciifc places. None of which are massive issues I feel like need fixing before doing this port.
Member
|
@llvm/pr-subscribers-lldb Author: David Spickett (DavidSpickett) ChangesWhich is also used by AArch64 and LoongArch. To do this I had to make some adjustments to NativeRegisterContextDBReg:
To test this, I ran the test suite as usual on Arm and found no new Finally I tested it on AArch64, but this is mostly a build test, Prior to this change, these tests failed with After this change, no new failures occurred. So I am confident it is Looking at those failures, it's a few things:
None of which are massive issues I feel like need fixing before doing this port. Patch is 33.24 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/152284.diff 11 Files Affected:
diff --git a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
index fdafacf410d64..85395f1c15125 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
@@ -76,7 +76,7 @@ NativeRegisterContextLinux_arm::NativeRegisterContextLinux_arm(
::memset(&m_fpr, 0, sizeof(m_fpr));
::memset(&m_gpr_arm, 0, sizeof(m_gpr_arm));
::memset(&m_hwp_regs, 0, sizeof(m_hwp_regs));
- ::memset(&m_hbr_regs, 0, sizeof(m_hbr_regs));
+ ::memset(&m_hbp_regs, 0, sizeof(m_hbp_regs));
// 16 is just a maximum value, query hardware for actual watchpoint count
m_max_hwp_supported = 16;
@@ -283,522 +283,43 @@ bool NativeRegisterContextLinux_arm::IsFPR(unsigned reg) const {
return false;
}
-uint32_t NativeRegisterContextLinux_arm::NumSupportedHardwareBreakpoints() {
- Log *log = GetLog(POSIXLog::Breakpoints);
-
- LLDB_LOGF(log, "NativeRegisterContextLinux_arm::%s()", __FUNCTION__);
-
- Status error;
-
- // Read hardware breakpoint and watchpoint information.
- error = ReadHardwareDebugInfo();
-
- if (error.Fail())
- return 0;
-
- LLDB_LOG(log, "{0}", m_max_hbp_supported);
- return m_max_hbp_supported;
-}
-
-uint32_t
-NativeRegisterContextLinux_arm::SetHardwareBreakpoint(lldb::addr_t addr,
- size_t size) {
- Log *log = GetLog(POSIXLog::Breakpoints);
- LLDB_LOG(log, "addr: {0:x}, size: {1:x}", addr, size);
-
- // Read hardware breakpoint and watchpoint information.
- Status error = ReadHardwareDebugInfo();
-
- if (error.Fail())
- return LLDB_INVALID_INDEX32;
-
- uint32_t control_value = 0, bp_index = 0;
-
- // Setup address and control values.
- // Use size to get a hint of arm vs thumb modes.
- switch (size) {
- case 2:
- control_value = (0x3 << 5) | 7;
- addr &= ~1;
- break;
- case 4:
- control_value = (0xfu << 5) | 7;
- addr &= ~3;
- break;
- default:
- return LLDB_INVALID_INDEX32;
- }
-
- // 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 ((m_hbr_regs[i].control & 1) == 0) {
- bp_index = i; // Mark last free slot
- } else if (m_hbr_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_hbr_regs[bp_index].real_addr = addr;
- m_hbr_regs[bp_index].address = addr;
- m_hbr_regs[bp_index].control = control_value;
-
- // PTRACE call to set corresponding hardware breakpoint register.
- error = WriteHardwareDebugRegs(NativeRegisterContextDBReg::eDREGTypeBREAK,
- bp_index);
-
- if (error.Fail()) {
- m_hbr_regs[bp_index].address = 0;
- m_hbr_regs[bp_index].control &= ~1;
-
- return LLDB_INVALID_INDEX32;
- }
-
- return bp_index;
-}
-
-bool NativeRegisterContextLinux_arm::ClearHardwareBreakpoint(uint32_t hw_idx) {
- Log *log = GetLog(POSIXLog::Breakpoints);
- LLDB_LOG(log, "hw_idx: {0}", hw_idx);
-
- // Read hardware breakpoint and watchpoint information.
- Status error = ReadHardwareDebugInfo();
-
- if (error.Fail())
- 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_hbr_regs[hw_idx].address;
- uint32_t tempControl = m_hbr_regs[hw_idx].control;
-
- m_hbr_regs[hw_idx].control &= ~1;
- m_hbr_regs[hw_idx].address = 0;
-
- // PTRACE call to clear corresponding hardware breakpoint register.
- error = WriteHardwareDebugRegs(NativeRegisterContextDBReg::eDREGTypeBREAK,
- hw_idx);
-
- if (error.Fail()) {
- m_hbr_regs[hw_idx].control = tempControl;
- m_hbr_regs[hw_idx].address = tempAddr;
-
- return false;
- }
-
- return true;
-}
-
-Status NativeRegisterContextLinux_arm::GetHardwareBreakHitIndex(
- uint32_t &bp_index, lldb::addr_t trap_addr) {
- Log *log = GetLog(POSIXLog::Breakpoints);
-
- LLDB_LOGF(log, "NativeRegisterContextLinux_arm::%s()", __FUNCTION__);
-
- lldb::addr_t break_addr;
-
- for (bp_index = 0; bp_index < m_max_hbp_supported; ++bp_index) {
- break_addr = m_hbr_regs[bp_index].address;
-
- if ((m_hbr_regs[bp_index].control & 0x1) && (trap_addr == break_addr)) {
- m_hbr_regs[bp_index].hit_addr = trap_addr;
- return Status();
- }
- }
-
- bp_index = LLDB_INVALID_INDEX32;
- return Status();
-}
-
-Status NativeRegisterContextLinux_arm::ClearAllHardwareBreakpoints() {
- Log *log = GetLog(POSIXLog::Breakpoints);
-
- LLDB_LOGF(log, "NativeRegisterContextLinux_arm::%s()", __FUNCTION__);
-
- Status error;
-
- // Read hardware breakpoint and watchpoint information.
- error = ReadHardwareDebugInfo();
-
- if (error.Fail())
- return error;
-
- lldb::addr_t tempAddr = 0;
- uint32_t tempControl = 0;
-
- for (uint32_t i = 0; i < m_max_hbp_supported; i++) {
- if (m_hbr_regs[i].control & 0x01) {
- // Create a backup we can revert to in case of failure.
- tempAddr = m_hbr_regs[i].address;
- tempControl = m_hbr_regs[i].control;
-
- // Clear breakpoints in local cache
- m_hbr_regs[i].control &= ~1;
- m_hbr_regs[i].address = 0;
-
- // Ptrace call to update hardware debug registers
- error =
- WriteHardwareDebugRegs(NativeRegisterContextDBReg::eDREGTypeBREAK, i);
-
- if (error.Fail()) {
- m_hbr_regs[i].control = tempControl;
- m_hbr_regs[i].address = tempAddr;
-
- return error;
- }
- }
- }
-
- return Status();
-}
-
-uint32_t NativeRegisterContextLinux_arm::NumSupportedHardwareWatchpoints() {
- Log *log = GetLog(POSIXLog::Watchpoints);
-
- // Read hardware breakpoint and watchpoint information.
- Status error = ReadHardwareDebugInfo();
-
- if (error.Fail())
- return 0;
-
- LLDB_LOG(log, "{0}", m_max_hwp_supported);
- return m_max_hwp_supported;
-}
-
-uint32_t NativeRegisterContextLinux_arm::SetHardwareWatchpoint(
- lldb::addr_t addr, size_t size, uint32_t watch_flags) {
- Log *log = GetLog(POSIXLog::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())
- return LLDB_INVALID_INDEX32;
-
- uint32_t control_value = 0, wp_index = 0, addr_word_offset = 0, byte_mask = 0;
- lldb::addr_t real_addr = addr;
-
- // Check if we are setting watchpoint other than read/write/access Also
- // update watchpoint flag to match Arm write-read bit configuration.
- switch (watch_flags) {
- case 1:
- watch_flags = 2;
- break;
- case 2:
- watch_flags = 1;
- break;
- case 3:
- break;
- default:
- return LLDB_INVALID_INDEX32;
- }
-
- // Can't watch zero bytes
- // Can't watch more than 4 bytes per WVR/WCR pair
-
- if (size == 0 || size > 4)
- return LLDB_INVALID_INDEX32;
-
- // Check 4-byte alignment for hardware watchpoint target address. Below is a
- // hack to recalculate address and size in order to make sure we can watch
- // non 4-byte aligned addresses as well.
- if (addr & 0x03) {
- uint8_t watch_mask = (addr & 0x03) + size;
-
- if (watch_mask > 0x04)
- return LLDB_INVALID_INDEX32;
- else if (watch_mask <= 0x02)
- size = 2;
- else
- size = 4;
-
- addr = addr & (~0x03);
- }
-
- // We can only watch up to four bytes that follow a 4 byte aligned address
- // per watchpoint register pair, so make sure we can properly encode this.
- addr_word_offset = addr % 4;
- byte_mask = ((1u << size) - 1u) << addr_word_offset;
-
- // Check if we need multiple watchpoint register
- if (byte_mask > 0xfu)
- return LLDB_INVALID_INDEX32;
-
- // Setup control value
- // Make the byte_mask into a valid Byte Address Select mask
- control_value = byte_mask << 5;
-
- // Turn on appropriate watchpoint flags read or write
- control_value |= (watch_flags << 3);
-
- // Enable this watchpoint and make it stop in privileged or user mode;
- control_value |= 7;
-
- // Make sure bits 1:0 are clear in our address
- addr &= ~((lldb::addr_t)3);
-
- // 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 ((m_hwp_regs[i].control & 1) == 0) {
- 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].real_addr = real_addr;
- m_hwp_regs[wp_index].address = addr;
- m_hwp_regs[wp_index].control = control_value;
-
- // PTRACE call to set corresponding watchpoint register.
- error = WriteHardwareDebugRegs(NativeRegisterContextDBReg::eDREGTypeWATCH,
- wp_index);
-
- if (error.Fail()) {
- m_hwp_regs[wp_index].address = 0;
- m_hwp_regs[wp_index].control &= ~1;
-
- return LLDB_INVALID_INDEX32;
- }
-
- return wp_index;
-}
-
-bool NativeRegisterContextLinux_arm::ClearHardwareWatchpoint(
- uint32_t wp_index) {
- Log *log = GetLog(POSIXLog::Watchpoints);
- LLDB_LOG(log, "wp_index: {0}", wp_index);
-
- // Read hardware breakpoint and watchpoint information.
- Status error = ReadHardwareDebugInfo();
-
- if (error.Fail())
- 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 &= ~1;
- m_hwp_regs[wp_index].address = 0;
-
- // Ptrace call to update hardware debug registers
- error = WriteHardwareDebugRegs(NativeRegisterContextDBReg::eDREGTypeWATCH,
- wp_index);
-
- if (error.Fail()) {
- m_hwp_regs[wp_index].control = tempControl;
- m_hwp_regs[wp_index].address = tempAddr;
-
- return false;
- }
-
- return true;
-}
-
-Status NativeRegisterContextLinux_arm::ClearAllHardwareWatchpoints() {
- // Read hardware breakpoint and watchpoint information.
- Status error = ReadHardwareDebugInfo();
-
- if (error.Fail())
- return error;
-
- lldb::addr_t tempAddr = 0;
- uint32_t tempControl = 0;
-
- for (uint32_t i = 0; i < m_max_hwp_supported; i++) {
- if (m_hwp_regs[i].control & 0x01) {
- // Create a backup we can revert to in case of failure.
- tempAddr = m_hwp_regs[i].address;
- tempControl = m_hwp_regs[i].control;
-
- // Clear watchpoints in local cache
- m_hwp_regs[i].control &= ~1;
- m_hwp_regs[i].address = 0;
-
- // Ptrace call to update hardware debug registers
- error =
- WriteHardwareDebugRegs(NativeRegisterContextDBReg::eDREGTypeWATCH, i);
-
- if (error.Fail()) {
- m_hwp_regs[i].control = tempControl;
- m_hwp_regs[i].address = tempAddr;
-
- return error;
- }
- }
- }
-
- return Status();
-}
-
-uint32_t NativeRegisterContextLinux_arm::GetWatchpointSize(uint32_t wp_index) {
- Log *log = GetLog(POSIXLog::Watchpoints);
- LLDB_LOG(log, "wp_index: {0}", wp_index);
-
- switch ((m_hwp_regs[wp_index].control >> 5) & 0x0f) {
- case 0x01:
- return 1;
- case 0x03:
- return 2;
- case 0x07:
- return 3;
- case 0x0f:
- return 4;
- default:
- return 0;
- }
-}
-bool NativeRegisterContextLinux_arm::WatchpointIsEnabled(uint32_t wp_index) {
- Log *log = GetLog(POSIXLog::Watchpoints);
- LLDB_LOG(log, "wp_index: {0}", wp_index);
-
- if ((m_hwp_regs[wp_index].control & 0x1) == 0x1)
- return true;
- else
- return false;
-}
-
-Status
-NativeRegisterContextLinux_arm::GetWatchpointHitIndex(uint32_t &wp_index,
- lldb::addr_t trap_addr) {
- Log *log = GetLog(POSIXLog::Watchpoints);
- LLDB_LOG(log, "wp_index: {0}, trap_addr: {1:x}", wp_index, trap_addr);
-
- 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_arm::GetWatchpointAddress(uint32_t wp_index) {
- Log *log = GetLog(POSIXLog::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].real_addr;
- else
- return LLDB_INVALID_ADDRESS;
-}
-
-lldb::addr_t
-NativeRegisterContextLinux_arm::GetWatchpointHitAddress(uint32_t wp_index) {
- Log *log = GetLog(POSIXLog::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;
- else
- return LLDB_INVALID_ADDRESS;
-}
-
-Status NativeRegisterContextLinux_arm::ReadHardwareDebugInfo() {
- Status error;
-
- if (!m_refresh_hwdebug_info) {
- return Status();
- }
+llvm::Error NativeRegisterContextLinux_arm::ReadHardwareDebugInfo() {
+ if (!m_refresh_hwdebug_info)
+ return llvm::Error::success();
#ifdef __arm__
unsigned int cap_val;
-
- error = NativeProcessLinux::PtraceWrapper(PTRACE_GETHBPREGS, m_thread.GetID(),
- nullptr, &cap_val,
- sizeof(unsigned int));
+ Status error = NativeProcessLinux::PtraceWrapper(
+ PTRACE_GETHBPREGS, m_thread.GetID(), nullptr, &cap_val,
+ sizeof(unsigned int));
if (error.Fail())
- return error;
+ return error.ToError();
m_max_hwp_supported = (cap_val >> 8) & 0xff;
m_max_hbp_supported = cap_val & 0xff;
m_refresh_hwdebug_info = false;
- return error;
+ return error.ToError();
#else // __aarch64__
return arm64::ReadHardwareDebugInfo(m_thread.GetID(), m_max_hwp_supported,
- m_max_hbp_supported);
+ m_max_hbp_supported)
+ .ToError();
#endif // ifdef __arm__
}
-Status NativeRegisterContextLinux_arm::WriteHardwareDebugRegs(
- NativeRegisterContextDBReg::DREGType hwbType, int hwb_index) {
- Status error;
-
+llvm::Error
+NativeRegisterContextLinux_arm::WriteHardwareDebugRegs(DREGType hwbType) {
#ifdef __arm__
- lldb::addr_t *addr_buf;
- uint32_t *ctrl_buf;
-
- if (hwbType == NativeRegisterContextDBReg::eDREGTypeWATCH) {
- addr_buf = &m_hwp_regs[hwb_index].address;
- ctrl_buf = &m_hwp_regs[hwb_index].control;
-
- error = NativeProcessLinux::PtraceWrapper(
- PTRACE_SETHBPREGS, m_thread.GetID(),
- (PTRACE_TYPE_ARG3)(intptr_t) - ((hwb_index << 1) + 1), addr_buf,
- sizeof(unsigned int));
-
- if (error.Fail())
- return error;
-
- error = NativeProcessLinux::PtraceWrapper(
- PTRACE_SETHBPREGS, m_thread.GetID(),
- (PTRACE_TYPE_ARG3)(intptr_t) - ((hwb_index << 1) + 2), ctrl_buf,
- sizeof(unsigned int));
- } else {
- addr_buf = &m_hbr_regs[hwb_index].address;
- ctrl_buf = &m_hbr_regs[hwb_index].control;
-
- error = NativeProcessLinux::PtraceWrapper(
- PTRACE_SETHBPREGS, m_thread.GetID(),
- (PTRACE_TYPE_ARG3)(intptr_t)((hwb_index << 1) + 1), addr_buf,
- sizeof(unsigned int));
+ uint32_t max_index = m_max_hbp_supported;
+ if (hwbType == eDREGTypeWATCH)
+ max_index = m_max_hwp_supported;
- if (error.Fail())
+ for (uint32_t idx = 0; idx < max_index; ++idx)
+ if (auto error = WriteHardwareDebugReg(hwbType, idx))
return error;
- error = NativeProcessLinux::PtraceWrapper(
- PTRACE_SETHBPREGS, m_thread.GetID(),
- (PTRACE_TYPE_ARG3)(intptr_t)((hwb_index << 1) + 2), ctrl_buf,
- sizeof(unsigned int));
- }
-
- return error;
+ return llvm::Error::success();
#else // __aarch64__
uint32_t max_supported =
(hwbType == NativeRegisterContextDBReg::eDREGTypeWATCH)
@@ -806,12 +327,47 @@ Status NativeRegisterContextLinux_arm::WriteHardwareDebugRegs(
: m_max_hbp_supported;
auto ®s = (hwbType == NativeRegisterContextDBReg::eDREGTypeWATCH)
? m_hwp_regs
- : m_hbr_regs;
+ : m_hbp_regs;
return arm64::WriteHardwareDebugRegs(hwbType, m_thread.GetID(), max_supported,
- regs);
+ regs).ToError();
#endif // ifdef __arm__
}
+#ifdef __arm__
+llvm::Error
+NativeRegisterContextLinux_arm::WriteHardwareDebugReg(DREGType hwbType,
+ int hwb_index) {
+ Status error;
+ lldb::addr_t *addr_buf;
+ uint32_t *ctrl_buf;
+ int addr_idx = (hwb_index << 1) + 1;
+ int ctrl_idx = addr_idx + 1;
+
+ if (hwbType == NativeRegisterContextDBReg::eDREGTypeWATCH) {
+ addr_idx *= -1;
+ addr_buf = &m_hwp_regs[hwb_index].address;
+ ctrl_idx *= -1;
+ ctrl_buf = &m_hwp_regs[hwb_index].control;
+ } else {
+ addr_buf = &m_hbp_regs[hwb_index].address;
+ ctrl_buf = &m_hbp_regs[hwb_index].control;
+ }
+
+ error = NativeProcessLinux::PtraceWrapper(
+ PTRACE_SETHBPREGS, m_thread.GetID(), (PTRACE_TYPE_ARG3)(intptr_t)addr_idx,
+ addr_buf, sizeof(unsigned int));
+
+ if (error.Fail())
+ return error.ToError();
+
+ error = NativeProcessLinux::PtraceWrapper(
+ PTRACE_SETHBPREGS, m_thread.GetID(), (PTRACE_TYPE_ARG3)(intptr_t)ctrl_idx,
+ ctrl_buf, sizeof(unsigned int));
+
+ return error.ToError();
+}
+#endif // ifdef __arm__
+
uint32_t NativeRegisterContextLinux_arm::CalculateFprOffset(
const RegisterInfo *reg_info) const {
return reg_info->byte_offset - GetGPRSize();
diff --git a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.h b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.h
index 3a31d68d7a3c4..cf36859b16ad4 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.h
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.h
@@ -12,7 +12,7 @@
#define lldb_NativeRegisterContextLinux_arm_h
#include "Plugins/Process/Linux/NativeRegisterContextLinux.h"
-#include "Plugins/Process/Utility/NativeRegisterContextDBReg.h"
+#include "Plugins/Process/Utility/NativeRegisterContextDBReg_arm.h"
#include "Plugins/Process/Utility/RegisterInfoPOSIX_arm.h"
#include "Plugins/Process/Utility/lldb-arm-register-enums.h"
@@ -21,7 +21,8 @@ namespace process_linux {
class NativeProcessLinux;
-class NativeRegisterContextLinux_arm : public NativeRegisterContextLinux {
+class NativeRegisterContextLinux_arm : public NativeRegisterContextLinux,
+ public NativeRegisterContextDBReg_arm {
public:
NativeRegisterContextLinux_arm(const ArchSpec &target_arch,
NativeThreadProtocol &native_thread);
@@ -42,39 +43,6 @@ class NativeRegisterContextLinux_arm : public NativeRegisterContextLinux {
Status WriteAllRegisterValues(const lldb::DataBufferSP &data_sp) override;
- // Hardware breakpoints/watchpoint 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;
-
- 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;
...
[truncated]
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Collaborator
Author
|
@b10902118 please try this out in the scenarios you fixed previously. In theory I could have run the test suite with everything AArch32 but my worry is that too much in lldb will make assumptions about the host being AArch64. Plus, I want you to test this anyway. |
DavidSpickett
force-pushed
the
lldb-arm-bkpts-squashed-with-fn-fix
branch
from
abc7017 to
9b066a6
Compare
Collaborator
Author
|
This is a lot of change so it isn't a great review experience. A decent strategy would be to look at each of the strange details we had before, and check there is an equivalent in the new code. I leaned a lot on existing tests to validate this. If you want to know why something exists, ask and I'll do the research. Some of them I just ported over on the assumption they must be important if they were there already. |
b10902118
reviewed
Aug 7, 2025
lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_arm.cpp
Outdated
Show resolved
Hide resolved
b10902118
reviewed
Aug 7, 2025
lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg.h
Outdated
Show resolved
Hide resolved
Contributor
|
Other parts looks the same as my previous try. I haven't test this one, but older version seems to already have problem with thumb breakpoint. In my manual test, soft bp caused segmentation fault on 6.15 arm64 linux kernel (just remember qemu complained [GUP] and gpt says it is stack allocation issue), but ok on my old arm64 android somehow. Hard bp (on old android) works only in the first trigger and then stuck there. I'll trace this and may take a while. I just started with the infinite loop program below and manually attached to it, kind of clumsy. Any suggestions are welcomed. int main() {
volatile int a = 42;
while (true) {
a += 1;
}
return 0;
} |
Collaborator
Author
|
Please raise an issue describing the problem in as much detail as you can. As this refactoring isn't urgent, we should figure out if what you're seeing needs fixing in the original code first. |
Contributor
After investigation, found a kernel bug for 32bit hardware breakpoint: https://lkml.org/lkml/2025/8/24/258 The soft breakpoint issue is caused by my outdated client, with latest one it can set distinguish thumb soft breakpoint with +1 addr offset and works well. |
Contributor
|
I have built and tested this PR with arm/thumb hardware breakpoint, watchpoints of different lengths. It is good. |
Collaborator
Author
|
Thanks for investigating, going to address your comments then land this. |
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/59/builds/23315 Here is the relevant piece of the build log for the reference |
intel-lab-lkp
pushed a commit
to intel-lab-lkp/linux
that referenced
this pull request
Oct 16, 2025
This patch fixes the failure of PTRACE_SETREGSET when setting a hardware breakpoint on a non-4-byte aligned address with a valid control to a 32-bit tracee. The issue was discovered while testing LLDB. Link: llvm/llvm-project#152284 The failure happens because hw_break_set() checks and sets the breakpoint address and control separately. This can result in an check failure when it first validates the address to be set with old control. For example, the control are initialized with breakpoint length of 4. Combining with a non-4-byte aligned address would cross a 4-byte boundary, which is invalid. However, the user-provided control may actually specify a length of 1, which should be valid. The fix is to set the address and control together. This is supported by modify_user_hw_breakpoint(), the function that sets and checks breakpoints. The original implementation wrap this function to ptrace_hbp_set_addr() and ptrace_hbp_set_ctrl() for 32-bit API PTRACE_SETHBPREGS simply because it can only modify one register (address or control) per call. For the 64-bit PTRACE_SETREGSET API, this restriction does not apply, so a new helper function ptrace_hbp_set() was added to set both in a single call to modify_user_hw_breakpoint(). For reference, the check is in arch/arm64/kernel/hw_breakpoint.c:hw_breakpoint_arch_parse() which is called via: modify_user_hw_breakpoint() -> modify_user_hw_breakpoint_check() -> hw_breakpoint_parse() -> hw_breakpoint_arch_parse() Signed-off-by: Bill Tsui <[email protected]>
intel-lab-lkp
pushed a commit
to intel-lab-lkp/linux
that referenced
this pull request
Oct 18, 2025
This patch fixes the failure of PTRACE_SETREGSET when setting a hardware breakpoint on a non-4-byte aligned address with a valid control to a 32-bit tracee. The issue was discovered while testing LLDB. Link: llvm/llvm-project#152284 The failure happens because hw_break_set() checks and sets the breakpoint address and control separately. This can result in an check failure when it first validates the address to be set with old control. For example, the control are initialized with breakpoint length of 4. Combining with a non-4-byte aligned address would cross a 4-byte boundary, which is invalid. However, the user-provided control may actually specify a length of 1, which should be valid. The fix is to set the address and control together. This is supported by modify_user_hw_breakpoint(), the function that sets and checks breakpoints. The original implementation wrap this function to ptrace_hbp_set_addr() and ptrace_hbp_set_ctrl() for 32-bit API PTRACE_SETHBPREGS simply because it can only modify one register (address or control) per call. For the 64-bit PTRACE_SETREGSET API, this restriction does not apply, so a new helper function ptrace_hbp_set() was added to set both in a single call to modify_user_hw_breakpoint(). Then ptrace_hbp_set_addr() and ptrace_hbp_set_ctrl() are only used by compat_ptrace_hbp_set(), so moved into CONFIG_COMPAT block and renamed with prefix compat_. For reference, the check is in arch/arm64/kernel/hw_breakpoint.c:hw_breakpoint_arch_parse() which is called via: modify_user_hw_breakpoint() -> modify_user_hw_breakpoint_check() -> hw_breakpoint_parse() -> hw_breakpoint_arch_parse() Signed-off-by: Bill Tsui <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
Which is also used by AArch64 and LoongArch.
To test this, I ran the test suite as usual on Arm and found no new
failures, then ran it again with all test programs compiled with
-mthumb. This means the binaries will be entirely Thumb code.Finally I tested it on AArch64, but this is mostly a build test,
as I did not run the entire test suite compiled to AArch32.
Prior to this change, these tests failed with
-mthumb:After this change, no new failures occurred. So I am confident it is
correct / as bad as it always was.
Looking at those failures, it's a few things:
likely nothing to do with the real breakpoints.
via. a veneer that might do a mode switch.
None of which are issues I feel like need fixing before doing this port.
I suspect there is redundant code in this, particularly aligning addresses. I've not made an effort to remove it because A: I'm scared to break Thumb support because it's not commonly used and B: it may be there to handle clients other than LLDB, which don't align breakpoint addresses before requesting them.