This revision was automatically updated to reflect the committed changes.
Closed by commit rG7f09ab08de5a: [lldb] Fix [some] leaks in python bindings 
(authored by labath).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114259/new/

https://reviews.llvm.org/D114259

Files:
  lldb/bindings/python/python-swigsafecast.swig
  lldb/bindings/python/python-wrapper.swig
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp

Index: lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
===================================================================
--- lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
@@ -129,7 +129,7 @@
 
 extern "C" void *LLDBSwigPythonCreateScriptedBreakpointResolver(
     const char *python_class_name, const char *session_dictionary_name,
-    lldb_private::StructuredDataImpl *args, lldb::BreakpointSP &bkpt_sp) {
+    lldb_private::StructuredDataImpl *args, const lldb::BreakpointSP &bkpt_sp) {
   return nullptr;
 }
 
@@ -248,7 +248,7 @@
 
 extern "C" bool LLDBSWIGPythonRunScriptKeywordProcess(
     const char *python_function_name, const char *session_dictionary_name,
-    lldb::ProcessSP &process, std::string &output) {
+    const lldb::ProcessSP &process, std::string &output) {
   return false;
 }
 
@@ -260,7 +260,7 @@
 
 extern "C" bool LLDBSWIGPythonRunScriptKeywordTarget(
     const char *python_function_name, const char *session_dictionary_name,
-    lldb::TargetSP &target, std::string &output) {
+    const lldb::TargetSP &target, std::string &output) {
   return false;
 }
 
@@ -272,7 +272,7 @@
 
 extern "C" bool LLDBSWIGPythonRunScriptKeywordValue(
     const char *python_function_name, const char *session_dictionary_name,
-    lldb::ValueObjectSP &value, std::string &output) {
+    const lldb::ValueObjectSP &value, std::string &output) {
   return false;
 }
 
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===================================================================
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -127,7 +127,7 @@
 
 extern "C" void *LLDBSwigPythonCreateScriptedBreakpointResolver(
     const char *python_class_name, const char *session_dictionary_name,
-    lldb_private::StructuredDataImpl *args, lldb::BreakpointSP &bkpt_sp);
+    lldb_private::StructuredDataImpl *args, const lldb::BreakpointSP &bkpt_sp);
 
 extern "C" unsigned int
 LLDBSwigPythonCallBreakpointResolver(void *implementor, const char *method_name,
@@ -196,7 +196,7 @@
 
 extern "C" bool LLDBSWIGPythonRunScriptKeywordProcess(
     const char *python_function_name, const char *session_dictionary_name,
-    lldb::ProcessSP &process, std::string &output);
+    const lldb::ProcessSP &process, std::string &output);
 
 extern "C" bool LLDBSWIGPythonRunScriptKeywordThread(
     const char *python_function_name, const char *session_dictionary_name,
@@ -204,7 +204,7 @@
 
 extern "C" bool LLDBSWIGPythonRunScriptKeywordTarget(
     const char *python_function_name, const char *session_dictionary_name,
-    lldb::TargetSP &target, std::string &output);
+    const lldb::TargetSP &target, std::string &output);
 
 extern "C" bool LLDBSWIGPythonRunScriptKeywordFrame(
     const char *python_function_name, const char *session_dictionary_name,
@@ -212,7 +212,7 @@
 
 extern "C" bool LLDBSWIGPythonRunScriptKeywordValue(
     const char *python_function_name, const char *session_dictionary_name,
-    lldb::ValueObjectSP &value, std::string &output);
+    const lldb::ValueObjectSP &value, std::string &output);
 
 extern "C" void *
 LLDBSWIGPython_GetDynamicSetting(void *module, const char *setting,
@@ -2653,11 +2653,11 @@
   }
 
   {
-    ProcessSP process_sp(process->shared_from_this());
     Locker py_lock(this,
                    Locker::AcquireLock | Locker::InitSession | Locker::NoSTDIN);
     ret_val = LLDBSWIGPythonRunScriptKeywordProcess(
-        impl_function, m_dictionary_name.c_str(), process_sp, output);
+        impl_function, m_dictionary_name.c_str(), process->shared_from_this(),
+        output);
     if (!ret_val)
       error.SetErrorString("python script evaluation failed");
   }
@@ -2753,11 +2753,10 @@
   }
 
   {
-    ValueObjectSP value_sp(value->GetSP());
     Locker py_lock(this,
                    Locker::AcquireLock | Locker::InitSession | Locker::NoSTDIN);
     ret_val = LLDBSWIGPythonRunScriptKeywordValue(
-        impl_function, m_dictionary_name.c_str(), value_sp, output);
+        impl_function, m_dictionary_name.c_str(), value->GetSP(), output);
     if (!ret_val)
       error.SetErrorString("python script evaluation failed");
   }
