From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 99980 invoked by alias); 22 Jan 2019 22:03:57 -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 99873 invoked by uid 89); 22 Jan 2019 22:03:57 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_SHORT,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx1.suse.de Received: from mx2.suse.de (HELO mx1.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 22 Jan 2019 22:03:54 +0000 Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 8314BADBD; Tue, 22 Jan 2019 22:03:51 +0000 (UTC) To: Ian Lance Taylor Cc: gcc-patches References: <20181211101411.7067-1-tdevries@suse.de> <20181211101411.7067-8-tdevries@suse.de> From: Tom de Vries Subject: [libbacktrace] Use size_t for low_offset/high_offset fields of struct unit Message-ID: Date: Tue, 22 Jan 2019 22:17:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/mixed; boundary="------------20CEE8235DAF6FF3A2A65713" X-IsSubscribed: yes X-SW-Source: 2019-01/txt/msg01328.txt.bz2 This is a multi-part message in MIME format. --------------20CEE8235DAF6FF3A2A65713 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Content-length: 3151 [ 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? Thanks, - Tom --------------20CEE8235DAF6FF3A2A65713 Content-Type: text/x-patch; name="0001-libbacktrace-Use-size_t-for-low_offset-high_offset-fields-of-struct-unit.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename*0="0001-libbacktrace-Use-size_t-for-low_offset-high_offset-fiel"; filename*1="ds-of-struct-unit.patch" Content-length: 2055 [libbacktrace] Use size_t for low_offset/high_offset fields of struct unit 2019-01-22 Tom de Vries * dwarf.c (struct unit): Use size_t for low_offset/high_offset fields. (units_search, find_unit): Use size_t for offset. (build_address_map): Use size_t for unit_offset. --- libbacktrace/dwarf.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/libbacktrace/dwarf.c b/libbacktrace/dwarf.c index aacbd3a453d..e78d36b0b43 100644 --- a/libbacktrace/dwarf.c +++ b/libbacktrace/dwarf.c @@ -285,10 +285,10 @@ struct unit size_t unit_data_offset; /* Offset of the start of the compilation unit from the start of the .debug_info section. */ - off_t low_offset; + size_t low_offset; /* Offset of the end of the compilation unit from the start of the .debug_info section. */ - off_t high_offset; + size_t high_offset; /* DWARF version. */ int version; /* Whether unit is DWARF64. */ @@ -891,9 +891,9 @@ read_attribute (enum dwarf_form form, struct dwarf_buf *buf, static int units_search (const void *vkey, const void *ventry) { - const off_t *key = (const off_t *) vkey; + const size_t *key = (const size_t *) vkey; const struct unit *entry = *((const struct unit *const *) ventry); - off_t offset; + size_t offset; offset = *key; if (offset < entry->low_offset) @@ -907,7 +907,7 @@ units_search (const void *vkey, const void *ventry) /* Find a unit in PU containing OFFSET. */ static struct unit * -find_unit (struct unit **pu, size_t units_count, off_t offset) +find_unit (struct unit **pu, size_t units_count, size_t offset) { struct unit **u; u = bsearch (&offset, pu, units_count, sizeof (struct unit *), units_search); @@ -1514,7 +1514,7 @@ build_address_map (struct backtrace_state *state, uintptr_t base_address, size_t i; struct unit **pu; size_t prev_addrs_count; - off_t unit_offset = 0; + size_t unit_offset = 0; memset (&addrs->vec, 0, sizeof addrs->vec); memset (&unit_vec->vec, 0, sizeof unit_vec->vec); --------------20CEE8235DAF6FF3A2A65713--