[lldb] Clarify StructuredDataImpl ownership

StructuredDataImpl ownership semantics is unclear at best. Various
structures were holding a non-owning pointer to it, with a comment that
the object is owned somewhere else. From what I was able to gather that
"somewhere else" was the SBStructuredData object, but I am not sure that
all created object eventually made its way there. (It wouldn't matter
even if they did, as we are leaking most of our SBStructuredData
objects.)

Since StructuredDataImpl is just a collection of two (shared) pointers,
there's really no point in elaborate lifetime management, so this patch
replaces all StructuredDataImpl pointers with actual objects or
unique_ptrs to it. This makes it much easier to resolve SBStructuredData
leaks in a follow-up patch.

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

lldb/bindings/lua/lua-wrapper.swig

@@ -11,23 +11,21 @@ lldb_private::LLDBSwigLuaBreakpointCallbackFunction
    lua_State *L,
    lldb::StackFrameSP stop_frame_sp,
    lldb::BreakpointLocationSP bp_loc_sp,
-   StructuredDataImpl *extra_args_impl
+   const StructuredDataImpl &extra_args_impl
 )
 {
    lldb::SBFrame sb_frame(stop_frame_sp);
    lldb::SBBreakpointLocation sb_bp_loc(bp_loc_sp);
    int nargs = 2;
 
-   llvm::Optional<lldb::SBStructuredData> extra_args;
+   lldb::SBStructuredData extra_args(extra_args_impl);
-   if (extra_args_impl)
-      extra_args = lldb::SBStructuredData(extra_args_impl);
 
    // Push the Lua wrappers
    PushSBClass(L, &sb_frame);
    PushSBClass(L, &sb_bp_loc);
 
-   if (extra_args.hasValue()) {
+   if (extra_args.IsValid()) {
-      PushSBClass(L, extra_args.getPointer());
+      PushSBClass(L, &extra_args);
       nargs++;
    }
 

lldb/bindings/python/python-wrapper.swig

@@ -29,7 +29,7 @@ lldb_private::LLDBSwigPythonBreakpointCallbackFunction
     const char *session_dictionary_name,
     const lldb::StackFrameSP& frame_sp,
     const lldb::BreakpointLocationSP& bp_loc_sp,
-    lldb_private::StructuredDataImpl *args_impl
+    const lldb_private::StructuredDataImpl &args_impl
 )
 {
     using namespace llvm;
@@ -254,7 +254,7 @@ lldb_private::LLDBSwigPythonCreateScriptedProcess
     const char *python_class_name,
     const char *session_dictionary_name,
     const lldb::TargetSP& target_sp,
-    lldb_private::StructuredDataImpl *args_impl,
+    const lldb_private::StructuredDataImpl &args_impl,
     std::string &error_string
 )
 {
@@ -290,7 +290,9 @@ lldb_private::LLDBSwigPythonCreateScriptedProcess
     PythonObject result = {};
     if (arg_info.get().max_positional_args == 2) {
         // FIXME: SBStructuredData leaked here
-        PythonObject args_arg(PyRefType::Owned, SBTypeToSWIGWrapper(*new lldb::SBStructuredData(args_impl)));
+        PythonObject args_arg(
+            PyRefType::Owned,
+            SBTypeToSWIGWrapper(*new lldb::SBStructuredData(args_impl)));
         result = pfunc(target_arg, args_arg);
     } else {
         error_string.assign("wrong number of arguments in __init__, should be 2 (not including self)");
@@ -308,7 +310,7 @@ lldb_private::LLDBSwigPythonCreateScriptedThread
     const char *python_class_name,
     const char *session_dictionary_name,
     const lldb::ProcessSP& process_sp,
-    lldb_private::StructuredDataImpl *args_impl,
+    const StructuredDataImpl &args_impl,
     std::string &error_string
 )
 {
@@ -342,7 +344,9 @@ lldb_private::LLDBSwigPythonCreateScriptedThread
     PythonObject result = {};
     if (arg_info.get().max_positional_args == 2) {
         // FIXME: SBStructuredData leaked here
-        PythonObject args_arg(PyRefType::Owned, SBTypeToSWIGWrapper(*new lldb::SBStructuredData(args_impl)));
+        PythonObject args_arg(
+            PyRefType::Owned,
+            SBTypeToSWIGWrapper(*new lldb::SBStructuredData(args_impl)));
         result = pfunc(ToSWIGWrapper(process_sp), args_arg);
     } else {
         error_string.assign("wrong number of arguments in __init__, should be 2 (not including self)");
@@ -359,7 +363,7 @@ lldb_private::LLDBSwigPythonCreateScriptedThreadPlan
 (
     const char *python_class_name,
     const char *session_dictionary_name,
-    lldb_private::StructuredDataImpl *args_impl,
+    const lldb_private::StructuredDataImpl &args_impl,
     std::string &error_string,
     const lldb::ThreadPlanSP& thread_plan_sp
 )
@@ -395,15 +399,16 @@ lldb_private::LLDBSwigPythonCreateScriptedThreadPlan
     }
 
     PythonObject result = {};
+    // FIXME: SBStructuredData leaked here
+    auto *args_sb = new lldb::SBStructuredData(args_impl);
     if (arg_info.get().max_positional_args == 2) {
-        if (args_impl != nullptr) {
+        if (args_sb->IsValid()) {
            error_string.assign("args passed, but __init__ does not take an args dictionary");
            Py_RETURN_NONE;
         }
         result = pfunc(tp_arg, dict);
     } else if (arg_info.get().max_positional_args >= 3) {
-        // FIXME: SBStructuredData leaked here
+        PythonObject args_arg(PyRefType::Owned, SBTypeToSWIGWrapper(*args_sb));
-        PythonObject args_arg(PyRefType::Owned, SBTypeToSWIGWrapper(*new lldb::SBStructuredData(args_impl)));
         result = pfunc(tp_arg, args_arg, dict);
     } else {
         error_string.assign("wrong number of arguments in __init__, should be 2 or 3 (not including self)");
@@ -467,7 +472,7 @@ lldb_private::LLDBSWIGPythonCallThreadPlan
 
 void *lldb_private::LLDBSwigPythonCreateScriptedBreakpointResolver(
     const char *python_class_name, const char *session_dictionary_name,
-    lldb_private::StructuredDataImpl *args_impl,
+    const StructuredDataImpl &args_impl,
     const lldb::BreakpointSP &breakpoint_sp) {
 
     if (python_class_name == NULL || python_class_name[0] == '\0' || !session_dictionary_name)
@@ -558,7 +563,7 @@ lldb_private::LLDBSwigPythonCreateScriptedStopHook
     lldb::TargetSP target_sp,
     const char *python_class_name,
     const char *session_dictionary_name,
-    lldb_private::StructuredDataImpl *args_impl,
+    const StructuredDataImpl &args_impl,
     Status &error
 )
 {

lldb/include/lldb/API/SBStructuredData.h

@@ -22,7 +22,7 @@ public:
 
   SBStructuredData(const lldb::EventSP &event_sp);
 
-  SBStructuredData(lldb_private::StructuredDataImpl *impl);
+  SBStructuredData(const lldb_private::StructuredDataImpl &impl);
 
   ~SBStructuredData();
 

lldb/include/lldb/Breakpoint/BreakpointResolverScripted.h

@@ -9,10 +9,10 @@
 #ifndef LLDB_BREAKPOINT_BREAKPOINTRESOLVERSCRIPTED_H
 #define LLDB_BREAKPOINT_BREAKPOINTRESOLVERSCRIPTED_H
 
-#include "lldb/lldb-forward.h"
 #include "lldb/Breakpoint/BreakpointResolver.h"
 #include "lldb/Core/ModuleSpec.h"
-
+#include "lldb/Core/StructuredDataImpl.h"
+#include "lldb/lldb-forward.h"
 
 namespace lldb_private {
 
@@ -26,7 +26,7 @@ public:
   BreakpointResolverScripted(const lldb::BreakpointSP &bkpt,
                              const llvm::StringRef class_name,
                              lldb::SearchDepth depth,
-                             StructuredDataImpl *args_data);
+                             const StructuredDataImpl &args_data);
 
   ~BreakpointResolverScripted() override = default;
 
@@ -64,10 +64,7 @@ private:
   
   std::string m_class_name;
   lldb::SearchDepth m_depth;
-  StructuredDataImpl *m_args_ptr; // We own this, but the implementation
+  StructuredDataImpl m_args;
-                                  // has to manage the UP (since that is
-                                  // how it gets stored in the
-                                  // SBStructuredData).
   StructuredData::GenericSP m_implementation_sp;
 
   BreakpointResolverScripted(const BreakpointResolverScripted &) = delete;

lldb/include/lldb/Core/StructuredDataImpl.h

@@ -29,6 +29,9 @@ public:
 
   StructuredDataImpl(const StructuredDataImpl &rhs) = default;
 
+  StructuredDataImpl(StructuredData::ObjectSP obj)
+      : m_data_sp(std::move(obj)) {}
+
   StructuredDataImpl(const lldb::EventSP &event_sp)
       : m_plugin_wp(
             EventDataStructuredData::GetPluginFromEvent(event_sp.get())),

lldb/include/lldb/Interpreter/ScriptInterpreter.h

@@ -274,7 +274,7 @@ public:
 
   virtual StructuredData::ObjectSP
   CreateScriptedThreadPlan(const char *class_name,
-                           StructuredDataImpl *args_data,
+                           const StructuredDataImpl &args_data,
                            std::string &error_str,
                            lldb::ThreadPlanSP thread_plan_sp) {
     return StructuredData::ObjectSP();
@@ -310,7 +310,7 @@ public:
 
   virtual StructuredData::GenericSP
   CreateScriptedBreakpointResolver(const char *class_name,
-                                   StructuredDataImpl *args_data,
+                                   const StructuredDataImpl &args_data,
                                    lldb::BreakpointSP &bkpt_sp) {
     return StructuredData::GenericSP();
   }
@@ -330,7 +330,7 @@ public:
 
   virtual StructuredData::GenericSP
   CreateScriptedStopHook(lldb::TargetSP target_sp, const char *class_name,
-                         StructuredDataImpl *args_data, Status &error) {
+                         const StructuredDataImpl &args_data, Status &error) {
     error.SetErrorString("Creating scripted stop-hooks with the current "
                          "script interpreter is not supported.");
     return StructuredData::GenericSP();

lldb/include/lldb/Target/Target.h

@@ -21,6 +21,7 @@
 #include "lldb/Core/Architecture.h"
 #include "lldb/Core/Disassembler.h"
 #include "lldb/Core/ModuleList.h"
+#include "lldb/Core/StructuredDataImpl.h"
 #include "lldb/Core/UserSettingsController.h"
 #include "lldb/Expression/Expression.h"
 #include "lldb/Host/ProcessLaunchInfo.h"
@@ -1309,8 +1310,7 @@ public:
     std::string m_class_name;
     /// This holds the dictionary of keys & values that can be used to
     /// parametrize any given callback's behavior.
-    StructuredDataImpl *m_extra_args; // We own this structured data,
+    StructuredDataImpl m_extra_args;
-                                      // but the SD itself manages the UP.
     /// This holds the python callback object.
     StructuredData::GenericSP m_implementation_sp;
 

lldb/include/lldb/Target/ThreadPlanPython.h

@@ -12,8 +12,7 @@
 
 #include <string>
 
-#include "lldb/lldb-forward.h"
+#include "lldb/Core/StructuredDataImpl.h"
-
 #include "lldb/Target/Process.h"
 #include "lldb/Target/StopInfo.h"
 #include "lldb/Target/Target.h"
@@ -22,6 +21,7 @@
 #include "lldb/Target/ThreadPlanTracer.h"
 #include "lldb/Utility/StructuredData.h"
 #include "lldb/Utility/UserID.h"
+#include "lldb/lldb-forward.h"
 #include "lldb/lldb-private.h"
 
 namespace lldb_private {
@@ -32,8 +32,7 @@ namespace lldb_private {
 class ThreadPlanPython : public ThreadPlan {
 public:
   ThreadPlanPython(Thread &thread, const char *class_name,
-                   StructuredDataImpl *args_data);
+                   const StructuredDataImpl &args_data);
-  ~ThreadPlanPython() override;
 
   void GetDescription(Stream *s, lldb::DescriptionLevel level) override;
 
@@ -62,10 +61,7 @@ protected:
 
 private:
   std::string m_class_name;
-  StructuredDataImpl *m_args_data; // We own this, but the implementation
+  StructuredDataImpl m_args_data;
-                                   // has to manage the UP (since that is
-                                   // how it gets stored in the
-                                   // SBStructuredData).
   std::string m_error_str;
   StructuredData::ObjectSP m_implementation_sp;
   bool m_did_push;

lldb/source/API/SBStructuredData.cpp

@@ -39,10 +39,10 @@ SBStructuredData::SBStructuredData(const lldb::EventSP &event_sp)
   LLDB_RECORD_CONSTRUCTOR(SBStructuredData, (const lldb::EventSP &), event_sp);
 }
 
-SBStructuredData::SBStructuredData(lldb_private::StructuredDataImpl *impl)
+SBStructuredData::SBStructuredData(const lldb_private::StructuredDataImpl &impl)
-    : m_impl_up(impl ? impl : new StructuredDataImpl()) {
+    : m_impl_up(new StructuredDataImpl(impl)) {
   LLDB_RECORD_CONSTRUCTOR(SBStructuredData,
-                          (lldb_private::StructuredDataImpl *), impl);
+                          (const lldb_private::StructuredDataImpl &), impl);
 }
 
 SBStructuredData::~SBStructuredData() = default;
@@ -210,7 +210,7 @@ template <> void RegisterMethods<SBStructuredData>(Registry &R) {
   LLDB_REGISTER_CONSTRUCTOR(SBStructuredData, (const lldb::SBStructuredData &));
   LLDB_REGISTER_CONSTRUCTOR(SBStructuredData, (const lldb::EventSP &));
   LLDB_REGISTER_CONSTRUCTOR(SBStructuredData,
-                            (lldb_private::StructuredDataImpl *));
+                            (const lldb_private::StructuredDataImpl &));
   LLDB_REGISTER_METHOD(
       lldb::SBStructuredData &,
       SBStructuredData, operator=,(const lldb::SBStructuredData &));

lldb/source/API/SBThreadPlan.cpp

@@ -69,8 +69,8 @@ SBThreadPlan::SBThreadPlan(lldb::SBThread &sb_thread, const char *class_name) {
 
   Thread *thread = sb_thread.get();
   if (thread)
-    m_opaque_wp =
+    m_opaque_wp = std::make_shared<ThreadPlanPython>(*thread, class_name,
-        std::make_shared<ThreadPlanPython>(*thread, class_name, nullptr);
+                                                     StructuredDataImpl());
 }
 
 SBThreadPlan::SBThreadPlan(lldb::SBThread &sb_thread, const char *class_name,
@@ -82,7 +82,7 @@ SBThreadPlan::SBThreadPlan(lldb::SBThread &sb_thread, const char *class_name,
   Thread *thread = sb_thread.get();
   if (thread)
     m_opaque_wp = std::make_shared<ThreadPlanPython>(*thread, class_name,
-                                                     args_data.m_impl_up.get());
+                                                     *args_data.m_impl_up);
 }
 
 // Assignment operator

lldb/source/Breakpoint/BreakpointResolverScripted.cpp

@@ -27,10 +27,9 @@ using namespace lldb_private;
 // BreakpointResolverScripted:
 BreakpointResolverScripted::BreakpointResolverScripted(
     const BreakpointSP &bkpt, const llvm::StringRef class_name,
-    lldb::SearchDepth depth, StructuredDataImpl *args_data)
+    lldb::SearchDepth depth, const StructuredDataImpl &args_data)
     : BreakpointResolver(bkpt, BreakpointResolver::PythonResolver),
-      m_class_name(std::string(class_name)), m_depth(depth),
+      m_class_name(std::string(class_name)), m_depth(depth), m_args(args_data) {
-      m_args_ptr(args_data) {
   CreateImplementationIfNeeded(bkpt);
 }
 
@@ -52,7 +51,7 @@ void BreakpointResolverScripted::CreateImplementationIfNeeded(
     return;
 
   m_implementation_sp = script_interp->CreateScriptedBreakpointResolver(
-      m_class_name.c_str(), m_args_ptr, breakpoint_sp);
+      m_class_name.c_str(), m_args, breakpoint_sp);
 }
 
 void BreakpointResolverScripted::NotifyBreakpointSet() {
@@ -76,13 +75,11 @@ BreakpointResolverScripted::CreateFromStructuredData(
   // placeholder.
   lldb::SearchDepth depth = lldb::eSearchDepthTarget;
 
-  StructuredDataImpl *args_data_impl = new StructuredDataImpl();
+  StructuredDataImpl args_data_impl;
   StructuredData::Dictionary *args_dict = nullptr;
-  success = options_dict.GetValueForKeyAsDictionary(
+  if (options_dict.GetValueForKeyAsDictionary(GetKey(OptionNames::ScriptArgs),
-    GetKey(OptionNames::ScriptArgs), args_dict);
+                                              args_dict))
-  if (success) {
+    args_data_impl.SetObjectSP(args_dict->shared_from_this());
-      args_data_impl->SetObjectSP(args_dict->shared_from_this());
-  }
   return new BreakpointResolverScripted(bkpt, class_name, depth, 
                                         args_data_impl);
 }
@@ -94,9 +91,9 @@ BreakpointResolverScripted::SerializeToStructuredData() {
 
   options_dict_sp->AddStringItem(GetKey(OptionNames::PythonClassName),
                                    m_class_name);
-  if (m_args_ptr->IsValid())
+  if (m_args.IsValid())
     options_dict_sp->AddItem(GetKey(OptionNames::ScriptArgs),
-          m_args_ptr->GetObjectSP());
+                             m_args.GetObjectSP());
 
   return WrapOptionsDict(options_dict_sp);
 }
@@ -151,10 +148,6 @@ void BreakpointResolverScripted::Dump(Stream *s) const {}
 
 lldb::BreakpointResolverSP
 BreakpointResolverScripted::CopyForBreakpoint(BreakpointSP &breakpoint) {
-  // FIXME: Have to make a copy of the arguments from the m_args_ptr and then
+  return std::make_shared<BreakpointResolverScripted>(breakpoint, m_class_name,
-  // pass that to the new resolver.
+                                                      m_depth, m_args);
-  lldb::BreakpointResolverSP ret_sp(
-      new BreakpointResolverScripted(breakpoint, m_class_name, m_depth, 
-                                     nullptr));
-  return ret_sp;
 }

lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp

@@ -82,13 +82,7 @@ Lua::CallBreakpointCallback(void *baton, lldb::StackFrameSP stop_frame_sp,
 
   lua_pushlightuserdata(m_lua_state, baton);
   lua_gettable(m_lua_state, LUA_REGISTRYINDEX);
-  auto *extra_args_impl = [&]() -> StructuredDataImpl * {
+  StructuredDataImpl extra_args_impl(std::move(extra_args_sp));
-    if (extra_args_sp == nullptr)
-      return nullptr;
-    auto *extra_args_impl = new StructuredDataImpl();
-    extra_args_impl->SetObjectSP(extra_args_sp);
-    return extra_args_impl;
-  }();
   return LLDBSwigLuaBreakpointCallbackFunction(m_lua_state, stop_frame_sp,
                                                bp_loc_sp, extra_args_impl);
 }

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

@@ -17,7 +17,8 @@ namespace lldb_private {
 
 llvm::Expected<bool> LLDBSwigLuaBreakpointCallbackFunction(
     lua_State *L, lldb::StackFrameSP stop_frame_sp,
-    lldb::BreakpointLocationSP bp_loc_sp, StructuredDataImpl *extra_args_impl);
+    lldb::BreakpointLocationSP bp_loc_sp,
+    const StructuredDataImpl &extra_args_impl);
 
 llvm::Expected<bool> LLDBSwigLuaWatchpointCallbackFunction(
     lua_State *L, lldb::StackFrameSP stop_frame_sp, lldb::WatchpointSP wp_sp);

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

@@ -57,20 +57,20 @@ void *LLDBSWIGPython_CastPyObjectToSBMemoryRegionInfo(PyObject *data);
 void *LLDBSwigPythonCreateScriptedProcess(const char *python_class_name,
                                           const char *session_dictionary_name,
                                           const lldb::TargetSP &target_sp,
-                                          StructuredDataImpl *args_impl,
+                                          const StructuredDataImpl &args_impl,
                                           std::string &error_string);
 
 void *LLDBSwigPythonCreateScriptedThread(const char *python_class_name,
                                          const char *session_dictionary_name,
                                          const lldb::ProcessSP &process_sp,
-                                         StructuredDataImpl *args_impl,
+                                         const StructuredDataImpl &args_impl,
                                          std::string &error_string);
 
 llvm::Expected<bool> LLDBSwigPythonBreakpointCallbackFunction(
     const char *python_function_name, const char *session_dictionary_name,
     const lldb::StackFrameSP &sb_frame,
     const lldb::BreakpointLocationSP &sb_bp_loc,
-    lldb_private::StructuredDataImpl *args_impl);
+    const lldb_private::StructuredDataImpl &args_impl);
 
 bool LLDBSwigPythonWatchpointCallbackFunction(
     const char *python_function_name, const char *session_dictionary_name,
@@ -94,7 +94,7 @@ void *LLDBSwigPythonCreateCommandObject(const char *python_class_name,
 
 void *LLDBSwigPythonCreateScriptedThreadPlan(
     const char *python_class_name, const char *session_dictionary_name,
-    lldb_private::StructuredDataImpl *args_data, std::string &error_string,
+    const StructuredDataImpl &args_data, std::string &error_string,
     const lldb::ThreadPlanSP &thread_plan_sp);
 
 bool LLDBSWIGPythonCallThreadPlan(void *implementor, const char *method_name,
@@ -103,15 +103,16 @@ bool LLDBSWIGPythonCallThreadPlan(void *implementor, const char *method_name,
 
 void *LLDBSwigPythonCreateScriptedBreakpointResolver(
     const char *python_class_name, const char *session_dictionary_name,
-    lldb_private::StructuredDataImpl *args, const lldb::BreakpointSP &bkpt_sp);
+    const StructuredDataImpl &args, const lldb::BreakpointSP &bkpt_sp);
 
 unsigned int
 LLDBSwigPythonCallBreakpointResolver(void *implementor, const char *method_name,
                                      lldb_private::SymbolContext *sym_ctx);
 
-void *LLDBSwigPythonCreateScriptedStopHook(
+void *LLDBSwigPythonCreateScriptedStopHook(lldb::TargetSP target_sp,
-    lldb::TargetSP target_sp, const char *python_class_name,
+                                           const char *python_class_name,
-    const char *session_dictionary_name, lldb_private::StructuredDataImpl *args,
+                                           const char *session_dictionary_name,
+                                           const StructuredDataImpl &args,
                                            lldb_private::Status &error);
 
 bool LLDBSwigPythonStopHookCallHandleStop(void *implementor,

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

@@ -1718,7 +1718,7 @@ StructuredData::DictionarySP ScriptInterpreterPythonImpl::OSPlugin_CreateThread(
 }
 
 StructuredData::ObjectSP ScriptInterpreterPythonImpl::CreateScriptedThreadPlan(
-    const char *class_name, StructuredDataImpl *args_data,
+    const char *class_name, const StructuredDataImpl &args_data,
     std::string &error_str, lldb::ThreadPlanSP thread_plan_sp) {
   if (class_name == nullptr || class_name[0] == '\0')
     return StructuredData::ObjectSP();
@@ -1820,7 +1820,7 @@ lldb::StateType ScriptInterpreterPythonImpl::ScriptedThreadPlanGetRunState(
 
 StructuredData::GenericSP
 ScriptInterpreterPythonImpl::CreateScriptedBreakpointResolver(
-    const char *class_name, StructuredDataImpl *args_data,
+    const char *class_name, const StructuredDataImpl &args_data,
     lldb::BreakpointSP &bkpt_sp) {
 
   if (class_name == nullptr || class_name[0] == '\0')
@@ -1890,8 +1890,8 @@ ScriptInterpreterPythonImpl::ScriptedBreakpointResolverSearchDepth(
 }
 
 StructuredData::GenericSP ScriptInterpreterPythonImpl::CreateScriptedStopHook(
-    TargetSP target_sp, const char *class_name, StructuredDataImpl *args_data,
+    TargetSP target_sp, const char *class_name,
-    Status &error) {
+    const StructuredDataImpl &args_data, Status &error) {
 
   if (!target_sp) {
     error.SetErrorString("No target for scripted stop-hook.");
@@ -2197,7 +2197,7 @@ bool ScriptInterpreterPythonImpl::BreakpointCallbackFunction(
               LLDBSwigPythonBreakpointCallbackFunction(
                   python_function_name,
                   python_interpreter->m_dictionary_name.c_str(), stop_frame_sp,
-                  bp_loc_sp, bp_option_data->m_extra_args_up.get());
+                  bp_loc_sp, bp_option_data->m_extra_args);
 
           if (!maybe_ret_val) {
 

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

@@ -33,13 +33,12 @@ public:
     CommandDataPython() : BreakpointOptions::CommandData() {
       interpreter = lldb::eScriptLanguagePython;
     }
-    CommandDataPython(StructuredData::ObjectSP extra_args_sp) :
+    CommandDataPython(StructuredData::ObjectSP extra_args_sp)
-        BreakpointOptions::CommandData(),
+        : BreakpointOptions::CommandData(),
-        m_extra_args_up(new StructuredDataImpl()) {
+          m_extra_args(std::move(extra_args_sp)) {
       interpreter = lldb::eScriptLanguagePython;
-        m_extra_args_up->SetObjectSP(extra_args_sp);
     }
-    lldb::StructuredDataImplUP m_extra_args_up;
+    StructuredDataImpl m_extra_args;
   };
 
   ScriptInterpreterPython(Debugger &debugger)

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

@@ -79,7 +79,7 @@ public:
 
   StructuredData::ObjectSP
   CreateScriptedThreadPlan(const char *class_name,
-                           StructuredDataImpl *args_data,
+                           const StructuredDataImpl &args_data,
                            std::string &error_str,
                            lldb::ThreadPlanSP thread_plan) override;
 
@@ -99,7 +99,7 @@ public:
 
   StructuredData::GenericSP
   CreateScriptedBreakpointResolver(const char *class_name,
-                                   StructuredDataImpl *args_data,
+                                   const StructuredDataImpl &args_data,
                                    lldb::BreakpointSP &bkpt_sp) override;
   bool ScriptedBreakpointResolverSearchCallback(
       StructuredData::GenericSP implementor_sp,
@@ -110,7 +110,8 @@ public:
 
   StructuredData::GenericSP
   CreateScriptedStopHook(lldb::TargetSP target_sp, const char *class_name,
-                         StructuredDataImpl *args_data, Status &error) override;
+                         const StructuredDataImpl &args_data,
+                         Status &error) override;
 
   bool ScriptedStopHookHandleStop(StructuredData::GenericSP implementor_sp,
                                   ExecutionContext &exc_ctx,

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

@@ -37,11 +37,7 @@ StructuredData::GenericSP ScriptedProcessPythonInterface::CreatePluginObject(
     return {};
 
   TargetSP target_sp = exe_ctx.GetTargetSP();
-  StructuredDataImpl *args_impl = nullptr;
+  StructuredDataImpl args_impl(args_sp);
-  if (args_sp) {
-    args_impl = new StructuredDataImpl();
-    args_impl->SetObjectSP(args_sp);
-  }
   std::string error_string;
 
   Locker py_lock(&m_interpreter, Locker::AcquireLock | Locker::NoSTDIN,

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

@@ -37,11 +37,7 @@ StructuredData::GenericSP ScriptedThreadPythonInterface::CreatePluginObject(
     return {};
 
   ProcessSP process_sp = exe_ctx.GetProcessSP();
-  StructuredDataImpl *args_impl = nullptr;
+  StructuredDataImpl args_impl(args_sp);
-  if (args_sp) {
-    args_impl = new StructuredDataImpl();
-    args_impl->SetObjectSP(args_sp);
-  }
   std::string error_string;
 
   Locker py_lock(&m_interpreter, Locker::AcquireLock | Locker::NoSTDIN,

lldb/source/Target/Target.cpp

@@ -616,12 +616,8 @@ lldb::BreakpointSP Target::CreateScriptedBreakpoint(
         shared_from_this());
   }
 
-  StructuredDataImpl *extra_args_impl = new StructuredDataImpl();
-  if (extra_args_sp)
-    extra_args_impl->SetObjectSP(extra_args_sp);
-
   BreakpointResolverSP resolver_sp(new BreakpointResolverScripted(
-      nullptr, class_name, depth, extra_args_impl));
+      nullptr, class_name, depth, StructuredDataImpl(extra_args_sp)));
   return CreateBreakpoint(filter_sp, resolver_sp, internal, false, true);
 }
 
@@ -3484,11 +3480,7 @@ Status Target::StopHookScripted::SetScriptCallback(
   }
 
   m_class_name = class_name;
-
+  m_extra_args.SetObjectSP(extra_args_sp);
-  m_extra_args = new StructuredDataImpl();
-
-  if (extra_args_sp)
-    m_extra_args->SetObjectSP(extra_args_sp);
 
   m_implementation_sp = script_interp->CreateScriptedStopHook(
       GetTarget(), m_class_name.c_str(), m_extra_args, error);
@@ -3526,9 +3518,9 @@ void Target::StopHookScripted::GetSubclassDescription(
   // Now print the extra args:
   // FIXME: We should use StructuredData.GetDescription on the m_extra_args
   // but that seems to rely on some printing plugin that doesn't exist.
-  if (!m_extra_args->IsValid())
+  if (!m_extra_args.IsValid())
     return;
-  StructuredData::ObjectSP object_sp = m_extra_args->GetObjectSP();
+  StructuredData::ObjectSP object_sp = m_extra_args.GetObjectSP();
   if (!object_sp || !object_sp->IsValid())
     return;
 

lldb/source/Target/Thread.cpp

@@ -1371,14 +1371,8 @@ lldb::ThreadPlanSP Thread::QueueThreadPlanForStepScripted(
     StructuredData::ObjectSP extra_args_sp,  bool stop_other_threads,
     Status &status) {
 
-  StructuredDataImpl *extra_args_impl = nullptr; 
+  ThreadPlanSP thread_plan_sp(new ThreadPlanPython(
-  if (extra_args_sp) {
+      *this, class_name, StructuredDataImpl(extra_args_sp)));
-    extra_args_impl = new StructuredDataImpl();
-    extra_args_impl->SetObjectSP(extra_args_sp);
-  }
-
-  ThreadPlanSP thread_plan_sp(new ThreadPlanPython(*this, class_name, 
-                                                   extra_args_impl));
   thread_plan_sp->SetStopOthers(stop_other_threads);
   status = QueueThreadPlan(thread_plan_sp, abort_other_plans);
   return thread_plan_sp;

lldb/source/Target/ThreadPlanPython.cpp

@@ -26,7 +26,7 @@ using namespace lldb_private;
 // ThreadPlanPython
 
 ThreadPlanPython::ThreadPlanPython(Thread &thread, const char *class_name,
-                                   StructuredDataImpl *args_data)
+                                   const StructuredDataImpl &args_data)
     : ThreadPlan(ThreadPlan::eKindPython, "Python based Thread Plan", thread,
                  eVoteNoOpinion, eVoteNoOpinion),
       m_class_name(class_name), m_args_data(args_data), m_did_push(false),
@@ -36,11 +36,6 @@ ThreadPlanPython::ThreadPlanPython(Thread &thread, const char *class_name,
   SetPrivate(false);
 }
 
-ThreadPlanPython::~ThreadPlanPython() {
-  // FIXME, do I need to decrement the ref count on this implementation object
-  // to make it go away?
-}
-
 bool ThreadPlanPython::ValidatePlan(Stream *error) {
   if (!m_did_push)
     return true;

lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp

@@ -16,7 +16,8 @@ extern "C" int luaopen_lldb(lua_State *L) { return 0; }
 
 llvm::Expected<bool> lldb_private::LLDBSwigLuaBreakpointCallbackFunction(
     lua_State *L, lldb::StackFrameSP stop_frame_sp,
-    lldb::BreakpointLocationSP bp_loc_sp, StructuredDataImpl *extra_args_impl) {
+    lldb::BreakpointLocationSP bp_loc_sp,
+    const StructuredDataImpl &extra_args_impl) {
   return false;
 }
 

lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp

@@ -63,7 +63,7 @@ llvm::Expected<bool> lldb_private::LLDBSwigPythonBreakpointCallbackFunction(
     const char *python_function_name, const char *session_dictionary_name,
     const lldb::StackFrameSP &sb_frame,
     const lldb::BreakpointLocationSP &sb_bp_loc,
-    StructuredDataImpl *args_impl) {
+    const StructuredDataImpl &args_impl) {
   return false;
 }
 
@@ -94,7 +94,7 @@ void *lldb_private::LLDBSwigPythonCreateCommandObject(
 
 void *lldb_private::LLDBSwigPythonCreateScriptedThreadPlan(
     const char *python_class_name, const char *session_dictionary_name,
-    StructuredDataImpl *args_data, std::string &error_string,
+    const StructuredDataImpl &args_data, std::string &error_string,
     const lldb::ThreadPlanSP &thread_plan_sp) {
   return nullptr;
 }
@@ -108,7 +108,7 @@ bool lldb_private::LLDBSWIGPythonCallThreadPlan(void *implementor,
 
 void *lldb_private::LLDBSwigPythonCreateScriptedBreakpointResolver(
     const char *python_class_name, const char *session_dictionary_name,
-    lldb_private::StructuredDataImpl *args, const lldb::BreakpointSP &bkpt_sp) {
+    const StructuredDataImpl &args, const lldb::BreakpointSP &bkpt_sp) {
   return nullptr;
 }
 
@@ -200,14 +200,14 @@ lldb_private::LLDBSWIGPythonCreateOSPlugin(const char *python_class_name,
 
 void *lldb_private::LLDBSwigPythonCreateScriptedProcess(
     const char *python_class_name, const char *session_dictionary_name,
-    const lldb::TargetSP &target_sp, StructuredDataImpl *args_impl,
+    const lldb::TargetSP &target_sp, const StructuredDataImpl &args_impl,
     std::string &error_string) {
   return nullptr;
 }
 
 void *lldb_private::LLDBSwigPythonCreateScriptedThread(
     const char *python_class_name, const char *session_dictionary_name,
-    const lldb::ProcessSP &process_sp, StructuredDataImpl *args_impl,
+    const lldb::ProcessSP &process_sp, const StructuredDataImpl &args_impl,
     std::string &error_string) {
   return nullptr;
 }
@@ -259,8 +259,8 @@ void *lldb_private::LLDBSWIGPython_GetDynamicSetting(
 
 void *lldb_private::LLDBSwigPythonCreateScriptedStopHook(
     lldb::TargetSP target_sp, const char *python_class_name,
-    const char *session_dictionary_name,
+    const char *session_dictionary_name, const StructuredDataImpl &args_impl,
-    lldb_private::StructuredDataImpl *args_impl, Status &error) {
+    Status &error) {
   return nullptr;
 }