[PATCH 5/5] Add missing incref when creating Inferior Python object
Simon Marchi
simon.marchi@polymtl.ca
Sat Feb 25 18:41:00 GMT 2017
On 2017-01-23 17:40, Simon Marchi wrote:
> The test py-inferior.exp fails when using my debug build of Python 3.6.
> I don't see it failing with my system's default Python, but it might be
> related to the different memory allocation scheme used when doing a
> build with pydebug.
>
> The issue is that we are missing a Py_INCREF in
> inferior_to_inferior_object. The PyObject_New function initializes the
> object with a refcount of 1. If we assume that this refcount
> corresponds to the reference we are keeping in the inferior data, then
> we are missing an incref for the reference we are returning. We can
> also see it the other way. If the refcount added by PyObject_New is
> for
> the reference we are returning, then we are missing one for the
> inferior
> data.
>
> The counterpart for this incref is in py_free_inferior.
>
> Here's how I can get it to crash:
>
> $ ./gdb -nx -ex "set debug python 1"
> (gdb) add-inferior
> Added inferior 2
> (gdb) python infs = gdb.inferiors()
> Creating Python Inferior object inf = 1
> Creating Python Inferior object inf = 2
> (gdb) remove-inferiors 2
> py_free_inferior inf = 2
> infpy_dealloc inf = <unknown>
> (gdb) python infs = None
> Fatal Python error: Objects/tupleobject.c:243 object at
> 0x7f9cf1a568d8 has negative ref count -1
>
> Current thread 0x00007f9cf1b68780 (most recent call first):
> File "<string>", line 1 in <module>
> [1] 408 abort (core dumped) ./gdb -nx -ex "set debug python 1"
>
> After having created the inferiors object, their refcount is 1 (which
> comes from PyObject_New), but it should be two. The gdb inferior
> object
> has a reference and the "infs" list has a reference.
>
> When invoking remove-inferiors, py_free_inferior gets called. It does
> the decref that corresponds to the reference that the gdb inferior
> object kept. At this moment, the refcount drops to 0 and the object
> gets deallocated, even though the "infs" list still has a reference.
> When we set "infs" to None, Python tries to decref the already zero
> refcount and the assert triggers.
>
> With this patch, it looks better:
>
> (gdb) add-inferior
> Added inferior 2
> (gdb) python infs = gdb.inferiors()
> Creating Python Inferior object inf = 1
> Creating Python Inferior object inf = 2
> (gdb) remove-inferiors 2
> py_free_inferior inf = 2
> (gdb) python infs = None
> infpy_dealloc inf = <unknown>
>
> gdb/ChangeLog:
>
> * python/py-inferior.c (inferior_to_inferior_object): Manually
> increment reference count when creating the object as well.
> ---
> gdb/python/py-inferior.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
> index 340dddcfbd..24ef4f0ec8 100644
> --- a/gdb/python/py-inferior.c
> +++ b/gdb/python/py-inferior.c
> @@ -227,10 +227,13 @@ inferior_to_inferior_object (struct inferior
> *inferior)
> inf_obj->threads = NULL;
> inf_obj->nthreads = 0;
>
> + /* PyObject_New initializes the new object with a refcount of 1.
> This
> + counts for the reference we are keeping in the inferior data. */
> set_inferior_data (inferior, infpy_inf_data_key, inf_obj);
> }
> - else
> - Py_INCREF ((PyObject *)inf_obj);
> +
> + /* We are returning a new reference. */
> + Py_INCREF (inf_obj);
>
> return gdbpy_inf_ref (inf_obj);
> }
Ping for this patch only. It's actually not dependent on the rest of
the series and fixes an actual bug, so I think it could go in by itself.
More information about the Gdb-patches
mailing list