Add support for -Zsanitizer-cfi-minimal-runtime - #162493
Conversation
rustbot
added
A-LLVM
|
r? @folkertdev rustbot has assigned @folkertdev. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
r? rcvalle |
|
Would you mind adding tests it similarly to https://github.com/rust-lang/rust/blob/main/tests/ui/sanitizer/cfi/generalize-pointers-requires-cfi.rs and https://github.com/rust-lang/rust/blob/main/tests/ui/sanitizer/cfi/normalize-integers-requires-cfi.rs (i.e., requires CFI, not just CFI diagnostics or CFI recovery)? |
I'm not entirely sure what the best behavior is here. Now that we have tests that cfi-diag and cfi-recover require cfi, this should already be enough. Otherwise we would need to add a separate error message for all the different cases (you need a specific error message for |
b2af13f to
5ee5ebd
Compare
This comment has been minimized.
This comment has been minimized.
Failing tests I need to look at / fix before another round of reviews. |
|
Would you mind adding tests it similarly to https://github.com/rust-lang/rust/blob/main/tests/ui/sanitizer/cfi/generalize-pointers-requires-cfi.rs and https://github.com/rust-lang/rust/blob/main/tests/ui/sanitizer/cfi/normalize-integers-requires-cfi.rs (i.e., requires CFI, not just CFI diagnostics or CFI recovery)?
Yes, I think this is how the Rust compiler does it and historically this is how we've been doing it. This also aligns with the philosophy of having clear and helpful error messages. In this case, with only one incorrect run, the user would be able to see both requirements without requiring one additional run. |
5ee5ebd to
b47e873
Compare
Makes sense, should be added now.
These should also be addressed now. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
For production use, we should only link in the ubsan_minimal runtime, instead of the complete ubsan runtime. This adds support for both cfi-recover and cfi-diag to use the minimal runtime when `-Zsanitizer-cfi-minimal-runtime` is specified. This also includes tests, to ensure the flag can only be used if either cfi-recover or cfi-diag is enabled, it doesn't disrupt the original behavior, and links in the correct runtime when specified. Co-Authored-By: Bastian Kersting <bkersting@google.com>
Co-Authored-By: Bastian Kersting <bkersting@google.com>
…uwer Rollup of 7 pull requests Successful merges: - #162493 (Add support for -Zsanitizer-cfi-minimal-runtime) - #163133 ([rustdoc] Fix invalid jump to def link when `#[rustc_allow_incoherent_impl]` is involved) - #163215 (Fix suggestion for Option to bool with proper precedence handling) - #163266 (More deferred liveness cleanups) - #163274 (Support -Z merge-functions with gcc and add stack-protector asm tests) - #163312 (Add rustdoc regression test for glob import of a crate that re-exports) - #163332 (Add some docs to `Global`)
Given two rollup failures, consider running a try job before further approval. |
rust-bors
Bot
added
S-waiting-on-author
|
This pull request was unapproved. This PR was contained in a rollup (#163358), which was unapproved. |
|
@bors try |
|
@jakos-sec: 🔑 Insufficient privileges: not in try users |
|
@rcvalle I don't seem to have try privileges, could you start one? :/ |
|
@bors try |
This comment has been minimized.
This comment has been minimized.
Add support for -Zsanitizer-cfi-minimal-runtime
|
@bors r+ |
rust-bors
Bot
added
S-waiting-on-bors
…=rcvalle Add support for -Zsanitizer-cfi-minimal-runtime For production use, we should only link in the ubsan_minimal runtime, instead of the complete ubsan runtime. This adds support for both cfi-recover and cfi-diag to use the minimal runtime when `-Zsanitizer-cfi-minimal-runtime` is specified. This also includes tests, to ensure the flag can only be used if either cfi-recover or cfi-diag is enabled, it doesn't disrupt the original behavior, and links in the correct runtime when specified. cc @1c3t3a ?r rcvalle
…uwer Rollup of 15 pull requests Successful merges: - #158936 (Add `std::fs::{Home|Media}Dirs`) - #129036 (Additional NonZero conversions) - #158997 (Avoid recording unnameable `extern crate` aliases in diagnostic metadata) - #161015 (Stabilize `funnel_shifts` (including `const`)) - #161712 (Stabilize `Result::into_{ok,err}`) - #162493 (Add support for -Zsanitizer-cfi-minimal-runtime) - #162655 (next solver: prefer to select impl candidates over global where-clause candidates) - #162862 (Fix intra doc link resolution when a doc comment is composed of both inner and outer doc comment) - #163200 (make `RustaceansAreAwesome` satisfy trait bounds) - #163331 (Move `Arc` and `Rc` into `rcs` mod) - #163427 (implement #![feature(gca_adts)]) - #163428 (do not complain about unstable target features on nightly) - #163444 (Add `stable_rustc` helper in `run-make-support`) - #163447 (Allow using different index types when reading and writing to tables) - #163450 (Force the correct type variable to never for method resolution on an adjusted never type)
…=rcvalle Add support for -Zsanitizer-cfi-minimal-runtime For production use, we should only link in the ubsan_minimal runtime, instead of the complete ubsan runtime. This adds support for both cfi-recover and cfi-diag to use the minimal runtime when `-Zsanitizer-cfi-minimal-runtime` is specified. This also includes tests, to ensure the flag can only be used if either cfi-recover or cfi-diag is enabled, it doesn't disrupt the original behavior, and links in the correct runtime when specified. cc @1c3t3a ?r rcvalle
Rollup of 9 pull requests Successful merges: - #161015 (Stabilize `funnel_shifts` (including `const`)) - #161712 (Stabilize `Result::into_{ok,err}`) - #162493 (Add support for -Zsanitizer-cfi-minimal-runtime) - #163427 (implement #![feature(gca_adts)]) - #163390 (add `automatically_derived` attribute documentation) - #163428 (do not complain about unstable target features on nightly) - #163444 (Add `stable_rustc` helper in `run-make-support`) - #163447 (Allow using different index types when reading and writing to tables) - #163459 (Stabilize vec_try_remove)
Rollup of 9 pull requests Successful merges: - rust-lang/rust#161015 (Stabilize `funnel_shifts` (including `const`)) - rust-lang/rust#161712 (Stabilize `Result::into_{ok,err}`) - rust-lang/rust#162493 (Add support for -Zsanitizer-cfi-minimal-runtime) - rust-lang/rust#163427 (implement #![feature(gca_adts)]) - rust-lang/rust#163390 (add `automatically_derived` attribute documentation) - rust-lang/rust#163428 (do not complain about unstable target features on nightly) - rust-lang/rust#163444 (Add `stable_rustc` helper in `run-make-support`) - rust-lang/rust#163447 (Allow using different index types when reading and writing to tables) - rust-lang/rust#163459 (Stabilize vec_try_remove)
Rollup of 9 pull requests Successful merges: - rust-lang/rust#161015 (Stabilize `funnel_shifts` (including `const`)) - rust-lang/rust#161712 (Stabilize `Result::into_{ok,err}`) - rust-lang/rust#162493 (Add support for -Zsanitizer-cfi-minimal-runtime) - rust-lang/rust#163427 (implement #![feature(gca_adts)]) - rust-lang/rust#163390 (add `automatically_derived` attribute documentation) - rust-lang/rust#163428 (do not complain about unstable target features on nightly) - rust-lang/rust#163444 (Add `stable_rustc` helper in `run-make-support`) - rust-lang/rust#163447 (Allow using different index types when reading and writing to tables) - rust-lang/rust#163459 (Stabilize vec_try_remove)
Rollup of 9 pull requests Successful merges: - rust-lang/rust#161015 (Stabilize `funnel_shifts` (including `const`)) - rust-lang/rust#161712 (Stabilize `Result::into_{ok,err}`) - rust-lang/rust#162493 (Add support for -Zsanitizer-cfi-minimal-runtime) - rust-lang/rust#163427 (implement #![feature(gca_adts)]) - rust-lang/rust#163390 (add `automatically_derived` attribute documentation) - rust-lang/rust#163428 (do not complain about unstable target features on nightly) - rust-lang/rust#163444 (Add `stable_rustc` helper in `run-make-support`) - rust-lang/rust#163447 (Allow using different index types when reading and writing to tables) - rust-lang/rust#163459 (Stabilize vec_try_remove)
7 participants
View all comments
For production use, we should only link in the ubsan_minimal runtime,
instead of the complete ubsan runtime. This adds support for both
cfi-recover and cfi-diag to use the minimal runtime when
-Zsanitizer-cfi-minimal-runtimeis specified.This also includes tests, to ensure the flag can only be used if either
cfi-recover or cfi-diag is enabled, it doesn't disrupt the original
behavior, and links in the correct runtime when specified.
cc @1c3t3a
?r rcvalle