jasonmolenda updated this revision to Diff 512577.
jasonmolenda added a comment.
Update patch to incorporate Jonas' & David's feedback. I'm holding this patch
until I can get the lldb changes to fix the reason:watchpoint `description`
handling corrected, or the test case in this patch will fail. This patch
changes debugserver's watchpoint notification to be the same as AArch64
lldb-server, but that currently skips over watchpoint traps that originate
outside the watched region and the test case will fail with that behavior.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D147820/new/
https://reviews.llvm.org/D147820
Files:
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 position \a bit is set in \a 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() > 2) {
+ 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,45 @@
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();
+ uint32_t delta = addr < start_addr ? start_addr - addr : 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,42 @@
+"""
+Watch one byte in the middle of a doubleword, mutate the
+entire doubleword including the watched byte. On AArch64
+the trap address is probably the start of the doubleword,
+instead of the address of our watched byte. Test that lldb
+correctly associates this watchpoint trap with our 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
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
