Skip to content

[PAC] Encoder and hash (1/8)#159071

Open
jchlanda wants to merge 2 commits into
rust-lang:mainfrom
jchlanda:jakub/pac_ty_disc_PR_1
Open

[PAC] Encoder and hash (1/8)#159071
jchlanda wants to merge 2 commits into
rust-lang:mainfrom
jchlanda:jakub/pac_ty_disc_PR_1

Conversation

@jchlanda

@jchlanda jchlanda commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

This patch implements Rust's equivalent of Clang's function pointer type discriminator computation used for pointer authentication. Compatibility with Clang is a primary design goal. For a given extern "C" function type, the discriminator produced by Rust 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.


This is part 1 of a sequence of 8 PRs that together implement support for function pointer type discrimination:

  1. Encoder and hash
  2. FnAbi, llvm.ptrauth.resign and Session API change
  3. FPTR_TYPE_DISCR in ABI Version
  4. Static allocs
  5. Transmutes
  6. Propagate discriminator logic through remaining get_fn_ptr calls sites
  7. Minicore updates to support fn ptr type discriminator tests
  8. Fn ptr type discrimination tests

Useful links:

@rustbot rustbot added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 10, 2026
@jchlanda
jchlanda force-pushed the jakub/pac_ty_disc_PR_1 branch 2 times, most recently from bca55be to 8cf67b9 Compare July 14, 2026 07:26
@rust-log-analyzer

This comment has been minimized.

@rust-bors

This comment has been minimized.

@jchlanda
jchlanda force-pushed the jakub/pac_ty_disc_PR_1 branch from 8cf67b9 to 7ee3f2c Compare July 14, 2026 08:51
@rust-log-analyzer

This comment has been minimized.

@rust-bors

This comment has been minimized.

@jchlanda
jchlanda force-pushed the jakub/pac_ty_disc_PR_1 branch from 7ee3f2c to fdd1189 Compare July 17, 2026 07:01
@rust-log-analyzer

This comment has been minimized.

@jchlanda
jchlanda force-pushed the jakub/pac_ty_disc_PR_1 branch from d798449 to 4c1b0ac Compare July 17, 2026 09:46

@kovdan01 kovdan01 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to add unit tests for hash computation like the ones present in llvm source tree at llvm/unittests/Support/SipHashTest.cpp?

View changes since this review

@jchlanda

Copy link
Copy Markdown
Contributor Author

Would it be possible to add unit tests for hash computation like the ones present in llvm source tree at llvm/unittests/Support/SipHashTest.cpp?

View changes since this review

Done in: 5e05159

@jchlanda
jchlanda force-pushed the jakub/pac_ty_disc_PR_1 branch 2 times, most recently from 03337eb to 8b0e0ca Compare July 24, 2026 15:29
@jchlanda jchlanda changed the title [WIP] 1 - Encoder and hash [PAC] Encoder and hash (1/8) Jul 24, 2026
@jchlanda
jchlanda marked this pull request as ready for review July 24, 2026 18:07
@rustbot

rustbot commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Some changes occurred in src/doc/rustc/src/platform-support

cc @Noratrieb

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 24, 2026
@rustbot

rustbot commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

r? @wesleywiser

rustbot has assigned @wesleywiser.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler
  • compiler expanded to 74 candidates
  • Random selection from 18 candidates

@jchlanda

Copy link
Copy Markdown
Contributor Author

r? compiler

@rustbot rustbot assigned tiif and unassigned wesleywiser Jul 24, 2026
jchlanda added 2 commits July 27, 2026 08:11
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>.
@jchlanda
jchlanda force-pushed the jakub/pac_ty_disc_PR_1 branch from 8b0e0ca to 67fd305 Compare July 27, 2026 08:12
@rustbot

rustbot commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@davidtwco

Copy link
Copy Markdown
Member

r? @davidtwco

@rustbot rustbot assigned davidtwco and unassigned tiif Jul 27, 2026
@@ -0,0 +1,493 @@
/*!

@davidtwco davidtwco Jul 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use //! so that this is a documentation comment

View changes since the review

@@ -0,0 +1,142 @@
// LLVM SipHash-2-4

@davidtwco davidtwco Jul 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can't we just call LLVM's definitions of these over FFI from rustc_codegen_llvm? That let us avoid the need for this to remain identical to LLVM's definition

View changes since the review

Comment on lines +355 to +357
/// This is not a full semantic translation of Rust types. It is a lossy mapping
/// that intentionally matches Clang's function pointer authentication encoding
/// rules where Rust has a direct language-level equivalent.

@davidtwco davidtwco Jul 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These lines are duplicated

View changes since the review

let ty = canonicalize_c_type(tcx, ty);
match ty.kind() {
// C void / Rust ()
ty::Tuple(list) if list.is_empty() => ClangDiscTy::Void,

@davidtwco davidtwco Jul 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_ if ty.is_unit() is clearer

View changes since the review


loop {
match ty.kind() {
ty::Adt(def, args) if tcx.is_diagnostic_item(sym::Option, def.did()) => {

@davidtwco davidtwco Jul 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should use the LangItem::Option, rather than the diagnostic item, given this isn't for a diagnostic.

View changes since the review


// Clang disc type.
#[derive(Debug)]
enum ClangDiscTy<'tcx> {

@davidtwco davidtwco Jul 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what we're gaining from having this intermediate representation - the mapping seems straightforward enough that you could just go direct from a Ty to a encoding

View changes since the review

All tests from `assembly-llvm`, `codegen-llvm`, `codegen-units`, `coverage`,
`crashes`, `incremental`, `library`, `mir-opt`, `run-make`, `ui` and
`ui-fulldeps` subsets are expected to pass.
`crashes`, `incremental`, `library`, `mir-opt`, `run-make`, `rust_middle` `ui`

@davidtwco davidtwco Jul 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
`crashes`, `incremental`, `library`, `mir-opt`, `run-make`, `rust_middle` `ui`
`crashes`, `incremental`, `library`, `mir-opt`, `run-make`, `rust_middle`, `ui`

View changes since the review

@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 27, 2026
@rustbot

rustbot commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@rustbot rustbot added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jul 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants