From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5486 invoked by alias); 12 Feb 2014 06:58:43 -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 5476 invoked by uid 89); 12 Feb 2014 06:58:42 -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-yh0-f54.google.com Received: from mail-yh0-f54.google.com (HELO mail-yh0-f54.google.com) (209.85.213.54) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Wed, 12 Feb 2014 06:58:41 +0000 Received: by mail-yh0-f54.google.com with SMTP id z6so8034111yhz.13 for ; Tue, 11 Feb 2014 22:58:39 -0800 (PST) MIME-Version: 1.0 X-Received: by 10.236.125.67 with SMTP id y43mr12602311yhh.58.1392188319743; Tue, 11 Feb 2014 22:58:39 -0800 (PST) Received: by 10.170.189.212 with HTTP; Tue, 11 Feb 2014 22:58:39 -0800 (PST) In-Reply-To: <20140211021937.GD5485@adacore.com> References: <1390374431-17981-1-git-send-email-manjian2006@gmail.com> <20140128120600.GG4101@adacore.com> <20140210142831.GY5485@adacore.com> <20140211021937.GD5485@adacore.com> Date: Wed, 12 Feb 2014 06:58: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/msg00389.txt.bz2 On Mon, Feb 10, 2014 at 6:19 PM, Joel Brobecker wrote: >> How hard is it to write a testcase that triggers the problem? > > I wrote one and I thought I posted it here. But I had forgotten > to attach the patch! (git reflog just saved my life). > > Here it is again. > > gdb/testsuite/ChangeLog: > > * gdb.dwarf2/dw2-icycle.S, gdb.dwarf2/dw2-icycle.c, > gdb.dwarf2/dw2-icycle.exp: New files. > > This testcase fails without the patch being proposed. > > Thanks, > -- > Joel Hi. Thanks very much for the testcase! 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. 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?