This revision was automatically updated to reflect the committed changes.
Closed by commit rGe38b0fa83a93: Remove AArch64 out of MIPS watchpoint-skip,
doc wp description (authored by jasonmolenda).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D147816/new/
https://reviews.llvm.org/D147816
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
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_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,
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,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);
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
@@ -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
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 (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
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
