Interpret ESR/FAR bits directly on watchpoint exceptions in debugserver, clarify how watchpoint descriptions in stop packets work

[jasonmolenda]

Summary

This patch originally started with handling the case where a watchpoint traps when a write starts before the watched region, and the trap address reported may be outside the watched region. I wanted to find the nearest watched region on these events, and relay that up to lldb so they behave correctly. On reading about how the ESR and FAR registers contain details about the watchpoint, including possibly the watchpoint index that was triggered, I added code to parse those, use the watchpoint index if it's available, and pass along the other fields to lldb in case they become interesting.

On the lldb side, in the stop packet, watchpoints can be flagged by one of two ways. One is the bog standard gdb remote serial protocol watch:/awatch:/rwatch: followed by a watchpoint address. The second is an lldb extension to do "reason:watchpoint" followed by a "description:" whose value is an asciihex encoded string of three integers base10 encoded, space separated. These fields are an address within the watchpoint that was triggered, the watchpoint index, and optionally, the actual address that caused the watchpoint fault, which may be outside the range of any watched region. ProcessGDBRemote translates watch:/awatch:/rwatch: k-v pairs into this reason:watchpoint+description format (with only a single address).

This third integer was added by Jaydeep Patil in 2015 ( https://reviews.llvm.org/D11672 ) for supporting MIPS where the hardware can only watch 8 byte granules, so if you watch 4 bytes and an access happens within that 8B granule but outside that range, execution will stop. These false positive watchpoints are intended to be hidden from the user, so when we had a third integer that is not contained by a watchpoint region on MIPS, we would silently step past the watchpoint and continue without notifying the user.

In 2016 Omair Javaid ( https://reviews.llvm.org/D21516 ) started passing the actual trap address on Linux arm targets up as a third integer, and updated the conditional that turns this into a StopInfo so that the above "watchpoint hit outside a watched region" behavior would happen on arm. Arm systems have a different behavior than the MIPS one -- if you watch 8 bytes at address 0x1008 and someone does a 16-byte STP instruction starting at address 0x1000, 0x1000-0x100f are modified but the watchpoint trap address reported is 0x1000. This address is not within the range of any watched region, so lldb can fail to know how to proceed. I believe Omair was handling this, and while passing this value in the third argument is a good idea, enabling the same "silently step past this watchpoint" behavior was a mistake. This was all took me quite a while to figure out, I'm not surprised it was misunderstood. I've spent so much time staring at this I'm pretty sure I've got it correct now.

I documented how the three fields of this reason:watchpoint description should be provided. I've kept the MIPS behavior of skipping over false positive watchpoints, but instead of passing an address which is outside of any Watchpoint range to CreateStopReasonWithWatchpointID, I pass a boolean silently_continue instead. I think it is a little easier to understand what it's being used for. Nothing actually did anything with the address passed to CreateStopReasonWithWatchpointID except to detect that it was not contained within a watchpoint region, and use this to indicate that we're doing the silent skipping of the watchpoint.

I wanted debugserver to use this reason:watchpoint description style reporting, and that dragged me into figuring out what these fields were meant to be, and what the "silently skip watchpoint" behavior was intended to be handling. By the time I figured all the intended behaviors out, I needed to start documenting and, hopefully, clarifying them for when I forget again in a year.

Outside of the documentation and debugserver changes, the real changes to generic lldb are in ProcessGDBRemote::SetThreadStopInfo where I've documented what this is doing with the fields and, hopefully, made it a little easier to understand in the future. And a minor change to StopInfoWatchpoint/CreateStopReasonWithWatchpointID to use a boolean flag for this behavior.

I added a test which has a uint8_t[8] array and I watch one member of that, cast the address to a uint64_t* and write to it. I suspect this will Just Work on non-AArch64 targets (the watchpoint trap address will be the address of the uint8_t we're watching, instead of the start of the uint64_t* write I get on our AArch64 systems).

In debugserver, I'm sending all of the ESR fields that I decode up to lldb in the stop info packets. This is mostly to aid in debugging of these in the medium term, so we can see them in packet logs we request easily if we have any reports of problems with watchpoints. My next trick I'm going to do is look into adding MASK watchpoint support to debugserver in addition to BAS watchpoints. I'll slowly be ongoing with work in this area I think. It could be argued that these should be logged to console like most debugserver debug logging, only enabled when requested, but there will be no measurable perf impact from putting these in watchpoint stop notifications so I'm starting here.

I know the MIPS support was removed c. 2021 but it may be added again, and the ability to skip a false watchpoint hit is a useful one that we may need on AArch64 in the future if we can detect it appropriately.

[jasonmolenda]

One possible criticism I have of the current reason:watchpoint + description-up-to-three-integers is that we should add key-value pairs to the stop-info packet wp-address: wp-index: wp-hit-address and honestly, silently-continue: instead of doing an architectural hardcode in lldb. The remote stub is probably in the best position to know if this is a false positive watch trigger that needs to be silently moved past, instead of depending on "a third integer which isn't within the range of a watched region on MIPS". If I wanted to add a 4th field for "silently skip" to the current description, now I'm requiring that the stub reporting flag a false hit watchpoint needs to provide the first three always. And maybe it only has the one address or something.

[jasonmolenda]

Intermixing generic/linux AArch64 lldb watchpoint StopInfo changes in with a change to debugserver made this a difficult patch to ask anyone to review in total. I've separated it into one phab for the lldb watchpoint StopInfo handling https://reviews.llvm.org/D147816 and a separate phab for the debugserver changes to find the nearest watchpoint when we have a trap address outside any watched memory region and pass the ESR fields up to lldb https://reviews.llvm.org/D147820 . Closing this phab.

[omjavaid]

This comment was removed by omjavaid.

lldb/docs/lldb-gdb-remote.txt

 //                          "trace" the program stopped after a single instruction
 //                              was executed on a core. Usually done when single
 //                              stepping past a breakpoint
 //                          "breakpoint" a breakpoint set using a 'z' packet was hit.
 //                          "trap" stopped due to user interruption
 //                          "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 (v. 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
 //                                  request that this be included in the T packet via
 //                                  the QListThreadsInStopReply packet earlier in
 //                                  the debug session.
 //
 //                                  Example:

lldb/include/lldb/Target/StopInfo.h

public:
   CreateStopReasonWithBreakpointSiteID(Thread &thread,
                                        lldb::break_id_t break_id);
 
   // This creates a StopInfo for the thread where the should_stop is already
   // set, and won't be recalculated.
   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,
                              const char *description = nullptr,
                              std::optional<int> code = std::nullopt);
 
   static lldb::StopInfoSP CreateStopReasonToTrace(Thread &thread);
 

lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_arm64.h

public:
 
   bool WatchpointIsEnabled(uint32_t wp_index);
 
   // Debug register type select
   enum DREGType { eDREGTypeWATCH = 0, eDREGTypeBREAK };
 
 protected:
   // Debug register info for hardware breakpoints and watchpoints management.
+  // Watchpoints: For a user requested size 4 at addr 0x1004,
+  //   \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
                            // occurred.
     lldb::addr_t real_addr; // Address value that should cause target to stop.
     uint32_t control;       // Breakpoint/watchpoint control value.
   };
 

lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

             } else {
               StopInfoSP invalid_stop_info_sp;
               thread_sp->SetStopInfo(invalid_stop_info_sp);
             }
           }
         } 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;
