[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