[WIP] 2 - FnAbi, llvm.ptrauth.resign and Session API change#159074
[WIP] 2 - FnAbi, llvm.ptrauth.resign and Session API change#159074jchlanda wants to merge 3 commits into
Conversation
rustbot
added
A-LLVM
jchlanda
changed the title
8f37395 to
0792cdd
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
0792cdd to
281565d
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
281565d to
360a700
Compare
This comment has been minimized.
This comment has been minimized.
360a700 to
089232b
Compare
This patch implements Rust's equivalent of Clang's function pointer type discriminator computation used in pointer authentication. Compatibility with Clang is a primary goal. The discriminator produced for a given external "C" function type must match the value computed by Clang so that function pointers can be exchanged safely between Rust and C code while preserving pointer authentication semantics. The implementation mirrors Clang's behavior in `ASTContext::encodeTypeForFunctionPointerAuth`, ensuring that identical C-compatible function types produce identical discriminators. See: <https://clang.llvm.org/doxygen/ASTContext_8cpp.html#abb1375e068e807917527842d05cadea3>.
089232b to
d85aa6b
Compare
| let discriminator = if self.sess().pointer_authentication_fn_ptr_type_discrimination() { | ||
| fn_abi?.ptrauth_type_discriminator | ||
| } else { | ||
| 0 |
There was a problem hiding this comment.
Do we need this check here? As far as I can see, we already conditionally set ptrauth_type_discriminator to 0 if pointer_authentication_fn_ptr_type_discrimination() is false in compiler/rustc_ty_utils/src/abi.rs. Maybe we can even rename ptrauth_type_discriminator field in FnAbi to smth like just ptrauth_discriminator and assume it's storing the "effective" discriminator (either 0 or type discriminator)?
Just loudly thinking, I might easily miss smth - I would appreciate your explanation if I've got smth wrong
There was a problem hiding this comment.
You are right, we don't need it here, added comment about it though.
Also renaming to ptrauth_discriminator.
| ), | ||
| ptrauth_type_discriminator: if tcx.sess.pointer_authentication_fn_ptr_type_discrimination() | ||
| { | ||
| compute_fn_ptr_type_discriminator_for(tcx, sig).unwrap_or(0).into() |
There was a problem hiding this comment.
Just in case: do I get it correct that the sequence of PRs is not about "at each point we are expected to have a successful build", it's about ease of review only?
I'm just not seeing compute_fn_ptr_type_discriminator_for defined at this point - was it intentional or should PR content split decisions be reconsidered?
There was a problem hiding this comment.
No, you do not. It is intended to be buildable at each stage.
The split is twofold: logical, to show the progression of the implementation, and functional, since each commit is expected to be independently buildable. They are not exactly atomic in the sense that the functionality will not be complete, so if you check out a commit in the middle of the sequence, it should build, but the generated code might not be fully correct.
The function you are referring to was added in the first PR of the series:
https://github.com/rust-lang/rust/pull/159071/changes#diff-40f6b89998ba7d54d3f76fde93944eab9b8ba5a16eb1d763c598e2802a180af7R166
| /// Indicates if an unwind may happen across a call to this function. | ||
| pub can_unwind: bool, | ||
| /// Computed type discriminator for pointer authentication purpose. | ||
| pub ptrauth_type_discriminator: u64, |
There was a problem hiding this comment.
Just a future note: when submitting as final non-draft PR for mainline review, it might be worth to provide a summary why a new field in FnAbi is considered at this point the proper solution for our problem and why we really need that. The summary could be placed somewhere in the PR description/PR comments/elsewhere if a more appropriate place exists.
There was a problem hiding this comment.
Done, in the PR description.
d85aa6b to
7708e94
Compare
This comment has been minimized.
This comment has been minimized.
This patch introduces the following: * Extends `FnAbi` (`callconv`) with a `ptrauth_type_discriminator` field. This field is only used when emitting pointer authentication call bundles. It is stored in `FnAbi` because the call site is not guaranteed to have access to an `Instance`, so the discriminator cannot always be computed on demand. * Adds support for `llvm.ptrauth.resign`. This intrinsic will be used when support for semantic transmute is added. * Performs a minor API redesign as groundwork for allowing call sites to modify schemas in place.
7708e94 to
8ed3f55
Compare
4 participants
This patch introduces the following:
FnAbi(callconv) with aptrauth_type_discriminatorfield. This field is only used when emitting pointer authentication call bundles. It is stored inFnAbibecause the call site is not guaranteed to have access to anInstance, so the discriminator cannot always be computed on demand.llvm.ptrauth.resign. This intrinsic will be used when support for semantic transmute is added.The use of an extended
FnAbiis dictated by the need to issue a ptrauth bundle for call-like instructions. At that point, we only have access to theFnAbistruct (Instanceis not guaranteed to be available).Fundamentally, the discriminator originates from the source-level function type. Deriving it at the codegen stage would introduce a layering violation: it would require relying on
FnAbi'sArgAbi(for both return values and arguments) to preserve source-level type information. This is not a guarantee provided by these abstractions (and in code comments), asFnAbirepresents a lowered calling convention rather than the original function signature.Computing the discriminator at this stage would therefore create a hard dependency on type information being available where it is not intended to be preserved. Carrying the value explicitly avoids reconstructing source-level information from lowered ABI data and keeps the abstraction boundaries intact.
This is part 2 of a sequence of 8 PRs, that together aim to bring function pointer type discrimination support:
Useful links:
pauthtestintroduction: Introduce aarch64-unknown-linux-pauthtest target #155722