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

[George Rimar via Phabricator via llvm-commits]

grimar added a comment.

In https://reviews.llvm.org/D31464#715325, @jhenderson wrote:

> Could we reduce the size of the test by removing many of the debug info attributes? I don't know what dwarfdump requires, but I expect we could get rid of basically all the DW_AT_... entries except the DW_AT_ranges tag.


I tried to reduce the testcase as much as possible in updated diff.



================
Comment at: ELF/InputSection.cpp:533-538
+    // If address range list contains entry that belongs to discarded
+    // section, we should perform a fix up. We can not write zero as start and
+    // end address because that would mean unexpected end of range. So we put a
+    // placeholder value equal to 1, that is correct to do because a range list
+    // entry whose begining and end adress are equal has no effect, because size
+    // of range is zero in such case.
----------------
jhenderson wrote:
> A few grammar nits and minor improvements to this comment. I suggest the following:
> 
> "If an address range list contains an entry that belongs to a discarded section, we should perform a fix-up. We cannot write zero as the start and end address because such a pair is an end of range list marker in the .debug_ranges section. We put a placeholder value equal to 1, which is fine because a range list entry whose beginning and end address are equal has no effect, because the size of the range is zero in such case."
Fixed. thanks !


================
Comment at: test/ELF/debug-ranges-reloc.s:4
+# RUN: ld.lld %t1.o -o %t -gc-sections -e main
+# RUN: llvm-dwarfdump -debug-dump=all %t | FileCheck %s
+
----------------
jhenderson wrote:
> We only need the -debug-dump=ranges, since we are only interested in the .debug_ranges output.
It does not work. Looks llvm-dwarfdump has an issue here (it does not dump anything with =ranges only for this testcase), I'll try to investigate and may be prepare patch for it separatelly. I suggest to go with =all for now.


================
Comment at: test/ELF/debug-ranges-reloc.s:11
+
+.text
+.section .text._Z3aaav,"ax", at progbits
----------------
jhenderson wrote:
> No need for this line.
Removed.


================
Comment at: test/ELF/debug-ranges-reloc.s:13-15
+.globl _Z3aaav
+.type _Z3aaav, at function
+_Z3aaav:
----------------
jhenderson wrote:
> I think we could get rid of these lines (similarly below in main).
Removed.


https://reviews.llvm.org/D31464