[LLD][COFF] use offset in archive to disambiguate archive members

Summary:
Archives can contain multiple members with the same name. This would
cause ThinLTO links to fail ("Expected at most one ThinLTO module per
bitcode file"). This change implements the same strategy we use in
the ELF linker: make the offset in the archive part of the module
name so that names are unique.

Reviewers: pcc, mehdi_amini, ruiu

Reviewed By: ruiu

Subscribers: eraman, steven_wu, dexonsmith, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D60549

llvm-svn: 358440

6 files changed, 65 insertions, 14 deletions

diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index 2d6aeb598fe..92df6c839aa 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -169,13 +169,13 @@ void LinkerDriver::addBuffer(std::unique_ptr<MemoryBuffer> MB,
           CHECK(Archive::create(MBRef), Filename + ": failed to parse archive");
 
       for (MemoryBufferRef M : getArchiveMembers(File.get()))
-        addArchiveBuffer(M, "<whole-archive>", Filename);
+        addArchiveBuffer(M, "<whole-archive>", Filename, 0);
       return;
     }
     Symtab->addFile(make<ArchiveFile>(MBRef));
     break;
   case file_magic::bitcode:
-    Symtab->addFile(make<BitcodeFile>(MBRef));
+    Symtab->addFile(make<BitcodeFile>(MBRef, "", 0));
     break;
   case file_magic::coff_object:
   case file_magic::coff_import_library:
@@ -211,7 +211,8 @@ void LinkerDriver::enqueuePath(StringRef Path, bool WholeArchive) {
 }
 
 void LinkerDriver::addArchiveBuffer(MemoryBufferRef MB, StringRef SymName,
-                                    StringRef ParentName) {
+                                    StringRef ParentName,
+                                    uint64_t OffsetInArchive) {
   file_magic Magic = identify_magic(MB.getBuffer());
   if (Magic == file_magic::coff_import_library) {
     InputFile *Imp = make<ImportFile>(MB);
@@ -224,7 +225,7 @@ void LinkerDriver::addArchiveBuffer(MemoryBufferRef MB, StringRef SymName,
   if (Magic == file_magic::coff_object) {
     Obj = make<ObjFile>(MB);
   } else if (Magic == file_magic::bitcode) {
-    Obj = make<BitcodeFile>(MB);
+    Obj = make<BitcodeFile>(MB, ParentName, OffsetInArchive);
   } else {
     error("unknown file type: " + MB.getBufferIdentifier());
     return;
@@ -247,11 +248,14 @@ void LinkerDriver::enqueueArchiveMember(const Archive::Child &C,
   };
 
   if (!C.getParent()->isThin()) {
+    uint64_t OffsetInArchive = C.getChildOffset();
     Expected<MemoryBufferRef> MBOrErr = C.getMemoryBufferRef();
     if (!MBOrErr)
       ReportBufferError(MBOrErr.takeError(), check(C.getFullName()));
     MemoryBufferRef MB = MBOrErr.get();
-    enqueueTask([=]() { Driver->addArchiveBuffer(MB, SymName, ParentName); });
+    enqueueTask([=]() {
+      Driver->addArchiveBuffer(MB, SymName, ParentName, OffsetInArchive);
+    });
     return;
   }
 
@@ -266,7 +270,7 @@ void LinkerDriver::enqueueArchiveMember(const Archive::Child &C,
     if (MBOrErr.second)
       ReportBufferError(errorCodeToError(MBOrErr.second), ChildName);
     Driver->addArchiveBuffer(takeBuffer(std::move(MBOrErr.first)), SymName,
-                             ParentName);
+                             ParentName, /* OffsetInArchive */ 0);
   });
 }
 
diff --git a/lld/COFF/Driver.h b/lld/COFF/Driver.h
index 4ad473b9bee..02c806741be 100644
--- a/lld/COFF/Driver.h
+++ b/lld/COFF/Driver.h
@@ -118,7 +118,7 @@ private:
 
   void addBuffer(std::unique_ptr<MemoryBuffer> MB, bool WholeArchive);
   void addArchiveBuffer(MemoryBufferRef MBRef, StringRef SymName,
-                        StringRef ParentName);
+                        StringRef ParentName, uint64_t OffsetInArchive);
 
   void enqueuePath(StringRef Path, bool WholeArchive);
 
diff --git a/lld/COFF/InputFiles.cpp b/lld/COFF/InputFiles.cpp
index 3c7f4b418a6..2fa2f03fc47 100644
--- a/lld/COFF/InputFiles.cpp
+++ b/lld/COFF/InputFiles.cpp
@@ -769,9 +769,26 @@ void ImportFile::parse() {
         Name, cast_or_null<DefinedImportData>(ImpSym), Hdr->Machine);
 }
 
