From 82de8df26f15778793dc6b1526e14779f435f2e1 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Thu, 25 Nov 2021 14:01:41 +0100 Subject: [PATCH] [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 | 10 +++--- lldb/bindings/python/python-wrapper.swig | 27 ++++++++------- lldb/include/lldb/API/SBStructuredData.h | 2 +- .../Breakpoint/BreakpointResolverScripted.h | 11 +++---- lldb/include/lldb/Core/StructuredDataImpl.h | 3 ++ .../lldb/Interpreter/ScriptInterpreter.h | 6 ++-- lldb/include/lldb/Target/Target.h | 4 +-- lldb/include/lldb/Target/ThreadPlanPython.h | 14 +++----- lldb/source/API/SBStructuredData.cpp | 8 ++--- lldb/source/API/SBThreadPlan.cpp | 6 ++-- .../Breakpoint/BreakpointResolverScripted.cpp | 33 ++++++++----------- .../Plugins/ScriptInterpreter/Lua/Lua.cpp | 8 +---- .../ScriptInterpreter/Lua/SWIGLuaBridge.h | 3 +- .../Python/SWIGPythonBridge.h | 19 ++++++----- .../Python/ScriptInterpreterPython.cpp | 10 +++--- .../Python/ScriptInterpreterPython.h | 11 +++---- .../Python/ScriptInterpreterPythonImpl.h | 7 ++-- .../Python/ScriptedProcessPythonInterface.cpp | 6 +--- .../Python/ScriptedThreadPythonInterface.cpp | 6 +--- lldb/source/Target/Target.cpp | 16 +++------ lldb/source/Target/Thread.cpp | 10 ++---- lldb/source/Target/ThreadPlanPython.cpp | 7 +--- .../ScriptInterpreter/Lua/LuaTests.cpp | 3 +- .../Python/PythonTestSuite.cpp | 14 ++++---- 24 files changed, 103 insertions(+), 141 deletions(-) diff --git a/lldb/bindings/lua/lua-wrapper.swig b/lldb/bindings/lua/lua-wrapper.swig index 4ca406137d6c..9e4b8d1224bd 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 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 079f8d12dafa..da943f73c435 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 07075abbf1d0..533dcc8fc07c 100644 --- a/lldb/include/lldb/API/SBStructuredData.h +++ b/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(); diff --git a/lldb/include/lldb/Breakpoint/BreakpointResolverScripted.h b/lldb/include/lldb/Breakpoint/BreakpointResolverScripted.h index 26fd6f2f04d7..cecd0f92a21a 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 @@ 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 - // 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 d6f64451e5c2..8930ebff8166 100644 --- a/lldb/include/lldb/Core/StructuredDataImpl.h +++ b/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())), diff --git a/lldb/include/lldb/Interpreter/ScriptInterpreter.h b/lldb/include/lldb/Interpreter/ScriptInterpreter.h index 2b96021fffc9..83f784bde712 100644 --- a/lldb/include/lldb/Interpreter/ScriptInterpreter.h +++ b/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(); diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h index 7e8e1373a506..b85839c15b2f 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 @@ 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, - // 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 7b37b2b9ce5a..f148f88d4c46 100644 --- a/lldb/include/lldb/Target/ThreadPlanPython.h +++ b/lldb/include/lldb/Target/ThreadPlanPython.h @@ -12,8 +12,7 @@ #include -#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 @@ protected: 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 97a9eadcaf07..e99c6194c516 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(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 9af673b0f3a9..99ecb321595f 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(*thread, class_name, nullptr); + m_opaque_wp = std::make_shared(*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(*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 92297fbc7c4b..308c3b987f58 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(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 ce726c8de625..b82a2647e9a0 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 9efc20ce07fc..5fca18f2dd6d 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 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 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 c7af13598843..56365a9b6fba 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 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 5f282d74e364..9187129ab0c0 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 8cfc24e71283..2e8301a85eb6 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h +++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h @@ -33,13 +33,12 @@ public: 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 a3f83b696ed4..defc2acffcfa 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h +++ b/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, diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp index 29680dab5a14..e3c1931a565a 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 d2c28bc426ee..6a881bfe625c 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 ea65bb1a3efa..fa860399aca7 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 1b32331d98f7..481a39a576e9 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 cd63d28a3934..a8a36ae65c46 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 7483fdeb0c30..5e9fca597465 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 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 1295ab2a7b78..c27fc037bce2 100644 --- a/lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp +++ b/lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp @@ -63,7 +63,7 @@ llvm::Expected 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; }