[binutils-gdb] Don't steal references in the gdb Python code

[Tom Tromey <tromey at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=1a3389079dd5c93419846f44d42027a526ce19cf

commit 1a3389079dd5c93419846f44d42027a526ce19cf
Author: Tom Tromey <tom@tromey.com>
Date:   Sun Sep 16 07:32:23 2018 -0600

    Don't steal references in the gdb Python code
    
    Some Python APIs steal references from their caller, and the refcount
    checker supports this via an attribute.
    
    However, in gdb with C++ we have a better idiom available: we can use
    std::move on a gdbpy_ref<> instead.  This makes the semantics obvious
    at the point of call, and is safer at runtime as well, because the
    callee's gdbpy_ref<> will be emptied.
    
    This patch changes the reference-stealing code in gdb to use rvalue
    references instead.
    
    Tested on x86-64 Fedora 28.
    
    gdb/ChangeLog
    2018-09-16  Tom Tromey  <tom@tromey.com>
    
    	* python/python-internal.h (CPYCHECKER_STEALS_REFERENCE_TO_ARG):
    	Remove.
    	* python/py-varobj.c (py_varobj_iter_ctor): Change pyiter to
    	rvalue reference.  Remove CPYCHECKER_STEALS_REFERENCE_TO_ARG.
    	(py_varobj_iter_new): Likewise.
    	(py_varobj_get_iterator): Use gdbpy_ref.

Diff:
---
 gdb/ChangeLog                |  9 +++++++++
 gdb/python/py-varobj.c       | 17 ++++++++---------
 gdb/python/python-internal.h |  7 -------
 3 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 4f8bd2b..6f66a35 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,14 @@
 2018-09-16  Tom Tromey  <tom@tromey.com>
 
+	* python/python-internal.h (CPYCHECKER_STEALS_REFERENCE_TO_ARG):
+	Remove.
+	* python/py-varobj.c (py_varobj_iter_ctor): Change pyiter to
+	rvalue reference.  Remove CPYCHECKER_STEALS_REFERENCE_TO_ARG.
+	(py_varobj_iter_new): Likewise.
+	(py_varobj_get_iterator): Use gdbpy_ref.
+
+2018-09-16  Tom Tromey  <tom@tromey.com>
+
 	* python/py-threadevent.c (py_get_event_thread): Simplify.
 	* python/py-inferior.c (infpy_thread_from_thread_handle):
 	Return immediately after calling thread_to_thread_object.  Use
diff --git a/gdb/python/py-varobj.c b/gdb/python/py-varobj.c
index e6cf645..8946ef8 100644
--- a/gdb/python/py-varobj.c
+++ b/gdb/python/py-varobj.c
@@ -130,14 +130,14 @@ static const struct varobj_iter_ops py_varobj_iter_ops =
    whose children the iterator will be iterating over.  PYITER is the
    python iterator actually responsible for the iteration.  */
 
-static void CPYCHECKER_STEALS_REFERENCE_TO_ARG (3)
+static void
 py_varobj_iter_ctor (struct py_varobj_iter *self,
-		      struct varobj *var, PyObject *pyiter)
+		     struct varobj *var, gdbpy_ref<> &&pyiter)
 {
   self->base.var = var;
   self->base.ops = &py_varobj_iter_ops;
   self->base.next_raw_index = 0;
-  self->iter = pyiter;
+  self->iter = pyiter.release ();
 }
 
 /* Allocate and construct a pretty-printed varobj iterator.  VAR is
@@ -145,13 +145,13 @@ py_varobj_iter_ctor (struct py_varobj_iter *self,
    PYITER is the python iterator actually responsible for the
    iteration.  */
 
-static struct py_varobj_iter * CPYCHECKER_STEALS_REFERENCE_TO_ARG (2)
-py_varobj_iter_new (struct varobj *var, PyObject *pyiter)
+static struct py_varobj_iter *
+py_varobj_iter_new (struct varobj *var, gdbpy_ref<> &&pyiter)
 {
   struct py_varobj_iter *self;
 
   self = XNEW (struct py_varobj_iter);
-  py_varobj_iter_ctor (self, var, pyiter);
+  py_varobj_iter_ctor (self, var, std::move (pyiter));
   return self;
 }
 
@@ -161,7 +161,6 @@ py_varobj_iter_new (struct varobj *var, PyObject *pyiter)
 struct varobj_iter *
 py_varobj_get_iterator (struct varobj *var, PyObject *printer)
 {
-  PyObject *iter;
   struct py_varobj_iter *py_iter;
 
   gdbpy_enter_varobj enter_py (var);
@@ -177,14 +176,14 @@ py_varobj_get_iterator (struct varobj *var, PyObject *printer)
       error (_("Null value returned for children"));
     }
 
-  iter = PyObject_GetIter (children.get ());
+  gdbpy_ref<> iter (PyObject_GetIter (children.get ()));
   if (iter == NULL)
     {
       gdbpy_print_stack ();
       error (_("Could not get children iterator"));
     }
 
-  py_iter = py_varobj_iter_new (var, iter);
+  py_iter = py_varobj_iter_new (var, std::move (iter));
 
   return &py_iter->base;
 }
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index dc42978..e32502d 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -38,13 +38,6 @@
 #define CPYCHECKER_TYPE_OBJECT_FOR_TYPEDEF(ARG)
 #endif
 
-#ifdef WITH_CPYCHECKER_STEALS_REFERENCE_TO_ARG_ATTRIBUTE
-#define CPYCHECKER_STEALS_REFERENCE_TO_ARG(n) \
-   __attribute__ ((cpychecker_steals_reference_to_arg (n)))
-#else
-#define CPYCHECKER_STEALS_REFERENCE_TO_ARG(n)
-#endif
-
 #ifdef WITH_CPYCHECKER_SETS_EXCEPTION_ATTRIBUTE
 #define CPYCHECKER_SETS_EXCEPTION __attribute__ ((cpychecker_sets_exception))
 #else