[lld] r297049 - Fully precise gc handling of __start and __stop symbols.

[Rafael Espindola via llvm-commits]

Author: rafael
Date: Mon Mar  6 12:48:18 2017
New Revision: 297049

URL: http://llvm.org/viewvc/llvm-project?rev=297049&view=rev
Log:
Fully precise gc handling of __start and __stop symbols.

This puts us at parity with bfd, which could already gc this case.

I noticed the sections not being gced when linking a modified freebsd
kernel. A section that was not gced and not mentioned in the linker
script would end up breaking the expected layout. Since fixing the gc
is relatively simple and an improvement, that seems better than trying
to hack the orphan placement code.

There are 173 input section in the entire link whose names are valid C
identifiers, so this is probably not too performance critical.

Modified:
    lld/trunk/ELF/MarkLive.cpp
    lld/trunk/test/ELF/linkerscript/sections-gc2.s

Modified: lld/trunk/ELF/MarkLive.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/MarkLive.cpp?rev=297049&r1=297048&r2=297049&view=diff
==============================================================================
--- lld/trunk/ELF/MarkLive.cpp (original)
+++ lld/trunk/ELF/MarkLive.cpp Mon Mar  6 12:48:18 2017
@@ -22,6 +22,7 @@
 
 #include "InputSection.h"
 #include "LinkerScript.h"
+#include "Memory.h"
 #include "OutputSections.h"
 #include "Strings.h"
 #include "SymbolTable.h"
@@ -63,17 +64,25 @@ static typename ELFT::uint getAddend(Inp
   return Rel.r_addend;
 }
 
+// There are normally few input sections whose names are valid C
+// identifiers, so we just store a std::vector instead of a multimap.
+static DenseMap<StringRef, std::vector<InputSectionBase *>> CNamedSections;
+
 template <class ELFT, class RelT>
 static void resolveReloc(InputSectionBase &Sec, RelT &Rel,
                          std::function<void(ResolvedReloc)> Fn) {
   SymbolBody &B = Sec.getFile<ELFT>()->getRelocTargetSym(Rel);
-  auto *D = dyn_cast<DefinedRegular>(&B);
-  if (!D || !D->Section)
-    return;
-  typename ELFT::uint Offset = D->Value;
-  if (D->isSection())
-    Offset += getAddend<ELFT>(Sec, Rel);
-  Fn({D->Section->Repl, Offset});
+  if (auto *D = dyn_cast<DefinedRegular>(&B)) {
+    if (!D->Section)
+      return;
+    typename ELFT::uint Offset = D->Value;
+    if (D->isSection())
+      Offset += getAddend<ELFT>(Sec, Rel);
+    Fn({D->Section->Repl, Offset});
+  } else if (auto *U = dyn_cast<Undefined>(&B)) {
+    for (InputSectionBase *Sec : CNamedSections.lookup(U->getName()))
+      Fn({Sec, 0});
+  }
 }
 
 // Calls Fn for each section that Sec refers to via relocations.
@@ -184,6 +193,7 @@ template <class ELFT> static bool isRese
 // sections to set their "Live" bits.
 template <class ELFT> void elf::markLive() {
   SmallVector<InputSection *, 256> Q;
+  CNamedSections.clear();
 
   auto Enqueue = [&](ResolvedReloc R) {
     // Skip over discarded sections. This in theory shouldn't happen, because
@@ -223,22 +233,11 @@ template <class ELFT> void elf::markLive
   for (StringRef S : Config->Undefined)
     MarkSymbol(Symtab<ELFT>::X->find(S));
 
-  // Remember which __start_* or __stop_* symbols are used so that we don't gc
-  // those sections.
-  DenseSet<StringRef> UsedStartStopNames;
-
   // Preserve externally-visible symbols if the symbols defined by this
   // file can interrupt other ELF file's symbols at runtime.
-  for (const Symbol *S : Symtab<ELFT>::X->getSymbols()) {
-    if (auto *U = dyn_cast_or_null<Undefined>(S->body())) {
-      StringRef Name = U->getName();
-      for (StringRef Prefix : {"__start_", "__stop_"})
-        if (Name.startswith(Prefix))
-          UsedStartStopNames.insert(Name.substr(Prefix.size()));
-    } else if (S->includeInDynsym()) {
+  for (const Symbol *S : Symtab<ELFT>::X->getSymbols())
+    if (S->includeInDynsym())
       MarkSymbol(S->body());
-    }
-  }
 
   // Preserve special sections and those which are specified in linker
   // script KEEP command.
@@ -248,9 +247,12 @@ template <class ELFT> void elf::markLive
     // referred by .eh_frame here.
     if (auto *EH = dyn_cast_or_null<EhInputSection<ELFT>>(Sec))
       scanEhFrameSection<ELFT>(*EH, Enqueue);
-    if (isReserved<ELFT>(Sec) || Script<ELFT>::X->shouldKeep(Sec) ||
-        UsedStartStopNames.count(Sec->Name))
+    if (isReserved<ELFT>(Sec) || Script<ELFT>::X->shouldKeep(Sec))
       Enqueue({Sec, 0});
+    else if (isValidCIdentifier(Sec->Name)) {
+      CNamedSections[Saver.save("__start_" + Sec->Name)].push_back(Sec);
+      CNamedSections[Saver.save("__end_" + Sec->Name)].push_back(Sec);
+    }
   }
 
   // Mark all reachable sections.

Modified: lld/trunk/test/ELF/linkerscript/sections-gc2.s
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/ELF/linkerscript/sections-gc2.s?rev=297049&r1=297048&r2=297049&view=diff
==============================================================================
--- lld/trunk/test/ELF/linkerscript/sections-gc2.s (original)
+++ lld/trunk/test/ELF/linkerscript/sections-gc2.s Mon Mar  6 12:48:18 2017
@@ -25,7 +25,7 @@ _start:
         .quad 0
 
         .section used_in_script,"a"
-        .quad 0
+        .quad __start_used_in_script
 
         .section used_in_reloc,"a"
         .quad 0