diff --git a/lldb/docs/lldb-gdb-remote.txt b/lldb/docs/lldb-gdb-remote.txt --- a/lldb/docs/lldb-gdb-remote.txt +++ b/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 (see 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 diff --git a/lldb/include/lldb/Target/StopInfo.h b/lldb/include/lldb/Target/StopInfo.h --- a/lldb/include/lldb/Target/StopInfo.h +++ b/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, diff --git a/lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_arm64.h b/lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_arm64.h --- a/lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_arm64.h +++ b/lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_arm64.h @@ -55,7 +55,14 @@ enum DREGType { eDREGTypeWATCH = 0, eDREGTypeBREAK }; protected: - // Debug register info for hardware breakpoints and watchpoints management. + /// Debug register info for hardware breakpoints and watchpoints management. + /// Watchpoints: For a user requested size 4 at addr 0x1004, where BAS + /// watchpoints are at doubleword (8-byte) alignment. + /// \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 diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/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,9 @@ if (wp_sp) wp_index = wp_sp->GetHardwareIndex(); + // Rewrite gdb standard watch/rwatch/awatch to + // "reason:watchpoint" + "description:ADDR", + // which is parsed in SetThreadStopInfo. reason = "watchpoint"; StreamString ostr; ostr.Printf("%" PRIu64 " %" PRIu32, wp_addr, wp_index); diff --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp --- a/lldb/source/Target/StopInfo.cpp +++ b/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_wp) + : StopInfo(thread, watch_id), m_silently_skip_wp(silently_skip_wp) {} ~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_wp) { + m_should_stop = false; + wp_sp->IncrementFalseAlarmsAndReviseHitCount(); } if (wp_sp->GetHitCount() <= wp_sp->GetIgnoreCount()) { @@ -1035,7 +1016,17 @@ bool m_should_stop = false; bool m_should_stop_is_valid = false; - lldb::addr_t m_watch_hit_addr; + // A false watchpoint hit has happened - + // the thread stopped with a watchpoint + // hit notification, but the watched region + // was not actually accessed (as determined + // by the gdb stub we're talking to). + // Continue past this watchpoint without + // notifying the user; on some targets this + // may mean disable wp, instruction step, + // re-enable wp, continue. + // On others, just continue. + bool m_silently_skip_wp = false; bool m_step_over_plan_complete = false; bool m_using_step_over_plan = false; }; @@ -1372,10 +1363,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,