[Lldb-commits] [lldb] 82de8df - [lldb] Clarify StructuredDataImpl ownership

Pavel Labath via lldb-commits lldb-commits at lists.llvm.org
Mon Dec 13 12:06:05 PST 2021


Author: Pavel Labath
Date: 2021-12-13T21:04:51+01:00
New Revision: 82de8df26f15778793dc6b1526e14779f435f2e1

URL: https://github.com/llvm/llvm-project/commit/82de8df26f15778793dc6b1526e14779f435f2e1
DIFF: https://github.com/llvm/llvm-project/commit/82de8df26f15778793dc6b1526e14779f435f2e1.diff

LOG: [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

Added: 
    

Modified: 
    lldb/bindings/lua/lua-wrapper.swig
    lldb/bindings/python/python-wrapper.swig
    lldb/include/lldb/API/SBStructuredData.h
    lldb/include/lldb/Breakpoint/BreakpointResolverScripted.h
    lldb/include/lldb/Core/StructuredDataImpl.h
    lldb/include/lldb/Interpreter/ScriptInterpreter.h
    lldb/include/lldb/Target/Target.h
    lldb/include/lldb/Target/ThreadPlanPython.h
    lldb/source/API/SBStructuredData.cpp
    lldb/source/API/SBThreadPlan.cpp
    lldb/source/Breakpoint/BreakpointResolverScripted.cpp
    lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
    lldb/source/Plugins/ScriptInterpreter/Lua/SWIGLuaBridge.h
    lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
    lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
    lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
    lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
    lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
    lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp
    lldb/source/Target/Target.cpp
    lldb/source/Target/Thread.cpp
    lldb/source/Target/ThreadPlanPython.cpp
    lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
    lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/bindings/lua/lua-wrapper.swig b/lldb/bindings/lua/lua-wrapper.swig
index 4ca406137d6cf..9e4b8d1224bd2 100644
--- a/lldb/bindings/lua/lua-wrapper.swig
+++ b/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;
-   if (extra_args_impl)
-      extra_args = lldb::SBStructuredData(extra_args_impl);
+   lldb::SBStructuredData extra_args(extra_args_impl);
 
    // Push the Lua wrappers
    PushSBClass(L, &sb_frame);
    PushSBClass(L, &sb_bp_loc);
 
-   if (extra_args.hasValue()) {
-      PushSBClass(L, extra_args.getPointer());
+   if (extra_args.IsValid()) {
+      PushSBClass(L, &extra_args);
       nargs++;
    }
 

diff  --git a/lldb/bindings/python/python-wrapper.swig b/lldb/bindings/python/python-wrapper.swig
index 079f8d12dafab..da943f73c4351 100644
--- a/lldb/bindings/python/python-wrapper.swig
+++ b/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(*new lldb::SBStructuredData(args_impl)));
+        PythonObject args_arg(PyRefType::Owned, SBTypeToSWIGWrapper(*args_sb));
         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
 )
 {

diff  --git a/lldb/include/lldb/API/SBStructuredData.h b/lldb/include/lldb/API/SBStructuredData.h
index 07075abbf1d07..533dcc8fc07c3 100644
--- a/lldb/include/lldb/API/SBStructuredData.h
+++ b/lldb/include/lldb/API/SBStructuredData.h
@@ -22,7 +22,7 @@ class SBStructuredData {
 
   SBStructuredData(const lldb::EventSP &event_sp);
 
-  SBStructuredData(lldb_private::StructuredDataImpl *impl);
+  SBStructuredData(const lldb_private::StructuredDataImpl &impl);
 
   ~SBStructuredData();
 

diff  --git a/lldb/include/lldb/Breakpoint/BreakpointResolverScripted.h b/lldb/include/lldb/Breakpoint/BreakpointResolverScripted.h
index 26fd6f2f04d7a..cecd0f92a21a8 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointResolverScripted.h
+++ b/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 @@ class BreakpointResolverScripted : public BreakpointResolver {
   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 @@ class BreakpointResolverScripted : public BreakpointResolver {
   
   std::string m_class_name;
   lldb::SearchDepth m_depth;
-  StructuredDataImpl *m_args_ptr; // We own this, but the implementation
-                                  // has to manage the UP (since that is
-                                  // how it gets stored in the
-                                  // SBStructuredData).
+  StructuredDataImpl m_args;
   StructuredData::GenericSP m_implementation_sp;
 
   BreakpointResolverScripted(const BreakpointResolverScripted &) = delete;

diff  --git a/lldb/include/lldb/Core/StructuredDataImpl.h b/lldb/include/lldb/Core/StructuredDataImpl.h
index d6f64451e5c22..8930ebff81667 100644
--- a/lldb/include/lldb/Core/StructuredDataImpl.h
+++ b/lldb/include/lldb/Core/StructuredDataImpl.h
@@ -29,6 +29,9 @@ class StructuredDataImpl {
 
   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())),

diff  --git a/lldb/include/lldb/Interpreter/ScriptInterpreter.h b/lldb/include/lldb/Interpreter/ScriptInterpreter.h
index 2b96021fffc99..83f784bde712e 100644
--- a/lldb/include/lldb/Interpreter/ScriptInterpreter.h
+++ b/lldb/include/lldb/Interpreter/ScriptInterpreter.h
@@ -274,7 +274,7 @@ class ScriptInterpreter : public PluginInterface {
 
   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 @@ class ScriptInterpreter : public PluginInterface {
 
   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 @@ class ScriptInterpreter : public PluginInterface {
 
   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();

diff  --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h
index 7e8e1373a5068..b85839c15b2f9 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/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 @@ class Target : public std::enable_shared_from_this<Target>,
     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,
-                                      // but the SD itself manages the UP.
+    StructuredDataImpl m_extra_args;
     /// This holds the python callback object.
     StructuredData::GenericSP m_implementation_sp;
 

diff  --git a/lldb/include/lldb/Target/ThreadPlanPython.h b/lldb/include/lldb/Target/ThreadPlanPython.h
index 7b37b2b9ce5ab..f148f88d4c46d 100644
--- a/lldb/include/lldb/Target/ThreadPlanPython.h
+++ b/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 {
@@ -31,9 +31,8 @@ namespace lldb_private {
 
 class ThreadPlanPython : public ThreadPlan {
 public:
-  ThreadPlanPython(Thread &thread, const char *class_name, 
-                   StructuredDataImpl *args_data);
-  ~ThreadPlanPython() override;
+  ThreadPlanPython(Thread &thread, const char *class_name,
+                   const StructuredDataImpl &args_data);
 
   void GetDescription(Stream *s, lldb::DescriptionLevel level) override;
 
@@ -62,10 +61,7 @@ class ThreadPlanPython : public ThreadPlan {
 
 private:
   std::string m_class_name;
-  StructuredDataImpl *m_args_data; // We own this, but the implementation
-                                   // has to manage the UP (since that is
-                                   // how it gets stored in the
-                                   // SBStructuredData).
+  StructuredDataImpl m_args_data;
   std::string m_error_str;
   StructuredData::ObjectSP m_implementation_sp;
   bool m_did_push;

diff  --git a/lldb/source/API/SBStructuredData.cpp b/lldb/source/API/SBStructuredData.cpp
index 97a9eadcaf075..e99c6194c5164 100644
--- a/lldb/source/API/SBStructuredData.cpp
+++ b/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)
-    : m_impl_up(impl ? impl : new StructuredDataImpl()) {
+SBStructuredData::SBStructuredData(const lldb_private::StructuredDataImpl &impl)
+    : 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 &));

diff  --git a/lldb/source/API/SBThreadPlan.cpp b/lldb/source/API/SBThreadPlan.cpp
index 9af673b0f3a99..99ecb321595fb 100644
--- a/lldb/source/API/SBThreadPlan.cpp
+++ b/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 =
-        std::make_shared<ThreadPlanPython>(*thread, class_name, nullptr);
+    m_opaque_wp = std::make_shared<ThreadPlanPython>(*thread, class_name,
+                                                     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

diff  --git a/lldb/source/Breakpoint/BreakpointResolverScripted.cpp b/lldb/source/Breakpoint/BreakpointResolverScripted.cpp
index 92297fbc7c4b3..308c3b987f581 100644
--- a/lldb/source/Breakpoint/BreakpointResolverScripted.cpp
+++ b/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_args_ptr(args_data) {
+      m_class_name(std::string(class_name)), m_depth(depth), m_args(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() {
@@ -75,14 +74,12 @@ BreakpointResolverScripted::CreateFromStructuredData(
   // The Python function will actually provide the search depth, this is a
   // 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(
-    GetKey(OptionNames::ScriptArgs), args_dict);
-  if (success) {
-      args_data_impl->SetObjectSP(args_dict->shared_from_this());
-  }
+  if (options_dict.GetValueForKeyAsDictionary(GetKey(OptionNames::ScriptArgs),
+                                              args_dict))
+    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())
-      options_dict_sp->AddItem(GetKey(OptionNames::ScriptArgs),
-          m_args_ptr->GetObjectSP());
+  if (m_args.IsValid())
+    options_dict_sp->AddItem(GetKey(OptionNames::ScriptArgs),
+                             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
-  // pass that to the new resolver.
-  lldb::BreakpointResolverSP ret_sp(
-      new BreakpointResolverScripted(breakpoint, m_class_name, m_depth, 
-                                     nullptr));
-  return ret_sp;
+  return std::make_shared<BreakpointResolverScripted>(breakpoint, m_class_name,
+                                                      m_depth, m_args);
 }

diff  --git a/lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp b/lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
index ce726c8de6254..b82a2647e9a08 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
+++ b/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 * {
-    if (extra_args_sp == nullptr)
-      return nullptr;
-    auto *extra_args_impl = new StructuredDataImpl();
-    extra_args_impl->SetObjectSP(extra_args_sp);
-    return extra_args_impl;
-  }();
+  StructuredDataImpl extra_args_impl(std::move(extra_args_sp));
   return LLDBSwigLuaBreakpointCallbackFunction(m_lua_state, stop_frame_sp,
                                                bp_loc_sp, extra_args_impl);
 }

diff  --git a/lldb/source/Plugins/ScriptInterpreter/Lua/SWIGLuaBridge.h b/lldb/source/Plugins/ScriptInterpreter/Lua/SWIGLuaBridge.h
index 9efc20ce07fc1..5fca18f2dd6d5 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Lua/SWIGLuaBridge.h
+++ b/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);

diff  --git a/lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h b/lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
index c7af135988433..56365a9b6fba4 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
+++ b/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,16 +103,17 @@ 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(
-    lldb::TargetSP target_sp, const char *python_class_name,
-    const char *session_dictionary_name, lldb_private::StructuredDataImpl *args,
-    lldb_private::Status &error);
+void *LLDBSwigPythonCreateScriptedStopHook(lldb::TargetSP target_sp,
+                                           const char *python_class_name,
+                                           const char *session_dictionary_name,
+                                           const StructuredDataImpl &args,
+                                           lldb_private::Status &error);
 
 bool LLDBSwigPythonStopHookCallHandleStop(void *implementor,
                                           lldb::ExecutionContextRefSP exc_ctx,

diff  --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
index 5f282d74e3649..9187129ab0c00 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ b/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,
-    Status &error) {
+    TargetSP target_sp, const char *class_name,
+    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) {
 

diff  --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
index 8cfc24e71283a..2e8301a85eb6c 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
@@ -33,13 +33,12 @@ class ScriptInterpreterPython : public ScriptInterpreter,
     CommandDataPython() : BreakpointOptions::CommandData() {
       interpreter = lldb::eScriptLanguagePython;
     }
-    CommandDataPython(StructuredData::ObjectSP extra_args_sp) :
-        BreakpointOptions::CommandData(),
-        m_extra_args_up(new StructuredDataImpl()) {
-        interpreter = lldb::eScriptLanguagePython;
-        m_extra_args_up->SetObjectSP(extra_args_sp);
+    CommandDataPython(StructuredData::ObjectSP extra_args_sp)
+        : BreakpointOptions::CommandData(),
+          m_extra_args(std::move(extra_args_sp)) {
+      interpreter = lldb::eScriptLanguagePython;
     }
-    lldb::StructuredDataImplUP m_extra_args_up;
+    StructuredDataImpl m_extra_args;
   };
 
   ScriptInterpreterPython(Debugger &debugger)

diff  --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
index a3f83b696ed49..defc2acffcfa9 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
@@ -79,7 +79,7 @@ class ScriptInterpreterPythonImpl : public ScriptInterpreterPython {
 
   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 @@ class ScriptInterpreterPythonImpl : public ScriptInterpreterPython {
 
   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 @@ class ScriptInterpreterPythonImpl : public ScriptInterpreterPython {
 
   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,

diff  --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
index 29680dab5a14a..e3c1931a565ac 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
+++ b/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;
-  if (args_sp) {
-    args_impl = new StructuredDataImpl();
-    args_impl->SetObjectSP(args_sp);
-  }
+  StructuredDataImpl args_impl(args_sp);
   std::string error_string;
 
   Locker py_lock(&m_interpreter, Locker::AcquireLock | Locker::NoSTDIN,

diff  --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp
index d2c28bc426ee6..6a881bfe625c6 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp
+++ b/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;
-  if (args_sp) {
-    args_impl = new StructuredDataImpl();
-    args_impl->SetObjectSP(args_sp);
-  }
+  StructuredDataImpl args_impl(args_sp);
   std::string error_string;
 
   Locker py_lock(&m_interpreter, Locker::AcquireLock | Locker::NoSTDIN,

diff  --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index ea65bb1a3efa3..fa860399aca7b 100644
--- a/lldb/source/Target/Target.cpp
+++ b/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 = new StructuredDataImpl();
-
-  if (extra_args_sp)
-    m_extra_args->SetObjectSP(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;
 

diff  --git a/lldb/source/Target/Thread.cpp b/lldb/source/Target/Thread.cpp
index 1b32331d98f76..481a39a576e9a 100644
--- a/lldb/source/Target/Thread.cpp
+++ b/lldb/source/Target/Thread.cpp
@@ -1370,15 +1370,9 @@ lldb::ThreadPlanSP Thread::QueueThreadPlanForStepScripted(
     bool abort_other_plans, const char *class_name, 
     StructuredData::ObjectSP extra_args_sp,  bool stop_other_threads,
     Status &status) {
-    
-  StructuredDataImpl *extra_args_impl = nullptr; 
-  if (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));
+  ThreadPlanSP thread_plan_sp(new ThreadPlanPython(
+      *this, class_name, StructuredDataImpl(extra_args_sp)));
   thread_plan_sp->SetStopOthers(stop_other_threads);
   status = QueueThreadPlan(thread_plan_sp, abort_other_plans);
   return thread_plan_sp;

diff  --git a/lldb/source/Target/ThreadPlanPython.cpp b/lldb/source/Target/ThreadPlanPython.cpp
index cd63d28a3934c..a8a36ae65c462 100644
--- a/lldb/source/Target/ThreadPlanPython.cpp
+++ b/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;

diff  --git a/lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp b/lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
index 7483fdeb0c306..5e9fca597465f 100644
--- a/lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp
+++ b/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;
 }
 

diff  --git a/lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp b/lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
index 1295ab2a7b78b..c27fc037bce2d 100644
--- a/lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
+++ b/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,
-    lldb_private::StructuredDataImpl *args_impl, Status &error) {
+    const char *session_dictionary_name, const StructuredDataImpl &args_impl,
+    Status &error) {
   return nullptr;
 }
 


        


More information about the lldb-commits mailing list