[Lldb-commits] [lldb] f32d24d - Revert "[lldb] Disable variable watchpoints when going out of scope"

Jason Molenda via lldb-commits lldb-commits at lists.llvm.org
Fri May 26 15:32:19 PDT 2023


Author: Jason Molenda
Date: 2023-05-26T15:32:10-07:00
New Revision: f32d24d57742054cd1131dfa5c21f363139bf59b

URL: https://github.com/llvm/llvm-project/commit/f32d24d57742054cd1131dfa5c21f363139bf59b
DIFF: https://github.com/llvm/llvm-project/commit/f32d24d57742054cd1131dfa5c21f363139bf59b.diff

LOG: Revert "[lldb] Disable variable watchpoints when going out of scope"

Reverting https://reviews.llvm.org/D151366 until Ismail has a chance
to look at the ubuntu CI test failures and can reland.

This reverts commit 7c847ac4bd1bd8a89c7fbb4581328fa8cb0498f1.

Added: 
    

Modified: 
    lldb/include/lldb/Breakpoint/Watchpoint.h
    lldb/source/Breakpoint/Watchpoint.cpp
    lldb/source/Commands/CommandObjectWatchpoint.cpp
    lldb/test/Shell/ScriptInterpreter/Lua/watchpoint_callback.test

Removed: 
    lldb/test/Shell/Watchpoint/Inputs/val.c
    lldb/test/Shell/Watchpoint/Inputs/watchpoint.in
    lldb/test/Shell/Watchpoint/LocalVariableWatchpointDisabler.test


################################################################################
diff  --git a/lldb/include/lldb/Breakpoint/Watchpoint.h b/lldb/include/lldb/Breakpoint/Watchpoint.h
index 3ee75516debe8..037be4539066c 100644
--- a/lldb/include/lldb/Breakpoint/Watchpoint.h
+++ b/lldb/include/lldb/Breakpoint/Watchpoint.h
@@ -90,40 +90,6 @@ class Watchpoint : public std::enable_shared_from_this<Watchpoint>,
   void SetWatchVariable(bool val);
   bool CaptureWatchedValue(const ExecutionContext &exe_ctx);
 
-  /// \struct WatchpointVariableContext
-  /// \brief Represents the context of a watchpoint variable.
-  ///
-  /// This struct encapsulates the information related to a watchpoint variable,
-  /// including the watch ID and the execution context in which it is being
-  /// used. This struct is passed as a Baton to the \b
-  /// VariableWatchpointDisabler breakpoint callback.
-  struct WatchpointVariableContext {
-    /// \brief Constructor for WatchpointVariableContext.
-    /// \param watch_id The ID of the watchpoint.
-    /// \param exe_ctx The execution context associated with the watchpoint.
-    WatchpointVariableContext(lldb::watch_id_t watch_id,
-                              ExecutionContext exe_ctx)
-        : watch_id(watch_id), exe_ctx(exe_ctx) {}
-
-    lldb::watch_id_t watch_id; ///< The ID of the watchpoint.
-    ExecutionContext
-        exe_ctx; ///< The execution context associated with the watchpoint.
-  };
-
-  class WatchpointVariableBaton : public TypedBaton<WatchpointVariableContext> {
-  public:
-    WatchpointVariableBaton(std::unique_ptr<WatchpointVariableContext> Data)
-        : TypedBaton(std::move(Data)) {}
-  };
-
-  bool SetupVariableWatchpointDisabler(lldb::StackFrameSP frame_sp) const;
-
-  /// Callback routine to disable the watchpoint set on a local variable when
-  ///  it goes out of scope.
-  static bool VariableWatchpointDisabler(
-      void *baton, lldb_private::StoppointCallbackContext *context,
-      lldb::user_id_t break_id, lldb::user_id_t break_loc_id);
-
   void GetDescription(Stream *s, lldb::DescriptionLevel level);
   void Dump(Stream *s) const override;
   void DumpSnapshots(Stream *s, const char *prefix = nullptr) const;

diff  --git a/lldb/source/Breakpoint/Watchpoint.cpp b/lldb/source/Breakpoint/Watchpoint.cpp
index fa95d6db6cc5e..d8b8bd505db95 100644
--- a/lldb/source/Breakpoint/Watchpoint.cpp
+++ b/lldb/source/Breakpoint/Watchpoint.cpp
@@ -83,94 +83,6 @@ void Watchpoint::SetCallback(WatchpointHitCallback callback,
   SendWatchpointChangedEvent(eWatchpointEventTypeCommandChanged);
 }
 
