Revert "[lldb] Add 'modify' type watchpoints, make it default (#66308)"

This reverts commit 933ad5c.

This caused 1 test failure and an unexpected pass on AArch64 Linux:
https://lab.llvm.org/buildbot/#/builders/96/builds/45765

Wasn't reported because the bot was already red at the time.

Author: DavidSpickett

lldb/bindings/headers.swig

@@ -77,5 +77,4 @@
 #include "lldb/API/SBValueList.h"
 #include "lldb/API/SBVariablesOptions.h"
 #include "lldb/API/SBWatchpoint.h"
-#include "lldb/API/SBWatchpointOptions.h"
 %}

lldb/bindings/interface/SBWatchpointOptionsDocstrings.i

@@ -80,7 +80,6 @@
 %include "./interface/SBValueListDocstrings.i"
 %include "./interface/SBVariablesOptionsDocstrings.i"
 %include "./interface/SBWatchpointDocstrings.i"
-%include "./interface/SBWatchpointOptionsDocstrings.i"
 
 /* API headers */
 %include "lldb/API/SBAddress.h"
@@ -153,7 +152,6 @@
 %include "lldb/API/SBValueList.h"
 %include "lldb/API/SBVariablesOptions.h"
 %include "lldb/API/SBWatchpoint.h"
-%include "lldb/API/SBWatchpointOptions.h"
 
 /* Extensions for SB classes */
 %include "./interface/SBAddressExtensions.i"

lldb/bindings/interfaces.swig

@@ -21,7 +21,6 @@
 #include "lldb/API/SBType.h"
 #include "lldb/API/SBValue.h"
 #include "lldb/API/SBWatchpoint.h"
