[PATCH] D132386: [AArch64][PAC] Lower auth/resign into checked sequence.

[Kristof Beyls via Phabricator via llvm-commits]

kristof.beyls added a comment.

I finally found time to go through the patch from beginning to end. I only have 2 more - hopefully minor questions.
Apart from that, LGTM.



================
Comment at: llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp:1278
+  // In the checked sequence, we only trap if explicitly requested.
+  bool ShouldTrap = MF->getFunction().hasFnAttribute("ptrauth-auth-traps");
+
----------------
I wonder if this function attribute is already documented as part of another patch?
If not, maybe this patch should contain some LLVM-IR LangRef documentation for it?


================
Comment at: llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp:1371-1376
+      // Non-trapping checked sequences return the stripped result in x16,
+      // skipping over the PAC if there is one.
+
+      // FIXME: can we simply return the AUT result, already in x16? without..
+      //        ..traps this is usable as an oracle anyway, based on high bits
+      //  mov x17, x16
----------------
I would, maybe naively, assume that returning the stripped pointer is worse than returning the failed-authenticated pointer. The stripped pointer is a valid raw pointer;  the failed-authenticated pointer is an invalid raw pointer, i.e. should trigger a memory access fault when dereferenced.
Therefore, why is it better to return the stripped pointer?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132386