[llvm-branch-commits] [llvm] [BOLT] Gadget scanner: detect signing oracles (PR #134146)

[Kristof Beyls via llvm-branch-commits]

================
@@ -622,6 +628,40 @@ class MCPlusBuilder {
     return std::make_pair(getNoRegister(), getNoRegister());
   }
 
+  /// Analyzes if a pointer is checked to be valid by the end of BB.
+  ///
+  /// It is possible for pointer authentication instructions not to terminate
+  /// the program abnormally on authentication failure and return some *invalid
+  /// pointer* instead (like it is done on AArch64 when FEAT_FPAC is not
+  /// implemented). This might be enough to crash on invalid memory access
+  /// when the pointer is later used as the destination of load/store or branch
+  /// instruction. On the other hand, when the pointer is not used right away,
+  /// it may be important for the compiler to check the address explicitly not
+  /// to introduce signing or authentication oracle.
+  ///
+  /// If this function returns a (Reg, Inst) pair, then it is known that in any
+  /// successor of BB either
+  /// * Reg is trusted, provided it was safe-to-dereference before Inst, or
+  /// * the program is terminated abnormally without introducing any signing
+  ///   or authentication oracles
+  virtual std::optional<std::pair<MCPhysReg, MCInst *>>
----------------
kbeyls wrote:

Thank you for those improved comments and the test case, that makes a lot more sense to me now!

One thing I'm still wondering about before resolving this review thread, is if you have any thoughts about whether any instruction patterns that guarantee the program will terminate on an authentication fault might not be detected by either of the 2 versions of `getAuthCheckedReg`?

It makes sense to update the documentation in a separate PR, but it might be useful to record the answer to the above question in a review comment to later help write up in the documentation what classes of false positive/negatives the user may still expect to see?

https://github.com/llvm/llvm-project/pull/134146