[PAC] ABI-compatibility rules for discriminator calculation (4.5/8)

[jchlanda]

View all comments

This patch extends the discriminator calculation logic to honour Rust's ABI compatibility rules. If two types are ABI compatible they must end up with the same discriminator.

The patch also includes a UI-based test (inspired by layout dump test) that enforces the rule.

---

This is part 2 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

• 4.5 ABI-compatibility

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:

• Previous PAC work:
  • pauthtest introduction: Introduce aarch64-unknown-linux-pauthtest target #155722
  • Library support follow up: Library support for aarch64-unknown-linux-pauthtest target #156548
  • Config follow up: Pointer authentication config and user facing options #156712
• Project goal and tracking issue
• Clang's implementation of ASTContext::encodeTypeForFunctionPointerAuth
• LLVM's SpiHash

[rustbot]

Some changes occurred in compiler/rustc_passes/src/check_attr.rs

cc @jdonszelmann, @JonathanBrouwer

rustc_codegen_gcc is developed in its own repository. If possible, consider making this change to rust-lang/rustc_codegen_gcc instead.

cc @antoyo, @GuillaumeGomez

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_attr_ir

cc @jdonszelmann, @JonathanBrouwer

[rustbot]

r? @folkertdev

rustbot has assigned @folkertdev.
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 77 candidates
• Random selection from 18 candidates

[folkertdev]

r? davidtwco (who I believe is the dedicated reviewer for PAC)

[bjorn3]

any reason not to do this?

Suggested change

	extern "C" fn g1_a(_: *const i32) {} // expect: "FvPE": 10942 (0x2abe)
	extern "C" fn g1_a(_: *const i32) {} //~ ERROR ptrauth discriminator encoding: "FvPE"

View changes since the review

[jchlanda]

I followed how rustc_dump_layout is handled, for example: https://github.com/rust-lang/rust/blob/main/tests/ui/attributes/doc_examples/rustc_dump_layout_align.rs#L1Press

//@ dont-require-annotations: ERROR seems like a nice trick to reduce the noise.

[bjorn3]

In this case you are already saying what you expect in the test itself, but not letting compiletest check that the expectation matches. If you are worried about the "ptrauth discriminator encoding" noise, maybe it would be possible to normalize that away?

[jchlanda]

Not sure if I understand your point. My idea was to be able to check what the discriminator is right at the point of calculating it, not when it's attached to a call instruction or function pointer address. That's why I decided to use attribute. The down side of it is that we effectively are saying "as long as the stderr match all is good", rather than doing more sophisticated check (like FileCheck capture plus match for example).

[bjorn3]

//~ ERROR ptrauth discriminator encoding: "FvPE" will match the error message that was emitted to stderr similar to filecheck. Just without regex support. So my question is why you used // expect: "FvPE": 10942 (0x2abe) (which is not checked by compiletest) instead of //~ ERROR ptrauth discriminator encoding: "FvPE" (which is checked)?

[jchlanda]

Right, so for each use of #[rustc_dump_ptrauth_discriminator(ptrauth_encoding, ptrauth_hash)] you get:

error: ptrauth discriminator encoding: "FvPE"
  --> $DIR/discriminator-abi-compat-encoder-and-hash.rs:47:1
   |
LL | extern "C" fn g1_a(_: *const i32) {} // expect: "FvPE": 10942 (0x2abe)
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: ptrauth discriminator hash: 10942 (0x2abe)
  --> $DIR/discriminator-abi-compat-encoder-and-hash.rs:47:1
   |
LL | extern "C" fn g1_a(_: *const i32) {} // expect: "FvPE": 10942 (0x2abe)
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Which is a mixture of:

• compiler generated message error: ptrauth discriminator encoding: "FvPE" / error: ptrauth discriminator hash: 10942 (0x2abe)
• my comment // expect: "FvPE": 10942 (0x2abe)

I though it was quite neat, as it gives you an immediate feedback, should this ever fail it will be clear what two values were expected.

[bjorn3]

I missed that you produce two errors. That makes the //~ ERROR way less ergonomic.

[jchlanda]

cc @RalfJung
Looks like the zulip discussion have turned into a generic aarch64-unknown-linux-pauthtest stream, so pinging you here.

The nature of this series of PRs is a bit messy (with the subsequent PR containing all the patches from the previous), but in the last commit I've tried to enforce ABI compatibility rules for discriminators. I've tested that members of the same "compatibility group" indeed end up with identical discriminator.

One notable departure is that the rule about function pointers:

"Any two fn (function pointer) types are ABI-compatible with each other if they have the same ABI string or the ABI string only differs in a trailing -unwind, independent of the rest of their signature. (This means you can pass fn() to a function expecting fn(i32), and the call will be valid ABI-wise. The callee receives the result of transmuting the function pointer from fn() to fn(i32); that transmutation is itself a well-defined operation, it’s just almost certainly UB to later call that function pointer.)"

This only applies when function pointers are used as values (as arguments to a function call). Applying it literally/across the board would make type discrimination impossible.

As a side note, I'm not running away from the idea of a C-compatible layer; I'm still happy to discuss and contribute once it crystallizes.

[RalfJung]

This only applies when function pointers are used as values (as arguments to a function call).

Yes, that is what these docs are talking about.

I've tested that members of the same "compatibility group" indeed end up with identical discriminator.

Great, thanks!
I don't have the capacity to review the code, unfortunately.

[rustbot]

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.

[rust-bors]

☔ The latest upstream changes (presumably #163208) made this pull request unmergeable. Please resolve the merge conflicts by rebasing.