[lldb] Add QSupported key to report watchpoint types supported (#80376)

debugserver on arm64 devices can manage both Byte Address Select
watchpoints (1-8 bytes) and MASK watchpoints (8 bytes-2 gigabytes). This
adds a SupportedWatchpointTypes key to the QSupported response from
debugserver with a list of these, so lldb can take full advantage of
them when creating larger regions with a single hardware watchpoint.

Also add documentation for this, and two other lldb extensions, to the
lldb-gdb-remote.txt documentation.

Re-enable TestLargeWatchpoint.py on Darwin systems when testing with the
in-tree built debugserver. I can remove the "in-tree built debugserver"
in the future when this new key is handled by an Xcode debugserver.

Author: jasonmolenda

lldb/docs/lldb-gdb-remote.txt

@@ -38,7 +38,45 @@ read packet: +
 read packet: $OK#9a
 send packet: +
 
+//----------------------------------------------------------------------
+// "QSupported"
+//
+// BRIEF
+//  Query the GDB remote server for features it supports
+//
+// PRIORITY TO IMPLEMENT
+//  Optional.
+//----------------------------------------------------------------------
+
+QSupported is a standard GDB Remote Serial Protocol packet, but
+there are several additions to the response that lldb can parse.
+They are not all listed here.
+
+An example exchange:
+
+send packet: qSupported:xmlRegisters=i386,arm,mips,arc;multiprocess+;fork-events+;vfork-events+
+
+read packet: qXfer:features:read+;PacketSize=20000;qEcho+;native-signals+;SupportedCompressions=lzfse,zlib-deflate,lz4,lzma;SupportedWatchpointTypes=aarch64-mask,aarch64-bas;
+
+In the example above, three lldb extensions are shown:
 
+  PacketSize=20000
+    The base 16 maximum packet size that the stub can handle.
+  SupportedCompressions=<item,item,...>
+    A list of compression types that the stub can use to compress packets 
+    when the QEnableCompression packet is used to request one of them.
+  SupportedWatchpointTypes=<item,item,...>
+    A list of watchpoint types that this stub can manage.
+    Currently defined names are:
+        x86_64       64-bit x86-64 watchpoints
+                     (1, 2, 4, 8 byte watchpoints aligned to those amounts)
+        aarch64-bas  AArch64 Byte Address Select watchpoints
+                     (any number of contiguous bytes within a doubleword)
+        aarch64-mask AArch64 MASK watchpoints
+                     (any power-of-2 region of memory from 8 to 2GB, aligned)
+    If nothing is specified, lldb will default to sending power-of-2 
+    watchpoints, up to a pointer size, `sizeof(void*)`, a reasonable 
+    baseline assumption.
 
 //----------------------------------------------------------------------
 // "A" - launch args packet
@@ -594,7 +632,7 @@ read packet: <binary data>/E<error code>;AAAAAAAAA
 
 With LLDB, for register information, remote GDB servers can add
 support for the "qRegisterInfoN" packet where "N" is a zero based
-base16 register number that must start at zero and increase by one
+base 16 register number that must start at zero and increase by one
 for each register that is supported.  The response is done in typical
 GDB remote fashion where a series of "KEY:VALUE;" pairs are returned.
 An example for the x86_64 registers is included below:
@@ -1053,7 +1091,7 @@ Suggested key names:
 //  64-bit slices so it may be impossible to know until you're attached to a real
 //  process to know what you're working with.
 //
-//  All numeric fields return base-16 numbers without any "0x" prefix.
+//  All numeric fields return base 16 numbers without any "0x" prefix.
 //----------------------------------------------------------------------
 
 An i386 process:
@@ -1085,7 +1123,7 @@ main-binary-uuid: is the UUID of a firmware type binary that the gdb stub knows
 main-binary-address: is the load address of the firmware type binary
 main-binary-slide: is the slide of the firmware type binary, if address isn't known
 
-binary-addresses: A comma-separated list of binary load addresses base16.  
+binary-addresses: A comma-separated list of binary load addresses base 16.  
                   lldb will parse the binaries in memory to get UUIDs, then
                   try to find the binaries & debug info by UUID.  Intended for
                   use with a small number of firmware type binaries where the 
@@ -1302,7 +1340,7 @@ tuples to return are:
 
     dirty-pages:[<hexaddr>][,<hexaddr]; // A list of memory pages within this
                  // region that are "dirty" -- they have been modified.
-                 // Page addresses are in base16.  The size of a page can
+                 // Page addresses are in base 16.  The size of a page can
                  // be found from the qHostInfo's page-size key-value.
                  //
                  // If the stub supports identifying dirty pages within a
@@ -1585,7 +1623,7 @@ for this region.
 //           describe why something stopped.
 //
 //           For "reason:watchpoint", "description" is an ascii-hex
-//           encoded string with between one and three base10 numbers,
+//           encoded string with between one and three base 10 numbers,
 //           space separated.  The three numbers are
 //             1. watchpoint address.  This address should always be within
 //                a memory region lldb has a watchpoint on.  
@@ -1911,7 +1949,7 @@ for this region.
 //
 //  jThreadExtendedInfo:{"thread":612910}
 //
-// Because this is a JSON string, the thread number is provided in base10.
+// Because this is a JSON string, the thread number is provided in base 10.
 // Additional key-value pairs may be provided by lldb to the gdb remote
 // stub.  For instance, on some versions of macOS, lldb can read offset
 // information out of the system libraries.  Using those offsets, debugserver
@@ -1980,13 +2018,13 @@ for this region.
 //
 //   $N<uncompressed payload>#00
 //
-//   $C<size of uncompressed payload in base10>:<compressed payload>#00
+//   $C<size of uncompressed payload in base 10>:<compressed payload>#00
 //
 //  Where "#00" is the actual checksum value if noack mode is not enabled.  The checksum
 //  value is for the "N<uncompressed payload>" or
-//  "C<size of uncompressed payload in base10>:<compressed payload>" bytes in the packet.
+//  "C<size of uncompressed payload in base 10>:<compressed payload>" bytes in the packet.
 //
-//  The size of the uncompressed payload in base10 is provided because it will simplify
+//  The size of the uncompressed payload in base 10 is provided because it will simplify
 //  decompression if the final buffer size needed is known ahead of time.
 //
 //  Compression on low-latency connections is unlikely to be an improvement.  Particularly

lldb/include/lldb/Breakpoint/WatchpointAlgorithms.h

@@ -11,7 +11,7 @@
 
 #include "lldb/Breakpoint/WatchpointResource.h"
 #include "lldb/Utility/ArchSpec.h"
-#include "lldb/lldb-public.h"
+#include "lldb/lldb-private.h"
 
 #include <vector>
 
@@ -58,7 +58,7 @@ class WatchpointAlgorithms {
   ///     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);
+      WatchpointHardwareFeature supported_features, ArchSpec &arch);
 
 protected:
   struct Region {

lldb/include/lldb/lldb-enumerations.h

@@ -448,32 +448,6 @@ 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/include/lldb/lldb-private-enumerations.h

@@ -9,6 +9,7 @@
 #ifndef LLDB_LLDB_PRIVATE_ENUMERATIONS_H
 #define LLDB_LLDB_PRIVATE_ENUMERATIONS_H
 
+#include "lldb/lldb-enumerations.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/FormatProviders.h"
 #include "llvm/Support/raw_ostream.h"
@@ -282,4 +283,30 @@ enum InterruptionControl : bool {
   DoNotAllowInterruption = false,
 };
 
+/// 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)
+
 #endif // LLDB_LLDB_PRIVATE_ENUMERATIONS_H

lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py

@@ -921,6 +921,8 @@ def add_qSupported_packets(self, client_features=[]):
         "qSaveCore",
         "native-signals",
         "QNonStop",
+        "SupportedWatchpointTypes",
+        "SupportedCompressions",
     ]
 
     def parse_qSupported_response(self, context):

lldb/source/Breakpoint/WatchpointAlgorithms.cpp

@@ -27,8 +27,7 @@ WatchpointAlgorithms::AtomizeWatchpointRequest(
 
   std::vector<Region> entries;
 
-  if (supported_features &
-      WatchpointHardwareFeature::eWatchpointHardwareArmMASK) {
+  if (supported_features & eWatchpointHardwareArmMASK) {
     entries =
         PowerOf2Watchpoints(addr, size,
                             /*min_byte_size*/ 1,

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

@@ -403,6 +403,18 @@ void GDBRemoteCommunicationClient::GetRemoteQSupported() {
         x.split(compressions, ',');
         if (!compressions.empty())
           MaybeEnableCompression(compressions);
+      } else if (x.consume_front("SupportedWatchpointTypes=")) {
+        llvm::SmallVector<llvm::StringRef, 4> watchpoint_types;
+        x.split(watchpoint_types, ',');
+        m_watchpoint_types = eWatchpointHardwareFeatureUnknown;
+        for (auto wp_type : watchpoint_types) {
+          if (wp_type == "x86_64")
+            m_watchpoint_types |= eWatchpointHardwareX86;
+          if (wp_type == "aarch64-mask")
+            m_watchpoint_types |= eWatchpointHardwareArmMASK;
+          if (wp_type == "aarch64-bas")
+            m_watchpoint_types |= eWatchpointHardwareArmBAS;
+        }
       } else if (x.consume_front("PacketSize=")) {
         StringExtractorGDBRemote packet_response(x);
         m_max_packet_size =
@@ -1828,6 +1840,11 @@ std::optional<uint32_t> GDBRemoteCommunicationClient::GetWatchpointSlotCount() {
   return num;
 }
 
+WatchpointHardwareFeature
+GDBRemoteCommunicationClient::GetSupportedWatchpointTypes() {
+  return m_watchpoint_types;
+}
+
 std::optional<bool> GDBRemoteCommunicationClient::GetWatchpointReportedAfter() {
   if (m_qHostInfo_is_valid == eLazyBoolCalculate)
     GetHostInfo();

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h

@@ -199,6 +199,8 @@ class GDBRemoteCommunicationClient : public GDBRemoteClientBase {
 
   std::optional<bool> GetWatchpointReportedAfter();
 
+  WatchpointHardwareFeature GetSupportedWatchpointTypes();
+
   const ArchSpec &GetHostArchitecture();
 
   std::chrono::seconds GetHostDefaultPacketTimeout();
@@ -581,6 +583,8 @@ class GDBRemoteCommunicationClient : public GDBRemoteClientBase {
   lldb::tid_t m_curr_tid_run = LLDB_INVALID_THREAD_ID;
 
   uint32_t m_num_supported_hardware_watchpoints = 0;
+  WatchpointHardwareFeature m_watchpoint_types =
+      eWatchpointHardwareFeatureUnknown;
   uint32_t m_low_mem_addressing_bits = 0;
   uint32_t m_high_mem_addressing_bits = 0;
 

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

@@ -3156,16 +3156,7 @@ Status ProcessGDBRemote::EnableWatchpoint(WatchpointSP wp_sp, bool notify) {
 
   ArchSpec target_arch = GetTarget().GetArchitecture();
   WatchpointHardwareFeature supported_features =
-      eWatchpointHardwareFeatureUnknown;
-
-  // LWP_TODO: enable MASK watchpoint for arm64 debugserver
-  // when it reports that it supports them.
-  if (target_arch.GetTriple().getOS() == llvm::Triple::MacOSX &&
-      target_arch.GetTriple().getArch() == llvm::Triple::aarch64) {
-#if 0
-       supported_features |= eWatchpointHardwareArmMASK;
-#endif
-  }
+      m_gdb_comm.GetSupportedWatchpointTypes();
 
   std::vector<WatchpointResourceSP> resources =
       WatchpointAlgorithms::AtomizeWatchpointRequest(

lldb/test/API/functionalities/watchpoint/large-watchpoint/TestLargeWatchpoint.py

@@ -25,11 +25,6 @@ def continue_and_report_stop_reason(self, process, iter_str):
     @skipIf(archs=no_match(["arm64", "arm64e", "aarch64"]))
     @skipUnlessDarwin
 
-    # LWP_TODO: until debugserver advertises that it supports
-    # MASK watchpoints, this test can't be enabled, lldb won't
-    # try to send watchpoints larger than 8 bytes.
-    @skipIfDarwin
-
     # debugserver only gained the ability to watch larger regions
     # with this patch.
     @skipIfOutOfTreeDebugserver

lldb/tools/debugserver/source/RNBRemote.cpp

@@ -3557,10 +3557,10 @@ static bool GetProcessNameFrom_vAttach(const char *&p,
 rnb_err_t RNBRemote::HandlePacket_qSupported(const char *p) {
   uint32_t max_packet_size = 128 * 1024; // 128KBytes is a reasonable max packet
                                          // size--debugger can always use less
-  char buf[256];
-  snprintf(buf, sizeof(buf),
-           "qXfer:features:read+;PacketSize=%x;qEcho+;native-signals+",
-           max_packet_size);
+  std::stringstream reply;
+  reply << "qXfer:features:read+;PacketSize=" << std::hex << max_packet_size
+        << ";";
+  reply << "qEcho+;native-signals+;";
 
   bool enable_compression = false;
   (void)enable_compression;
@@ -3574,15 +3574,19 @@ rnb_err_t RNBRemote::HandlePacket_qSupported(const char *p) {
 #endif
 
   if (enable_compression) {
-    strcat(buf, ";SupportedCompressions=lzfse,zlib-deflate,lz4,lzma;"
-                "DefaultCompressionMinSize=");
-    char numbuf[16];
-    snprintf(numbuf, sizeof(numbuf), "%zu", m_compression_minsize);
-    numbuf[sizeof(numbuf) - 1] = '\0';
-    strcat(buf, numbuf);
-  } 
-
-  return SendPacket(buf);
+    reply << "SupportedCompressions=lzfse,zlib-deflate,lz4,lzma;";
+    reply << "DefaultCompressionMinSize=" << std::dec << m_compression_minsize
+          << ";";
+  }
+
+#if (defined(__arm64__) || defined(__aarch64__))
+  reply << "SupportedWatchpointTypes=aarch64-mask,aarch64-bas;";
+#endif
+#if defined(__x86_64__)
+  reply << "SupportedWatchpointTypes=x86_64;";
+#endif
+
+  return SendPacket(reply.str().c_str());
 }
 
 static bool process_does_not_exist (nub_process_t pid) {