Update in response to feedback from Chris:

- Use enums instead of magic numbers.

- Rework algorithm to use the bytes size from the target to determine when to
  emit stack protectors.

- Get rid of "propolice" in any comments.

- Renamed an option to its expanded form.

- Other miscellanenous changes.

More changes will come after this.

llvm-svn: 58723

include/llvm/CodeGen/Passes.h

@@ -23,8 +23,19 @@ namespace llvm {
   class FunctionPass;
   class PassInfo;
   class TargetMachine;
+  class TargetLowering;
   class RegisterCoalescer;
 
+  /// StackProtectorLevel - An enumeration for when to determin when to turn
+  /// stack smashing protection (SSP) on.
+  namespace SSP {
+    enum StackProtectorLevel {
+      OFF,          // Stack protectors are off.
+      SOME,         // Stack protectors on only for functions that require them.
+      ALL           // Stack protectors on for all functions.
+    };
+  } // end SSP namespace
+
   /// createUnreachableBlockEliminationPass - The LLVM code generator does not
   /// work well with unreachable basic blocks (what live ranges make sense for a
   /// block that cannot be reached?).  As such, a code generator should either
@@ -193,7 +204,8 @@ namespace llvm {
   FunctionPass *createStackSlotColoringPass();
 
   /// createStackProtectorPass - This pass adds stack protectors to functions.
-  FunctionPass *createStackProtectorPass(int Level);
+  FunctionPass *createStackProtectorPass(SSP::StackProtectorLevel lvl,
+                                         const TargetLowering *tli);
 
 } // End llvm namespace
 

lib/CodeGen/LLVMTargetMachine.cpp

@@ -61,9 +61,17 @@ EnableFastISelOption("fast-isel", cl::Hidden,
   cl::desc("Enable the experimental \"fast\" instruction selector"));
 
 // Enable stack protectors.
-static cl::opt<int>
-EnableStackProtector("enable-stack-protector", cl::init(0),
-                     cl::desc("Use ProPolice as a stack protection method."));
+static cl::opt<SSP::StackProtectorLevel>
+EnableStackProtector("enable-stack-protector",
+                     cl::desc("Stack canary protection level: (default: off)"),
+                     cl::init(SSP::OFF),
+                     cl::values(clEnumValN(SSP::ALL,  "all",
+                                         "All functions get stack protectors."),
+                                clEnumValN(SSP::SOME, "some",
+                         "Only functions requiring stack protectors get them."),
+                                clEnumValN(SSP::OFF,  "off",
+                                          "No functions get stack protectors."),
+                                clEnumValEnd));
 
 FileModel::Model
 LLVMTargetMachine::addPassesToEmitFile(PassManagerBase &PM,
@@ -170,7 +178,8 @@ bool LLVMTargetMachine::addCommonCodeGenPasses(PassManagerBase &PM, bool Fast) {
   if (!Fast)
     PM.add(createCodeGenPreparePass(getTargetLowering()));
 
-  PM.add(createStackProtectorPass(EnableStackProtector));
+  if (EnableStackProtector != SSP::OFF)
+    PM.add(createStackProtectorPass(EnableStackProtector, getTargetLowering()));
 
   if (PrintISelInput)
     PM.add(createPrintFunctionPass("\n\n"

lib/CodeGen/StackProtector.cpp

@@ -7,10 +7,9 @@
 //
 //===----------------------------------------------------------------------===//
 //
-// This pass inserts stack protectors into functions which need them. The stack
-// protectors this uses are the type that ProPolice used. A variable with a
-// random value in it is stored onto the stack before the local variables are
-// allocated. Upon exitting the block, the stored value is checked. If it's
+// This pass inserts stack protectors into functions which need them. A variable
+// with a random value in it is stored onto the stack before the local variables
+// are allocated. Upon exiting the block, the stored value is checked. If it's
 // changed, then there was some sort of violation and the program aborts.
 //
 //===----------------------------------------------------------------------===//
@@ -25,20 +24,24 @@
 #include "llvm/Pass.h"
 #include "llvm/ADT/APInt.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Target/TargetData.h"
+#include "llvm/Target/TargetLowering.h"
 using namespace llvm;
 
 // Enable stack protectors.
 static cl::opt<unsigned>
-SSPBufferSize("ssp-buffer-size", cl::init(8),
+SSPBufferSize("stack-protector-buffer-size", cl::init(8),
               cl::desc("The lower bound for a buffer to be considered for "
                        "stack smashing protection."));
 
 namespace {
   class VISIBILITY_HIDDEN StackProtector : public FunctionPass {
-    // Level == 0  --  Stack protectors are off.
-    // Level == 1  --  Stack protectors are on only for some functions.
-    // Level == 2  --  Stack protectors are on for all functions.
-    int Level;
+    /// Level - The level of stack protection.
+    SSP::StackProtectorLevel Level;
+
+    /// TLI - Keep a pointer of a TargetLowering to consult for determining
+    /// target type sizes.
+    const TargetLowering *TLI;
 
     /// FailBB - Holds the basic block to jump to when the stack protector check
     /// fails.
@@ -70,10 +73,12 @@ namespace {
 
     /// RequiresStackProtector - Check whether or not this function needs a
     /// stack protector based upon the stack protector level.
-    bool RequiresStackProtector();
+    bool RequiresStackProtector() const;
   public:
     static char ID;             // Pass identification, replacement for typeid.
-    StackProtector(int lvl = 0) : FunctionPass(&ID), Level(lvl), FailBB(0) {}
+    StackProtector() : FunctionPass(&ID), Level(SSP::OFF), TLI(0), FailBB(0) {}
+    StackProtector(SSP::StackProtectorLevel lvl, const TargetLowering *tli)
+      : FunctionPass(&ID), Level(lvl), TLI(tli), FailBB(0) {}
 
     virtual bool runOnFunction(Function &Fn);
   };
@@ -83,8 +88,9 @@ char StackProtector::ID = 0;
 static RegisterPass<StackProtector>
 X("stack-protector", "Insert stack protectors");
 
-FunctionPass *llvm::createStackProtectorPass(int lvl) {
-  return new StackProtector(lvl);
+FunctionPass *llvm::createStackProtectorPass(SSP::StackProtectorLevel lvl,
+                                             const TargetLowering *tli) {
+  return new StackProtector(lvl, tli);
 }
 
 bool StackProtector::runOnFunction(Function &Fn) {
@@ -119,8 +125,8 @@ void StackProtector::InsertStackProtectorPrologue() {
 
   StackProtFrameSlot = new AllocaInst(PointerType::getUnqual(Type::Int8Ty),
                                       "StackProt_Frame", &InsertPt);
-  LoadInst *LI = new LoadInst(StackGuardVar, "StackGuard", true, &InsertPt);
-  new StoreInst(LI, StackProtFrameSlot, true, &InsertPt);
+  LoadInst *LI = new LoadInst(StackGuardVar, "StackGuard", false, &InsertPt);
+  new StoreInst(LI, StackProtFrameSlot, false, &InsertPt);
 }
 
 /// InsertStackProtectorEpilogue - Insert code before the return instructions
@@ -135,7 +141,7 @@ void StackProtector::InsertStackProtectorEpilogue() {
   ReturnBBs.reserve(F->size());
 
   for (; I != E; ++I)
-    if (isa<ReturnInst>((*I).getTerminator()))
+    if (isa<ReturnInst>(I->getTerminator()))
       ReturnBBs.push_back(I);
 
   if (ReturnBBs.empty()) return; // Odd, but could happen. . .
@@ -188,41 +194,41 @@ void StackProtector::CreateFailBB() {
   FailBB = BasicBlock::Create("CallStackCheckFailBlk", F);
   std::vector<const Type*> Params;
   Constant *StackChkFail =
-    M->getOrInsertFunction("__stack_chk_fail",
-                           FunctionType::get(Type::VoidTy, Params, false));
+    M->getOrInsertFunction("__stack_chk_fail", Type::VoidTy, NULL);
   CallInst::Create(StackChkFail, "", FailBB);
   new UnreachableInst(FailBB);
 }
 
 /// RequiresStackProtector - Check whether or not this function needs a stack
 /// protector based upon the stack protector level.
-bool StackProtector::RequiresStackProtector() {
+bool StackProtector::RequiresStackProtector() const {
   switch (Level) {
   default: return false;
-  case 2:  return true;
-  case 1: {
+  case SSP::ALL: return true;
+  case SSP::SOME: {
     // If the size of the local variables allocated on the stack is greater than
     // SSPBufferSize, then we require a stack protector.
     uint64_t StackSize = 0;
+    const TargetData *TD = TLI->getTargetData();
 
     for (Function::iterator I = F->begin(), E = F->end(); I != E; ++I) {
       BasicBlock *BB = I;
 
       for (BasicBlock::iterator
              II = BB->begin(), IE = BB->end(); II != IE; ++II)
-        if (AllocaInst *AI = dyn_cast<AllocaInst>(II))
+        if (AllocaInst *AI = dyn_cast<AllocaInst>(II)) {
           if (ConstantInt *CI = dyn_cast<ConstantInt>(AI->getArraySize())) {
+            uint64_t Bytes = TD->getTypeSizeInBits(AI->getAllocatedType()) / 8;
             const APInt &Size = CI->getValue();
-            StackSize += Size.getZExtValue() * 8;
-          }
-    }
+            StackSize += Bytes * Size.getZExtValue();
 
-    if (SSPBufferSize <= StackSize)
-      return true;
+            if (SSPBufferSize <= StackSize)
+              return true;
+          }
+        }
+    }
 
     return false;
   }
   }
 }
-
-// [EOF] StackProtector.cpp