[PATCH] D81402: [AArch64] Extend AArch64SLSHardeningPass to harden BLR instructions.

[Kristof Beyls via Phabricator via llvm-commits]

kristof.beyls marked 6 inline comments as done.
kristof.beyls added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.td:2020
+  def BLR : BranchReg<0b0001, "blr", []>;
+  def BLRCall : Pseudo<(outs), (ins GPR64:$Rn), []>,
+                Sched<[WriteBrReg]>;
----------------
ostannard wrote:
> Is the `BLRCall` pseudo actually needed, or could we just use `BLR` when not doing the mitigation?
Indeed BLRCall is not needed. Now removed in the latest version of the patch.


================
Comment at: llvm/lib/Target/AArch64/AArch64SLSHardening.cpp:111
+  switch (MI.getOpcode()) {
+  case AArch64::BLRCall:
+  case AArch64::BLRCallNoIP:
----------------
ostannard wrote:
> `BLRCall` shouldn't be selected when this pass is enabled, so maybe we should assert here?
I've changed the pseudo expansion to use PseudoInstExpansion.
Depending on when that expansion happens in the pipeline (before or after the mitigation pass), one could see BLR or BLRNoIP here.
I thought it's safest to just handle both here. There's an assert in ConvertBLRToBL that should catch what we actually care about, i.e. not using X16/X17 (nor LR).


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

https://reviews.llvm.org/D81402