Skip to content

Commit

Permalink
Remove AArch64 out of MIPS watchpoint-skip, doc wp description
Browse files Browse the repository at this point in the history 
Watchpoints from lldb-server are sent in the stop info packet
as a `reason:watchpoint` and `description:asciihex` keys; the
latter's asciihex has one to three integer values.  This patch
documents the purpose of those three different numbers, and
clarifies the behavior on MIPS with the third number which is
outside the range of any watched memory range means to silently
skip the watchpoint.

lldb was previously using this silently skip watchpoint behavior
for AArch64 as well, but in the case of AArch64 we see a watchpoint
address outside of a watched memory range when the write BEGINS
before the watched memory range, but extends in to it.  We don't
want to silently skip these.

Differential Revision: https://reviews.llvm.org/D147816
rdar://83996471
  • Loading branch information
@jasonmolenda
jasonmolenda committed Apr 13, 2023
1 parent b0e0c1e commit e38b0fa
Show file tree
Hide file tree
Showing 5 changed files with 123 additions and 54 deletions.
52 changes: 46 additions & 6 deletions lldb/docs/lldb-gdb-remote.txt
View file
Expand Up @@ -1559,17 +1559,57 @@ for this region.
// "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
Expand Down
6 changes: 3 additions & 3 deletions lldb/include/lldb/Target/StopInfo.h
View file
Expand Up @@ -109,9 +109,9 @@ class StopInfo : public std::enable_shared_from_this<StopInfo> {
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,
Expand Down
9 changes: 8 additions & 1 deletion lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_arm64.h
View file
Expand Up @@ -55,7 +55,14 @@ class NativeRegisterContextDBReg_arm64
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
Expand Down
60 changes: 45 additions & 15 deletions lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
View file
Expand Up @@ -1749,33 +1749,60 @@ ThreadSP ProcessGDBRemote::SetThreadStopInfo(
} 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(
Expand Down Expand Up @@ -2202,6 +2229,9 @@ StateType ProcessGDBRemote::SetThreadStopInfo(StringExtractor &stop_packet) {
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);
Expand Down
50 changes: 21 additions & 29 deletions lldb/source/Target/StopInfo.cpp
View file
Expand Up @@ -666,9 +666,8 @@ class StopInfoWatchpoint : public StopInfo {
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;

Expand Down Expand Up @@ -893,27 +892,9 @@ class StopInfoWatchpoint : public StopInfo {

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()) {
Expand Down Expand Up @@ -1035,7 +1016,17 @@ class StopInfoWatchpoint : public StopInfo {

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;
};
Expand Down Expand Up @@ -1372,10 +1363,11 @@ StopInfoSP StopInfo::CreateStopReasonWithBreakpointSiteID(Thread &thread,
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,
Expand Down

0 comments on commit e38b0fa

Please sign in to comment.