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

[Tom Roeder]

Comments on the current set of comments. I'm also about to update this with a patch that handles the other comments.

================
Comment at: include/llvm/CodeGen/ForwardControlFlowIntegrity.h:56
@@ +55,3 @@
+private:
+  typedef DenseSet<Instruction *> CallSet;
+
----------------
Kostya Serebryany wrote:
> Can this be SmallVector?
I don't know SmallVector well, so I'm not sure. It's true that I don't need efficient search over this data structure, since it's just filled and iterated, so at least it could be a vector. Does SmallVector suffer if it gets large?

================
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()) {
----------------
Nick Lewycky wrote:
> 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?
I've found that sometimes the jumptable code was getting emitted in a section that was not .text. It might be possible to fix this by emitting the code somewhere else, but it looked to me like each chunk of output was emitting a section switch if it needed to make sure it was in a given section. This shouldn't add an unnecessary section switch if there's no jumptable, since it's guarded by the condition

if (JITI && !JITI->getTables().empty())

There's no way to turn jumptable off, though, since we agreed in the original jumptable code review that the mere presence of the jumptable attribute should be enough to trigger the inclusion of a jumptable.

As for removing the MCSA_Global emission, this fixes a problem I noticed recently with separate compilation. As it stands, if you compile two different modules with jumptable annotations then try to link them, the link will fail, since both will have, e.g., __llvm_jump_instr_table_0_1 as global symbols. But that symbol doesn't need to be global, since it can only be referenced by the current module. Removing the MCSA_Global annotation allows multiple modules to be built with jumptable and to link correctly.

Note that pointers to __llvm_jump_instr_table_0_1 can be handed to other modules, but other modules can't directly refer to this pointer in their code; this was part of the whole problem with &f != <received pointer to f> that led to the requirement for unnamed_addr on all jumptable-annotated functions.

================
Comment at: lib/CodeGen/ForwardControlFlowIntegrity.cpp:105
@@ +104,3 @@
+
+ForwardControlFlowIntegrity::~ForwardControlFlowIntegrity() {}
+
----------------
Kostya Serebryany wrote:
> maybe inline this in the header? 
> 
I was asked in a review for a different class to take the destructor out of the header and put it in the cc

http://reviews.llvm.org/D4167