From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21354 invoked by alias); 10 Feb 2014 14:28:36 -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 21343 invoked by uid 89); 10 Feb 2014 14:28:35 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.5 required=5.0 tests=AWL,BAYES_00 autolearn=ham version=3.3.2 X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Mon, 10 Feb 2014 14:28:34 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 8CF7611640B; Mon, 10 Feb 2014 09:28:32 -0500 (EST) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id Y-jB9L9ljqCp; Mon, 10 Feb 2014 09:28:32 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id A3F4211641A; Mon, 10 Feb 2014 09:28:31 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id DA5B1E075B; Mon, 10 Feb 2014 18:28:31 +0400 (RET) Date: Mon, 10 Feb 2014 14:28:00 -0000 From: Joel Brobecker To: manjian2006@gmail.com Cc: gdb-patches@sourceware.org, tromey@redhat.com, xdje42@gmail.com, linzj Subject: PING: [PATCH v4] fixed inherit_abstract_dies infinite recursive call Message-ID: <20140210142831.GY5485@adacore.com> References: <1390374431-17981-1-git-send-email-manjian2006@gmail.com> <20140128120600.GG4101@adacore.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20140128120600.GG4101@adacore.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-SW-Source: 2014-02/txt/msg00302.txt.bz2 Ping! It would be interesting to have a formal review of this patch, to know if it is an acceptable fix or not. If not, I can schedule some time to follow any recommendation that might come out of this review. Thank you! On Tue, Jan 28, 2014 at 04:06:00PM +0400, Joel Brobecker wrote: > > > btw, do you have a copyright assignment on file? > > > This change feels small enough to me to not need one, > > > but it's not clear. > > > > I am a Chinese guy,and Chinese have not clue about the copyright. > > (A joke.I don't need copyright.) > > It's actually not for your personal benefit, but rather to help the FSF > enforce the GPL license on the code you are contributing, thus helping > it defend the freedom of our collective code. See: > http://www.gnu.org/licenses/why-assign.html > > > >> Please Joel Brobecker helps with the testcases. > > Attached is a testcase that causes the debugger to crash on > x86_64-linux. It should work on all ELF targets. > > A plea to the dwarf2read.c gurus: > > Would it be possible to take a look at this patch, to see if it is > going in the right direction? Otherwise, I'll take a deeper look, > and see if I can solve it better. Intuitively, I think it may work, > but almost as a side-effect. Could the recursion check introduced > here do more than what we'd want to, for instance? > > Thanks! > > > >>> The c++ code causing the problem is: > > >>> > > >>> // Integer variants of certain metrics, used for HTML rendering. > > >>> int ascent(FontBaseline baselineType = AlphabeticBaseline) const > > >>> { > > >>> if (baselineType == AlphabeticBaseline) > > >>> return lroundf(m_ascent); > > >>> return height() - height() / 2; > > >>> } > > >>> > > >>> int height(FontBaseline baselineType = AlphabeticBaseline) const > > >>> { > > >>> return ascent(baselineType) + descent(baselineType); > > >>> } > > >>> > > >>> As you can see,ascent(0x5816d55) calls height(0x5812c1b),and height calls > > >>> ascent(0x5816d55) recursivly.And the compiler generates these dwarf code > > >>> representing this relationship preciously. > > >>> > > >>> A dwarf die may have the following relationship: > > >>> 564860c<----------------------------- > > >>> | | > > >>> |(abstract origin) | > > >>> | | > > >>> V | > > >>> 5816d55 | (abstract origin) > > >>> | | > > >>> |(child) | > > >>> | | > > >>> V | > > >>> ... | > > >>> 5812c34------------------------------ > > >>> So inherit_abstract_dies may results in infinite recursive call. > > >>> A bit field call in_process has been add to struct die_info to fix this problem. > > >>> process_die would first check if a die is in processing state, if so,just return. > > >>> Then in_process bit is set.Before process_die returns,this bit field is unset.--- > > ChangeLog | 4 ++++ > > gdb/dwarf2read.c | 20 ++++++++++++++++++++ > > 2 files changed, 24 insertions(+) > > > > diff --git a/ChangeLog b/ChangeLog > > index 9b1cbfa..0098a72 100644 > > --- a/ChangeLog > > +++ b/ChangeLog > > @@ -1,3 +1,7 @@ > > +2013-01-20 lin zuojian > > + * dwarf2read.c (struct die_info): New member in_process. > > + (reset_die_in_process): New function. > > + (process_die): Set it at the start, reset when returning. > > 2013-12-19 Keven Boell > > > > * cp-namespace.c (cp_lookup_nested_symbol): Enable > > diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c > > index 7ca527d..ffedde5 100644 > > --- a/gdb/dwarf2read.c > > +++ b/gdb/dwarf2read.c > > @@ -1225,6 +1225,9 @@ struct die_info > > type derived from this DIE. */ > > unsigned char building_fullname : 1; > > > > + /* True if this die is in process. */ > > + unsigned char in_process : 1; > > + > > /* Abbrev number */ > > unsigned int abbrev; > > > > @@ -8008,11 +8011,27 @@ process_imported_unit_die (struct die_info *die, struct dwarf2_cu *cu) > > } > > } > > > > +/* Reset the in_process bit of a die. */ > > + > > +static void > > +reset_die_in_process (void *arg) > > +{ > > + struct die_info *die = arg; > > + die->in_process = 0; > > +} > > + > > /* Process a die and its children. */ > > > > static void > > process_die (struct die_info *die, struct dwarf2_cu *cu) > > { > > + struct cleanup *in_process; > > + > > + /* Only process those not already in process. */ > > + if (die->in_process) > > + return; > > + die->in_process = 1; > > + in_process = make_cleanup (reset_die_in_process,die); > > switch (die->tag) > > { > > case DW_TAG_padding: > > @@ -8100,6 +8119,7 @@ process_die (struct die_info *die, struct dwarf2_cu *cu) > > new_symbol (die, NULL, cu); > > break; > > } > > + do_cleanups (in_process); > > } > > > > /* DWARF name computation. */ > > -- > > 1.8.3.2 > > -- > Joel -- Joel