From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16425 invoked by alias); 21 Jan 2014 07:54:33 -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 16406 invoked by uid 89); 21 Jan 2014 07:54:30 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.3 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; Tue, 21 Jan 2014 07:54:29 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 7350A11616B; Tue, 21 Jan 2014 02:54:27 -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 dZWkADND0RYA; Tue, 21 Jan 2014 02:54:27 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 8FD9811616A; Tue, 21 Jan 2014 02:54:26 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id 38769E07C9; Tue, 21 Jan 2014 11:54:27 +0400 (RET) Date: Tue, 21 Jan 2014 07:54:00 -0000 From: Joel Brobecker To: manjian2006@gmail.com Cc: gdb-patches@sourceware.org, tromey@redhat.com, linzj Subject: Re: [PATCH] fixed inherit_abstract_dies infinite recursive call Message-ID: <20140121075427.GY4762@adacore.com> References: <87wqhvi4gs.fsf@fleche.redhat.com> <1390268608-16913-1-git-send-email-manjian2006@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1390268608-16913-1-git-send-email-manjian2006@gmail.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-SW-Source: 2014-01/txt/msg00805.txt.bz2 For the record, we've actually seen the exact same behavior from with GNAT a few years ago. The problem occured when when we had the following situation: procedure Outer (...) is [...] procedure Inner (...) is [...] -- Recurse by calling Outer.. Outer (...); end Inner; begin [...] Inner (...); In that case, when compiled at -O3, the compiler generated an Abstract Instance Root (AIR) for procedure Outer, which owned/contained a DIE defining procedure Inner (an out-of-line instance), which itself contains a Concrete Instance Entry (CIIE) corresponding to the inlined version of Outer's AIR. The CIIE has a reference to its AIR via the DW_AT_abstract_origin attribute, hence the cycle. Not being all quite sure how to make sense of the cycle in terms of inheritance, we initially tried to fix in the compiler. Although the patch was initially approved in GCC, it was noted that the output appeared to be conformant with the DWARF specifications (version 3,i at the time, now version 4), particularly in light of the examples in section D.7 of the specifications (3rd example). Eventually, it was found that our GCC patch was causing some issues, and thus was reverted. So far, we've kept the patch in AdaCore's GCC tree, with a note to look into a GDB fix at some point. We are indeed very close to the example from the DWARF specs cited above, but it does not deal with recursion as we do here, so I think that the DWARF specifications do have a hole when it comes to that. Logically speaking, its seems that the sensible thing to do is to inherit the whole Abstract Instance Tree (AIT) but excluding oneself. If that's what the proposed patch is doing, then I think it's a step in the right direction. It's not completely obvious to me what the actual impact is, however. I'll need to study the rest of the code a little more. A review from this file's gurus, with the above info as complement to this patch, would probably make better sense - and be greatly appreciated! Now, linzj@ucweb.com/manjian2006@gmail.com, a few comments and suggestions: . Your patch is not complete, it's missing at least a ChangeLog entry, and I think a testcase would be nice. Did you have a chance to read our little checklist for submitting patches? https://sourceware.org/gdb/wiki/ContributionChecklist For the testcase, I might be able to help. . You re-posted the patch several times, without indicating why you did so, or what changed from version to version. May I kindly request, when you repost a version: + Make sure to add a version number (check for "PATCH v" in the archives at http://www.sourceware.org/ml/gdb-patches/2014-01/, plenty of examples there) + Always indicate what changed. . Do you have an FSF assignment on file? (it'd be nice to know how to address you, btw...) Thank you. On Tue, Jan 21, 2014 at 09:43:28AM +0800, manjian2006@gmail.com wrote: > From: linzj > > 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. > --- > gdb/dwarf2read.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c > index 7ca527d..c226a52 100644 > --- a/gdb/dwarf2read.c > +++ b/gdb/dwarf2read.c > @@ -1224,6 +1224,8 @@ struct die_info > /* True if we're presently building the full type name for the > 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; > @@ -8013,6 +8015,10 @@ process_imported_unit_die (struct die_info *die, struct dwarf2_cu *cu) > static void > process_die (struct die_info *die, struct dwarf2_cu *cu) > { > + /* Only process those who are not in process. */ > + if (die->in_process) > + return; > + die->in_process = 1; > switch (die->tag) > { > case DW_TAG_padding: > @@ -8100,6 +8106,7 @@ process_die (struct die_info *die, struct dwarf2_cu *cu) > new_symbol (die, NULL, cu); > break; > } > + die->in_process = 0; > } > > /* DWARF name computation. */ > -- > 1.8.3.2 -- Joel