From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 96590 invoked by alias); 22 Jan 2019 23:02:48 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 96574 invoked by uid 89); 22 Jan 2019 23:02:48 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-lj1-f181.google.com Received: from mail-lj1-f181.google.com (HELO mail-lj1-f181.google.com) (209.85.208.181) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 22 Jan 2019 23:02:46 +0000 Received: by mail-lj1-f181.google.com with SMTP id l15-v6so195581lja.9 for ; Tue, 22 Jan 2019 15:02:45 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=golang-org.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=8MIb4bsBBAuj8RrclNAKbyaqQx2zjJMObcz0ErhMacY=; b=n0w2ph0QeAftogMZYxrtaDkhe90rmOwhq25Glp+Baizc8ogYwgp9DjA+u+3wZcFOA2 /rUdF4k1OkZoEDE1qps/Vzphs+/ZPzmHUiwpdk4pSkRSt1UGFajZr/B9d16fJnPC6qFc +MSaAWae7Bl0cB8bHTrbbuCPr+wBIrPZh4/sDmv3/DE8Kfh0Wu7Z9ywbWuiKLOUPPxZu 2OsF18NoQI9JJHjvknbzKVzie3+1hagd8ifQV8SE4ESksFBaiMA3FiXqgh6Z3Eu0EGQH FvvpJRqmWocxFUvNGiBQM0eKnW3Hlj0BO0dHQnd+sCYFBaLqjuk7H/JRbfipQc2dBa8E 418Q== MIME-Version: 1.0 References: <20181211101411.7067-1-tdevries@suse.de> <20181211101411.7067-8-tdevries@suse.de> In-Reply-To: From: Ian Lance Taylor Date: Tue, 22 Jan 2019 23:05:00 -0000 Message-ID: Subject: Re: [libbacktrace] Use size_t for low_offset/high_offset fields of struct unit To: Tom de Vries Cc: gcc-patches Content-Type: text/plain; charset="UTF-8" X-SW-Source: 2019-01/txt/msg01336.txt.bz2 On Tue, Jan 22, 2019 at 2:03 PM Tom de Vries wrote: > > [ was: Re: [PATCH 7/9] [libbacktrace] Handle DW_FORM_GNU_ref_alt ] > On 18-01-19 15:25, Ian Lance Taylor wrote: > > On Thu, Jan 17, 2019 at 6:14 AM Tom de Vries wrote: > >> > >> On 17-01-19 01:35, Ian Lance Taylor wrote: > >>> On Wed, Jan 16, 2019 at 4:17 PM Tom de Vries wrote: > >>>> > >>>> this handles DW_FORM_GNU_ref_alt which references the .debug_info > >>>> section in the .gnu_debugaltlink file. > >>>> > >>>> OK for trunk? > >>>> > >>>> Thanks, > >>>> - Tom > >>>> > >>>> On 11-12-18 11:14, Tom de Vries wrote: > >>>>> 2018-12-10 Tom de Vries > >>>>> > >>>>> * dwarf.c (enum attr_val_encoding): Add ATTR_VAL_REF_ALT_INFO. > >>>>> (struct unit): Add low and high fields. > >>>>> (struct unit_vector): New type. > >>>>> (struct dwarf_data): Add units and units_counts fields. > >>>>> (read_attribute): Handle DW_FORM_GNU_ref_alt using > >>>>> ATTR_VAL_REF_ALT_INFO. > >>>>> (find_unit): New function. > >>>>> (find_address_ranges): Add and handle unit_tag parameter. > >>>>> (build_address_map): Add and handle units_vec parameter. > >>>>> (read_referenced_name_1): Handle DW_FORM_GNU_ref_alt. > >>>>> (build_dwarf_data): Pass units_vec to build_address_map. Store resulting > >>>>> units vector. > >>> > >>> > >>>>> @@ -281,6 +283,10 @@ struct unit > >>>>> /* The offset of UNIT_DATA from the start of the information for > >>>>> this compilation unit. */ > >>>>> size_t unit_data_offset; > >>>>> + /* Start of the compilation unit. */ > >>>>> + size_t low; > >>>>> + /* End of the compilation unit. */ > >>>>> + size_t high; > >>> > >>> The comments should say what low and high are measured in, which I > >>> guess is file offset. Or is it offset from the start of UNIT_DATA? > >>> Either way, If that is right, then the fields should be named > >>> low_offset and high_offset. Otherwise it seems easy to confuse with > >>> function_addrs, where low and high refer to PC values. > >>> > >> > >> Done. > >> > >>> Also if they are offsets from UNIT_DATA then size_t is OK, but if the > >>> are file offsets they should be off_t. > >>> > >> > >> AFAIU, in the case where off_t vs size_t would make a difference, we're > >> running into trouble much earlier. I've filed PR 88890 - "libbacktrace > >> on 32-bit system with _FILE_OFFSET_BITS == 64" ( > >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88890 ) about this. > >> > >> Anyway, I've made the conservative choice of using off_t for now (but I > >> could argue that it's a memory offset, given that the assumption is that > >> the entire debug section is read into memory). > > > > Since the entire debug section is read into memory, if they are > > offsets from UNIT_DATA, they should be size_t, not off_t. I wasn't > > trying to say that we should make a conservative choice here, I was > > trying to say that we should make a correct choice. An offset from > > UNIT_DATA should be size_t, a file offset should be off_t. > > These are offsets from the start of the .debug_info section, so AFAIU > that means these could be size_t. > > Tested with x86_64 build and host libbacktrace make check. > > OK for trunk if bootstrap and reg-test on x86_64 succeeds? This is OK. Thanks. Ian