[lld] 5acc6d4 - [lld-macho] Disambiguate bitcode files with the same name by archive name/offset in archive

Nico Weber via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 22 19:50:39 PDT 2021


Author: Leonard Grey
Date: 2021-07-22T22:50:25-04:00
New Revision: 5acc6d45727a8a603ad937e393d942eb7cc79a0d

URL: https://github.com/llvm/llvm-project/commit/5acc6d45727a8a603ad937e393d942eb7cc79a0d
DIFF: https://github.com/llvm/llvm-project/commit/5acc6d45727a8a603ad937e393d942eb7cc79a0d.diff

LOG: [lld-macho] Disambiguate bitcode files with the same name by archive name/offset in archive

Ported from COFF/ELF; test is adapted from
test/COFF/thinlto-archivecollision.ll

LTO expects every bitcode file to have a unique name. If given multiple bitcode
files with the same name, it errors with "Expected at most one ThinLTO module
per bitcode file".

This change incorporates the archive name, to disambiguate members with the
same name in different archives and the offset in archive to disambiguate
members with the same name in the same archive.

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

Added: 
    lld/test/MachO/lto-archivecollision.ll

Modified: 
    lld/MachO/Driver.cpp
    lld/MachO/Driver.h
    lld/MachO/DriverUtils.cpp
    lld/MachO/InputFiles.cpp
    lld/MachO/InputFiles.h

Removed: 
    


################################################################################
diff  --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp
index 4fab867fefc1e..a8c11b6994b95 100644
--- a/lld/MachO/Driver.cpp
+++ b/lld/MachO/Driver.cpp
@@ -228,6 +228,7 @@ namespace {
 struct ArchiveMember {
   MemoryBufferRef mbref;
   uint32_t modTime;
+  uint64_t offsetInArchive;
 };
 } // namespace
 
@@ -257,7 +258,7 @@ static std::vector<ArchiveMember> getArchiveMembers(MemoryBufferRef mb) {
         CHECK(c.getLastModified(), mb.getBufferIdentifier() +
                                        ": could not get the modification "
                                        "time for a child of the archive"));
