llvmbot wrote:

<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: Jason Molenda (jasonmolenda)

<details>
<summary>Changes</summary>

This patch is rearranging code a bit to add WatchpointResources to Process.  A 
WatchpointResource is meant to represent a hardware watchpoint register in the 
inferior process.  It has an address, a size, a type, and a list of Watchpoints 
that are using this WatchpointResource.

This current patch doesn't add any of the features of WatchpointResources that 
make them interesting -- a user asking to watch a 24 byte object could watch 
this with three 8 byte WatchpointResources.  Or a Watchpoint on 1 byte at 
0x1002 and a second watchpoint on 1 byte at 0x1003, these must both be served 
by a single WatchpointResource on that doubleword at 0x1000 on a 64-bit target, 
if two hardware watchpoint registers were used to track these separately, one 
of them may not be hit.  Or if you have one Watchpoint on a variable with a 
condition set, and another Watchpoint on that same variable with a command 
defined or different condition, or ignorecount, both of those Watchpoints need 
to evaluate their criteria/commands when their WatchpointResource has been hit.

There's a bit of code movement to rearrange things in the direction I'll need 
for implementing this feature, so I want to start with reviewing &amp; landing 
this mostly NFC patch and we can focus on the algorithmic choices about how 
WatchpointResources are shared and handled as they're triggeed, separately.

This patch also stops printing "Watchpoint &lt;n&gt; hit: old value: &lt;x&gt;, 
new vlaue: &lt;y&gt;" for Read watchpoints.  I could make an argument for print 
"Watchpoint &lt;n&gt; hit: current value &lt;x&gt;" but the current output 
doesn't make any sense, and the user can print the value if they are 
particularly interested.  Read watchpoints are used primarily to understand 
what code is reading a variable.

This patch adds more fallbacks for how to print the objects being watched if we 
have types, instead of assuming they are all integral values, so a struct will 
print its elements.  As large watchpoints are added, we'll be doing a lot more 
of those.

To track the WatchpointSP in the WatchpointResources, I changed the internal 
API which took a WatchpointSP and devolved it to a Watchpoint*, which meant 
touching several different Process files. I removed the watchpoint code in 
ProcessKDP which only reported that watchpoints aren't supported, the base 
class does that already.

I haven't yet changed how we receive a watchpoint to identify the 
WatchpointResource responsible for the trigger, and identify all Watchpoints 
that are using this Resource to evaluate their conditions etc.  This is the 
same work that a BreakpointSite needs to do when it has been tiggered, where 
multiple Breakpoints may be at the same address.

There is not yet any printing of the Resources that a Watchpoint is implemented 
in terms of ("watchpoint list", or
SBWatchpoint::GetDescription).

"watchpoint set var" and "watchpoint set expression" take a size argument which 
was previously 1, 2, 4, or 8 (an enum).  I've changed this to an unsigned int.  
Most hardware implementations can only watch 1, 2, 4, 8 byte ranges, but with 
Resources we'll allow a user to ask for different sized watchpoints and set 
them in hardware-expressble terms soon.

I've annotated areas where I know there is work still needed with LWP_TODO that 
I'll be working on once this is landed.

I've tested this on aarch64 macOS, aarch64 Linux, and Intel macOS.

https://discourse.llvm.org/t/rfc-large-watchpoint-support-in-lldb/72116

---

Patch is 54.63 KiB, truncated to 20.00 KiB below, full version: 
https://github.com/llvm/llvm-project/pull/68845.diff


28 Files Affected:

