[PATCH] Add Forward-Edge Control-Flow Integrity support
Tom Roeder
tmroeder at google.com
Mon Jul 14 14:34:57 PDT 2014
================
Comment at: include/llvm/CodeGen/CommandFlags.h:245
@@ +244,3 @@
+CFIFuncName("cfi-func-name", cl::desc("The name of the CFI function to call"),
+ cl::init(""));
+
----------------
JF Bastien wrote:
> Could you clarify that the above two options are linked? Or just have cfi-func-name, implying a call to abort otherwise? It kind of seems like the default cfi-func could just call abort?
>
> Also, what's the function's expected signature?
>
> What happens if the cfi-func itself gets CFI instrumented, and it fails the check? *That* should probably call abort directly, not recurse :)
Is there an abort function that's guaranteed to be present for any IR? Right now, my "abort" is to execute a trap instruction.
================
Comment at: lib/CodeGen/ForwardControlFlowIntegrity.cpp:151
@@ +150,3 @@
+ FunctionType *FunTy = dyn_cast<FunctionType>(EltTy);
+ if (!FunTy)
+ continue;
----------------
JF Bastien wrote:
> Same.
I think you're right here and in the previous comment: this shouldn't ever happen.
================
Comment at: lib/CodeGen/ForwardControlFlowIntegrity.cpp:198
@@ +197,3 @@
+ // really get this information from the Target in some way.
+ int64_t MaskValue = ((TableSize << 3) - 1) & -8;
+ Constant *JumpTableMaskValue = ConstantInt::get(Int64Ty, MaskValue);
----------------
JF Bastien wrote:
> Could you fix this?
I'd like to fix this, but I don't know where I can get this information in a principled manner. This requires the size of a jumptable entry, and that depends on the size of the jump instruction in bytes. In principle, this is available from MCInstrDesc::getSize(), but that returns 0 when I've tried it for JMP_4 on X86.
I can certainly encode it myself in the Targets, but that seems like dangerous duplication of functionality with the descriptions in the instruction tables.
================
Comment at: lib/CodeGen/ForwardControlFlowIntegrity.cpp:297
@@ +296,3 @@
+ Constant *RightShift = ConstantInt::get(Int64Ty, 3);
+ Constant *LeftShift = ConstantInt::get(Int64Ty, 64 - 3);
+
----------------
JF Bastien wrote:
> Shouldn't you get the pointer size from the Target here too?
It's not a question of pointer size but rather the size of an unconditional jump instruction. I haven't yet figured out where to get that information.
http://reviews.llvm.org/D4167
More information about the llvm-commits
mailing list