+BitcodeFile::BitcodeFile(MemoryBufferRef MB, StringRef ArchiveName,
+                         uint64_t OffsetInArchive)
+    : InputFile(BitcodeKind, MB) {
+  std::string Path = MB.getBufferIdentifier().str();
+
+  // ThinLTO assumes that all MemoryBufferRefs given to it have a unique
+  // name. If two archives define two members with the same name, this
+  // causes a collision which result in only one of the objects being taken
+  // into consideration at LTO time (which very likely causes undefined
+  // symbols later in the link stage). So we append file offset to make
+  // filename unique.
+  MemoryBufferRef MBRef(
+      MB.getBuffer(),
+      Saver.save(ArchiveName + Path +
+                 (ArchiveName.empty() ? "" : utostr(OffsetInArchive))));
+
+  Obj = check(lto::InputFile::create(MBRef));
+}
+
 void BitcodeFile::parse() {
-  Obj = check(lto::InputFile::create(MemoryBufferRef(
-      MB.getBuffer(), Saver.save(ParentName + MB.getBufferIdentifier()))));
   std::vector<std::pair<Symbol *, bool>> Comdat(Obj->getComdatTable().size());
   for (size_t I = 0; I != Obj->getComdatTable().size(); ++I)
     // FIXME: lto::InputFile doesn't keep enough data to do correct comdat
diff --git a/lld/COFF/InputFiles.h b/lld/COFF/InputFiles.h
index 8a81ea114f6..e4363e84961 100644
--- a/lld/COFF/InputFiles.h
+++ b/lld/COFF/InputFiles.h
@@ -295,7 +295,8 @@ public:
 // Used for LTO.
 class BitcodeFile : public InputFile {
 public:
-  explicit BitcodeFile(MemoryBufferRef M) : InputFile(BitcodeKind, M) {}
+  BitcodeFile(MemoryBufferRef MB, StringRef ArchiveName,
+              uint64_t OffsetInArchive);
   static bool classof(const InputFile *F) { return F->kind() == BitcodeKind; }
   ArrayRef<Symbol *> getSymbols() { return Symbols; }
   MachineTypes getMachineType() override;
diff --git a/lld/test/COFF/thinlto-archivecollision.ll b/lld/test/COFF/thinlto-archivecollision.ll
new file mode 100644
index 00000000000..3db00af745d
--- /dev/null
+++ b/lld/test/COFF/thinlto-archivecollision.ll
@@ -0,0 +1,29 @@
+; REQUIRES: x86
+; RUN: rm -fr %t
+; RUN: mkdir %t %t/a %t/b
+; RUN: opt -thinlto-bc -o %t/main.obj %s
+; RUN: opt -thinlto-bc -o %t/a/bar.obj %S/Inputs/lto-dep.ll
+; RUN: opt -thinlto-bc -o %t/b/bar.obj %S/Inputs/bar.ll
+; RUN: llvm-ar crs %t/libbar.lib %t/a/bar.obj %t/b/bar.obj
+; RUN: lld-link -out:%t/main.exe -entry:main -lldsavetemps \
+; RUN:     -subsystem:console %t/main.obj %t/libbar.lib
+; RUN: FileCheck %s < %t/main.exe.resolution.txt
+
+; CHECK: {{/thinlto-archivecollision.ll.tmp/main.obj$}}
+; CHECK: {{^-r=.*/thinlto-archivecollision.ll.tmp/main.obj,main,px$}}
+; CHECK: {{/thinlto-archivecollision.ll.tmp/libbar.libbar.obj[0-9]+$}}
+; CHECK-NEXT: {{^-r=.*/thinlto-archivecollision.ll.tmp/libbar.libbar.obj[0-9]+,foo,p$}}
+; CHECK-NEXT: {{/thinlto-archivecollision.ll.tmp/libbar.libbar.obj[0-9]+$}}
+; CHECK-NEXT: {{^-r=.*/thinlto-archivecollision.ll.tmp/libbar.libbar.obj[0-9]+,bar,p$}}
+
+target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-windows-msvc"
+
+declare void @bar()
+declare void @foo()
+
+define i32 @main() {
+  call void @foo()
+  call void @bar()
+  ret i32 0
+}
diff --git a/lld/test/COFF/thinlto-archives.ll b/lld/test/COFF/thinlto-archives.ll
index 6a27115705b..a3de6c9e557 100644
--- a/lld/test/COFF/thinlto-archives.ll
+++ b/lld/test/COFF/thinlto-archives.ll
@@ -13,10 +13,10 @@
 
 ; CHECK: {{/thinlto-archives/main.obj$}}
 ; CHECK: {{^-r=.*/thinlto-archives/main.obj,main,px$}}
-; CHECK: {{/thinlto-archives/a.libbar.obj$}}
-; CHECK-NEXT: {{^-r=.*/thinlto-archives/a.libbar.obj,foo,p$}}
-; CHECK-NEXT: {{/thinlto-archives/b.libbar.obj$}}
-; CHECK-NEXT: {{^-r=.*/thinlto-archives/b.libbar.obj,bar,p$}}
+; CHECK: {{/thinlto-archives/a.libbar.obj[0-9]+$}}
+; CHECK-NEXT: {{^-r=.*/thinlto-archives/a.libbar.obj[0-9]+,foo,p$}}
+; CHECK-NEXT: {{/thinlto-archives/b.libbar.obj[0-9]+$}}
+; CHECK-NEXT: {{^-r=.*/thinlto-archives/b.libbar.obj[0-9]+,bar,p$}}
 
 target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-pc-windows-msvc"