-    v.push_back({mbref, modTime});
+    v.push_back({mbref, modTime, c.getChildOffset()});
   }
   if (err)
     fatal(mb.getBufferIdentifier() +
@@ -298,7 +299,8 @@ static InputFile *addFile(StringRef path, bool forceLoadArchive,
       if (Optional<MemoryBufferRef> buffer = readFile(path)) {
         for (const ArchiveMember &member : getArchiveMembers(*buffer)) {
           if (Optional<InputFile *> file = loadArchiveMember(
-                  member.mbref, member.modTime, path, /*objCOnly=*/false)) {
+                  member.mbref, member.modTime, path, /*objCOnly=*/false,
+                  member.offsetInArchive)) {
             inputFiles.insert(*file);
             printArchiveMemberLoad(
                 (forceLoadArchive ? "-force_load" : "-all_load"),
@@ -319,7 +321,8 @@ static InputFile *addFile(StringRef path, bool forceLoadArchive,
       if (Optional<MemoryBufferRef> buffer = readFile(path)) {
         for (const ArchiveMember &member : getArchiveMembers(*buffer)) {
           if (Optional<InputFile *> file = loadArchiveMember(
-                  member.mbref, member.modTime, path, /*objCOnly=*/true)) {
+                  member.mbref, member.modTime, path, /*objCOnly=*/true,
+                  member.offsetInArchive)) {
             inputFiles.insert(*file);
             printArchiveMemberLoad("-ObjC", inputFiles.back());
           }
@@ -343,7 +346,7 @@ static InputFile *addFile(StringRef path, bool forceLoadArchive,
     }
     break;
   case file_magic::bitcode:
-    newFile = make<BitcodeFile>(mbref);
+    newFile = make<BitcodeFile>(mbref, "", 0);
     break;
   case file_magic::macho_executable:
   case file_magic::macho_bundle:

diff  --git a/lld/MachO/Driver.h b/lld/MachO/Driver.h
index 1006cc1ac6eae..10f307e780c27 100644
--- a/lld/MachO/Driver.h
+++ b/lld/MachO/Driver.h
@@ -70,7 +70,8 @@ llvm::StringRef rerootPath(llvm::StringRef path);
 
 llvm::Optional<InputFile *> loadArchiveMember(MemoryBufferRef, uint32_t modTime,
                                               StringRef archiveName,
-                                              bool objCOnly);
+                                              bool objCOnly,
+                                              uint64_t offsetInArchive);
 
 uint32_t getModTime(llvm::StringRef path);
 

diff  --git a/lld/MachO/DriverUtils.cpp b/lld/MachO/DriverUtils.cpp
index 5f46812e0c1f6..fc25182d91405 100644
--- a/lld/MachO/DriverUtils.cpp
+++ b/lld/MachO/DriverUtils.cpp
@@ -280,7 +280,8 @@ StringRef macho::rerootPath(StringRef path) {
 Optional<InputFile *> macho::loadArchiveMember(MemoryBufferRef mb,
                                                uint32_t modTime,
                                                StringRef archiveName,
-                                               bool objCOnly) {
+                                               bool objCOnly,
+                                               uint64_t offsetInArchive) {
   if (config->zeroModTime)
     modTime = 0;
 
@@ -291,7 +292,7 @@ Optional<InputFile *> macho::loadArchiveMember(MemoryBufferRef mb,
     return None;
   case file_magic::bitcode:
     if (!objCOnly || check(isBitcodeContainingObjCCategory(mb)))
-      return make<BitcodeFile>(mb);
+      return make<BitcodeFile>(mb, archiveName, offsetInArchive);
     return None;
   default:
     error(archiveName + ": archive member " + mb.getBufferIdentifier() +

diff  --git a/lld/MachO/InputFiles.cpp b/lld/MachO/InputFiles.cpp
index e1fc35d2f534c..22b74ff73539a 100644
--- a/lld/MachO/InputFiles.cpp
+++ b/lld/MachO/InputFiles.cpp
@@ -1235,8 +1235,8 @@ void ArchiveFile::fetch(const object::Archive::Symbol &sym) {
   // to it later.
   const object::Archive::Symbol symCopy = sym;
 
-  if (Optional<InputFile *> file =
-          loadArchiveMember(mb, modTime, getName(), /*objCOnly=*/false)) {
+  if (Optional<InputFile *> file = loadArchiveMember(
+          mb, modTime, getName(), /*objCOnly=*/false, c.getChildOffset())) {
     inputFiles.insert(*file);
     // ld64 doesn't demangle sym here even with -demangle.
     // Match that: intentionally don't call toMachOString().
@@ -1275,8 +1275,22 @@ static macho::Symbol *createBitcodeSymbol(const lto::InputFile::Symbol &objSym,
                             /*noDeadStrip=*/false);
 }
 
-BitcodeFile::BitcodeFile(MemoryBufferRef mbref)
-    : InputFile(BitcodeKind, mbref) {
+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 members with the same name are provided, this causes a
+  // collision and ThinLTO can't proceed.
+  // So, we append the archive name to disambiguate two members with the same
+  // name from multiple 
diff erent archives, and offset within the archive to
+  // disambiguate two members of the same name from a single archive.
+  MemoryBufferRef mbref(
+      mb.getBuffer(),
+      saver.save(archiveName.empty() ? path
+                                     : archiveName + sys::path::filename(path) +
+                                           utostr(offsetInArchive)));
+
   obj = check(lto::InputFile::create(mbref));
 
   // Convert LTO Symbols to LLD Symbols in order to perform resolution. The

diff  --git a/lld/MachO/InputFiles.h b/lld/MachO/InputFiles.h
index 3bf9e77cf95a4..0101fb71c8a38 100644
--- a/lld/MachO/InputFiles.h
+++ b/lld/MachO/InputFiles.h
@@ -196,7 +196,8 @@ class ArchiveFile final : public InputFile {
 
 class BitcodeFile final : public InputFile {
 public:
-  explicit BitcodeFile(MemoryBufferRef mb);
+  explicit BitcodeFile(MemoryBufferRef mb, StringRef archiveName,
+                       uint64_t offsetInArchive);
   static bool classof(const InputFile *f) { return f->kind() == BitcodeKind; }
 
   std::unique_ptr<llvm::lto::InputFile> obj;

diff  --git a/lld/test/MachO/lto-archivecollision.ll b/lld/test/MachO/lto-archivecollision.ll
new file mode 100644
index 0000000000000..03999a4c0970e
--- /dev/null
+++ b/lld/test/MachO/lto-archivecollision.ll
@@ -0,0 +1,55 @@
+; RUN: rm -rf %t; split-file %s %t
+; RUN: mkdir %t/a %t/b
+; RUN: opt -thinlto-bc -o %t/main.o %t/main.ll
+; RUN: opt -thinlto-bc -o %t/a/bar.o %t/foo.ll
+; RUN: opt -thinlto-bc -o %t/b/bar.o %t/bar.ll
+; RUN: llvm-ar crs %t/libbar.a %t/a/bar.o %t/b/bar.o
+; RUN: %lld -save-temps %t/main.o %t/libbar.a -o %t/test
+; RUN: FileCheck %s --check-prefix=SAME-ARCHIVE < %t/test.resolution.txt
+
+; RUN: llvm-ar crs %t/liba.a %t/a/bar.o
+; RUN: llvm-ar crs %t/libb.a %t/b/bar.o
+; RUN: %lld -save-temps %t/main.o %t/liba.a %t/libb.a -o %t/test
+; RUN: FileCheck %s --check-prefix=DIFFERENT-ARCHIVES < %t/test.resolution.txt
+
+; SAME-ARCHIVE: libbar.abar.o[[#OFFSET:]]
+; SAME-ARCHIVE-NEXT: -r={{.*}}/libbar.abar.o[[#OFFSET:]],_foo,p
+; SAME-ARCHIVE-NEXT: libbar.abar.o[[#OTHEROFFSET:]]
+; SAME-ARCHIVE-NEXT: -r={{.*}}/libbar.abar.o[[#OTHEROFFSET:]],_bar,p
+
+; DIFFERENT-ARCHIVES: liba.abar.o[[#OFFSET:]]
+; DIFFERENT-ARCHIVES-NEXT: -r={{.*}}/liba.abar.o[[#OFFSET:]],_foo,p
+; DIFFERENT-ARCHIVES-NEXT: libb.abar.o[[#OTHEROFFSET:]]
+; DIFFERENT-ARCHIVES-NEXT: -r={{.*}}/libb.abar.o[[#OTHEROFFSET:]],_bar,p
+
+;--- main.ll
+
+target triple = "x86_64-apple-macosx10.15.0"
+target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+
+declare void @bar()
+declare void @foo()
+
+define i32 @main() {
+  call void @foo()
+  call void @bar()
+  ret i32 0
+}
+
+;--- foo.ll
+
+target triple = "x86_64-apple-macosx10.15.0"
+target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+
+define void @foo() {
+  ret void
+}
+
+;--- bar.ll
+
+target triple = "x86_64-apple-macosx10.15.0"
+target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+
+define void @bar() {
+  ret void
+}


        


More information about the llvm-commits mailing list