[PATCH] D27449: [ARM] Implement execute-only support in CodeGen

Jim Grosbach via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 8 13:31:48 PST 2016


grosbach added a comment.

In https://reviews.llvm.org/D27449#617442, @rengolin wrote:

> In https://reviews.llvm.org/D27449#617363, @grosbach wrote:
>
> > Seems a reasonable thing to support in general. Were the concerns about supporting execute-only conceptually or about the design of this approach?
>
>
> About the concept of unreadable code areas in general, and what benefits this really brings to security and performance of deeply embedded devices.


Gotcha. My recollection is that it's generally not to valuable in isolation, but can be used in combination with other measures to provide some legitimate mitigations. Usual caveats apply. I'm not a security expert, blah blah. :)

> I honestly think this is a feature that ARM hardware needs support and I'm not a hardware expert to decide what features people need on such an individualised market. With the change itself being reasonably non-intrusive, I can't see why we'd block such a feature on those grounds only.

Agreed.



================
Comment at: lib/Target/ARM/ARMSubtarget.cpp:173
+  // Execute only support requires movt support
+  if (genExecuteOnly()) {
+    NoMovt = false;
----------------
rengolin wrote:
> grosbach wrote:
> > prakhar wrote:
> > > rengolin wrote:
> > > > I'm worried about the silent behaviour here.
> > > > 
> > > > If movt is disabled by some other part or user flag, than this should be an error, not a silent change.
> > > Yes, this should be an error. I've changed this to report an error if NoMovt is enabled or the target doesn't support MOVT. I'll also add a diagnostic to clang for this.
> > The error should be detected earlier and a proper diagnostic issued if at all possible. Using report_fatal_error() for user-facing diagnostics is an option of last resort and an indicator of a layering failure elsewhere.
> There is an error in Clang (D27450) that detects this. Is there a better way to do this in LLVM itself?
If there's a legitimate user-facing way to get here and this be the first time we can diagnose the error, we can use the emitError() method on the LLVMContext to complain about it. If, OTOH, getting here in that state indicates a bug earlier in the pipeline (probably true?), then perhaps this could just be an assert().


https://reviews.llvm.org/D27449





More information about the llvm-commits mailing list