[Lldb-commits] [lldb] 9a9fce1 - [lldb] Fix {break, watch}point command function stopping behaviour

Med Ismail Bennani via lldb-commits lldb-commits at lists.llvm.org
Tue Feb 28 11:40:15 PST 2023


Author: Med Ismail Bennani
Date: 2023-02-28T11:39:58-08:00
New Revision: 9a9fce1fed6d2af3fb6d1d7073c4a0b3e00bc515

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

LOG: [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 <medismail.bennani at gmail.com>

Added: 
    

Modified: 
    lldb/include/lldb/Interpreter/ScriptInterpreter.h
    lldb/source/API/SBBreakpoint.cpp
    lldb/source/API/SBBreakpointLocation.cpp
    lldb/source/API/SBBreakpointName.cpp
    lldb/source/Commands/CommandObjectWatchpointCommand.cpp
    lldb/source/Interpreter/ScriptInterpreter.cpp
    lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
    lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
    lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
    lldb/test/API/commands/watchpoints/watchpoint_commands/command/TestWatchpointCommandPython.py
    lldb/test/API/commands/watchpoints/watchpoint_commands/command/main.cpp
    lldb/test/API/commands/watchpoints/watchpoint_commands/command/watchpoint_command.py

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Interpreter/ScriptInterpreter.h b/lldb/include/lldb/Interpreter/ScriptInterpreter.h
index 4917b7a508aed..7db1e41d6cb06 100644
--- a/lldb/include/lldb/Interpreter/ScriptInterpreter.h
+++ b/lldb/include/lldb/Interpreter/ScriptInterpreter.h
@@ -183,17 +183,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;
   }
 
@@ -359,7 +360,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;
@@ -379,7 +381,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;
@@ -410,7 +413,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,

diff  --git a/lldb/source/API/SBBreakpoint.cpp b/lldb/source/API/SBBreakpoint.cpp
index 19b2a4376cf8a..c83fe8c13752e 100644
--- a/lldb/source/API/SBBreakpoint.cpp
+++ b/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");

diff  --git a/lldb/source/API/SBBreakpointLocation.cpp b/lldb/source/API/SBBreakpointLocation.cpp
index a253530f5718b..58b51d0832d91 100644
--- a/lldb/source/API/SBBreakpointLocation.cpp
+++ b/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");

diff  --git a/lldb/source/API/SBBreakpointName.cpp b/lldb/source/API/SBBreakpointName.cpp
index 796229d04ce49..372238a6685e5 100644
--- a/lldb/source/API/SBBreakpointName.cpp
+++ b/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);

diff  --git a/lldb/source/Commands/CommandObjectWatchpointCommand.cpp b/lldb/source/Commands/CommandObjectWatchpointCommand.cpp
index b33f403a69647..37052ddd62c88 100644
--- a/lldb/source/Commands/CommandObjectWatchpointCommand.cpp
+++ b/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);

diff  --git a/lldb/source/Interpreter/ScriptInterpreter.cpp b/lldb/source/Interpreter/ScriptInterpreter.cpp
index 60b676d2a5cd0..90884d20f5243 100644
--- a/lldb/source/Interpreter/ScriptInterpreter.cpp
+++ b/lldb/source/Interpreter/ScriptInterpreter.cpp
@@ -112,7 +112,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;
   }

diff  --git a/lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h b/lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
index b601779ff301c..b90a12d72bfa1 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.h
+++ b/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;

