[lldb] Add support for large watchpoints in lldb

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);