Index: lldb/bindings/python/python-wrapper.swig
===================================================================
--- lldb/bindings/python/python-wrapper.swig
+++ lldb/bindings/python/python-wrapper.swig
@@ -145,7 +145,6 @@
     std::string& retval
 )
 {
-    lldb::SBValue sb_value (valobj_sp);
     lldb::SBTypeSummaryOptions sb_options(options_sp.get());
 
     retval.clear();
@@ -195,7 +194,7 @@
         return false;
     }
 
-    PythonObject value_arg(PyRefType::Owned, SBTypeToSWIGWrapper(sb_value));
+    PythonObject value_arg = ToSWIGWrapper(valobj_sp);
     PythonObject options_arg(PyRefType::Owned, SBTypeToSWIGWrapper(sb_options));
 
     if (argc.get().max_positional_args < 3)
@@ -227,11 +226,10 @@
     if (!pfunc.IsAllocated())
         Py_RETURN_NONE;
 
-    // FIXME: SBValue leaked here
-    lldb::SBValue *sb_value = new lldb::SBValue(valobj_sp);
+    auto sb_value = std::make_unique<lldb::SBValue>(valobj_sp);
     sb_value->SetPreferSyntheticValue(false);
 
-    PythonObject val_arg(PyRefType::Owned, SBTypeToSWIGWrapper(*sb_value));
+    PythonObject val_arg = ToSWIGWrapper(std::move(sb_value));
     if (!val_arg.IsAllocated())
         Py_RETURN_NONE;
 
@@ -295,12 +293,7 @@
         return nullptr;
     }
 
-    // FIXME: SBTarget leaked here
-    PythonObject target_arg(
-        PyRefType::Owned, SBTypeToSWIGWrapper(*new lldb::SBTarget(target_sp)));
-
-    if (!target_arg.IsAllocated())
-        Py_RETURN_NONE;
+    PythonObject target_arg = ToSWIGWrapper(target_sp);
 
     llvm::Expected<PythonCallable::ArgInfo> arg_info = pfunc.GetArgInfo();
     if (!arg_info) {
@@ -354,14 +347,6 @@
         return nullptr;
     }
 
