From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25016 invoked by alias); 25 Feb 2017 18:41:25 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 24997 invoked by uid 89); 25 Feb 2017 18:41:24 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-23.7 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS,SPF_SOFTFAIL autolearn=ham version=3.3.2 spammy=3.6, counterpart, Manually X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 25 Feb 2017 18:41:22 +0000 Received: by simark.ca (Postfix, from userid 33) id B53CD1E896; Sat, 25 Feb 2017 13:41:21 -0500 (EST) To: Simon Marchi Subject: Re: [PATCH 5/5] Add missing incref when creating Inferior Python object X-PHP-Originating-Script: 33:rcube.php MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Sat, 25 Feb 2017 18:41:00 -0000 From: Simon Marchi Cc: gdb-patches@sourceware.org In-Reply-To: <20170123224004.8893-6-simon.marchi@ericsson.com> References: <20170123224004.8893-1-simon.marchi@ericsson.com> <20170123224004.8893-6-simon.marchi@ericsson.com> Message-ID: <81b15bf924ea176fc601dea7be679200@polymtl.ca> X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.2.3 X-IsSubscribed: yes X-SW-Source: 2017-02/txt/msg00696.txt.bz2 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 = > (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 "", line 1 in > [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 = > > 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.