diff --git a/lldb/bindings/python/python-swigsafecast.swig b/lldb/bindings/python/python-swigsafecast.swig index 7d639e664f531a..eb684133abef77 100644 --- a/lldb/bindings/python/python-swigsafecast.swig +++ b/lldb/bindings/python/python-swigsafecast.swig @@ -1,19 +1,32 @@ namespace lldb_private { namespace python { -PyObject *SBTypeToSWIGWrapper(lldb::SBEvent &event_sb) { - return SWIG_NewPointerObj(&event_sb, SWIGTYPE_p_lldb__SBEvent, 0); -} - -PyObject *SBTypeToSWIGWrapper(lldb::SBCommandReturnObject &cmd_ret_obj_sb) { - return SWIG_NewPointerObj(&cmd_ret_obj_sb, - SWIGTYPE_p_lldb__SBCommandReturnObject, 0); -} - PythonObject ToSWIGHelper(void *obj, swig_type_info *info) { return {PyRefType::Owned, SWIG_NewPointerObj(obj, info, SWIG_POINTER_OWN)}; } +/// A class that automatically clears an SB object when it goes out of scope. +/// Use for cases where the SB object points to a temporary/unowned entity. +template class ScopedPythonObject : PythonObject { +public: + ScopedPythonObject(T *sb, swig_type_info *info) + : PythonObject(ToSWIGHelper(sb, info)), m_sb(sb) {} + ~ScopedPythonObject() { + if (m_sb) + *m_sb = T(); + } + ScopedPythonObject(ScopedPythonObject &&rhs) + : PythonObject(std::move(rhs)), m_sb(std::exchange(rhs.m_sb, nullptr)) {} + ScopedPythonObject(const ScopedPythonObject &) = delete; + ScopedPythonObject &operator=(const ScopedPythonObject &) = delete; + ScopedPythonObject &operator=(ScopedPythonObject &&) = delete; + + const PythonObject &obj() const { return *this; } + +private: + T *m_sb; +}; + PythonObject ToSWIGWrapper(std::unique_ptr value_sb) { return ToSWIGHelper(value_sb.release(), SWIGTYPE_p_lldb__SBValue); } @@ -94,5 +107,17 @@ PythonObject ToSWIGWrapper(const SymbolContext &sym_ctx) { SWIGTYPE_p_lldb__SBSymbolContext); } +ScopedPythonObject +ToSWIGWrapper(CommandReturnObject &cmd_retobj) { + return ScopedPythonObject( + new lldb::SBCommandReturnObject(cmd_retobj), + SWIGTYPE_p_lldb__SBCommandReturnObject); +} + +ScopedPythonObject ToSWIGWrapper(Event *event) { + return ScopedPythonObject(new lldb::SBEvent(event), + SWIGTYPE_p_lldb__SBEvent); +} + } // namespace python } // namespace lldb_private diff --git a/lldb/bindings/python/python-wrapper.swig b/lldb/bindings/python/python-wrapper.swig index a2c1f756a0a2d8..4f1d65200b10fc 100644 --- a/lldb/bindings/python/python-wrapper.swig +++ b/lldb/bindings/python/python-wrapper.swig @@ -376,9 +376,8 @@ bool lldb_private::LLDBSWIGPythonCallThreadPlan( PythonObject result; if (event != nullptr) { - lldb::SBEvent sb_event(event); - PythonObject event_arg(PyRefType::Owned, SBTypeToSWIGWrapper(sb_event)); - result = pfunc(event_arg); + ScopedPythonObject event_arg = ToSWIGWrapper(event); + result = pfunc(event_arg.obj()); } else result = pfunc(); @@ -795,7 +794,6 @@ bool lldb_private::LLDBSwigPythonCallCommand( lldb::DebuggerSP debugger, const char *args, lldb_private::CommandReturnObject &cmd_retobj, lldb::ExecutionContextRefSP exe_ctx_ref_sp) { - lldb::SBCommandReturnObject cmd_retobj_sb(cmd_retobj); PyErr_Cleaner py_err_cleaner(true); auto dict = PythonModule::MainModule().ResolveName( @@ -812,14 +810,13 @@ bool lldb_private::LLDBSwigPythonCallCommand( return false; } PythonObject debugger_arg = ToSWIGWrapper(std::move(debugger)); - PythonObject cmd_retobj_arg(PyRefType::Owned, - SBTypeToSWIGWrapper(cmd_retobj_sb)); + auto cmd_retobj_arg = ToSWIGWrapper(cmd_retobj); if (argc.get().max_positional_args < 5u) - pfunc(debugger_arg, PythonString(args), cmd_retobj_arg, dict); + pfunc(debugger_arg, PythonString(args), cmd_retobj_arg.obj(), dict); else pfunc(debugger_arg, PythonString(args), - ToSWIGWrapper(std::move(exe_ctx_ref_sp)), cmd_retobj_arg, dict); + ToSWIGWrapper(std::move(exe_ctx_ref_sp)), cmd_retobj_arg.obj(), dict); return true; } @@ -828,7 +825,6 @@ bool lldb_private::LLDBSwigPythonCallCommandObject( PyObject *implementor, lldb::DebuggerSP debugger, const char *args, lldb_private::CommandReturnObject &cmd_retobj, lldb::ExecutionContextRefSP exe_ctx_ref_sp) { - lldb::SBCommandReturnObject cmd_retobj_sb(cmd_retobj); PyErr_Cleaner py_err_cleaner(true); @@ -838,11 +834,10 @@ bool lldb_private::LLDBSwigPythonCallCommandObject( if (!pfunc.IsAllocated()) return false; - PythonObject cmd_retobj_arg(PyRefType::Owned, - SBTypeToSWIGWrapper(cmd_retobj_sb)); + auto cmd_retobj_arg = ToSWIGWrapper(cmd_retobj); pfunc(ToSWIGWrapper(std::move(debugger)), PythonString(args), - ToSWIGWrapper(exe_ctx_ref_sp), cmd_retobj_arg); + ToSWIGWrapper(exe_ctx_ref_sp), cmd_retobj_arg.obj()); return true; } diff --git a/lldb/test/API/commands/command/script/TestCommandScript.py b/lldb/test/API/commands/command/script/TestCommandScript.py index 33e6a00a404f89..eed36c95ec32c7 100644 --- a/lldb/test/API/commands/command/script/TestCommandScript.py +++ b/lldb/test/API/commands/command/script/TestCommandScript.py @@ -167,7 +167,17 @@ def cleanup(): self.runCmd('bug11569', check=False) def test_persistence(self): + """ + Ensure that function arguments meaningfully persist (and do not crash!) + even after the function terminates. + """ self.runCmd("command script import persistence.py") self.runCmd("command script add -f persistence.save_debugger save_debugger") self.expect("save_debugger", substrs=[str(self.dbg)]) + + # After the command completes, the debugger object should still be + # valid. self.expect("script str(persistence.debugger_copy)", substrs=[str(self.dbg)]) + # The result object will be replaced by an empty result object (in the + # "Started" state). + self.expect("script str(persistence.result_copy)", substrs=["Started"]) diff --git a/lldb/test/API/commands/command/script/persistence.py b/lldb/test/API/commands/command/script/persistence.py index bc08b4f4dcf487..5c0be34a7d1835 100644 --- a/lldb/test/API/commands/command/script/persistence.py +++ b/lldb/test/API/commands/command/script/persistence.py @@ -1,9 +1,11 @@ import lldb debugger_copy = None +result_copy = None def save_debugger(debugger, command, context, result, internal_dict): - global debugger_copy + global debugger_copy, result_copy debugger_copy = debugger + result_copy = result result.AppendMessage(str(debugger)) result.SetStatus(lldb.eReturnStatusSuccessFinishResult)