-#include "lldb/API/SBWatchpointOptions.h"
 
 namespace lldb_private {
 namespace python {
@@ -829,13 +828,8 @@ class LLDB_API SBTarget {
 
   lldb::SBWatchpoint FindWatchpointByID(lldb::watch_id_t watch_id);
 
-  LLDB_DEPRECATED("WatchAddress deprecated, use WatchpointCreateByAddress")
   lldb::SBWatchpoint WatchAddress(lldb::addr_t addr, size_t size, bool read,
-                                  bool modify, SBError &error);
-
-  lldb::SBWatchpoint
-  WatchpointCreateByAddress(lldb::addr_t addr, size_t size,
-                            lldb::SBWatchpointOptions options, SBError &error);
+                                  bool write, SBError &error);
 
   bool EnableAllWatchpoints();
 

lldb/include/lldb/API/SBTarget.h

@@ -76,14 +76,12 @@ class Watchpoint : public std::enable_shared_from_this<Watchpoint>,
 
   bool WatchpointRead() const;
   bool WatchpointWrite() const;
-  bool WatchpointModify() const;
   uint32_t GetIgnoreCount() const;
   void SetIgnoreCount(uint32_t n);
   void SetWatchpointType(uint32_t type, bool notify = true);
   void SetDeclInfo(const std::string &str);
   std::string GetWatchSpec();
   void SetWatchSpec(const std::string &str);
-  bool WatchedValueReportable(const ExecutionContext &exe_ctx);
 
   // Snapshot management interface.
   bool IsWatchVariable() const;
@@ -214,8 +212,7 @@ class Watchpoint : public std::enable_shared_from_this<Watchpoint>,
   // again, we check the count, if it is more than 1, it means the user-
   // supplied actions actually want the watchpoint to be disabled!
   uint32_t m_watch_read : 1, // 1 if we stop when the watched data is read from
-      m_watch_write : 1,     // 1 if we stop when the watched data is written to
-      m_watch_modify : 1;    // 1 if we stop when the watched data is changed
+      m_watch_write : 1;     // 1 if we stop when the watched data is written to
   uint32_t m_ignore_count;      // Number of times to ignore this watchpoint
   std::string m_decl_str;       // Declaration information, if any.
   std::string m_watch_spec_str; // Spec for the watchpoint.

lldb/include/lldb/API/SBWatchpointOptions.h

@@ -30,15 +30,13 @@ class OptionGroupWatchpoint : public OptionGroup {
 
   void OptionParsingStarting(ExecutionContext *execution_context) override;
 
-  /// eWatchRead == LLDB_WATCH_TYPE_READ
-  /// eWatchWrite == LLDB_WATCH_TYPE_WRITE
-  /// eWatchModify == LLDB_WATCH_TYPE_MODIFY
-  /// eWatchReadWrite == LLDB_WATCH_TYPE_READ | LLDB_WATCH_TYPE_WRITE
+  // Note:
+  // eWatchRead == LLDB_WATCH_TYPE_READ; and
+  // eWatchWrite == LLDB_WATCH_TYPE_WRITE
   enum WatchType {
     eWatchInvalid = 0,
     eWatchRead,
     eWatchWrite,
-    eWatchModify,
     eWatchReadWrite
   };
 

lldb/include/lldb/Breakpoint/Watchpoint.h

@@ -44,10 +44,8 @@
 #define LLDB_WATCH_ID_IS_VALID(uid) ((uid) != (LLDB_INVALID_WATCH_ID))
 #define LLDB_WATCH_TYPE_READ (1u << 0)
 #define LLDB_WATCH_TYPE_WRITE (1u << 1)
-#define LLDB_WATCH_TYPE_MODIFY (1u << 2)
 #define LLDB_WATCH_TYPE_IS_VALID(type)                                         \
-  ((type & LLDB_WATCH_TYPE_READ) || (type & LLDB_WATCH_TYPE_WRITE) ||          \
-   (type & LLDB_WATCH_TYPE_MODIFY))
+  ((type & LLDB_WATCH_TYPE_READ) || (type & LLDB_WATCH_TYPE_WRITE))
 
 // Generic Register Numbers
 #define LLDB_REGNUM_GENERIC_PC 0    // Program Counter

lldb/include/lldb/Interpreter/OptionGroupWatchpoint.h

@@ -431,21 +431,6 @@ FLAGS_ENUM(WatchpointEventType){
     eWatchpointEventTypeThreadChanged = (1u << 11),
     eWatchpointEventTypeTypeChanged = (1u << 12)};
 
-enum WatchpointWriteType {
-  /// Don't stop when the watched memory region is written to.
-  eWatchpointWriteTypeDisabled,
-  /// Stop on any write access to the memory region, even if
-  /// the value doesn't change.  On some architectures, a write
-  /// near the memory region may be falsely reported as a match,
-  /// and notify this spurious stop as a watchpoint trap.
-  eWatchpointWriteTypeAlways,
-  /// Stop on a write to the memory region that changes its value.
-  /// This is most likely the behavior a user expects, and is the
-  /// behavior in gdb.  lldb can silently ignore writes near the
-  /// watched memory region that are reported as accesses to lldb.
-  eWatchpointWriteTypeOnModify
-};
-
 /// Programming language type.
 ///
 /// These enumerations use the same language enumerations as the DWARF

lldb/include/lldb/lldb-defines.h

@@ -284,7 +284,6 @@ class VariableList;
 class Watchpoint;
 class WatchpointList;
 class WatchpointOptions;
-class WatchpointSetOptions;
 struct CompilerContext;
 struct LineEntry;
 struct PropertyDefinition;

lldb/include/lldb/lldb-enumerations.h

@@ -91,7 +91,6 @@ add_lldb_library(liblldb SHARED ${option_framework}
   SBValueList.cpp
   SBVariablesOptions.cpp
   SBWatchpoint.cpp
-  SBWatchpointOptions.cpp
   SBUnixSignals.cpp
   SystemInitializerFull.cpp
   ${lldb_python_wrapper}

lldb/include/lldb/lldb-forward.h

@@ -1322,39 +1322,27 @@ SBWatchpoint SBTarget::FindWatchpointByID(lldb::watch_id_t wp_id) {
 }
 
 lldb::SBWatchpoint SBTarget::WatchAddress(lldb::addr_t addr, size_t size,
-                                          bool read, bool modify,
+                                          bool read, bool write,
                                           SBError &error) {
   LLDB_INSTRUMENT_VA(this, addr, size, read, write, error);
 
-  SBWatchpointOptions options;
-  options.SetWatchpointTypeRead(read);
-  options.SetWatchpointTypeWrite(eWatchpointWriteTypeOnModify);
-  return WatchpointCreateByAddress(addr, size, options, error);
-}
-
-lldb::SBWatchpoint
-SBTarget::WatchpointCreateByAddress(lldb::addr_t addr, size_t size,
-                                    SBWatchpointOptions options,
-                                    SBError &error) {
-  LLDB_INSTRUMENT_VA(this, addr, size, options, error);
-
   SBWatchpoint sb_watchpoint;
   lldb::WatchpointSP watchpoint_sp;
   TargetSP target_sp(GetSP());
-  uint32_t watch_type = 0;
-  if (options.GetWatchpointTypeRead())
-    watch_type |= LLDB_WATCH_TYPE_READ;
-  if (options.GetWatchpointTypeWrite() == eWatchpointWriteTypeAlways)
-    watch_type |= LLDB_WATCH_TYPE_WRITE;
-  if (options.GetWatchpointTypeWrite() == eWatchpointWriteTypeOnModify)
-    watch_type |= LLDB_WATCH_TYPE_MODIFY;
-  if (watch_type == 0) {
-    error.SetErrorString("Can't create a watchpoint that is neither read nor "
-                         "write nor modify.");
-    return sb_watchpoint;
-  }
-  if (target_sp && addr != LLDB_INVALID_ADDRESS && size > 0) {
+  if (target_sp && (read || write) && addr != LLDB_INVALID_ADDRESS &&
+      size > 0) {
     std::lock_guard<std::recursive_mutex> guard(target_sp->GetAPIMutex());
+    uint32_t watch_type = 0;
+    if (read)
+      watch_type |= LLDB_WATCH_TYPE_READ;
+    if (write)
+      watch_type |= LLDB_WATCH_TYPE_WRITE;
+    if (watch_type == 0) {
+      error.SetErrorString(
+          "Can't create a watchpoint that is neither read nor write.");
+      return sb_watchpoint;
+    }
+
     // Target::CreateWatchpoint() is thread safe.
     Status cw_error;
     // This API doesn't take in a type, so we can't figure out what it is.

lldb/source/API/CMakeLists.txt

@@ -1433,17 +1433,10 @@ lldb::SBWatchpoint SBValue::Watch(bool resolve_location, bool read, bool write,
       return sb_watchpoint;
 
     uint32_t watch_type = 0;
-    if (read) {
+    if (read)
       watch_type |= LLDB_WATCH_TYPE_READ;
-      // read + write, the most likely intention
-      // is to catch all writes to this, not just
-      // value modifications.
-      if (write)
-        watch_type |= LLDB_WATCH_TYPE_WRITE;
-    } else {
-      if (write)
-        watch_type |= LLDB_WATCH_TYPE_MODIFY;
-    }
+    if (write)
+      watch_type |= LLDB_WATCH_TYPE_WRITE;
 
     Status rc;
     CompilerType type(value_sp->GetCompilerType());

lldb/source/API/SBTarget.cpp

@@ -354,8 +354,7 @@ bool SBWatchpoint::IsWatchingWrites() {
     std::lock_guard<std::recursive_mutex> guard(
         watchpoint_sp->GetTarget().GetAPIMutex());
 
-    return watchpoint_sp->WatchpointWrite() ||
-           watchpoint_sp->WatchpointModify();
+    return watchpoint_sp->WatchpointWrite();
   }
 
   return false;