[PATCH] D81400: [AArch64] Introduce AArch64SLSHardeningPass, implementing hardening of RET and BR instructions.

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

ostannard added a comment.

The pre-commit test failures are in the newly added test, so need to be fixed before this can be committed. My inline comments could be addressed as follow up patches given that this is related to a recently-published security vulnerability.



================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.h:388
 
 static inline bool isIndirectBranchOpcode(int Opc) {
+  switch (Opc) {
----------------
There's a `isIndirectBranch` flag on instructions in the tablegen, so is this needed?


================
Comment at: llvm/lib/Target/AArch64/AArch64SLSHardening.cpp:106
+  MachineBasicBlock::iterator NextMBBI;
+  for (; MBBI != E; MBBI = NextMBBI) {
+    MachineInstr &MI = *MBBI;
----------------
Since we only care about instructions which are terminators, could this me made more efficient by just scanning the terminators?


================
Comment at: llvm/test/CodeGen/AArch64/speculation-hardening-sls.ll:22
+  ret i32 %retval.0
+; CHECK-LABEL: double_return:
+; CHECK:       {{ret$}}
----------------
I assume that the return instruction is being split into two in the backend somewhere, but it would be clearer to just have two returns in the IR. If this is testing for a bug triggered when that optimisation happening, then this could do with a comment explaining that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81400