[PATCH] D31464: [ELF] - Stop producing broken entries in .debug_ranges section

[David Blaikie via llvm-commits]

If anything like this is going to be done - I suspect the right place to
discuss it is on the DWARF committee (or at the very least, crossposted
between llvm-dev and lldb-dev with all the Usual Suspects of debug info
support included (echristo, aprantl, Paul Robinson, myself))

On Thu, Mar 30, 2017 at 3:59 AM George Rimar via Phabricator <
reviews at reviews.llvm.org> wrote:

> grimar added a comment.
>
> In https://reviews.llvm.org/D31464#713928, @jhenderson wrote:
>
> > Generally looks okay to me. I've made a few inline comments.
> >
> > This approach works for .debug_ranges, because we are working with pairs
> of addresses, but we have to be more careful with other DWARF sections,
> because on some architectures, address zero is a valid address and should
> not be used, and we cannot simply set the size to zero, because we don't
> know where that is (if anywhere at all). gdb recognizes zero as special, if
> the architecture does not allow zero addresses, but otherwise it is not
> treated as special, and so we end up with (potentially) overlapping debug
> info entries which can cause problems with the debugger. Similarly,
> .debug_aranges uses an address and a fixed size, which we have to update
> correctly. There's a related discussion on the LLDB mailing list I started
> about handling GC'ed sections in the debugger:
> http://www.mail-archive.com/lldb-dev@lists.llvm.org/msg04140.html. The
> general agreement is that we should use a special non-zero address. I would
> suggest -1 (i.e. 0xFFFFFFFF in 32-bit DWARF addresses and
> 0xFFFFFFFFFFFFFFFF in 64-bit DWARF addresses), but we should have that
> discussion on the mailing lists.
>
>
> Interesting. I worried about other sections and so this patch touches only
> .debug_ranges. Thanks for your comments !
>
>
>
> ================
> Comment at: ELF/InputSection.cpp:487
>
> +static bool isDiscardedSection(SymbolBody &Body) {
> +  DefinedRegular *D = dyn_cast<DefinedRegular>(&Body);
> ----------------
> jhenderson wrote:
> > Should be called isInDiscardedSection, because it is working with a
> symbol not a section. "isDiscardedSection" suggests to me that it is asking
> if a section has been discarded or not, and so would take an InputSection
> or section index.
> I agree.
>
>
> ================
> Comment at: ELF/InputSection.cpp:510
> +
> +  bool IsDebugRanges = Name == ".debug_ranges";
>    for (const RelTy &Rel : Rels) {
> ----------------
> jhenderson wrote:
> > Is there anywhere else that we check section names? If so, I'd
> personally prefer if we set some flag on the section early rather than
> doing string comparisons on every input section to test the name multiple
> times.
> This method is isolated and used for doing relocations for non allocatable
> things like debug sections.
> I would not let this a bit wierd but reasonable logic to go outside.
>
>
> https://reviews.llvm.org/D31464
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170330/b2a106d3/attachment.html>