From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6075 invoked by alias); 13 Feb 2014 07:31: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 6065 invoked by uid 89); 13 Feb 2014 07:31:16 -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; Thu, 13 Feb 2014 07:31:13 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id D74A711640D; Thu, 13 Feb 2014 02:31:11 -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 DYQTFJVtvPPv; Thu, 13 Feb 2014 02:31:11 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id EF9E91163F7; Thu, 13 Feb 2014 02:31:10 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id B3933E0D08; Thu, 13 Feb 2014 11:31:12 +0400 (RET) Date: Thu, 13 Feb 2014 07:31:00 -0000 From: Joel Brobecker To: Doug Evans , manjian2006@gmail.com Cc: "gdb-patches@sourceware.org" , Tom Tromey , linzj Subject: Re: PING: [PATCH v4] fixed inherit_abstract_dies infinite recursive call Message-ID: <20140213073112.GS5485@adacore.com> References: <1390374431-17981-1-git-send-email-manjian2006@gmail.com> <20140128120600.GG4101@adacore.com> <20140210142831.GY5485@adacore.com> <20140211021937.GD5485@adacore.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-SW-Source: 2014-02/txt/msg00446.txt.bz2 Hi Doug, > Hi. Thanks very much for the testcase! You are very welcome. Surprisingly, it took me a 2-3 hours to reduce it down. I tried using the DWARF assembler, but to no avail, so had to work from an asm file instead. We might be able to reproduce that asm file using the DWARF assembler, but I don't want to spend any more time on the testcase, unless I really have to. > I don't have much experience with abstract_origin. I've read what I > can from DWARF4.pdf. > I can imagine that the bug is really in inherit_abstract_dies, and > thus the check to avoid re-processing the die belongs there. > Then we could maybe assert-fail in process_die if it's already being > processed. Same here. > E.g., where inherit_abstract_dies has this: > > /* Found that ORIGIN_CHILD_DIE is really not referenced. */ > process_die (origin_child_die, origin_cu); > > add a check for origin_child_die->in_process here, and only call > process_die if it's zero, > and add a comment saying the check is to avoid the case of mutually > referenced abstract_origins. > And include a reference to a bug number because for me one high order > bit here is finding the testcase that exercises this check. > > And then in process_die add at the start: > > gdb_assert (!die->in_process); > > I don't have a strong opinion on which way to go though. > I *do* have a strong opinion on understanding *why* the code is > checking die->in_process. > If we go with the current patch, which is fine with me, though I'm > slightly leading towards the above instead but am happy to defer to > others, then I would replace this part of your patch: > > + /* Only process those not already in process. */ > + if (die->in_process) > + return; > > with: > > + /* Only process those not already in process. PR 12345. */ > + if (die->in_process) > + return; > > And in either case file a bug that includes a description of the > problem (obviously :-)) and a reference to the testcase name. > > Thoughts? I agree with your all you suggestions. manjian2006@gmail.com, do you want to take care of this, or would you like me to? This involves first creating a PR, change your patch according to Doug's suggestion, re-test, and re-submit. -- Joel