-bool Watchpoint::SetupVariableWatchpointDisabler(StackFrameSP frame_sp) const {
-  if (!frame_sp)
-    return false;
-
-  ThreadSP thread_sp = frame_sp->GetThread();
-  if (!thread_sp)
-    return false;
-
-  uint32_t return_frame_index =
-      thread_sp->GetSelectedFrameIndex(DoNoSelectMostRelevantFrame) + 1;
-  if (return_frame_index >= LLDB_INVALID_FRAME_ID)
-    return false;
-
-  StackFrameSP return_frame_sp(
-      thread_sp->GetStackFrameAtIndex(return_frame_index));
-  if (!return_frame_sp)
-    return false;
-
-  ExecutionContext exe_ctx(return_frame_sp);
-  TargetSP target_sp = exe_ctx.GetTargetSP();
-  if (!target_sp)
-    return false;
-
-  Address return_address(return_frame_sp->GetFrameCodeAddress());
-  lldb::addr_t return_addr = return_address.GetLoadAddress(target_sp.get());
-  if (return_addr == LLDB_INVALID_ADDRESS)
-    return false;
-
-  BreakpointSP bp_sp = target_sp->CreateBreakpoint(
-      return_addr, /*internal=*/true, /*request_hardware=*/false);
-  if (!bp_sp || !bp_sp->HasResolvedLocations())
-    return false;
-
-  auto wvc_up = std::make_unique<WatchpointVariableContext>(GetID(), exe_ctx);
-  auto baton_sp = std::make_shared<WatchpointVariableBaton>(std::move(wvc_up));
-  bp_sp->SetCallback(VariableWatchpointDisabler, baton_sp);
-  bp_sp->SetOneShot(true);
-  bp_sp->SetBreakpointKind("variable watchpoint disabler");
-  return true;
-}
-
-bool Watchpoint::VariableWatchpointDisabler(void *baton,
-                                            StoppointCallbackContext *context,
-                                            user_id_t break_id,
-                                            user_id_t break_loc_id) {
-  assert(baton && "null baton");
-  if (!baton || !context)
-    return false;
-
-  Log *log = GetLog(LLDBLog::Watchpoints);
-
-  WatchpointVariableContext *wvc =
-      static_cast<WatchpointVariableContext *>(baton);
-
-  LLDB_LOGF(log, "Watchpoint::%s called by breakpoint %" PRIu64 ".%" PRIu64,
-            __FUNCTION__, break_id, break_loc_id);
-
-  if (wvc->watch_id == LLDB_INVALID_WATCH_ID)
-    return false;
-
-  TargetSP target_sp = context->exe_ctx_ref.GetTargetSP();
-  if (!target_sp)
-    return false;
-
-  ProcessSP process_sp = target_sp->GetProcessSP();
-  if (!process_sp)
-    return false;
-
-  WatchpointSP watch_sp =
-      target_sp->GetWatchpointList().FindByID(wvc->watch_id);
-  if (!watch_sp)
-    return false;
-
-  if (wvc->exe_ctx == context->exe_ctx_ref) {
-    LLDB_LOGF(log,
-              "Watchpoint::%s callback for watchpoint %" PRId32
-              " matched internal breakpoint execution context",
-              __FUNCTION__, watch_sp->GetID());
-    process_sp->DisableWatchpoint(watch_sp.get());
-    return false;
-  }
-  LLDB_LOGF(log,
-            "Watchpoint::%s callback for watchpoint %" PRId32
-            " didn't match internal breakpoint execution context",
-            __FUNCTION__, watch_sp->GetID());
-  return false;
-}
-
 void Watchpoint::ClearCallback() {
   m_options.ClearCallback();
   SendWatchpointChangedEvent(eWatchpointEventTypeCommandChanged);

diff  --git a/lldb/source/Commands/CommandObjectWatchpoint.cpp b/lldb/source/Commands/CommandObjectWatchpoint.cpp
index 28411407c923a..14180f9b56d36 100644
--- a/lldb/source/Commands/CommandObjectWatchpoint.cpp
+++ b/lldb/source/Commands/CommandObjectWatchpoint.cpp
@@ -9,7 +9,6 @@
 #include "CommandObjectWatchpoint.h"
 #include "CommandObjectWatchpointCommand.h"
 
-#include <memory>
 #include <vector>
 
 #include "llvm/ADT/StringRef.h"
@@ -21,7 +20,6 @@
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/CommandOptionArgumentTable.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
-#include "lldb/Symbol/Function.h"
 #include "lldb/Symbol/Variable.h"
 #include "lldb/Symbol/VariableList.h"
 #include "lldb/Target/StackFrame.h"
@@ -952,32 +950,27 @@ corresponding to the byte size of the data type.");
     error.Clear();
     WatchpointSP watch_sp =
         target->CreateWatchpoint(addr, size, &compiler_type, watch_type, error);
-    if (!watch_sp) {
+    if (watch_sp) {
+      watch_sp->SetWatchSpec(command.GetArgumentAtIndex(0));
+      watch_sp->SetWatchVariable(true);
+      if (var_sp && var_sp->GetDeclaration().GetFile()) {
+        StreamString ss;
+        // True to show fullpath for declaration file.
+        var_sp->GetDeclaration().DumpStopContext(&ss, true);
+        watch_sp->SetDeclInfo(std::string(ss.GetString()));
+      }
+      output_stream.Printf("Watchpoint created: ");
+      watch_sp->GetDescription(&output_stream, lldb::eDescriptionLevelFull);
+      output_stream.EOL();
+      result.SetStatus(eReturnStatusSuccessFinishResult);
+    } else {
       result.AppendErrorWithFormat(
           "Watchpoint creation failed (addr=0x%" PRIx64 ", size=%" PRIu64
           ", variable expression='%s').\n",
           addr, (uint64_t)size, command.GetArgumentAtIndex(0));
       if (error.AsCString(nullptr))
         result.AppendError(error.AsCString());
-      return result.Succeeded();
-    }
-
-    watch_sp->SetWatchSpec(command.GetArgumentAtIndex(0));
-    watch_sp->SetWatchVariable(true);
-    if (var_sp) {
-      if (var_sp->GetDeclaration().GetFile()) {
-        StreamString ss;
-        // True to show fullpath for declaration file.
-        var_sp->GetDeclaration().DumpStopContext(&ss, true);
-        watch_sp->SetDeclInfo(std::string(ss.GetString()));
-      }
-      if (var_sp->GetScope() == eValueTypeVariableLocal)
-        watch_sp->SetupVariableWatchpointDisabler(m_exe_ctx.GetFrameSP());
     }
-    output_stream.Printf("Watchpoint created: ");
-    watch_sp->GetDescription(&output_stream, lldb::eDescriptionLevelFull);
-    output_stream.EOL();
-    result.SetStatus(eReturnStatusSuccessFinishResult);
 
     return result.Succeeded();
   }

diff  --git a/lldb/test/Shell/ScriptInterpreter/Lua/watchpoint_callback.test b/lldb/test/Shell/ScriptInterpreter/Lua/watchpoint_callback.test
index 11bbe03410774..d72496c25c2ba 100644
--- a/lldb/test/Shell/ScriptInterpreter/Lua/watchpoint_callback.test
+++ b/lldb/test/Shell/ScriptInterpreter/Lua/watchpoint_callback.test
@@ -1,4 +1,4 @@
-# XFAIL: system-netbsd
+# XFAIL: system-netbsd, system-darwin
 # RUN: %clang_host -x c %S/Inputs/val.c -g -o %t
 # RUN: %lldb -b -s %S/Inputs/watchpoint1.in --script-language lua %t 2>&1 | FileCheck %S/Inputs/watchpoint1.in
 # RUN: %lldb -b -s %S/Inputs/watchpoint2.in --script-language lua %t 2>&1 | FileCheck %S/Inputs/watchpoint2.in

diff  --git a/lldb/test/Shell/Watchpoint/Inputs/val.c b/lldb/test/Shell/Watchpoint/Inputs/val.c
deleted file mode 100644
index 5c70375c9d0fa..0000000000000
--- a/lldb/test/Shell/Watchpoint/Inputs/val.c
+++ /dev/null
@@ -1,7 +0,0 @@
-int main() {
-  int val = 0;
-  // Break here
-  val++;
-  val++;
-  return 0;
-}

diff  --git a/lldb/test/Shell/Watchpoint/Inputs/watchpoint.in b/lldb/test/Shell/Watchpoint/Inputs/watchpoint.in
deleted file mode 100644
index c32f1bc958780..0000000000000
--- a/lldb/test/Shell/Watchpoint/Inputs/watchpoint.in
+++ /dev/null
@@ -1,13 +0,0 @@
-breakpoint set -p "Break here"
-r
-watchpoint set variable val
-watchpoint modify -c "val == 1"
-c
-# CHECK: Watchpoint 1 hit:
-# CHECK-NEXT: old value: 0
-# CHECK-NEXT: new value: 1
-# CHECK-NEXT: Process {{[0-9]+}} resuming
-# CHECK-NEXT: Process {{[0-9]+}} stopped
-# CHECK-NEXT: {{.*}} stop reason = watchpoint 1
-c
-# CHECK: Process {{[0-9]+}} exited with status = 0 (0x00000000)

diff  --git a/lldb/test/Shell/Watchpoint/LocalVariableWatchpointDisabler.test b/lldb/test/Shell/Watchpoint/LocalVariableWatchpointDisabler.test
deleted file mode 100644
index 85a19c6f1a42b..0000000000000
--- a/lldb/test/Shell/Watchpoint/LocalVariableWatchpointDisabler.test
+++ /dev/null
@@ -1,3 +0,0 @@
-# XFAIL: system-netbsd
-# RUN: %clang_host -x c %S/Inputs/val.c -g -o %t
-# RUN: %lldb -b -s %S/Inputs/watchpoint.in %t 2>&1 | FileCheck %S/Inputs/watchpoint.in


        


More information about the lldb-commits mailing list