[ELF] Drop .rel[a].debug_gnu_pub{names,types} for --gdb-index --emit-relocs

[MaskRay]

Summary

Fixes PR48693: --emit-relocs keeps relocation sections. --gdb-index drops
.debug_gnu_pubnames and .debug_gnu_pubtypes but not their relocation sections.
This can cause a null pointer dereference in getOutputSectionName.

Also delete debug-gnu-pubnames.s which is covered by gdb-index.s

[grimar]

Does this behavior of -gdb-index --emit-relocs match GNU ld?

lld/ELF/SyntheticSections.cpp

2887

if (InputSectionBase *rel = isec->getRelocatedSection())
  return !rel->isLive();

lld/test/ELF/gdb-index.s

4

I think we should test --gdb-index independently too.

55–56

Perhaps, just

SECTION-NOT: .debug_gnu_pub

And move the comment from above here to clarify that this line checks we
don't emit .debug_gnu_pubnames, .debug_gnu_pubtypes and their relocation sections if --emit-relocs is specified.

[MaskRay]

In D94354#2489729, @grimar wrote:

Does this behavior of -gdb-index --emit-relocs match GNU ld?

GNU ld does not have --gdb-index. The behavior matches gold.

lld/ELF/SyntheticSections.cpp

2887

if (!rel->isLive())
   return true;

is deliberate.

The suggested version is less reliable: if .debug_pubnames erroneously relocates other sections, it may still trigger a crash...

[MaskRay]

lld/ELF/SyntheticSections.cpp

2887

nvm. it is not needed.

[MaskRay]

comments

lld/test/ELF/gdb-index.s

55–56

Having both is probably clearer. Moved.

[grimar]

LGTM

[MaskRay]

improve a comment

[MaskRay]

drop an unneeded change

[grimar]

Still LG.

[MaskRay]

Fix test

lld/ELF/SyntheticSections.cpp

for (InputSectionBase *s : inputSections) {
     // .debug_gnu_pub{names,types} are useless in executables.
     // They are present in input object files solely for creating
     // a .gdb_index. So we can remove them from the output.
     if (s->name == ".debug_gnu_pubnames" || s->name == ".debug_gnu_pubtypes")
       s->markDead();
     else if (isec->name == ".debug_info")
       files.insert(isec->file);
   }
+  // Drop .rel[a].debug_gnu_pub{names,types} for --emit-relocs.
+  llvm::erase_if(inputSections, [](InputSectionBase *s) {
+    if (auto *isec = dyn_cast<InputSection>(s))
+      if (InputSectionBase *rel = isec->getRelocatedSection())
+        return !rel->isLive();
+    return !s->isLive();
+  });
 
   std::vector<GdbChunk> chunks(files.size());
   std::vector<std::vector<NameAttrEntry>> nameAttrs(files.size());
 
   parallelForEachN(0, files.size(), [&](size_t i) {
     // To keep memory usage low, we don't want to keep cached DWARFContext, so
     // avoid getDwarf() here.
     ObjFile<ELFT> *file = cast<ObjFile<ELFT>>(files[i]);

lld/test/ELF/debug-gnu-pubnames.s

-# REQUIRES: x86
-# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
-
-# RUN: ld.lld %t.o -o %t1.exe
-# RUN: llvm-readobj --sections %t1.exe | FileCheck %s
-# CHECK: .debug_gnu_pubnames
-# CHECK: .debug_gnu_pubtypes
-
-# RUN: ld.lld --gdb-index %t.o -o %t2.exe
-# RUN: llvm-readobj --sections %t2.exe | FileCheck %s --check-prefix=GDB
-# GDB-NOT: .debug_gnu_pubnames
-# GDB-NOT: .debug_gnu_pubtypes
-
-.section .debug_gnu_pubnames,"",@progbits
-.long 0
-
-.section .debug_gnu_pubtypes,"",@progbits
-.long 0

lld/test/ELF/gdb-index.s

 # REQUIRES: x86, zlib
 # RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t1.o
 # RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %p/Inputs/gdb-index.s -o %t2.o
 # RUN: ld.lld --gdb-index %t1.o %t2.o -o %t
 
 # RUN: llvm-objdump -d %t | FileCheck %s --check-prefix=DISASM
 # RUN: llvm-dwarfdump -gdb-index %t | FileCheck %s --check-prefix=DWARF
-# RUN: llvm-readelf -sections %t | FileCheck %s --check-prefix=SECTION
+
+## Drop .debug_gnu_pubnames and .debug_gnu_pubtypes.
+## Also drop their relocation sections if --emit-relocs is specified.
+# RUN: ld.lld --gdb-index --emit-relocs %t1.o %t2.o -o %t1
+# RUN: llvm-readelf --sections %t1 | FileCheck %s --check-prefix=SECTION
+
+# SECTION-NOT: .debug_gnu_pubnames
+# SECTION-NOT: .debug_gnu_pubtypes
 
 # RUN: llvm-mc -compress-debug-sections=zlib-gnu -filetype=obj -triple=x86_64-pc-linux \
 # RUN:   %p/Inputs/gdb-index.s -o %t2.o
 # RUN: ld.lld --gdb-index %t1.o %t2.o -o %t
 
 # RUN: llvm-objdump -d %t | FileCheck %s --check-prefix=DISASM
 # RUN: llvm-dwarfdump -gdb-index %t | FileCheck %s --check-prefix=DWARF
 # RUN: llvm-readelf -sections %t | FileCheck %s --check-prefix=SECTION

 # DWARF-NEXT:    754: Name offset = 0x38, CU vector offset = 0x10
 # DWARF-NEXT:      String name: int, CU vector index: 2
 # DWARF-NEXT:    822: Name offset = 0x2d, CU vector offset = 0x8
 # DWARF-NEXT:      String name: entrypoint, CU vector index: 1
 # DWARF:       Constant pool offset = 0x2060, has 3 CU vectors:
 # DWARF-NEXT:    0(0x0): 0x30000001
 # DWARF-NEXT:    1(0x8): 0x30000000
 # DWARF-NEXT:    2(0x10): 0x90000000 0x90000001
 
-# SECTION-NOT: debug_gnu_pubnames
-
 # RUN: ld.lld --gdb-index --no-gdb-index %t1.o %t2.o -o %t2
 # RUN: llvm-readobj --sections %t2 | FileCheck -check-prefix=NOGDB %s
 # NOGDB-NOT: Name: .gdb_index
 
 ## The following section contents are created by this using gcc 7.1.0:
 ## echo 'int entrypoint() { return 0; }' | gcc -gsplit-dwarf -xc++ -S -o- -
 
 .text
 .Ltext0:

 .uleb128 0x7
 .byte 0
 .byte 0
 .byte 0
 
 .section .debug_gnu_pubnames,"",@progbits
 .long 0x1e
 .value 0x2
-.long 0
+.long .debug_info
 .long 0x33
 .long 0x18
 .byte 0x30
 .string "entrypoint"
 .long 0
 
 .section .debug_gnu_pubtypes,"",@progbits
 .long 0x17
 .value 0x2
-.long 0
+.long .debug_info
 .long 0x33
 .long 0x2b
 .byte 0x90
 .string "int"
 .long 0