ELFAsmParser: Remove non-SHF_ALLOC or non-executable sections' line info/address ranges contribution for -g

[MaskRay]

Summary

I filed the issue https://sourceware.org/bugzilla/show_bug.cgi?id=26850 ,
which was acknowledged and fixed in GNU binutils 2.36

This patch adds the similar behavior to MC.

[dblaikie]

The new behavior seems reasonable and matches GNU as 2.36 https://sourceware.org/bugzilla/show_bug.cgi?id=26850

Probably worth including a bit more detail from that bug (it's useful to know that you filed/fixed that bug - not that that discredits the work, but clarifies that the sentiment (while endorsed by the bug/patches being approved by other members of the compiler community) was initially expressed/motivated/brought up by you there as well). Maybe something like "My patch to implement similar functionality in GNU binutils was accepted and released in GNU binutils 2.36" or the like might be suitable.

Does this LLVM patch include the same generalization discussed in the GNU Binutils bug ( https://sourceware.org/bugzilla/show_bug.cgi?id=26850#c2 ) regarding non-loaded sections?

llvm/test/MC/ARM/dwarf-asm-multiple-sections.s

79–85

Why the change to DWARFv3 here?

[MaskRay]

llvm/test/MC/ARM/dwarf-asm-multiple-sections.s

79–85

DWARF v3 and v4 use the same format. I think picking a check prefix which represents the lowest standard that something is implemented is reasonable.

[dblaikie]

llvm/test/MC/ARM/dwarf-asm-multiple-sections.s

79–85

Ah, sure - might be better suited as a separate patch, though?

[MaskRay]

Does this LLVM patch include the same generalization discussed in the GNU Binutils bug ( https://sourceware.org/bugzilla/show_bug.cgi?id=26850#c2 ) regarding non-loaded sections?

Thanks for spotting this! (I struggled whether SHF_EXECINSTR should imply SHF_ALLOC)
Since neither GNU as nor MC disallows "x", I assume it is meaningful and check the SHF_ALLOC flag.

[MaskRay]

Pre-commit improvement to the test file

[dblaikie]

Sounds good to me (I guess ALLOC and EXECINSTR can both be enabled separately? Is this consistent with the Binutils change? (or does one subsume the other?))

[MaskRay]

In D91505#2398603, @dblaikie wrote:

Sounds good to me (I guess ALLOC and EXECINSTR can both be enabled separately? Is this consistent with the Binutils change? (or does one subsume the other?))

Thanks for the review!

This is consistent with the GNU as change: only "ax" generates an address range.

llvm/lib/MC/MCParser/ELFAsmParser.cpp

Error(loc, "changed section type for " + SectionName + ", expected: 0x" +
                    utohexstr(Section->getType()));
   if (Section->getFlags() != Flags)
     Error(loc, "changed section flags for " + SectionName + ", expected: 0x" +
                    utohexstr(Section->getFlags()));
   if (Section->getEntrySize() != Size)
     Error(loc, "changed section entsize for " + SectionName +
                    ", expected: " + Twine(Section->getEntrySize()));
 
-  if (getContext().getGenDwarfForAssembly()) {
+  if (getContext().getGenDwarfForAssembly() &&
+      (Section->getFlags() & ELF::SHF_ALLOC) &&
+      (Section->getFlags() & ELF::SHF_EXECINSTR)) {
     bool InsertResult = getContext().addGenDwarfSection(Section);
     if (InsertResult) {
       if (getContext().getDwarfVersion() <= 2)
         Warning(loc, "DWARF2 only supports one section per compilation unit");
 
       if (!Section->getBeginSymbol()) {
         MCSymbol *SectionStartSymbol = getContext().createTempSymbol();
         getStreamer().emitLabel(SectionStartSymbol);

llvm/test/MC/ARM/dwarf-asm-multiple-sections.s

// RUN: not llvm-mc < %s -triple=armv7-linux-gnueabi -filetype=obj -o %t -g -dwarf-version 6 2>&1 | FileCheck -check-prefix DWARF6 %s
   .section .text, "ax"
 a:
   mov r0, r0
 
   .section foo, "ax"
 b:
   mov r1, r1
 
+// Non-executable sections do not contribute address ranges.
+  .section .rodata, "a"
+c:
+  mov r2, r2
+
+// Non-SHF_ALLOC sections do not contribute address ranges.
+  .section nonalloc, "x"
+d:
+  mov r3, r3
+
 // DWARF: .debug_abbrev contents:
 // DWARF: Abbrev table for offset: 0x00000000
 // DWARF: [1] DW_TAG_compile_unit DW_CHILDREN_yes
 // DWARF3:        DW_AT_stmt_list DW_FORM_data4
 // DWARF4:        DW_AT_stmt_list DW_FORM_sec_offset
 // DWARF5:        DW_AT_stmt_list DW_FORM_sec_offset
 // DWARF3:        DW_AT_ranges    DW_FORM_data4
 // DWARF4:        DW_AT_ranges    DW_FORM_sec_offset

 // DWARF-DL: version: [[DWVER]]
 // DWARF-DL-5:    address_size: 4
 // DWARF-DL-5:    include_directories[  0] = "/tmp"
 // DWARF-DL:      file_names[  [[DWFILE]]]:
 // DWARF-DL:      name: "{{(<stdin>|-)}}"
 // DWARF-DL-5:      0x0000000000000000     17      0      0   0   0  is_stmt
 // DWARF-DL-5-NEXT: 0x0000000000000004     17      0      0   0   0  is_stmt end_sequence
 // DWARF-DL-5-NEXT: 0x0000000000000000     21      0      0   0   0  is_stmt
 // DWARF-DL-5-NEXT: 0x0000000000000004     21      0      0   0   0  is_stmt end_sequence
 // DWARF-DL-3:      0x0000000000000000     17      0      1   0   0  is_stmt
 // DWARF-DL-3-NEXT: 0x0000000000000004     17      0      1   0   0  is_stmt end_sequence
 // DWARF-DL-3-NEXT: 0x0000000000000000     21      0      1   0   0  is_stmt
 // DWARF-DL-3-NEXT: 0x0000000000000004     21      0      1   0   0  is_stmt end_sequence
 // DWARF-DL-EMPTY:
 
 
 // DWARF34:      .debug_ranges contents:
 // DWARF34-NEXT: 00000000 ffffffff 00000000
 // DWARF34-NEXT: 00000000 00000000 00000004
 // DWARF34-NEXT: 00000000 ffffffff 00000000
 // DWARF34-NEXT: 00000000 00000000 00000004
 // DWARF34-NEXT: 00000000 <End of list>