[lld] r241391 - COFF: Numerous fixes for interaction between LTO and weak externals.

Peter Collingbourne peter at pcc.me.uk
Fri Jul 3 22:28:42 PDT 2015


Author: pcc
Date: Sat Jul  4 00:28:41 2015
New Revision: 241391

URL: http://llvm.org/viewvc/llvm-project?rev=241391&view=rev
Log:
COFF: Numerous fixes for interaction between LTO and weak externals.

We were previously hitting assertion failures in the writer in cases where
a regular object file defined a weak external symbol that was defined by
a bitcode file. Because /export and /entry name mangling were implemented
using weak externals, the same problem affected mangled symbol names in
bitcode files.

The underlying cause of the problem was that weak external symbols were
being resolved before doing LTO, so the symbol table may have contained stale
references to bitcode symbols. The fix here is to defer weak external symbol
resolution until after LTO.

Also implement support for weak external symbols in bitcode files
by modelling them as replaceable DefinedBitcode symbols.

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

Added:
    lld/trunk/test/COFF/Inputs/entry-mangled.ll
    lld/trunk/test/COFF/Inputs/weak-external.ll
    lld/trunk/test/COFF/Inputs/weak-external2.ll
    lld/trunk/test/COFF/Inputs/weak-external3.ll
    lld/trunk/test/COFF/weak-external2.test
      - copied, changed from r241388, lld/trunk/test/COFF/weak-external.test
    lld/trunk/test/COFF/weak-external3.test
      - copied, changed from r241388, lld/trunk/test/COFF/weak-external.test
Modified:
    lld/trunk/COFF/Driver.cpp
    lld/trunk/COFF/InputFiles.cpp
    lld/trunk/COFF/SymbolTable.cpp
    lld/trunk/COFF/SymbolTable.h
    lld/trunk/COFF/Symbols.cpp
    lld/trunk/COFF/Symbols.h
    lld/trunk/test/COFF/entry-mangled.test
    lld/trunk/test/COFF/weak-external.test

Modified: lld/trunk/COFF/Driver.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/Driver.cpp?rev=241391&r1=241390&r2=241391&view=diff
==============================================================================
--- lld/trunk/COFF/Driver.cpp (original)
+++ lld/trunk/COFF/Driver.cpp Sat Jul  4 00:28:41 2015
@@ -594,10 +594,6 @@ bool LinkerDriver::link(llvm::ArrayRef<c
     }
   }
 
