Skip to content

CFI: Cover each InstanceKind explicitly and reorganize UI tests - #160230

Open
rcvalle wants to merge 4 commits into
rust-lang:mainfrom
rcvalle:rust-cfi
Open

CFI: Cover each InstanceKind explicitly and reorganize UI tests#160230
rcvalle wants to merge 4 commits into
rust-lang:mainfrom
rcvalle:rust-cfi

Conversation

@rcvalle

@rcvalle rcvalle commented Jul 30, 2026

Copy link
Copy Markdown
Member
  • Changes transform_instance to cover each InstanceKind (and ShimKind) explicitly, similarly to how encoding is done, so the intent is expressed clearly and it is known when an instance is handled (or not) and the side effects of it (also clearly), instead of relying on fallthrough behavior. This also makes adding a new InstanceKind (or ShimKind) result in a compile-time error until it is explicitly handled. No functional changes intended.
  • Reorganizes the CFI and KCFI UI tests under the cfi and kcfi directories similarly to how the CFI codegen tests are organized: each test now tests a type or a language construct or feature that is handled by the CFI and KCFI transform or encoding, with complete coverage of what can be exercised at run time, and is named works-with-, and tests that covered both CFI and KCFI using revisions are split into separate tests. It also adds tests for the types, language constructs, and features that were not covered (i.e., intrinsics, thread locals, builtin Clone and FnPtr implementations, pattern types, the never type, extern types, C variadics, and the generalize-pointers and normalize-integers options), removes the tests that became redundant, and changes the drop tests to run-pass, as they now pass at run time. Tests for flags/options are unchanged (only moved into the cfi and kcfi directories when at the top level).

r? @bjorn3

@rustbot rustbot added A-tidy Area: The tidy tool PG-exploit-mitigations Project group: Exploit mitigations S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 30, 2026
@rcvalle rcvalle added the A-control-flow-integrity Area: Control Flow Integrity (CFI) security mitigation label Jul 30, 2026
@rcvalle
rcvalle force-pushed the rust-cfi branch 2 times, most recently from a7acb1a to b8945d8 Compare July 30, 2026 21:23
@rustbot

rustbot commented Jul 30, 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.

@bjorn3

bjorn3 commented Aug 4, 2026

Copy link
Copy Markdown
Member

The first commit does both mechanical refactorings and non-refactoring changes together. Would you mind splitting the variable renamings and outlining into a separate commit to make it easier to review? For the second commit it might also be nice to move file renamings (+ minor adjustments) into a commit separate from new tests and wholesale replacements of existing tests.

rcvalle added 4 commits August 7, 2026 11:27
Moves the DropGlue, virtual call, VTableShim, and closure-like
transformations in transform_instance into the transform_drop_glue,
transform_virtual_call, transform_vtable_shim, and
transform_closure_like functions, and renames variables for consistency
(e.g., invoke_ty to self_ty).
Changes transform_instance to cover each InstanceKind (and ShimKind)
explicitly, similarly to how encoding is done, so the intent is
expressed clearly and it is known when an instance is handled (or not)
and the side effects of it (also clearly), instead of relying on
fallthrough behavior. This also makes adding a new InstanceKind (or
ShimKind) result in a compile-time error until it is explicitly handled.
Moves the CFI and KCFI UI tests that are at the top level into the cfi
and kcfi directories (removing the now redundant kcfi- prefix from
their names), and renames the regression tests to
<topic>-issue-<number> (removing their entries from issues.txt
accordingly, and adding the missing regression test description to
reveal-opaques-issue-114160.rs).
Reorganizes the CFI and KCFI UI tests under the cfi and kcfi
directories similarly to how the CFI codegen tests are organized: each
test now tests a type or a language construct or feature that is
handled by the CFI and KCFI transform or encoding, with complete
coverage of what can be exercised at run time, and is named
works-with-<type-construct-or-feature>, and tests that covered both CFI
and KCFI using revisions are split into separate tests.

It also adds tests for the types, language constructs, and features
that were not covered (i.e., intrinsics, thread locals, builtin Clone
and FnPtr implementations, pattern types, the never type, extern types,
C variadics, and the generalize-pointers and normalize-integers
options), removes the tests that became redundant, and changes the drop
tests to run-pass, as they now pass at run time.

Tests for flags/options and the regression tests are unchanged (moved
and renamed in the previous commit).
@rcvalle

rcvalle commented Aug 7, 2026

Copy link
Copy Markdown
Member Author

The first commit does both mechanical refactorings and non-refactoring changes together. Would you mind splitting the variable renamings and outlining into a separate commit to make it easier to review? For the second commit it might also be nice to move file renamings (+ minor adjustments) into a commit separate from new tests and wholesale replacements of existing tests.

Done. For whenever you have time. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-control-flow-integrity Area: Control Flow Integrity (CFI) security mitigation A-tidy Area: The tidy tool PG-exploit-mitigations Project group: Exploit mitigations S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) 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.

3 participants