[lldb/Plugins] Serialize ProcessLaunchInfo ScriptedProcess Dictionary

[mib]

Summary

This patch replaces the type of the ProcessLaunchInfo ScriptedProcess
Dictionary from a StructuredData pointer to be serialized in a string.

The reason behind this change is that the lifetime of the dictionary's
pointer could be managed by an actor other than the debugger (i.e. the
Python Interpreter). In some cases, this caused the dictionary's pointer
to point to corrupted memory, which can interfere with the good functionning
of Scripted Processes and Scripted Threads.

This patch also updates the related-code to take these changes into account.

Signed-off-by: Med Ismail Bennani <[e-mail]>

[JDevlieghere]

I'd like to understand what the actual problem is and whether it can be avoided before we use a workaround like this. We have different places where we use structured data to communicate between Python and LLDB. What's special about this use case? If we really do need the workaround it would be good to keep that information somewhere so that the next person doesn't have to go through the same process.

[mib]

Dropping this in favour of D112107

lldb/include/lldb/Host/ProcessLaunchInfo.h

public:
   std::string GetScriptedProcessClassName() const {
     return m_scripted_process_class_name;
   }
 
   void SetScriptedProcessClassName(std::string name) {
     m_scripted_process_class_name = name;
   }
 
-  lldb_private::StructuredData::DictionarySP
-  GetScriptedProcessDictionarySP() const {
-    return m_scripted_process_dictionary_sp;
+  std::string GetScriptedProcessDictionary() const {
+    return m_scripted_process_dict;
   }
 
-  void SetScriptedProcessDictionarySP(
-      lldb_private::StructuredData::DictionarySP dictionary_sp) {
-    m_scripted_process_dictionary_sp = dictionary_sp;
+  void SetScriptedProcessDictionary(std::string dict) {
+    m_scripted_process_dict = dict;
   }
 
 protected:
   FileSpec m_working_dir;
   std::string m_plugin_name;
   FileSpec m_shell;
   Flags m_flags; // Bitwise OR of bits from lldb::LaunchFlags
   std::vector<FileAction> m_file_actions; // File actions for any other files
   std::shared_ptr<PseudoTerminal> m_pty;
   uint32_t m_resume_count = 0; // How many times do we resume after launching
   Host::MonitorChildProcessCallback m_monitor_callback;
   void *m_monitor_callback_baton = nullptr;
   bool m_monitor_signals = false;
   std::string m_event_data; // A string passed to the plugin launch, having no
                             // meaning to the upper levels of lldb.
   lldb::ListenerSP m_listener_sp;
   lldb::ListenerSP m_hijack_listener_sp;
   std::string m_scripted_process_class_name; // The name of the class that will
                                              // manage a scripted process.
-  StructuredData::DictionarySP
-      m_scripted_process_dictionary_sp; // A dictionary that holds key/value
-                                        // pairs passed to the scripted process.
+  std::string
+      m_scripted_process_dict; // A serialized dictionary that holds key/value
 };
 }
 
 #endif // LLDB_HOST_PROCESSLAUNCHINFO_H

lldb/source/API/SBLaunchInfo.cpp

