From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from qproxy3-pub.mail.unifiedlayer.com (qproxy3-pub.mail.unifiedlayer.com [67.222.38.20]) by sourceware.org (Postfix) with ESMTPS id 08E6A3858C41 for ; Fri, 9 Jun 2023 16:57:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 08E6A3858C41 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=tromey.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=tromey.com Received: from gproxy3-pub.mail.unifiedlayer.com (gproxy3-pub.mail.unifiedlayer.com [69.89.30.42]) by qproxy3.mail.unifiedlayer.com (Postfix) with ESMTP id 2C0C080333C3 for ; Fri, 9 Jun 2023 16:57:16 +0000 (UTC) Received: from cmgw12.mail.unifiedlayer.com (unknown [10.0.90.127]) by progateway5.mail.pro1.eigbox.com (Postfix) with ESMTP id 1DC1310048791 for ; Fri, 9 Jun 2023 16:56:16 +0000 (UTC) Received: from box5379.bluehost.com ([162.241.216.53]) by cmsmtp with ESMTP id 7fPMq1uunbVUp7fPMq3Gs6; Fri, 09 Jun 2023 16:56:16 +0000 X-Authority-Reason: nr=8 X-Authority-Analysis: v=2.4 cv=f9iNuM+M c=1 sm=1 tr=0 ts=648359b0 a=ApxJNpeYhEAb1aAlGBBbmA==:117 a=ApxJNpeYhEAb1aAlGBBbmA==:17 a=dLZJa+xiwSxG16/P+YVxDGlgEgI=:19 a=of4jigFt-DYA:10:nop_rcvd_month_year a=Qbun_eYptAEA:10:endurance_base64_authed_username_1 a=CCpqsmhAAAAA:8 a=mDV3o1hIAAAA:8 a=yKlPzbiGcr6i79rn-RMA:9 a=ul9cdbp4aOFLsgKbc677:22 a=_FVE-zBwftR9WsbkzFJk:22 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=tromey.com; s=default; h=Content-Type:MIME-Version:Message-ID:In-Reply-To:Date:References :Subject:Cc:To:From:Sender:Reply-To:Content-Transfer-Encoding:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=KCUbr8lE3pjtrK/rCjLa7nOeUkeZV3jFCr+q7guhQU0=; b=qntE13LQQ2uVTGr/DrjNWauuQw qfKy2YVeW7gawQldSsvX1uIxBj+cHw9biBcAuK/wQ6Ip8iSkEVf0xNg8nlWINWFSg/NibSoA+wFTu Ibknn1RVF/yNCCXlBDo24pEMo; Received: from 75-166-136-83.hlrn.qwest.net ([75.166.136.83]:34484 helo=murgatroyd) by box5379.bluehost.com with esmtpsa (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1q7fPL-003Pi3-Ol; Fri, 09 Jun 2023 10:56:15 -0600 From: Tom Tromey To: Matheus Branco Borella via Gdb-patches Cc: Matheus Branco Borella Subject: Re: [PATCH] Add name_of_main and language_of_main to the DWARF index References: <20230608214012.1561-1-dark.ryu.550@gmail.com> X-Attribution: Tom Date: Fri, 09 Jun 2023 10:56:14 -0600 In-Reply-To: <20230608214012.1561-1-dark.ryu.550@gmail.com> (Matheus Branco Borella via Gdb-patches's message of "Thu, 8 Jun 2023 18:40:13 -0300") Message-ID: <87cz247exd.fsf@tromey.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - box5379.bluehost.com X-AntiAbuse: Original Domain - sourceware.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - tromey.com X-BWhitelist: no X-Source-IP: 75.166.136.83 X-Source-L: No X-Exim-ID: 1q7fPL-003Pi3-Ol X-Source: X-Source-Args: X-Source-Dir: X-Source-Sender: 75-166-136-83.hlrn.qwest.net (murgatroyd) [75.166.136.83]:34484 X-Source-Auth: tom+tromey.com X-Email-Count: 1 X-Source-Cap: ZWx5bnJvYmk7ZWx5bnJvYmk7Ym94NTM3OS5ibHVlaG9zdC5jb20= X-Local-Domain: yes X-Spam-Status: No, score=-3019.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,JMQ_SPF_NEUTRAL,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: >>>>> Matheus Branco Borella via Gdb-patches writes: Hi. Thank you for the patch. I think it's a good addition and is fundamentally fine. There are a few nits below. For a patch of this size, I think we will need a copyright assignment. The form is here: http://git.savannah.gnu.org/cgit/gnulib.git/plain/doc/Copyright/request-assign.future Normally this is a pretty quick process. > This patch adds a new section to the DWARF index containing the name > and the language of the main function symbol, gathered from > `cooked_index::get_main`, if available. Currently, for lack of a better name, > this section is called the "shortcut table" (suggestions for a better name are > appreciated). Seems fine to me :) > In my testing (a binary with about 1.8GB worth of DWARF data) this change brings > startup time down from about 34 seconds to about 1.5 seconds. Very nice. > (I feel like I might've changed too much about the index format by adding a > breaking change. If there's a better way to do this it'd be glad to hear about > it.) The index format is documented in gdb/doc/gdb.texinfo. So, your patch will at least need an update to that file. I suspect a brief entry in gdb/NEWS describing the change would also be good. There's a note on the compatibility issue inline, below. > /* The version number. */ > - contents.append_offset (8); > + contents.append_offset (9); Someday we should probably use a #define for this. Not your problem though. > +/* Write shortcut information. */ > + > +static void > +write_shortcuts_table (cooked_index *table, data_buf& shortcuts, gdb style puts the "&" next to "shortcuts". There's a few of these I think. > + lang = main_info->per_cu->lang (); ... > + shortcuts.append_uint (4, BFD_ENDIAN_LITTLE, lang); I think it would be better to use a DW_LANG_ constant here -- it's better not to expose gdb's enum language values to the world, whereas the DWARF values are already fixed. Those aren't really preserved exactly in the reader, but mapping back from the gdb language to DWARF would be fine (probably). > + ++i; > + > + const gdb_byte *shortcut_table = addr + metadata[i]; > + const gdb_byte *shortcut_table_end = addr + metadata[i + 1]; > + map->shortcut_table > + = gdb::array_view (shortcut_table, shortcut_table_end); This section probably has to be conditional on 'version >= 8. Otherwise a new gdb will fail with an older version of the index. > +/* Sets the name and language of the main function from the shortcut table. */ > + > +static void > +set_main_name_from_gdb_index (dwarf2_per_objfile *per_objfile, > + mapped_gdb_index *index) > +{ > + auto ptr = index->shortcut_table.data (); > + const auto lang = extract_unsigned_integer (ptr, 4, BFD_ENDIAN_LITTLE); This code should probably check the size of the data, both for safety and also because that's a decent way to handle the index version issue. Tom