[SafeStack] [SSP] Use llvm.stackguard intrinsic.

[• koriakin]

Summary

This is necessary for SafeStack support on PPC64.

[• koriakin]

One thing I really don't like here is the necessity of calling insertSSPDeclarations - that should be handled by whoever is expanding the intrinsic. Eg. the following:

define i8 *@foo() {
        %1 = call i8* @llvm.stackguard();
        ret i8 *%1;
}

declare i8* @llvm.stackguard() nounwind;

currently segfaults llc on x86_64-freebsd, since it lacks __stack_chk_guard declaration. However, I don't really know where to add these global variables - the Module is const in SelectionDAGBuilder, where we expand the intrinsics. Any hints?

[timshen]

Sorry about insertSSPDeclarations - I don't like it neither, but I didn't know a better solution.

lib/CodeGen/SafeStack.cpp

400

You may want to take a look at "getStackGuard()" in lib/CodeGen/StackProtector.cpp and possible factor it out (SupportsSelectionDAGSP is a bit weird, sorry), since it also takes care of IR form SSP.

[eugenis]

Cool!

I wonder if we could get SDAG SSP for free now by updating the following code to look at SafeStack, too:
getAnalysis<StackProtector>().shouldEmitSDCheck(*LLVMBB)

[timshen]

In D19407#409330, @eugenis wrote:

I wonder if we could get SDAG SSP for free now by updating the following code to look at SafeStack, too:
getAnalysis<StackProtector>().shouldEmitSDCheck(*LLVMBB)

I'm not familiar with SafeStack, but SafeStack SSP does have a different type of canary, right? Then this line will crash: https://github.com/llvm-mirror/llvm/blob/ac94d4bd341c54f663f9622357da7eaad905a7c9/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp#L5336

[• koriakin]

In D19407#409330, @eugenis wrote:

Cool!

I wonder if we could get SDAG SSP for free now by updating the following code to look at SafeStack, too:
getAnalysis<StackProtector>().shouldEmitSDCheck(*LLVMBB)

I don't quite follow - the canary in SafeStack is a different beast (for one, it's on the other stack), and needs much fewer hacks (eg. we don't need to avoid spilling it).

[• koriakin]

lib/CodeGen/SafeStack.cpp

400

The functions look about the same. The one in StackProtector emits a volatile load, but changing SafeStack to use one too shouldn't be much of a problem, right? However, I now see I've introduced a problem in SafeStack - I'm calling insertSSPDeclarations on TL, which may be null (in which case getIRStackGuard isn't called either, ugh). I'm not really sure how to deal with that - should we require a target machine when instantiating the SafeStack pass? It would be best if we could just emit the stackguard intrinsic there and somone lower just took care of it...

[timshen]

lib/CodeGen/SafeStack.cpp

400

I think the nullability of TL is a separate issue. If you don't have TL around then I don't know how may that happen, nor what to do.

If TL isn't nullptr, calling the factored out getStackGuard() in StackProtector.cpp seems to be good.

And as I said, I don't know any clean solution for insertSSPDeclarations. There exists very low level code (e.g. expandLoadStackGuard in X86InstrInfo.cpp) that is using the Value* form of the global variable.

[• koriakin]

The only instance of creating a global variable during SelectionDAG phase I found is in LowerToTLSEmulatedModel, and it rather unceremoniously const_cast<>s the module. I'll keep digging...

lib/CodeGen/SafeStack.cpp

Value *SafeStack::getOrCreateUnsafeStackPtr(IRBuilder<> &IRB, Function &F) {
   }
   return UnsafeStackPtr;
 }
 
 Value *SafeStack::getStackGuard(IRBuilder<> &IRB, Function &F) {
   Value *StackGuardVar = nullptr;
   if (TL)
     StackGuardVar = TL->getIRStackGuard(IRB);
-  if (!StackGuardVar)
-    StackGuardVar =
-        F.getParent()->getOrInsertGlobal("__stack_chk_guard", StackPtrTy);
+  if (!StackGuardVar) {
+    Module *M = F.getParent();
+    TL->insertSSPDeclarations(*M);
+    Function *SG = Intrinsic::getDeclaration(M, Intrinsic::stackguard);
+    return IRB.CreateCall(SG);
+  }
   return IRB.CreateLoad(StackGuardVar, "StackGuard");
 }
 
 void SafeStack::findInsts(Function &F,
                           SmallVectorImpl<AllocaInst *> &StaticAllocas,
                           SmallVectorImpl<AllocaInst *> &DynamicAllocas,
                           SmallVectorImpl<Argument *> &ByValArguments,
                           SmallVectorImpl<ReturnInst *> &Returns,

test/Transforms/SafeStack/X86/ssp.ll

 ; RUN: opt -safe-stack -S -mtriple=x86_64-unknown < %s -o - | FileCheck %s
 
 define void @foo() safestack sspreq {
 entry:
 ; CHECK: %[[USP:.*]] = load i8*, i8** @__safestack_unsafe_stack_ptr
 ; CHECK: %[[USST:.*]] = getelementptr i8, i8* %[[USP]], i32 -16
 ; CHECK: store i8* %[[USST]], i8** @__safestack_unsafe_stack_ptr
 
 ; CHECK: %[[A:.*]] = getelementptr i8, i8* %[[USP]], i32 -8
 ; CHECK: %[[StackGuardSlot:.*]] = bitcast i8* %[[A]] to i8**
-; CHECK: %[[StackGuard:.*]] = load i8*, i8** @__stack_chk_guard
+; CHECK: %[[StackGuard:.*]] = call i8* @llvm.stackguard()
 ; CHECK: store i8* %[[StackGuard]], i8** %[[StackGuardSlot]]
   %a = alloca i8, align 1
 
 ; CHECK: call void @Capture
   call void @Capture(i8* %a)
 
 ; CHECK: %[[B:.*]] = load i8*, i8** %[[StackGuardSlot]]
 ; CHECK: %[[COND:.*]] = icmp ne i8* %[[StackGuard]], %[[B]]