- (modified) lldb/include/lldb/Breakpoint/Watchpoint.h (+1-1) 
- (modified) lldb/include/lldb/Interpreter/OptionGroupWatchpoint.h (+2-3) 
- (modified) lldb/include/lldb/Target/Process.h (+10-2) 
- (added) lldb/include/lldb/Target/WatchpointResource.h (+57) 
- (added) lldb/include/lldb/Target/WatchpointResourceList.h (+85) 
- (modified) lldb/include/lldb/lldb-forward.h (+2) 
- (modified) lldb/source/API/SBWatchpoint.cpp (+2-2) 
- (modified) lldb/source/Breakpoint/Watchpoint.cpp (+56-15) 
- (modified) lldb/source/Commands/CommandObjectWatchpoint.cpp (+4-4) 
- (modified) lldb/source/Interpreter/OptionGroupWatchpoint.cpp (+6-37) 
- (modified) lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp (-14) 
- (modified) lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h (-7) 
- (modified) lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp (+9) 
- (modified) lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp 
(+21-19) 
- (modified) lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h (+4-2) 
- (modified) lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp 
(+143-61) 
- (modified) lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h (+4-2) 
- (modified) lldb/source/Target/CMakeLists.txt (+2) 
- (modified) lldb/source/Target/Process.cpp (+4-3) 
- (modified) lldb/source/Target/StopInfo.cpp (+11-8) 
- (modified) lldb/source/Target/Target.cpp (+19-12) 
- (added) lldb/source/Target/WatchpointResource.cpp (+49) 
- (added) lldb/source/Target/WatchpointResourceList.cpp (+61) 
- (modified) lldb/test/API/commands/watchpoints/watchpoint_count/main.c (+2-2) 
- (modified) 
lldb/test/API/python_api/watchpoint/watchlocation/TestTargetWatchAddress.py 
(+1-1) 
- (modified) lldb/test/Shell/Watchpoint/Inputs/val.c (+3) 
- (modified) lldb/test/Shell/Watchpoint/LocalVariableWatchpointDisabler.test 
(+7) 
- (modified) lldb/test/Shell/Watchpoint/SetErrorCases.test (+1-1) 


``````````diff
diff --git a/lldb/include/lldb/Breakpoint/Watchpoint.h 
b/lldb/include/lldb/Breakpoint/Watchpoint.h
index c86d66663c7f8c0..851162af24c74e0 100644
--- a/lldb/include/lldb/Breakpoint/Watchpoint.h
+++ b/lldb/include/lldb/Breakpoint/Watchpoint.h
@@ -126,7 +126,7 @@ class Watchpoint : public 
std::enable_shared_from_this<Watchpoint>,
 
   void GetDescription(Stream *s, lldb::DescriptionLevel level);
   void Dump(Stream *s) const override;
-  void DumpSnapshots(Stream *s, const char *prefix = nullptr) const;
+  bool DumpSnapshots(Stream *s, const char *prefix = nullptr) const;
   void DumpWithLevel(Stream *s, lldb::DescriptionLevel description_level) 
const;
   Target &GetTarget() { return m_target; }
   const Status &GetError() { return m_error; }
diff --git a/lldb/include/lldb/Interpreter/OptionGroupWatchpoint.h 
b/lldb/include/lldb/Interpreter/OptionGroupWatchpoint.h
index f009fa145f30344..527a2612b189bd3 100644
--- a/lldb/include/lldb/Interpreter/OptionGroupWatchpoint.h
+++ b/lldb/include/lldb/Interpreter/OptionGroupWatchpoint.h
@@ -9,6 +9,7 @@
 #ifndef LLDB_INTERPRETER_OPTIONGROUPWATCHPOINT_H
 #define LLDB_INTERPRETER_OPTIONGROUPWATCHPOINT_H
 
