[lld-macho] Basic support for linkage and visibility attributes in LTO
When parsing bitcode, convert LTO Symbols to LLD Symbols in order to perform
resolution. The "winning" symbol will then be marked as Prevailing at LTO
compilation time. This is similar to what the other LLD ports do.
This change allows us to handle `linkonce` symbols correctly, and to deal with
duplicate bitcode symbols gracefully. Previously, both scenarios would result in
an assertion failure inside the LTO code, complaining that multiple Prevailing
definitions are not allowed.
While at it, I also added basic logic around visibility. We don't do anything
useful with it yet, but we do check that its value is valid. LLD-ELF appears to
use it only to set FinalDefinitionInLinkageUnit for LTO, which I think is just a
performance optimization.
From my local experimentation, the linker itself doesn't seem to do anything
differently when encountering linkonce / linkonce_odr / weak / weak_odr. So I've
only written a test for one of them. LLD-ELF has more, but they seem to mostly
be testing the intermediate bitcode output of their LTO backend...? I'm far from
an expert here though, so I might very well be missing things.
Reviewed By: #lld-macho, MaskRay, smeenai
Differential Revision: https://reviews.llvm.org/D94342
GitOrigin-RevId: 84579fc24f03c8ca778e70325dad2166f1deaee3
diff --git a/MachO/InputFiles.cpp b/MachO/InputFiles.cpp
index 238d9a4..249cde3 100644
--- a/MachO/InputFiles.cpp
+++ b/MachO/InputFiles.cpp
@@ -779,7 +779,41 @@
}
}
+static macho::Symbol *createBitcodeSymbol(const lto::InputFile::Symbol &objSym,
+ BitcodeFile &file) {
+ StringRef name = saver.save(objSym.getName());
+
+ // TODO: support weak references
+ if (objSym.isUndefined())
+ return symtab->addUndefined(name, &file, /*isWeakRef=*/false);
+
+ assert(!objSym.isCommon() && "TODO: support common symbols in LTO");
+
+ // TODO: Write a test demonstrating why computing isPrivateExtern before
+ // LTO compilation is important.
+ bool isPrivateExtern = false;
+ switch (objSym.getVisibility()) {
+ case GlobalValue::HiddenVisibility:
+ isPrivateExtern = true;
+ break;
+ case GlobalValue::ProtectedVisibility:
+ error(name + " has protected visibility, which is not supported by Mach-O");
+ break;
+ case GlobalValue::DefaultVisibility:
+ break;
+ }
+
+ return symtab->addDefined(name, &file, /*isec=*/nullptr, /*value=*/0,
+ objSym.isWeak(), isPrivateExtern);
+}
+
BitcodeFile::BitcodeFile(MemoryBufferRef mbref)
: InputFile(BitcodeKind, mbref) {
obj = check(lto::InputFile::create(mbref));
+
+ // Convert LTO Symbols to LLD Symbols in order to perform resolution. The
+ // "winning" symbol will then be marked as Prevailing at LTO compilation
+ // time.
+ for (const lto::InputFile::Symbol &objSym : obj->symbols())
+ symbols.push_back(createBitcodeSymbol(objSym, *this));
}
diff --git a/MachO/LTO.cpp b/MachO/LTO.cpp
index f48bc24..fb74968 100644
--- a/MachO/LTO.cpp
+++ b/MachO/LTO.cpp
@@ -10,6 +10,7 @@
#include "Config.h"
#include "Driver.h"
#include "InputFiles.h"
+#include "Symbols.h"
#include "lld/Common/ErrorHandler.h"
#include "lld/Common/Strings.h"
@@ -50,16 +51,24 @@
resols.reserve(objSyms.size());
// Provide a resolution to the LTO API for each symbol.
+ auto symIt = f.symbols.begin();
for (const lto::InputFile::Symbol &objSym : objSyms) {
resols.emplace_back();
lto::SymbolResolution &r = resols.back();
+ Symbol *sym = *symIt++;
// Ideally we shouldn't check for SF_Undefined but currently IRObjectFile
// reports two symbols for module ASM defined. Without this check, lld
// flags an undefined in IR with a definition in ASM as prevailing.
// Once IRObjectFile is fixed to report only one symbol this hack can
// be removed.
- r.Prevailing = !objSym.isUndefined();
+ r.Prevailing = !objSym.isUndefined() && sym->getFile() == &f;
+
+ // Un-define the symbol so that we don't get duplicate symbol errors when we
+ // load the ObjFile emitted by LTO compilation.
+ if (r.Prevailing)
+ replaceSymbol<Undefined>(sym, sym->getName(), sym->getFile(),
+ RefState::Strong);
// TODO: set the other resolution configs properly
r.VisibleToRegularObj = true;
diff --git a/test/MachO/invalid/duplicate-symbol.ll b/test/MachO/invalid/duplicate-symbol.ll
new file mode 100644
index 0000000..5fafe28
--- /dev/null
+++ b/test/MachO/invalid/duplicate-symbol.ll
@@ -0,0 +1,15 @@
+; REQUIRES: x86
+; RUN: rm -rf %t; mkdir -p %t
+; RUN: opt -module-summary %s -o %t/first.o
+; RUN: opt -module-summary %s -o %t/second.o
+; RUN: not %lld -dylib -lSystem %t/first.o %t/second.o -o /dev/null 2>&1 | FileCheck %s
+; CHECK: error: duplicate symbol: _foo
+; CHECK-NEXT: >>> defined in {{.*}}/first.o
+; CHECK-NEXT: >>> defined in {{.*}}/second.o
+
+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
+}
diff --git a/test/MachO/invalid/protected.ll b/test/MachO/invalid/protected.ll
new file mode 100644
index 0000000..f3dd49a
--- /dev/null
+++ b/test/MachO/invalid/protected.ll
@@ -0,0 +1,11 @@
+; REQUIRES: x86
+; RUN: opt -module-summary %s -o %t.o
+; RUN: not %lld -dylib -lSystem %t.o -o /dev/null 2>&1 | FileCheck %s
+; CHECK: error: _foo has protected visibility, which is not supported by Mach-O
+
+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 protected void @foo() {
+ ret void
+}
diff --git a/test/MachO/linkonce.ll b/test/MachO/linkonce.ll
new file mode 100644
index 0000000..a1f4c50
--- /dev/null
+++ b/test/MachO/linkonce.ll
@@ -0,0 +1,29 @@
+; REQUIRES: x86
+; RUN: rm -rf %t; split-file %s %t
+; RUN: opt -module-summary %t/first.ll -o %t/first.o
+; RUN: opt -module-summary %t/second.ll -o %t/second.o
+; RUN: %lld -dylib -lSystem %t/first.o %t/second.o -o %t/12
+; RUN: llvm-objdump --syms %t/12 | FileCheck %s --check-prefix=FIRST
+; RUN: %lld -dylib -lSystem %t/second.o %t/first.o -o %t/21
+; RUN: llvm-objdump --syms %t/21 | FileCheck %s --check-prefix=SECOND
+
+; FIRST: w O __TEXT,first _foo
+; SECOND: w O __TEXT,second _foo
+
+#--- first.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 linkonce void @foo() section "__TEXT,first" {
+ ret void
+}
+
+#--- second.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 linkonce void @foo() section "__TEXT,second" {
+ ret void
+}
