[PATCH] D19407: [SafeStack] [SSP] Use llvm.stackguard intrinsic.

[Tim Shen via llvm-commits]

timshen added inline comments.

================
Comment at: lib/CodeGen/SafeStack.cpp:400
@@ +399,3 @@
+    TL->insertSSPDeclarations(*M);
+    Function *SG = Intrinsic::getDeclaration(M, Intrinsic::stackguard);
+    return IRB.CreateCall(SG);
----------------
koriakin wrote:
> timshen wrote:
> > 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.
> 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...
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.


Repository:
  rL LLVM

http://reviews.llvm.org/D19407