mib created this revision. mib added a reviewer: JDevlieghere. mib added a project: LLVM. mib requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits.
This patch replaces all the ScriptedInterface object instance shared pointer by a raw pointer. The reason behind the change is that when the smart pointer gets re-assigned, that triggers calling the default deleter to the previously pointer object. However, in this case, the pointed memory was allocated in Python, so when another object tries to read it, it causes a heap-use-after-free. By switching to a raw pointer, it prevents lldb from decrementing the reference counting to 0 and calling the deleter for that object. rdar://87425859 Signed-off-by: Med Ismail Bennani <medismail.benn...@gmail.com> Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D117065 Files: lldb/include/lldb/Interpreter/ScriptedInterface.h lldb/include/lldb/Interpreter/ScriptedProcessInterface.h lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp lldb/source/Plugins/Process/scripted/ScriptedProcess.h lldb/source/Plugins/Process/scripted/ScriptedThread.cpp lldb/source/Plugins/Process/scripted/ScriptedThread.h lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.h lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.h
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.h =================================================================== --- lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.h +++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.h @@ -22,7 +22,7 @@ public: ScriptedThreadPythonInterface(ScriptInterpreterPythonImpl &interpreter); - StructuredData::GenericSP + StructuredData::Generic * CreatePluginObject(llvm::StringRef class_name, ExecutionContext &exe_ctx, StructuredData::DictionarySP args_sp) override; Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp =================================================================== --- lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp +++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp @@ -29,7 +29,7 @@ ScriptInterpreterPythonImpl &interpreter) : ScriptedThreadInterface(), ScriptedPythonInterface(interpreter) {} -StructuredData::GenericSP ScriptedThreadPythonInterface::CreatePluginObject( +StructuredData::Generic *ScriptedThreadPythonInterface::CreatePluginObject( const llvm::StringRef class_name, ExecutionContext &exe_ctx, StructuredData::DictionarySP args_sp) { @@ -50,10 +50,10 @@ if (!ret_val) return {}; - m_object_instance_sp = - StructuredData::GenericSP(new StructuredPythonObject(ret_val)); + m_object_instance = static_cast<StructuredData::Generic *>( + new StructuredPythonObject(ret_val)); - return m_object_instance_sp; + return m_object_instance; } lldb::tid_t ScriptedThreadPythonInterface::GetThreadID() { Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h =================================================================== --- lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h +++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h @@ -42,15 +42,16 @@ llvm::Twine(LLVM_PRETTY_FUNCTION + llvm::Twine(" (") + llvm::Twine(method_name) + llvm::Twine(")")) .str(); - if (!m_object_instance_sp) - return ErrorWithMessage<T>(caller_signature, "Python object ill-formed", - error); Locker py_lock(&m_interpreter, Locker::AcquireLock | Locker::NoSTDIN, Locker::FreeLock); + if (!m_object_instance && m_object_instance->IsValid()) + return ErrorWithMessage<T>(caller_signature, "Python object ill-formed", + error); + PythonObject implementor(PyRefType::Borrowed, - (PyObject *)m_object_instance_sp->GetValue()); + (PyObject *)m_object_instance->GetValue()); if (!implementor.IsAllocated()) return ErrorWithMessage<T>(caller_signature, Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.h =================================================================== --- lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.h +++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.h @@ -22,7 +22,7 @@ public: ScriptedProcessPythonInterface(ScriptInterpreterPythonImpl &interpreter); - StructuredData::GenericSP + StructuredData::Generic * CreatePluginObject(const llvm::StringRef class_name, ExecutionContext &exe_ctx, StructuredData::DictionarySP args_sp) override; Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp =================================================================== --- lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp +++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp @@ -30,7 +30,7 @@ ScriptInterpreterPythonImpl &interpreter) : ScriptedProcessInterface(), ScriptedPythonInterface(interpreter) {} -StructuredData::GenericSP ScriptedProcessPythonInterface::CreatePluginObject( +StructuredData::Generic *ScriptedProcessPythonInterface::CreatePluginObject( llvm::StringRef class_name, ExecutionContext &exe_ctx, StructuredData::DictionarySP args_sp) { if (class_name.empty()) @@ -50,10 +50,10 @@ if (!ret_val) return {}; - m_object_instance_sp = - StructuredData::GenericSP(new StructuredPythonObject(ret_val)); + m_object_instance = static_cast<StructuredData::Generic *>( + new StructuredPythonObject(ret_val)); - return m_object_instance_sp; + return m_object_instance; } Status ScriptedProcessPythonInterface::Launch() { Index: lldb/source/Plugins/Process/scripted/ScriptedThread.h =================================================================== --- lldb/source/Plugins/Process/scripted/ScriptedThread.h +++ lldb/source/Plugins/Process/scripted/ScriptedThread.h @@ -60,7 +60,7 @@ const ScriptedProcess &m_scripted_process; std::shared_ptr<DynamicRegisterInfo> m_register_info_sp = nullptr; - lldb_private::StructuredData::ObjectSP m_script_object_sp = nullptr; + lldb_private::StructuredData::Generic *m_script_object = nullptr; }; } // namespace lldb_private Index: lldb/source/Plugins/Process/scripted/ScriptedThread.cpp =================================================================== --- lldb/source/Plugins/Process/scripted/ScriptedThread.cpp +++ lldb/source/Plugins/Process/scripted/ScriptedThread.cpp @@ -24,7 +24,7 @@ using namespace lldb_private; void ScriptedThread::CheckInterpreterAndScriptObject() const { - lldbassert(m_script_object_sp && "Invalid Script Object."); + lldbassert(m_script_object && "Invalid Script Object."); lldbassert(GetInterface() && "Invalid Scripted Thread Interface."); } @@ -52,16 +52,16 @@ ExecutionContext exe_ctx(process); - StructuredData::GenericSP object_sp = + StructuredData::Generic *script_object = scripted_thread_interface->CreatePluginObject( class_name->c_str(), exe_ctx, process.m_scripted_process_info.GetArgsSP()); - if (!object_sp || !object_sp->IsValid()) { + if (!script_object || !script_object->IsValid()) { error.SetErrorString("Failed to create valid script object"); return; } - m_script_object_sp = object_sp; + m_script_object = script_object; SetID(scripted_thread_interface->GetThreadID()); } Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.h =================================================================== --- lldb/source/Plugins/Process/scripted/ScriptedProcess.h +++ lldb/source/Plugins/Process/scripted/ScriptedProcess.h @@ -110,8 +110,7 @@ // Member variables. const ScriptedProcessInfo m_scripted_process_info; lldb_private::ScriptInterpreter *m_interpreter = nullptr; - lldb_private::StructuredData::ObjectSP m_script_object_sp = nullptr; - //@} + lldb_private::StructuredData::Generic *m_script_object = nullptr; }; } // namespace lldb_private Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp =================================================================== --- lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp +++ lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp @@ -46,7 +46,7 @@ void ScriptedProcess::CheckInterpreterAndScriptObject() const { lldbassert(m_interpreter && "Invalid Script Interpreter."); - lldbassert(m_script_object_sp && "Invalid Script Object."); + lldbassert(m_script_object && "Invalid Script Object."); } lldb::ProcessSP ScriptedProcess::CreateInstance(lldb::TargetSP target_sp, @@ -64,8 +64,8 @@ auto process_sp = std::make_shared<ScriptedProcess>( target_sp, listener_sp, scripted_process_info, error); - if (error.Fail() || !process_sp || !process_sp->m_script_object_sp || - !process_sp->m_script_object_sp->IsValid()) { + if (error.Fail() || !process_sp || !process_sp->m_script_object || + !process_sp->m_script_object->IsValid()) { LLDB_LOGF(GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS), "%s", error.AsCString()); return nullptr; @@ -103,18 +103,18 @@ ExecutionContext exe_ctx(target_sp, /*get_process=*/false); - StructuredData::GenericSP object_sp = GetInterface().CreatePluginObject( + StructuredData::Generic *script_object = GetInterface().CreatePluginObject( m_scripted_process_info.GetClassName().c_str(), exe_ctx, m_scripted_process_info.GetArgsSP()); - if (!object_sp || !object_sp->IsValid()) { + if (!script_object || !script_object->IsValid()) { error.SetErrorStringWithFormat("ScriptedProcess::%s () - ERROR: %s", __FUNCTION__, "Failed to create valid script object"); return; } - m_script_object_sp = object_sp; + m_script_object = script_object; } ScriptedProcess::~ScriptedProcess() { @@ -217,7 +217,7 @@ Status ScriptedProcess::DoDestroy() { return Status(); } bool ScriptedProcess::IsAlive() { - if (m_interpreter && m_script_object_sp) + if (m_interpreter && m_script_object) return GetInterface().IsAlive(); return false; } Index: lldb/include/lldb/Interpreter/ScriptedProcessInterface.h =================================================================== --- lldb/include/lldb/Interpreter/ScriptedProcessInterface.h +++ lldb/include/lldb/Interpreter/ScriptedProcessInterface.h @@ -21,7 +21,7 @@ namespace lldb_private { class ScriptedProcessInterface : virtual public ScriptedInterface { public: - StructuredData::GenericSP + StructuredData::Generic * CreatePluginObject(llvm::StringRef class_name, ExecutionContext &exe_ctx, StructuredData::DictionarySP args_sp) override { return nullptr; @@ -75,7 +75,7 @@ class ScriptedThreadInterface : virtual public ScriptedInterface { public: - StructuredData::GenericSP + StructuredData::Generic * CreatePluginObject(llvm::StringRef class_name, ExecutionContext &exe_ctx, StructuredData::DictionarySP args_sp) override { return nullptr; Index: lldb/include/lldb/Interpreter/ScriptedInterface.h =================================================================== --- lldb/include/lldb/Interpreter/ScriptedInterface.h +++ lldb/include/lldb/Interpreter/ScriptedInterface.h @@ -25,7 +25,7 @@ ScriptedInterface() = default; virtual ~ScriptedInterface() = default; - virtual StructuredData::GenericSP + virtual StructuredData::Generic * CreatePluginObject(llvm::StringRef class_name, ExecutionContext &exe_ctx, StructuredData::DictionarySP args_sp) = 0; @@ -67,8 +67,12 @@ return true; } + void SetPluginObject(StructuredData::Generic *object_instance) { + m_object_instance = object_instance; + } + protected: - StructuredData::GenericSP m_object_instance_sp; + StructuredData::Generic *m_object_instance; }; } // namespace lldb_private #endif // LLDB_INTERPRETER_SCRIPTEDINTERFACE_H
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits