[PATCH] D30546: Fully precise gc handling of __start and __stop symbols

[Rafael Ávila de Espíndola via Phabricator via llvm-commits]

rafael created this revision.
Herald added a subscriber: emaste.

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 a std::vector is probably better than a multimap.


https://reviews.llvm.org/D30546

Files:
  ELF/MarkLive.cpp
  test/ELF/linkerscript/sections-gc2.s


Index: test/ELF/linkerscript/sections-gc2.s
===================================================================
--- test/ELF/linkerscript/sections-gc2.s
+++ test/ELF/linkerscript/sections-gc2.s
@@ -25,7 +25,7 @@
         .quad 0
 
         .section used_in_script,"a"
-        .quad 0
+        .quad __start_used_in_script
 
         .section used_in_reloc,"a"
         .quad 0
Index: ELF/MarkLive.cpp
===================================================================
--- ELF/MarkLive.cpp
+++ ELF/MarkLive.cpp
@@ -63,17 +63,32 @@
   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 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)) {
+    StringRef Name = U->getName();
+    for (StringRef Prefix : {"__start_", "__stop_"}) {
+      if (!Name.startswith(Prefix))
+        continue;
+      StringRef SecName = Name.substr(Prefix.size());
+      for (InputSectionBase *Sec : CNamedSections)
+        if (Sec->Name == SecName)
+          Fn({Sec, 0});
+    }
+  }
 }
 
 // Calls Fn for each section that Sec refers to via relocations.
@@ -223,22 +238,11 @@
   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 +252,10 @@
     // 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.push_back(Sec);
   }
 
   // Mark all reachable sections.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D30546.90366.patch
Type: text/x-patch
Size: 3374 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170302/7cb739c1/attachment.bin>