Clarify how watchpoint description in stop packets work, fix AArch64 unintended behavior

[jasonmolenda]

Summary

Orig posted in https://reviews.llvm.org/D147674 as part of a debugserver patch, creating two phabs to track those separate patches so it's easier to review.

This patch fixes a problem with watchpoints on AArch targets using lldb-server (and soon debugserver), where a large write that begins before a watched region, and extends into the watched region, is reported as a trap address before the watched memory range. I can show this on an AArch64 Ubuntu install:

{
   uint8_t buf[8];
   uint64_t *u64_p = (uint64_t *) buf;
   *u64_p = 5; // break here
   (*u64_p)++;

At the break here line, if and do watch set variable buf[2] and c, the packets from lldb server look like

<  21> send packet: $Z2,fffffffff36a,1#75
<   6> read packet: $OK#9a
<   5> send packet: $c#63

<838> read packet: $T05thread:493;name:a.out;  [...] ;reason:watchpoint;description:323831343734393736373037343334203320323831343734393736373037343332;#db

The description is 281474976707434 3 281474976707432 aka 0x0000fffffffff36a 3 0x0000fffffffff368. The first address is an address contained within the hit watchpoint. The second number is the hardware register index. The third number is the actual address that was accessed,

(lldb)  v &buf u64_p
(uint8_t (*)[8]) &buf = 0x0000fffffffff368
(uint64_t *) u64_p = 0x0000fffffffff368
(lldb)

Right now, lldb has a rule that "for MIPS an ARM, if the third number is NOT contained within any watchpoint region, silently continue past this watchpoint". From the user's perspective, lldb misses the watchpoint modification. We see this a lot on macOS with bzero()/memset() when you're watching an element of an object and it is overwritten.

For MIPS this behavior is correct because any access to an 8B granule triggers a watchpoint in that granule. If I watch 4 bytes at 0x1004, and someone accesses 1 byte at 0x1002, a watchpoint will trigger. Jaydeep Patil in 2015 ( https://reviews.llvm.org/D11672 ) added this third argument and the "silently step over it and don't tell the user" behavior; that patch decodes the instruction that caused the trap to determine the actual address that was accessed.

However, the intended use/meaning of this third field was very confusing (to me), and in 2016 Omair ( https://reviews.llvm.org/D21516 ) enabled the same behavior on ARM targets and reported the actual accessed address. I'm not exactly sure what issue motivated this, it may have simply been a misunderstanding of how StopInfo decodes and uses these three addresses. If nothing is done on ARM targets and an access outside a watched region, lldb stops execution and doesn't know how to disable the correct watchpoint & advance execution, my guess is that he was addressing that issue.

As the reason:watchpoint's description value has grown from "wp addr + hw reg index" to "wp addr + hw reg index + actual trap address", with implicit behavior on MIPS that an actual trap address silently continues if it's outside the range of a watchpoint, this is not the best. We should stop using description and add 2-4 keys in the stop info packet, e.g. wp-address:, wp-reg-index:, wp-hit-address:, silently-continue: that can be provided as needed. We can remove the "on MIPS do this" behavior from generic lldb -- the stub is probably in the best position to know if this is a false watchpoint tigger that should be ignored, and it can tell lldb. Looking at the latest ARM ARM A-Profile docs, this seems like a situation that can exist in some modes as well, we may need to flag this from AArch64 debugserver/lldb-server in the future.

We don't currently have MIPS supported in lldb - it was removed c. 2021 in anticipation of someone starting to actively maintain it again. But this "silently continue" behavior may be needed for AArch64, and it may be needed again for MIPS if that target is revived.

[omjavaid]

I agree that silent continue was wrong and should be fixed. I tried for to remember what I was trying to do when I wrote that patch ... still dont remember much but digged out some information that may be useful for this patch review.

We had a bunch of funny behaving hardware mostly Nexus phones with different types of watchpoint behavior being implement by every vendor.

From our local record i found that stp issue was never fixed. Some vendor machines reported correct hit_address while some didnt. In LLVM we do have bug report for another of these issues in one of the cases where STP instruction can trigger multiple watchpoints located side by side. https://bugs.llvm.org/show_bug.cgi?id=30758

On Linux ptrace is responsible for reporting a watchpoint hit address and also responsible for setting/unsetting watchpoints. In case of Arm64 ptrace while reporting watchpoints performs some heuristic based calculations to exactly cater for the case you have mentioned where access reports a address out of range. See watchpoint_handler code here:https://elixir.bootlin.com/linux/latest/source/arch/arm64/kernel/hw_breakpoint.c#L754

And this comment copied from same file :
/*

Arm64 hardware does not always report a watchpoint hit address that matches
one of the watchpoints set. It can also report an address "near" the
watchpoint if a single instruction access both watched and unwatched
addresses. There is no straight-forward way, short of disassembling the
offending instruction, to map that address back to the watchpoint. This
function computes the distance of the memory access from the watchpoint as a
heuristic for the likelihood that a given access triggered the watchpoint. *
See Section D2.10.5 "Determining the memory location that caused a Watchpoint
exception" of ARMv8 Architecture Reference Manual for details. *
The function returns the distance of the address from the bytes watched by
the watchpoint. In case of an exact match, it returns 0. */

[jasonmolenda]

Thanks Omair! Yeah we have the same problem on Darwin - AArch64 doesn't require that the address we get on a watchpoint trap in the FAR register point within the address range we are watching. I never fixed this on Darwin so I know the failure mode - lldb says "there was a watchpoint hit, but I couldn't possibly figure out which watchpoint it is, I will stop execution now" and the user has to figure out that they need to manually disable the watchpoint, instruction step, and re-insert the watchpoint. I'm sure you were looking at the same issue and fixed it by using the MIPS silently-skip codepath in 2016 by accident. I misunderstood what those three fields in the description were doing for a few days before I finally figured it out myself, I'm not at all surprised other people misunderstood it too.

It looks like some future AArch64 cpus may give us a watchpoint number in the ESR, instead of an address that was accessed, that will be a nice improvement over having heuristics to figure out which watchpoint was likely tripped.

[DavidSpickett]

Thanks for looking into all this, it makes my head hurt thinking about it :)

It will likely take us (Linaro) a while to do the lldb-server changes, but I do want to see this fixed eventually. We have some work upcoming for memcopy operations that will need special care for watchpoints, so that is a good excuse to do it.

lldb/docs/lldb-gdb-remote.txt

1609

What does this "v." mean? Is it short for "see here " or some kind of citation?

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

62

Worth noting why here? (minimum watch size and alignment I guess)

64

Also does this need to use /// for the doxygen \a thing to work?

(I've no idea, Jonas is forever telling me to use /// though :) )

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

2232–2235

Is this a note to self or is this code now actually doing that?

lldb/source/Target/StopInfo.cpp

1019

Please document this here as well.

I am not 100% whether silently skip true means always silently skip this watchpoint, or silently skip out of range hits attributed to this watchpoint.

[jasonmolenda]

lldb/docs/lldb-gdb-remote.txt

1609

Yeah, maybe not widely known. "v." short for "vide" latin, see. cf https://latinlexicon.org/LNS_abbreviations.php ;)

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

62

I was taking notes as I read the source to understand how arm linux behaved, and left those notes in the header, maybe I should just remove them. The DREG struct has three addresses in it, and it wasn't really clear to me what was stored in them.

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

2232–2235

Sorry that wasn't clear. We can receive watchpoint notifications one of three ways: watch/awatch/rwatch (standard gdb RSP), reason:watchpoint with a description asciihex string of 1-to-3 numbers, and on Darwin systems, a mach exception. ProcessGDBRemote rewrites watch/awatch/rwatch to reason:watchpoint + description with only the one address, and that's the only thing decoded into a StopInfo object.

lldb/source/Target/StopInfo.cpp

1019

Good point.

[jasonmolenda]

Update the patch to incorporate David's first feedback. I know this is a bit of a tricky patch to review because it is 95% comments/documentation/renaming of variables, followed by the one key change of removing core >= ArchSpec::eCore_arm_generic && core <= ArchSpec::eCore_arm_aarch64) which means ARM systems won't follow this MIPS false-watchpoint-hit behavior any more, for a watchpoint trap address outside the address range being watched (but DOES access the watched range on ARM).

[DavidSpickett]

Looks good to me, @omjavaid ?

[omjavaid]

Looks good to me as well

[jasonmolenda]

FWIW to show the motivation for this patch (which, again comes down to removing core >= ArchSpec::eCore_arm_generic && core <= ArchSpec::eCore_arm_aarch64 when creating the StopInfo from the watchpoint description), the test case I wrote in https://reviews.llvm.org/D147820 shows the problem where I adopt the lldb-server mechanism of watchpoint reporting on Darwin systems and I hit this problem:

#include <stdint.h>
#include <stdio.h>
int main() {
  union {
	  uint8_t buf[8];
	  uint64_t val;
  } a;
  a.val = 0;  // break here
  for (int i = 0; i < 5; i++) {
    a.val = i;
    printf ("a.val is %lu\n", a.val);
  }
}

(lldb) br s -p break
(lldb) r
Process 2182 stopped
-> 8   	  a.val = 0;  // break here

(lldb) w s v a.buf[2]
Watchpoint created: Watchpoint 1: addr = 0xfffffffff3a2 size = 1 state = enabled type = w
    declare @ '/home/jason/a.c:7'
    watchpoint spec = 'a.buf[2]'
    new value: '\xff'
(lldb) c
Process 2182 resuming
a.val is 0
a.val is 1
a.val is 2
a.val is 3
a.val is 4
Process 2182 exited with status = 0 (0x00000000) 
(lldb)

These watchpoint hits are silently skipped over on AArch64. If you turn on the packet log, you'll see the watchpoint hits reported by debugserver, but lldb silently skips them. This is correct behavior on MIPS; ARM targets using lldb-server should not have opted into this behavior. When we set aside the comments and variable renaming, that's what we're fixing with this patch.

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 (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
 //                                  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:
   uint32_t GetWatchpointSize(uint32_t wp_index);
 
   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.
+  /// 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
                            // 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

if (exc_type != 0) {
             } 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)
           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(
while (stop_packet.GetNameColonValue(key, value)) {
 
         WatchpointSP wp_sp =
             GetTarget().GetWatchpointList().FindByAddress(wp_addr);
         uint32_t wp_index = LLDB_INVALID_INDEX32;
 
         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);
         description = std::string(ostr.GetString());
       } else if (key.compare("library") == 0) {
         auto error = LoadModules();
         if (error) {
           Log *log(GetLog(GDBRLog::Process));
           LLDB_LOG_ERROR(log, std::move(error), "Failed to load modules: {0}");

lldb/source/Target/StopInfo.cpp

static bool SentryPreResumeAction(void *sentry_void) {
         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_wp)
+      : StopInfo(thread, watch_id), m_silently_skip_wp(silently_skip_wp) {}
 
   ~StopInfoWatchpoint() override = default;
 
   StopReason GetStopReason() const override { return eStopReasonWatchpoint; }
 
   const char *GetDescription() override {
     if (m_description.empty()) {
       StreamString strm;
if (thread_sp) {
         // 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_wp) {
 
         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;
+  // 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;
 };
 
 // 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));
 }