[PATCH] D12952: Android support for SafeStack

[Evgeniy Stepanov via llvm-commits]

eugenis added inline comments.

================
Comment at: lib/Transforms/Instrumentation/SafeStack.cpp:22
@@ -21,3 +21,3 @@
 #include "llvm/Analysis/AliasAnalysis.h"
-#include "llvm/Analysis/TargetTransformInfo.h"
+#include "llvm/CodeGen/Passes.h"
 #include "llvm/IR/Constants.h"
----------------
echristo wrote:
> echristo wrote:
> > ?
> Can you remove the dependence on TTI separately? I don't see any use otherwise?
Passes.h needed for INITIALIZE_TM_PASS_BEGIN

================
Comment at: lib/Transforms/Instrumentation/SafeStack.cpp:263-268
@@ -258,1 +262,8 @@
+
+  if (TargetTriple.getEnvironment() == llvm::Triple::Android) {
+    Value *Fn = M.getOrInsertFunction(kUnsafeStackPtrAddrFn,
+                                      StackPtrTy->getPointerTo(0), nullptr);
+    IRBuilder<> IRB(F.begin()->getFirstInsertionPt());
+    Value *V = IRB.CreateCall(Fn);
+    return V;
   } else {
----------------
echristo wrote:
> Needs a comment.
> 
> Also, can we do this in such a way that we don't need to check for android? Similar to the stack protector pattern?
We could move this logic to TargetLowering.h, making the X86 implementation delegate to the common implementation if  not android, but it does not improve anything.

================
Comment at: lib/Transforms/Instrumentation/SafeStack.cpp:387-390
@@ -358,3 +386,6 @@
 
-  IRBuilder<> IRB(F.getEntryBlock().getFirstInsertionPt());
+  IRBuilder<> IRB(
+      isa<Instruction>(UnsafeStackPtr)
+          ? cast<Instruction>(UnsafeStackPtr)->getNextNode()
+          : cast<Instruction>(F.getEntryBlock().getFirstInsertionPt()));
   DIBuilder DIB(*F.getParent());
----------------
echristo wrote:
> ?
> 
Simplified.
This code moved insertion point after the calculation of UnsafeStackPtr, unless that's a constant.


Repository:
  rL LLVM

http://reviews.llvm.org/D12952