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

[Oliver Stannard (Linaro) via Phabricator via llvm-commits]

ostannard accepted this revision.
ostannard added a comment.
This revision is now accepted and ready to land.

LGTM, with some comments which can be addressed later given the time-sensitivity of this.



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


================
Comment at: llvm/lib/Target/AArch64/AArch64SLSHardening.cpp:111
+  switch (MI.getOpcode()) {
+  case AArch64::BLRCall:
+  case AArch64::BLRCallNoIP:
----------------
`BLRCall` shouldn't be selected when this pass is enabled, so maybe we should assert here?


================
Comment at: llvm/lib/Target/AArch64/AArch64SLSHardening.cpp:156
+    "__llvm_slsblr_thunk_x14", "__llvm_slsblr_thunk_x15",
+    "__llvm_slsblr_thunk_x18", "__llvm_slsblr_thunk_x19",
+    "__llvm_slsblr_thunk_x20", "__llvm_slsblr_thunk_x21",
----------------
Please add a comment noting that x16 and x17 are deliberately omitted.


================
Comment at: llvm/lib/Target/AArch64/AArch64SLSHardening.cpp:162
+    "__llvm_slsblr_thunk_x28", "__llvm_slsblr_thunk_x29",
+    "__llvm_slsblr_thunk_x30", "__llvm_slsblr_thunk_x31",
+};
----------------
X30 doesn't need to be in these lists either, it (correctly) isn't allowed with `BLRCallNoIP`.


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

https://reviews.llvm.org/D81402