Conversation
|
Some changes occurred in compiler/rustc_passes/src/check_attr.rs cc @jdonszelmann, @JonathanBrouwer
Some changes occurred in compiler/rustc_attr_parsing cc @jdonszelmann, @JonathanBrouwer Some changes occurred in compiler/rustc_attr_ir |
rustbot
added
A-LLVM
|
r? @folkertdev rustbot has assigned @folkertdev. Use Why was this reviewer chosen?The reviewer was selected based on:
|
This comment has been minimized.
This comment has been minimized.
|
r? davidtwco (who I believe is the dedicated reviewer for PAC) |
jchlanda
changed the title
jchlanda
changed the title
e049e45 to
1eb204a
Compare
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.
Also tighten the handling of enums.
Also remove error messages/tests that used to guarded it.
The codegen now walks the layout of static initializer types to find extern "C" function pointer fields, computes their type discriminators, and applies those discriminators when emitting authenticated function pointer relocations. Also make sure that type discrimination is never applied to init/fini entries.
This covers standalone function pointer constants, promoted temporaries, immutable and mutable statics, arrays of function pointers, and mixed structs containing function pointers. Consult pauth-fn-ptr-type-discrimination-static-allocs.rs test for example uses. Revolves around threading PAC information through: * static_addr_of (StaticCodegenMethods) * from_const and from_const_alloc (both on rustc_codegen_ssa::mir::operand / OperandRef)
Also a fix for non function (closure, coroutines, etc) in discriminator_input. Fix in v-table assert.
Meaning if two types are ABI compatible they must have the same encoding and hash value. Provide a ui test which groups the function pointers by the ABI compatibility rules and enforces the rule.
1eb204a to
bb798a1
Compare
|
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. |
| // also ABI-compatible with each other for different T if they have the same | ||
| // metadata type (<T as Pointee>::Metadata)." | ||
| #[rustc_dump_ptrauth_discriminator(ptrauth_encoding, ptrauth_hash)] | ||
| extern "C" fn g1_a(_: *const i32) {} // expect: "FvPE": 10942 (0x2abe) |
There was a problem hiding this comment.
any reason not to do this?
| extern "C" fn g1_a(_: *const i32) {} // expect: "FvPE": 10942 (0x2abe) | |
| extern "C" fn g1_a(_: *const i32) {} //~ ERROR ptrauth discriminator encoding: "FvPE" |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
//~ 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)?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I missed that you produce two errors. That makes the //~ ERROR way less ergonomic.
6 participants
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:
Useful links:
pauthtestintroduction: Introduce aarch64-unknown-linux-pauthtest target #155722