Don't steal references in the gdb Python code
Commit Message
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.
---
gdb/ChangeLog | 9 +++++++++
gdb/python/py-varobj.c | 17 ++++++++---------
gdb/python/python-internal.h | 7 -------
3 files changed, 17 insertions(+), 16 deletions(-)
Comments
>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:
Tom> Some Python APIs steal references from their caller, and the refcount
Tom> checker supports this via an attribute.
Tom> However, in gdb with C++ we have a better idiom available: we can use
Tom> std::move on a gdbpy_ref<> instead. This makes the semantics obvious
Tom> at the point of call, and is safer at runtime as well, because the
Tom> callee's gdbpy_ref<> will be emptied.
Tom> This patch changes the reference-stealing code in gdb to use rvalue
Tom> references instead.
I got email from Simon saying this looked good, so I pushed it in. But,
I don't see that email in the archives. It was CC'd though.
Tom
@@ -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;
}
@@ -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