From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9301 invoked by alias); 12 Feb 2014 01:29:05 -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 9288 invoked by uid 89); 12 Feb 2014 01:29:04 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL,BAYES_00,FREEMAIL_ENVFROM_END_DIGIT,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-pa0-f43.google.com Received: from mail-pa0-f43.google.com (HELO mail-pa0-f43.google.com) (209.85.220.43) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Wed, 12 Feb 2014 01:29:03 +0000 Received: by mail-pa0-f43.google.com with SMTP id rd3so8453234pab.16 for ; Tue, 11 Feb 2014 17:29:01 -0800 (PST) X-Received: by 10.68.197.8 with SMTP id iq8mr47745637pbc.124.1392168541393; Tue, 11 Feb 2014 17:29:01 -0800 (PST) Received: from ubuntu.dhcp.ucweb.local ([70.39.187.196]) by mx.google.com with ESMTPSA id id1sm57474218pbc.11.2014.02.11.17.28.58 for (version=TLSv1.1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Tue, 11 Feb 2014 17:29:00 -0800 (PST) From: manjian2006@gmail.com To: gdb-patches@sourceware.org, tromey@redhat.com, brobecker@adacore.com, xdje42@gmail.com Subject: Re: PING: [PATCH v4] fixed inherit_abstract_dies infinite recursive call Date: Wed, 12 Feb 2014 01:29:00 -0000 Message-Id: <1392168529-20445-1-git-send-email-manjian2006@gmail.com> In-Reply-To: <20140210142831.GY5485@adacore.com> References: <20140210142831.GY5485@adacore.com> X-SW-Source: 2014-02/txt/msg00373.txt.bz2 I don't see anything I can review. Did you forget to attach the patch? > 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 >