[lldb] Add support for large watchpoints in lldb (llvm#79962)

This patch is the next piece of work in my Large Watchpoint proposal,
https://discourse.llvm.org/t/rfc-large-watchpoint-support-in-lldb/72116

This patch breaks a user's watchpoint into one or more
WatchpointResources which reflect what the hardware registers can cover.
This means we can watch objects larger than 8 bytes, and we can watched
unaligned address ranges. On a typical 64-bit target with 4 watchpoint
registers you can watch 32 bytes of memory if the start address is
doubleword aligned.

Additionally, if the remote stub implements AArch64 MASK style
watchpoints (e.g. debugserver on Darwin), we can watch any power-of-2
size region of memory up to 2GB, aligned to that same size.

I updated the Watchpoint constructor and CommandObjectWatchpoint to
create a CompilerType of Array<UInt8> when the size of the watched
region is greater than pointer-size and we don't have a variable type to
use. For pointer-size and smaller, we can display the watched granule as
an integer value; for larger-than-pointer-size we will display as an
array of bytes.

I have `watchpoint list` now print the WatchpointResources used to
implement the watchpoint.

I added a WatchpointAlgorithm class which has a top-level static method
that takes an enum flag mask WatchpointHardwareFeature and a user
address and size, and returns a vector of WatchpointResources covering
the request. It does not take into account the number of watchpoint
registers the target has, or the number still available for use. Right
now there is only one algorithm, which monitors power-of-2 regions of
memory. For up to pointer-size, this is what Intel hardware supports.
AArch64 Byte Address Select watchpoints can watch any number of
contiguous bytes in a pointer-size memory granule, that is not currently
supported so if you ask to watch bytes 3-5, the algorithm will watch the
entire doubleword (8 bytes). The newly default "modify" style means we
will silently ignore modifications to bytes outside the watched range.

I've temporarily skipped TestLargeWatchpoint.py for all targets. It was
only run on Darwin when using the in-tree debugserver, which was a proxy
for "debugserver supports MASK watchpoints". I'll be adding the
aforementioned feature flag from the stub and enabling full mask
watchpoints when a debugserver with that feature is enabled, and
re-enable this test.

I added a new TestUnalignedLargeWatchpoint.py which only has one test
but it's a great one, watching a 22-byte range that is unaligned and
requires four 8-byte watchpoints to cover.

I also added a unit test, WatchpointAlgorithmsTests, which has a number
of simple tests against WatchpointAlgorithms::PowerOf2Watchpoints. I
think there's interesting possible different approaches to how we cover
these; I note in the unit test that a user requesting a watch on address
0x12e0 of 120 bytes will be covered by two watchpoints today, a
128-bytes at 0x1280 and at 0x1300. But it could be done with a 16-byte
watchpoint at 0x12e0 and a 128-byte at 0x1300, which would have fewer
false positives/private stops. As we try refining this one, it's helpful
to have a collection of tests to make sure things don't regress.

I tested this on arm64 macOS, (genuine) x86_64 macOS, and AArch64
Ubuntu. I have not modifed the Windows process plugins yet, I might try
that as a standalone patch, I'd be making the change blind, but the
necessary changes (see ProcessGDBRemote::EnableWatchpoint) are pretty
small so it might be obvious enough that I can change it and see what
the Windows CI thinks.

There isn't yet a packet (or a qSupported feature query) for the gdb
remote serial protocol stub to communicate its watchpoint capabilities
to lldb. I'll be doing that in a patch right after this is landed,
having debugserver advertise its capability of AArch64 MASK watchpoints,
and have ProcessGDBRemote add eWatchpointHardwareArmMASK to
WatchpointAlgorithms so we can watch larger than 32-byte requests on
Darwin.

I haven't yet tackled WatchpointResource *sharing* by multiple
Watchpoints. This is all part of the goal, especially when we may be
watching a larger memory range than the user requested, if they then add
another watchpoint next to their first request, it may be covered by the
same WatchpointResource (hardware watchpoint register). Also one "read"
watchpoint and one "write" watchpoint on the same memory granule need to
be handled, making the WatchpointResource cover all requests.

As WatchpointResources aren't shared among multiple Watchpoints yet,
there's no handling of running the conditions/commands/etc on multiple
Watchpoints when their shared WatchpointResource is hit. The goal beyond
"large watchpoint" is to unify (much more) the Watchpoint and Breakpoint
behavior and commands. I have a feeling I may be slowly chipping away at
this for a while.

rdar://108234227

Author: jasonmolenda

lldb/include/lldb/Breakpoint/WatchpointAlgorithms.h

@@ -0,0 +1,109 @@
+//===-- WatchpointAlgorithms.h ----------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_BREAKPOINT_WATCHPOINTALGORITHMS_H
+#define LLDB_BREAKPOINT_WATCHPOINTALGORITHMS_H
+
+#include "lldb/Breakpoint/WatchpointResource.h"
+#include "lldb/Utility/ArchSpec.h"
+#include "lldb/lldb-public.h"
+
+#include <vector>
+
+namespace lldb_private {
+
+class WatchpointAlgorithms {
+
+public:
+  /// Convert a user's watchpoint request into an array of memory
+  /// regions that can be watched by one hardware watchpoint register
+  /// on the current target.
+  ///
+  /// \param[in] addr
+  ///     The start address specified by the user.
+  ///
+  /// \param[in] size
+  ///     The number of bytes the user wants to watch.
+  ///
+  /// \param[in] read
+  ///     True if we are watching for read accesses.
+  ///
+  /// \param[in] write
+  ///     True if we are watching for write accesses.
+  ///     \a read and \a write may both be true.
+  ///     There is no "modify" style for WatchpointResources -
+  ///     WatchpointResources are akin to the hardware watchpoint
+  ///     registers which are either in terms of read or write.
+  ///     "modify" distinction is done at the Watchpoint layer, where
+  ///     we check the actual range of bytes the user requested.
+  ///
+  /// \param[in] supported_features
+  ///     The bit flags in this parameter are set depending on which
+  ///     WatchpointHardwareFeature enum values the current target supports.
+  ///     The eWatchpointHardwareFeatureUnknown bit may be set if we
+  ///     don't have specific information about what the remote stub
+  ///     can support, and a reasonablec default will be used.
+  ///
+  /// \param[in] arch
+  ///     The ArchSpec of the current Target.
+  ///
+  /// \return
+  ///     A vector of WatchpointResourceSP's, one per hardware watchpoint
+  ///     register needed.  We may return more WatchpointResources than the
+  ///     target can watch at once; if all resources cannot be set, the
+  ///     watchpoint cannot be set.
+  static std::vector<lldb::WatchpointResourceSP> AtomizeWatchpointRequest(
+      lldb::addr_t addr, size_t size, bool read, bool write,
+      lldb::WatchpointHardwareFeature supported_features, ArchSpec &arch);
+
+  struct Region {
+    lldb::addr_t addr;
+    size_t size;
+  };
+
+protected:
+  /// Convert a user's watchpoint request into an array of addr+size that
+  /// can be watched with power-of-2 style hardware watchpoints.
+  ///
+  /// This is the default algorithm if we have no further information;
+  /// most watchpoint implementations can be assumed to be able to watch up
+  /// to pointer-size regions of memory in power-of-2 sizes and alingments.
+  ///
+  /// \param[in] user_addr
+  ///     The user's start address.
+  ///
+  /// \param[in] user_size
+  ///     The user's specified byte length.
+  ///
+  /// \param[in] min_byte_size
+  ///     The minimum byte size supported on this target.
+  ///     In most cases, this will be 1.  AArch64 MASK watchpoints can
+  ///     watch a minimum of 8 bytes (although Byte Address Select watchpoints
+  ///     can watch 1 to pointer-size bytes in a pointer-size aligned granule).
+  ///
+  /// \param[in] max_byte_size
+  ///     The maximum byte size supported for one watchpoint on this target.
+  ///
+  /// \param[in] address_byte_size
+  ///     The address byte size on this target.
+  static std::vector<Region> PowerOf2Watchpoints(lldb::addr_t user_addr,
+                                                 size_t user_size,
+                                                 size_t min_byte_size,
+                                                 size_t max_byte_size,
+                                                 uint32_t address_byte_size);
+};
+
+// For the unittests to have access to the individual algorithms
+class WatchpointAlgorithmsTest : public WatchpointAlgorithms {
+public:
+  using WatchpointAlgorithms::PowerOf2Watchpoints;
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_BREAKPOINT_WATCHPOINTALGORITHMS_H

lldb/include/lldb/lldb-enumerations.h

@@ -448,6 +448,32 @@ enum WatchpointWriteType {
   eWatchpointWriteTypeOnModify
 };
 
+/// The hardware and native stub capabilities for a given target,
+/// for translating a user's watchpoint request into hardware
+/// capable watchpoint resources.
+FLAGS_ENUM(WatchpointHardwareFeature){
+    /// lldb will fall back to a default that assumes the target
+    /// can watch up to pointer-size power-of-2 regions, aligned to
+    /// power-of-2.
+    eWatchpointHardwareFeatureUnknown = (1u << 0),
+
+    /// Intel systems can watch 1, 2, 4, or 8 bytes (in 64-bit targets),
+    /// aligned naturally.
+    eWatchpointHardwareX86 = (1u << 1),
+
+    /// ARM systems with Byte Address Select watchpoints
+    /// can watch any consecutive series of bytes up to the
+    /// size of a pointer (4 or 8 bytes), at a pointer-size
+    /// alignment.
+    eWatchpointHardwareArmBAS = (1u << 2),
+
+    /// ARM systems with MASK watchpoints can watch any power-of-2
+    /// sized region from 8 bytes to 2 gigabytes, aligned to that
+    /// same power-of-2 alignment.
+    eWatchpointHardwareArmMASK = (1u << 3),
+};
+LLDB_MARK_AS_BITMASK_ENUM(WatchpointHardwareFeature)
+
 /// Programming language type.
 ///
 /// These enumerations use the same language enumerations as the DWARF

lldb/packages/Python/lldbsuite/test/concurrent_base.py

@@ -166,7 +166,12 @@ def do_thread_actions(
 
         # Initialize the (single) watchpoint on the global variable (g_watchme)
         if num_watchpoint_threads + num_delay_watchpoint_threads > 0:
-            self.runCmd("watchpoint set variable g_watchme")
+            # The concurrent tests have multiple threads modifying a variable
+            # with the same value.  The default "modify" style watchpoint will
+            # only report this as 1 hit for all threads, because they all wrote
+            # the same value.  The testsuite needs "write" style watchpoints to
+            # get the correct number of hits reported.
+            self.runCmd("watchpoint set variable -w write g_watchme")
             for w in self.inferior_target.watchpoint_iter():
                 self.thread_watchpoint = w
                 self.assertTrue(

lldb/source/Breakpoint/CMakeLists.txt

@@ -21,6 +21,7 @@ add_lldb_library(lldbBreakpoint NO_PLUGIN_DEPENDENCIES
   StoppointSite.cpp
   StopPointSiteList.cpp
   Watchpoint.cpp
+  WatchpointAlgorithms.cpp
   WatchpointList.cpp
   WatchpointOptions.cpp
   WatchpointResource.cpp

lldb/source/Breakpoint/Watchpoint.cpp

@@ -45,10 +45,16 @@ Watchpoint::Watchpoint(Target &target, lldb::addr_t addr, uint32_t size,
       LLDB_LOG_ERROR(GetLog(LLDBLog::Watchpoints), std::move(err),
                      "Failed to set type: {0}");
     } else {
-      if (auto ts = *type_system_or_err)
-        m_type =
-            ts->GetBuiltinTypeForEncodingAndBitSize(eEncodingUint, 8 * size);
-      else
+      if (auto ts = *type_system_or_err) {
+        if (size <= target.GetArchitecture().GetAddressByteSize()) {
+          m_type =
+              ts->GetBuiltinTypeForEncodingAndBitSize(eEncodingUint, 8 * size);
+        } else {
+          CompilerType clang_uint8_type =
+              ts->GetBuiltinTypeForEncodingAndBitSize(eEncodingUint, 8);
+          m_type = clang_uint8_type.GetArrayType(size);
+        }
+      } else
         LLDB_LOG_ERROR(GetLog(LLDBLog::Watchpoints), std::move(err),
                        "Failed to set type: Typesystem is no longer live: {0}");
     }
@@ -352,6 +358,20 @@ void Watchpoint::DumpWithLevel(Stream *s,
       s->Printf("\n    declare @ '%s'", m_decl_str.c_str());
     if (!m_watch_spec_str.empty())
       s->Printf("\n    watchpoint spec = '%s'", m_watch_spec_str.c_str());
+    if (IsEnabled()) {
+      if (ProcessSP process_sp = m_target.GetProcessSP()) {
+        auto &resourcelist = process_sp->GetWatchpointResourceList();
+        size_t idx = 0;
+        s->Printf("\n    watchpoint resources:");
+        for (WatchpointResourceSP &wpres : resourcelist.Sites()) {
+          if (wpres->ConstituentsContains(this)) {
+            s->Printf("\n       #%zu: ", idx);
+            wpres->Dump(s);
+          }
+          idx++;
+        }
+      }
+    }
 
     // Dump the snapshots we have taken.
     DumpSnapshots(s, "    ");

lldb/source/Breakpoint/WatchpointAlgorithms.cpp

@@ -0,0 +1,142 @@
+//===-- WatchpointAlgorithms.cpp ------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/Breakpoint/WatchpointAlgorithms.h"
+#include "lldb/Breakpoint/WatchpointResource.h"
+#include "lldb/Target/Process.h"
+#include "lldb/Utility/ArchSpec.h"
+
+#include <utility>
+#include <vector>
+
+using namespace lldb;
+using namespace lldb_private;
+
+std::vector<WatchpointResourceSP>
+WatchpointAlgorithms::AtomizeWatchpointRequest(
+    addr_t addr, size_t size, bool read, bool write,
+    WatchpointHardwareFeature supported_features, ArchSpec &arch) {
+
+  std::vector<Region> entries;
+
+  if (supported_features &
+      WatchpointHardwareFeature::eWatchpointHardwareArmMASK) {
+    entries =
+        PowerOf2Watchpoints(addr, size,
+                            /*min_byte_size*/ 1,
+                            /*max_byte_size*/ INT32_MAX,
+                            /*address_byte_size*/ arch.GetAddressByteSize());
+  } else {
+    // As a fallback, assume we can watch any power-of-2
+    // number of bytes up through the size of an address in the target.
+    entries =
+        PowerOf2Watchpoints(addr, size,
+                            /*min_byte_size*/ 1,
+                            /*max_byte_size*/ arch.GetAddressByteSize(),
+                            /*address_byte_size*/ arch.GetAddressByteSize());
+  }
+
+  std::vector<WatchpointResourceSP> resources;
+  for (Region &ent : entries) {
+    WatchpointResourceSP wp_res_sp =
+        std::make_shared<WatchpointResource>(ent.addr, ent.size, read, write);
+    resources.push_back(wp_res_sp);
+  }
+
+  return resources;
+}
+
+/// Convert a user's watchpoint request (\a user_addr and \a user_size)
+/// into hardware watchpoints, for a target that can watch a power-of-2
+/// region of memory (1, 2, 4, 8, etc), aligned to that same power-of-2
+/// memory address.
+///
+/// If a user asks to watch 4 bytes at address 0x1002 (0x1002-0x1005
+/// inclusive) we can implement this with two 2-byte watchpoints
+/// (0x1002 and 0x1004) or with an 8-byte watchpoint at 0x1000.
+/// A 4-byte watchpoint at 0x1002 would not be properly 4 byte aligned.
+///
+/// If a user asks to watch 16 bytes at 0x1000, and this target supports
+/// 8-byte watchpoints, we can implement this with two 8-byte watchpoints
+/// at 0x1000 and 0x1008.
+std::vector<WatchpointAlgorithms::Region>
+WatchpointAlgorithms::PowerOf2Watchpoints(addr_t user_addr, size_t user_size,
+                                          size_t min_byte_size,
+                                          size_t max_byte_size,
+                                          uint32_t address_byte_size) {
+
+  // Can't watch zero bytes.
+  if (user_size == 0)
+    return {};
+
+  // The aligned watch region will be less than/equal to the size of
+  // an address in this target.
+  const int address_bit_size = address_byte_size * 8;
+
+  size_t aligned_size = std::max(user_size, min_byte_size);
+  /// Round up \a user_size to the next power-of-2 size
+  /// user_size == 8   -> aligned_size == 8
+  /// user_size == 9   -> aligned_size == 16
+  /// user_size == 15  -> aligned_size == 16
+  /// user_size == 192 -> aligned_size == 256
+  /// Could be `std::bit_ceil(aligned_size)` when we build with C++20?
+
+  aligned_size = 1ULL << (address_bit_size - __builtin_clzll(aligned_size - 1));
+
+  addr_t aligned_start = user_addr & ~(aligned_size - 1);
+
+  // Does this power-of-2 memory range, aligned to power-of-2 that the
+  // hardware can watch, completely cover the requested region.
+  if (aligned_size <= max_byte_size &&
+      aligned_start + aligned_size >= user_addr + user_size)
+    return {{aligned_start, aligned_size}};
+
+  // If the maximum region we can watch is larger than the aligned
+  // size, try increasing the region size by one power of 2 and see
+  // if aligning to that amount can cover the requested region.
+  //
+  // Increasing the aligned_size repeatedly instead of splitting the
+  // watchpoint can result in us watching large regions of memory
+  // unintentionally when we could use small two watchpoints.  e.g.
+  //    user_addr 0x3ff8 user_size 32
+  // can be watched with four 8-byte watchpoints or if it's done with one
+  // MASK watchpoint, it would need to be a 32KB watchpoint (a 16KB
+  // watchpoint at 0x0 only covers 0x0000-0x4000).  A user request
+  // at the end of a power-of-2 region can lead to these undesirably
+  // large watchpoints and many false positive hits to ignore.
+  if (max_byte_size >= (aligned_size << 1)) {
+    aligned_size <<= 1;
+    aligned_start = user_addr & ~(aligned_size - 1);
+    if (aligned_size <= max_byte_size &&
+        aligned_start + aligned_size >= user_addr + user_size)
+      return {{aligned_start, aligned_size}};
+
+    // Go back to our original aligned size, to try the multiple
+    // watchpoint approach.
+    aligned_size >>= 1;
+  }
+
+  // We need to split the user's watchpoint into two or more watchpoints
+  // that can be monitored by hardware, because of alignment and/or size
+  // reasons.
+  aligned_size = std::min(aligned_size, max_byte_size);
+  aligned_start = user_addr & ~(aligned_size - 1);
+
+  std::vector<Region> result;
+  addr_t current_address = aligned_start;
+  const addr_t user_end_address = user_addr + user_size;
+  while (current_address + aligned_size < user_end_address) {
+    result.push_back({current_address, aligned_size});
+    current_address += aligned_size;
+  }
+
+  if (current_address < user_end_address)
+    result.push_back({current_address, aligned_size});
+
+  return result;
+}

lldb/source/Breakpoint/WatchpointResource.cpp

@@ -9,6 +9,7 @@
 #include <assert.h>
 
 #include "lldb/Breakpoint/WatchpointResource.h"
+#include "lldb/Utility/Stream.h"
 
 #include <algorithm>
 
@@ -113,7 +114,8 @@ bool WatchpointResource::ShouldStop(StoppointCallbackContext *context) {
 }
 
 void WatchpointResource::Dump(Stream *s) const {
-  return; // LWP_TODO
+  s->Printf("addr = 0x%8.8" PRIx64 " size = %zu", m_addr, m_size);
+  return;
 }
 
 wp_resource_id_t WatchpointResource::GetNextID() {

lldb/source/Commands/CommandObjectWatchpoint.cpp

@@ -1139,9 +1139,22 @@ class CommandObjectWatchpointSetExpression : public CommandObjectRaw {
 
     // Fetch the type from the value object, the type of the watched object is
     // the pointee type
-    /// of the expression, so convert to that if we  found a valid type.
+    /// of the expression, so convert to that if we found a valid type.
     CompilerType compiler_type(valobj_sp->GetCompilerType());
 
+    std::optional<uint64_t> valobj_size = valobj_sp->GetByteSize();
+    // Set the type as a uint8_t array if the size being watched is
+    // larger than the ValueObject's size (which is probably the size
+    // of a pointer).
+    if (valobj_size && size > *valobj_size) {
+      auto type_system = compiler_type.GetTypeSystem();
+      if (type_system) {
+        CompilerType clang_uint8_type =
+            type_system->GetBuiltinTypeForEncodingAndBitSize(eEncodingUint, 8);
+        compiler_type = clang_uint8_type.GetArrayType(size);
+      }
+    }
+
     Status error;
     WatchpointSP watch_sp =
         target->CreateWatchpoint(addr, size, &compiler_type, watch_type, error);