diff  --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
index edce7c894e4d3..00d8d38f4421d 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -521,7 +521,8 @@ void ScriptInterpreterPythonImpl::IOHandlerInputComplete(IOHandler &io_handler,
       StructuredData::ObjectSP empty_args_sp;
       if (GenerateBreakpointCommandCallbackData(data_up->user_source,
                                                 data_up->script_source,
-                                                false)
+                                                /*has_extra_args=*/false,
+                                                /*is_callback=*/false)
               .Success()) {
         auto baton_sp = std::make_shared<BreakpointOptions::CommandBaton>(
             std::move(data_up));
@@ -544,7 +545,8 @@ void ScriptInterpreterPythonImpl::IOHandlerInputComplete(IOHandler &io_handler,
     data_up->user_source.SplitIntoLines(data);
 
     if (GenerateWatchpointCommandCallbackData(data_up->user_source,
-                                              data_up->script_source)) {
+                                              data_up->script_source,
+                                              /*is_callback=*/false)) {
       auto baton_sp =
           std::make_shared<WatchpointOptions::CommandBaton>(std::move(data_up));
       wp_options->SetCallback(
@@ -1158,8 +1160,7 @@ Status ScriptInterpreterPythonImpl::SetBreakpointCommandCallbackFunction(
     StructuredData::ObjectSP extra_args_sp) {
   Status error;
   // For now just cons up a oneliner that calls the provided function.
-  std::string oneliner("return ");
-  oneliner += function_name;
+  std::string function_signature = function_name;
 
   llvm::Expected<unsigned> maybe_args =
       GetMaxPositionalArgumentsForCallable(function_name);
@@ -1174,7 +1175,7 @@ Status ScriptInterpreterPythonImpl::SetBreakpointCommandCallbackFunction(
   bool uses_extra_args = false;
   if (max_args >= 4) {
     uses_extra_args = true;
-    oneliner += "(frame, bp_loc, extra_args, internal_dict)";
+    function_signature += "(frame, bp_loc, extra_args, internal_dict)";
   } else if (max_args >= 3) {
     if (extra_args_sp) {
       error.SetErrorString("cannot pass extra_args to a three argument callback"
@@ -1182,7 +1183,7 @@ Status ScriptInterpreterPythonImpl::SetBreakpointCommandCallbackFunction(
       return error;
     }
     uses_extra_args = false;
-    oneliner += "(frame, bp_loc, internal_dict)";
+    function_signature += "(frame, bp_loc, internal_dict)";
   } else {
     error.SetErrorStringWithFormat("expected 3 or 4 argument "
                                    "function, %s can only take %zu",
@@ -1190,8 +1191,9 @@ Status ScriptInterpreterPythonImpl::SetBreakpointCommandCallbackFunction(
     return error;
   }
 
-  SetBreakpointCommandCallback(bp_options, oneliner.c_str(), extra_args_sp,
-                               uses_extra_args);
+  SetBreakpointCommandCallback(bp_options, function_signature.c_str(),
+                               extra_args_sp, uses_extra_args,
+                               /*is_callback=*/true);
   return error;
 }
 
@@ -1201,7 +1203,8 @@ Status ScriptInterpreterPythonImpl::SetBreakpointCommandCallback(
   Status error;
   error = GenerateBreakpointCommandCallbackData(cmd_data_up->user_source,
                                                 cmd_data_up->script_source,
-                                                false);
+                                                /*has_extra_args=*/false,
+                                                /*is_callback=*/false);
   if (error.Fail()) {
     return error;
   }
@@ -1213,14 +1216,17 @@ Status ScriptInterpreterPythonImpl::SetBreakpointCommandCallback(
 }
 
 Status ScriptInterpreterPythonImpl::SetBreakpointCommandCallback(
-    BreakpointOptions &bp_options, const char *command_body_text) {
-  return SetBreakpointCommandCallback(bp_options, command_body_text, {},false);
+    BreakpointOptions &bp_options, const char *command_body_text,
+    const bool is_callback) {
+  return SetBreakpointCommandCallback(bp_options, command_body_text, {},
+                                      /*uses_extra_args=*/false, is_callback);
 }
 
 // Set a Python one-liner as the callback for the breakpoint.
 Status ScriptInterpreterPythonImpl::SetBreakpointCommandCallback(
     BreakpointOptions &bp_options, const char *command_body_text,
-    StructuredData::ObjectSP extra_args_sp, bool uses_extra_args) {
+    StructuredData::ObjectSP extra_args_sp, bool uses_extra_args,
+    const bool is_callback) {
   auto data_up = std::make_unique<CommandDataPython>(extra_args_sp);
   // Split the command_body_text into lines, and pass that to
   // GenerateBreakpointCommandCallbackData.  That will wrap the body in an
@@ -1228,9 +1234,9 @@ Status ScriptInterpreterPythonImpl::SetBreakpointCommandCallback(
   // That is what the callback will actually invoke.
 
   data_up->user_source.SplitIntoLines(command_body_text);
-  Status error = GenerateBreakpointCommandCallbackData(data_up->user_source,
-                                                       data_up->script_source,
-                                                       uses_extra_args);
+  Status error = GenerateBreakpointCommandCallbackData(
+      data_up->user_source, data_up->script_source, uses_extra_args,
+      is_callback);
   if (error.Success()) {
     auto baton_sp =
         std::make_shared<BreakpointOptions::CommandBaton>(std::move(data_up));
@@ -1243,7 +1249,8 @@ Status ScriptInterpreterPythonImpl::SetBreakpointCommandCallback(
 
 // Set a Python one-liner as the callback for the watchpoint.
 void ScriptInterpreterPythonImpl::SetWatchpointCommandCallback(
-    WatchpointOptions *wp_options, const char *oneliner) {
+    WatchpointOptions *wp_options, const char *user_input,
+    const bool is_callback) {
   auto data_up = std::make_unique<WatchpointOptions::CommandData>();
 
   // It's necessary to set both user_source and script_source to the oneliner.
@@ -1251,11 +1258,11 @@ void ScriptInterpreterPythonImpl::SetWatchpointCommandCallback(
   // command list) while the latter is used for Python to interpret during the
   // actual callback.
 
-  data_up->user_source.AppendString(oneliner);
-  data_up->script_source.assign(oneliner);
+  data_up->user_source.AppendString(user_input);
+  data_up->script_source.assign(user_input);
 
-  if (GenerateWatchpointCommandCallbackData(data_up->user_source,
-                                            data_up->script_source)) {
+  if (GenerateWatchpointCommandCallbackData(
+          data_up->user_source, data_up->script_source, is_callback)) {
     auto baton_sp =
         std::make_shared<WatchpointOptions::CommandBaton>(std::move(data_up));
     wp_options->SetCallback(
@@ -1275,7 +1282,8 @@ Status ScriptInterpreterPythonImpl::ExportFunctionDefinitionToInterpreter(
 }
 
 Status ScriptInterpreterPythonImpl::GenerateFunction(const char *signature,
-                                                     const StringList &input) {
+                                                     const StringList &input,
+                                                     const bool is_callback) {
   Status error;
   int num_lines = input.GetSize();
   if (num_lines == 0) {
@@ -1292,40 +1300,61 @@ Status ScriptInterpreterPythonImpl::GenerateFunction(const char *signature,
   StringList auto_generated_function;
   auto_generated_function.AppendString(signature);
   auto_generated_function.AppendString(
-      "     global_dict = globals()"); // Grab the global dictionary
+      "    global_dict = globals()"); // Grab the global dictionary
   auto_generated_function.AppendString(
-      "     new_keys = internal_dict.keys()"); // Make a list of keys in the
-                                               // session dict
+      "    new_keys = internal_dict.keys()"); // Make a list of keys in the
+                                              // session dict
   auto_generated_function.AppendString(
-      "     old_keys = global_dict.keys()"); // Save list of keys in global dict
+      "    old_keys = global_dict.keys()"); // Save list of keys in global dict
   auto_generated_function.AppendString(
-      "     global_dict.update (internal_dict)"); // Add the session dictionary
-                                                  // to the
-  // global dictionary.
-
-  // Wrap everything up inside the function, increasing the indentation.
-
-  auto_generated_function.AppendString("     if True:");
-  for (int i = 0; i < num_lines; ++i) {
-    sstr.Clear();
-    sstr.Printf("       %s", input.GetStringAtIndex(i));
-    auto_generated_function.AppendString(sstr.GetData());
+      "    global_dict.update(internal_dict)"); // Add the session dictionary
+                                                // to the global dictionary.
+
+  if (is_callback) {
+    // If the user input is a callback to a python function, make sure the input
+    // is only 1 line, otherwise appending the user input would break the
+    // generated wrapped function
+    if (num_lines == 1) {
+      sstr.Clear();
+      sstr.Printf("    __return_val = %s", input.GetStringAtIndex(0));
+      auto_generated_function.AppendString(sstr.GetData());
+    } else {
+      return Status("ScriptInterpreterPythonImpl::GenerateFunction(is_callback="
+                    "true) = ERROR: python function is multiline.");
+    }
+  } else {
+    auto_generated_function.AppendString(
+        "    __return_val = None"); // Initialize user callback return value.
+    auto_generated_function.AppendString(
+        "    def __user_code():"); // Create a nested function that will wrap
+                                   // the user input. This is necessary to
+                                   // capture the return value of the user input
+                                   // and prevent early returns.
+    for (int i = 0; i < num_lines; ++i) {
+      sstr.Clear();
+      sstr.Printf("      %s", input.GetStringAtIndex(i));
+      auto_generated_function.AppendString(sstr.GetData());
+    }
+    auto_generated_function.AppendString(
+        "    __return_val = __user_code()"); //  Call user code and capture
+                                             //  return value
   }
   auto_generated_function.AppendString(
-      "     for key in new_keys:"); // Iterate over all the keys from session
-                                    // dict
+      "    for key in new_keys:"); // Iterate over all the keys from session
+                                   // dict
   auto_generated_function.AppendString(
-      "         internal_dict[key] = global_dict[key]"); // Update session dict
-                                                         // values
+      "        internal_dict[key] = global_dict[key]"); // Update session dict
+                                                        // values
   auto_generated_function.AppendString(
-      "         if key not in old_keys:"); // If key was not originally in
-                                           // global dict
+      "        if key not in old_keys:"); // If key was not originally in
+                                          // global dict
   auto_generated_function.AppendString(
-      "             del global_dict[key]"); //  ...then remove key/value from
-                                            //  global dict
+      "            del global_dict[key]"); //  ...then remove key/value from
+                                           //  global dict
+  auto_generated_function.AppendString(
+      "    return __return_val"); //  Return the user callback return value.
 
   // Verify that the results are valid Python.
-
   error = ExportFunctionDefinitionToInterpreter(auto_generated_function);
 
   return error;
@@ -1350,7 +1379,8 @@ bool ScriptInterpreterPythonImpl::GenerateTypeScriptFunction(
   sstr.Printf("def %s (valobj, internal_dict):",
               auto_generated_function_name.c_str());
 
-  if (!GenerateFunction(sstr.GetData(), user_input).Success())
+  if (!GenerateFunction(sstr.GetData(), user_input, /*is_callback=*/false)
+           .Success())
     return false;
 
   // Store the name of the auto-generated function to be called.
@@ -1374,7 +1404,8 @@ bool ScriptInterpreterPythonImpl::GenerateScriptAliasFunction(
   sstr.Printf("def %s (debugger, args, exe_ctx, result, internal_dict):",
               auto_generated_function_name.c_str());
 
-  if (!GenerateFunction(sstr.GetData(), user_input).Success())
+  if (!GenerateFunction(sstr.GetData(), user_input, /*is_callback=*/true)
+           .Success())
     return false;
 
   // Store the name of the auto-generated function to be called.
@@ -1971,8 +2002,8 @@ bool ScriptInterpreterPythonImpl::GenerateTypeSynthClass(
 }
 
 Status ScriptInterpreterPythonImpl::GenerateBreakpointCommandCallbackData(
-    StringList &user_input, std::string &output,
-    bool has_extra_args) {
+    StringList &user_input, std::string &output, bool has_extra_args,
+    const bool is_callback) {
   static uint32_t num_created_functions = 0;
   user_input.RemoveBlankLines();
   StreamString sstr;
@@ -1991,7 +2022,7 @@ Status ScriptInterpreterPythonImpl::GenerateBreakpointCommandCallbackData(
     sstr.Printf("def %s (frame, bp_loc, internal_dict):",
                 auto_generated_function_name.c_str());
 
-  error = GenerateFunction(sstr.GetData(), user_input);
+  error = GenerateFunction(sstr.GetData(), user_input, is_callback);
   if (!error.Success())
     return error;
 
@@ -2001,7 +2032,7 @@ Status ScriptInterpreterPythonImpl::GenerateBreakpointCommandCallbackData(
 }
 
 bool ScriptInterpreterPythonImpl::GenerateWatchpointCommandCallbackData(
-    StringList &user_input, std::string &output) {
+    StringList &user_input, std::string &output, const bool is_callback) {
   static uint32_t num_created_functions = 0;
   user_input.RemoveBlankLines();
   StreamString sstr;
@@ -2014,7 +2045,7 @@ bool ScriptInterpreterPythonImpl::GenerateWatchpointCommandCallbackData(
   sstr.Printf("def %s (frame, wp, internal_dict):",
               auto_generated_function_name.c_str());
 
-  if (!GenerateFunction(sstr.GetData(), user_input).Success())
+  if (!GenerateFunction(sstr.GetData(), user_input, is_callback).Success())
     return false;
 
   // Store the name of the auto-generated function to be called.

diff  --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
index 98a05d816b20d..b3568e6c91eda 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
@@ -188,16 +188,17 @@ class ScriptInterpreterPythonImpl : public ScriptInterpreterPython {
       lldb_private::CommandReturnObject &cmd_retobj, Status &error,
       const lldb_private::ExecutionContext &exe_ctx) override;
 
-  Status GenerateFunction(const char *signature,
-                          const StringList &input) override;
+  Status GenerateFunction(const char *signature, const StringList &input,
+                          const bool is_callback) override;
 
-  Status GenerateBreakpointCommandCallbackData(
-      StringList &input,
-      std::string &output,
-      bool has_extra_args) override;
+  Status GenerateBreakpointCommandCallbackData(StringList &input,
+                                               std::string &output,
+                                               bool has_extra_args,
+                                               const bool is_callback) override;
 
   bool GenerateWatchpointCommandCallbackData(StringList &input,
-                                             std::string &output) override;
+                                             std::string &output,
+                                             const bool is_callback) override;
 
   bool GetScriptedSummary(const char *function_name, lldb::ValueObjectSP valobj,
                           StructuredData::ObjectSP &callee_wrapper_sp,
@@ -260,7 +261,8 @@ class ScriptInterpreterPythonImpl : public ScriptInterpreterPython {
 
   /// Set the callback body text into the callback for the breakpoint.
   Status SetBreakpointCommandCallback(BreakpointOptions &bp_options,
-                                      const char *callback_body) override;
+                                      const char *callback_body,
+                                      const bool is_callback) override;
 
   Status SetBreakpointCommandCallbackFunction(
       BreakpointOptions &bp_options, const char *function_name,
@@ -274,11 +276,13 @@ class ScriptInterpreterPythonImpl : public ScriptInterpreterPython {
   Status SetBreakpointCommandCallback(BreakpointOptions &bp_options,
                                       const char *command_body_text,
                                       StructuredData::ObjectSP extra_args_sp,
-                                      bool uses_extra_args);
+                                      bool uses_extra_args,
+                                      const bool is_callback);
 
   /// Set a one-liner as the callback for the watchpoint.
   void SetWatchpointCommandCallback(WatchpointOptions *wp_options,
-                                    const char *oneliner) override;
+                                    const char *user_input,
+                                    const bool is_callback) override;
 
   const char *GetDictionaryName() { return m_dictionary_name.c_str(); }
 

diff  --git a/lldb/test/API/commands/watchpoints/watchpoint_commands/command/TestWatchpointCommandPython.py b/lldb/test/API/commands/watchpoints/watchpoint_commands/command/TestWatchpointCommandPython.py
index b6e11d661ded3..7823426253616 100644
--- a/lldb/test/API/commands/watchpoints/watchpoint_commands/command/TestWatchpointCommandPython.py
+++ b/lldb/test/API/commands/watchpoints/watchpoint_commands/command/TestWatchpointCommandPython.py
@@ -1,4 +1,4 @@
-"""
+"""
 Test 'watchpoint command'.
 """
 
@@ -22,6 +22,8 @@ def setUp(self):
         # Find the line number to break inside main().
         self.line = line_number(
             self.source, '// Set break point at this line.')
+        self.second_line = line_number(
+            self.source, '// Set another breakpoint here.')
         # And the watchpoint variable declaration line number.
         self.decl = line_number(self.source,
                                 '// Watchpoint variable declaration.')
@@ -143,6 +145,32 @@ def test_continue_in_watchpoint_command(self):
         self.expect("thread backtrace", STOPPED_DUE_TO_WATCHPOINT,
                     substrs=['stop reason = watchpoint'])
 
+        # We should have hit the watchpoint once, set cookie to 888, since the
+        # user callback returned True.
+        self.expect("frame variable --show-globals cookie",
+                    substrs=['(int32_t)', 'cookie = 888'])
+
+        self.runCmd("process continue")
+
+        # We should be stopped again due to the watchpoint (write type).
+        # The stop reason of the thread should be watchpoint.
+        self.expect("thread backtrace", STOPPED_DUE_TO_WATCHPOINT,
+                    substrs=['stop reason = watchpoint'])
+
+        # We should have hit the watchpoint a second time, set cookie to 666,
+        # even if the user callback didn't return anything and then continue.
+        self.expect("frame variable --show-globals cookie",
+                    substrs=['(int32_t)', 'cookie = 666'])
+
+        # Add a breakpoint to set a watchpoint when stopped on the breakpoint.
+        lldbutil.run_break_set_by_file_and_line(
+            self, None, self.second_line, num_expected_locations=1)
+
+        self.runCmd("process continue")
+
+        self.expect("thread backtrace", STOPPED_DUE_TO_BREAKPOINT,
+                    substrs=['stop reason = breakpoint'])
+
         # We should have hit the watchpoint once, set cookie to 888, then continued to the
         # second hit and set it to 999
         self.expect("frame variable --show-globals cookie",

diff  --git a/lldb/test/API/commands/watchpoints/watchpoint_commands/command/main.cpp b/lldb/test/API/commands/watchpoints/watchpoint_commands/command/main.cpp
index 7dfdd62d50bcc..35292960d7d17 100644
--- a/lldb/test/API/commands/watchpoints/watchpoint_commands/command/main.cpp
+++ b/lldb/test/API/commands/watchpoints/watchpoint_commands/command/main.cpp
@@ -16,5 +16,5 @@ int main(int argc, char** argv) {
         modify(global);
 
     printf("global=%d\n", global);
-    printf("cookie=%d\n", cookie);
+    printf("cookie=%d\n", cookie); // Set another breakpoint here.
 }

diff  --git a/lldb/test/API/commands/watchpoints/watchpoint_commands/command/watchpoint_command.py b/lldb/test/API/commands/watchpoints/watchpoint_commands/command/watchpoint_command.py
index ae5913a500e0d..7e41d52960421 100644
--- a/lldb/test/API/commands/watchpoints/watchpoint_commands/command/watchpoint_command.py
+++ b/lldb/test/API/commands/watchpoints/watchpoint_commands/command/watchpoint_command.py
@@ -9,7 +9,12 @@ def watchpoint_command(frame, wp, dict):
         print ("I stopped the first time")
         frame.EvaluateExpression("cookie = 888")
         num_hits += 1
-        frame.thread.process.Continue()
+        return True
+    if num_hits == 1:
+        print ("I stopped the second time, but with no return")
+        frame.EvaluateExpression("cookie = 666")
+        num_hits += 1
     else:
         print ("I stopped the %d time" % (num_hits))
         frame.EvaluateExpression("cookie = 999")
+        return False # This cause the process to continue.


        


More information about the lldb-commits mailing list