[PATCH] D112941: [clang] Add support for the new pointer authentication builtins.

[Ahmed Bougacha via Phabricator via cfe-commits]

ab added a comment.

In D112941#3105297 <https://reviews.llvm.org/D112941#3105297>, @tschuett wrote:

> The original `ptrauth.h` has the same comment style. Would doxygen style be an improvement?

Hmm, what do you have in mind?  Markup for the builtin arguments/returns?  The need for `__` prefixes makes that a bit awkward, but I suppose that's not a big deal (and we do live with that for x86 intrins already)



================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:839
+def err_ptrauth_disabled :
+  Error<"pointer authentication is disabled for the current target">;
+def err_ptrauth_invalid_key :
----------------
rjmccall wrote:
> apazos wrote:
> > These two error types are confusing. 
> > In which situation would err_ptrauth_disabled be printed?
> > With this patch, it is only supported with arm64e triple, all other targets it is unsupported.
> We could probably just have one of these, yes.
With this patch you'd get `err_ptrauth_disabled` with `-target arm64e- -fno-ptrauth-intrinsics`, instead of `err_ptrauth_disabled_target` with say `-target x86_64`.  But yeah, I suppose we could live with only one of the two, with a less accurate message (either `err_ptrauth_disabled_target` renamed to `err_ptrauth_disabled`, or another more vague message altogether, say "pointer authentication is not supported")

This mostly removes the need for `Sema::diagnosePointerAuthDisabled` and `TargetInfo::isPointerAuthSupported` and lets us just rely on the individual langopts, which seems okay.


================
Comment at: clang/lib/Basic/Targets/AArch64.cpp:847
+    const llvm::APSInt &value) const {
+  return 0 <= value && value <= 3;
+}
----------------
rjmccall wrote:
> There's an LLVM constants header for this now, right?
Hmm, I don't think so, do you have something in mind?  The AArch64 backend has helpers for this, but that's confined there.  IR doesn't know anything about keys (for instance, the IR verifier doesn't have the same validation done here)

There's a related latent concern I have: currently, nothing prevents optimizations from moving ptrauth ops into global constant initializers, and since we don't support global initializers with process-specific keys (at least on darwin), we may need to expose this knowledge somehow (undoing this in the backend is not straightforward, but is an option). 

Knowing which keys are used in what way would need to be triple-specific, though I guess that's not an obstacle.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112941