From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 47081 invoked by alias); 10 Sep 2019 01:37:07 -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 47072 invoked by uid 89); 10 Sep 2019 01:37:07 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-19.8 required=5.0 tests=AWL,BAYES_00,ENV_AND_HDR_SPF_MATCH,RCVD_IN_DNSWL_NONE,SPF_PASS,USER_IN_DEF_SPF_WL autolearn=ham version=3.3.1 spammy=D*ca, wonderful X-HELO: mail-wr1-f66.google.com Received: from mail-wr1-f66.google.com (HELO mail-wr1-f66.google.com) (209.85.221.66) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 10 Sep 2019 01:37:05 +0000 Received: by mail-wr1-f66.google.com with SMTP id h7so15710739wrw.8 for ; Mon, 09 Sep 2019 18:37:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=MYWPrudUBVh8KFX22PR8cWL5LnINOKbrRjL5HMfN9Z0=; b=Fya7SlPy/viP8z/6W9NpRHERyryPAPSyNkqmmnIBID9mLYXbDh4jgVFfCD85v4zz1l thvX3GvQAkbyLzUzs3ZB6QlI94d1Ki+bA3tr4Bnk8pTGQx4NLt2WED5mRZDcDjNKsN6C /dYezVWA/RZ3sjPeReT87Gl95ME/LZ55SAnO6es73SqcAK8viCCgCyK8CG98xwaG0MYe kW3u++slUtcRBT4LOrsATKGCkoyxcbp9KaKYtw78FBUr9yXB1fz1apNiYe6qwUDpHQl+ V7o2jcFjMlJ11FH1wDobK8zpPSN5BZ7TAV+OOGyMjw/rxKMMt0RGnRN8SRxMdtK8F9d0 aMKA== MIME-Version: 1.0 References: <4caf107e-8766-8a6d-df57-b824c287fa70@simark.ca> <20190909185803.182231-1-tamur@google.com> In-Reply-To: From: "Ali Tamur via gdb-patches" Reply-To: Ali Tamur Date: Tue, 10 Sep 2019 01:37:00 -0000 Message-ID: Subject: Re: [PATCH v2 1/4] DWARF 5 support: Handle dwo_id To: Simon Marchi Cc: gdb-patches@sourceware.org Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2019-09/txt/msg00153.txt.bz2 Thank you for the review, I am submitting now. On Mon, Sep 9, 2019 at 2:21 PM Simon Marchi wrote: > > On 2019-09-09 2:58 p.m., Ali Tamur via gdb-patches wrote: > >> Since you're touching the split-DWARF code, it might be a good idea to run the whole > >> testsuite with -gsplit-dwarf, with DWARF 4 (to verify there are no regressions) and > >> DWARF 5 (to check how things improve). > > Done. Both with and without -gsplit-dwarf the set of tests that fail is > > identical. This is expected as unless I introduced a bug, there should be no > > behavioral change at all for DWARF 4. I also ran the testsuite with -gdwarf-5 > > and the number of failures went down from 32687 to 1163, but until gdb can > > handle 'hello world' (hopefully at the end my patch series) I think that is > > not a very meaningful metric. > > You're right that it's not very meaningful, but still encouraging :). > > An interesting one will be to compare the results for DWARF 4 vs for DWARF 5. > > >> And since DWARF 5 is relatively new stuff, results can vary greatly if using different versions of the > >> same compiler to run the tests. So if you could mention in the commit message which gcc version the > >> tests were ran against, I think it would be useful. > > Updated the commit message to include the gcc version (8.3.0). > > Thanks. > > > Sorry for the style mistakes; I am accustomed to a different style and also > > have become too dependent on clang-tidy. > > No problem, and I completely understand. I had a taste of clang-format (I > presume you meant clang-format) for C++ and black for Python, and it's really > nice not to have to think about formatting. It would be wonderful to have > something equivalent here :). > > I just found one last little thing: > > > gdb/ChangeLog: > > > > * gdb/dwarf2read.c (comp_unit_head): Update comment. > > The file name here should just be "dwarf2read.c", as it should be relative to > the ChangeLog file the entry is in. > > The patch LGTM with that fixed, you can go ahead and push. Thanks for following > up quickly and efficiently on review comments. > > I'd like to take a look at the other patches of the series, but that won't be > before at least next week, as I'll be quite busy with the GNU Cauldron Conference > until then. But if somebody else wants to review them, please go ahead. > > Simon