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

[Tom Roeder]

================
Comment at: include/llvm/Analysis/JumpInstrTableInfo.h:61
@@ +60,3 @@
+
+  /// A power-of-two alignment of a jumptable entry.
+  uint64_t Alignment;
----------------
JF Bastien wrote:
> This invariant isn't actually enforced in the code, instead the code tries to round up on use. I think it would be better to enforce the invariant on construction.
Hmm...I thought I did the rounding in the createJumpInstrTableInfo; but I think you're right that it should be moved to the constructor.

================
Comment at: include/llvm/Target/TargetInstrInfo.h:342
@@ +341,3 @@
+  /// either the instruction returned by getUnconditionalBranch or the
+  /// instruction returned by getTrap. This only makes sense because
+  /// getUnconditionalBranch returns a single, specific instruction. This
----------------
JF Bastien wrote:
> The maximum of uncond branch and trap?
Right.

================
Comment at: lib/Target/ARM/ARMBaseInstrInfo.cpp:4364
@@ -4363,2 +4363,3 @@
 
+// This must be kept in sync with getJumpInstrTableEntryBound.
 void ARMBaseInstrInfo::getUnconditionalBranch(
----------------
JF Bastien wrote:
> This comment (and the others below and in other files) isn't clear: what has to be kept in sync exactly?
See my comment after getJumpInstrTableEntryBound: the bound in that function must be a bound on the possible instruction lengths returned by these two functions.

================
Comment at: lib/Target/ARM/ARMBaseInstrInfo.cpp:4391
@@ +4390,3 @@
+  // At worst, this will be a branch to a 32-bit value. So, that's one byte for
+  // the branch instruction, and 4 bytes for the value. The trap instruction
+  // fits in 4 bytes, so this suffices as a bound.
----------------
JF Bastien wrote:
> Thumb instructions are 2 or 4 bytes, and ARM instructions are always 4. The largest immediate on a direct branch has 26 bits, so that's insufficient for a lot of cases, you'll therefore need a PC-relative load followed by an indirect branch, and a location to store the 32-bit constant pool entry for the address. Assuming you do:
>   ldr rX, [PC, +#8]
>   bx rX
>   0xdeadbeef ; The address
> Then you need 12 bytes, which I think is the worst case. Note that the constant pool entry could be elsewhere, preferably *not* in executable memory, but that would require another register and more infrastructure.
> 
> Note that you may want to use blx instead, if you intend on balancing the CPU's call/return stack: blx would be for calls, see the following page for returns
>   http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0438i/BABGEAEF.html
I don't understand, but that's probably because I don't know ARM well. The goal here is to do an direct, unconditional branch using the code that's already checked in in the function getUnconditionalBranch. Are you saying that there are circumstances in which that code doesn't work?

If not, then are you saying that there are circumstances in which that code will generate this 12 byte sequence? The bound code needs to provide an upper bound for the sequences produced by getUnconditionalBranch and getTrap.

With respect to b vs blx, the goal is to perform an unconditional branch without touching any stacks at all; does b fail this condition? I'm trying to do the equivalent of the X86 jmp.

http://reviews.llvm.org/D4167