From 8b1ec798b5620e10eaa49446af67b3191022f88d Mon Sep 17 00:00:00 2001 From: Bob Haarman Date: Mon, 15 Apr 2019 19:48:32 +0000 Subject: [PATCH] [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 | 16 ++++++++----- lld/COFF/Driver.h | 2 +- lld/COFF/InputFiles.cpp | 21 ++++++++++++++-- lld/COFF/InputFiles.h | 3 ++- lld/test/COFF/thinlto-archivecollision.ll | 29 +++++++++++++++++++++++ lld/test/COFF/thinlto-archives.ll | 8 +++---- 6 files changed, 65 insertions(+), 14 deletions(-) create mode 100644 lld/test/COFF/thinlto-archivecollision.ll diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp index 2d6aeb598fe3..92df6c839aa7 100644 --- a/lld/COFF/Driver.cpp +++ b/lld/COFF/Driver.cpp @@ -169,13 +169,13 @@ void LinkerDriver::addBuffer(std::unique_ptr MB, CHECK(Archive::create(MBRef), Filename + ": failed to parse archive"); for (MemoryBufferRef M : getArchiveMembers(File.get())) - addArchiveBuffer(M, "", Filename); + addArchiveBuffer(M, "", Filename, 0); return; } Symtab->addFile(make(MBRef)); break; case file_magic::bitcode: - Symtab->addFile(make(MBRef)); + Symtab->addFile(make(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(MB); @@ -224,7 +225,7 @@ void LinkerDriver::addArchiveBuffer(MemoryBufferRef MB, StringRef SymName, if (Magic == file_magic::coff_object) { Obj = make(MB); } else if (Magic == file_magic::bitcode) { - Obj = make(MB); + Obj = make(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 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 4ad473b9bee1..02c806741be5 100644 --- a/lld/COFF/Driver.h +++ b/lld/COFF/Driver.h @@ -118,7 +118,7 @@ private: void addBuffer(std::unique_ptr 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 3c7f4b418a65..2fa2f03fc474 100644 --- a/lld/COFF/InputFiles.cpp +++ b/lld/COFF/InputFiles.cpp @@ -769,9 +769,26 @@ void ImportFile::parse() { Name, cast_or_null(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> 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 8a81ea114f66..e4363e84961c 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 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 000000000000..3db00af745df --- /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 6a27115705b1..a3de6c9e557e 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"