-  // Make sure we have resolved all symbols.
-  if (Symtab.reportRemainingUndefines())
-    return false;
-
   // Do LTO by compiling bitcode input files to a native COFF file
   // then link that file.
   if (auto EC = Symtab.addCombinedLTOObject()) {
@@ -605,6 +601,10 @@ bool LinkerDriver::link(llvm::ArrayRef<c
     return false;
   }
 
+  // Make sure we have resolved all symbols.
+  if (Symtab.reportRemainingUndefines(/*Resolve=*/true))
+    return false;
+
   // Windows specific -- if no /subsystem is given, we need to infer
   // that from entry point name.
   if (Config->Subsystem == IMAGE_SUBSYSTEM_UNKNOWN) {

Modified: lld/trunk/COFF/InputFiles.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/InputFiles.cpp?rev=241391&r1=241390&r2=241391&view=diff
==============================================================================
--- lld/trunk/COFF/InputFiles.cpp (original)
+++ lld/trunk/COFF/InputFiles.cpp Sat Jul  4 00:28:41 2015
@@ -293,8 +293,11 @@ std::error_code BitcodeFile::parse() {
     if (SymbolDef == LTO_SYMBOL_DEFINITION_UNDEFINED) {
       SymbolBodies.push_back(new (Alloc) Undefined(SymName));
     } else {
-      bool Replaceable = (SymbolDef == LTO_SYMBOL_DEFINITION_TENTATIVE ||
-                          (Attrs & LTO_SYMBOL_COMDAT));
+      bool Replaceable =
+          (SymbolDef == LTO_SYMBOL_DEFINITION_TENTATIVE || // common
+           (Attrs & LTO_SYMBOL_COMDAT) ||                  // comdat
+           (SymbolDef == LTO_SYMBOL_DEFINITION_WEAK &&     // weak external
+            (Attrs & LTO_SYMBOL_ALIAS)));
       SymbolBodies.push_back(new (Alloc) DefinedBitcode(this, SymName,
                                                         Replaceable));
     }

Modified: lld/trunk/COFF/SymbolTable.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/SymbolTable.cpp?rev=241391&r1=241390&r2=241391&view=diff
==============================================================================
--- lld/trunk/COFF/SymbolTable.cpp (original)
+++ lld/trunk/COFF/SymbolTable.cpp Sat Jul  4 00:28:41 2015
@@ -122,7 +122,7 @@ bool SymbolTable::queueEmpty() {
   return ArchiveQueue.empty() && ObjectQueue.empty();
 }
 
-bool SymbolTable::reportRemainingUndefines() {
+bool SymbolTable::reportRemainingUndefines(bool Resolve) {
   bool Ret = false;
   for (auto &I : Symtab) {
     Symbol *Sym = I.second;
@@ -130,20 +130,19 @@ bool SymbolTable::reportRemainingUndefin
     if (!Undef)
       continue;
     StringRef Name = Undef->getName();
-    // A weak alias may have been resolved, so check for that. A weak alias
-    // may be a weak alias to another symbol, so check recursively.
-    for (SymbolBody *A = Undef->WeakAlias; A;
-         A = cast<Undefined>(A)->WeakAlias) {
-      if (auto *D = dyn_cast<Defined>(A->repl())) {
+    // A weak alias may have been resolved, so check for that.
+    if (Defined *D = Undef->getWeakAlias()) {
+      if (Resolve)
         Sym->Body = D;
-        goto next;
-      }
+      continue;
     }
     // If we can resolve a symbol by removing __imp_ prefix, do that.
     // This odd rule is for compatibility with MSVC linker.
     if (Name.startswith("__imp_")) {
       Symbol *Imp = find(Name.substr(strlen("__imp_")));
       if (Imp && isa<Defined>(Imp->Body)) {
+        if (!Resolve)
+          continue;
         auto *D = cast<Defined>(Imp->Body);
         auto *S = new (Alloc) DefinedLocalImport(Name, D);
         LocalImportChunks.push_back(S->getChunk());
@@ -155,11 +154,11 @@ bool SymbolTable::reportRemainingUndefin
     // Remaining undefined symbols are not fatal if /force is specified.
     // They are replaced with dummy defined symbols.
     if (Config->Force) {
-      Sym->Body = new (Alloc) DefinedAbsolute(Name, 0);
+      if (Resolve)
+        Sym->Body = new (Alloc) DefinedAbsolute(Name, 0);
       continue;
     }
     Ret = true;
-    next:;
   }
   return Ret;
 }
@@ -299,6 +298,12 @@ std::error_code SymbolTable::addCombined
   if (BitcodeFiles.empty())
     return std::error_code();
 
+  // Diagnose any undefined symbols early, but do not resolve weak externals,
+  // as resolution breaks the invariant that each Symbol points to a unique
+  // SymbolBody, which we rely on to replace DefinedBitcode symbols correctly.
+  if (reportRemainingUndefines(/*Resolve=*/false))
+    return make_error_code(LLDError::BrokenFile);
+
   // Create an object file and add it to the symbol table by replacing any
   // DefinedBitcode symbols with the definitions in the object file.
   LTOCodeGenerator CG;
@@ -310,21 +315,12 @@ std::error_code SymbolTable::addCombined
   for (SymbolBody *Body : Obj->getSymbols()) {
     if (!Body->isExternal())
       continue;
-    // Find an existing Symbol. We should not see any new undefined symbols at
-    // this point.
+    // We should not see any new undefined symbols at this point, but we'll
+    // diagnose them later in reportRemainingUndefines().
     StringRef Name = Body->getName();
     Symbol *Sym = insert(Body);
-    if (Sym->Body == Body && !isa<Defined>(Body)) {
-      llvm::errs() << "LTO: undefined symbol: " << Name << '\n';
-      return make_error_code(LLDError::BrokenFile);
-    }
 
     if (isa<DefinedBitcode>(Sym->Body)) {
-      // The symbol should now be defined.
-      if (!isa<Defined>(Body)) {
-        llvm::errs() << "LTO: undefined symbol: " << Name << '\n';
-        return make_error_code(LLDError::BrokenFile);
-      }
       Sym->Body = Body;
       continue;
     }
@@ -353,9 +349,6 @@ std::error_code SymbolTable::addCombined
     return make_error_code(LLDError::BrokenFile);
   }
 
-  // New runtime library symbol references may have created undefined references.
-  if (reportRemainingUndefines())
-    return make_error_code(LLDError::BrokenFile);
   return std::error_code();
 }
 
@@ -375,9 +368,12 @@ ErrorOr<ObjectFile *> SymbolTable::creat
         CG->addMustPreserveSymbol(Body->getName());
 
   // Likewise for other symbols that must be preserved.
-  for (Undefined *U : Config->GCRoot)
-    if (isa<DefinedBitcode>(U->repl()))
-      CG->addMustPreserveSymbol(U->getName());
+  for (Undefined *U : Config->GCRoot) {
+    if (auto *S = dyn_cast<DefinedBitcode>(U->repl()))
+      CG->addMustPreserveSymbol(S->getName());
+    else if (auto *S = dyn_cast_or_null<DefinedBitcode>(U->getWeakAlias()))
+      CG->addMustPreserveSymbol(S->getName());
+  }
 
   CG->setModule(BitcodeFiles[0]->releaseModule());
   for (unsigned I = 1, E = BitcodeFiles.size(); I != E; ++I)

Modified: lld/trunk/COFF/SymbolTable.h
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/SymbolTable.h?rev=241391&r1=241390&r2=241391&view=diff
==============================================================================
--- lld/trunk/COFF/SymbolTable.h (original)
+++ lld/trunk/COFF/SymbolTable.h Sat Jul  4 00:28:41 2015
@@ -46,8 +46,9 @@ public:
   std::error_code run();
   bool queueEmpty();
 
-  // Print an error message on undefined symbols.
-  bool reportRemainingUndefines();
+  // Print an error message on undefined symbols. If Resolve is true, try to
+  // resolve any undefined symbols and update the symbol table accordingly.
+  bool reportRemainingUndefines(bool Resolve);
 
   // Returns a list of chunks of selected symbols.
   std::vector<Chunk *> getChunks();

Modified: lld/trunk/COFF/Symbols.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/Symbols.cpp?rev=241391&r1=241390&r2=241391&view=diff
==============================================================================
--- lld/trunk/COFF/Symbols.cpp (original)
+++ lld/trunk/COFF/Symbols.cpp Sat Jul  4 00:28:41 2015
@@ -236,5 +236,13 @@ ErrorOr<std::unique_ptr<InputFile>> Lazy
   return std::move(Obj);
 }
 
+Defined *Undefined::getWeakAlias() {
+  // A weak alias may be a weak alias to another symbol, so check recursively.
+  for (SymbolBody *A = WeakAlias; A; A = cast<Undefined>(A)->WeakAlias)
+    if (auto *D = dyn_cast<Defined>(A->repl()))
+      return D;
+  return nullptr;
+}
+
 } // namespace coff
 } // namespace lld

Modified: lld/trunk/COFF/Symbols.h
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/Symbols.h?rev=241391&r1=241390&r2=241391&view=diff
==============================================================================
--- lld/trunk/COFF/Symbols.h (original)
+++ lld/trunk/COFF/Symbols.h Sat Jul  4 00:28:41 2015
@@ -254,6 +254,11 @@ public:
   // If it remains undefined, it'll be replaced with whatever the
   // Alias pointer points to.
   SymbolBody *WeakAlias = nullptr;
+
+  // If this symbol is external weak, try to resolve it to a defined
+  // symbol by searching the chain of fallback symbols. Returns the symbol if
+  // successful, otherwise returns null.
+  Defined *getWeakAlias();
 };
 
 // Windows-specific classes.

Added: lld/trunk/test/COFF/Inputs/entry-mangled.ll
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/COFF/Inputs/entry-mangled.ll?rev=241391&view=auto
==============================================================================
--- lld/trunk/test/COFF/Inputs/entry-mangled.ll (added)
+++ lld/trunk/test/COFF/Inputs/entry-mangled.ll Sat Jul  4 00:28:41 2015
@@ -0,0 +1,6 @@
+target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-windows-msvc18.0.0"
+
+define void @"\01?main@@YAHXZ"() {
+  ret void
+}

Added: lld/trunk/test/COFF/Inputs/weak-external.ll
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/COFF/Inputs/weak-external.ll?rev=241391&view=auto
==============================================================================
--- lld/trunk/test/COFF/Inputs/weak-external.ll (added)
+++ lld/trunk/test/COFF/Inputs/weak-external.ll Sat Jul  4 00:28:41 2015
@@ -0,0 +1,6 @@
+target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-windows-msvc"
+
+define void @g() {
+  ret void
+}

Added: lld/trunk/test/COFF/Inputs/weak-external2.ll
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/COFF/Inputs/weak-external2.ll?rev=241391&view=auto
==============================================================================
--- lld/trunk/test/COFF/Inputs/weak-external2.ll (added)
+++ lld/trunk/test/COFF/Inputs/weak-external2.ll Sat Jul  4 00:28:41 2015
@@ -0,0 +1,6 @@
+target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-windows-msvc"
+
+define void @f() {
+  ret void
+}

Added: lld/trunk/test/COFF/Inputs/weak-external3.ll
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/COFF/Inputs/weak-external3.ll?rev=241391&view=auto
==============================================================================
--- lld/trunk/test/COFF/Inputs/weak-external3.ll (added)
+++ lld/trunk/test/COFF/Inputs/weak-external3.ll Sat Jul  4 00:28:41 2015
@@ -0,0 +1,8 @@
+target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-windows-msvc"
+
+ at f = weak alias void()* @g
+
+define void @g() {
+  ret void
+}

Modified: lld/trunk/test/COFF/entry-mangled.test
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/COFF/entry-mangled.test?rev=241391&r1=241390&r2=241391&view=diff
==============================================================================
--- lld/trunk/test/COFF/entry-mangled.test (original)
+++ lld/trunk/test/COFF/entry-mangled.test Sat Jul  4 00:28:41 2015
@@ -1,5 +1,7 @@
 # RUN: yaml2obj < %s > %t.obj
 # RUN: lld -flavor link2 /out:%t.exe /entry:main %t.obj
+# RUN: llvm-as -o %t.lto.obj %S/Inputs/entry-mangled.ll
+# RUN: lld -flavor link2 /out:%t.exe /entry:main %t.lto.obj
 
 ---
 header:

Modified: lld/trunk/test/COFF/weak-external.test
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/COFF/weak-external.test?rev=241391&r1=241390&r2=241391&view=diff
==============================================================================
--- lld/trunk/test/COFF/weak-external.test (original)
+++ lld/trunk/test/COFF/weak-external.test Sat Jul  4 00:28:41 2015
@@ -1,5 +1,12 @@
 # RUN: yaml2obj %s > %t.obj
-# RUN: lld -flavor link2 /out:%t.exe /entry:g /subsystem:console %t.obj
+# RUN: llvm-as -o %t.lto.obj %S/Inputs/weak-external.ll
+# RUN: lld -flavor link2 /out:%t1.exe /entry:g /subsystem:console %t.obj
+# RUN: lld -flavor link2 /out:%t2.exe /entry:g /subsystem:console /lldmap:%t2.map %t.obj %t.lto.obj
+# RUN: FileCheck %s < %t2.map
+
+# CHECK: lto-llvm{{.*}}:
+# CHECK-NOT: :
+# CHECK: {{ g$}}
 
 ---
 header:          

Copied: lld/trunk/test/COFF/weak-external2.test (from r241388, lld/trunk/test/COFF/weak-external.test)
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/COFF/weak-external2.test?p2=lld/trunk/test/COFF/weak-external2.test&p1=lld/trunk/test/COFF/weak-external.test&r1=241388&r2=241391&rev=241391&view=diff
==============================================================================
--- lld/trunk/test/COFF/weak-external.test (original)
+++ lld/trunk/test/COFF/weak-external2.test Sat Jul  4 00:28:41 2015
@@ -1,5 +1,6 @@
 # RUN: yaml2obj %s > %t.obj
-# RUN: lld -flavor link2 /out:%t.exe /entry:g /subsystem:console %t.obj
+# RUN: llvm-as -o %t.lto.obj %S/Inputs/weak-external2.ll
+# RUN: lld -flavor link2 /out:%t.exe /entry:g /subsystem:console %t.obj %t.lto.obj
 
 ---
 header:          
@@ -13,7 +14,7 @@ sections:
 symbols:         
   - Name:            'f'
     Value:           0
-    SectionNumber:   1
+    SectionNumber:   0
     SimpleType:      IMAGE_SYM_TYPE_NULL
     ComplexType:     IMAGE_SYM_DTYPE_FUNCTION
     StorageClass:    IMAGE_SYM_CLASS_EXTERNAL

Copied: lld/trunk/test/COFF/weak-external3.test (from r241388, lld/trunk/test/COFF/weak-external.test)
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/COFF/weak-external3.test?p2=lld/trunk/test/COFF/weak-external3.test&p1=lld/trunk/test/COFF/weak-external.test&r1=241388&r2=241391&rev=241391&view=diff
==============================================================================
--- lld/trunk/test/COFF/weak-external.test (original)
+++ lld/trunk/test/COFF/weak-external3.test Sat Jul  4 00:28:41 2015
@@ -1,5 +1,17 @@
 # RUN: yaml2obj %s > %t.obj
-# RUN: lld -flavor link2 /out:%t.exe /entry:g /subsystem:console %t.obj
+# RUN: llvm-as -o %t.lto.obj %S/Inputs/weak-external3.ll
+# RUN: lld -flavor link2 /out:%t1.exe /entry:f /subsystem:console /lldmap:%t1.map %t.lto.obj
+# RUN: FileCheck --check-prefix=CHECK1 %s < %t1.map
+# RUN: lld -flavor link2 /out:%t2.exe /entry:f /subsystem:console /lldmap:%t2.map %t.obj %t.lto.obj
+# RUN: FileCheck --check-prefix=CHECK2 %s < %t2.map
+
+# CHECK1: lto-llvm{{.*}}:
+# CHECK1-NOT: :
+# CHECK1: {{ g$}}
+
+# CHECK2: weak-external3{{.*}}:
+# CHECK2-NOT: :
+# CHECK2: {{ f$}}
 
 ---
 header:          
@@ -17,13 +29,4 @@ symbols:
     SimpleType:      IMAGE_SYM_TYPE_NULL
     ComplexType:     IMAGE_SYM_DTYPE_FUNCTION
     StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
-  - Name:            'g'
-    Value:           0
-    SectionNumber:   0
-    SimpleType:      IMAGE_SYM_TYPE_NULL
-    ComplexType:     IMAGE_SYM_DTYPE_FUNCTION
-    StorageClass:    IMAGE_SYM_CLASS_WEAK_EXTERNAL
-    WeakExternal:    
-      TagIndex:        0
-      Characteristics: IMAGE_WEAK_EXTERN_SEARCH_LIBRARY
 ...





More information about the llvm-commits mailing list