From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19764 invoked by alias); 13 Feb 2014 08:01:28 -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 19745 invoked by uid 89); 13 Feb 2014 08:01:27 -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-f47.google.com Received: from mail-pa0-f47.google.com (HELO mail-pa0-f47.google.com) (209.85.220.47) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Thu, 13 Feb 2014 08:01:26 +0000 Received: by mail-pa0-f47.google.com with SMTP id kp14so10410626pab.34 for ; Thu, 13 Feb 2014 00:01:25 -0800 (PST) X-Received: by 10.68.255.101 with SMTP id ap5mr89409pbd.41.1392278485092; Thu, 13 Feb 2014 00:01:25 -0800 (PST) Received: from [10.1.44.183] ([70.39.187.196]) by mx.google.com with ESMTPSA id bz4sm3611388pbb.12.2014.02.13.00.01.20 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Thu, 13 Feb 2014 00:01:24 -0800 (PST) Message-ID: <52FC7BC8.2010400@gmail.com> Date: Thu, 13 Feb 2014 08:01:00 -0000 From: lin zuojian User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.2.0 MIME-Version: 1.0 To: Joel Brobecker , Doug Evans , "gdb-patches@sourceware.org" CC: Tom Tromey , linzj Subject: Re: PING: [PATCH v4] fixed inherit_abstract_dies infinite recursive call References: <1390374431-17981-1-git-send-email-manjian2006@gmail.com> <20140128120600.GG4101@adacore.com> <20140210142831.GY5485@adacore.com> <20140211021937.GD5485@adacore.com> <20140213073112.GS5485@adacore.com> In-Reply-To: <20140213073112.GS5485@adacore.com> Content-Type: text/plain; charset=GB2312 Content-Transfer-Encoding: 7bit X-SW-Source: 2014-02/txt/msg00448.txt.bz2 Hi Joel, I am not really familiar with these routines,please help me with that. Thank you. (I carelessly used the company email in the last reply,sorry). > 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. >