From 7f02d67fce456a0873449989064f888df992f671 Mon Sep 17 00:00:00 2001 From: Ulrich Weigand Date: Tue, 23 May 2017 14:51:18 +0000 Subject: [PATCH] [RuntimeDyld, PowerPC] Fix check for external symbols when detecting reloction overflow The PowerPC part of processRelocationRef currently assumes that external symbols can be identified by checking for SymType == SymbolRef::ST_Unknown. This is actually incorrect in some cases, causing relocation overflows to be mis-detected. The correct check is to test whether Value.SymbolName is null. Includes test case. Note that it is a bit tricky to replicate the exact condition that triggers the bug in a test case. The one included here seems to fail reliably (before the fix) across different operating system versions on Power, but it still makes a few assumptions (called out in the test case comments). Also add ppc64le platform name to the supported list in the lit.local.cfg files for the MCJIT and OrcMCJIT directories, since those tests were currently not run at all. Fixes PR32650. Reviewer: hfinkel Differential Revision: https://reviews.llvm.org/D33402 llvm-svn: 303637 --- .../RuntimeDyld/RuntimeDyldELF.cpp | 13 ++++----- llvm/test/ExecutionEngine/MCJIT/lit.local.cfg | 3 +- .../ExecutionEngine/OrcMCJIT/lit.local.cfg | 3 +- llvm/test/ExecutionEngine/OrcMCJIT/pr32650.ll | 28 +++++++++++++++++++ 4 files changed, 37 insertions(+), 10 deletions(-) create mode 100644 llvm/test/ExecutionEngine/OrcMCJIT/pr32650.ll diff --git a/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp b/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp index c4b9b9618858..777db30dd534 100644 --- a/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp +++ b/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp @@ -1324,12 +1324,12 @@ RuntimeDyldELF::processRelocationRef( Obj.getPlatformFlags(AbiVariant); AbiVariant &= ELF::EF_PPC64_ABI; // A PPC branch relocation will need a stub function if the target is - // an external symbol (Symbol::ST_Unknown) or if the target address + // an external symbol (Value.SymbolName set) or if the target address // is not within the signed 24-bits branch address. SectionEntry &Section = Sections[SectionID]; uint8_t *Target = Section.getAddressWithOffset(Offset); bool RangeOverflow = false; - if (SymType != SymbolRef::ST_Unknown) { + if (!Value.SymbolName) { if (AbiVariant != 2) { // In the ELFv1 ABI, a function call may point to the .opd entry, // so the final symbol value is calculated based on the relocation @@ -1348,15 +1348,12 @@ RuntimeDyldELF::processRelocationRef( // If it is within 26-bits branch range, just set the branch target if (SignExtend64<26>(delta) == delta) { RelocationEntry RE(SectionID, Offset, RelType, Value.Addend); - if (Value.SymbolName) - addRelocationForSymbol(RE, Value.SymbolName); - else - addRelocationForSection(RE, Value.SectionID); + addRelocationForSection(RE, Value.SectionID); } else { RangeOverflow = true; } } - if (SymType == SymbolRef::ST_Unknown || RangeOverflow) { + if (Value.SymbolName || RangeOverflow) { // It is an external symbol (SymbolRef::ST_Unknown) or within a range // larger than 24-bits. StubMap::const_iterator i = Stubs.find(Value); @@ -1412,7 +1409,7 @@ RuntimeDyldELF::processRelocationRef( RelType, 0); Section.advanceStubOffset(getMaxStubSize()); } - if (SymType == SymbolRef::ST_Unknown) { + if (Value.SymbolName) { // Restore the TOC for external calls if (AbiVariant == 2) writeInt32BE(Target + 4, 0xE8410018); // ld r2,28(r1) diff --git a/llvm/test/ExecutionEngine/MCJIT/lit.local.cfg b/llvm/test/ExecutionEngine/MCJIT/lit.local.cfg index f98140357736..e2535ef1dbfd 100644 --- a/llvm/test/ExecutionEngine/MCJIT/lit.local.cfg +++ b/llvm/test/ExecutionEngine/MCJIT/lit.local.cfg @@ -9,7 +9,8 @@ else: # FIXME: autoconf and cmake produce different arch names. We should normalize # them before getting here. if root.host_arch not in ['i386', 'x86', 'x86_64', 'AMD64', - 'AArch64', 'ARM', 'Mips', 'PowerPC', 'ppc64', 'SystemZ']: + 'AArch64', 'ARM', 'Mips', + 'PowerPC', 'ppc64', 'ppc64le', 'SystemZ']: config.unsupported = True if 'armv7' in root.host_arch: diff --git a/llvm/test/ExecutionEngine/OrcMCJIT/lit.local.cfg b/llvm/test/ExecutionEngine/OrcMCJIT/lit.local.cfg index f98140357736..e2535ef1dbfd 100644 --- a/llvm/test/ExecutionEngine/OrcMCJIT/lit.local.cfg +++ b/llvm/test/ExecutionEngine/OrcMCJIT/lit.local.cfg @@ -9,7 +9,8 @@ else: # FIXME: autoconf and cmake produce different arch names. We should normalize # them before getting here. if root.host_arch not in ['i386', 'x86', 'x86_64', 'AMD64', - 'AArch64', 'ARM', 'Mips', 'PowerPC', 'ppc64', 'SystemZ']: + 'AArch64', 'ARM', 'Mips', + 'PowerPC', 'ppc64', 'ppc64le', 'SystemZ']: config.unsupported = True if 'armv7' in root.host_arch: diff --git a/llvm/test/ExecutionEngine/OrcMCJIT/pr32650.ll b/llvm/test/ExecutionEngine/OrcMCJIT/pr32650.ll new file mode 100644 index 000000000000..bbf68aea512a --- /dev/null +++ b/llvm/test/ExecutionEngine/OrcMCJIT/pr32650.ll @@ -0,0 +1,28 @@ +; RUN: %lli -jit-kind=orc-mcjit %s + +; This test is intended to verify that a function weakly defined in +; JITted code, and strongly defined in the main executable, can be +; correctly resolved when called from elsewhere in JITted code. + +; This test makes the assumption that the lli executable in compiled +; to export symbols (e.g. --export-dynamic), and that is actually does +; contain the symbol LLVMInitializeCodeGen. (Note that this function +; is not actually called by the test. The test simply verifes that +; the reference can be resolved without relocation errors.) + +define linkonce_odr void @LLVMInitializeCodeGen() { +entry: + ret void +} + +define void @test() { +entry: + call void @LLVMInitializeCodeGen() + ret void +} + +define i32 @main() { +entry: + ret i32 0 +} +