Author: Pavel Labath Date: 2021-12-22T13:47:06+01:00 New Revision: 2efc6892d89dc15c1697c411e0031d126385a5f1
URL: https://github.com/llvm/llvm-project/commit/2efc6892d89dc15c1697c411e0031d126385a5f1 DIFF: https://github.com/llvm/llvm-project/commit/2efc6892d89dc15c1697c411e0031d126385a5f1.diff LOG: [lldb/python] Avoid more dangling pointers in python glue code Added: Modified: lldb/bindings/python/python-swigsafecast.swig lldb/bindings/python/python-wrapper.swig lldb/include/lldb/API/SBSymbolContext.h lldb/include/lldb/API/SBTypeSummary.h lldb/source/API/SBFrame.cpp lldb/source/API/SBSymbolContext.cpp lldb/source/API/SBSymbolContextList.cpp lldb/source/API/SBTypeSummary.cpp Removed: ################################################################################ diff --git a/lldb/bindings/python/python-swigsafecast.swig b/lldb/bindings/python/python-swigsafecast.swig index 25c2f44106bc4..7d639e664f531 100644 --- a/lldb/bindings/python/python-swigsafecast.swig +++ b/lldb/bindings/python/python-swigsafecast.swig @@ -5,38 +5,11 @@ PyObject *SBTypeToSWIGWrapper(lldb::SBEvent &event_sb) { return SWIG_NewPointerObj(&event_sb, SWIGTYPE_p_lldb__SBEvent, 0); } -PyObject *SBTypeToSWIGWrapper(lldb::SBWatchpoint &watchpoint_sb) { - return SWIG_NewPointerObj(&watchpoint_sb, SWIGTYPE_p_lldb__SBWatchpoint, 0); -} - -PyObject * -SBTypeToSWIGWrapper(lldb::SBBreakpointLocation &breakpoint_location_sb) { - return SWIG_NewPointerObj(&breakpoint_location_sb, - SWIGTYPE_p_lldb__SBBreakpointLocation, 0); -} - PyObject *SBTypeToSWIGWrapper(lldb::SBCommandReturnObject &cmd_ret_obj_sb) { return SWIG_NewPointerObj(&cmd_ret_obj_sb, SWIGTYPE_p_lldb__SBCommandReturnObject, 0); } -PyObject *SBTypeToSWIGWrapper(lldb::SBExecutionContext &ctx_sb) { - return SWIG_NewPointerObj(&ctx_sb, SWIGTYPE_p_lldb__SBExecutionContext, 0); -} - -PyObject *SBTypeToSWIGWrapper(lldb::SBTypeSummaryOptions &summary_options_sb) { - return SWIG_NewPointerObj(&summary_options_sb, - SWIGTYPE_p_lldb__SBTypeSummaryOptions, 0); -} - -PyObject *SBTypeToSWIGWrapper(lldb::SBSymbolContext &sym_ctx_sb) { - return SWIG_NewPointerObj(&sym_ctx_sb, SWIGTYPE_p_lldb__SBSymbolContext, 0); -} - -PyObject *SBTypeToSWIGWrapper(lldb::SBStream &stream_sb) { - return SWIG_NewPointerObj(&stream_sb, SWIGTYPE_p_lldb__SBStream, 0); -} - PythonObject ToSWIGHelper(void *obj, swig_type_info *info) { return {PyRefType::Owned, SWIG_NewPointerObj(obj, info, SWIG_POINTER_OWN)}; } @@ -69,6 +42,10 @@ PythonObject ToSWIGWrapper(lldb::BreakpointSP breakpoint_sp) { SWIGTYPE_p_lldb__SBBreakpoint); } +PythonObject ToSWIGWrapper(std::unique_ptr<lldb::SBStream> stream_sb) { + return ToSWIGHelper(stream_sb.release(), SWIGTYPE_p_lldb__SBStream); +} + PythonObject ToSWIGWrapper(std::unique_ptr<lldb::SBStructuredData> data_sb) { return ToSWIGHelper(data_sb.release(), SWIGTYPE_p_lldb__SBStructuredData); } @@ -92,5 +69,30 @@ PythonObject ToSWIGWrapper(lldb::DebuggerSP debugger_sp) { SWIGTYPE_p_lldb__SBDebugger); } +PythonObject ToSWIGWrapper(lldb::WatchpointSP watchpoint_sp) { + return ToSWIGHelper(new lldb::SBWatchpoint(std::move(watchpoint_sp)), + SWIGTYPE_p_lldb__SBWatchpoint); +} + +PythonObject ToSWIGWrapper(lldb::BreakpointLocationSP bp_loc_sp) { + return ToSWIGHelper(new lldb::SBBreakpointLocation(std::move(bp_loc_sp)), + SWIGTYPE_p_lldb__SBBreakpointLocation); +} + +PythonObject ToSWIGWrapper(lldb::ExecutionContextRefSP ctx_sp) { + return ToSWIGHelper(new lldb::SBExecutionContext(std::move(ctx_sp)), + SWIGTYPE_p_lldb__SBExecutionContext); +} + +PythonObject ToSWIGWrapper(const TypeSummaryOptions &summary_options) { + return ToSWIGHelper(new lldb::SBTypeSummaryOptions(summary_options), + SWIGTYPE_p_lldb__SBTypeSummaryOptions); +} + +PythonObject ToSWIGWrapper(const SymbolContext &sym_ctx) { + return ToSWIGHelper(new lldb::SBSymbolContext(sym_ctx), + SWIGTYPE_p_lldb__SBSymbolContext); +} + } // namespace python } // namespace lldb_private diff --git a/lldb/bindings/python/python-wrapper.swig b/lldb/bindings/python/python-wrapper.swig index 5f85af2ba6ce2..a2c1f756a0a2d 100644 --- a/lldb/bindings/python/python-wrapper.swig +++ b/lldb/bindings/python/python-wrapper.swig @@ -38,14 +38,12 @@ llvm::Expected<bool> lldb_private::LLDBSwigPythonBreakpointCallbackFunction( return arg_info.takeError(); PythonObject frame_arg = ToSWIGWrapper(frame_sp); - PythonObject bp_loc_arg(PyRefType::Owned, SBTypeToSWIGWrapper(sb_bp_loc)); + PythonObject bp_loc_arg = ToSWIGWrapper(bp_loc_sp); - auto result = [&]() -> Expected<PythonObject> { - // If the called function doesn't take extra_args, drop them here: - if (max_positional_args < 4) - return pfunc.Call(frame_arg, bp_loc_arg, dict); - return pfunc.Call(frame_arg, bp_loc_arg, ToSWIGWrapper(args_impl), dict); - }(); + auto result = + max_positional_args < 4 + ? pfunc.Call(frame_arg, bp_loc_arg, dict) + : pfunc.Call(frame_arg, bp_loc_arg, ToSWIGWrapper(args_impl), dict); if (!result) return result.takeError(); @@ -70,7 +68,6 @@ llvm::Expected<bool> lldb_private::LLDBSwigPythonBreakpointCallbackFunction( bool lldb_private::LLDBSwigPythonWatchpointCallbackFunction( const char *python_function_name, const char *session_dictionary_name, const lldb::StackFrameSP &frame_sp, const lldb::WatchpointSP &wp_sp) { - lldb::SBWatchpoint sb_wp(wp_sp); bool stop_at_watchpoint = true; @@ -84,9 +81,8 @@ bool lldb_private::LLDBSwigPythonWatchpointCallbackFunction( if (!pfunc.IsAllocated()) return stop_at_watchpoint; - PythonObject frame_arg = ToSWIGWrapper(frame_sp); - PythonObject wp_arg(PyRefType::Owned, SBTypeToSWIGWrapper(sb_wp)); - PythonObject result = pfunc(frame_arg, wp_arg, dict); + PythonObject result = + pfunc(ToSWIGWrapper(frame_sp), ToSWIGWrapper(wp_sp), dict); if (result.get() == Py_False) stop_at_watchpoint = false; @@ -98,7 +94,6 @@ bool lldb_private::LLDBSwigPythonCallTypeScript( const char *python_function_name, const void *session_dictionary, const lldb::ValueObjectSP &valobj_sp, void **pyfunct_wrapper, const lldb::TypeSummaryOptionsSP &options_sp, std::string &retval) { - lldb::SBTypeSummaryOptions sb_options(options_sp.get()); retval.clear(); @@ -146,12 +141,11 @@ bool lldb_private::LLDBSwigPythonCallTypeScript( } PythonObject value_arg = ToSWIGWrapper(valobj_sp); - PythonObject options_arg(PyRefType::Owned, SBTypeToSWIGWrapper(sb_options)); if (argc.get().max_positional_args < 3) result = pfunc(value_arg, dict); else - result = pfunc(value_arg, dict, options_arg); + result = pfunc(value_arg, dict, ToSWIGWrapper(*options_sp)); retval = result.Str().GetString().str(); @@ -452,13 +446,7 @@ unsigned int lldb_private::LLDBSwigPythonCallBreakpointResolver( if (!pfunc.IsAllocated()) return 0; - PythonObject result; - if (sym_ctx != nullptr) { - lldb::SBSymbolContext sb_sym_ctx(sym_ctx); - PythonObject sym_ctx_arg(PyRefType::Owned, SBTypeToSWIGWrapper(sb_sym_ctx)); - result = pfunc(sym_ctx_arg); - } else - result = pfunc(); + PythonObject result = sym_ctx ? pfunc(ToSWIGWrapper(*sym_ctx)) : pfunc(); if (PyErr_Occurred()) { PyErr_Print(); @@ -560,12 +548,11 @@ bool lldb_private::LLDBSwigPythonStopHookCallHandleStop( if (!pfunc.IsAllocated()) return true; - PythonObject result; - lldb::SBExecutionContext sb_exc_ctx(exc_ctx_sp); - PythonObject exc_ctx_arg(PyRefType::Owned, SBTypeToSWIGWrapper(sb_exc_ctx)); - lldb::SBStream sb_stream; - PythonObject sb_stream_arg(PyRefType::Owned, SBTypeToSWIGWrapper(sb_stream)); - result = pfunc(exc_ctx_arg, sb_stream_arg); + auto *sb_stream = new lldb::SBStream(); + PythonObject sb_stream_arg = + ToSWIGWrapper(std::unique_ptr<lldb::SBStream>(sb_stream)); + PythonObject result = + pfunc(ToSWIGWrapper(std::move(exc_ctx_sp)), sb_stream_arg); if (PyErr_Occurred()) { stream->PutCString("Python error occurred handling stop-hook."); @@ -577,7 +564,7 @@ bool lldb_private::LLDBSwigPythonStopHookCallHandleStop( // Now add the result to the output stream. SBStream only // makes an internally help StreamString which I can't interpose, so I // have to copy it over here. - stream->PutCString(sb_stream.GetData()); + stream->PutCString(sb_stream->GetData()); if (result.get() == Py_False) return false; @@ -809,7 +796,6 @@ bool lldb_private::LLDBSwigPythonCallCommand( lldb_private::CommandReturnObject &cmd_retobj, lldb::ExecutionContextRefSP exe_ctx_ref_sp) { lldb::SBCommandReturnObject cmd_retobj_sb(cmd_retobj); - lldb::SBExecutionContext exe_ctx_sb(exe_ctx_ref_sp); PyErr_Cleaner py_err_cleaner(true); auto dict = PythonModule::MainModule().ResolveName<PythonDictionary>( @@ -826,14 +812,14 @@ bool lldb_private::LLDBSwigPythonCallCommand( return false; } PythonObject debugger_arg = ToSWIGWrapper(std::move(debugger)); - PythonObject exe_ctx_arg(PyRefType::Owned, SBTypeToSWIGWrapper(exe_ctx_sb)); PythonObject cmd_retobj_arg(PyRefType::Owned, SBTypeToSWIGWrapper(cmd_retobj_sb)); if (argc.get().max_positional_args < 5u) pfunc(debugger_arg, PythonString(args), cmd_retobj_arg, dict); else - pfunc(debugger_arg, PythonString(args), exe_ctx_arg, cmd_retobj_arg, dict); + pfunc(debugger_arg, PythonString(args), + ToSWIGWrapper(std::move(exe_ctx_ref_sp)), cmd_retobj_arg, dict); return true; } @@ -843,7 +829,6 @@ bool lldb_private::LLDBSwigPythonCallCommandObject( lldb_private::CommandReturnObject &cmd_retobj, lldb::ExecutionContextRefSP exe_ctx_ref_sp) { lldb::SBCommandReturnObject cmd_retobj_sb(cmd_retobj); - lldb::SBExecutionContext exe_ctx_sb(exe_ctx_ref_sp); PyErr_Cleaner py_err_cleaner(true); @@ -853,12 +838,11 @@ bool lldb_private::LLDBSwigPythonCallCommandObject( if (!pfunc.IsAllocated()) return false; - PythonObject exe_ctx_arg(PyRefType::Owned, SBTypeToSWIGWrapper(exe_ctx_sb)); PythonObject cmd_retobj_arg(PyRefType::Owned, SBTypeToSWIGWrapper(cmd_retobj_sb)); - pfunc(ToSWIGWrapper(std::move(debugger)), PythonString(args), exe_ctx_arg, - cmd_retobj_arg); + pfunc(ToSWIGWrapper(std::move(debugger)), PythonString(args), + ToSWIGWrapper(exe_ctx_ref_sp), cmd_retobj_arg); return true; } diff --git a/lldb/include/lldb/API/SBSymbolContext.h b/lldb/include/lldb/API/SBSymbolContext.h index 16ad29ea87307..b4c5921d81a94 100644 --- a/lldb/include/lldb/API/SBSymbolContext.h +++ b/lldb/include/lldb/API/SBSymbolContext.h @@ -25,7 +25,7 @@ class LLDB_API SBSymbolContext { SBSymbolContext(const lldb::SBSymbolContext &rhs); - SBSymbolContext(const lldb_private::SymbolContext *sc_ptr); + SBSymbolContext(const lldb_private::SymbolContext &sc_ptr); ~SBSymbolContext(); @@ -72,8 +72,6 @@ class LLDB_API SBSymbolContext { lldb_private::SymbolContext *get() const; - void SetSymbolContext(const lldb_private::SymbolContext *sc_ptr); - private: std::unique_ptr<lldb_private::SymbolContext> m_opaque_up; }; diff --git a/lldb/include/lldb/API/SBTypeSummary.h b/lldb/include/lldb/API/SBTypeSummary.h index 929bfb6124b2d..e9963682f7ab5 100644 --- a/lldb/include/lldb/API/SBTypeSummary.h +++ b/lldb/include/lldb/API/SBTypeSummary.h @@ -19,7 +19,7 @@ class LLDB_API SBTypeSummaryOptions { SBTypeSummaryOptions(const lldb::SBTypeSummaryOptions &rhs); - SBTypeSummaryOptions(const lldb_private::TypeSummaryOptions *lldb_object_ptr); + SBTypeSummaryOptions(const lldb_private::TypeSummaryOptions &lldb_object); ~SBTypeSummaryOptions(); @@ -48,8 +48,6 @@ class LLDB_API SBTypeSummaryOptions { const lldb_private::TypeSummaryOptions &ref() const; - void SetOptions(const lldb_private::TypeSummaryOptions *lldb_object_ptr); - private: std::unique_ptr<lldb_private::TypeSummaryOptions> m_opaque_up; }; diff --git a/lldb/source/API/SBFrame.cpp b/lldb/source/API/SBFrame.cpp index 7107768ba884b..c6bc3288c4b2f 100644 --- a/lldb/source/API/SBFrame.cpp +++ b/lldb/source/API/SBFrame.cpp @@ -119,15 +119,13 @@ SBSymbolContext SBFrame::GetSymbolContext(uint32_t resolve_scope) const { std::unique_lock<std::recursive_mutex> lock; ExecutionContext exe_ctx(m_opaque_sp.get(), lock); SymbolContextItem scope = static_cast<SymbolContextItem>(resolve_scope); - StackFrame *frame = nullptr; Target *target = exe_ctx.GetTargetPtr(); Process *process = exe_ctx.GetProcessPtr(); if (target && process) { Process::StopLocker stop_locker; if (stop_locker.TryLock(&process->GetRunLock())) { - frame = exe_ctx.GetFramePtr(); - if (frame) - sb_sym_ctx.SetSymbolContext(&frame->GetSymbolContext(scope)); + if (StackFrame *frame = exe_ctx.GetFramePtr()) + sb_sym_ctx = frame->GetSymbolContext(scope); } } diff --git a/lldb/source/API/SBSymbolContext.cpp b/lldb/source/API/SBSymbolContext.cpp index 488d498849038..89fe051658ffa 100644 --- a/lldb/source/API/SBSymbolContext.cpp +++ b/lldb/source/API/SBSymbolContext.cpp @@ -22,12 +22,10 @@ SBSymbolContext::SBSymbolContext() : m_opaque_up() { LLDB_RECORD_CONSTRUCTOR_NO_ARGS(SBSymbolContext); } -SBSymbolContext::SBSymbolContext(const SymbolContext *sc_ptr) : m_opaque_up() { +SBSymbolContext::SBSymbolContext(const SymbolContext &sc) + : m_opaque_up(std::make_unique<SymbolContext>(sc)) { LLDB_RECORD_CONSTRUCTOR(SBSymbolContext, - (const lldb_private::SymbolContext *), sc_ptr); - - if (sc_ptr) - m_opaque_up = std::make_unique<SymbolContext>(*sc_ptr); + (const lldb_private::SymbolContext &), sc); } SBSymbolContext::SBSymbolContext(const SBSymbolContext &rhs) : m_opaque_up() { @@ -49,13 +47,6 @@ const SBSymbolContext &SBSymbolContext::operator=(const SBSymbolContext &rhs) { return LLDB_RECORD_RESULT(*this); } -void SBSymbolContext::SetSymbolContext(const SymbolContext *sc_ptr) { - if (sc_ptr) - m_opaque_up = std::make_unique<SymbolContext>(*sc_ptr); - else - m_opaque_up->Clear(true); -} - bool SBSymbolContext::IsValid() const { LLDB_RECORD_METHOD_CONST_NO_ARGS(bool, SBSymbolContext, IsValid); return this->operator bool(); @@ -237,7 +228,7 @@ template <> void RegisterMethods<SBSymbolContext>(Registry &R) { LLDB_REGISTER_CONSTRUCTOR(SBSymbolContext, ()); LLDB_REGISTER_CONSTRUCTOR(SBSymbolContext, - (const lldb_private::SymbolContext *)); + (const lldb_private::SymbolContext &)); LLDB_REGISTER_CONSTRUCTOR(SBSymbolContext, (const lldb::SBSymbolContext &)); LLDB_REGISTER_METHOD( const lldb::SBSymbolContext &, diff --git a/lldb/source/API/SBSymbolContextList.cpp b/lldb/source/API/SBSymbolContextList.cpp index 9db84dc1bf4b8..70a8bbe6694c9 100644 --- a/lldb/source/API/SBSymbolContextList.cpp +++ b/lldb/source/API/SBSymbolContextList.cpp @@ -56,9 +56,8 @@ SBSymbolContext SBSymbolContextList::GetContextAtIndex(uint32_t idx) { SBSymbolContext sb_sc; if (m_opaque_up) { SymbolContext sc; - if (m_opaque_up->GetContextAtIndex(idx, sc)) { - sb_sc.SetSymbolContext(&sc); - } + if (m_opaque_up->GetContextAtIndex(idx, sc)) + sb_sc = sc; } return LLDB_RECORD_RESULT(sb_sc); } diff --git a/lldb/source/API/SBTypeSummary.cpp b/lldb/source/API/SBTypeSummary.cpp index 3800ae940c703..2d7f8ef340c9b 100644 --- a/lldb/source/API/SBTypeSummary.cpp +++ b/lldb/source/API/SBTypeSummary.cpp @@ -100,20 +100,11 @@ const lldb_private::TypeSummaryOptions &SBTypeSummaryOptions::ref() const { } SBTypeSummaryOptions::SBTypeSummaryOptions( - const lldb_private::TypeSummaryOptions *lldb_object_ptr) { + const lldb_private::TypeSummaryOptions &lldb_object) + : m_opaque_up(std::make_unique<TypeSummaryOptions>(lldb_object)) { LLDB_RECORD_CONSTRUCTOR(SBTypeSummaryOptions, - (const lldb_private::TypeSummaryOptions *), - lldb_object_ptr); - - SetOptions(lldb_object_ptr); -} - -void SBTypeSummaryOptions::SetOptions( - const lldb_private::TypeSummaryOptions *lldb_object_ptr) { - if (lldb_object_ptr) - m_opaque_up = std::make_unique<TypeSummaryOptions>(*lldb_object_ptr); - else - m_opaque_up = std::make_unique<TypeSummaryOptions>(); + (const lldb_private::TypeSummaryOptions &), + lldb_object); } SBTypeSummary::SBTypeSummary() : m_opaque_sp() { @@ -175,7 +166,7 @@ SBTypeSummary SBTypeSummary::CreateWithCallback(FormatCallback cb, const TypeSummaryOptions &opt) -> bool { SBStream stream; SBValue sb_value(valobj.GetSP()); - SBTypeSummaryOptions options(&opt); + SBTypeSummaryOptions options(opt); if (!cb(sb_value, options, stream)) return false; stm.Write(stream.GetData(), stream.GetSize()); @@ -492,7 +483,7 @@ void RegisterMethods<SBTypeSummaryOptions>(Registry &R) { LLDB_REGISTER_METHOD(void, SBTypeSummaryOptions, SetCapping, (lldb::TypeSummaryCapping)); LLDB_REGISTER_CONSTRUCTOR(SBTypeSummaryOptions, - (const lldb_private::TypeSummaryOptions *)); + (const lldb_private::TypeSummaryOptions &)); } template <> _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits