[PATCH] D73070: Add function attribute "patchable-function-prefix" to support -fpatchable-function-entry=N,M where M>0

Nick Desaulniers via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 22 09:18:25 PST 2020


nickdesaulniers added a comment.

I don't mind landing this without aarch64 bti interplay matching GCC precisely; I think it can be done in a follow up.  I just request we don't try to land this in the clang-10 release unless that is also resolved.  Otherwise clang-11.



================
Comment at: llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp:1303
+  while (N--)
+    EmitToStreamer(*OutStreamer, MCInstBuilder(AArch64::HINT).addImm(0));
+}
----------------
nickdesaulniers wrote:
> MaskRay wrote:
> > nickdesaulniers wrote:
> > > I think it would be better to use `TargetInstrInfo::getNoop()` via `MF->getSubtarget().getInstrInfo()->getNoop(Noop);`. Then you could make this a non-virtual function call in the base `AsmPrinter` class.  In fact, emitting nops is useful and could be a method on the base `AsmPrinter`, and that could be reused between `AsmPrinter::EmitFunctionHeader()` and `AsmPrinter::EmitFunctionBody()`.
> > TIL getNoop. I was looking for such an operation bug did not find one. This is much more elegant! Added `AsmPrinter::emitNops`.
> > 
> > Unfortunately we cannot retire `LowerPATCHABLE_FUNCTION_ENTER`.
> > 
> > AsmPrinter (assembly printer/object file emitter) does the following:
> > 
> > 1. Emit data before the function entry
> > 2. Emit the function entry label and the label for __patchable_function_entries
> > 3. Emit the function body (which may have optional `bti c` or `endbr64`)
> > 
> > Due to `bti c` (AArch64) and `endbr64` (x86), we cannot make the handling of the function attribute "patchable-function-entry" generic.
> > (-fpatchable-function-entry=, -mbranch-protection= (AArch64 BTI) and -fcf-protection= (Intel Indirect Branch Tracking) are implemented as passes which can prepend instructions to the function body.)
> Looks like we can still modify `AsmPrinter::EmitFunctionBody()` and `ARMAsmPrinter::EmitSled()` to now make use of `AsmPrinter::emitNops()` in this change.
bump


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73070/new/

https://reviews.llvm.org/D73070





More information about the llvm-commits mailing list