-    // FIXME: This leaks the SBProcess object
-    PythonObject process_arg(
-        PyRefType::Owned,
-        SBTypeToSWIGWrapper(*new lldb::SBProcess(process_sp)));
-
-    if (!process_arg.IsAllocated())
-        Py_RETURN_NONE;
-
     llvm::Expected<PythonCallable::ArgInfo> arg_info = pfunc.GetArgInfo();
     if (!arg_info) {
         llvm::handleAllErrors(
@@ -379,7 +364,7 @@
     if (arg_info.get().max_positional_args == 2) {
         // FIXME: SBStructuredData leaked here
         PythonObject args_arg(PyRefType::Owned, SBTypeToSWIGWrapper(*new lldb::SBStructuredData(args_impl)));
-        result = pfunc(process_arg, args_arg);
+        result = pfunc(ToSWIGWrapper(process_sp), args_arg);
     } else {
         error_string.assign("wrong number of arguments in __init__, should be 2 (not including self)");
         Py_RETURN_NONE;
@@ -415,13 +400,7 @@
         return nullptr;
     }
 
-    // FIXME: SBThreadPlan leaked here
-    PythonObject tp_arg(
-        PyRefType::Owned,
-        SBTypeToSWIGWrapper(*new lldb::SBThreadPlan(thread_plan_sp)));
-
-    if (!tp_arg.IsAllocated())
-        Py_RETURN_NONE;
+    PythonObject tp_arg = ToSWIGWrapper(thread_plan_sp);
 
     llvm::Expected<PythonCallable::ArgInfo> arg_info = pfunc.GetArgInfo();
     if (!arg_info) {
@@ -507,15 +486,11 @@
     return false;
 }
 
-SWIGEXPORT void *
-LLDBSwigPythonCreateScriptedBreakpointResolver
-(
-    const char *python_class_name,
-    const char *session_dictionary_name,
+SWIGEXPORT void *LLDBSwigPythonCreateScriptedBreakpointResolver(
+    const char *python_class_name, const char *session_dictionary_name,
     lldb_private::StructuredDataImpl *args_impl,
-    lldb::BreakpointSP &breakpoint_sp
-)
-{
+    const lldb::BreakpointSP &breakpoint_sp) {
+
     if (python_class_name == NULL || python_class_name[0] == '\0' || !session_dictionary_name)
         Py_RETURN_NONE;
 
@@ -527,16 +502,11 @@
     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));
-
     // FIXME: SBStructuredData leaked here
     lldb::SBStructuredData *args_value = new lldb::SBStructuredData(args_impl);
     PythonObject args_arg(PyRefType::Owned, SBTypeToSWIGWrapper(*args_value));
 
-    PythonObject result = pfunc(bkpt_arg, args_arg, dict);
+    PythonObject result = pfunc(ToSWIGWrapper(breakpoint_sp), args_arg, dict);
     // FIXME: At this point we should check that the class we found supports all the methods
     // that we need.
 
@@ -637,16 +607,11 @@
         return nullptr;
     }
 
-    // FIXME: SBTarget leaked here
-    lldb::SBTarget *target_val 
-        = new lldb::SBTarget(target_sp);
-    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 result = pfunc(target_arg, args_arg, dict);
+    PythonObject result = pfunc(ToSWIGWrapper(target_sp), args_arg, dict);
 
     if (result.IsAllocated())
     {
@@ -1076,13 +1041,7 @@
     if (!pfunc.IsAllocated())
         Py_RETURN_NONE;
 
-    // FIXME: This leaks the SBProcess object
-    lldb::SBProcess *process_sb = new lldb::SBProcess(process_sp);
-    PythonObject process_arg(PyRefType::Owned, SBTypeToSWIGWrapper(*process_sb));
-    if (!process_arg.IsAllocated())
-        Py_RETURN_NONE;
-
-    auto result = pfunc(process_arg);
+    auto result = pfunc(ToSWIGWrapper(process_sp));
 
     if (result.IsAllocated())
         return result.release();
@@ -1147,21 +1106,15 @@
     if (!pfunc.IsAllocated())
         Py_RETURN_NONE;
 
-    lldb::SBTarget target_sb(target_sp);
-    PythonObject target_arg(PyRefType::Owned, SBTypeToSWIGWrapper(target_sb));
-    auto result = pfunc(target_arg, PythonString(setting));
+    auto result = pfunc(ToSWIGWrapper(target_sp), PythonString(setting));
 
     return result.release();
 }
 
-SWIGEXPORT bool
-LLDBSWIGPythonRunScriptKeywordProcess
-(const char* python_function_name,
-const char* session_dictionary_name,
-lldb::ProcessSP& process,
-std::string& output)
+SWIGEXPORT bool LLDBSWIGPythonRunScriptKeywordProcess(
+    const char *python_function_name, const char *session_dictionary_name,
+    const lldb::ProcessSP &process, std::string &output) {
 
-{
     if (python_function_name == NULL || python_function_name[0] == '\0' || !session_dictionary_name)
         return false;
 
@@ -1173,9 +1126,7 @@
     if (!pfunc.IsAllocated())
         return false;
 
-    lldb::SBProcess process_sb(process);
-    PythonObject process_arg(PyRefType::Owned, SBTypeToSWIGWrapper(process_sb));
-    auto result = pfunc(process_arg, dict);
+    auto result = pfunc(ToSWIGWrapper(process), dict);
 
     output = result.Str().GetString().str();
 
@@ -1210,14 +1161,10 @@
     return true;
 }
 
-SWIGEXPORT bool
-LLDBSWIGPythonRunScriptKeywordTarget
-(const char* python_function_name,
-const char* session_dictionary_name,
-lldb::TargetSP& target,
-std::string& output)
+SWIGEXPORT bool LLDBSWIGPythonRunScriptKeywordTarget(
+    const char *python_function_name, const char *session_dictionary_name,
+    const lldb::TargetSP &target, std::string &output) {
 
-{
     if (python_function_name == NULL || python_function_name[0] == '\0' || !session_dictionary_name)
         return false;
 
@@ -1229,9 +1176,7 @@
     if (!pfunc.IsAllocated())
         return false;
 
-    lldb::SBTarget target_sb(target);
-    PythonObject target_arg(PyRefType::Owned, SBTypeToSWIGWrapper(target_sb));
-    auto result = pfunc(target_arg, dict);
+    auto result = pfunc(ToSWIGWrapper(target), dict);
 
     output = result.Str().GetString().str();
 
@@ -1266,14 +1211,10 @@
     return true;
 }
 
-SWIGEXPORT bool
-LLDBSWIGPythonRunScriptKeywordValue
-(const char* python_function_name,
-const char* session_dictionary_name,
-lldb::ValueObjectSP& value,
-std::string& output)
+SWIGEXPORT bool LLDBSWIGPythonRunScriptKeywordValue(
+    const char *python_function_name, const char *session_dictionary_name,
+    const lldb::ValueObjectSP &value, std::string &output) {
 
-{
     if (python_function_name == NULL || python_function_name[0] == '\0' || !session_dictionary_name)
         return false;
 
@@ -1285,9 +1226,7 @@
     if (!pfunc.IsAllocated())
         return false;
 
-    lldb::SBValue value_sb(value);
-    PythonObject value_arg(PyRefType::Owned, SBTypeToSWIGWrapper(value_sb));
-    auto result = pfunc(value_arg, dict);
+    auto result = pfunc(ToSWIGWrapper(value), dict);
 
     output = result.Str().GetString().str();
 
Index: lldb/bindings/python/python-swigsafecast.swig
===================================================================
--- lldb/bindings/python/python-swigsafecast.swig
+++ lldb/bindings/python/python-swigsafecast.swig
@@ -1,23 +1,14 @@
+namespace lldb_private {
+namespace python {
+
 PyObject *SBTypeToSWIGWrapper(lldb::SBEvent &event_sb) {
   return SWIG_NewPointerObj(&event_sb, SWIGTYPE_p_lldb__SBEvent, 0);
 }
 
-PyObject *SBTypeToSWIGWrapper(lldb::SBProcess &process_sb) {
-  return SWIG_NewPointerObj(&process_sb, SWIGTYPE_p_lldb__SBProcess, 0);
-}
-
 PyObject *SBTypeToSWIGWrapper(lldb::SBThread &thread_sb) {
   return SWIG_NewPointerObj(&thread_sb, SWIGTYPE_p_lldb__SBThread, 0);
 }
 
-PyObject *SBTypeToSWIGWrapper(lldb::SBThreadPlan &thread_plan_sb) {
-  return SWIG_NewPointerObj(&thread_plan_sb, SWIGTYPE_p_lldb__SBThreadPlan, 0);
-}
-
-PyObject *SBTypeToSWIGWrapper(lldb::SBTarget &target_sb) {
-  return SWIG_NewPointerObj(&target_sb, SWIGTYPE_p_lldb__SBTarget, 0);
-}
-
 PyObject *SBTypeToSWIGWrapper(lldb::SBFrame &frame_sb) {
   return SWIG_NewPointerObj(&frame_sb, SWIGTYPE_p_lldb__SBFrame, 0);
 }
@@ -26,10 +17,6 @@
   return SWIG_NewPointerObj(&debugger_sb, SWIGTYPE_p_lldb__SBDebugger, 0);
 }
 
-PyObject *SBTypeToSWIGWrapper(lldb::SBBreakpoint &breakpoint_sb) {
-  return SWIG_NewPointerObj(&breakpoint_sb, SWIGTYPE_p_lldb__SBBreakpoint, 0);
-}
-
 PyObject *SBTypeToSWIGWrapper(lldb::SBWatchpoint &watchpoint_sb) {
   return SWIG_NewPointerObj(&watchpoint_sb, SWIGTYPE_p_lldb__SBWatchpoint, 0);
 }
@@ -40,10 +27,6 @@
                             SWIGTYPE_p_lldb__SBBreakpointLocation, 0);
 }
 
-PyObject *SBTypeToSWIGWrapper(lldb::SBValue &value_sb) {
-  return SWIG_NewPointerObj(&value_sb, SWIGTYPE_p_lldb__SBValue, 0);
-}
-
 PyObject *SBTypeToSWIGWrapper(lldb::SBCommandReturnObject &cmd_ret_obj_sb) {
   return SWIG_NewPointerObj(&cmd_ret_obj_sb,
                             SWIGTYPE_p_lldb__SBCommandReturnObject, 0);
@@ -70,3 +53,38 @@
 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)};
+}
+
+PythonObject ToSWIGWrapper(std::unique_ptr<lldb::SBValue> value_sb) {
+  return ToSWIGHelper(value_sb.release(), SWIGTYPE_p_lldb__SBValue);
+}
+
+PythonObject ToSWIGWrapper(lldb::ValueObjectSP value_sp) {
+  return ToSWIGWrapper(std::make_unique<lldb::SBValue>(std::move(value_sp)));
+}
+
+PythonObject ToSWIGWrapper(lldb::TargetSP target_sp) {
+  return ToSWIGHelper(new lldb::SBTarget(std::move(target_sp)),
+                      SWIGTYPE_p_lldb__SBTarget);
+}
+
+PythonObject ToSWIGWrapper(lldb::ProcessSP process_sp) {
+  return ToSWIGHelper(new lldb::SBProcess(std::move(process_sp)),
+                      SWIGTYPE_p_lldb__SBProcess);
+}
+
+PythonObject ToSWIGWrapper(lldb::ThreadPlanSP thread_plan_sp) {
+  return ToSWIGHelper(new lldb::SBThreadPlan(std::move(thread_plan_sp)),
+                      SWIGTYPE_p_lldb__SBThreadPlan);
+}
+
+PythonObject ToSWIGWrapper(lldb::BreakpointSP breakpoint_sp) {
+  return ToSWIGHelper(new lldb::SBBreakpoint(std::move(breakpoint_sp)),
+                      SWIGTYPE_p_lldb__SBBreakpoint);
+}
+
+} // namespace python
+} // namespace lldb_private
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to