jasonmolenda created this revision. jasonmolenda added a reviewer: JDevlieghere. jasonmolenda added a project: LLDB. Herald added subscribers: omjavaid, atanasyan, kristof.beyls, arichardson, sdardis. Herald added a project: All. jasonmolenda requested review of this revision. Herald added a subscriber: lldb-commits.
Currently on Darwin arm64 systems using debugserver, if you watch a memory region and a large write starts before the watched region, and extends in to the watched region, we will have a watchpoint exception but the address reported may be the start of the access -- before the watched memory range. In this case, lldb will not recognize which watchpoint to disable to step past the watchpoint and execution will stop, leaving the user to disable/stepi/re-enable manually. This patch takes the trap address (FAR register) and finds the nearest watchpoint if it is not contained in any watched region. It also parses the ESR register flags and if the processor reported the watchpoint index number instead of an address in the FAR register, handle that. Send (1) an address within the watched mem range, (2) the watchpoint hardware index, and (3) the actual trap address which may exist outside a watched mem range to lldb in a `description` string in the stop packet. Add a test case that has a uint8_t[8] array, watches a one-byte element in that array, and then does a 64-bit write to the entire array - so our FAR address may be the start of the uint8_t[8] array, and confirm that lldb correctly associates this with the watchpoint we set. This patch depends on https://reviews.llvm.org/D147816 ("Clarify how watchpoint description in stop packets work, fix AArch64 unintended behavior") being present. Without that patch, when we have a trap address outside the range of all watched memory ranges, lldb will silently continue past this watchpoint. Correct behavior on MIPS targets, but not on AArch64. Repository: rG LLVM Github Monorepo 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 "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
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits