[lld] 28848e9 - [lld][WebAssembly] Handle duplicate archive member names in ThinLTO

Sam Clegg via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 28 11:51:05 PDT 2021


Author: Sam Clegg
Date: 2021-10-28T11:48:04-07:00
New Revision: 28848e9e1bc06f9837cc653ddb9e7aff83d22320

URL: https://github.com/llvm/llvm-project/commit/28848e9e1bc06f9837cc653ddb9e7aff83d22320
DIFF: https://github.com/llvm/llvm-project/commit/28848e9e1bc06f9837cc653ddb9e7aff83d22320.diff

LOG: [lld][WebAssembly] Handle duplicate archive member names in ThinLTO

This entire change, including the test case, comes almost verbatim
from the ELF driver.

Fixes: https://github.com/emscripten-core/emscripten/issues/12763

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

Added: 
    lld/test/wasm/lto/Inputs/thin1.ll
    lld/test/wasm/lto/Inputs/thin2.ll
    lld/test/wasm/lto/thinlto-thin-archive-collision.ll

Modified: 
    lld/wasm/InputFiles.cpp
    lld/wasm/InputFiles.h

Removed: 
    


################################################################################
diff  --git a/lld/test/wasm/lto/Inputs/thin1.ll b/lld/test/wasm/lto/Inputs/thin1.ll
new file mode 100644
index 000000000000..d68a3f6bce84
--- /dev/null
+++ b/lld/test/wasm/lto/Inputs/thin1.ll
@@ -0,0 +1,14 @@
+; Copied from lld/test/ELF/lto/Inputs/thin1.ll
+
+target datalayout = "e-m:e-p:32:32-p10:8:8-p20:8:8-i64:64-n32:64-S128"
+target triple = "wasm32-unknown-unknown"
+
+define i32 @foo(i32 %goo) {
+entry:
+  %goo.addr = alloca i32, align 4
+  store i32 %goo, i32* %goo.addr, align 4
+  %0 = load i32, i32* %goo.addr, align 4
+  %1 = load i32, i32* %goo.addr, align 4
+  %mul = mul nsw i32 %0, %1
+  ret i32 %mul
+}

diff  --git a/lld/test/wasm/lto/Inputs/thin2.ll b/lld/test/wasm/lto/Inputs/thin2.ll
new file mode 100644
index 000000000000..a8db5d6cd41c
--- /dev/null
+++ b/lld/test/wasm/lto/Inputs/thin2.ll
@@ -0,0 +1,13 @@
+; Copied from lld/test/ELF/lto/Inputs/thin2.ll
+
+target datalayout = "e-m:e-p:32:32-p10:8:8-p20:8:8-i64:64-n32:64-S128"
+target triple = "wasm32-unknown-unknown"
+
+define i32 @blah(i32 %meh) #0 {
+entry:
+  %meh.addr = alloca i32, align 4
+  store i32 %meh, i32* %meh.addr, align 4
+  %0 = load i32, i32* %meh.addr, align 4
+  %sub = sub nsw i32 %0, 48
+  ret i32 %sub
+}

diff  --git a/lld/test/wasm/lto/thinlto-thin-archive-collision.ll b/lld/test/wasm/lto/thinlto-thin-archive-collision.ll
new file mode 100644
index 000000000000..d1000b371136
--- /dev/null
+++ b/lld/test/wasm/lto/thinlto-thin-archive-collision.ll
@@ -0,0 +1,28 @@
+; Copied from lld/test/ELF/lto/thinlto-thin-archive-collision.ll
+
+; RUN: rm -fr %t && mkdir %t && cd %t
+; RUN: mkdir thinlto-archives thinlto-archives/a thinlto-archives/b
+; RUN: opt -thinlto-bc -o thinlto-archives/main.o %s
+; RUN: opt -thinlto-bc -o thinlto-archives/a/thin.o %S/Inputs/thin1.ll
+; RUN: opt -thinlto-bc -o thinlto-archives/b/thin.o %S/Inputs/thin2.ll
+; RUN: llvm-ar qcT thinlto-archives/thin.a thinlto-archives/a/thin.o thinlto-archives/b/thin.o
+; RUN: wasm-ld thinlto-archives/main.o thinlto-archives/thin.a -o thinlto-archives/main.exe --save-temps
+; RUN: FileCheck %s < thinlto-archives/main.exe.resolution.txt
+
+; CHECK: thinlto-archives/main.o
+; CHECK: thinlto-archives/thin.a(thin.o at {{[1-9][0-9]+}})
+; CHECK-NEXT: -r=thinlto-archives/thin.a(thin.o at {{[1-9][0-9]+}}),foo,p
+; CHECK: thinlto-archives/thin.a(thin.o at {{[1-9][0-9]+}})
+; CHECK-NEXT: -r=thinlto-archives/thin.a(thin.o at {{[1-9][0-9]+}}),blah,p
+
+target datalayout = "e-m:e-p:32:32-p10:8:8-p20:8:8-i64:64-n32:64-S128"
+target triple = "wasm32-unknown-unknown"
+
+declare i32 @blah(i32 %meh)
+declare i32 @foo(i32 %goo)
+
+define i32 @_start() {
+  call i32 @foo(i32 0)
+  call i32 @blah(i32 0)
+  ret i32 0
+}

diff  --git a/lld/wasm/InputFiles.cpp b/lld/wasm/InputFiles.cpp
index 58da9cce0815..bf47f8bb2467 100644
--- a/lld/wasm/InputFiles.cpp
+++ b/lld/wasm/InputFiles.cpp
@@ -17,6 +17,7 @@
 #include "lld/Common/Reproduce.h"
 #include "llvm/Object/Binary.h"
 #include "llvm/Object/Wasm.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/TarWriter.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -25,6 +26,7 @@
 using namespace llvm;
 using namespace llvm::object;
 using namespace llvm::wasm;
+using namespace llvm::sys;
 
 namespace lld {
 
@@ -71,7 +73,8 @@ Optional<MemoryBufferRef> readFile(StringRef path) {
   return mbref;
 }
 
-InputFile *createObjectFile(MemoryBufferRef mb, StringRef archiveName) {
+InputFile *createObjectFile(MemoryBufferRef mb, StringRef archiveName,
+                            uint64_t offsetInArchive) {
   file_magic magic = identify_magic(mb.getBuffer());
   if (magic == file_magic::wasm_object) {
     std::unique_ptr<Binary> bin =
@@ -83,7 +86,7 @@ InputFile *createObjectFile(MemoryBufferRef mb, StringRef archiveName) {
   }
 
   if (magic == file_magic::bitcode)
-    return make<BitcodeFile>(mb, archiveName);
+    return make<BitcodeFile>(mb, archiveName, offsetInArchive);
 
   fatal("unknown file type: " + mb.getBufferIdentifier());
 }
@@ -709,7 +712,7 @@ void ArchiveFile::addMember(const Archive::Symbol *sym) {
             "could not get the buffer for the member defining symbol " +
                 sym->getName());
 
-  InputFile *obj = createObjectFile(mb, getName());
+  InputFile *obj = createObjectFile(mb, getName(), c.getChildOffset());
   symtab->addFile(obj);
 }
 
@@ -748,6 +751,32 @@ static Symbol *createBitcodeSymbol(const std::vector<bool> &keptComdats,
   return symtab->addDefinedData(name, flags, &f, nullptr, 0, 0);
 }
 
+BitcodeFile::BitcodeFile(MemoryBufferRef m, StringRef archiveName,
+                         uint64_t offsetInArchive)
+    : InputFile(BitcodeKind, m) {
+  this->archiveName = std::string(archiveName);
+
+  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.
+  StringRef name = archiveName.empty()
+                       ? saver.save(path)
+                       : saver.save(archiveName + "(" + path::filename(path) +
+                                    " at " + utostr(offsetInArchive) + ")");
+  MemoryBufferRef mbref(mb.getBuffer(), name);
+
+  obj = check(lto::InputFile::create(mbref));
+
+  // If this isn't part of an archive, it's eagerly linked, so mark it live.
+  if (archiveName.empty())
+    markLive();
+}
+
 bool BitcodeFile::doneLTO = false;
 
 void BitcodeFile::parse() {
@@ -756,8 +785,6 @@ void BitcodeFile::parse() {
     return;
   }
 
-  obj = check(lto::InputFile::create(MemoryBufferRef(
-      mb.getBuffer(), saver.save(archiveName + mb.getBufferIdentifier()))));
   Triple t(obj->getTargetTriple());
   if (!t.isWasm()) {
     error(toString(this) + ": machine type must be wasm32 or wasm64");

diff  --git a/lld/wasm/InputFiles.h b/lld/wasm/InputFiles.h
index 7c5305cdbfb3..cf2c1739e322 100644
--- a/lld/wasm/InputFiles.h
+++ b/lld/wasm/InputFiles.h
@@ -170,14 +170,8 @@ class SharedFile : public InputFile {
 // .bc file
 class BitcodeFile : public InputFile {
 public:
-  explicit BitcodeFile(MemoryBufferRef m, StringRef archiveName)
-      : InputFile(BitcodeKind, m) {
-    this->archiveName = std::string(archiveName);
-
-    // If this isn't part of an archive, it's eagerly linked, so mark it live.
-    if (archiveName.empty())
-      markLive();
-  }
+  BitcodeFile(MemoryBufferRef m, StringRef archiveName,
+              uint64_t offsetInArchive);
   static bool classof(const InputFile *f) { return f->kind() == BitcodeKind; }
 
   void parse();
@@ -194,7 +188,8 @@ inline bool isBitcode(MemoryBufferRef mb) {
 
 // Will report a fatal() error if the input buffer is not a valid bitcode
 // or wasm object file.
-InputFile *createObjectFile(MemoryBufferRef mb, StringRef archiveName = "");
+InputFile *createObjectFile(MemoryBufferRef mb, StringRef archiveName = "",
+                            uint64_t offsetInArchive = 0);
 
 // Opens a given file.
 llvm::Optional<MemoryBufferRef> readFile(StringRef path);


        


More information about the llvm-commits mailing list