rjmccall added inline comments.
================
Comment at: clang/include/clang/Driver/Options.td:2865-2872
+let Group = f_Group in {
+ let Flags = [CC1Option] in {
+ def fptrauth_intrinsics : Flag<["-"], "fptrauth-intrinsics">,
+ HelpText<"Enable pointer-authentication intrinsics">;
+ }
+ def fno_ptrauth_intrinsics : Flag<["-"], "fno-ptrauth-intrinsics">;
+}
----------------
kristof.beyls wrote:
> My impression is that generally for `__builtin_XXX` intrinsics, there are no
> compiler flags to make them available or remove their availability.
> Is there a good reason why a command line option is needed for the
> `__builtin_ptrauth` intrinsics, but not (IIUC) for most or any other existing
> `__builtin_XXX` intrinsic?
> If there is no good reason, it seems better to me to not have a command line
> option so there is better consistency across all `__builtin_XXX` intrinsics?
>
> (after having read more of the patch): my impression has changed now that the
> f(no-)ptrauth-intrinsics flag rather selects whether the ptrauth intrinsics
> get lowered to PAuth hardware instructions, or to "regular" instructions
> emulating the behavior of authenticated pointers. If that is correct (and
> assuming it's a useful option to have), I would guess a different name for
> the command line option could be less misleading. As is, it suggests this
> selects whether ptrauth_ intrinsics are available or not. If instead, as I'm
> guessing above, this selects whether ptrauth_ intrinsics get lowered to PAuth
> instructions or not, maybe something like '-femulate-ptrauth' would describe
> the effect of the command line switch a bit better?
The ptrauth features were implemented gradually, beginning with the intrinsics.
Originally we needed a way to enable the intrinsics feature without relying on
target information. We do still need a way to enable them without necessarily
enabling automatic type/qualifier-based pointer authentication. I don't know
if we need to be able to *disable* them when the target supports them; I agree
that that would be a little strange.
If not, we could just enable the intrinsics whenever either the target says
they're okay or software emulation (a separate, experimental feature) is
enabled. The AArch64 target has a `+pauth` target feature. However, I don't
know if `-arch arm64e` actually adds that feature on Apple targets. Also, the
`HasPAuth` field in the clang `TargetInfo` does not appear to be properly
initialized to `false` when `+pauth` *isn't* present; fortunately, that field
never used.
I'm not sure if it would actually be okay to remove the `-fptrauth-intrinsics`
driver option if we just enabled the intrinsics based on the target feature.
That does feel cleaner, but unfortunately, we at Apple probably have explicit
uses of the option that we'd have to clean up before we could remove the
option. We could treat that as an Apple problem and keep it out of the open
source tree, though, and maybe remove the option altogether someday.
Ahmed, thoughts?
================
Comment at: clang/lib/Headers/ptrauth.h:19-37
+ /* A process-independent key which can be used to sign code pointers.
+ Signing and authenticating with this key is a no-op in processes
+ which disable ABI pointer authentication. */
+ ptrauth_key_process_independent_code = ptrauth_key_asia,
+
+ /* A process-specific key which can be used to sign code pointers.
+ Signing and authenticating with this key is enforced even in processes
----------------
kristof.beyls wrote:
> I think, but am not sure, that the decision of which keys are process
> independent and which ones are process-dependent is a software platform
> choice?
> If so, maybe ptrauth_key_process_{in,}dependent_* should only get defined
> conditionally?
> I'm not sure if any decisions have been taken already for e.g. linux,
> Android, other platforms.
> If not, maybe this block of code should be surrounded by an ifdef that is
> enabled only when targeting Darwin?
Yes, in retrospect it was a bad idea to define these particular generic names.
I believe Apple platforms no longer have "process-independent" keys. It should
really just be (1) the concrete keys, (2) recommended default keys for code and
data pointers, and then (3) the specific keys used in specific schemas. Beyond
that, if people want a specific different key for some purpose, they should ask
for it.
Unfortunately, there's already a fair amount of code using these names. We
could deprecate the old names and point people towards the new names, though.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112941/new/
https://reviews.llvm.org/D112941
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
