Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

CFI: Support complex receivers #123005

Merged
merged 1 commit into from
Mar 25, 2024
Merged

CFI: Support complex receivers #123005

merged 1 commit into from
Mar 25, 2024

Conversation

maurer
Copy link
Contributor

@maurer maurer commented Mar 24, 2024

Right now, we only support rewriting &self and &mut self into &dyn MyTrait and &mut dyn MyTrait. This expands it to handle the full gamut of receivers by calculating the receiver based on substitution rather than based on a rewrite. This means that, for example, Arc<Self> will become Arc<dyn MyTrait> appropriately with this change.

This approach also allows us to support associated type constraints as well, so we will correctly rewrite &self into &dyn MyTrait<T=i32>, for example.

r? @workingjubilee

@rustbot rustbot added PG-exploit-mitigations Project group: Exploit mitigations S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 24, 2024
@rustbot
Copy link
Collaborator

rustbot commented Mar 24, 2024

Some changes occurred in tests/ui/sanitizer

cc @rust-lang/project-exploit-mitigations, @rcvalle

Some changes occurred in compiler/rustc_symbol_mangling/src/typeid

cc @rust-lang/project-exploit-mitigations, @rcvalle

@compiler-errors
Copy link
Member

r? compiler-errors

@workingjubilee
Copy link
Member

yeah my ability to say much pretty much dies above cg_ssa.

@rustbot
Copy link
Collaborator

rustbot commented Mar 24, 2024

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

@maurer
Copy link
Contributor Author

maurer commented Mar 24, 2024

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged. Otherwise, you can ignore this comment.

This happened because moving the query into the mangler resulted in a new dep on rustc_infer. If there's another way I should do this, let me know and I'll adjust.

@compiler-errors
Copy link
Member

I'll take another look in a few hours, since I'm on a run. The approach makes sense, but I'll probably leave some suggestions for comments for why it's correct.

compiler-errors
compiler-errors requested changes Mar 24, 2024
View reviewed changes
Copy link
Member

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

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

Few tweaks

compiler/rustc_symbol_mangling/src/typeid/typeid_itanium_cxx_abi.rs Outdated Show resolved Hide resolved
compiler/rustc_symbol_mangling/src/typeid/typeid_itanium_cxx_abi.rs Outdated Show resolved Hide resolved
compiler/rustc_symbol_mangling/src/typeid/typeid_itanium_cxx_abi.rs Outdated Show resolved Hide resolved
@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 24, 2024
@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 Mar 24, 2024
@maurer
CFI: Support arbitrary receivers
40f41e7 
Previously, we only rewrote `&self` and `&mut self` receivers. By
instantiating the method from the trait definition, we can make this
work work with arbitrary legal receivers instead.
@compiler-errors
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Mar 24, 2024

📌 Commit 40f41e7 has been approved by compiler-errors

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 24, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 25, 2024
@matthiaskrgr
Rollup merge of rust-lang#123005 - maurer:cfi-arbitrary-receivers, r=…
4ccb5a7 
…compiler-errors

CFI: Support complex receivers

Right now, we only support rewriting `&self` and `&mut self` into `&dyn MyTrait` and `&mut dyn MyTrait`. This expands it to handle the full gamut of receivers by calculating the receiver based on *substitution* rather than based on a rewrite. This means that, for example, `Arc<Self>` will become `Arc<dyn MyTrait>` appropriately with this change.

This approach also allows us to support associated type constraints as well, so we will correctly rewrite `&self` into `&dyn MyTrait<T=i32>`, for example.

r? `@workingjubilee`
@matthiaskrgr matthiaskrgr mentioned this pull request Mar 25, 2024
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 25, 2024
@bors
Auto merge of rust-lang#123023 - matthiaskrgr:rollup-xtfciuv, r=matth…
33c0005 
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#122802 (Provide structured suggestion for unconstrained generic constant)
 - rust-lang#122858 (Tweak `parse_dot_suffix_expr`)
 - rust-lang#122923 (In `pretty_print_type()`, print `async fn` futures' paths instead of spans.)
 - rust-lang#122990 (Clarify transmute example)
 - rust-lang#122995 (Clean up unnecessary headers/flags in coverage mir-opt tests)
 - rust-lang#123003 (CFI: Handle dyn with no principal)
 - rust-lang#123005 (CFI: Support complex receivers)
 - rust-lang#123020 (Temporarily remove nnethercote from the review rotation.)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 25, 2024
@matthiaskrgr
Rollup merge of rust-lang#123005 - maurer:cfi-arbitrary-receivers, r=…
870fae9 
…compiler-errors

CFI: Support complex receivers

Right now, we only support rewriting `&self` and `&mut self` into `&dyn MyTrait` and `&mut dyn MyTrait`. This expands it to handle the full gamut of receivers by calculating the receiver based on *substitution* rather than based on a rewrite. This means that, for example, `Arc<Self>` will become `Arc<dyn MyTrait>` appropriately with this change.

