From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22878 invoked by alias); 10 Feb 2014 17:37:17 -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 22869 invoked by uid 89); 10 Feb 2014 17:37:16 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.6 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-yk0-f170.google.com Received: from mail-yk0-f170.google.com (HELO mail-yk0-f170.google.com) (209.85.160.170) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Mon, 10 Feb 2014 17:37:15 +0000 Received: by mail-yk0-f170.google.com with SMTP id 9so7941707ykp.1 for ; Mon, 10 Feb 2014 09:37:12 -0800 (PST) MIME-Version: 1.0 X-Received: by 10.236.123.193 with SMTP id v41mr3318934yhh.68.1392053832864; Mon, 10 Feb 2014 09:37:12 -0800 (PST) Received: by 10.170.189.212 with HTTP; Mon, 10 Feb 2014 09:37:12 -0800 (PST) In-Reply-To: <20140210142831.GY5485@adacore.com> References: <1390374431-17981-1-git-send-email-manjian2006@gmail.com> <20140128120600.GG4101@adacore.com> <20140210142831.GY5485@adacore.com> Date: Mon, 10 Feb 2014 17:37:00 -0000 Message-ID: Subject: Re: PING: [PATCH v4] fixed inherit_abstract_dies infinite recursive call From: Doug Evans To: Joel Brobecker Cc: =?UTF-8?B?5p6X5L2c5YGl?= , "gdb-patches@sourceware.org" , Tom Tromey , linzj Content-Type: text/plain; charset=ISO-8859-1 X-IsSubscribed: yes X-SW-Source: 2014-02/txt/msg00321.txt.bz2 On Mon, Feb 10, 2014 at 6:28 AM, Joel Brobecker wrote: > 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 >> Hi. How hard is it to write a testcase that triggers the problem?