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 +- .../lldb/Breakpoint/BreakpointResolverScripted.h | 11 +++----- lldb/include/lldb/Core/StructuredDataImpl.h | 3 ++ lldb/include/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 +++++++++------------- lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp | 8 +----- .../Plugins/ScriptInterpreter/Lua/SWIGLuaBridge.h | 3 +- .../ScriptInterpreter/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 +---- lldb/unittests/ScriptInterpreter/Lua/LuaTests.cpp | 3 +- .../ScriptInterpreter/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 4ca4061..9e4b8d1 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 079f8d1..da943f7 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 07075ab..533dcc8 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 26fd6f2..cecd0f9 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 d6f6445..8930ebf 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 2b96021..83f784b 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 7e8e137..b85839c 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 7b37b2b..f148f88 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 97a9ead..e99c619 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 9af673b..99ecb32 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 92297fb..308c3b9 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 ce726c8..b82a264 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 9efc20c..5fca18f 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 c7af135..56365a9 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 5f282d7..9187129 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 8cfc24e..2e8301a 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 a3f83b6..defc2ac 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 29680da..e3c1931 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 d2c28bc..6a881bf 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 ea65bb1..fa86039 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 1b32331..481a39a 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 cd63d28..a8a36ae 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 7483fde..5e9fca5 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 1295ab2..c27fc03 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; } -- 2.7.4