ELF: Undefine all symbols, not just those that we expect to be defined.

This allows the combined LTO object to provide a definition with the same
name as a symbol that was internalized without causing a duplicate symbol
error. This normally happens during parallel codegen which externalizes
originally-internal symbols, for example.

In order to make this work, I needed to relax the undefined symbol error to
only report an error for symbols that are used in regular objects.

Differential Revision: http://reviews.llvm.org/D19954

llvm-svn: 268649

lld/ELF/LTO.cpp

@@ -106,13 +106,6 @@ void BitcodeCompiler::add(BitcodeFile &F) {
   SmallPtrSet<GlobalValue *, 8> Used;
   collectUsedGlobalVariables(M, Used, /* CompilerUsed */ false);
 
-  // This function is called if we know that the combined LTO object will
-  // provide a definition of a symbol. It undefines the symbol so that the
-  // definition in the combined LTO object will replace it when parsed.
-  auto Undefine = [](Symbol *S) {
-    replaceBody<Undefined>(S, S->body()->getName(), STV_DEFAULT, 0);
-  };
-
   for (const BasicSymbolRef &Sym : Obj->symbols()) {
     uint32_t Flags = Sym.getFlags();
     GlobalValue *GV = Obj->getSymbolGV(Sym.getRawDataRefImpl());
@@ -123,14 +116,30 @@ void BitcodeCompiler::add(BitcodeFile &F) {
     Symbol *S = Syms[BodyIndex++];
     if (Flags & BasicSymbolRef::SF_Undefined)
       continue;
-    if (!GV) {
-      // Module asm symbol.
-      Undefine(S);
-      continue;
-    }
     auto *B = dyn_cast<DefinedBitcode>(S->body());
     if (!B || B->File != &F)
       continue;
+
+    // We collect the set of symbols we want to internalize here
+    // and change the linkage after the IRMover executed, i.e. after
+    // we imported the symbols and satisfied undefined references
+    // to it. We can't just change linkage here because otherwise
+    // the IRMover will just rename the symbol.
+    if (GV && shouldInternalize(Used, S, GV))
+      InternalizedSyms.insert(GV->getName());
+
+    // At this point we know that either the combined LTO object will provide a
+    // definition of a symbol, or we will internalize it. In either case, we
+    // need to undefine the symbol. In the former case, the real definition
+    // needs to be able to replace the original definition without conflicting.
+    // In the latter case, we need to allow the combined LTO object to provide a
+    // definition with the same name, for example when doing parallel codegen.
+    replaceBody<Undefined>(S, S->body()->getName(), STV_DEFAULT, 0);
+
+    if (!GV)
+      // Module asm symbol.
+      continue;
+
     switch (GV->getLinkage()) {
     default:
       break;
@@ -142,16 +151,6 @@ void BitcodeCompiler::add(BitcodeFile &F) {
       break;
     }
 
-    // We collect the set of symbols we want to internalize here
-    // and change the linkage after the IRMover executed, i.e. after
-    // we imported the symbols and satisfied undefined references
-    // to it. We can't just change linkage here because otherwise
-    // the IRMover will just rename the symbol.
-    if (shouldInternalize(Used, S, GV))
-      InternalizedSyms.insert(GV->getName());
-    else
-      Undefine(S);
-
     Keep.push_back(GV);
   }
 

lld/ELF/Writer.cpp

@@ -1391,7 +1391,9 @@ template <class ELFT> void Writer<ELFT>::createSections() {
       if (auto *SS = dyn_cast<SharedSymbol<ELFT>>(Body))
         SS->File->IsUsed = true;
 
-    if (Body->isUndefined() && !S->isWeak())
+    // We only report undefined symbols in regular objects. This means that we
+    // will accept an undefined reference in bitcode if it can be optimized out.
+    if (S->IsUsedInRegularObj && Body->isUndefined() && !S->isWeak())
       reportUndefined<ELFT>(Symtab, Body);
 
     if (auto *C = dyn_cast<DefinedCommon>(Body))

lld/test/ELF/lto/parallel-internalize.ll

@@ -0,0 +1,57 @@
+; REQUIRES: x86
+; RUN: llvm-as -o %t.bc %s
+; RUN: ld.lld -m elf_x86_64 --lto-jobs=2 -save-temps -o %t %t.bc -e foo --lto-O0
+; RUN: llvm-readobj -t -dyn-symbols %t | FileCheck %s
+; RUN: llvm-nm %t0.lto.o | FileCheck --check-prefix=CHECK0 %s
+; RUN: llvm-nm %t1.lto.o | FileCheck --check-prefix=CHECK1 %s
+
+; CHECK:      Symbols [
+; CHECK-NEXT:   Symbol {
+; CHECK-NEXT:     Name:  (0)
+; CHECK-NEXT:     Value: 0x0
+; CHECK-NEXT:     Size: 0
+; CHECK-NEXT:     Binding: Local (0x0)
+; CHECK-NEXT:     Type: None (0x0)
+; CHECK-NEXT:     Other: 0
+; CHECK-NEXT:     Section: Undefined (0x0)
+; CHECK-NEXT:   }
+; CHECK-NEXT:   Symbol {
+; CHECK-NEXT:     Name: bar (5)
+; CHECK-NEXT:     Value: 0x11010
+; CHECK-NEXT:     Size: 8
+; CHECK-NEXT:     Binding: Local (0x0)
+; CHECK-NEXT:     Type: Function (0x2)
+; CHECK-NEXT:     Other [ (0x2)
+; CHECK-NEXT:       STV_HIDDEN (0x2)
+; CHECK-NEXT:     ]
+; CHECK-NEXT:     Section: .text (0x2)
+; CHECK-NEXT:   }
+; CHECK-NEXT:   Symbol {
+; CHECK-NEXT:     Name: foo (1)
+; CHECK-NEXT:     Value: 0x11000
+; CHECK-NEXT:     Size: 8
+; CHECK-NEXT:     Binding: Global (0x1)
+; CHECK-NEXT:     Type: Function (0x2)
+; CHECK-NEXT:     Other: 0
+; CHECK-NEXT:     Section: .text (0x2)
+; CHECK-NEXT:   }
+; CHECK-NEXT: ]
+; CHECK-NEXT: DynamicSymbols [
+; CHECK-NEXT: ]
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; CHECK0: U bar
+; CHECK0: T foo
+define void @foo() {
+  call void @bar()
+  ret void
+}
+
+; CHECK1: T bar
+; CHECK1: U foo
+define void @bar() {
+  call void @foo()
+  ret void
+}