From c49837f5f688ff2cd70ecc6d5aefd71af2afb22b Mon Sep 17 00:00:00 2001 From: Lang Hames Date: Wed, 24 Jul 2024 13:26:17 +1000 Subject: [PATCH] Revert "[ORC][JITLink] Treat common symbols as weak definitions." This reverts commit 785d376d1231167688dd12f93c5c0a5d46cd4086 while I investigate some bot failures (e.g. https://lab.llvm.org/buildbot/#/builders/3/builds/1983). --- .../JITLink/COFFLinkGraphBuilder.cpp | 2 +- .../JITLink/ELFLinkGraphBuilder.h | 2 +- .../JITLink/MachOLinkGraphBuilder.cpp | 2 +- llvm/lib/ExecutionEngine/Orc/Core.cpp | 22 ++++--------- .../Inputs/MachO_common_x_and_addr_getter.s | 10 ------ .../Inputs/MachO_strong_x_and_addr_getter.s | 15 --------- .../MachO_common_symbol_x_multiple_defs.s | 33 ------------------- .../JITLink/x86-64/COFF_common_symbol.s | 2 +- 8 files changed, 11 insertions(+), 77 deletions(-) delete mode 100644 llvm/test/ExecutionEngine/JITLink/AArch64/Inputs/MachO_common_x_and_addr_getter.s delete mode 100644 llvm/test/ExecutionEngine/JITLink/AArch64/Inputs/MachO_strong_x_and_addr_getter.s delete mode 100644 llvm/test/ExecutionEngine/JITLink/AArch64/MachO_common_symbol_x_multiple_defs.s diff --git a/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp b/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp index 92630650ec93..1fd2a33d3f11 100644 --- a/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp +++ b/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp @@ -467,7 +467,7 @@ Expected COFFLinkGraphBuilder::createDefinedSymbol( return &G->addDefinedSymbol( G->createZeroFillBlock(getCommonSection(), Symbol.getValue(), orc::ExecutorAddr(), Symbol.getValue(), 0), - 0, SymbolName, Symbol.getValue(), Linkage::Weak, Scope::Default, + 0, SymbolName, Symbol.getValue(), Linkage::Strong, Scope::Default, false, false); } if (Symbol.isAbsolute()) diff --git a/llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h b/llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h index c05961c64837..5dae60062939 100644 --- a/llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h +++ b/llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h @@ -472,7 +472,7 @@ template Error ELFLinkGraphBuilder::graphifySymbols() { Symbol &GSym = G->addDefinedSymbol( G->createZeroFillBlock(getCommonSection(), Sym.st_size, orc::ExecutorAddr(), Sym.getValue(), 0), - 0, *Name, Sym.st_size, Linkage::Weak, Scope::Default, false, false); + 0, *Name, Sym.st_size, Linkage::Strong, 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 260999b4fb45..bb21f633d982 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::Weak, + 0, *NSym.Name, orc::ExecutorAddrDiff(NSym.Value), Linkage::Strong, 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 198af09aff3a..3e6de62c8b7d 100644 --- a/llvm/lib/ExecutionEngine/Orc/Core.cpp +++ b/llvm/lib/ExecutionEngine/Orc/Core.cpp @@ -932,17 +932,13 @@ Error JITDylib::resolve(MaterializationResponsibility &MR, if (SymI->second.getFlags().hasError()) SymbolsInErrorState.insert(KV.first); else { - auto ExpectedFlags = SymI->second.getFlags(); - if (ExpectedFlags & JITSymbolFlags::Common) { - ExpectedFlags &= ~JITSymbolFlags::Common; - ExpectedFlags |= JITSymbolFlags::Weak; - } - - assert(KV.second.getFlags() == ExpectedFlags && + auto Flags = KV.second.getFlags(); + Flags &= ~JITSymbolFlags::Common; + assert(Flags == + (SymI->second.getFlags() & ~JITSymbolFlags::Common) && "Resolved flags should match the declared flags"); - Worklist.push_back( - {SymI, {KV.second.getAddress(), SymI->second.getFlags()}}); + Worklist.push_back({SymI, {KV.second.getAddress(), Flags}}); } } @@ -2903,12 +2899,8 @@ Error ExecutionSession::OL_notifyResolved(MaterializationResponsibility &MR, "Resolving symbol outside this responsibility set"); assert(!I->second.hasMaterializationSideEffectsOnly() && "Can't resolve materialization-side-effects-only symbol"); - auto ExpectedFlags = I->second; - if (ExpectedFlags & JITSymbolFlags::Common) { - ExpectedFlags &= ~JITSymbolFlags::Common; - ExpectedFlags |= JITSymbolFlags::Weak; - } - assert(KV.second.getFlags() == ExpectedFlags && + assert((KV.second.getFlags() & ~JITSymbolFlags::Common) == + (I->second & ~JITSymbolFlags::Common) && "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 deleted file mode 100644 index 0f352f18a60f..000000000000 --- a/llvm/test/ExecutionEngine/JITLink/AArch64/Inputs/MachO_common_x_and_addr_getter.s +++ /dev/null @@ -1,10 +0,0 @@ - .section __TEXT,__text,regular,pure_instructions - .globl _getXAddr - .p2align 2 -_getXAddr: - adrp x0, _x@GOTPAGE - ldr x0, [x0, _x@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 deleted file mode 100644 index 88486ff000f5..000000000000 --- a/llvm/test/ExecutionEngine/JITLink/AArch64/Inputs/MachO_strong_x_and_addr_getter.s +++ /dev/null @@ -1,15 +0,0 @@ - .section __TEXT,__text,regular,pure_instructions - .globl _getXAddr - .p2align 2 -_getXAddr: - adrp x0, _x@PAGE - add x0, x0, _x@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 deleted file mode 100644 index 5a8a916d30af..000000000000 --- a/llvm/test/ExecutionEngine/JITLink/AArch64/MachO_common_symbol_x_multiple_defs.s +++ /dev/null @@ -1,33 +0,0 @@ -# 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@GOTPAGE - ldr x8, [x8, _x@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 2d4ad30f94d8..cd1401ebd33a 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: weak, scope: default, dead - var +# CHECK-NEXT: 0x0 (block + 0x00000000): size: 0x00000004, linkage: strong, scope: default, dead - var .text