[PATCH] Add Forward-Edge Control-Flow Integrity support

[Nick Lewycky]

================
Comment at: include/llvm/CodeGen/CommandFlags.h:240
@@ +239,3 @@
+CFIEnforcing("cfi-enforcing",
+             cl::desc("Enforce CFI or pass a the violation to a funcion."),
+             cl::init(false));
----------------
Typo: funcion -> function
Grammar, "... pass a the violation ..."

================
Comment at: include/llvm/CodeGen/ForwardControlFlowIntegrity.h:51
@@ +50,3 @@
+  const char *getPassName() const override {
+    return "ForwardControlFlowIntegrity";
+  }
----------------
IIRC, this is a debug-ish string, used for things like -ftime-report output. Feel free to add spaces between the words (a quick survey shows that we do it both with spaces and with StudyCaps in LLVM).

================
Comment at: include/llvm/CodeGen/ForwardControlFlowIntegrity.h:58-63
@@ +57,8 @@
+
+  /// A structure that is used to keep track of constant table information.
+  typedef struct CFIConstants {
+    Constant *StartValue;
+    Constant *MaskValue;
+    Constant *Size;
+  } CFIConstants;
+
----------------
C++ please. :) Just "struct CFIConstants { ... };" will do.

================
Comment at: include/llvm/Target/TargetOptions.h:76-80
@@ -65,5 +75,7 @@
           CompressDebugSections(false), FunctionSections(false),
           DataSections(false), TrapUnreachable(false), TrapFuncName(""),
           FloatABIType(FloatABI::Default),
-          AllowFPOpFusion(FPOpFusion::Standard), JTType(JumpTable::Single) {}
+          AllowFPOpFusion(FPOpFusion::Standard), JTType(JumpTable::Single),
+          FCFI(false), CFIType(CFIntegrity::Sub), CFIEnforcing(false),
+          CFIFuncName("") {}
 
----------------
I was going to point out that you could use CFIFuncName() instead, but then I noticed TrapFuncName(""). Feel free to fix both of them.

================
Comment at: lib/CodeGen/AsmPrinter/AsmPrinter.cpp:903
@@ -901,1 +902,3 @@
+    // Emit the right section for these functions.
+    OutStreamer.SwitchSection(OutContext.getObjectFileInfo()->getTextSection());
     for (const auto &KV : JITI->getTables()) {
----------------
Could you explain this change a little more? And the deletion of:
507     OutStreamer.EmitSymbolAttribute(FunSym, MCSA_Global);
?

Does this cause us to emit an unnecessary section switch even when jump tables are off?

================
Comment at: lib/CodeGen/ForwardControlFlowIntegrity.cpp:118-121
@@ +117,6 @@
+  for (Function &F : M) {
+    for (auto FI = F.begin(), FE = F.end(); FI != FE; ++FI) {
+      BasicBlock *BB = FI;
+      for (auto BI = BB->begin(), BE = BB->end(); BI != BE; ++BI) {
+        Instruction *I = BI;
+
----------------
  for (BasicBlock &BB : F) {
    for (Instruction &I : BB) {
? (Just use &I in place of I for the rest of this function.)

================
Comment at: lib/CodeGen/ForwardControlFlowIntegrity.cpp:233-234
@@ +232,4 @@
+void ForwardControlFlowIntegrity::addWarningFunction(Module &M) {
+  Type *CharTy = Type::getInt8Ty(M.getContext());
+  PointerType *CharPtrTy = PointerType::getUnqual(CharTy);
+
----------------
  PointerType *CharPtrTy = Type::getInt8PtrTy(M.getContext());
which leaves CharTy dead and you can remove it.

================
Comment at: test/CodeGen/X86/jump_tables.ll:91
@@ -76,4 +90,3 @@
 
-; SINGLE-DAG:         .globl  __llvm_jump_instr_table_0_1
-; SINGLE-DAG:         .align  8, 0x90
+; SINGLE-DAG:         .align  8
 ; SINGLE-DAG:         .type   __llvm_jump_instr_table_0_1, at function
----------------
Why the change in fill byte?

http://reviews.llvm.org/D4167