[Lldb-commits] [lldb] 2efc689 - [lldb/python] Avoid more dangling pointers in python glue code
Pavel Labath via lldb-commits
lldb-commits at lists.llvm.org
Wed Dec 22 04:47:18 PST 2021
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 <>
More information about the lldb-commits
mailing list