From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 89918 invoked by alias); 9 Sep 2019 21:21:22 -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 89902 invoked by uid 89); 9 Sep 2019 21:21:22 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-6.6 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.1 spammy=wonderful, pm, busy, conference X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 09 Sep 2019 21:21:20 +0000 Received: from [172.16.0.120] (192-222-181-218.qc.cable.ebox.net [192.222.181.218]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id C084C1E08C; Mon, 9 Sep 2019 17:21:18 -0400 (EDT) Subject: Re: [PATCH v2 1/4] DWARF 5 support: Handle dwo_id To: Ali Tamur , gdb-patches@sourceware.org References: <4caf107e-8766-8a6d-df57-b824c287fa70@simark.ca> <20190909185803.182231-1-tamur@google.com> From: Simon Marchi Message-ID: Date: Mon, 09 Sep 2019 21:21:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 MIME-Version: 1.0 In-Reply-To: <20190909185803.182231-1-tamur@google.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-SW-Source: 2019-09/txt/msg00149.txt.bz2 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