[llvm] 785d376 - [ORC][JITLink] Treat common symbols as weak definitions.
Lang Hames via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 23 20:12:21 PDT 2024
Author: Lang Hames
Date: 2024-07-24T13:12:14+10:00
New Revision: 785d376d1231167688dd12f93c5c0a5d46cd4086
URL: https://github.com/llvm/llvm-project/commit/785d376d1231167688dd12f93c5c0a5d46cd4086
DIFF: https://github.com/llvm/llvm-project/commit/785d376d1231167688dd12f93c5c0a5d46cd4086.diff
LOG: [ORC][JITLink] Treat common symbols as weak definitions.
Duplicate common definitions should be coaleseced, rather than being treated as
duplicate definitions. Strong definitions should override common definitions.
rdar://132314264
Added:
llvm/test/ExecutionEngine/JITLink/AArch64/Inputs/MachO_common_x_and_addr_getter.s
llvm/test/ExecutionEngine/JITLink/AArch64/Inputs/MachO_strong_x_and_addr_getter.s
llvm/test/ExecutionEngine/JITLink/AArch64/MachO_common_symbol_x_multiple_defs.s
Modified:
llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp
llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h
llvm/lib/ExecutionEngine/JITLink/MachOLinkGraphBuilder.cpp
llvm/lib/ExecutionEngine/Orc/Core.cpp
llvm/test/ExecutionEngine/JITLink/x86-64/COFF_common_symbol.s
Removed:
################################################################################
diff --git a/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp b/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp
index 1fd2a33d3f11f..92630650ec937 100644
--- a/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp
+++ b/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp
@@ -467,7 +467,7 @@ Expected<Symbol *> COFFLinkGraphBuilder::createDefinedSymbol(
return &G->addDefinedSymbol(
G->createZeroFillBlock(getCommonSection(), Symbol.getValue(),
orc::ExecutorAddr(), Symbol.getValue(), 0),
- 0, SymbolName, Symbol.getValue(), Linkage::Strong, Scope::Default,
+ 0, SymbolName, Symbol.getValue(), Linkage::Weak, Scope::Default,
false, false);
}
if (Symbol.isAbsolute())
diff --git a/llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h b/llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h
index 5dae600629395..c05961c64837c 100644
--- a/llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h
+++ b/llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h
@@ -472,7 +472,7 @@ template <typename ELFT> Error ELFLinkGraphBuilder<ELFT>::graphifySymbols() {
Symbol &GSym = G->addDefinedSymbol(
G->createZeroFillBlock(getCommonSection(), Sym.st_size,
orc::ExecutorAddr(), Sym.getValue(), 0),
- 0, *Name, Sym.st_size, Linkage::Strong, Scope::Default, false, false);
+ 0, *Name, Sym.st_size, Linkage::Weak, Scope::Default, false, false);
setGraphSymbol(SymIndex, GSym);
continue;
}
diff --git a/llvm/lib/ExecutionEngine/JITLink/MachOLinkGraphBuilder.cpp b/llvm/lib/ExecutionEngine/JITLink/MachOLinkGraphBuilder.cpp
index bb21f633d9829..260999b4fb454 100644
--- a/llvm/lib/ExecutionEngine/JITLink/MachOLinkGraphBuilder.cpp
+++ b/llvm/lib/ExecutionEngine/JITLink/MachOLinkGraphBuilder.cpp
@@ -366,7 +366,7 @@ Error MachOLinkGraphBuilder::graphifyRegularSymbols() {
orc::ExecutorAddrDiff(NSym.Value),
orc::ExecutorAddr(),
1ull << MachO::GET_COMM_ALIGN(NSym.Desc), 0),
- 0, *NSym.Name, orc::ExecutorAddrDiff(NSym.Value), Linkage::Strong,
+ 0, *NSym.Name, orc::ExecutorAddrDiff(NSym.Value), Linkage::Weak,
NSym.S, false, NSym.Desc & MachO::N_NO_DEAD_STRIP);
} else {
if (!NSym.Name)
diff --git a/llvm/lib/ExecutionEngine/Orc/Core.cpp b/llvm/lib/ExecutionEngine/Orc/Core.cpp
index 3e6de62c8b7dd..198af09aff3a9 100644
--- a/llvm/lib/ExecutionEngine/Orc/Core.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/Core.cpp
@@ -932,13 +932,17 @@ Error JITDylib::resolve(MaterializationResponsibility &MR,
if (SymI->second.getFlags().hasError())
SymbolsInErrorState.insert(KV.first);
else {
- auto Flags = KV.second.getFlags();
- Flags &= ~JITSymbolFlags::Common;
- assert(Flags ==
- (SymI->second.getFlags() & ~JITSymbolFlags::Common) &&
+ auto ExpectedFlags = SymI->second.getFlags();
+ if (ExpectedFlags & JITSymbolFlags::Common) {
+ ExpectedFlags &= ~JITSymbolFlags::Common;
+ ExpectedFlags |= JITSymbolFlags::Weak;
+ }
+
+ assert(KV.second.getFlags() == ExpectedFlags &&
"Resolved flags should match the declared flags");
- Worklist.push_back({SymI, {KV.second.getAddress(), Flags}});
+ Worklist.push_back(
+ {SymI, {KV.second.getAddress(), SymI->second.getFlags()}});
}
}
@@ -2899,8 +2903,12 @@ Error ExecutionSession::OL_notifyResolved(MaterializationResponsibility &MR,
"Resolving symbol outside this responsibility set");
assert(!I->second.hasMaterializationSideEffectsOnly() &&
"Can't resolve materialization-side-effects-only symbol");
- assert((KV.second.getFlags() & ~JITSymbolFlags::Common) ==
- (I->second & ~JITSymbolFlags::Common) &&
+ auto ExpectedFlags = I->second;
+ if (ExpectedFlags & JITSymbolFlags::Common) {
+ ExpectedFlags &= ~JITSymbolFlags::Common;
+ ExpectedFlags |= JITSymbolFlags::Weak;
+ }
+ assert(KV.second.getFlags() == ExpectedFlags &&
"Resolving symbol with incorrect flags");
}
#endif
diff --git a/llvm/test/ExecutionEngine/JITLink/AArch64/Inputs/MachO_common_x_and_addr_getter.s b/llvm/test/ExecutionEngine/JITLink/AArch64/Inputs/MachO_common_x_and_addr_getter.s
new file mode 100644
index 0000000000000..0f352f18a60f1
--- /dev/null
+++ b/llvm/test/ExecutionEngine/JITLink/AArch64/Inputs/MachO_common_x_and_addr_getter.s
@@ -0,0 +1,10 @@
+ .section __TEXT,__text,regular,pure_instructions
+ .globl _getXAddr
+ .p2align 2
+_getXAddr:
+ adrp x0, _x at GOTPAGE
+ ldr x0, [x0, _x at GOTPAGEOFF]
+ ret
+
+ .comm _x,4,2
+.subsections_via_symbols
diff --git a/llvm/test/ExecutionEngine/JITLink/AArch64/Inputs/MachO_strong_x_and_addr_getter.s b/llvm/test/ExecutionEngine/JITLink/AArch64/Inputs/MachO_strong_x_and_addr_getter.s
new file mode 100644
index 0000000000000..88486ff000f5c
--- /dev/null
+++ b/llvm/test/ExecutionEngine/JITLink/AArch64/Inputs/MachO_strong_x_and_addr_getter.s
@@ -0,0 +1,15 @@
+ .section __TEXT,__text,regular,pure_instructions
+ .globl _getXAddr
+ .p2align 2
+_getXAddr:
+ adrp x0, _x at PAGE
+ add x0, x0, _x at PAGEOFF
+ ret
+
+ .section __DATA,__data
+ .globl _x
+ .p2align 2, 0x0
+_x:
+ .long 42
+
+.subsections_via_symbols
diff --git a/llvm/test/ExecutionEngine/JITLink/AArch64/MachO_common_symbol_x_multiple_defs.s b/llvm/test/ExecutionEngine/JITLink/AArch64/MachO_common_symbol_x_multiple_defs.s
new file mode 100644
index 0000000000000..5a8a916d30afe
--- /dev/null
+++ b/llvm/test/ExecutionEngine/JITLink/AArch64/MachO_common_symbol_x_multiple_defs.s
@@ -0,0 +1,33 @@
+# RUN: rm -rf %t && mkdir -p %t
+# RUN: llvm-mc -triple=arm64-apple-darwin19 -filetype=obj -o %t/main.o %s
+# RUN: llvm-mc -triple=arm64-apple-darwin19 -filetype=obj -o %t/aux_common.o \
+# RUN: %S/Inputs/MachO_common_x_and_addr_getter.s
+# RUN: llvm-mc -triple=arm64-apple-darwin19 -filetype=obj -o %t/aux_strong.o \
+# RUN: %S/Inputs/MachO_strong_x_and_addr_getter.s
+# RUN: llvm-jitlink -noexec %t/main.o %t/aux_common.o
+# RUN: llvm-jitlink -noexec -check=%s %t/main.o %t/aux_strong.o
+#
+# Check that linking multiple common definitions of the same symbol (in this
+# case _x) doesn't lead to an "Unexpected definitions" error.
+#
+# rdar://132314264
+#
+# Check that strong defs override:
+# jitlink-check: *{8}_x = 42
+
+ .section __TEXT,__text,regular,pure_instructions
+ .globl _main
+ .p2align 2
+_main:
+ stp x29, x30, [sp, #-16]!
+ mov x29, sp
+ bl _getXAddr
+ adrp x8, _x at GOTPAGE
+ ldr x8, [x8, _x at GOTPAGEOFF]
+ cmp x0, x8
+ cset w0, eq
+ ldp x29, x30, [sp], #16
+ ret
+
+ .comm _x,4,2
+.subsections_via_symbols
diff --git a/llvm/test/ExecutionEngine/JITLink/x86-64/COFF_common_symbol.s b/llvm/test/ExecutionEngine/JITLink/x86-64/COFF_common_symbol.s
index cd1401ebd33a3..2d4ad30f94d8d 100644
--- a/llvm/test/ExecutionEngine/JITLink/x86-64/COFF_common_symbol.s
+++ b/llvm/test/ExecutionEngine/JITLink/x86-64/COFF_common_symbol.s
@@ -6,7 +6,7 @@
#
# CHECK: Creating graph symbols...
# CHECK: 7: Creating defined graph symbol for COFF symbol "var" in (common) (index: 0)
-# CHECK-NEXT: 0x0 (block + 0x00000000): size: 0x00000004, linkage: strong, scope: default, dead - var
+# CHECK-NEXT: 0x0 (block + 0x00000000): size: 0x00000004, linkage: weak, scope: default, dead - var
.text
More information about the llvm-commits
mailing list