[PATCH] D31424: [ELF] - Use relocated content when generating .gdb_index

[David Blaikie via llvm-commits]

On Wed, Mar 29, 2017 at 2:16 AM George Rimar via Phabricator <
reviews at reviews.llvm.org> wrote:

> grimar added inline comments.
>
>
> ================
> Comment at: ELF/SyntheticSections.cpp:1739-1740
> +  // consists of a 0 for the beginning address offset and a 0 for the
> ending
> +  // address offset. We assume here that whole section data starting from
> offset
> +  // is a range, terminated with null entry.
> +  if (Optional<uint64_t> Offset =
> toSectionOffset(Die.find(DW_AT_ranges))) {
> ----------------
> dblaikie wrote:
> > I don't understand what assumption you're describing.
> >
> > Oh, because relocations haven't been performed, nor handled at all -
> it's not possible to tell where the address ranges end, so you're providing
> an overestimate here? That's a bit subtle and the function name should at
> least mention that I would think (& the function that calls this one too)
> >
> > Is there any penalty to this overestimate in how the gdb_index is
> created? Does a larger sized record need to be used to handle a maximum
> that may never be needed?
> >
> > [looking at the implementation, it seems there is a penalty for
> overestimate - a table is reserved for that many address ranges, right? so
> much of that table may go unused if this is a significant overestimate
> (though that probably only happens if someone uses ld -r or, perhaps more
> likely: LTO)]
> >
> > I wouldn't imagine it'd be too hard to look at enough relocation data
> (essentially parse the relocations applicable to debug_ranges and look for
> holes (bytes that don't have relocations for them) - those are the ends of
> range lists - you wouldn't need to apply any relocations or examine their
> addends, etc, only their offsets)
> >
> >
> >I don't understand what assumption you're describing.
> >
> >Oh, because relocations haven't been performed, nor handled at all - it's
> not possible to tell where the address ranges end, so you're providing >an
> overestimate here? That's a bit subtle and the function name should at
> least mention that I would think (& the function that calls this one >too)
>
> Right. Its straightforward and fastest way to do things I believe. It is
> possible to tell where is address ranges end only after handling or some
> parsing of relocations. I think we can find the relocation with largest
> offset in .debug_range section and that be enough to know that next offset
> is the end here. (but that would not solve everything, please see below)
> I can probably rename getDieRangesCount to estimateDieRangesCount and
> getAddressRangesCount to estimateAddressRangesCount ?
>
> >Is there any penalty to this overestimate in how the gdb_index is
> created? Does a larger sized record need to be used to handle a maximum
> >that may never be needed?
> >
> >[looking at the implementation, it seems there is a penalty for
> overestimate - a table is reserved for that many address ranges, right? so
> much >of that table may go unused if this is a significant overestimate
> (though that probably only happens if someone uses ld -r or, perhaps more
> >likely: LTO)]
>
> Yes, penalty is size. I am not aware about LTO,


In LTO (& ThinLTO, I think) it's expected that there will be object files
with multiple CUs (& thus multiple ranges)

Actually, for that matter, even in a non-LTO situation, especially in an
optimized build, the CU's range list won't be the only range list in the
debug_ranges section, so this may be quite a substantial overestimate for
optimized debug info.

I'll leave it up to Rui if that's a reasonable intermediate step to commit
- or whether you should find a solution to this overestimate before
committing this change.


> I knew about ld -r here only. I think perfomance is preferable over
> possible size penalty in general. And this patch just a first step in
> direction to use relocated data, so we can think about how to solve that
> after this change probably.
>
> >I wouldn't imagine it'd be too hard to look at enough relocation data
> (essentially parse the relocations applicable to debug_ranges and look for
> >holes (bytes that don't have relocations for them) - those are the ends of
> range lists - you wouldn't need to apply any relocations or examine >their
> addends, etc, only their offsets)
>
> There is one more issue I mentioned in description - is when address range
> belongs to discarded section. This case can not be catched until
> relocations are scanned at later stages, because we cant just scan
> relocations early when we do not know which sections were discarded.
> So since these issues are related, I would fix them in a separate
> patch(es). Scanning of relocations also will probably affect perfomance,
> and should be done in a some smart way.
>
>
> https://reviews.llvm.org/D31424
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170329/9c03778f/attachment.html>