ELF: --gdb-index: Change findSection to return an InputSection.

[pcc]

Summary

We should only ever expect this function to return a regular
InputSection; I would not expect a function definition to be in a
MergeInputSection or EhInputSection. We were previously crashing
in writeTo if this function returned a section that was not an
InputSection because we do not set OutSec for such sections.

This can happen in practice if a function is defined in an empty
section which shares its offset-in-file with a MergeInputSection,
as in the provided test case.

[grimar]

So findSection() approach does not seem to be clean itself and that is root of problem it seems.
We probably can land this one to stop crashing (and as a bonus it adds testcase which also seems useful to have),
but for solving general issue I just posted D33183 which removes findSection() need and not only
speedup things but also should fix issue this patch addresses either.

I am OK to land this for start too. Rui ?

[ruiu]

How did you create that object file? At least you want to leave a comment in the test about how you created it.

lld/ELF/SyntheticSections.cpp

1707 ↗

(On Diff #98937)

Use dyn_cast instead of isa and cast.

[pcc]

In D33176#754713, @grimar wrote:

So findSection() approach does not seem to be clean itself and that is root of problem it seems.
We probably can land this one to stop crashing (and as a bonus it adds testcase which also seems useful to have),
but for solving general issue I just posted D33183 which removes findSection() need and not only
speedup things but also should fix issue this patch addresses either.

Agreed that we should fix the interface (thanks for working on that!), and that this is useful at least for the test case.

In D33176#755234, @ruiu wrote:

How did you create that object file? At least you want to leave a comment in the test about how you created it.

See lld/test/ELF/Inputs/gdb-index-empty.o.sh, but I'll convert this to a .s.

[pcc]

• Address review comments

[ruiu]

LGTM. Let's land this first to fix the crash bug.

[grimar]

lld/trunk/test/ELF/gdb-index-empty.s

15

I think this *.s can be significantly reduced.

Honestly I was fine with binary.
But if we have .s, I would expect to see much less code here.

lld/trunk/ELF/GdbIndex.h

 
 namespace lld {
 namespace elf {
 
 class InputSection;
 
 // Struct represents single entry of address area of gdb index.
 struct AddressEntry {
-  InputSectionBase *Section;
+  InputSection *Section;
   uint64_t LowAddress;
   uint64_t HighAddress;
   size_t CuIndex;
 };
 
 // Element of GdbHashTab hash table.
 struct GdbSymbol {
   GdbSymbol(uint32_t Hash, size_t Offset)

lld/trunk/ELF/SyntheticSections.cpp

 static std::vector<std::pair<uint64_t, uint64_t>>
 readCuList(DWARFContext &Dwarf, InputSection *Sec) {
   std::vector<std::pair<uint64_t, uint64_t>> Ret;
   for (std::unique_ptr<DWARFCompileUnit> &CU : Dwarf.compile_units())
     Ret.push_back({Sec->OutSecOff + CU->getOffset(), CU->getLength() + 4});
   return Ret;
 }
 
-static InputSectionBase *findSection(ArrayRef<InputSectionBase *> Arr,
-                                     uint64_t Offset) {
+static InputSection *findSection(ArrayRef<InputSectionBase *> Arr,
   for (InputSectionBase *S : Arr)
-    if (S && S != &InputSection::Discarded && S->Live)
-      if (Offset >= S->getOffsetInFile() &&
-          Offset < S->getOffsetInFile() + S->getSize())
-        return S;
+    if (auto *IS = dyn_cast_or_null<InputSection>(S))
+      if (IS != &InputSection::Discarded && IS->Live &&
+          Offset >= IS->getOffsetInFile() &&
+          Offset < IS->getOffsetInFile() + IS->getSize())
+        return IS;
   return nullptr;
 }
 
 static std::vector<AddressEntry>
 readAddressArea(DWARFContext &Dwarf, InputSection *Sec, size_t CurrentCU) {
   std::vector<AddressEntry> Ret;
 
   for (std::unique_ptr<DWARFCompileUnit> &CU : Dwarf.compile_units()) {
     DWARFAddressRangesVector Ranges;
     CU->collectAddressRanges(Ranges);
 
     ArrayRef<InputSectionBase *> Sections = Sec->File->getSections();
     for (std::pair<uint64_t, uint64_t> &R : Ranges)
-      if (InputSectionBase *S = findSection(Sections, R.first))
+      if (InputSection *S = findSection(Sections, R.first))
         Ret.push_back({S, R.first - S->getOffsetInFile(),
                        R.second - S->getOffsetInFile(), CurrentCU});
     ++CurrentCU;
   }
   return Ret;
 }
 
 static std::vector<std::pair<StringRef, uint8_t>>

lld/trunk/test/ELF/gdb-index-empty.s

+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux -o %t %s
+# RUN: ld.lld --gdb-index --gc-sections -o %t2 %t
+# RUN: llvm-dwarfdump -debug-dump=gdb_index %t2 | FileCheck %s
+
+# CHECK: Address area offset = 0x28, has 0 entries:
+
+# Generated with: (clang r302976)
+# echo "void _start() { __builtin_unreachable(); }" | \
+# clang -Os -g -S -o gdb-index-empty.s -x c - -Xclang -fdebug-compilation-dir -Xclang .
+
+  .text
+  .file "-"
+  .globl  _start
+  .type _start,@function
+_start:                                 # @_start
+.Lfunc_begin0:
+  .cfi_startproc
+# BB#0:                                 # %entry
+.Lfunc_end0:
+  .size _start, .Lfunc_end0-_start
+  .cfi_endproc
+
+  .file 1 "<stdin>"
+  .section  .debug_str,"MS",@progbits,1
+.Linfo_string0:
+  .asciz  "clang version 5.0.0 "  # string offset=0
+.Linfo_string1:
+  .asciz  "-"                     # string offset=21
+.Linfo_string2:
+  .asciz  "."                     # string offset=23
+.Linfo_string3:
+  .asciz  "_start"                # string offset=25
+  .section  .debug_loc,"",@progbits
+  .section  .debug_abbrev,"",@progbits
+  .byte 1                       # Abbreviation Code
+  .byte 17                      # DW_TAG_compile_unit
+  .byte 1                       # DW_CHILDREN_yes
+  .byte 37                      # DW_AT_producer
+  .byte 14                      # DW_FORM_strp
+  .byte 19                      # DW_AT_language
+  .byte 5                       # DW_FORM_data2
+  .byte 3                       # DW_AT_name
+  .byte 14                      # DW_FORM_strp
+  .byte 16                      # DW_AT_stmt_list
+  .byte 23                      # DW_FORM_sec_offset
+  .byte 27                      # DW_AT_comp_dir
+  .byte 14                      # DW_FORM_strp
+  .byte 17                      # DW_AT_low_pc
+  .byte 1                       # DW_FORM_addr
+  .byte 18                      # DW_AT_high_pc
+  .byte 6                       # DW_FORM_data4
+  .byte 0                       # EOM(1)
+  .byte 0                       # EOM(2)
+  .byte 2                       # Abbreviation Code
+  .byte 46                      # DW_TAG_subprogram
+  .byte 0                       # DW_CHILDREN_no
+  .byte 17                      # DW_AT_low_pc
+  .byte 1                       # DW_FORM_addr
+  .byte 18                      # DW_AT_high_pc
+  .byte 6                       # DW_FORM_data4
+  .byte 64                      # DW_AT_frame_base
+  .byte 24                      # DW_FORM_exprloc
+  .byte 3                       # DW_AT_name
+  .byte 14                      # DW_FORM_strp
+  .byte 58                      # DW_AT_decl_file
+  .byte 11                      # DW_FORM_data1
+  .byte 59                      # DW_AT_decl_line
+  .byte 11                      # DW_FORM_data1
+  .byte 63                      # DW_AT_external
+  .byte 25                      # DW_FORM_flag_present
+  .byte 0                       # EOM(1)
+  .byte 0                       # EOM(2)
+  .byte 0                       # EOM(3)
+  .section  .debug_info,"",@progbits
+.Lcu_begin0:
+  .long 60                      # Length of Unit
+  .short  4                       # DWARF version number
+  .long .debug_abbrev           # Offset Into Abbrev. Section
+  .byte 8                       # Address Size (in bytes)
+  .byte 1                       # Abbrev [1] 0xb:0x35 DW_TAG_compile_unit
+  .long .Linfo_string0          # DW_AT_producer
+  .short  12                      # DW_AT_language
+  .long .Linfo_string1          # DW_AT_name
+  .long .Lline_table_start0     # DW_AT_stmt_list
+  .long .Linfo_string2          # DW_AT_comp_dir
+  .quad .Lfunc_begin0           # DW_AT_low_pc
+  .long .Lfunc_end0-.Lfunc_begin0 # DW_AT_high_pc
+  .byte 2                       # Abbrev [2] 0x2a:0x15 DW_TAG_subprogram
+  .quad .Lfunc_begin0           # DW_AT_low_pc
+  .long .Lfunc_end0-.Lfunc_begin0 # DW_AT_high_pc
+  .byte 1                       # DW_AT_frame_base
+  .byte 87
+  .long .Linfo_string3          # DW_AT_name
+  .byte 1                       # DW_AT_decl_file
+  .byte 1                       # DW_AT_decl_line
+                                        # DW_AT_external
+  .byte 0                       # End Of Children Mark
+  .section  .debug_ranges,"",@progbits
+  .section  .debug_macinfo,"",@progbits
+.Lcu_macro_begin0:
+  .byte 0                       # End Of Macro List Mark
+  .section  .debug_pubnames,"",@progbits
+  .long .LpubNames_end0-.LpubNames_begin0 # Length of Public Names Info
+.LpubNames_begin0:
+  .short  2                       # DWARF Version
+  .long .Lcu_begin0             # Offset of Compilation Unit Info
+  .long 64                      # Compilation Unit Length
+  .long 42                      # DIE offset
+  .asciz  "_start"                # External Name
+  .long 0                       # End Mark
+.LpubNames_end0:
+
+  .ident  "clang version 5.0.0 "
+  .section  ".note.GNU-stack","",@progbits
+  .section  .debug_line,"",@progbits
+.Lline_table_start0: