[Lldb-commits] [lldb] f112791 - [lldb] Deobfuscate python-swigsafecast.swig

Pavel Labath via lldb-commits lldb-commits at lists.llvm.org
Thu Nov 18 10:27:37 PST 2021


Author: Pavel Labath
Date: 2021-11-18T19:27:09+01:00
New Revision: f1127914d3dcbfbe3bbd943ab84bea3805164295

URL: https://github.com/llvm/llvm-project/commit/f1127914d3dcbfbe3bbd943ab84bea3805164295
DIFF: https://github.com/llvm/llvm-project/commit/f1127914d3dcbfbe3bbd943ab84bea3805164295.diff

LOG: [lldb] Deobfuscate python-swigsafecast.swig

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.

Differential Revision: https://reviews.llvm.org/D114150

Added: 
    

Modified: 
    lldb/bindings/python/python-swigsafecast.swig
    lldb/bindings/python/python-wrapper.swig

Removed: 
    


################################################################################
diff  --git a/lldb/bindings/python/python-swigsafecast.swig b/lldb/bindings/python/python-swigsafecast.swig
index 091fc29b1057d..aa2bcfb8c8ae9 100644
--- a/lldb/bindings/python/python-swigsafecast.swig
+++ b/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);
 }

diff  --git a/lldb/bindings/python/python-wrapper.swig b/lldb/bindings/python/python-wrapper.swig
index 698d7d12cebca..6dc8ca1703905 100644
--- a/lldb/bindings/python/python-wrapper.swig
+++ b/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 @@ LLDBSwigPythonBreakpointCallbackFunction
         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 @@ LLDBSwigPythonCreateSyntheticProvider
     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 @@ LLDBSwigPythonCreateScriptedProcess
         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 @@ LLDBSwigPythonCreateScriptedProcess
 
     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 @@ LLDBSwigPythonCreateScriptedThread
         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 @@ LLDBSwigPythonCreateScriptedThread
 
     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 @@ LLDBSwigPythonCreateScriptedThreadPlan
         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 @@ LLDBSwigPythonCreateScriptedThreadPlan
         }
         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 @@ LLDBSwigPythonCreateScriptedBreakpointResolver
     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 @@ LLDBSwigPythonCreateScriptedStopHook
         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 @@ LLDBSwigPythonCallCommand
     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 @@ LLDBSwigPythonCallCommand
     }
     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 @@ LLDBSwigPythonCallCommandObject
     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 @@ LLDBSWIGPythonCreateOSPlugin
     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;
 


        


More information about the lldb-commits mailing list