From 1be7ea773a8396a50a70dbd43fd18dec42c9ea05 Mon Sep 17 00:00:00 2001 From: Evgeniy Stepanov Date: Wed, 23 Sep 2015 01:23:22 +0000 Subject: [PATCH] Revert "Android support for SafeStack." test/Transforms/SafeStack/abi.ll breaks when target is not supported; needs refactoring. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@248358 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/Target/TargetLowering.h | 8 -- include/llvm/Transforms/Instrumentation.h | 4 +- lib/CodeGen/Passes.cpp | 2 +- lib/Target/X86/X86ISelLowering.cpp | 23 ---- lib/Target/X86/X86ISelLowering.h | 6 - lib/Target/X86/X86Subtarget.h | 3 - lib/Transforms/Instrumentation/SafeStack.cpp | 109 +++++++------------ test/Transforms/SafeStack/abi.ll | 38 ------- 8 files changed, 41 insertions(+), 152 deletions(-) delete mode 100644 test/Transforms/SafeStack/abi.ll diff --git a/include/llvm/Target/TargetLowering.h b/include/llvm/Target/TargetLowering.h index 64e0ee55030..caf28d2b118 100644 --- a/include/llvm/Target/TargetLowering.h +++ b/include/llvm/Target/TargetLowering.h @@ -995,14 +995,6 @@ public: return false; } - /// Return true if the target stores SafeStack pointer at a fixed offset in - /// some non-standard address space, and populates the address space and - /// offset as appropriate. - virtual bool getSafeStackPointerLocation(unsigned & /*AddressSpace*/, - unsigned & /*Offset*/) const { - return false; - } - /// Returns true if a cast between SrcAS and DestAS is a noop. virtual bool isNoopAddrSpaceCast(unsigned SrcAS, unsigned DestAS) const { return false; diff --git a/include/llvm/Transforms/Instrumentation.h b/include/llvm/Transforms/Instrumentation.h index cfe6b432895..da4c0782cb9 100644 --- a/include/llvm/Transforms/Instrumentation.h +++ b/include/llvm/Transforms/Instrumentation.h @@ -34,8 +34,6 @@ inline void *getDFSanRetValTLSPtrForJIT() { namespace llvm { -class TargetMachine; - /// Instrumentation passes often insert conditional checks into entry blocks. /// Call this function before splitting the entry block to move instructions /// that must remain in the entry block up before the split point. Static @@ -145,7 +143,7 @@ FunctionPass *createBoundsCheckingPass(); /// \brief This pass splits the stack into a safe stack and an unsafe stack to /// protect against stack-based overflow vulnerabilities. -FunctionPass *createSafeStackPass(const TargetMachine *TM = nullptr); +FunctionPass *createSafeStackPass(); } // End llvm namespace diff --git a/lib/CodeGen/Passes.cpp b/lib/CodeGen/Passes.cpp index 114c9d11be8..7ea7b6e45ce 100644 --- a/lib/CodeGen/Passes.cpp +++ b/lib/CodeGen/Passes.cpp @@ -466,7 +466,7 @@ void TargetPassConfig::addISelPrepare() { // Add both the safe stack and the stack protection passes: each of them will // only protect functions that have corresponding attributes. - addPass(createSafeStackPass(TM)); + addPass(createSafeStackPass()); addPass(createStackProtectorPass(TM)); if (PrintISelInput) diff --git a/lib/Target/X86/X86ISelLowering.cpp b/lib/Target/X86/X86ISelLowering.cpp index d366e280739..fbead736ec0 100644 --- a/lib/Target/X86/X86ISelLowering.cpp +++ b/lib/Target/X86/X86ISelLowering.cpp @@ -2079,29 +2079,6 @@ bool X86TargetLowering::getStackCookieLocation(unsigned &AddressSpace, return true; } -/// Android provides a fixed TLS slot for the SafeStack pointer. -/// See the definition of TLS_SLOT_SAFESTACK in -/// https://android.googlesource.com/platform/bionic/+/master/libc/private/bionic_tls.h -bool X86TargetLowering::getSafeStackPointerLocation(unsigned &AddressSpace, - unsigned &Offset) const { - if (!Subtarget->isTargetAndroid()) - return false; - - if (Subtarget->is64Bit()) { - // %fs:0x48, unless we're using a Kernel code model, in which case it's %gs: - Offset = 0x48; - if (getTargetMachine().getCodeModel() == CodeModel::Kernel) - AddressSpace = 256; - else - AddressSpace = 257; - } else { - // %gs:0x24 on i386 - Offset = 0x24; - AddressSpace = 256; - } - return true; -} - bool X86TargetLowering::isNoopAddrSpaceCast(unsigned SrcAS, unsigned DestAS) const { assert(SrcAS != DestAS && "Expected different address spaces!"); diff --git a/lib/Target/X86/X86ISelLowering.h b/lib/Target/X86/X86ISelLowering.h index ff680a52b1a..949c6511bde 100644 --- a/lib/Target/X86/X86ISelLowering.h +++ b/lib/Target/X86/X86ISelLowering.h @@ -891,12 +891,6 @@ namespace llvm { bool getStackCookieLocation(unsigned &AddressSpace, unsigned &Offset) const override; - /// Return true if the target stores SafeStack pointer at a fixed offset in - /// some non-standard address space, and populates the address space and - /// offset as appropriate. - bool getSafeStackPointerLocation(unsigned &AddressSpace, - unsigned &Offset) const override; - SDValue BuildFILD(SDValue Op, EVT SrcVT, SDValue Chain, SDValue StackSlot, SelectionDAG &DAG) const; diff --git a/lib/Target/X86/X86Subtarget.h b/lib/Target/X86/X86Subtarget.h index 52e68c08180..c5d74e66b7b 100644 --- a/lib/Target/X86/X86Subtarget.h +++ b/lib/Target/X86/X86Subtarget.h @@ -394,9 +394,6 @@ public: bool isTargetMachO() const { return TargetTriple.isOSBinFormatMachO(); } bool isTargetLinux() const { return TargetTriple.isOSLinux(); } - bool isTargetAndroid() const { - return TargetTriple.getEnvironment() == Triple::Android; - } bool isTargetNaCl() const { return TargetTriple.isOSNaCl(); } bool isTargetNaCl32() const { return isTargetNaCl() && !is64Bit(); } bool isTargetNaCl64() const { return isTargetNaCl() && is64Bit(); } diff --git a/lib/Transforms/Instrumentation/SafeStack.cpp b/lib/Transforms/Instrumentation/SafeStack.cpp index 6e5e85f0825..fdf96cc130a 100644 --- a/lib/Transforms/Instrumentation/SafeStack.cpp +++ b/lib/Transforms/Instrumentation/SafeStack.cpp @@ -19,7 +19,6 @@ #include "llvm/ADT/Statistic.h" #include "llvm/ADT/Triple.h" #include "llvm/Analysis/AliasAnalysis.h" -#include "llvm/CodeGen/Passes.h" #include "llvm/IR/Constants.h" #include "llvm/IR/DataLayout.h" #include "llvm/IR/DerivedTypes.h" @@ -37,8 +36,6 @@ #include "llvm/Support/Format.h" #include "llvm/Support/MathExtras.h" #include "llvm/Support/raw_os_ostream.h" -#include "llvm/Target/TargetLowering.h" -#include "llvm/Target/TargetSubtargetInfo.h" #include "llvm/Transforms/Utils/Local.h" #include "llvm/Transforms/Utils/ModuleUtils.h" @@ -46,9 +43,6 @@ using namespace llvm; #define DEBUG_TYPE "safestack" -static const char *const kUnsafeStackPtrVar = "__safestack_unsafe_stack_ptr"; -static const char *const kUnsafeStackPtrAddrFn = "__safestack_pointer_address"; - namespace llvm { STATISTIC(NumFunctions, "Total number of functions"); @@ -163,8 +157,6 @@ bool IsSafeStackAlloca(const AllocaInst *AI) { /// (as determined statically), and the unsafe stack, which contains all /// local variables that are accessed in unsafe ways. class SafeStack : public FunctionPass { - const TargetMachine *TM; - const TargetLoweringBase *TLI; const DataLayout *DL; Type *StackPtrTy; @@ -172,7 +164,7 @@ class SafeStack : public FunctionPass { Type *Int32Ty; Type *Int8Ty; - Value *UnsafeStackPtr = nullptr; + Constant *UnsafeStackPtr = nullptr; /// Unsafe stack alignment. Each stack frame must ensure that the stack is /// aligned to this value. We need to re-align the unsafe stack if the @@ -184,7 +176,7 @@ class SafeStack : public FunctionPass { /// \brief Build a constant representing a pointer to the unsafe stack /// pointer. - Value *getOrCreateUnsafeStackPtr(IRBuilder<> &IRB, Function &F); + Constant *getOrCreateUnsafeStackPtr(Module &M); /// \brief Find all static allocas, dynamic allocas, return instructions and /// stack restore points (exception unwind blocks and setjmp calls) in the @@ -200,7 +192,7 @@ class SafeStack : public FunctionPass { /// /// \returns A pointer to the top of the unsafe stack after all unsafe static /// allocas are allocated. - Value *moveStaticAllocasToUnsafeStack(IRBuilder<> &IRB, Function &F, + Value *moveStaticAllocasToUnsafeStack(Function &F, ArrayRef StaticAllocas, ArrayRef Returns); @@ -223,11 +215,9 @@ class SafeStack : public FunctionPass { public: static char ID; // Pass identification, replacement for typeid. - SafeStack(const TargetMachine *TM) - : FunctionPass(ID), TM(TM), TLI(nullptr), DL(nullptr) { + SafeStack() : FunctionPass(ID), DL(nullptr) { initializeSafeStackPass(*PassRegistry::getPassRegistry()); } - SafeStack() : SafeStack(nullptr) {} void getAnalysisUsage(AnalysisUsage &AU) const override { AU.addRequired(); @@ -247,53 +237,35 @@ public: bool runOnFunction(Function &F) override; }; // class SafeStack -Value *SafeStack::getOrCreateUnsafeStackPtr(IRBuilder<> &IRB, Function &F) { - Module &M = *F.getParent(); - Triple TargetTriple(M.getTargetTriple()); - - unsigned Offset; - unsigned AddressSpace; - // Check if the target keeps the unsafe stack pointer at a fixed offset. - if (TLI->getSafeStackPointerLocation(Offset, AddressSpace)) { - Constant *OffsetVal = - ConstantInt::get(Type::getInt32Ty(F.getContext()), Offset); - return ConstantExpr::getIntToPtr(OffsetVal, - StackPtrTy->getPointerTo(AddressSpace)); - } - - // Android provides a libc function that returns the stack pointer address. - if (TargetTriple.getEnvironment() == llvm::Triple::Android) { - Value *Fn = M.getOrInsertFunction(kUnsafeStackPtrAddrFn, - StackPtrTy->getPointerTo(0), nullptr); - return IRB.CreateCall(Fn); +Constant *SafeStack::getOrCreateUnsafeStackPtr(Module &M) { + // The unsafe stack pointer is stored in a global variable with a magic name. + const char *kUnsafeStackPtrVar = "__safestack_unsafe_stack_ptr"; + + auto UnsafeStackPtr = + dyn_cast_or_null(M.getNamedValue(kUnsafeStackPtrVar)); + + if (!UnsafeStackPtr) { + // The global variable is not defined yet, define it ourselves. + // We use the initial-exec TLS model because we do not support the variable + // living anywhere other than in the main executable. + UnsafeStackPtr = new GlobalVariable( + /*Module=*/M, /*Type=*/StackPtrTy, + /*isConstant=*/false, /*Linkage=*/GlobalValue::ExternalLinkage, + /*Initializer=*/0, /*Name=*/kUnsafeStackPtrVar, + /*InsertBefore=*/nullptr, + /*ThreadLocalMode=*/GlobalValue::InitialExecTLSModel); } else { - // Otherwise, declare a thread-local variable with a magic name. - auto UnsafeStackPtr = - dyn_cast_or_null(M.getNamedValue(kUnsafeStackPtrVar)); - - if (!UnsafeStackPtr) { - // The global variable is not defined yet, define it ourselves. - // We use the initial-exec TLS model because we do not support the - // variable - // living anywhere other than in the main executable. - UnsafeStackPtr = new GlobalVariable( - /*Module=*/M, /*Type=*/StackPtrTy, - /*isConstant=*/false, /*Linkage=*/GlobalValue::ExternalLinkage, - /*Initializer=*/0, /*Name=*/kUnsafeStackPtrVar, - /*InsertBefore=*/nullptr, - /*ThreadLocalMode=*/GlobalValue::InitialExecTLSModel); - } else { - // The variable exists, check its type and attributes. - if (UnsafeStackPtr->getValueType() != StackPtrTy) { - report_fatal_error(Twine(kUnsafeStackPtrVar) + " must have void* type"); - } + // The variable exists, check its type and attributes. + if (UnsafeStackPtr->getValueType() != StackPtrTy) { + report_fatal_error(Twine(kUnsafeStackPtrVar) + " must have void* type"); + } - if (!UnsafeStackPtr->isThreadLocal()) { - report_fatal_error(Twine(kUnsafeStackPtrVar) + " must be thread-local"); - } + if (!UnsafeStackPtr->isThreadLocal()) { + report_fatal_error(Twine(kUnsafeStackPtrVar) + " must be thread-local"); } - return UnsafeStackPtr; } + + return UnsafeStackPtr; } void SafeStack::findInsts(Function &F, @@ -377,12 +349,13 @@ SafeStack::createStackRestorePoints(Function &F, } Value * -SafeStack::moveStaticAllocasToUnsafeStack(IRBuilder<> &IRB, Function &F, +SafeStack::moveStaticAllocasToUnsafeStack(Function &F, ArrayRef StaticAllocas, ArrayRef Returns) { if (StaticAllocas.empty()) return nullptr; + IRBuilder<> IRB(F.getEntryBlock().getFirstInsertionPt()); DIBuilder DIB(*F.getParent()); // We explicitly compute and set the unsafe stack layout for all unsafe @@ -540,8 +513,6 @@ void SafeStack::moveDynamicAllocasToUnsafeStack( bool SafeStack::runOnFunction(Function &F) { auto AA = &getAnalysis().getAAResults(); - TLI = TM->getSubtargetImpl(F)->getTargetLowering(); - DEBUG(dbgs() << "[SafeStack] Function: " << F.getName() << "\n"); if (!F.hasFnAttribute(Attribute::SafeStack)) { @@ -601,11 +572,11 @@ bool SafeStack::runOnFunction(Function &F) { if (!StackRestorePoints.empty()) ++NumUnsafeStackRestorePointsFunctions; - IRBuilder<> IRB(F.begin()->getFirstInsertionPt()); - UnsafeStackPtr = getOrCreateUnsafeStackPtr(IRB, F); + if (!UnsafeStackPtr) + UnsafeStackPtr = getOrCreateUnsafeStackPtr(*F.getParent()); // The top of the unsafe stack after all unsafe static allocas are allocated. - Value *StaticTop = moveStaticAllocasToUnsafeStack(IRB, F, StaticAllocas, Returns); + Value *StaticTop = moveStaticAllocasToUnsafeStack(F, StaticAllocas, Returns); // Safe stack object that stores the current unsafe stack top. It is updated // as unsafe dynamic (non-constant-sized) allocas are allocated and freed. @@ -627,11 +598,9 @@ bool SafeStack::runOnFunction(Function &F) { } // end anonymous namespace char SafeStack::ID = 0; -INITIALIZE_TM_PASS_BEGIN(SafeStack, "safe-stack", - "Safe Stack instrumentation pass", false, false) -INITIALIZE_TM_PASS_END(SafeStack, "safe-stack", - "Safe Stack instrumentation pass", false, false) +INITIALIZE_PASS_BEGIN(SafeStack, "safe-stack", + "Safe Stack instrumentation pass", false, false) +INITIALIZE_PASS_END(SafeStack, "safe-stack", "Safe Stack instrumentation pass", + false, false) -FunctionPass *llvm::createSafeStackPass(const llvm::TargetMachine *TM) { - return new SafeStack(TM); -} +FunctionPass *llvm::createSafeStackPass() { return new SafeStack(); } diff --git a/test/Transforms/SafeStack/abi.ll b/test/Transforms/SafeStack/abi.ll deleted file mode 100644 index c8d0553e527..00000000000 --- a/test/Transforms/SafeStack/abi.ll +++ /dev/null @@ -1,38 +0,0 @@ -; RUN: opt -safe-stack -S -mtriple=i386-pc-linux-gnu < %s -o - | FileCheck %s --check-prefix=TLS -; RUN: opt -safe-stack -S -mtriple=x86_64-pc-linux-gnu < %s -o - | FileCheck %s --check-prefix=TLS -; RUN: opt -safe-stack -S -mtriple=i686-linux-android < %s -o - | FileCheck %s --check-prefix=DIRECT-TLS32 -; RUN: opt -safe-stack -S -mtriple=x86_64-linux-android < %s -o - | FileCheck %s --check-prefix=DIRECT-TLS64 -; RUN: opt -safe-stack -S -mtriple=arm-linux-android < %s -o - | FileCheck %s --check-prefix=CALL -; RUN: opt -safe-stack -S -mtriple=aarch64-linux-android < %s -o - | FileCheck %s --check-prefix=CALL - - -define void @foo() nounwind uwtable safestack { -entry: -; TLS: %[[USP:.*]] = load i8*, i8** @__safestack_unsafe_stack_ptr -; TLS: %[[USST:.*]] = getelementptr i8, i8* %[[USP]], i32 -16 -; TLS: store i8* %[[USST]], i8** @__safestack_unsafe_stack_ptr - -; DIRECT-TLS32: %[[USP:.*]] = load i8*, i8* addrspace(36)* inttoptr (i32 256 to i8* addrspace(36)*) -; DIRECT-TLS32: %[[USST:.*]] = getelementptr i8, i8* %[[USP]], i32 -16 -; DIRECT-TLS32: store i8* %[[USST]], i8* addrspace(36)* inttoptr (i32 256 to i8* addrspace(36)*) - -; DIRECT-TLS64: %[[USP:.*]] = load i8*, i8* addrspace(72)* inttoptr (i32 257 to i8* addrspace(72)*) -; DIRECT-TLS64: %[[USST:.*]] = getelementptr i8, i8* %[[USP]], i32 -16 -; DIRECT-TLS64: store i8* %[[USST]], i8* addrspace(72)* inttoptr (i32 257 to i8* addrspace(72)*) - -; CALL: %[[SPA:.*]] = call i8** @__safestack_pointer_address() -; CALL: %[[USP:.*]] = load i8*, i8** %[[SPA]] -; CALL: %[[USST:.*]] = getelementptr i8, i8* %[[USP]], i32 -16 -; CALL: store i8* %[[USST]], i8** %[[SPA]] - - %a = alloca i8, align 8 - call void @Capture(i8* %a) - -; TLS: store i8* %[[USP]], i8** @__safestack_unsafe_stack_ptr -; DIRECT-TLS32: store i8* %[[USP]], i8* addrspace(36)* inttoptr (i32 256 to i8* addrspace(36)*) -; DIRECT-TLS64: store i8* %[[USP]], i8* addrspace(72)* inttoptr (i32 257 to i8* addrspace(72)*) -; CALL: store i8* %[[USP]], i8** %[[SPA]] - ret void -} - -declare void @Capture(i8*) -- 2.34.1