+          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(
               *thread_sp, description.c_str()));
           handled = true;
         } else if (reason == "exec") {
           did_exec = true;
           thread_sp->SetStopInfo(

 
         WatchpointSP wp_sp =
             GetTarget().GetWatchpointList().FindByAddress(wp_addr);
         uint32_t wp_index = LLDB_INVALID_INDEX32;
 
         if (wp_sp)
           wp_index = wp_sp->GetHardwareIndex();
 
+        // Rewrite this to mimic the "reason:watchpoint" and
+        // "description:ADDR" entries in the
+        // stop-reply packet, handled in
+        // ProcessGDBRemote::SetThreadStopInfo
         reason = "watchpoint";
         StreamString ostr;
         ostr.Printf("%" PRIu64 " %" PRIu32, wp_addr, wp_index);
         description = std::string(ostr.GetString());
       } else if (key.compare("library") == 0) {
         auto error = LoadModules();
         if (error) {
           Log *log(GetLog(GDBRLog::Process));

lldb/source/Target/StopInfo.cpp

         return true;
     }
 
   private:
     ProcessSP process_sp;
     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)
+      : StopInfo(thread, watch_id), m_silently_skip(silently_skip) {}
 
   ~StopInfoWatchpoint() override = default;
 
   StopReason GetStopReason() const override { return eStopReasonWatchpoint; }
 
   const char *GetDescription() override {
     if (m_description.empty()) {
       StreamString strm;

         // This sentry object makes sure the current watchpoint is disabled
         // while performing watchpoint actions, and it is then enabled after we
         // are finished.
         ExecutionContext exe_ctx(thread_sp->GetStackFrameAtIndex(0));
         ProcessSP process_sp = exe_ctx.GetProcessSP();
 
         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) {
+          m_should_stop = false;
+          wp_sp->IncrementFalseAlarmsAndReviseHitCount();
         }
 
         if (wp_sp->GetHitCount() <= wp_sp->GetIgnoreCount()) {
           m_should_stop = false;
           m_should_stop_is_valid = true;
         }
 
         Debugger &debugger = exe_ctx.GetTargetRef().GetDebugger();

 private:
   void SetStepOverPlanComplete() {
     assert(m_using_step_over_plan);
     m_step_over_plan_complete = true;
   }
   
   bool m_should_stop = false;
   bool m_should_stop_is_valid = false;
-  lldb::addr_t m_watch_hit_addr;
+  bool m_silently_skip = false;
   bool m_step_over_plan_complete = false;
   bool m_using_step_over_plan = false;
 };
 
 // StopInfoUnixSignal
 
 class StopInfoUnixSignal : public StopInfo {
 public:

 }
 
 StopInfoSP StopInfo::CreateStopReasonWithBreakpointSiteID(Thread &thread,
                                                           break_id_t break_id,
                                                           bool should_stop) {
   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,
                                                 const char *description,
                                                 std::optional<int> code) {
   thread.GetProcess()->GetUnixSignals()->IncrementSignalHitCount(signo);
   return StopInfoSP(new StopInfoUnixSignal(thread, signo, description, code));
 }

lldb/test/API/commands/watchpoints/unaligned-watchpoint/Makefile

+C_SOURCES := main.c
+
+include Makefile.rules

lldb/test/API/commands/watchpoints/unaligned-watchpoint/TestUnalignedWatchpoint.py

+"""
+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'])

lldb/test/API/commands/watchpoints/unaligned-watchpoint/main.c

+#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)++);
+}
+

lldb/tools/debugserver/source/DNBBreakpoint.h

 class DNBBreakpointList {
 public:
   DNBBreakpointList();
   ~DNBBreakpointList();
 
   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);
 
   void Dump() const;
 
   size_t Size() const { return m_breakpoints.size(); }
   void DisableAll();

lldb/tools/debugserver/source/DNBBreakpoint.cpp

 const DNBBreakpoint *DNBBreakpointList::FindByAddress(nub_addr_t addr) const {
   const_iterator pos = m_breakpoints.find(addr);
   if (pos != m_breakpoints.end())
     return &pos->second;
 
   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) {
   bps.clear();
   iterator end = m_breakpoints.end();
   // Find the first breakpoint with an address >= to "addr"
   iterator pos = m_breakpoints.lower_bound(addr);
   if (pos != end) {

lldb/tools/debugserver/source/DNBDefs.h

struct DNBThreadResumeAction {
   nub_addr_t addr; // If not INVALID_NUB_ADDRESS, then set the PC for the thread
                    // to ADDR before resuming/stepping
 };
 
 enum DNBThreadStopType {
   eStopTypeInvalid = 0,
   eStopTypeSignal,
   eStopTypeException,
-  eStopTypeExec
+  eStopTypeExec,
+  eStopTypeWatchpoint
 };
 
 enum DNBMemoryPermissions {
   eMemoryPermissionsWritable = (1 << 0),
   eMemoryPermissionsReadable = (1 << 1),
   eMemoryPermissionsExecutable = (1 << 2)
 };
 
union {
     } signal;
 
     // eStopTypeException
     struct {
       uint32_t type;
       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;
 };
 
 struct DNBRegisterValue {
   struct DNBRegisterInfo info; // Register information for this register
   union {
     int8_t sint8;
     int16_t sint16;

lldb/tools/debugserver/source/MacOSX/MachException.cpp

bool MachException::Data::GetStopInfo(
   // Zero out the structure.
   memset(stop_info, 0, sizeof(struct DNBThreadStopInfo));
 
   if (exc_type == 0) {
     stop_info->reason = eStopTypeInvalid;
     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
   stop_info->details.exception.type = exc_type;
 
   // Fill in a text description
   const char *exc_name = MachException::Name(exc_type);
   char *desc = stop_info->description;

lldb/tools/debugserver/source/MacOSX/MachProcess.h

     eMachProcessProfileCancel = (1 << 0)
   };
 
   void Clear(bool detaching = false);
   void ReplyToAllExceptions();
   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);
 
   pid_t m_pid;           // Process ID of child process
   cpu_type_t m_cpu_type; // The CPU type of this process
   uint32_t m_platform;   // The platform of this process
   int m_child_stdin;

lldb/tools/debugserver/source/MacOSX/MachProcess.mm

   return m_thread_list.SetCurrentThread(tid);
 }
 
 bool MachProcess::GetThreadStoppedReason(nub_thread_t tid,
                                          struct DNBThreadStopInfo *stop_info) {
   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;
 }
 
 void MachProcess::DumpThreadStoppedReason(nub_thread_t tid) const {
   return m_thread_list.DumpThreadStoppedReason(tid);
 }

   if (m_profile_thread == NULL)
     return;
   m_profile_events.SetEvents(eMachProcessProfileCancel);
   pthread_join(m_profile_thread, NULL);
   m_profile_thread = NULL;
   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, &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, &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
   return ::pthread_create(&m_profile_thread, NULL, MachProcess::ProfileThread,
                           this) == 0;
 }
 
 nub_addr_t MachProcess::LookupSymbol(const char *name, const char *shlib) {

lldb/tools/debugserver/source/RNBRemote.cpp

         }
       }
     }
 
     if (g_num_reg_entries == 0)
       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,
                   g_reg_entries[reg].nub_info.reg, &reg_value))
             continue;
 
           debugserver_regnum_with_fixed_width_hex_register_value(
               ostrm, pid, tid, &g_reg_entries[reg], &reg_value);
         }
       }
     }
 
     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
             << ';';
       ostrm << "mecount:" << std::hex
             << tid_stop_info.details.exception.data_count << ';';
       for (nub_size_t i = 0; i < tid_stop_info.details.exception.data_count;
            ++i)
         ostrm << "medata:" << std::hex

               medata_array_sp->AddItem(
                   JSONGenerator::IntegerSP(new JSONGenerator::Integer(
                       tid_stop_info.details.exception.data[i])));
             }
             thread_dict_sp->AddItem("medata", medata_array_sp);
           }
           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;
         }
       }
 
       thread_dict_sp->AddStringItem("reason", reason_value);