[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

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);
   });
 }
 

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);
 

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

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;

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
+}

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"