sanitizers: Create the rustc_sanitizers crate by rcvalle · Pull Request #123620...
source link: https://github.com/rust-lang/rust/pull/123620
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
Conversation
Member
Create the rustc_sanitizers crate and move the source code for the CFI and KCFI sanitizers to it. The tracking issue for reviewing and moving sanitizers into a compiler crate is #123619. This is part of our work to organize and stabilize support for the sanitizers. (See our roadmap at https://hackmd.io/@rcvalle/S1Ou9K6H6.)
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
Collaborator
|
These commits modify the If this was unintentional then you should revert the changes before this PR is merged. |
Member
Author
|
r? @davidtwco |
LGTM, lots of small comments, but this is otherwise a clear improvement
compiler/rustc_sanitizers/src/lib.rs
Outdated Show resolved
compiler/rustc_sanitizers/src/kcfi/typeid/mod.rs
Outdated Show resolved
compiler/rustc_sanitizers/src/kcfi/mod.rs
Outdated Show resolved
/// This crate contains the source code for providing support for the sanitizers to the Rust |
||
/// compiler. |
||
pub mod cfi; |
||
pub mod kcfi; |
nit: we could probably simplify the file structure a little bit here, it's mostly an aesthetic concern so only if you agree:
rustc_sanitizers
|- lib.rs
|- cfi.rs (containing what is in cfi/typeid/mod.rs)
|- kcfi.rs (containing what is in kcfi/typeid/mod.rs)
\- typeid/* (private mod, containing what is in cfi/typeid/itanium_cxx_abi now)
It's also possible that this doesn't make sense anticipating some of the other things you want to move to this crate that exists elsewhere at the moment, but if we're only anticipating things that we might do by having this structure (e.g. manglings other than itanium), then we can change to this structure when we're adding that.
Member
Author
Yes, I thought about it too. The reason I ended up doing it this way was because I'm anticipating two things:
- Move as much as possible of CFI and KCFI related source code to cfi/mod.rs and kcfi/mod.rs (e.g., source code at declare.rs and builder.rs as cfi::emit_type_test and kcfi::emit_kcfi_bundle).
- Add possibly at least an additional encoding for Microsoft XFG and cross-language Microsoft XFG (but we might want to discuss if the rustc_sanitizers crate is the right place for it), or a more coarse-grained encoding for cross-language CFI compatibility with other languages.
What do you think?
compiler/rustc_sanitizers/src/cfi/typeid/mod.rs
Outdated Show resolved
compiler/rustc_sanitizers/src/cfi/typeid/itanium_cxx_abi/transform.rs
Outdated Show resolved
compiler/rustc_sanitizers/src/cfi/typeid/itanium_cxx_abi/mod.rs
Outdated Show resolved
compiler/rustc_sanitizers/src/cfi/mod.rs
Outdated Show resolved
compiler/rustc_sanitizers/src/cfi/typeid/itanium_cxx_abi/encode.rs
Outdated Show resolved
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
Member
Author
|
@rustbot ready |
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
Member
|
@bors r+ |
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
Successfully merging this pull request may close these issues.
None yet
Recommend
About Joyk
Aggregate valuable and interesting links.
Joyk means Joy of geeK