From 7a6f9c29967386171ba4b94e49dd4a3e6f388506 Mon Sep 17 00:00:00 2001 From: Martin Fink Date: Mon, 23 Aug 2021 17:38:52 +0200 Subject: [PATCH] [X86-64] Only treat PC-relative globals as memory content if they are dynamically relocated This makes sure that both of the following work as expected: 1. mov eax, [rip + myglob@GOTPCREL] used for shared libraries compiled to position independent code (-fPIC) Here myglob is dynamically relocated through the GOT 2. mov eax, [rip + myglob] used for non-PIC code to access a global directly without any indirection through the GOT [Test] Added one test to verify RIP-relative globals work as expected --- X86/X86MachineInstructionRaiser.cpp | 24 +++++----- X86/X86MachineInstructionRaiser.h | 9 ++++ X86/X86MachineInstructionRaiserUtils.cpp | 29 ++++++------ .../X86/raise-pcrel-global-variable.s | 46 +++++++++++++++++++ 4 files changed, 82 insertions(+), 26 deletions(-) create mode 100644 test/asm_test/X86/raise-pcrel-global-variable.s diff --git a/X86/X86MachineInstructionRaiser.cpp b/X86/X86MachineInstructionRaiser.cpp index 90da0cd1..ce9b2bcf 100644 --- a/X86/X86MachineInstructionRaiser.cpp +++ b/X86/X86MachineInstructionRaiser.cpp @@ -46,6 +46,8 @@ using namespace llvm; using namespace mctoll; using namespace X86RegisterUtils; +std::set X86MachineInstructionRaiser::DynRelocatedGlobalVariables; + // Constructor X86MachineInstructionRaiser::X86MachineInstructionRaiser(MachineFunction &MF, @@ -1881,18 +1883,18 @@ bool X86MachineInstructionRaiser::raiseMoveFromMemInstr(const MachineInstr &MI, // Following are the exceptions when MemRefValue needs to be considered as // memory content and not as memory reference. if (IsPCRelMemRef) { - // If it is a PC-relative global variable with an initializer, it is memory - // content and should not be loaded from. - if (auto GV = dyn_cast(MemRefValue)) - LoadFromMemrefValue = !(GV->hasInitializer()); - // If it is not a PC-relative constant expression accessed using - // GetElementPtrInst, it is memory content and should not be loaded from. - else { - const ConstantExpr *CExpr = dyn_cast(MemRefValue); - if (CExpr != nullptr) { - LoadFromMemrefValue = - (CExpr->getOpcode() == Instruction::GetElementPtr); + // If it is a PC-relative dynamically relocated global variable, it is + // memory content and should not be loaded from. + if (auto GV = dyn_cast(MemRefValue)) { + if (isDynRelocatedGlobalVariable(GV->getName().str())) { + LoadFromMemrefValue = false; + } else { + LoadFromMemrefValue = true; } + // If it is not a PC-relative constant expression accessed using + // GetElementPtrInst, it is memory content and should not be loaded from. + } else if (auto *CExpr = dyn_cast(MemRefValue)) { + LoadFromMemrefValue = (CExpr->getOpcode() == Instruction::GetElementPtr); } } diff --git a/X86/X86MachineInstructionRaiser.h b/X86/X86MachineInstructionRaiser.h index 99486c26..6f31df76 100644 --- a/X86/X86MachineInstructionRaiser.h +++ b/X86/X86MachineInstructionRaiser.h @@ -82,6 +82,10 @@ class X86MachineInstructionRaiser : public MachineInstructionRaiser { // at the exit of the MBB. std::map PerMBBDefinedPhysRegMap; + // Stores all global variables that are dynamically relocated, e.g. through PLT + // Usually these are global variables in shared objects + static std::set DynRelocatedGlobalVariables; + static const uint8_t FPUSTACK_SZ = 8; struct { int8_t TOP; @@ -218,6 +222,11 @@ class X86MachineInstructionRaiser : public MachineInstructionRaiser { bool isEffectiveAddrValue(Value *Val); std::vector jtList; + + // Check if there exists a dynamically relocated variable with the given name + inline bool isDynRelocatedGlobalVariable(std::string Name) { + return DynRelocatedGlobalVariables.find(Name) != DynRelocatedGlobalVariables.end(); + } }; #endif // LLVM_TOOLS_LLVM_MCTOLL_X86_X86MACHINEINSTRUCTIONRAISER_H diff --git a/X86/X86MachineInstructionRaiserUtils.cpp b/X86/X86MachineInstructionRaiserUtils.cpp index b9cd4a35..2b4ad28c 100644 --- a/X86/X86MachineInstructionRaiserUtils.cpp +++ b/X86/X86MachineInstructionRaiserUtils.cpp @@ -189,18 +189,18 @@ Value *X86MachineInstructionRaiser::loadMemoryRefValue( // Following are the exceptions when MemRefValue needs to be considered as // memory content and not as memory reference. if (IsPCRelMemRef) { - // If it is a PC-relative global variable with an initializer, it is memory - // content and should not be loaded from. - if (auto GV = dyn_cast(MemRefValue)) - LoadFromMemrefValue = !(GV->hasInitializer()); + // If it is a PC-relative dynamically relocated global variable, it is + // memory content and should not be loaded from. + if (auto GV = dyn_cast(MemRefValue)) { + if (isDynRelocatedGlobalVariable(GV->getName().str())) { + LoadFromMemrefValue = false; + } else { + LoadFromMemrefValue = true; + } // If it is not a PC-relative constant expression accessed using // GetElementPtrInst, it is memory content and should not be loaded from. - else { - const ConstantExpr *CExpr = dyn_cast(MemRefValue); - if (CExpr != nullptr) { - LoadFromMemrefValue = - (CExpr->getOpcode() == Instruction::GetElementPtr); - } + } else if (auto *CExpr = dyn_cast(MemRefValue)) { + LoadFromMemrefValue = (CExpr->getOpcode() == Instruction::GetElementPtr); } } @@ -745,17 +745,16 @@ Value *X86MachineInstructionRaiser::createPCRelativeAccesssValue( break; } } - Constant *GlobalInit; if (IncludedFileInfo::IsExternalVariable(Symname->str())) { GlobalInit = nullptr; Lnkg = GlobalValue::ExternalLinkage; + } else if (DynRelocType == ELF::R_X86_64_GLOB_DAT) { + GlobalInit = ConstantInt::get(GlobalValTy, SymbVal); + DynRelocatedGlobalVariables.insert(Symname->str()); } else { - GlobalInit = (DynRelocType == ELF::R_X86_64_GLOB_DAT) - ? ConstantInt::get(GlobalValTy, SymbVal) - : nullptr; + GlobalInit = nullptr; } - auto GlobalVal = new GlobalVariable(*(MR->getModule()), GlobalValTy, false /* isConstant */, Lnkg, GlobalInit, Symname->data()); diff --git a/test/asm_test/X86/raise-pcrel-global-variable.s b/test/asm_test/X86/raise-pcrel-global-variable.s new file mode 100644 index 00000000..8d6cb14c --- /dev/null +++ b/test/asm_test/X86/raise-pcrel-global-variable.s @@ -0,0 +1,46 @@ +// REQUIRES: x86_64-linux +// RUN: clang -O0 -o %t %s +// RUN: llvm-mctoll -d -I /usr/include/stdio.h %t +// RUN: clang -o %t-dis %t-dis.ll +// RUN: %t-dis 2>&1 | FileCheck %s +// CHECK: i = 0 +// CHECK: i = 10 +// CHECK-EMPTY + +.text +.intel_syntax noprefix +.file "raise-call64r-float.s" + +.globl main # -- Begin function main +.p2align 4, 0x90 +.type main,@function +main: # @main + movabs rdi, offset .L.str + mov esi, [rip + i] + mov al, 0 + call printf + + mov dword ptr [rip + i], 10 + + movabs rdi, offset .L.str + mov esi, [rip + i] + mov al, 0 + call printf + + xor rax, rax + ret + + +.type .L.str,@object # @.str +.section .rodata.str1.1,"aMS",@progbits,1 +.L.str: + .asciz "i = %d\n" + .size .L.str, 8 + +.bss +.type i,@object +.globl i +.p2align 2 +i: + .long 0 + .size i, 4