[lld] 84579fc - [lld-macho] Basic support for linkage and visibility attributes in LTO
Jez Ng via llvm-commits
llvm-commits at lists.llvm.org
Thu Feb 25 10:27:56 PST 2021
Author: Jez Ng
Date: 2021-02-25T13:27:40-05:00
New Revision: 84579fc24f03c8ca778e70325dad2166f1deaee3
URL: https://github.com/llvm/llvm-project/commit/84579fc24f03c8ca778e70325dad2166f1deaee3
DIFF: https://github.com/llvm/llvm-project/commit/84579fc24f03c8ca778e70325dad2166f1deaee3.diff
LOG: [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
Added:
lld/test/MachO/invalid/duplicate-symbol.ll
lld/test/MachO/invalid/protected.ll
lld/test/MachO/linkonce.ll
Modified:
lld/MachO/InputFiles.cpp
lld/MachO/LTO.cpp
Removed:
################################################################################
diff --git a/lld/MachO/InputFiles.cpp b/lld/MachO/InputFiles.cpp
index 238d9a4a8496..249cde3c8385 100644
--- a/lld/MachO/InputFiles.cpp
+++ b/lld/MachO/InputFiles.cpp
@@ -779,7 +779,41 @@ void ArchiveFile::fetch(const object::Archive::Symbol &sym) {
}
}
+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/lld/MachO/LTO.cpp b/lld/MachO/LTO.cpp
index f48bc24df3d7..fb749687e864 100644
--- a/lld/MachO/LTO.cpp
+++ b/lld/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 @@ void BitcodeCompiler::add(BitcodeFile &f) {
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/lld/test/MachO/invalid/duplicate-symbol.ll b/lld/test/MachO/invalid/duplicate-symbol.ll
new file mode 100644
index 000000000000..5fafe2816659
--- /dev/null
+++ b/lld/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/lld/test/MachO/invalid/protected.ll b/lld/test/MachO/invalid/protected.ll
new file mode 100644
index 000000000000..f3dd49a632d1
--- /dev/null
+++ b/lld/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/lld/test/MachO/linkonce.ll b/lld/test/MachO/linkonce.ll
new file mode 100644
index 000000000000..a1f4c507123a
--- /dev/null
+++ b/lld/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
+}
More information about the llvm-commits
mailing list