From 1a3389079dd5c93419846f44d42027a526ce19cf Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Sun, 16 Sep 2018 07:32:23 -0600 Subject: [PATCH] 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 * 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. --- 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 4f8bd2b9cb7..6f66a35e2de 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,12 @@ +2018-09-16 Tom Tromey + + * 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 * python/py-threadevent.c (py_get_event_thread): Simplify. diff --git a/gdb/python/py-varobj.c b/gdb/python/py-varobj.c index e6cf6455144..8946ef85af3 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 dc42978c6cf..e32502d0367 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 -- 2.30.2