[PATCH] D12952: Android support for SafeStack

Evgeniy Stepanov via llvm-commits llvm-commits at lists.llvm.org
Mon Sep 21 18:10:24 PDT 2015


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





More information about the llvm-commits mailing list