[PATCH] D46038: Also demote lazy symbols

[Rafael Avila de Espindola via Phabricator via llvm-commits]

espindola created this revision.
espindola added a reviewer: ruiu.
Herald added subscribers: arichardson, emaste.

This is not a big simplification right now, but the special cases for lazy symbols have been a common source of bugs in the past.


https://reviews.llvm.org/D46038

Files:
  ELF/Driver.cpp
  ELF/Symbols.cpp
  ELF/Symbols.h


Index: ELF/Symbols.h
===================================================================
--- ELF/Symbols.h
+++ ELF/Symbols.h
@@ -95,12 +95,8 @@
     return SymbolKind == LazyArchiveKind || SymbolKind == LazyObjectKind;
   }
 
-  // True if this is an undefined weak symbol. This only works once
-  // all input files have been added.
-  bool isUndefWeak() const {
-    // See comment on lazy symbols for details.
-    return isWeak() && (isUndefined() || isLazy());
-  }
+  // True if this is an undefined weak symbol.
+  bool isUndefWeak() const { return isWeak() && isUndefined(); }
 
   StringRef getName() const { return Name; }
   void parseSymbolVersion();
Index: ELF/Symbols.cpp
===================================================================
--- ELF/Symbols.cpp
+++ ELF/Symbols.cpp
@@ -109,8 +109,7 @@
     return 0;
   case Symbol::LazyArchiveKind:
   case Symbol::LazyObjectKind:
-    assert(Sym.IsUsedInRegularObj && "lazy symbol reached writer");
-    return 0;
+    llvm_unreachable("lazy symbol reached writer");
   }
   llvm_unreachable("invalid symbol kind");
 }
Index: ELF/Driver.cpp
===================================================================
--- ELF/Driver.cpp
+++ ELF/Driver.cpp
@@ -1119,19 +1119,27 @@
     Symtab->fetchLazy<ELFT>(Sym);
 }
 
-// If all references to a DSO happen to be weak, the DSO is not added
-// to DT_NEEDED. If that happens, we need to eliminate shared symbols
-// created from the DSO. Otherwise, they become dangling references
-// that point to a non-existent DSO.
-template <class ELFT> static void demoteSharedSymbols() {
+template <class ELFT> static bool shouldDemote(Symbol &Sym) {
+  // If all references to a DSO happen to be weak, the DSO is not added to
+  // DT_NEEDED. If that happens, we need to eliminate shared symbols created
+  // from the DSO. Otherwise, they become dangling references that point to a
+  // non-existent DSO.
+  if (auto *S = dyn_cast<SharedSymbol>(&Sym))
+    return !S->getFile<ELFT>().IsNeeded;
+
+  // We are done processing archives, so lazy symbols that were used but not
+  // found can be converted to undefined. We could also just delete the other
+  // lazy symbols, but that seems to be more work than it is worth.
+  return Sym.isLazy() && Sym.IsUsedInRegularObj;
+}
+
+template <class ELFT> static void demoteSymbols() {
   for (Symbol *Sym : Symtab->getSymbols()) {
-    if (auto *S = dyn_cast<SharedSymbol>(Sym)) {
-      if (!S->getFile<ELFT>().IsNeeded) {
-        bool Used = S->Used;
-        replaceSymbol<Undefined>(S, nullptr, S->getName(), STB_WEAK, S->StOther,
-                                 S->Type);
-        S->Used = Used;
-      }
+    if (shouldDemote<ELFT>(*Sym)) {
+      bool Used = Sym->Used;
+      replaceSymbol<Undefined>(Sym, nullptr, Sym->getName(), Sym->Binding,
+                               Sym->StOther, Sym->Type);
+      Sym->Used = Used;
     }
   }
 }
@@ -1296,7 +1304,7 @@
   // Do size optimizations: garbage collection, merging of SHF_MERGE sections
   // and identical code folding.
   markLive<ELFT>();
-  demoteSharedSymbols<ELFT>();
+  demoteSymbols<ELFT>();
   decompressSections();
   mergeSections();
   if (Config->ICF)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D46038.143851.patch
Type: text/x-patch
Size: 3177 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180425/29622cc4/attachment.bin>