labath created this revision.
labath added reviewers: JDevlieghere, jingham.
labath requested review of this revision.
Herald added a project: LLDB.
This file was way more complicated than it needed to be.
This patch removes the automagic reference-to-pointer delegation and
replaces the template specializations with regular free functions
(taking reference arguments).
The reason I chose references is twofold:
- there are more arguments being passed by reference than by pointer
- the reference arguments make it more obvious that there is a lot of leaking
going on in there.
Currently, the code was assuming that the pointer arguments have some
kind of a special meaning and that pointer functions take ownership of
their arguments, which isn't true (it's possible it was true at some
point in the past, I haven't done the archeology).
This makes it easier to implement proper lifetime management in
follow-up patches.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D114150
Files:
lldb/bindings/python/python-swigsafecast.swig
lldb/bindings/python/python-wrapper.swig
Index: lldb/bindings/python/python-wrapper.swig
===================================================================
--- lldb/bindings/python/python-wrapper.swig
+++ lldb/bindings/python/python-wrapper.swig
@@ -1,9 +1,5 @@
%header %{
-template <typename T>
-PyObject *
-SBTypeToSWIGWrapper (T* item);
-
class PyErr_Cleaner
{
public:
@@ -83,8 +79,9 @@
if (max_positional_args < 4) {
return pfunc.Call(frame_arg, bp_loc_arg, dict);
} else {
+ // FIXME: SBStructuredData leaked here
lldb::SBStructuredData *args_value = new lldb::SBStructuredData(args_impl);
- PythonObject args_arg(PyRefType::Owned, SBTypeToSWIGWrapper(args_value));
+ PythonObject args_arg(PyRefType::Owned, SBTypeToSWIGWrapper(*args_value));
return pfunc.Call(frame_arg, bp_loc_arg, args_arg, dict);
}
} ();
@@ -230,12 +227,11 @@
if (!pfunc.IsAllocated())
Py_RETURN_NONE;
- // I do not want the SBValue to be deallocated when going out of scope because python
- // has ownership of it and will manage memory for this object by itself
+ // FIXME: SBValue leaked here
lldb::SBValue *sb_value = new lldb::SBValue(valobj_sp);
sb_value->SetPreferSyntheticValue(false);
- PythonObject val_arg(PyRefType::Owned, SBTypeToSWIGWrapper(sb_value));
+ PythonObject val_arg(PyRefType::Owned, SBTypeToSWIGWrapper(*sb_value));
if (!val_arg.IsAllocated())
Py_RETURN_NONE;
@@ -299,10 +295,9 @@
return nullptr;
}
- // I do not want the SBTarget to be deallocated when going out of scope
- // because python has ownership of it and will manage memory for this
- // object by itself
- PythonObject target_arg(PyRefType::Owned, SBTypeToSWIGWrapper(new lldb::SBTarget(target_sp)));
+ // FIXME: SBTarget leaked here
+ PythonObject target_arg(
+ PyRefType::Owned, SBTypeToSWIGWrapper(*new lldb::SBTarget(target_sp)));
if (!target_arg.IsAllocated())
Py_RETURN_NONE;
@@ -322,7 +317,8 @@
PythonObject result = {};
if (arg_info.get().max_positional_args == 2) {
- PythonObject args_arg(PyRefType::Owned, SBTypeToSWIGWrapper(new lldb::SBStructuredData(args_impl)));
+ // FIXME: SBStructuredData leaked here
+ PythonObject args_arg(PyRefType::Owned, SBTypeToSWIGWrapper(*new lldb::SBStructuredData(args_impl)));
result = pfunc(target_arg, args_arg);
} else {
error_string.assign("wrong number of arguments in __init__, should be 2 (not including self)");
@@ -358,10 +354,10 @@
return nullptr;
}
- // I do not want the SBProcess to be deallocated when going out of scope
- // because python has ownership of it and will manage memory for this
- // object by itself
- PythonObject process_arg(PyRefType::Owned, SBTypeToSWIGWrapper(new lldb::SBProcess(process_sp)));
+ // FIXME: This leaks the SBProcess object
+ PythonObject process_arg(
+ PyRefType::Owned,
+ SBTypeToSWIGWrapper(*new lldb::SBProcess(process_sp)));
if (!process_arg.IsAllocated())
Py_RETURN_NONE;
@@ -381,7 +377,8 @@
PythonObject result = {};
if (arg_info.get().max_positional_args == 2) {
- PythonObject args_arg(PyRefType::Owned, SBTypeToSWIGWrapper(new lldb::SBStructuredData(args_impl)));
+ // FIXME: SBStructuredData leaked here
+ PythonObject args_arg(PyRefType::Owned, SBTypeToSWIGWrapper(*new lldb::SBStructuredData(args_impl)));
result = pfunc(process_arg, args_arg);
} else {
error_string.assign("wrong number of arguments in __init__, should be 2 (not including self)");
@@ -418,10 +415,10 @@
return nullptr;
}
- // I do not want the SBThreadPlan to be deallocated when going out of scope
- // because python has ownership of it and will manage memory for this
- // object by itself
- PythonObject tp_arg(PyRefType::Owned, SBTypeToSWIGWrapper(new lldb::SBThreadPlan(thread_plan_sp)));
+ // FIXME: SBThreadPlan leaked here
+ PythonObject tp_arg(
+ PyRefType::Owned,
+ SBTypeToSWIGWrapper(*new lldb::SBThreadPlan(thread_plan_sp)));
if (!tp_arg.IsAllocated())
Py_RETURN_NONE;
@@ -447,7 +444,8 @@
}
result = pfunc(tp_arg, dict);
} else if (arg_info.get().max_positional_args >= 3) {
- PythonObject args_arg(PyRefType::Owned, SBTypeToSWIGWrapper(new lldb::SBStructuredData(args_impl)));
+ // FIXME: SBStructuredData leaked here
+ PythonObject args_arg(PyRefType::Owned, SBTypeToSWIGWrapper(*new lldb::SBStructuredData(args_impl)));
result = pfunc(tp_arg, args_arg, dict);
} else {
error_string.assign("wrong number of arguments in __init__, should be 2 or 3 (not including self)");
@@ -529,12 +527,14 @@
if (!pfunc.IsAllocated())
return nullptr;
+ // FIXME: SBBreakpoint leaked here
lldb::SBBreakpoint *bkpt_value = new lldb::SBBreakpoint(breakpoint_sp);
- PythonObject bkpt_arg(PyRefType::Owned, SBTypeToSWIGWrapper(bkpt_value));
+ PythonObject bkpt_arg(PyRefType::Owned, SBTypeToSWIGWrapper(*bkpt_value));
+ // FIXME: SBStructuredData leaked here
lldb::SBStructuredData *args_value = new lldb::SBStructuredData(args_impl);
- PythonObject args_arg(PyRefType::Owned, SBTypeToSWIGWrapper(args_value));
+ PythonObject args_arg(PyRefType::Owned, SBTypeToSWIGWrapper(*args_value));
PythonObject result = pfunc(bkpt_arg, args_arg, dict);
// FIXME: At this point we should check that the class we found supports all the methods
@@ -637,13 +637,14 @@
return nullptr;
}
+ // FIXME: SBTarget leaked here
lldb::SBTarget *target_val
= new lldb::SBTarget(target_sp);
+ PythonObject target_arg(PyRefType::Owned, SBTypeToSWIGWrapper(*target_val));
- PythonObject target_arg(PyRefType::Owned, SBTypeToSWIGWrapper(target_val));
-
+ // FIXME: SBStructuredData leaked here
lldb::SBStructuredData *args_value = new lldb::SBStructuredData(args_impl);
- PythonObject args_arg(PyRefType::Owned, SBTypeToSWIGWrapper(args_value));
+ PythonObject args_arg(PyRefType::Owned, SBTypeToSWIGWrapper(*args_value));
PythonObject result = pfunc(target_arg, args_arg, dict);
@@ -1008,8 +1009,6 @@
if (!pfunc.IsAllocated())
return false;
- // pass the pointer-to cmd_retobj_sb or watch the underlying object disappear from under you
- // see comment above for SBCommandReturnObjectReleaser for further details
auto argc = pfunc.GetArgInfo();
if (!argc) {
llvm::consumeError(argc.takeError());
@@ -1017,7 +1016,7 @@
}
PythonObject debugger_arg(PyRefType::Owned, SBTypeToSWIGWrapper(debugger_sb));
PythonObject exe_ctx_arg(PyRefType::Owned, SBTypeToSWIGWrapper(exe_ctx_sb));
- PythonObject cmd_retobj_arg(PyRefType::Owned, SBTypeToSWIGWrapper(&cmd_retobj_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);
@@ -1049,11 +1048,9 @@
if (!pfunc.IsAllocated())
return false;
- // pass the pointer-to cmd_retobj_sb or watch the underlying object disappear from under you
- // see comment above for SBCommandReturnObjectReleaser for further details
PythonObject debugger_arg(PyRefType::Owned, SBTypeToSWIGWrapper(debugger_sb));
PythonObject exe_ctx_arg(PyRefType::Owned, SBTypeToSWIGWrapper(exe_ctx_sb));
- PythonObject cmd_retobj_arg(PyRefType::Owned, SBTypeToSWIGWrapper(&cmd_retobj_sb));
+ PythonObject cmd_retobj_arg(PyRefType::Owned, SBTypeToSWIGWrapper(cmd_retobj_sb));
pfunc(debugger_arg, PythonString(args), exe_ctx_arg, cmd_retobj_arg);
@@ -1079,10 +1076,9 @@
if (!pfunc.IsAllocated())
Py_RETURN_NONE;
- // I do not want the SBProcess to be deallocated when going out of scope because python
- // has ownership of it and will manage memory for this object by itself
+ // FIXME: This leaks the SBProcess object
lldb::SBProcess *process_sb = new lldb::SBProcess(process_sp);
- PythonObject process_arg(PyRefType::Owned, SBTypeToSWIGWrapper(process_sb));
+ PythonObject process_arg(PyRefType::Owned, SBTypeToSWIGWrapper(*process_sb));
if (!process_arg.IsAllocated())
Py_RETURN_NONE;
Index: lldb/bindings/python/python-swigsafecast.swig
===================================================================
--- lldb/bindings/python/python-swigsafecast.swig
+++ lldb/bindings/python/python-swigsafecast.swig
@@ -1,161 +1,72 @@
-// leaving this undefined ensures we will get a linker error if we try to use SBTypeToSWIGWrapper()
-// for a type for which we did not specialze this function
-template <typename SBClass>
-PyObject*
-SBTypeToSWIGWrapper (SBClass* sb_object);
-
-template <typename SBClass>
-PyObject*
-SBTypeToSWIGWrapper (SBClass& sb_object)
-{
- return SBTypeToSWIGWrapper(&sb_object);
-}
-
-template <typename SBClass>
-PyObject*
-SBTypeToSWIGWrapper (const SBClass& sb_object)
-{
- return SBTypeToSWIGWrapper(&sb_object);
-}
-
-template <>
-PyObject*
-SBTypeToSWIGWrapper (PyObject* py_object)
-{
- return py_object;
-}
-
-template <>
-PyObject*
-SBTypeToSWIGWrapper (unsigned int* c_int)
-{
- if (!c_int)
- return NULL;
- return PyInt_FromLong(*c_int);
-}
-
-template <>
-PyObject*
-SBTypeToSWIGWrapper (lldb::SBEvent* event_sb)
-{
- return SWIG_NewPointerObj((void *) event_sb, SWIGTYPE_p_lldb__SBEvent, 0);
-}
-
-template <>
-PyObject*
-SBTypeToSWIGWrapper (lldb::SBProcess* process_sb)
-{
- return SWIG_NewPointerObj((void *) process_sb, SWIGTYPE_p_lldb__SBProcess, 0);
+PyObject *SBTypeToSWIGWrapper(lldb::SBEvent &event_sb) {
+ return SWIG_NewPointerObj(&event_sb, SWIGTYPE_p_lldb__SBEvent, 0);
}
-template <>
-PyObject*
-SBTypeToSWIGWrapper (lldb::SBThread* thread_sb)
-{
- return SWIG_NewPointerObj((void *) thread_sb, SWIGTYPE_p_lldb__SBThread, 0);
+PyObject *SBTypeToSWIGWrapper(lldb::SBProcess &process_sb) {
+ return SWIG_NewPointerObj(&process_sb, SWIGTYPE_p_lldb__SBProcess, 0);
}
-template <>
-PyObject*
-SBTypeToSWIGWrapper (lldb::SBThreadPlan* thread_plan_sb)
-{
- return SWIG_NewPointerObj((void *) thread_plan_sb, SWIGTYPE_p_lldb__SBThreadPlan, 0);
+PyObject *SBTypeToSWIGWrapper(lldb::SBThread &thread_sb) {
+ return SWIG_NewPointerObj(&thread_sb, SWIGTYPE_p_lldb__SBThread, 0);
}
-template <>
-PyObject*
-SBTypeToSWIGWrapper (lldb::SBTarget* target_sb)
-{
- return SWIG_NewPointerObj((void *) target_sb, SWIGTYPE_p_lldb__SBTarget, 0);
+PyObject *SBTypeToSWIGWrapper(lldb::SBThreadPlan &thread_plan_sb) {
+ return SWIG_NewPointerObj(&thread_plan_sb, SWIGTYPE_p_lldb__SBThreadPlan, 0);
}
-template <>
-PyObject*
-SBTypeToSWIGWrapper (lldb::SBFrame* frame_sb)
-{
- return SWIG_NewPointerObj((void *) frame_sb, SWIGTYPE_p_lldb__SBFrame, 0);
+PyObject *SBTypeToSWIGWrapper(lldb::SBTarget &target_sb) {
+ return SWIG_NewPointerObj(&target_sb, SWIGTYPE_p_lldb__SBTarget, 0);
}
-template <>
-PyObject*
-SBTypeToSWIGWrapper (lldb::SBDebugger* debugger_sb)
-{
- return SWIG_NewPointerObj((void *) debugger_sb, SWIGTYPE_p_lldb__SBDebugger, 0);
+PyObject *SBTypeToSWIGWrapper(lldb::SBFrame &frame_sb) {
+ return SWIG_NewPointerObj(&frame_sb, SWIGTYPE_p_lldb__SBFrame, 0);
}
-template <>
-PyObject*
-SBTypeToSWIGWrapper (lldb::SBBreakpoint* breakpoint_sb)
-{
- return SWIG_NewPointerObj((void *) breakpoint_sb, SWIGTYPE_p_lldb__SBBreakpoint, 0);
+PyObject *SBTypeToSWIGWrapper(lldb::SBDebugger &debugger_sb) {
+ return SWIG_NewPointerObj(&debugger_sb, SWIGTYPE_p_lldb__SBDebugger, 0);
}
-template <>
-PyObject*
-SBTypeToSWIGWrapper (lldb::SBWatchpoint* watchpoint_sb)
-{
- return SWIG_NewPointerObj((void *) watchpoint_sb, SWIGTYPE_p_lldb__SBWatchpoint, 0);
+PyObject *SBTypeToSWIGWrapper(lldb::SBBreakpoint &breakpoint_sb) {
+ return SWIG_NewPointerObj(&breakpoint_sb, SWIGTYPE_p_lldb__SBBreakpoint, 0);
}
-template <>
-PyObject*
-SBTypeToSWIGWrapper (lldb::SBBreakpointLocation* breakpoint_location_sb)
-{
- return SWIG_NewPointerObj((void *) breakpoint_location_sb, SWIGTYPE_p_lldb__SBBreakpointLocation, 0);
+PyObject *SBTypeToSWIGWrapper(lldb::SBWatchpoint &watchpoint_sb) {
+ return SWIG_NewPointerObj(&watchpoint_sb, SWIGTYPE_p_lldb__SBWatchpoint, 0);
}
-template <>
-PyObject*
-SBTypeToSWIGWrapper (lldb::SBBreakpointName* breakpoint_name_sb)
-{
- return SWIG_NewPointerObj((void *) breakpoint_name_sb, SWIGTYPE_p_lldb__SBBreakpointName, 0);
+PyObject *
+SBTypeToSWIGWrapper(lldb::SBBreakpointLocation &breakpoint_location_sb) {
+ return SWIG_NewPointerObj(&breakpoint_location_sb,
+ SWIGTYPE_p_lldb__SBBreakpointLocation, 0);
}
-template <>
-PyObject*
-SBTypeToSWIGWrapper (lldb::SBValue* value_sb)
-{
- return SWIG_NewPointerObj((void *) value_sb, SWIGTYPE_p_lldb__SBValue, 0);
+PyObject *SBTypeToSWIGWrapper(lldb::SBValue &value_sb) {
+ return SWIG_NewPointerObj(&value_sb, SWIGTYPE_p_lldb__SBValue, 0);
}
-template <>
-PyObject*
-SBTypeToSWIGWrapper (lldb::SBCommandReturnObject* cmd_ret_obj_sb)
-{
- return SWIG_NewPointerObj((void *) cmd_ret_obj_sb, SWIGTYPE_p_lldb__SBCommandReturnObject, 0);
+PyObject *SBTypeToSWIGWrapper(lldb::SBCommandReturnObject &cmd_ret_obj_sb) {
+ return SWIG_NewPointerObj(&cmd_ret_obj_sb,
+ SWIGTYPE_p_lldb__SBCommandReturnObject, 0);
}
-template <>
-PyObject*
-SBTypeToSWIGWrapper (lldb::SBExecutionContext* ctx_sb)
-{
- return SWIG_NewPointerObj((void *) ctx_sb, SWIGTYPE_p_lldb__SBExecutionContext, 0);
+PyObject *SBTypeToSWIGWrapper(lldb::SBExecutionContext &ctx_sb) {
+ return SWIG_NewPointerObj(&ctx_sb, SWIGTYPE_p_lldb__SBExecutionContext, 0);
}
-template <>
-PyObject*
-SBTypeToSWIGWrapper (lldb::SBTypeSummaryOptions* summary_options_sb)
-{
- return SWIG_NewPointerObj((void *) summary_options_sb, SWIGTYPE_p_lldb__SBTypeSummaryOptions, 0);
+PyObject *SBTypeToSWIGWrapper(lldb::SBTypeSummaryOptions &summary_options_sb) {
+ return SWIG_NewPointerObj(&summary_options_sb,
+ SWIGTYPE_p_lldb__SBTypeSummaryOptions, 0);
}
-template <>
-PyObject*
-SBTypeToSWIGWrapper (lldb::SBStructuredData* structured_data_sb)
-{
- return SWIG_NewPointerObj((void *) structured_data_sb, SWIGTYPE_p_lldb__SBStructuredData, 0);
+PyObject *SBTypeToSWIGWrapper(lldb::SBStructuredData &structured_data_sb) {
+ return SWIG_NewPointerObj(&structured_data_sb,
+ SWIGTYPE_p_lldb__SBStructuredData, 0);
}
-template <>
-PyObject*
-SBTypeToSWIGWrapper (lldb::SBSymbolContext* sym_ctx_sb)
-{
- return SWIG_NewPointerObj((void *) sym_ctx_sb, SWIGTYPE_p_lldb__SBSymbolContext, 0);
+PyObject *SBTypeToSWIGWrapper(lldb::SBSymbolContext &sym_ctx_sb) {
+ return SWIG_NewPointerObj(&sym_ctx_sb, SWIGTYPE_p_lldb__SBSymbolContext, 0);
}
-template <>
-PyObject*
-SBTypeToSWIGWrapper (lldb::SBStream* stream_sb)
-{
- return SWIG_NewPointerObj((void *) stream_sb, SWIGTYPE_p_lldb__SBStream, 0);
+PyObject *SBTypeToSWIGWrapper(lldb::SBStream &stream_sb) {
+ return SWIG_NewPointerObj(&stream_sb, SWIGTYPE_p_lldb__SBStream, 0);
}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
