[PATCH] D40482: [X86] Instrument Control Flow For Indirect Branch Tracking

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added inline comments.


================
Comment at: include/llvm/Target/TargetOptions.h:221
+    /// mechanisms. For example: in X86 - Indirect Branch Tracking.
+    unsigned InstrumentControlFlow : 1;
+
----------------
oren_ben_simhon wrote:
> pcc wrote:
> > This doesn't seem like it needs to be a TargetOption. Can you test for the function attribute directly in your pass?
> Sure, I will add a check in the pass for the function attribute. 
> Still, I would like the user to be able to give the llc the command option "instrument-control-flow" so that it will be enabled even if the attribute is not set in the function.
> Is there a better way to do that than TargetOption?
Why do you need this capability?


================
Comment at: lib/Target/TargetMachine.cpp:76
   RESET_OPTION(NoTrappingFPMath, "no-trapping-math");
+  RESET_OPTION(InstrumentControlFlow, "instrument-control-flow");
 
----------------
oren_ben_simhon wrote:
> pcc wrote:
> > This name seems very generic and doesn't really describe what the attribute does. How about something like `cet-compatible`?
> I wouldn't like this attribute to be target independent.  cet-compatible refers to Intel specific way to protect against control flow attacks. Does "cf-protection" make more sense?
I assume that by "wouldn't" you meant "would".

I'm afraid that the name "cf-protection" doesn't really make sense to me either. There are a variety of ways to protect against control flow attacks, both purely software based (e.g. `-fsanitize=cfi`) and using hardware features such as CET, and specifically in the case of CET there also needs to be support from the rest of the operating system (so a user shouldn't expect this feature to provide any protection unless their hardware and operating system specifically supports it). So I think it would be less confusing if the attribute refers to the specific technology used.


Repository:
  rL LLVM

https://reviews.llvm.org/D40482