[lldb] Fix {break,watch}point command function stopping behaviour

In order to run a {break,watch}point command, lldb can resolve to the
script interpreter to run an arbitrary piece of code or call into a
user-provided function. To do so, we will generate a wrapping function,
where we first copy lldb's internal dictionary keys into the
interpreter's global dictionary, copied inline the user code before
resetting the global dictionary to its previous state.

However, {break,watch}point commands can optionally return a value that
would tell lldb whether we should stop or not. This feature was
only implemented for breakpoint commands and since we inlined the user
code directly into the wrapping function, introducing an early return,
that caused lldb to let the interpreter global dictionary tinted with the
internal dictionary keys.

This patch fixes that issue while also adding the stopping behaviour to
watchpoint commands.

To do so, this patch refactors the {break,watch}point command creation
method, to let the lldb wrapper function generator know if the user code is
a function call or a arbitrary expression.

Then the wrapper generator, if the user input was a function call, the
wrapper function will call the user function and save the return value into
a variable. If the user input was an arbitrary expression, the wrapper  will
inline it into a nested function, call the nested function and save the
return value into the same variable. After resetting the interpreter global
dictionary to its previous state, the generated wrapper function will return
the varible containing the return value.

rdar://105461140

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

Signed-off-by: Med Ismail Bennani <[email protected]>

Author: medismailben

lldb/include/lldb/Interpreter/ScriptInterpreter.h

@@ -178,17 +178,18 @@ class ScriptInterpreter : public PluginInterface {
     return error;
   }
 
-  virtual Status GenerateBreakpointCommandCallbackData(
-      StringList &input,
-      std::string &output,
-      bool has_extra_args) {
+  virtual Status GenerateBreakpointCommandCallbackData(StringList &input,
+                                                       std::string &output,
+                                                       bool has_extra_args,
+                                                       const bool is_callback) {
     Status error;
     error.SetErrorString("not implemented");
     return error;
   }
 
   virtual bool GenerateWatchpointCommandCallbackData(StringList &input,
-                                                     std::string &output) {
+                                                     std::string &output,
+                                                     const bool is_callback) {
     return false;
   }
 
@@ -354,7 +355,8 @@ class ScriptInterpreter : public PluginInterface {
   }
 
   virtual Status GenerateFunction(const char *signature,
-                                  const StringList &input) {
+                                  const StringList &input,
+                                  const bool is_callback) {
     Status error;
     error.SetErrorString("unimplemented");
     return error;
@@ -374,7 +376,8 @@ class ScriptInterpreter : public PluginInterface {
       const char *callback_text);
 
   virtual Status SetBreakpointCommandCallback(BreakpointOptions &bp_options,
-                                              const char *callback_text) {
+                                              const char *callback_text,
+                                              const bool is_callback) {
     Status error;
     error.SetErrorString("unimplemented");
     return error;
@@ -405,7 +408,8 @@ class ScriptInterpreter : public PluginInterface {
 
   /// Set a one-liner as the callback for the watchpoint.
   virtual void SetWatchpointCommandCallback(WatchpointOptions *wp_options,
-                                            const char *oneliner) {}
+                                            const char *user_input,
+                                            const bool is_callback) {}
 
   virtual bool GetScriptedSummary(const char *function_name,
                                   lldb::ValueObjectSP valobj,

lldb/source/API/SBBreakpoint.cpp

@@ -645,7 +645,8 @@ SBError SBBreakpoint::SetScriptCallbackBody(const char *callback_body_text) {
         bkpt_sp->GetTarget()
             .GetDebugger()
             .GetScriptInterpreter()
-            ->SetBreakpointCommandCallback(bp_options, callback_body_text);
+            ->SetBreakpointCommandCallback(bp_options, callback_body_text,
+                                           /*is_callback=*/false);
     sb_error.SetError(error);
   } else
     sb_error.SetErrorString("invalid breakpoint");

lldb/source/API/SBBreakpointLocation.cpp

@@ -263,7 +263,8 @@ SBBreakpointLocation::SetScriptCallbackBody(const char *callback_body_text) {
             .GetTarget()
             .GetDebugger()
             .GetScriptInterpreter()
-            ->SetBreakpointCommandCallback(bp_options, callback_body_text);
+            ->SetBreakpointCommandCallback(bp_options, callback_body_text,
+                                           /*is_callback=*/false);
     sb_error.SetError(error);
   } else
     sb_error.SetErrorString("invalid breakpoint");

lldb/source/API/SBBreakpointName.cpp

@@ -593,11 +593,11 @@ SBBreakpointName::SetScriptCallbackBody(const char *callback_body_text) {
         m_impl_up->GetTarget()->GetAPIMutex());
 
   BreakpointOptions &bp_options = bp_name->GetOptions();
-  Status error =
-      m_impl_up->GetTarget()
-          ->GetDebugger()
-          .GetScriptInterpreter()
-          ->SetBreakpointCommandCallback(bp_options, callback_body_text);
+  Status error = m_impl_up->GetTarget()
+                     ->GetDebugger()
+                     .GetScriptInterpreter()
+                     ->SetBreakpointCommandCallback(
+                         bp_options, callback_body_text, /*is_callback=*/false);
   sb_error.SetError(error);
   if (!sb_error.Fail())
     UpdateName(*bp_name);

lldb/source/Commands/CommandObjectWatchpointCommand.cpp

@@ -415,17 +415,18 @@ are no syntax errors may indicate that a function was declared but never called.
           // Special handling for one-liner specified inline.
           if (m_options.m_use_one_liner) {
             script_interp->SetWatchpointCommandCallback(
-                wp_options, m_options.m_one_liner.c_str());
+                wp_options, m_options.m_one_liner.c_str(),
+                /*is_callback=*/false);
           }
           // Special handling for using a Python function by name instead of
           // extending the watchpoint callback data structures, we just
           // automatize what the user would do manually: make their watchpoint
           // command be a function call
           else if (!m_options.m_function_name.empty()) {
-            std::string oneliner(m_options.m_function_name);
-            oneliner += "(frame, wp, internal_dict)";
+            std::string function_signature = m_options.m_function_name;
+            function_signature += "(frame, wp, internal_dict)";
             script_interp->SetWatchpointCommandCallback(
-                wp_options, oneliner.c_str());
+                wp_options, function_signature.c_str(), /*is_callback=*/true);
           } else {
             script_interp->CollectDataForWatchpointCommandCallback(wp_options,
                                                                    result);

lldb/source/Interpreter/ScriptInterpreter.cpp

@@ -108,7 +108,8 @@ Status ScriptInterpreter::SetBreakpointCommandCallback(
     const char *callback_text) {
   Status error;
   for (BreakpointOptions &bp_options : bp_options_vec) {
-    error = SetBreakpointCommandCallback(bp_options, callback_text);
+    error = SetBreakpointCommandCallback(bp_options, callback_text,
+                                         /*is_callback=*/false);
     if (!error.Success())
       break;
   }

lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h

@@ -88,7 +88,8 @@ class ScriptInterpreterLua : public ScriptInterpreter {
                                           CommandReturnObject &result) override;
 
   Status SetBreakpointCommandCallback(BreakpointOptions &bp_options,
-                                      const char *command_body_text) override;
+                                      const char *command_body_text,
+                                      const bool is_callback) override;
 
   void SetWatchpointCommandCallback(WatchpointOptions *wp_options,
                                     const char *command_body_text) override;