LLDB_RECORD_METHOD(void, SBLaunchInfo, SetScriptedProcessClassName,
                      (const char *), class_name);
 
   m_opaque_sp->SetScriptedProcessClassName(class_name);
 }
 
 lldb::SBStructuredData SBLaunchInfo::GetScriptedProcessDictionary() const {
   LLDB_RECORD_METHOD_CONST_NO_ARGS(lldb::SBStructuredData, SBLaunchInfo,
                                    GetScriptedProcessDictionary);
-
-  lldb_private::StructuredData::DictionarySP dict_sp =
-      m_opaque_sp->GetScriptedProcessDictionarySP();
-
   SBStructuredData data;
-  data.m_impl_up->SetObjectSP(dict_sp);
+  data.m_impl_up->SetObjectSP(
+      StructuredData::ParseJSON(m_opaque_sp->GetScriptedProcessDictionary()));
 
   return LLDB_RECORD_RESULT(data);
 }
 
 void SBLaunchInfo::SetScriptedProcessDictionary(lldb::SBStructuredData dict) {
   LLDB_RECORD_METHOD(void, SBLaunchInfo, SetScriptedProcessDictionary,
                      (lldb::SBStructuredData), dict);
 
   SBStream stream;
   SBError error = dict.GetAsJSON(stream);
 
   if (error.Fail())
     return;
 
-  StructuredData::DictionarySP dict_sp;
-  llvm::json::OStream s(stream.ref().AsRawOstream());
-  dict_sp->Serialize(s);
-
-  m_opaque_sp->SetScriptedProcessDictionarySP(dict_sp);
+  m_opaque_sp->SetScriptedProcessDictionary(stream.GetData());
 }
 
 namespace lldb_private {
 namespace repro {
 
 template <>
 void RegisterMethods<SBLaunchInfo>(Registry &R) {
   LLDB_REGISTER_CONSTRUCTOR(SBLaunchInfo, (const char **));

lldb/source/Commands/CommandObjectProcess.cpp

if (m_options.disable_aslr != eLazyBoolCalculate) {
       // back to the target.disable-aslr setting.
       disable_aslr = target->GetDisableASLR();
     }
 
     if (!m_class_options.GetName().empty()) {
       m_options.launch_info.SetProcessPluginName("ScriptedProcess");
       m_options.launch_info.SetScriptedProcessClassName(
           m_class_options.GetName());
-      m_options.launch_info.SetScriptedProcessDictionarySP(
-          m_class_options.GetStructuredData());
+      const StructuredData::DictionarySP dict_sp =
+          m_class_options.GetStructuredData();
+
+      if (dict_sp) {
+        StreamString stream;
+        llvm::json::OStream s(stream.AsRawOstream());
+        dict_sp->Serialize(s);
+        m_options.launch_info.SetScriptedProcessDictionary(stream.GetData());
+      }
+
       target->SetProcessLaunchInfo(m_options.launch_info);
     }
 
     if (disable_aslr)
       m_options.launch_info.GetFlags().Set(eLaunchFlagDisableASLR);
     else
       m_options.launch_info.GetFlags().Clear(eLaunchFlagDisableASLR);
 

lldb/source/Host/common/ProcessLaunchInfo.cpp

 using namespace lldb_private;
 
 // ProcessLaunchInfo member functions
 
 ProcessLaunchInfo::ProcessLaunchInfo()
     : ProcessInfo(), m_working_dir(), m_plugin_name(), m_flags(0),
       m_file_actions(), m_pty(new PseudoTerminal), m_monitor_callback(nullptr),
       m_listener_sp(), m_hijack_listener_sp(), m_scripted_process_class_name(),
-      m_scripted_process_dictionary_sp() {}
+      m_scripted_process_dict() {}
 
 ProcessLaunchInfo::ProcessLaunchInfo(const FileSpec &stdin_file_spec,
                                      const FileSpec &stdout_file_spec,
                                      const FileSpec &stderr_file_spec,
                                      const FileSpec &working_directory,
                                      uint32_t launch_flags)
     : ProcessInfo(), m_working_dir(), m_plugin_name(), m_flags(launch_flags),
       m_file_actions(), m_pty(new PseudoTerminal), m_resume_count(0),
       m_monitor_callback(nullptr), m_monitor_callback_baton(nullptr),
       m_monitor_signals(false), m_listener_sp(), m_hijack_listener_sp(),
-      m_scripted_process_class_name(), m_scripted_process_dictionary_sp() {
+      m_scripted_process_class_name(), m_scripted_process_dict() {
   if (stdin_file_spec) {
     FileAction file_action;
     const bool read = true;
     const bool write = false;
     if (file_action.Open(STDIN_FILENO, stdin_file_spec, read, write))
       AppendFileAction(file_action);
   }
   if (stdout_file_spec) {
void ProcessLaunchInfo::Clear() {
   m_plugin_name.clear();
   m_shell.Clear();
   m_flags.Clear();
   m_file_actions.clear();
   m_resume_count = 0;
   m_listener_sp.reset();
   m_hijack_listener_sp.reset();
   m_scripted_process_class_name.clear();
-  m_scripted_process_dictionary_sp.reset();
+  m_scripted_process_dict.clear();
 }
 
 void ProcessLaunchInfo::SetMonitorProcessCallback(
     const Host::MonitorChildProcessCallback &callback, bool monitor_signals) {
   m_monitor_callback = callback;
   m_monitor_signals = monitor_signals;
 }
 

lldb/source/Plugins/Process/scripted/ScriptedProcess.h

 namespace lldb_private {
 
 class ScriptedProcess : public Process {
 protected:
   class ScriptedProcessInfo {
   public:
     ScriptedProcessInfo(const ProcessLaunchInfo &launch_info) {
       m_class_name = launch_info.GetScriptedProcessClassName();
+      m_args_sp =
+          StructuredData::ParseJSON(launch_info.GetScriptedProcessDictionary());
     }
 
     std::string GetClassName() const { return m_class_name; }
     StructuredData::ObjectSP GetArgsSP() const { return m_args_sp; }
 
   private:
     std::string m_class_name;
     StructuredData::ObjectSP m_args_sp = nullptr;

lldb/source/Target/Target.cpp

Status Target::Launch(ProcessLaunchInfo &launch_info, Stream *stream) {
   if (launch_info.IsScriptedProcess()) {
     // Only copy scripted process launch options.
     ProcessLaunchInfo &default_launch_info = const_cast<ProcessLaunchInfo &>(
         GetGlobalProperties().GetProcessLaunchInfo());
 
     default_launch_info.SetProcessPluginName("ScriptedProcess");
     default_launch_info.SetScriptedProcessClassName(
         launch_info.GetScriptedProcessClassName());
-    default_launch_info.SetScriptedProcessDictionarySP(
-        launch_info.GetScriptedProcessDictionarySP());
+    default_launch_info.SetScriptedProcessDictionary(
+        launch_info.GetScriptedProcessDictionary());
 
     SetProcessLaunchInfo(launch_info);
   }
 
   // Get the value of synchronous execution here.  If you wait till after you
   // have started to run, then you could have hit a breakpoint, whose command
   // might switch the value, and then you'll pick up that incorrect value.
   Debugger &debugger = GetDebugger();