This approach also allows us to support associated type constraints as well, so we will correctly rewrite `&self` into `&dyn MyTrait<T=i32>`, for example.

r? ``@workingjubilee``
@matthiaskrgr matthiaskrgr mentioned this pull request Mar 25, 2024
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 25, 2024
@bors
Auto merge of rust-lang#123027 - matthiaskrgr:rollup-nng7h3z, r=matth…
282915c 
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#120557 (Add rust-lldb pretty printing for Path and PathBuf)
 - rust-lang#122802 (Provide structured suggestion for unconstrained generic constant)
 - rust-lang#122858 (Tweak `parse_dot_suffix_expr`)
 - rust-lang#122990 (Clarify transmute example)
 - rust-lang#122995 (Clean up unnecessary headers/flags in coverage mir-opt tests)
 - rust-lang#123003 (CFI: Handle dyn with no principal)
 - rust-lang#123005 (CFI: Support complex receivers)
 - rust-lang#123020 (Temporarily remove nnethercote from the review rotation.)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 25, 2024
@bors
Auto merge of rust-lang#123027 - matthiaskrgr:rollup-nng7h3z, r=matth…
0ce30e7 
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#120557 (Add rust-lldb pretty printing for Path and PathBuf)
 - rust-lang#122802 (Provide structured suggestion for unconstrained generic constant)
 - rust-lang#122858 (Tweak `parse_dot_suffix_expr`)
 - rust-lang#122990 (Clarify transmute example)
 - rust-lang#122995 (Clean up unnecessary headers/flags in coverage mir-opt tests)
 - rust-lang#123003 (CFI: Handle dyn with no principal)
 - rust-lang#123005 (CFI: Support complex receivers)
 - rust-lang#123020 (Temporarily remove nnethercote from the review rotation.)

r? `@ghost`
`@rustbot` modify labels: rollup
@matthiaskrgr matthiaskrgr mentioned this pull request Mar 25, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 25, 2024
@bors
Auto merge of rust-lang#123029 - matthiaskrgr:rollup-6qsevhx, r=matth…
af98101 
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#122858 (Tweak `parse_dot_suffix_expr`)
 - rust-lang#122982 (Add more comments to the bootstrap code that handles `tests/coverage`)
 - rust-lang#122990 (Clarify transmute example)
 - rust-lang#122995 (Clean up unnecessary headers/flags in coverage mir-opt tests)
 - rust-lang#123003 (CFI: Handle dyn with no principal)
 - rust-lang#123005 (CFI: Support complex receivers)
 - rust-lang#123020 (Temporarily remove nnethercote from the review rotation.)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit fe0222b into rust-lang:master Mar 25, 2024
11 checks passed
@rustbot rustbot added this to the 1.79.0 milestone Mar 25, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 25, 2024
@rust-timer
Unrolled build for rust-lang#123005
a1d28bd 
Rollup merge of rust-lang#123005 - maurer:cfi-arbitrary-receivers, r=compiler-errors

CFI: Support complex receivers

Right now, we only support rewriting `&self` and `&mut self` into `&dyn MyTrait` and `&mut dyn MyTrait`. This expands it to handle the full gamut of receivers by calculating the receiver based on *substitution* rather than based on a rewrite. This means that, for example, `Arc<Self>` will become `Arc<dyn MyTrait>` appropriately with this change.

This approach also allows us to support associated type constraints as well, so we will correctly rewrite `&self` into `&dyn MyTrait<T=i32>`, for example.

r? ```@workingjubilee```
@matthiaskrgr matthiaskrgr mentioned this pull request Mar 25, 2024
Closed
@compiler-errors compiler-errors mentioned this pull request Jul 17, 2024
Merged
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 29, 2024
@matthiaskrgr
Rollup merge of rust-lang#127882 - compiler-errors:cfi-sized-self-gat…
2e9d962 
…, r=oli-obk

Don't elaborate associated types with Sized bounds in `trait_object_ty` in cfi

The elaboration mechanism introduced in rust-lang#123005 didn't filter for associated types with `Self: Sized` bounds, which since rust-lang#112319 has excluded them from the object type.

Fixes rust-lang#127881
cc `@maurer` `@rcvalle`
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jul 29, 2024
@rust-timer
Unrolled build for rust-lang#127882
ba1aad4 
Rollup merge of rust-lang#127882 - compiler-errors:cfi-sized-self-gat, r=oli-obk

Don't elaborate associated types with Sized bounds in `trait_object_ty` in cfi

The elaboration mechanism introduced in rust-lang#123005 didn't filter for associated types with `Self: Sized` bounds, which since rust-lang#112319 has excluded them from the object type.

Fixes rust-lang#127881
cc `@maurer` `@rcvalle`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@compiler-errors compiler-errors compiler-errors approved these changes

Assignees

@compiler-errors compiler-errors

Labels
PG-exploit-mitigations Project group: Exploit mitigations S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.79.0
Development

Successfully merging this pull request may close these issues.
5 participants
@maurer @rustbot @compiler-errors @workingjubilee @bors