[libbacktrace] Use size_t for low_offset/high_offset fields of struct unit

[Ian Lance Taylor]

On Tue, Jan 22, 2019 at 2:03 PM Tom de Vries <tdevries@suse.de> 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 <tdevries@suse.de> wrote:
> >>
> >> On 17-01-19 01:35, Ian Lance Taylor wrote:
> >>> On Wed, Jan 16, 2019 at 4:17 PM Tom de Vries <tdevries@suse.de> 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  <tdevries@suse.de>
> >>>>>
> >>>>>       * 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