From ca1d18d599dcc620bf526fb22042af95b1b60b68 Mon Sep 17 00:00:00 2001
From: Andreas Sandberg <andreas.sandberg@arm.com>
Date: Tue, 9 May 2017 19:28:47 +0100
Subject: [PATCH] python: Prevent Python wrappers from deleting SimObjects

The PyBind wrappers could potentially delete SimObjects if they don't
have any references. This is not desirable since there could be
pointers to such objects within the C++ world. This problem doesn't
normally occur since Python typically holds a pointer to the root node
as long as the simulator is running.

Prevent SimObject and Param deletion by using a PyBind-prescribed
unique_ptr with a dummy deleter as the pointer wrapper for the Python
world.

Change-Id: Ied14602c9ee69a083a69c5dae1b5fcf8efb4548a
Signed-off-by: Andreas Sandberg <andreas.sandberg@arm.com>
Reviewed-by: Curtis Dunham <curtis.dunham@arm.com>
Reviewed-on: https://gem5-review.googlesource.com/3224
Reviewed-by: Gabe Black <gabeblack@google.com>
---
 src/python/m5/SimObject.py  | 18 ++++++++++++------
 src/python/pybind11/core.cc |  5 +++--
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/src/python/m5/SimObject.py b/src/python/m5/SimObject.py
index 569142e34..baeef73d9 100644
--- a/src/python/m5/SimObject.py
+++ b/src/python/m5/SimObject.py
@@ -701,10 +701,13 @@ module_init(py::module &m_internal)
 ''')
         code.indent()
         if cls._base:
-            code('py::class_<${cls}Params, ${{cls._base.type}}Params>(m, ' \
-                 '"${cls}Params")')
+            code('py::class_<${cls}Params, ${{cls._base.type}}Params, ' \
+                 'std::unique_ptr<${{cls}}Params, py::nodelete>>(' \
+                 'm, "${cls}Params")')
         else:
-            code('py::class_<${cls}Params>(m, "${cls}Params")')
+            code('py::class_<${cls}Params, ' \
+                 'std::unique_ptr<${cls}Params, py::nodelete>>(' \
+                 'm, "${cls}Params")')
 
         code.indent()
         if not hasattr(cls, 'abstract') or not cls.abstract:
@@ -729,10 +732,13 @@ module_init(py::module &m_internal)
                 cls.cxx_bases
         if bases:
             base_str = ", ".join(bases)
-            code('py::class_<${{cls.cxx_class}}, ${base_str}>(m, ' \
-                 '"${py_class_name}")')
+            code('py::class_<${{cls.cxx_class}}, ${base_str}, ' \
+                 'std::unique_ptr<${{cls.cxx_class}}, py::nodelete>>(' \
+                 'm, "${py_class_name}")')
         else:
-            code('py::class_<${{cls.cxx_class}}>(m, "${py_class_name}")')
+            code('py::class_<${{cls.cxx_class}}, ' \
+                 'std::unique_ptr<${{cls.cxx_class}}, py::nodelete>>(' \
+                 'm, "${py_class_name}")')
         code.indent()
         for exp in cls.cxx_exports:
             exp.export(code, cls.cxx_class)
diff --git a/src/python/pybind11/core.cc b/src/python/pybind11/core.cc
index 7ad45b408..159b19f9d 100644
--- a/src/python/pybind11/core.cc
+++ b/src/python/pybind11/core.cc
@@ -132,7 +132,8 @@ init_serialize(py::module &m_native)
 {
     py::module m = m_native.def_submodule("serialize");
 
-    py::class_<Serializable>(m, "Serializable")
+    py::class_<Serializable, std::unique_ptr<Serializable, py::nodelete>>(
+        m, "Serializable")
         ;
 
     py::class_<CheckpointIn>(m, "CheckpointIn")
@@ -165,7 +166,7 @@ init_range(py::module &m_native)
         .def("isSubset", &AddrRange::isSubset)
         ;
 
-    // We need to make vectors of AddrRange opaque to avoid a weird
+    // We need to make vectors of AddrRange opaque to avoid weird
     // memory allocation issues in PyBind's STL wrappers.
     py::bind_vector<std::vector<AddrRange>>(m, "AddrRangeVector");
 
-- 
2.30.2