+#include "lldb/Interpreter/OptionValueUInt64.h"
 #include "lldb/Interpreter/Options.h"
 
 namespace lldb_private {
@@ -21,8 +22,6 @@ class OptionGroupWatchpoint : public OptionGroup {
 
   ~OptionGroupWatchpoint() override = default;
 
-  static bool IsWatchSizeSupported(uint32_t watch_size);
-
   llvm::ArrayRef<OptionDefinition> GetDefinitions() override;
 
   Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_value,
@@ -43,7 +42,7 @@ class OptionGroupWatchpoint : public OptionGroup {
   };
 
   WatchType watch_type;
-  uint32_t watch_size;
+  OptionValueUInt64 watch_size;
   bool watch_type_specified;
   lldb::LanguageType language_type;
 
diff --git a/lldb/include/lldb/Target/Process.h 
b/lldb/include/lldb/Target/Process.h
index a6d3e6c2d16926e..7bc0b5f47a9a1d5 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -41,6 +41,8 @@
 #include "lldb/Target/ThreadList.h"
 #include "lldb/Target/ThreadPlanStack.h"
 #include "lldb/Target/Trace.h"
+#include "lldb/Target/WatchpointResource.h"
+#include "lldb/Target/WatchpointResourceList.h"
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/Broadcaster.h"
 #include "lldb/Utility/Event.h"
@@ -2133,9 +2135,10 @@ class Process : public 
std::enable_shared_from_this<Process>,
                                      lldb::BreakpointSiteSP &bp_site_sp);
 
   // Process Watchpoints (optional)
-  virtual Status EnableWatchpoint(Watchpoint *wp, bool notify = true);
+  virtual Status EnableWatchpoint(lldb::WatchpointSP wp_sp, bool notify = 
true);
 
-  virtual Status DisableWatchpoint(Watchpoint *wp, bool notify = true);
+  virtual Status DisableWatchpoint(lldb::WatchpointSP wp_sp,
+                                   bool notify = true);
 
   // Thread Queries
 
@@ -2989,6 +2992,8 @@ void PruneThreadPlans();
       m_queue_list; ///< The list of libdispatch queues at a given stop point
   uint32_t m_queue_list_stop_id; ///< The natural stop id when queue list was
                                  ///last fetched
+  WatchpointResourceList
+      m_watchpoint_resource_list; ///< Watchpoint resources currently in use.
   std::vector<Notifications> m_notifications; ///< The list of notifications
                                               ///that this process can deliver.
   std::vector<lldb::addr_t> m_image_tokens;
@@ -3069,6 +3074,9 @@ void PruneThreadPlans();
   std::unique_ptr<UtilityFunction> m_dlopen_utility_func_up;
   llvm::once_flag m_dlopen_utility_func_flag_once;
 
+  // Watchpoint hardware registers currently in use.
+  std::vector<lldb::WatchpointResourceSP> m_watchpoint_resources;
+
   /// Per process source file cache.
   SourceManager::SourceFileCache m_source_file_cache;
 
diff --git a/lldb/include/lldb/Target/WatchpointResource.h 
b/lldb/include/lldb/Target/WatchpointResource.h
new file mode 100644
index 000000000000000..73b539e79f9bfbb
--- /dev/null
+++ b/lldb/include/lldb/Target/WatchpointResource.h
@@ -0,0 +1,57 @@
+//===-- WatchpointResource.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_TARGET_WATCHPOINTRESOURCE_H
+#define LLDB_TARGET_WATCHPOINTRESOURCE_H
+
+#include "lldb/lldb-public.h"
+
+#include <set>
+
+namespace lldb_private {
+
+class WatchpointResource
+    : public std::enable_shared_from_this<WatchpointResource> {
+
+public:
+  // Constructors and Destructors
+  WatchpointResource(lldb::addr_t addr, size_t size, bool read, bool write);
+
+  ~WatchpointResource();
+
+  void GetResourceMemoryRange(lldb::addr_t &addr, size_t &size) const;
+
+  void GetResourceType(bool &read, bool &write) const;
+
+  void RegisterWatchpoint(lldb::WatchpointSP &wp_sp);
+
+  void DeregisterWatchpoint(lldb::WatchpointSP &wp_sp);
+
+  size_t GetNumDependantWatchpoints();
+
+  bool DependantWatchpointsContains(lldb::WatchpointSP wp_sp_to_match);
+
+private:
+  WatchpointResource(const WatchpointResource &) = delete;
+  const WatchpointResource &operator=(const WatchpointResource &) = delete;
+
+  // start address & size aligned & expanded to be a valid watchpoint
+  // memory granule on this target.
+  lldb::addr_t m_addr;
+  size_t m_size;
+
+  bool m_watch_read;  // true if we stop when the watched data is read from
+  bool m_watch_write; // true if we stop when the watched data is written to
+
+  // The watchpoints using this WatchpointResource.
+  std::set<lldb::WatchpointSP> m_watchpoints;
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_TARGET_WATCHPOINTRESOURCE_H
diff --git a/lldb/include/lldb/Target/WatchpointResourceList.h 
b/lldb/include/lldb/Target/WatchpointResourceList.h
new file mode 100644
index 000000000000000..9570e337f1e6ccb
--- /dev/null
+++ b/lldb/include/lldb/Target/WatchpointResourceList.h
@@ -0,0 +1,85 @@
+//===-- WatchpointResourceList.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_TARGET_WATCHPOINTRESOURCELIST_H
+#define LLDB_TARGET_WATCHPOINTRESOURCELIST_H
+
+#include "lldb/Utility/Iterable.h"
+#include "lldb/lldb-private.h"
+#include "lldb/lldb-public.h"
+
+#include <mutex>
+#include <vector>
+
+namespace lldb_private {
+
+class WatchpointResourceList {
+
+public:
+  // Constructors and Destructors
+  WatchpointResourceList();
+
+  ~WatchpointResourceList();
+
+  /// Get the number of WatchpointResources that are available.
+  ///
+  /// \return
+  ///     The number of WatchpointResources that are stored in the
+  ///     WatchpointResourceList.
+  uint32_t GetSize();
+
+  /// Get the WatchpointResource at a given index.
+  ///
+  /// \param [in] idx
+  ///     The index of the resource.
+  /// \return
+  ///     The WatchpointResource at that index number.
+  lldb::WatchpointResourceSP GetResourceAtIndex(uint32_t idx);
+
+  /// Remove a WatchpointResource from the list.
+  ///
+  /// The WatchpointResource must have already been disabled in the
+  /// Process; this method only removes it from the list.
+  ///
+  /// \param [in] wp_resource_sp
+  ///     The WatchpointResource to remove.
+  void RemoveWatchpointResource(lldb::WatchpointResourceSP wp_resource_sp);
+
+  typedef std::vector<lldb::WatchpointResourceSP> collection;
+  typedef LockingAdaptedIterable<collection, lldb::WatchpointResourceSP,
+                                 vector_adapter, std::mutex>
+      WatchpointResourceIterable;
+
+  /// Iterate over the list of WatchpointResources.
+  ///
+  /// \return
+  ///     An Iterable object which can be used to loop over the resources
+  ///     that exist.
+  WatchpointResourceIterable Resources() {
+    return WatchpointResourceIterable(m_resources, m_mutex);
+  }
+
+  /// Clear out the list of resources from the WatchpointResourceList
+  void Clear();
+
+  /// Add a WatchpointResource to the WatchpointResourceList.
+  ///
+  /// \param [in] resource
+  ///     A WatchpointResource to be added.
+  void AddResource(lldb::WatchpointResourceSP resource_sp);
+
+  std::mutex &GetMutex();
+
+private:
+  collection m_resources;
+  std::mutex m_mutex;
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_TARGET_WATCHPOINTRESOURCELIST_H
diff --git a/lldb/include/lldb/lldb-forward.h b/lldb/include/lldb/lldb-forward.h
index 3cd71c8a4ba3c0a..6b2f0dfd68c0b85 100644
--- a/lldb/include/lldb/lldb-forward.h
+++ b/lldb/include/lldb/lldb-forward.h
@@ -284,6 +284,7 @@ class VariableList;
 class Watchpoint;
 class WatchpointList;
 class WatchpointOptions;
+class WatchpointResource;
 class WatchpointSetOptions;
 struct CompilerContext;
 struct LineEntry;
@@ -461,6 +462,7 @@ typedef std::shared_ptr<lldb_private::Variable> VariableSP;
 typedef std::shared_ptr<lldb_private::VariableList> VariableListSP;
 typedef std::shared_ptr<lldb_private::ValueObjectList> ValueObjectListSP;
 typedef std::shared_ptr<lldb_private::Watchpoint> WatchpointSP;
+typedef std::shared_ptr<lldb_private::WatchpointResource> WatchpointResourceSP;
 
 } // namespace lldb
 
diff --git a/lldb/source/API/SBWatchpoint.cpp b/lldb/source/API/SBWatchpoint.cpp
index 8b4e0ad3178b182..f8bbc844532c2e1 100644
--- a/lldb/source/API/SBWatchpoint.cpp
+++ b/lldb/source/API/SBWatchpoint.cpp
@@ -147,9 +147,9 @@ void SBWatchpoint::SetEnabled(bool enabled) {
     const bool notify = true;
     if (process_sp) {
       if (enabled)
-        process_sp->EnableWatchpoint(watchpoint_sp.get(), notify);
+        process_sp->EnableWatchpoint(watchpoint_sp, notify);
       else
-        process_sp->DisableWatchpoint(watchpoint_sp.get(), notify);
+        process_sp->DisableWatchpoint(watchpoint_sp, notify);
     } else {
       watchpoint_sp->SetEnabled(enabled, notify);
     }
diff --git a/lldb/source/Breakpoint/Watchpoint.cpp 
b/lldb/source/Breakpoint/Watchpoint.cpp
index 14144214faea747..5417ec728577596 100644
--- a/lldb/source/Breakpoint/Watchpoint.cpp
+++ b/lldb/source/Breakpoint/Watchpoint.cpp
@@ -12,6 +12,7 @@
 #include "lldb/Core/Value.h"
 #include "lldb/Core/ValueObject.h"
 #include "lldb/Core/ValueObjectMemory.h"
+#include "lldb/DataFormatters/DumpValueObjectOptions.h"
 #include "lldb/Expression/UserExpression.h"
 #include "lldb/Symbol/TypeSystem.h"
 #include "lldb/Target/Process.h"
@@ -161,7 +162,7 @@ bool Watchpoint::VariableWatchpointDisabler(void *baton,
               "callback for watchpoint %" PRId32
               " matched internal breakpoint execution context",
               watch_sp->GetID());
-    process_sp->DisableWatchpoint(watch_sp.get());
+    process_sp->DisableWatchpoint(watch_sp);
     return false;
   }
   LLDB_LOGF(log,
@@ -266,33 +267,73 @@ void Watchpoint::Dump(Stream *s) const {
 
 // If prefix is nullptr, we display the watch id and ignore the prefix
 // altogether.
-void Watchpoint::DumpSnapshots(Stream *s, const char *prefix) const {
-  if (!prefix) {
-    s->Printf("\nWatchpoint %u hit:", GetID());
-    prefix = "";
-  }
+bool Watchpoint::DumpSnapshots(Stream *s, const char *prefix) const {
+  bool printed_anything = false;
+
+  // For read watchpoints, don't display any before/after value changes.
+  if (m_watch_read && !m_watch_modify && !m_watch_write)
+    return printed_anything;
+
+  s->Printf("\n");
+  s->Printf("Watchpoint %u hit:\n", GetID());
+
+  StreamString values_ss;
+  if (prefix)
+    values_ss.Indent(prefix);
 
   if (m_old_value_sp) {
     const char *old_value_cstr = m_old_value_sp->GetValueAsCString();
-    if (old_value_cstr && old_value_cstr[0])
-      s->Printf("\n%sold value: %s", prefix, old_value_cstr);
-    else {
+    if (old_value_cstr) {
+      values_ss.Printf("old value: %s", old_value_cstr);
+    } else {
       const char *old_summary_cstr = m_old_value_sp->GetSummaryAsCString();
-      if (old_summary_cstr && old_summary_cstr[0])
-        s->Printf("\n%sold value: %s", prefix, old_summary_cstr);
+      if (old_summary_cstr)
+        values_ss.Printf("old value: %s", old_summary_cstr);
+      else {
+        StreamString strm;
+        DumpValueObjectOptions options;
+        options.SetUseDynamicType(eNoDynamicValues)
+            .SetHideRootType(true)
+            .SetHideRootName(true)
+            .SetHideName(true);
+        m_old_value_sp->Dump(strm, options);
+        if (strm.GetData())
+          values_ss.Printf("old value: %s", strm.GetData());
+      }
     }
   }
 
   if (m_new_value_sp) {
+    if (values_ss.GetSize())
+      values_ss.Printf("\n");
+
     const char *new_value_cstr = m_new_value_sp->GetValueAsCString();
-    if (new_value_cstr && new_value_cstr[0])
-      s->Printf("\n%snew value: %s", prefix, new_value_cstr);
+    if (new_value_cstr)
+      values_ss.Printf("new value: %s", new_value_cstr);
     else {
       const char *new_summary_cstr = m_new_value_sp->GetSummaryAsCString();
-      if (new_summary_cstr && new_summary_cstr[0])
-        s->Printf("\n%snew value: %s", prefix, new_summary_cstr);
+      if (new_summary_cstr)
+        values_ss.Printf("new value: %s", new_summary_cstr);
+      else {
+        StreamString strm;
+        DumpValueObjectOptions options;
+        options.SetUseDynamicType(eNoDynamicValues)
+            .SetHideRootType(true)
+            .SetHideRootName(true)
+            .SetHideName(true);
+        m_new_value_sp->Dump(strm, options);
+        if (strm.GetData())
+          values_ss.Printf("new value: %s", strm.GetData());
+      }
     }
   }
+
+  if (values_ss.GetSize()) {
+    s->Printf("%s", values_ss.GetData());
+    printed_anything = true;
+  }
+
+  return printed_anything;
 }
 
 void Watchpoint::DumpWithLevel(Stream *s,
diff --git a/lldb/source/Commands/CommandObjectWatchpoint.cpp 
b/lldb/source/Commands/CommandObjectWatchpoint.cpp
index dc5be0da43f5e62..92d3999827db238 100644
--- a/lldb/source/Commands/CommandObjectWatchpoint.cpp
+++ b/lldb/source/Commands/CommandObjectWatchpoint.cpp
@@ -930,9 +930,9 @@ corresponding to the byte size of the data type.");
       if (addr_type == eAddressTypeLoad) {
         // We're in business.
         // Find out the size of this variable.
-        size = m_option_watchpoint.watch_size == 0
+        size = m_option_watchpoint.watch_size.GetCurrentValue() == 0
                    ? valobj_sp->GetByteSize().value_or(0)
-                   : m_option_watchpoint.watch_size;
+                   : m_option_watchpoint.watch_size.GetCurrentValue();
       }
       compiler_type = valobj_sp->GetCompilerType();
     } else {
@@ -1127,8 +1127,8 @@ class CommandObjectWatchpointSetExpression : public 
CommandObjectRaw {
       return false;
     }
 
-    if (m_option_watchpoint.watch_size != 0)
-      size = m_option_watchpoint.watch_size;
+    if (m_option_watchpoint.watch_size.GetCurrentValue() != 0)
+      size = m_option_watchpoint.watch_size.GetCurrentValue();
     else
       size = target->GetArchitecture().GetAddressByteSize();
 
diff --git a/lldb/source/Interpreter/OptionGroupWatchpoint.cpp 
b/lldb/source/Interpreter/OptionGroupWatchpoint.cpp
index c3708e7a1e80fd2..d1ae916cd74b1c0 100644
--- a/lldb/source/Interpreter/OptionGroupWatchpoint.cpp
+++ b/lldb/source/Interpreter/OptionGroupWatchpoint.cpp
@@ -39,35 +39,12 @@ static constexpr OptionEnumValueElement g_watch_type[] = {
     },
 };
 
-static constexpr OptionEnumValueElement g_watch_size[] = {
-    {
-        1,
-        "1",
-        "Watch for byte size of 1",
-    },
-    {
-        2,
-        "2",
-        "Watch for byte size of 2",
-    },
-    {
-        4,
-        "4",
-        "Watch for byte size of 4",
-    },
-    {
-        8,
-        "8",
-        "Watch for byte size of 8",
-    },
-};
-
 static constexpr OptionDefinition g_option_table[] = {
     {LLDB_OPT_SET_1, false, "watch", 'w', OptionParser::eRequiredArgument,
      nullptr, OptionEnumValues(g_watch_type), 0, eArgTypeWatchType,
      "Specify the type of watching to perform."},
     {LLDB_OPT_SET_1, false, "size", 's', OptionParser::eRequiredArgument,
-     nullptr, OptionEnumValues(g_watch_size), 0, eArgTypeByteSize,
+     nullptr, {}, 0, eArgTypeByteSize, 
      "Number of bytes to use to watch a region."},
     {LLDB_OPT_SET_2,
      false,
@@ -80,16 +57,6 @@ static constexpr OptionDefinition g_option_table[] = {
      eArgTypeLanguage,
      "Language of expression to run"}};
 
-bool OptionGroupWatchpoint::IsWatchSizeSupported(uint32_t watch_size) {
-  for (const auto& size : g_watch_size) {
-    if (0  == size.value)
-      break;
-    if (watch_size == size.value)
-      return true;
-  }
-  return false;
-}
-
 Status
 OptionGroupWatchpoint::SetOptionValue(uint32_t option_idx,
                                       llvm::StringRef option_arg,
@@ -120,8 +87,10 @@ OptionGroupWatchpoint::SetOptionValue(uint32_t option_idx,
     break;
   }
   case 's':
-    watch_size = (uint32_t)OptionArgParser::ToOptionEnum(
-        option_arg, g_option_table[option_idx].enum_values, 0, error);
+    error = watch_size.SetValueFromString(option_arg);
+    if (watch_size.GetCurrentValue() == 0)
+      error.SetErrorStringWithFormat("invalid --size option value '%s'",
+                                     option_arg.str().c_str());
     break;
 
   default:
@@ -135,7 +104,7 @@ void OptionGroupWatchpoint::OptionParsingStarting(
     ExecutionContext *execution_context) {
   watch_type_specified = false;
   watch_type = eWatchInvalid;
-  watch_size = 0;
+  watch_size.Clear();
   language_type = eLanguageTypeUnknown;
 }
 
diff --git a/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp 
b/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
index 79f8b15a7f229cc..2797b94ad67cc43 100644
--- a/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
+++ b/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
@@ -672,20 +672,6 @@ Status ProcessKDP::DisableBreakpointSite(BreakpointSite 
*bp_site) {
   return DisableSoftwareBreakpoint(bp_site);
 }
 
-Status ProcessKDP::EnableWatchpoint(Watchpoint *wp, bool notify) {
-  Status error;
-  error.SetErrorString(
-      "watchpoints are not supported in kdp remote debugging");
-  return error;
-}
-
-Status ProcessKDP::DisableWatchpoint(Watchpoint *wp, bool notify) {
-  Status error;
-  error.SetErrorString(
-      "watchpoints are not supported in kdp remote debugging");
-  return error;
-}
-
 void ProcessKDP::Clear() { m_thread_list.Clear(); }
 
 Status ProcessKDP::DoSignal(int signo) {
diff --git a/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h 
b/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h
index 3c12fd4074499a9..e5ec5914f9600d0 100644
--- a/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h
+++ b/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h
@@ -124,13 +124,6 @@ class ProcessKDP : public lldb_private::Process {
   lldb_private::Status
   DisableBreakpointSite(lldb_private::BreakpointSite *bp_site) override;
 
-  // Process Watchpoints
-  lldb_private::Status EnableWatchpoint(lldb_private::Watchpoint *wp,
-                                        bool notify = true) override;
-
-  lldb_private::Status DisableWatchpoint(lldb_private::Watchpoint *wp,
-                                         bool notify = true) override;
-
   CommunicationKDP &GetCommunication() { return m_comm; }
 
 protected:
diff --git a/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp 
b/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
index d60e6250c7c0aca..7ef0bebe54ef425 100644
--- a/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
+++ b/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
@@ -491,6 +491,9 @@ static StopInfoSP GetStopInfoForHardwareBP(Thread &thread, 
Target *target,
                                            uint64_t exc_sub_sub_code) {
   // Try hardware watchpoint.
   if (target) {
+    // LWP_TODO: We need to find the WatchpointResource that matches
+    // the address, and evaluate its Watchpoints.
+
     // The exc_sub_code indicates the data break address.
     lldb::Wat...
[truncated]

``````````

</details>


https://github.com/llvm/llvm-project/pull/68845
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to