From 82de8df26f15778793dc6b1526e14779f435f2e1 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Thu, 25 Nov 2021 14:01:41 +0100 Subject: [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 --- .../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 +----- 6 files changed, 26 insertions(+), 33 deletions(-) (limited to 'lldb/source/Plugins/ScriptInterpreter/Python') 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, -- cgit v1.1