[ELF] - Fix error reporting with --strip-debug/--strip-all.

[grimar]

Summary

Currently LLD tries to use information about functions and variables location
taking it from debug sections. When --strip-* is given we discard such sections
and that breaks error reporting.
Patch stops discarding such sections and just removes them from InputSections list.

[ruiu]

I don't think it is a good idea to remove debug sections using the garbage collector. That is unnecessarily complicated. You should simply remove debug sections from InputSections before markLive().

[grimar]

• Changed implementation.

[ruiu]

LGTM

It's now much better.

ELF/Driver.cpp

1120–1122 ↗

(On Diff #121443)

Just the first half should suffice.

// We do not want to emit debug sections if --strip-all or -strip-debug are given.

ELF/InputFiles.cpp

InputSectionBase *ObjFile<ELFT>::createInputSection(const Elf_Shdr &Sec) {
   // Only GNU gold supports that. We don't. For the details about that,
   // see https://gcc.gnu.org/wiki/SplitStacks
   if (Name == ".note.GNU-split-stack") {
     error(toString(this) +
           ": object file compiled with -fsplit-stack is not supported");
     return &InputSection::Discarded;
   }
 
-  if (Config->Strip != StripPolicy::None && Name.startswith(".debug"))
-    return &InputSection::Discarded;
-
   // The linkonce feature is a sort of proto-comdat. Some glibc i386 object
   // files contain definitions of symbol "__x86.get_pc_thunk.bx" in linkonce
   // sections. Drop those sections to avoid duplicate symbol errors.
   // FIXME: This is glibc PR20543, we should remove this hack once that has been
   // fixed for a while.
   if (Name.startswith(".gnu.linkonce."))
     return &InputSection::Discarded;
 

ELF/MarkLive.cpp

for (InputSectionBase *Sec : InputSections) {
     }
   }
 
   // Mark all reachable sections.
   while (!Q.empty())
     forEachSuccessor<ELFT>(*Q.pop_back_val(), Enqueue);
 }
 
+// Returns true if debug section should be removed from output. Used for
+// implementation of --strip-debug/--strip-all. We can't just discard such
+// sections earlier because may want to use their content for error reporting.
+static bool shouldStrip(InputSectionBase *Sec) {
+  return (Config->Strip != StripPolicy::None) && Sec->Name.startswith(".debug");
+}
+
 // Before calling this function, Live bits are off for all
 // input sections. This function make some or all of them on
 // so that they are emitted to the output file.
 template <class ELFT> void elf::markLive() {
-  // If -gc-sections is missing, no sections are removed.
+  // If -gc-sections is missing, no sections are garbage collected, but we
+  // want to mark debug sections as dead if --strip-debug/--strip-all was used.
   if (!Config->GcSections) {
     for (InputSectionBase *Sec : InputSections)
-      Sec->Live = true;
+      Sec->Live = !shouldStrip(Sec);
     return;
   }
 
   // The -gc-sections option works only for SHF_ALLOC sections
   // (sections that are memory-mapped at runtime). So we can
   // unconditionally make non-SHF_ALLOC sections alive.
   //
   // Non SHF_ALLOC sections are not removed even if they are
   // unreachable through relocations because reachability is not
   // a good signal whether they are garbage or not (e.g. there is
   // usually no section referring to a .comment section, but we
   // want to keep it.)
   //
   // Note on SHF_REL{,A}: Such sections reach here only when -r
   // or -emit-reloc were given. And they are subject of garbage
   // collection because, if we remove a text section, we also
   // remove its relocation section.
   for (InputSectionBase *Sec : InputSections) {
     bool IsAlloc = (Sec->Flags & SHF_ALLOC);
     bool IsRel = (Sec->Type == SHT_REL || Sec->Type == SHT_RELA);
-    if (!IsAlloc && !IsRel)
+    if (!IsAlloc && !IsRel && !shouldStrip(Sec))
       Sec->Live = true;
   }
 
   // Follow the graph to mark all live sections.
   doGcSections<ELFT>();
 
   // Report garbage-collected sections.
   if (Config->PrintGcSections)

test/ELF/conflict-debug-variable.s

 # CHECK-NEXT: >>> defined at 1.c:2
 # CHECK-NEXT: >>>            {{.*}}:(.data+0x0)
 # CHECK:      duplicate symbol: foo
 # CHECK-NEXT: >>> defined at 1.c:1
 # CHECK-NEXT: >>>            {{.*}}:(foo)
 # CHECK-NEXT: >>> defined at 1.c:1
 # CHECK-NEXT: >>>            {{.*}}:(.bss+0x0)
 
+## Check that stripping debug sections does not break error reporting.
+# RUN: not ld.lld --strip-debug %t.o %t.o -o %t 2>&1 | FileCheck %s
+
 # Used reduced output from following code and gcc 7.1.0
 # to produce this input file:
 # Source (1.c):
 #  int foo = 0;
 #  int bar = 1;
 # Invocation: g++ -g -S 1.c
 
 .bss