Fix serialization of Python breakpoint commands.

CommandData breakpoint commands didn't know whether they were
Python or Command line commands, so they couldn't serialize &
deserialize themselves properly.  Fix that.
I also changed the "breakpoint list" command to note in the output
when the commands are Python commands.  Fortunately only one test
was relying on this explicit bit of text output.

llvm-svn: 282432

lldb/include/lldb/Breakpoint/BreakpointOptions.h

@@ -34,10 +34,13 @@ namespace lldb_private {
 class BreakpointOptions {
 public:
   struct CommandData {
-    CommandData() : user_source(), script_source(), stop_on_error(true) {}
+    CommandData()
+        : user_source(), script_source(),
+          interpreter(lldb::eScriptLanguageNone), stop_on_error(true) {}
 
-    CommandData(const StringList &user_source)
+    CommandData(const StringList &user_source, lldb::ScriptLanguage interp)
-        : user_source(user_source), script_source(), stop_on_error(true) {}
+        : user_source(user_source), script_source(), interpreter(interp),
+          stop_on_error(true) {}
 
     ~CommandData() = default;
 
@@ -51,12 +54,14 @@ public:
 
     StringList user_source;
     std::string script_source;
+    enum lldb::ScriptLanguage
+        interpreter; // eScriptLanguageNone means command interpreter.
     bool stop_on_error;
 
   private:
     enum class OptionNames : uint32_t {
       UserSource = 0,
-      ScriptSource,
+      Interpreter,
       StopOnError,
       LastOptionName
     };
@@ -112,7 +117,8 @@ public:
   virtual ~BreakpointOptions();
 
   static std::unique_ptr<BreakpointOptions>
-  CreateFromStructuredData(const StructuredData::Dictionary &data_dict,
+  CreateFromStructuredData(Target &target,
+                           const StructuredData::Dictionary &data_dict,
                            Error &error);
 
   virtual StructuredData::ObjectSP SerializeToStructuredData();
@@ -366,16 +372,16 @@ protected:
     OneShotState,
     LastOptionName
   };
-  static const char *g_option_names[(size_t) OptionNames::LastOptionName];
+  static const char *g_option_names[(size_t)OptionNames::LastOptionName];
 
   static const char *GetKey(OptionNames enum_value) {
-    return g_option_names[(size_t) enum_value];
+    return g_option_names[(size_t)enum_value];
   }
 
   static bool BreakpointOptionsCallbackFunction(
       void *baton, StoppointCallbackContext *context, lldb::user_id_t break_id,
       lldb::user_id_t break_loc_id);
-      
+
   void SetThreadSpec(std::unique_ptr<ThreadSpec> &thread_spec_up);
 
 private:

lldb/include/lldb/Interpreter/ScriptInterpreter.h

@@ -16,6 +16,7 @@
 // Project includes
 #include "lldb/lldb-private.h"
 
+#include "lldb/Breakpoint/BreakpointOptions.h"
 #include "lldb/Core/Broadcaster.h"
 #include "lldb/Core/Error.h"
 #include "lldb/Core/PluginInterface.h"
@@ -270,6 +271,15 @@ public:
     return error;
   }
 
+  /// This one is for deserialization:
+  virtual Error SetBreakpointCommandCallback(
+      BreakpointOptions *bp_options,
+      std::unique_ptr<BreakpointOptions::CommandData> &data_up) {
+    Error error;
+    error.SetErrorString("unimplemented");
+    return error;
+  }
+
   void SetBreakpointCommandCallbackFunction(
       std::vector<BreakpointOptions *> &bp_options_vec,
       const char *function_name);
@@ -428,8 +438,12 @@ public:
 
   static std::string LanguageToString(lldb::ScriptLanguage language);
 
+  static lldb::ScriptLanguage StringToLanguage(const llvm::StringRef &string);
+
   virtual void ResetOutputFileHandle(FILE *new_fh) {} // By default, do nothing.
 
+  lldb::ScriptLanguage GetLanguage() { return m_script_lang; }
+
 protected:
   CommandInterpreter &m_interpreter;
   lldb::ScriptLanguage m_script_lang;

lldb/include/lldb/lldb-enumerations.h

@@ -187,7 +187,8 @@ enum DescriptionLevel {
 enum ScriptLanguage {
   eScriptLanguageNone,
   eScriptLanguagePython,
-  eScriptLanguageDefault = eScriptLanguagePython
+  eScriptLanguageDefault = eScriptLanguagePython,
+  eScriptLanguageUnknown
 };
 
 //----------------------------------------------------------------------

lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py

@@ -94,12 +94,12 @@ class BreakpointCommandTestCase(TestBase):
                     substrs=["Breakpoint commands:",
                              "frame variable --show-types --scope"])
         self.expect("breakpoint command list 2", "Breakpoint 2 command ok",
-                    substrs=["Breakpoint commands:",
+                    substrs=["Breakpoint commands (Python):",
                              "here = open",
                              "here.write",
                              "here.close()"])
         self.expect("breakpoint command list 3", "Breakpoint 3 command ok",
-                    substrs=["Breakpoint commands:",
+                    substrs=["Breakpoint commands (Python):",
                              "bktptcmd.function(frame, bp_loc, internal_dict)"])
 
         self.expect("breakpoint command list 4", "Breakpoint 4 command ok",

lldb/packages/Python/lldbsuite/test/functionalities/breakpoint/serialize/TestBreakpointSerialization.py

@@ -200,7 +200,7 @@ class BreakpointSerialization(TestBase):
         bkpt.SetQueueName("grubby")
         bkpt.AddName("FirstName")
         bkpt.AddName("SecondName")
-
+        bkpt.SetScriptCallbackBody('\tprint("I am a function that prints.")\n\tprint("I don\'t do anything else")\n')
         source_bps.Append(bkpt)
 
         bkpt = self.orig_target.BreakpointCreateBySourceRegex("dont really care", blubby_file_spec)

lldb/source/API/SBBreakpoint.cpp

@@ -457,7 +457,7 @@ void SBBreakpoint::SetCommandLineCommands(SBStringList &commands) {
   std::lock_guard<std::recursive_mutex> guard(
       m_opaque_sp->GetTarget().GetAPIMutex());
   std::unique_ptr<BreakpointOptions::CommandData> cmd_data_up(
-      new BreakpointOptions::CommandData(*commands));
+      new BreakpointOptions::CommandData(*commands, eScriptLanguageNone));
 
   m_opaque_sp->GetOptions()->SetCommandDataCallback(cmd_data_up);
 }

lldb/source/Breakpoint/Breakpoint.cpp

@@ -179,8 +179,8 @@ lldb::BreakpointSP Breakpoint::CreateFromStructuredData(
   success = breakpoint_dict->GetValueForKeyAsDictionary(
       BreakpointOptions::GetSerializationKey(), options_dict);
   if (success) {
-    options_up = BreakpointOptions::CreateFromStructuredData(*options_dict,
+    options_up = BreakpointOptions::CreateFromStructuredData(
-                                                             create_error);
+        target, *options_dict, create_error);
     if (create_error.Fail()) {
       error.SetErrorStringWithFormat(
           "Error creating breakpoint options from data: %s.",

lldb/source/Breakpoint/BreakpointOptions.cpp

@@ -55,17 +55,15 @@ BreakpointOptions::CommandData::SerializeToStructuredData() {
     options_dict_sp->AddItem(GetKey(OptionNames::UserSource), user_source_sp);
   }
 
-  if (!script_source.empty()) {
+  options_dict_sp->AddStringItem(
-    StructuredData::StringSP item_sp(new StructuredData::String(script_source));
+      GetKey(OptionNames::Interpreter),
-    options_dict_sp->AddItem(GetKey(OptionNames::ScriptSource), user_source_sp);
+      ScriptInterpreter::LanguageToString(interpreter));
-  }
   return options_dict_sp;
 }
 
 std::unique_ptr<BreakpointOptions::CommandData>
 BreakpointOptions::CommandData::CreateFromStructuredData(
     const StructuredData::Dictionary &options_dict, Error &error) {
-  std::string script_source;
   std::unique_ptr<CommandData> data_up(new CommandData());
   bool found_something = false;
 
@@ -75,11 +73,24 @@ BreakpointOptions::CommandData::CreateFromStructuredData(
   if (success)
     found_something = true;
 
+  std::string interpreter_str;
+  ScriptLanguage interp_language;
   success = options_dict.GetValueForKeyAsString(
-      GetKey(OptionNames::ScriptSource), data_up->script_source);
+      GetKey(OptionNames::Interpreter), interpreter_str);
 
-  if (success)
+  if (!success) {
-    found_something = true;
+    error.SetErrorString("Missing command language value.");
+    return data_up;
+  }
+
+  found_something = true;
+  interp_language = ScriptInterpreter::StringToLanguage(interpreter_str);
+  if (interp_language == eScriptLanguageUnknown) {
+    error.SetErrorStringWithFormat("Unknown breakpoint command language: %s.",
+                                   interpreter_str.c_str());
+    return data_up;
+  }
+  data_up->interpreter = interp_language;
 
   StructuredData::Array *user_source;
   success = options_dict.GetValueForKeyAsArray(GetKey(OptionNames::UserSource),
@@ -184,7 +195,8 @@ BreakpointOptions::CopyOptionsNoCallback(BreakpointOptions &orig) {
 BreakpointOptions::~BreakpointOptions() = default;
 
 std::unique_ptr<BreakpointOptions> BreakpointOptions::CreateFromStructuredData(
-    const StructuredData::Dictionary &options_dict, Error &error) {
+    Target &target, const StructuredData::Dictionary &options_dict,
+    Error &error) {
   bool enabled = true;
   bool one_shot = false;
   int32_t ignore_count = 0;
@@ -230,8 +242,34 @@ std::unique_ptr<BreakpointOptions> BreakpointOptions::CreateFromStructuredData(
 
   auto bp_options = llvm::make_unique<BreakpointOptions>(
       condition_text.c_str(), enabled, ignore_count, one_shot);
-  if (cmd_data_up.get())
+  if (cmd_data_up.get()) {
-    bp_options->SetCommandDataCallback(cmd_data_up);
+    if (cmd_data_up->interpreter == eScriptLanguageNone)
+      bp_options->SetCommandDataCallback(cmd_data_up);
+    else {
+      ScriptInterpreter *interp =
+          target.GetDebugger().GetCommandInterpreter().GetScriptInterpreter();
+      if (!interp) {
+        error.SetErrorStringWithFormat(
+            "Can't set script commands - no script interpreter");
+        return nullptr;
+      }
+      if (interp->GetLanguage() != cmd_data_up->interpreter) {
+        error.SetErrorStringWithFormat(
+            "Current script language doesn't match breakpoint's language: %s",
+            ScriptInterpreter::LanguageToString(cmd_data_up->interpreter)
+                .c_str());
+        return nullptr;
+      }
+      Error script_error;
+      script_error =
+          interp->SetBreakpointCommandCallback(bp_options.get(), cmd_data_up);
+      if (script_error.Fail()) {
+        error.SetErrorStringWithFormat("Error generating script callback: %s.",
+                                       error.AsCString());
+        return nullptr;
+      }
+    }
+  }
 
   StructuredData::Dictionary *thread_spec_dict;
   success = options_dict.GetValueForKeyAsDictionary(
@@ -456,7 +494,12 @@ void BreakpointOptions::CommandBaton::GetDescription(
   }
 
   s->IndentMore();
-  s->Indent("Breakpoint commands:\n");
+  s->Indent("Breakpoint commands");
+  if (data->interpreter != eScriptLanguageNone)
+    s->Printf(" (%s):\n",
+              ScriptInterpreter::LanguageToString(data->interpreter).c_str());
+  else
+    s->PutCString(":\n");
 
   s->IndentMore();
   if (data && data->user_source.GetSize() > 0) {
@@ -474,6 +517,7 @@ void BreakpointOptions::CommandBaton::GetDescription(
 
 void BreakpointOptions::SetCommandDataCallback(
     std::unique_ptr<CommandData> &cmd_data) {
+  cmd_data->interpreter = eScriptLanguageNone;
   auto baton_sp = std::make_shared<CommandBaton>(std::move(cmd_data));
   SetCallback(BreakpointOptions::BreakpointOptionsCallbackFunction, baton_sp);
 }

lldb/source/Commands/CommandObjectBreakpointCommand.cpp

@@ -264,14 +264,7 @@ are no syntax errors may indicate that a function was declared but never called.
     for (auto bp_options : bp_options_vec) {
       auto cmd_data = llvm::make_unique<BreakpointOptions::CommandData>();
 
-      // It's necessary to set both user_source and script_source to the
-      // oneliner.
-      // The former is used to generate callback description (as in breakpoint
-      // command list)
-      // while the latter is used for Python to interpret during the actual
-      // callback.
       cmd_data->user_source.AppendString(oneliner);
-      cmd_data->script_source.assign(oneliner);
       cmd_data->stop_on_error = m_options.m_stop_on_error;
 
       bp_options->SetCommandDataCallback(cmd_data);

lldb/source/Interpreter/ScriptInterpreter.cpp

@@ -57,11 +57,24 @@ std::string ScriptInterpreter::LanguageToString(lldb::ScriptLanguage language) {
   case eScriptLanguagePython:
     return_value = "Python";
     break;
+  case eScriptLanguageUnknown:
+    return_value = "Unknown";
+    break;
   }
 
   return return_value;
 }
 
+lldb::ScriptLanguage
+ScriptInterpreter::StringToLanguage(const llvm::StringRef &language) {
+  if (language.equals_lower(LanguageToString(eScriptLanguageNone)))
+    return eScriptLanguageNone;
+  else if (language.equals_lower(LanguageToString(eScriptLanguagePython)))
+    return eScriptLanguagePython;
+  else
+    return eScriptLanguageUnknown;
+}
+
 Error ScriptInterpreter::SetBreakpointCommandCallback(
     std::vector<BreakpointOptions *> &bp_options_vec,
     const char *callback_text) {

lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

@@ -412,7 +412,7 @@ void ScriptInterpreterPython::IOHandlerInputComplete(IOHandler &io_handler,
       if (!bp_options)
         continue;
 
-      auto data_ap = llvm::make_unique<BreakpointOptions::CommandData>();
+      auto data_ap = llvm::make_unique<CommandDataPython>();
       if (!data_ap)
         break;
       data_ap->user_source.SplitIntoLines(data);
@@ -1231,10 +1231,26 @@ void ScriptInterpreterPython::SetBreakpointCommandCallbackFunction(
       bp_options, oneliner.c_str());
 }
 
+Error ScriptInterpreterPython::SetBreakpointCommandCallback(
+    BreakpointOptions *bp_options,
+    std::unique_ptr<BreakpointOptions::CommandData> &cmd_data_up) {
+  Error error;
+  error = GenerateBreakpointCommandCallbackData(cmd_data_up->user_source,
+                                                cmd_data_up->script_source);
+  if (error.Fail()) {
+    return error;
+  }
+  auto baton_sp =
+      std::make_shared<BreakpointOptions::CommandBaton>(std::move(cmd_data_up));
+  bp_options->SetCallback(ScriptInterpreterPython::BreakpointCallbackFunction,
+                          baton_sp);
+  return error;
+}
+
 // Set a Python one-liner as the callback for the breakpoint.
 Error ScriptInterpreterPython::SetBreakpointCommandCallback(
     BreakpointOptions *bp_options, const char *command_body_text) {
-  auto data_ap = llvm::make_unique<BreakpointOptions::CommandData>();
+  auto data_ap = llvm::make_unique<CommandDataPython>();
 
   // Split the command_body_text into lines, and pass that to
   // GenerateBreakpointCommandCallbackData.  That will
@@ -2054,8 +2070,7 @@ void ScriptInterpreterPython::Clear() {
 bool ScriptInterpreterPython::BreakpointCallbackFunction(
     void *baton, StoppointCallbackContext *context, user_id_t break_id,
     user_id_t break_loc_id) {
-  BreakpointOptions::CommandData *bp_option_data =
+  CommandDataPython *bp_option_data = (CommandDataPython *)baton;
-      (BreakpointOptions::CommandData *)baton;
   const char *python_function_name = bp_option_data->script_source.c_str();
 
   if (!context)

lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h

@@ -25,6 +25,7 @@
 // Other libraries and framework includes
 // Project includes
 #include "PythonDataObjects.h"
+#include "lldb/Breakpoint/BreakpointOptions.h"
 #include "lldb/Core/IOHandler.h"
 #include "lldb/Host/Terminal.h"
 #include "lldb/Interpreter/ScriptInterpreter.h"
@@ -37,6 +38,13 @@ namespace lldb_private {
 class ScriptInterpreterPython : public ScriptInterpreter,
                                 public IOHandlerDelegateMultiline {
 public:
+  class CommandDataPython : public BreakpointOptions::CommandData {
+  public:
+    CommandDataPython() : BreakpointOptions::CommandData() {
+      interpreter = lldb::eScriptLanguagePython;
+    }
+  };
+
 #if PY_MAJOR_VERSION >= 3
   typedef PyObject *(*SWIGInitCallback)(void);
 #else
@@ -362,6 +370,11 @@ public:
   void SetBreakpointCommandCallbackFunction(BreakpointOptions *bp_options,
                                             const char *function_name) override;
 
+  /// This one is for deserialization:
+  Error SetBreakpointCommandCallback(
+      BreakpointOptions *bp_options,
+      std::unique_ptr<BreakpointOptions::CommandData> &data_up) override;
+
   /// Set a one-liner as the callback for the watchpoint.
   void SetWatchpointCommandCallback(WatchpointOptions *wp_options,
                                     const char *oneliner) override;