jasonmolenda created this revision.
jasonmolenda added reviewers: labath, JDevlieghere, omjavaid, DavidSpickett.
jasonmolenda added a project: LLDB.
Herald added subscribers: atanasyan, kristof.beyls, arichardson, sdardis.
Herald added a project: All.
jasonmolenda requested review of this revision.
Herald added a subscriber: lldb-commits.
This patch originally started with handling the case where a watchpoint traps
when a write starts before the watched region, and the trap address reported
may be outside the watched region. I wanted to find the nearest watched region
on these events, and relay that up to lldb so they behave correctly. On
reading about how the ESR and FAR registers contain details about the
watchpoint, including possibly the watchpoint index that was triggered, I added
code to parse those, use the watchpoint index if it's available, and pass along
the other fields to lldb in case they become interesting.
On the lldb side, in the stop packet, watchpoints can be flagged by one of two
ways. One is the bog standard gdb remote serial protocol
watch:/awatch:/rwatch: followed by a watchpoint address. The second is an lldb
extension to do "reason:watchpoint" followed by a "description:" whose value is
an asciihex encoded string of three integers base10 encoded, space separated.
These fields are an address within the watchpoint that was triggered, the
watchpoint index, and optionally, the actual address that caused the watchpoint
fault, which may be outside the range of any watched region. ProcessGDBRemote
translates watch:/awatch:/rwatch: k-v pairs into this
reason:watchpoint+description format (with only a single address).
This third integer was added by Jaydeep Patil in 2015 (
https://reviews.llvm.org/D11672 ) for supporting MIPS where the hardware can
only watch 8 byte granules, so if you watch 4 bytes and an access happens
within that 8B granule but outside that range, execution will stop. These
false positive watchpoints are intended to be hidden from the user, so when we
had a third integer that is not contained by a watchpoint region on MIPS, we
would silently step past the watchpoint and continue without notifying the user.
In 2016 Omair Javaid ( https://reviews.llvm.org/D21516 ) started passing the
actual trap address on Linux arm targets up as a third integer, and updated the
conditional that turns this into a StopInfo so that the above "watchpoint hit
outside a watched region" behavior would happen on arm. Arm systems have a
different behavior than the MIPS one -- if you watch 8 bytes at address 0x1008
and someone does a 16-byte STP instruction starting at address 0x1000,
0x1000-0x100f are modified but the watchpoint trap address reported is 0x1000.
This address is not within the range of any watched region, so lldb can fail to
know how to proceed. I believe Omair was handling this, and while passing this
value in the third argument is a good idea, enabling the same "silently step
past this watchpoint" behavior was a mistake. This was all took me quite a
while to figure out, I'm not surprised it was misunderstood. I've spent so
much time staring at this I'm pretty sure I've got it correct now.
I documented how the three fields of this reason:watchpoint description should
be provided. I've kept the MIPS behavior of skipping over false positive
watchpoints, but instead of passing an address which is outside of any
Watchpoint range to `CreateStopReasonWithWatchpointID`, I pass a boolean
`silently_continue` instead. I think it is a little easier to understand what
it's being used for. Nothing actually did anything with the address passed to
`CreateStopReasonWithWatchpointID` except to detect that it was not contained
within a watchpoint region, and use this to indicate that we're doing the
silent skipping of the watchpoint.
I wanted debugserver to use this reason:watchpoint description style reporting,
and that dragged me into figuring out what these fields were meant to be, and
what the "silently skip watchpoint" behavior was intended to be handling. By
the time I figured all the intended behaviors out, I needed to start
documenting and, hopefully, clarifying them for when I forget again in a year.
Outside of the documentation and debugserver changes, the real changes to
generic lldb are in `ProcessGDBRemote::SetThreadStopInfo` where I've documented
what this is doing with the fields and, hopefully, made it a little easier to
understand in the future. And a minor change to
StopInfoWatchpoint/CreateStopReasonWithWatchpointID to use a boolean flag for
this behavior.
I added a test which has a uint8_t[8] array and I watch one member of that,
cast the address to a uint64_t* and write to it. I suspect this will Just Work
on non-AArch64 targets (the watchpoint trap address will be the address of the
uint8_t we're watching, instead of the start of the uint64_t* write I get on
our AArch64 systems).
In debugserver, I'm sending all of the ESR fields that I decode up to lldb in
the stop info packets. This is mostly to aid in debugging of these in the
medium term, so we can see them in packet logs we request easily if we have any
reports of problems with watchpoints. My next trick I'm going to do is look
into adding MASK watchpoint support to debugserver in addition to BAS
watchpoints. I'll slowly be ongoing with work in this area I think. It could
be argued that these should be logged to console like most debugserver debug
logging, only enabled when requested, but there will be no measurable perf
impact from putting these in watchpoint stop notifications so I'm starting here.
I know the MIPS support was removed c. 2021 but it may be added again, and the
ability to skip a false watchpoint hit is a useful one that we may need on
AArch64 in the future if we can detect it appropriately.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D147674
Files:
lldb/docs/lldb-gdb-remote.txt
lldb/include/lldb/Target/StopInfo.h
lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_arm64.h
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Target/StopInfo.cpp
lldb/test/API/commands/watchpoints/unaligned-watchpoint/Makefile
lldb/test/API/commands/watchpoints/unaligned-watchpoint/TestUnalignedWatchpoint.py
lldb/test/API/commands/watchpoints/unaligned-watchpoint/main.c
lldb/tools/debugserver/source/DNBBreakpoint.cpp
lldb/tools/debugserver/source/DNBBreakpoint.h
lldb/tools/debugserver/source/DNBDefs.h
lldb/tools/debugserver/source/MacOSX/MachException.cpp
lldb/tools/debugserver/source/MacOSX/MachProcess.h
lldb/tools/debugserver/source/MacOSX/MachProcess.mm
lldb/tools/debugserver/source/RNBRemote.cpp
Index: lldb/tools/debugserver/source/RNBRemote.cpp
===================================================================
--- lldb/tools/debugserver/source/RNBRemote.cpp
+++ lldb/tools/debugserver/source/RNBRemote.cpp
@@ -2841,11 +2841,21 @@
InitializeRegisters();
if (g_reg_entries != NULL) {
+ auto interesting_regset = [](int regset) -> bool {
+#if defined(__arm64__) || defined(__aarch64__)
+ // GPRs and exception registers, helpful for debugging
+ // from packet logs.
+ return regset == 1 || regset == 3;
+#else
+ return regset == 1;
+#endif
+ };
+
DNBRegisterValue reg_value;
for (uint32_t reg = 0; reg < g_num_reg_entries; reg++) {
// Expedite all registers in the first register set that aren't
// contained in other registers
- if (g_reg_entries[reg].nub_info.set == 1 &&
+ if (interesting_regset(g_reg_entries[reg].nub_info.set) &&
g_reg_entries[reg].nub_info.value_regs == NULL) {
if (!DNBThreadGetRegisterValueByID(
pid, tid, g_reg_entries[reg].nub_info.set,
@@ -2860,6 +2870,50 @@
if (did_exec) {
ostrm << "reason:exec;";
+ } else if (tid_stop_info.reason == eStopTypeWatchpoint) {
+ ostrm << "reason:watchpoint;";
+ ostrm << "description:";
+ std::ostringstream wp_desc;
+ wp_desc << tid_stop_info.details.watchpoint.addr << " ";
+ wp_desc << tid_stop_info.details.watchpoint.hw_idx << " ";
+ wp_desc << tid_stop_info.details.watchpoint.mach_exception_addr;
+ append_hexified_string(ostrm, wp_desc.str());
+ ostrm << ";";
+
+ // Temporarily, print all of the fields we've parsed out of the ESR
+ // on a watchpoint exception. Normally this is something we would
+ // log for LOG_WATCHPOINTS only, but this was implemented from the
+ // ARM ARM spec and hasn't been exercised on real hardware that can
+ // set most of these fields yet. It may need to be debugged in the
+ // future, so include all of these purely for debugging by human reasons.
+ ostrm << "watch_addr:" << std::hex
+ << tid_stop_info.details.watchpoint.addr << ";";
+ ostrm << "me_watch_addr:" << std::hex
+ << tid_stop_info.details.watchpoint.mach_exception_addr << ";";
+ ostrm << "wp_hw_idx:" << std::hex
+ << tid_stop_info.details.watchpoint.hw_idx << ";";
+ if (tid_stop_info.details.watchpoint.esr_fields_set) {
+ ostrm << "wp_esr_iss:" << std::hex
+ << tid_stop_info.details.watchpoint.esr_fields.iss << ";";
+ ostrm << "wp_esr_wpt:" << std::hex
+ << tid_stop_info.details.watchpoint.esr_fields.wpt << ";";
+ ostrm << "wp_esr_wptv:"
+ << tid_stop_info.details.watchpoint.esr_fields.wptv << ";";
+ ostrm << "wp_esr_wpf:"
+ << tid_stop_info.details.watchpoint.esr_fields.wpf << ";";
+ ostrm << "wp_esr_fnp:"
+ << tid_stop_info.details.watchpoint.esr_fields.fnp << ";";
+ ostrm << "wp_esr_vncr:"
+ << tid_stop_info.details.watchpoint.esr_fields.vncr << ";";
+ ostrm << "wp_esr_fnv:"
+ << tid_stop_info.details.watchpoint.esr_fields.fnv << ";";
+ ostrm << "wp_esr_cm:" << tid_stop_info.details.watchpoint.esr_fields.cm
+ << ";";
+ ostrm << "wp_esr_wnr:"
+ << tid_stop_info.details.watchpoint.esr_fields.wnr << ";";
+ ostrm << "wp_esr_dfsc:" << std::hex
+ << tid_stop_info.details.watchpoint.esr_fields.dfsc << ";";
+ }
} else if (tid_stop_info.details.exception.type) {
ostrm << "metype:" << std::hex << tid_stop_info.details.exception.type
<< ';';
@@ -5475,6 +5529,20 @@
}
break;
+ case eStopTypeWatchpoint: {
+ reason_value = "watchpoint";
+ thread_dict_sp->AddIntegerItem("watchpoint",
+ tid_stop_info.details.watchpoint.addr);
+ thread_dict_sp->AddIntegerItem(
+ "me_watch_addr",
+ tid_stop_info.details.watchpoint.mach_exception_addr);
+ std::ostringstream wp_desc;
+ wp_desc << tid_stop_info.details.watchpoint.addr << " ";
+ wp_desc << tid_stop_info.details.watchpoint.hw_idx << " ";
+ wp_desc << tid_stop_info.details.watchpoint.mach_exception_addr;
+ thread_dict_sp->AddStringItem("description", wp_desc.str());
+ } break;
+
case eStopTypeExec:
reason_value = "exec";
break;
Index: lldb/tools/debugserver/source/MacOSX/MachProcess.mm
===================================================================
--- lldb/tools/debugserver/source/MacOSX/MachProcess.mm
+++ lldb/tools/debugserver/source/MacOSX/MachProcess.mm
@@ -1277,6 +1277,8 @@
if (m_thread_list.GetThreadStoppedReason(tid, stop_info)) {
if (m_did_exec)
stop_info->reason = eStopTypeExec;
+ if (stop_info->reason == eStopTypeWatchpoint)
+ RefineWatchpointStopInfo(tid, stop_info);
return true;
}
return false;
@@ -1420,6 +1422,135 @@
m_profile_events.ResetEvents(eMachProcessProfileCancel);
}
+// return 1 if bit "BIT" is set in "value"
+static uint32_t bit(uint32_t value, uint32_t bit) {
+ return (value >> bit) & 1u;
+}
+
+// return the bitfield "value[msbit:lsbit]".
+static uint64_t bits(uint64_t value, uint32_t msbit, uint32_t lsbit) {
+ assert(msbit >= lsbit);
+ uint64_t shift_left = sizeof(value) * 8 - 1 - msbit;
+ value <<=
+ shift_left; // shift anything above the msbit off of the unsigned edge
+ value >>= shift_left + lsbit; // shift it back again down to the lsbit
+ // (including undoing any shift from above)
+ return value; // return our result
+}
+
+void MachProcess::RefineWatchpointStopInfo(
+ nub_thread_t tid, struct DNBThreadStopInfo *stop_info) {
+ const DNBBreakpoint *wp = m_watchpoints.FindNearestWatchpoint(
+ stop_info->details.watchpoint.mach_exception_addr);
+ if (wp) {
+ stop_info->details.watchpoint.addr = wp->Address();
+ stop_info->details.watchpoint.hw_idx = wp->GetHardwareIndex();
+ DNBLogThreadedIf(LOG_WATCHPOINTS,
+ "MachProcess::RefineWatchpointStopInfo "
+ "mach exception addr 0x%llx moved in to nearest "
+ "watchpoint, 0x%llx-0x%llx",
+ stop_info->details.watchpoint.mach_exception_addr,
+ wp->Address(), wp->Address() + wp->ByteSize() - 1);
+ } else {
+ stop_info->details.watchpoint.addr =
+ stop_info->details.watchpoint.mach_exception_addr;
+ }
+
+ stop_info->details.watchpoint.esr_fields_set = false;
+ std::optional<uint64_t> esr, far;
+ nub_size_t num_reg_sets = 0;
+ const DNBRegisterSetInfo *reg_sets = GetRegisterSetInfo(tid, &num_reg_sets);
+ for (nub_size_t set = 0; set < num_reg_sets; set++) {
+ if (reg_sets[set].registers == NULL)
+ continue;
+ for (uint32_t reg = 0; reg < reg_sets[set].num_registers; ++reg) {
+ if (strcmp(reg_sets[set].registers[reg].name, "esr") == 0) {
+ DNBRegisterValue reg_value;
+ if (GetRegisterValue(tid, set, reg, ®_value)) {
+ esr = reg_value.value.uint64;
+ }
+ }
+ if (strcmp(reg_sets[set].registers[reg].name, "far") == 0) {
+ DNBRegisterValue reg_value;
+ if (GetRegisterValue(tid, set, reg, ®_value)) {
+ far = reg_value.value.uint64;
+ }
+ }
+ }
+ }
+
+ if (esr && far) {
+ if (*far != stop_info->details.watchpoint.mach_exception_addr) {
+ // AFAIK the kernel is going to put the FAR value in the mach
+ // exception, if they don't match, it's interesting enough to log it.
+ DNBLogThreadedIf(LOG_WATCHPOINTS,
+ "MachProcess::RefineWatchpointStopInfo mach exception "
+ "addr 0x%llx but FAR register has value 0x%llx",
+ stop_info->details.watchpoint.mach_exception_addr, *far);
+ }
+ uint32_t exception_class = bits(*esr, 31, 26);
+
+ // "Watchpoint exception from a lower Exception level"
+ if (exception_class == 0b110100) {
+ stop_info->details.watchpoint.esr_fields_set = true;
+ // Documented in the ARM ARM A-Profile Dec 2022 edition
+ // Section D17.2 ("General system control registers"),
+ // Section D17.2.37 "ESR_EL1, Exception Syndrome Register (EL1)",
+ // "Field Descriptions"
+ // "ISS encoding for an exception from a Watchpoint exception"
+ uint32_t iss = bits(*esr, 23, 0);
+ stop_info->details.watchpoint.esr_fields.iss = iss;
+ stop_info->details.watchpoint.esr_fields.wpt =
+ bits(iss, 23, 18); // Watchpoint number
+ stop_info->details.watchpoint.esr_fields.wptv =
+ bit(iss, 17); // Watchpoint number Valid
+ stop_info->details.watchpoint.esr_fields.wpf =
+ bit(iss, 16); // Watchpoint might be false-positive
+ stop_info->details.watchpoint.esr_fields.fnp =
+ bit(iss, 15); // FAR not Precise
+ stop_info->details.watchpoint.esr_fields.vncr =
+ bit(iss, 13); // watchpoint from use of VNCR_EL2 reg by EL1
+ stop_info->details.watchpoint.esr_fields.fnv =
+ bit(iss, 10); // FAR not Valid
+ stop_info->details.watchpoint.esr_fields.cm =
+ bit(iss, 6); // Cache maintenance
+ stop_info->details.watchpoint.esr_fields.wnr =
+ bit(iss, 6); // Write not Read
+ stop_info->details.watchpoint.esr_fields.dfsc =
+ bits(iss, 5, 0); // Data Fault Status Code
+
+ DNBLogThreadedIf(LOG_WATCHPOINTS,
+ "ESR watchpoint fields parsed: "
+ "iss = 0x%x, wpt = %u, wptv = %d, wpf = %d, fnp = %d, "
+ "vncr = %d, fnv = %d, cm = %d, wnr = %d, dfsc = 0x%x",
+ stop_info->details.watchpoint.esr_fields.iss,
+ stop_info->details.watchpoint.esr_fields.wpt,
+ stop_info->details.watchpoint.esr_fields.wptv,
+ stop_info->details.watchpoint.esr_fields.wpf,
+ stop_info->details.watchpoint.esr_fields.fnp,
+ stop_info->details.watchpoint.esr_fields.vncr,
+ stop_info->details.watchpoint.esr_fields.fnv,
+ stop_info->details.watchpoint.esr_fields.cm,
+ stop_info->details.watchpoint.esr_fields.wnr,
+ stop_info->details.watchpoint.esr_fields.dfsc);
+
+ if (stop_info->details.watchpoint.esr_fields.wptv) {
+ DNBLogThreadedIf(LOG_WATCHPOINTS,
+ "Watchpoint Valid field true, "
+ "finding startaddr of watchpoint %d",
+ stop_info->details.watchpoint.esr_fields.wpt);
+ stop_info->details.watchpoint.hw_idx =
+ stop_info->details.watchpoint.esr_fields.wpt;
+ const DNBBreakpoint *wp = m_watchpoints.FindByHardwareIndex(
+ stop_info->details.watchpoint.esr_fields.wpt);
+ if (wp) {
+ stop_info->details.watchpoint.addr = wp->Address();
+ }
+ }
+ }
+ }
+}
+
bool MachProcess::StartProfileThread() {
DNBLogThreadedIf(LOG_PROCESS, "MachProcess::%s ( )", __FUNCTION__);
// Create the thread that profiles the inferior and reports back if enabled
Index: lldb/tools/debugserver/source/MacOSX/MachProcess.h
===================================================================
--- lldb/tools/debugserver/source/MacOSX/MachProcess.h
+++ lldb/tools/debugserver/source/MacOSX/MachProcess.h
@@ -383,6 +383,9 @@
void PrivateResume();
void StopProfileThread();
+ void RefineWatchpointStopInfo(nub_thread_t tid,
+ struct DNBThreadStopInfo *stop_info);
+
uint32_t Flags() const { return m_flags; }
nub_state_t DoSIGSTOP(bool clear_bps_and_wps, bool allow_running,
uint32_t *thread_idx_ptr);
Index: lldb/tools/debugserver/source/MacOSX/MachException.cpp
===================================================================
--- lldb/tools/debugserver/source/MacOSX/MachException.cpp
+++ lldb/tools/debugserver/source/MacOSX/MachException.cpp
@@ -158,6 +158,19 @@
return true;
}
+#if defined(__arm64__) || defined(__aarch64__)
+ if (exc_type == EXC_BREAKPOINT && exc_data[0] == EXC_ARM_DA_DEBUG &&
+ exc_data.size() > 1) {
+ stop_info->reason = eStopTypeWatchpoint;
+ stop_info->details.watchpoint.mach_exception_addr = exc_data[1];
+ stop_info->details.watchpoint.addr = INVALID_NUB_ADDRESS;
+ if (exc_data.size() >= 3) {
+ stop_info->details.watchpoint.hw_idx = exc_data[2];
+ }
+ return true;
+ }
+#endif
+
// We always stop with a mach exceptions
stop_info->reason = eStopTypeException;
// Save the EXC_XXXX exception type
Index: lldb/tools/debugserver/source/DNBDefs.h
===================================================================
--- lldb/tools/debugserver/source/DNBDefs.h
+++ lldb/tools/debugserver/source/DNBDefs.h
@@ -234,7 +234,8 @@
eStopTypeInvalid = 0,
eStopTypeSignal,
eStopTypeException,
- eStopTypeExec
+ eStopTypeExec,
+ eStopTypeWatchpoint
};
enum DNBMemoryPermissions {
@@ -264,6 +265,37 @@
nub_size_t data_count;
nub_addr_t data[DNB_THREAD_STOP_INFO_MAX_EXC_DATA];
} exception;
+
+ // eStopTypeWatchpoint
+ struct {
+ // The trigger address from the mach exception
+ // (likely the contents of the FAR register)
+ nub_addr_t mach_exception_addr;
+
+ // The trigger address, adjusted to be the start
+ // address of one of the existing watchpoints for
+ // lldb's benefit.
+ nub_addr_t addr;
+
+ // The watchpoint hardware index.
+ uint32_t hw_idx;
+
+ // If the esr_fields bitfields have been filled in.
+ bool esr_fields_set;
+ struct {
+ uint32_t
+ iss; // "ISS encoding for an exception from a Watchpoint exception"
+ uint32_t wpt; // Watchpoint number
+ bool wptv; // Watchpoint number Valid
+ bool wpf; // Watchpoint might be false-positive
+ bool fnp; // FAR not Precise
+ bool vncr; // watchpoint from use of VNCR_EL2 reg by EL1
+ bool fnv; // FAR not Valid
+ bool cm; // Cache maintenance
+ bool wnr; // Write not Read
+ uint32_t dfsc; // Data Fault Status Code
+ } esr_fields;
+ } watchpoint;
} details;
};
Index: lldb/tools/debugserver/source/DNBBreakpoint.h
===================================================================
--- lldb/tools/debugserver/source/DNBBreakpoint.h
+++ lldb/tools/debugserver/source/DNBBreakpoint.h
@@ -122,7 +122,9 @@
DNBBreakpoint *Add(nub_addr_t addr, nub_size_t length, bool hardware);
bool Remove(nub_addr_t addr);
DNBBreakpoint *FindByAddress(nub_addr_t addr);
+ const DNBBreakpoint *FindNearestWatchpoint(nub_addr_t addr) const;
const DNBBreakpoint *FindByAddress(nub_addr_t addr) const;
+ const DNBBreakpoint *FindByHardwareIndex(uint32_t idx) const;
size_t FindBreakpointsThatOverlapRange(nub_addr_t addr, nub_addr_t size,
std::vector<DNBBreakpoint *> &bps);
Index: lldb/tools/debugserver/source/DNBBreakpoint.cpp
===================================================================
--- lldb/tools/debugserver/source/DNBBreakpoint.cpp
+++ lldb/tools/debugserver/source/DNBBreakpoint.cpp
@@ -82,6 +82,53 @@
return NULL;
}
+const DNBBreakpoint *
+DNBBreakpointList::FindByHardwareIndex(uint32_t idx) const {
+ for (const auto &pos : m_breakpoints)
+ if (pos.second.GetHardwareIndex() == idx)
+ return &pos.second;
+
+ return nullptr;
+}
+
+const DNBBreakpoint *
+DNBBreakpointList::FindNearestWatchpoint(nub_addr_t addr) const {
+ // Exact match
+ for (const auto &pos : m_breakpoints) {
+ if (pos.second.IsEnabled()) {
+ nub_addr_t start_addr = pos.second.Address();
+ nub_addr_t end_addr = start_addr + pos.second.ByteSize();
+ if (addr >= start_addr && addr <= end_addr)
+ return &pos.second;
+ }
+ }
+
+ // Find watchpoint nearest to this address
+ // before or after the watched region of memory
+ const DNBBreakpoint *closest = nullptr;
+ uint32_t best_match = UINT32_MAX;
+ for (const auto &pos : m_breakpoints) {
+ if (pos.second.IsEnabled()) {
+ nub_addr_t start_addr = pos.second.Address();
+ nub_addr_t end_addr = start_addr + pos.second.ByteSize();
+ if (addr < start_addr) {
+ uint32_t delta = start_addr - addr;
+ if (delta < best_match) {
+ closest = &pos.second;
+ best_match = delta;
+ } else {
+ uint32_t delta = addr - end_addr;
+ if (delta < best_match) {
+ closest = &pos.second;
+ best_match = delta;
+ }
+ }
+ }
+ }
+ }
+ return closest;
+}
+
// Finds the next breakpoint at an address greater than or equal to "addr"
size_t DNBBreakpointList::FindBreakpointsThatOverlapRange(
nub_addr_t addr, nub_addr_t size, std::vector<DNBBreakpoint *> &bps) {
Index: lldb/test/API/commands/watchpoints/unaligned-watchpoint/main.c
===================================================================
--- /dev/null
+++ lldb/test/API/commands/watchpoints/unaligned-watchpoint/main.c
@@ -0,0 +1,13 @@
+#include <stdint.h>
+#include <stdio.h>
+int main()
+{
+ uint8_t buf[8];
+ uint64_t *u64_p = (uint64_t *) buf;
+ *u64_p = 5; // break here
+ printf("%lld %lld\n", (*u64_p)++, (*u64_p)++);
+ printf("%lld %lld\n", (*u64_p)++, (*u64_p)++);
+ printf("%lld %lld\n", (*u64_p)++, (*u64_p)++);
+ printf("%lld %lld\n", (*u64_p)++, (*u64_p)++);
+}
+
Index: lldb/test/API/commands/watchpoints/unaligned-watchpoint/TestUnalignedWatchpoint.py
===================================================================
--- /dev/null
+++ lldb/test/API/commands/watchpoints/unaligned-watchpoint/TestUnalignedWatchpoint.py
@@ -0,0 +1,40 @@
+"""
+Put one byte watchpoint in the middle of a doubleword, mutate the
+entire doubleword including the watched byte, to test correctly
+associating the fault with the set watchpoint.
+"""
+
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class UnalignedWatchpointTestCase(TestBase):
+ NO_DEBUG_INFO_TESTCASE = True
+ @skipIfOutOfTreeDebugserver
+ def test_unaligned_watchpoint(self):
+ """Test an unaligned watchpoint triggered by a larger aligned write."""
+ self.build()
+ self.main_source_file = lldb.SBFileSpec("main.c")
+ (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self,
+ "break here", self.main_source_file)
+
+ frame = thread.GetFrameAtIndex(0)
+
+ self.expect("watchpoint set variable buf[2]")
+
+ self.runCmd("process continue")
+
+ # We should be stopped again due to the watchpoint (write type), but
+ # only once. The stop reason of the thread should be watchpoint.
+ self.expect("thread list", STOPPED_DUE_TO_WATCHPOINT,
+ substrs=['stopped',
+ 'stop reason = watchpoint'])
+
+ # Use the '-v' option to do verbose listing of the watchpoint.
+ # The hit count should now be 1.
+ self.expect("watchpoint list -v",
+ substrs=['hit_count = 1'])
Index: lldb/test/API/commands/watchpoints/unaligned-watchpoint/Makefile
===================================================================
--- /dev/null
+++ lldb/test/API/commands/watchpoints/unaligned-watchpoint/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules
Index: lldb/source/Target/StopInfo.cpp
===================================================================
--- lldb/source/Target/StopInfo.cpp
+++ lldb/source/Target/StopInfo.cpp
@@ -666,9 +666,8 @@
WatchpointSP watchpoint_sp;
};
- StopInfoWatchpoint(Thread &thread, break_id_t watch_id,
- lldb::addr_t watch_hit_addr)
- : StopInfo(thread, watch_id), m_watch_hit_addr(watch_hit_addr) {}
+ StopInfoWatchpoint(Thread &thread, break_id_t watch_id, bool silently_skip)
+ : StopInfo(thread, watch_id), m_silently_skip(silently_skip) {}
~StopInfoWatchpoint() override = default;
@@ -893,27 +892,9 @@
WatchpointSentry sentry(process_sp, wp_sp);
- /*
- * MIPS: Last 3bits of the watchpoint address are masked by the kernel.
- * For example:
- * 'n' is at 0x120010d00 and 'm' is 0x120010d04. When a watchpoint is
- * set at 'm', then
- * watch exception is generated even when 'n' is read/written. To handle
- * this case,
- * server emulates the instruction at PC and finds the base address of
- * the load/store
- * instruction and appends it in the description of the stop-info
- * packet. If watchpoint
- * is not set on this address by user then this do not stop.
- */
- if (m_watch_hit_addr != LLDB_INVALID_ADDRESS) {
- WatchpointSP wp_hit_sp =
- thread_sp->CalculateTarget()->GetWatchpointList().FindByAddress(
- m_watch_hit_addr);
- if (!wp_hit_sp) {
- m_should_stop = false;
- wp_sp->IncrementFalseAlarmsAndReviseHitCount();
- }
+ if (m_silently_skip) {
+ m_should_stop = false;
+ wp_sp->IncrementFalseAlarmsAndReviseHitCount();
}
if (wp_sp->GetHitCount() <= wp_sp->GetIgnoreCount()) {
@@ -1035,7 +1016,7 @@
bool m_should_stop = false;
bool m_should_stop_is_valid = false;
- lldb::addr_t m_watch_hit_addr;
+ bool m_silently_skip = false;
bool m_step_over_plan_complete = false;
bool m_using_step_over_plan = false;
};
@@ -1372,10 +1353,11 @@
return StopInfoSP(new StopInfoBreakpoint(thread, break_id, should_stop));
}
-StopInfoSP
-StopInfo::CreateStopReasonWithWatchpointID(Thread &thread, break_id_t watch_id,
- lldb::addr_t watch_hit_addr) {
- return StopInfoSP(new StopInfoWatchpoint(thread, watch_id, watch_hit_addr));
+StopInfoSP StopInfo::CreateStopReasonWithWatchpointID(Thread &thread,
+ break_id_t watch_id,
+ bool silently_continue) {
+ return StopInfoSP(
+ new StopInfoWatchpoint(thread, watch_id, silently_continue));
}
StopInfoSP StopInfo::CreateStopReasonWithSignal(Thread &thread, int signo,
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1749,33 +1749,60 @@
} else if (reason == "trap") {
// Let the trap just use the standard signal stop reason below...
} else if (reason == "watchpoint") {
+ // We will have between 1 and 3 fields in the description.
+ //
+ // \a wp_addr which is the original start address that
+ // lldb requested be watched, or an address that the
+ // hardware reported. This address should be within the
+ // range of a currently active watchpoint region - lldb
+ // should be able to find a watchpoint with this address.
+ //
+ // \a wp_index is the hardware watchpoint register number.
+ //
+ // \a wp_hit_addr is the actual address reported by the hardware,
+ // which may be outside the range of a region we are watching.
+ //
+ // On MIPS, we may get a false watchpoint exception where an
+ // access to the same 8 byte granule as a watchpoint will trigger,
+ // even if the access was not within the range of the watched
+ // region. When we get a \a wp_hit_addr outside the range of any
+ // set watchpoint, continue execution without making it visible to
+ // the user.
+ //
+ // On ARM, a related issue where a large access that starts
+ // before the watched region (and extends into the watched
+ // region) may report a hit address before the watched region.
+ // lldb will not find the "nearest" watchpoint to
+ // disable/step/re-enable it, so one of the valid watchpoint
+ // addresses should be provided as \a wp_addr.
StringExtractor desc_extractor(description.c_str());
addr_t wp_addr = desc_extractor.GetU64(LLDB_INVALID_ADDRESS);
uint32_t wp_index = desc_extractor.GetU32(LLDB_INVALID_INDEX32);
addr_t wp_hit_addr = desc_extractor.GetU64(LLDB_INVALID_ADDRESS);
watch_id_t watch_id = LLDB_INVALID_WATCH_ID;
- if (wp_addr != LLDB_INVALID_ADDRESS) {
- WatchpointSP wp_sp;
+ bool silently_continue = false;
+ WatchpointSP wp_sp;
+ if (wp_hit_addr != LLDB_INVALID_ADDRESS) {
+ wp_sp = GetTarget().GetWatchpointList().FindByAddress(wp_hit_addr);
+ // On MIPS, \a wp_hit_addr outside the range of a watched
+ // region means we should silently continue, it is a false hit.
ArchSpec::Core core = GetTarget().GetArchitecture().GetCore();
- if ((core >= ArchSpec::kCore_mips_first &&
- core <= ArchSpec::kCore_mips_last) ||
- (core >= ArchSpec::eCore_arm_generic &&
- core <= ArchSpec::eCore_arm_aarch64))
- wp_sp =
- GetTarget().GetWatchpointList().FindByAddress(wp_hit_addr);
- if (!wp_sp)
- wp_sp = GetTarget().GetWatchpointList().FindByAddress(wp_addr);
- if (wp_sp) {
- wp_sp->SetHardwareIndex(wp_index);
- watch_id = wp_sp->GetID();
- }
+ if (!wp_sp && core >= ArchSpec::kCore_mips_first &&
+ core <= ArchSpec::kCore_mips_last)
+ silently_continue = true;
+ }
+ if (!wp_sp && wp_addr != LLDB_INVALID_ADDRESS)
+ wp_sp = GetTarget().GetWatchpointList().FindByAddress(wp_addr);
+ if (wp_sp) {
+ wp_sp->SetHardwareIndex(wp_index);
+ watch_id = wp_sp->GetID();
}
if (watch_id == LLDB_INVALID_WATCH_ID) {
Log *log(GetLog(GDBRLog::Watchpoints));
LLDB_LOGF(log, "failed to find watchpoint");
}
thread_sp->SetStopInfo(StopInfo::CreateStopReasonWithWatchpointID(
- *thread_sp, watch_id, wp_hit_addr));
+ *thread_sp, watch_id, silently_continue));
handled = true;
} else if (reason == "exception") {
thread_sp->SetStopInfo(StopInfo::CreateStopReasonWithException(
@@ -2202,6 +2229,10 @@
if (wp_sp)
wp_index = wp_sp->GetHardwareIndex();
+ // Rewrite this to mimic the "reason:watchpoint" and
+ // "description:ADDR" entries in the
+ // stop-reply packet, handled in
+ // ProcessGDBRemote::SetThreadStopInfo
reason = "watchpoint";
StreamString ostr;
ostr.Printf("%" PRIu64 " %" PRIu32, wp_addr, wp_index);
Index: lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_arm64.h
===================================================================
--- lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_arm64.h
+++ lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_arm64.h
@@ -56,6 +56,12 @@
protected:
// Debug register info for hardware breakpoints and watchpoints management.
+ // Watchpoints: For a user requested size 4 at addr 0x1004,
+ // \a real_addr is 0x1004
+ // \a address is 0x1000
+ // size is 8
+ // If a one-byte write to 0x1006 is the most recent watchpoint trap,
+ // \a hit_addr is 0x1006
struct DREG {
lldb::addr_t address; // Breakpoint/watchpoint address value.
lldb::addr_t hit_addr; // Address at which last watchpoint trigger exception
Index: lldb/include/lldb/Target/StopInfo.h
===================================================================
--- lldb/include/lldb/Target/StopInfo.h
+++ lldb/include/lldb/Target/StopInfo.h
@@ -109,9 +109,9 @@
static lldb::StopInfoSP CreateStopReasonWithBreakpointSiteID(
Thread &thread, lldb::break_id_t break_id, bool should_stop);
- static lldb::StopInfoSP CreateStopReasonWithWatchpointID(
- Thread &thread, lldb::break_id_t watch_id,
- lldb::addr_t watch_hit_addr = LLDB_INVALID_ADDRESS);
+ static lldb::StopInfoSP
+ CreateStopReasonWithWatchpointID(Thread &thread, lldb::break_id_t watch_id,
+ bool silently_continue = false);
static lldb::StopInfoSP
CreateStopReasonWithSignal(Thread &thread, int signo,
Index: lldb/docs/lldb-gdb-remote.txt
===================================================================
--- lldb/docs/lldb-gdb-remote.txt
+++ lldb/docs/lldb-gdb-remote.txt
@@ -1559,17 +1559,57 @@
// "signal" stopped due to an actual unix signal, not
// just the debugger using a unix signal to keep
// the GDB remote client happy.
-// "watchpoint". Should be used in conjunction with
-// the "watch"/"rwatch"/"awatch" key value pairs.
+// "watchpoint". Can be used with of the
+// "watch"/"rwatch"/"awatch" key value pairs.
+// Or can be used *instead* of those keys,
+// with the specially formatted "description" field.
// "exception" an exception stop reason. Use with
// the "description" key/value pair to describe the
// exceptional event the user should see as the stop
// reason.
// "description" ascii-hex An ASCII hex string that contains a more descriptive
-// reason that the thread stopped. This is only needed
-// if none of the key/value pairs are enough to
-// describe why something stopped.
-//
+// reason that the thread stopped. This is only needed
+// if none of the key/value pairs are enough to
+// describe why something stopped.
+//
+// For "reason:watchpoint", "description" is an ascii-hex
+// encoded string with between one and three base10 numbers,
+// space separated. The three numbers are
+// 1. watchpoint address. This address should always be within
+// a memory region lldb has a watchpoint on.
+// On architectures where the actual reported hit address may
+// be outside the watchpoint that was triggered, the remote
+// stub should determine which watchpoint was triggered and
+// report an address from within its range.
+// 2. watchpoint hardware register index number.
+// 3. actual watchpoint trap address, which may be outside
+// the range of any watched region of memory. On MIPS, an addr
+// outside a watched range means lldb should disable the wp,
+// step, re-enable the wp and continue silently.
+//
+// On MIPS, the low 3 bits are masked so if a watchpoint is on
+// 0x1004, a 2-byte write to 0x1000 will trigger the watchpoint
+// (a false positive hit), and lldb needs to disable the
+// watchpoint at 0x1004, inst-step, then re-enable the watchpoint
+// and not make this a user visible event. The description here
+// would be "0x1004 0 0x1000". lldb needs a known watchpoint address
+// in the first field, so it can disable it & step.
+//
+// On AArch64 we have a related issue, where you watch 4 bytes at
+// 0x1004, an instruction does an 8-byte write starting at
+// 0x1000 (a true watchpoint hit) and the hardware may report the
+// trap address as 0x1000 - before the watched memory region -
+// with the write extending into the watched region. This can
+// be reported as "0x1004 0 0x1000". lldb will use 0x1004 to
+// identify which Watchpoint was triggered, and can report 0x1000
+// to the user. The behavior of silently stepping over the
+// watchpoint, with an 3rd field addr outside the range, is
+// restricted to MIPS.
+// There may be false-positive watchpoint hits on AArch64 as well,
+// in the SVE Streaming Mode, but that is less common (v. ESR
+// register flag "WPF", "Watchpoint might be False-Positive") and
+// not currently handled by lldb.
+//
// "threads" comma-sep-base16 A list of thread ids for all threads (including
// the thread that we're reporting as stopped) that
// are live in the process right now. lldb may
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
