[lldb] Disable variable watchpoints when going out of scope

If we use a variable watchpoint with a condition using a scope variable,
if we go out-of-scope, the watpoint remains active which can the
expression evaluator to fail to parse the watchpoint condition (because
of the missing varible bindings).

This was discovered after `watchpoint_callback.test` started failing on
the green dragon bot.

This patch should address that issue by setting an internal breakpoint
on the return addresss of the current frame when creating a variable
watchpoint. The breakpoint has a callback that will disable the watchpoint
if the the breakpoint execution context matches the watchpoint execution
context.

This is only enabled for local variables.

This patch also re-enables the failing test following e1086384e584.

rdar://109574319

Differential Revision: https://reviews.llvm.org/D151366

Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>

2 files changed, 110 insertions, 15 deletions

diff --git a/lldb/source/Breakpoint/Watchpoint.cpp b/lldb/source/Breakpoint/Watchpoint.cpp
index d8b8bd5..597e696 100644
--- a/lldb/source/Breakpoint/Watchpoint.cpp
+++ b/lldb/source/Breakpoint/Watchpoint.cpp
@@ -83,6 +83,94 @@ 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, "called by breakpoint %" PRIu64 ".%" PRIu64, 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,
+              "callback for watchpoint %" PRId32
+              " matched internal breakpoint execution context",
+              watch_sp->GetID());
+    process_sp->DisableWatchpoint(watch_sp.get());
+    return false;
+  }
+  LLDB_LOGF(log,
+            "callback for watchpoint %" PRId32
+            " didn't match internal breakpoint execution context",
+            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 b73c9a2..a4929ea 100644
--- a/lldb/source/Commands/CommandObjectWatchpoint.cpp
+++ b/lldb/source/Commands/CommandObjectWatchpoint.cpp
@@ -9,6 +9,7 @@
 #include "CommandObjectWatchpoint.h"
 #include "CommandObjectWatchpointCommand.h"
 
+#include <memory>
 #include <vector>
 
 #include "llvm/ADT/StringRef.h"
@@ -20,6 +21,7 @@
 #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"
@@ -950,27 +952,32 @@ protected:
     error.Clear();
     WatchpointSP watch_sp =
         target->CreateWatchpoint(addr, size, &compiler_type, watch_type, error);
-    if (watch_sp) {
-      watch_sp->SetWatchSpec(command.GetArgumentAtIndex(0));
-      watch_sp->SetWatchVariable(true);
-      if (var_sp && var_sp->GetDeclaration().GetFile()) {
+    if (!watch_sp) {
+      result.AppendErrorWithFormat(
+          "Watchpoint creation failed (addr=0x%" PRIx64 ", size=%" PRIu64
+          ", variable expression='%s').\n",
+          addr, static_cast<uint64_t>(size), command.GetArgumentAtIndex(0));
+      if (const char *error_message = error.AsCString(nullptr))
+        result.AppendError(error_message);
+      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()));
       }
-      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());
+      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();
   }