sanitizers: Implicitly disable mutually exclusive sanitizers - #161953
Conversation
This attempts to match clang's behavior of implicitly disabling sanitizers that are incompatible. Specifically, if a set of default sanitizers would be incompatible with ones provided by -Zsanitize=..., then clang (and now rust) will opt for keeping the ones specified via flags over the ones used as platform defaults. This helps maintain build consistency where we can just enable sanitizers via flags for both rust and c++ code without needing to manually disable others. The driving reason for this is asan and safestack where we'd like to enable safestack by default for x86_64 fuchsia but disable it if -Zsanitize=address is passed (matching clang's behavior). This commit also refactors all uses of `self.opts.unstable_opts.sanitizer` to go through the updated `sanitizer()` method. AI: Gemini was used to help review the code and write some tests, but it did not generate the whole patch. I edited and reviewed this PR to the best of my ability before pushing for review.
|
r? @fee1-dead rustbot has assigned @fee1-dead. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
cc @ilovepi just in case I'm missing something here |
|
@rustbot reroll |
|
@bors r+ rollup |
…ble-sanitizers, r=davidtwco sanitizers: Implicitly disable mutually exclusive sanitizers This attempts to match clang's behavior of implicitly disabling sanitizers that are incompatible. Specifically, if a set of default sanitizers would be incompatible with ones provided by -Zsanitize=..., then clang (and now rust) will opt for keeping the ones specified via flags over the ones used as platform defaults. This helps maintain build consistency where we can just enable sanitizers via flags for both rust and c++ code without needing to manually disable others. The driving reason for this is asan and safestack where we'd like to enable safestack by default for x86_64 fuchsia but disable it if -Zsanitize=address is passed (matching clang's behavior). This commit also refactors all uses of `self.opts.unstable_opts.sanitizer` to go through the updated `sanitizer()` method. AI: Gemini was used to help review the code and write some tests, but it did not generate the whole patch. I edited and reviewed this PR to the best of my ability before pushing for review.
…ble-sanitizers, r=davidtwco sanitizers: Implicitly disable mutually exclusive sanitizers This attempts to match clang's behavior of implicitly disabling sanitizers that are incompatible. Specifically, if a set of default sanitizers would be incompatible with ones provided by -Zsanitize=..., then clang (and now rust) will opt for keeping the ones specified via flags over the ones used as platform defaults. This helps maintain build consistency where we can just enable sanitizers via flags for both rust and c++ code without needing to manually disable others. The driving reason for this is asan and safestack where we'd like to enable safestack by default for x86_64 fuchsia but disable it if -Zsanitize=address is passed (matching clang's behavior). This commit also refactors all uses of `self.opts.unstable_opts.sanitizer` to go through the updated `sanitizer()` method. AI: Gemini was used to help review the code and write some tests, but it did not generate the whole patch. I edited and reviewed this PR to the best of my ability before pushing for review.
Rollup of 25 pull requests Successful merges: - #159074 ([PAC] FnAbi, llvm.ptrauth.resign and Session API change (2/8)) - #159792 (A more readable debug map for IndexMaps) - #161895 (std::sys::pal::sgx: fix mismatched alloc/free alignment) - #161900 (bootstrap: Include feature-gated items in bootstrap tool docs) - #161940 (Promote `wasm32-wasip3` to a tier 2 target) - #162072 (Add new Tier-3 target: `powerpc64-sony-ps3`) - #162179 (type system const items via direct rhs) - #162277 (Introduce `rustc_middle::middel::resolve`) - #162285 (box: fixup map/try_map deallocate calls) - #162286 (string: don't unwind prematurely) - #162289 (alloc: a bunch of safety comments) - #162292 (Update `askama` version to `0.16.1`) - #160509 (Remove `RegionExt`; move methods to `Region` in `rustc_type_ir`) - #160906 (Suggest usize instead of placeholder type for array length constants) - #160936 (traits: Represent live alias arguments as bitsets) - #161400 (Improve diagnostics for references to closures) - #161656 (Suggest mutable references for FnMut closure arguments) - #161711 (Add more splat fn type tests) - #161786 (Make `tcx.def_id_partial_cmp` public) - #161953 (sanitizers: Implicitly disable mutually exclusive sanitizers) - #162155 (add suggestion for `rustc_allowed_through_unstable_modules` attribute) - #162212 (Implement `Rng` for `Box`) - #162246 (Fix incorrect meta span) - #162266 (std: fix typo) - #162291 (Add regression test from 1.98.1)
…ble-sanitizers, r=davidtwco sanitizers: Implicitly disable mutually exclusive sanitizers This attempts to match clang's behavior of implicitly disabling sanitizers that are incompatible. Specifically, if a set of default sanitizers would be incompatible with ones provided by -Zsanitize=..., then clang (and now rust) will opt for keeping the ones specified via flags over the ones used as platform defaults. This helps maintain build consistency where we can just enable sanitizers via flags for both rust and c++ code without needing to manually disable others. The driving reason for this is asan and safestack where we'd like to enable safestack by default for x86_64 fuchsia but disable it if -Zsanitize=address is passed (matching clang's behavior). This commit also refactors all uses of `self.opts.unstable_opts.sanitizer` to go through the updated `sanitizer()` method. AI: Gemini was used to help review the code and write some tests, but it did not generate the whole patch. I edited and reviewed this PR to the best of my ability before pushing for review.
Rollup of 25 pull requests Successful merges: - #159074 ([PAC] FnAbi, llvm.ptrauth.resign and Session API change (2/8)) - #159792 (A more readable debug map for IndexMaps) - #160745 (make closures act like MaybeDangling) - #161895 (std::sys::pal::sgx: fix mismatched alloc/free alignment) - #161940 (Promote `wasm32-wasip3` to a tier 2 target) - #162072 (Add new Tier-3 target: `powerpc64-sony-ps3`) - #162179 (type system const items via direct rhs) - #162277 (Introduce `rustc_middle::middel::resolve`) - #162285 (box: fixup map/try_map deallocate calls) - #162286 (string: don't unwind prematurely) - #162289 (alloc: a bunch of safety comments) - #162292 (Update `askama` version to `0.16.1`) - #160509 (Remove `RegionExt`; move methods to `Region` in `rustc_type_ir`) - #160906 (Suggest usize instead of placeholder type for array length constants) - #160936 (traits: Represent live alias arguments as bitsets) - #161400 (Improve diagnostics for references to closures) - #161656 (Suggest mutable references for FnMut closure arguments) - #161711 (Add more splat fn type tests) - #161786 (Make `tcx.def_id_partial_cmp` public) - #161953 (sanitizers: Implicitly disable mutually exclusive sanitizers) - #162155 (add suggestion for `rustc_allowed_through_unstable_modules` attribute) - #162212 (Implement `Rng` for `Box`) - #162246 (Fix incorrect meta span) - #162266 (std: fix typo) - #162291 (Add regression test from 1.98.1)
…ble-sanitizers, r=davidtwco sanitizers: Implicitly disable mutually exclusive sanitizers This attempts to match clang's behavior of implicitly disabling sanitizers that are incompatible. Specifically, if a set of default sanitizers would be incompatible with ones provided by -Zsanitize=..., then clang (and now rust) will opt for keeping the ones specified via flags over the ones used as platform defaults. This helps maintain build consistency where we can just enable sanitizers via flags for both rust and c++ code without needing to manually disable others. The driving reason for this is asan and safestack where we'd like to enable safestack by default for x86_64 fuchsia but disable it if -Zsanitize=address is passed (matching clang's behavior). This commit also refactors all uses of `self.opts.unstable_opts.sanitizer` to go through the updated `sanitizer()` method. AI: Gemini was used to help review the code and write some tests, but it did not generate the whole patch. I edited and reviewed this PR to the best of my ability before pushing for review.
…ble-sanitizers, r=davidtwco sanitizers: Implicitly disable mutually exclusive sanitizers This attempts to match clang's behavior of implicitly disabling sanitizers that are incompatible. Specifically, if a set of default sanitizers would be incompatible with ones provided by -Zsanitize=..., then clang (and now rust) will opt for keeping the ones specified via flags over the ones used as platform defaults. This helps maintain build consistency where we can just enable sanitizers via flags for both rust and c++ code without needing to manually disable others. The driving reason for this is asan and safestack where we'd like to enable safestack by default for x86_64 fuchsia but disable it if -Zsanitize=address is passed (matching clang's behavior). This commit also refactors all uses of `self.opts.unstable_opts.sanitizer` to go through the updated `sanitizer()` method. AI: Gemini was used to help review the code and write some tests, but it did not generate the whole patch. I edited and reviewed this PR to the best of my ability before pushing for review.
Rollup of 27 pull requests Successful merges: - #159074 ([PAC] FnAbi, llvm.ptrauth.resign and Session API change (2/8)) - #159792 (A more readable debug map for IndexMaps) - #160745 (make closures act like MaybeDangling) - #161940 (Promote `wasm32-wasip3` to a tier 2 target) - #162030 (Prevent `--test` to be used in `rustdoc-html` testsuite) - #162072 (Add new Tier-3 target: `powerpc64-sony-ps3`) - #162179 (type system const items via direct rhs) - #162262 (Avoid manually instantiating some binders in error reporting with `-Znext-solver`) - #162277 (Introduce `rustc_middle::middel::resolve`) - #162285 (box: fixup map/try_map deallocate calls) - #162286 (string: don't unwind prematurely) - #162289 (alloc: a bunch of safety comments) - #162290 (abby test DSL: AliasTyOutlivesViaEnv) - #162292 (Update `askama` version to `0.16.1`) - #160509 (Remove `RegionExt`; move methods to `Region` in `rustc_type_ir`) - #160906 (Suggest usize instead of placeholder type for array length constants) - #160936 (traits: Represent live alias arguments as bitsets) - #161400 (Improve diagnostics for references to closures) - #161656 (Suggest mutable references for FnMut closure arguments) - #161711 (Add more splat fn type tests) - #161786 (Make `tcx.def_id_partial_cmp` public) - #161953 (sanitizers: Implicitly disable mutually exclusive sanitizers) - #162155 (add suggestion for `rustc_allowed_through_unstable_modules` attribute) - #162212 (Implement `Rng` for `Box`) - #162246 (Fix incorrect meta span) - #162266 (std: fix typo) - #162291 (Add regression test from 1.98.1)
…uwer Rollup of 25 pull requests Successful merges: - #162072 (Add new Tier-3 target: `powerpc64-sony-ps3`) - #162179 (type system const items via direct rhs) - #162255 (let people change rustc-dev-guide in-tree) - #162262 (Avoid manually instantiating some binders in error reporting with `-Znext-solver`) - #162277 (Introduce `rustc_middle::middel::resolve`) - #162285 (box: fixup map/try_map deallocate calls) - #162286 (string: don't unwind prematurely) - #162289 (alloc: a bunch of safety comments) - #162290 (abby test DSL: AliasTyOutlivesViaEnv) - #162292 (Update `askama` version to `0.16.1`) - #160509 (Remove `RegionExt`; move methods to `Region` in `rustc_type_ir`) - #160906 (Suggest usize instead of placeholder type for array length constants) - #160936 (traits: Represent live alias arguments as bitsets) - #161394 (fix `is_homogeneous_aggregate`: use unit size, not total size ) - #161400 (Improve diagnostics for references to closures) - #161656 (Suggest mutable references for FnMut closure arguments) - #161711 (Add more splat fn type tests) - #161786 (Make `tcx.def_id_partial_cmp` public) - #161953 (sanitizers: Implicitly disable mutually exclusive sanitizers) - #162155 (add suggestion for `rustc_allowed_through_unstable_modules` attribute) - #162212 (Implement `Rng` for `Box`) - #162246 (Fix incorrect meta span) - #162266 (std: fix typo) - #162291 (Add regression test from 1.98.1) - #162303 (Fix a minor spelling error in `library/std/src/sys/fs/unix.rs`)
…ble-sanitizers, r=davidtwco sanitizers: Implicitly disable mutually exclusive sanitizers This attempts to match clang's behavior of implicitly disabling sanitizers that are incompatible. Specifically, if a set of default sanitizers would be incompatible with ones provided by -Zsanitize=..., then clang (and now rust) will opt for keeping the ones specified via flags over the ones used as platform defaults. This helps maintain build consistency where we can just enable sanitizers via flags for both rust and c++ code without needing to manually disable others. The driving reason for this is asan and safestack where we'd like to enable safestack by default for x86_64 fuchsia but disable it if -Zsanitize=address is passed (matching clang's behavior). This commit also refactors all uses of `self.opts.unstable_opts.sanitizer` to go through the updated `sanitizer()` method. AI: Gemini was used to help review the code and write some tests, but it did not generate the whole patch. I edited and reviewed this PR to the best of my ability before pushing for review.
…uwer Rollup of 25 pull requests Successful merges: - #162072 (Add new Tier-3 target: `powerpc64-sony-ps3`) - #162179 (type system const items via direct rhs) - #162255 (let people change rustc-dev-guide in-tree) - #162262 (Avoid manually instantiating some binders in error reporting with `-Znext-solver`) - #162277 (Introduce `rustc_middle::middel::resolve`) - #162285 (box: fixup map/try_map deallocate calls) - #162286 (string: don't unwind prematurely) - #162289 (alloc: a bunch of safety comments) - #162290 (abby test DSL: AliasTyOutlivesViaEnv) - #162292 (Update `askama` version to `0.16.1`) - #160509 (Remove `RegionExt`; move methods to `Region` in `rustc_type_ir`) - #160906 (Suggest usize instead of placeholder type for array length constants) - #160936 (traits: Represent live alias arguments as bitsets) - #161394 (fix `is_homogeneous_aggregate`: use unit size, not total size ) - #161400 (Improve diagnostics for references to closures) - #161656 (Suggest mutable references for FnMut closure arguments) - #161711 (Add more splat fn type tests) - #161786 (Make `tcx.def_id_partial_cmp` public) - #161953 (sanitizers: Implicitly disable mutually exclusive sanitizers) - #162155 (add suggestion for `rustc_allowed_through_unstable_modules` attribute) - #162212 (Implement `Rng` for `Box`) - #162246 (Fix incorrect meta span) - #162266 (std: fix typo) - #162291 (Add regression test from 1.98.1) - #162303 (Fix a minor spelling error in `library/std/src/sys/fs/unix.rs`)
Rollup merge of #161953 - PiJoules:implicit-remove-incompatible-sanitizers, r=davidtwco sanitizers: Implicitly disable mutually exclusive sanitizers This attempts to match clang's behavior of implicitly disabling sanitizers that are incompatible. Specifically, if a set of default sanitizers would be incompatible with ones provided by -Zsanitize=..., then clang (and now rust) will opt for keeping the ones specified via flags over the ones used as platform defaults. This helps maintain build consistency where we can just enable sanitizers via flags for both rust and c++ code without needing to manually disable others. The driving reason for this is asan and safestack where we'd like to enable safestack by default for x86_64 fuchsia but disable it if -Zsanitize=address is passed (matching clang's behavior). This commit also refactors all uses of `self.opts.unstable_opts.sanitizer` to go through the updated `sanitizer()` method. AI: Gemini was used to help review the code and write some tests, but it did not generate the whole patch. I edited and reviewed this PR to the best of my ability before pushing for review.
|
Note This PR was benchmarked as part of triage of its containing rollup: triage URL. Finished benchmarking commit (18b0b44): comparison URL. Overall result: ❌ regressions - please read:Our benchmarks found a performance regression caused by this PR. Next Steps:
@rustbot label: +perf-regression Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary -7.3%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary 3.9%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis perf run didn't have relevant results for this metric. Bootstrap: missing data |
|
This caused a performance regression, which is quite surprising to me. |
|
The |
Cache sanitizer set in `Session` Trying to address the perf. regression from #161953.
Cache sanitizer set in `Session` Trying to address the perf. regression from rust-lang/rust#161953.
…uwer Rollup of 25 pull requests Successful merges: - rust-lang/rust#162072 (Add new Tier-3 target: `powerpc64-sony-ps3`) - rust-lang/rust#162179 (type system const items via direct rhs) - rust-lang/rust#162255 (let people change rustc-dev-guide in-tree) - rust-lang/rust#162262 (Avoid manually instantiating some binders in error reporting with `-Znext-solver`) - rust-lang/rust#162277 (Introduce `rustc_middle::middel::resolve`) - rust-lang/rust#162285 (box: fixup map/try_map deallocate calls) - rust-lang/rust#162286 (string: don't unwind prematurely) - rust-lang/rust#162289 (alloc: a bunch of safety comments) - rust-lang/rust#162290 (abby test DSL: AliasTyOutlivesViaEnv) - rust-lang/rust#162292 (Update `askama` version to `0.16.1`) - rust-lang/rust#160509 (Remove `RegionExt`; move methods to `Region` in `rustc_type_ir`) - rust-lang/rust#160906 (Suggest usize instead of placeholder type for array length constants) - rust-lang/rust#160936 (traits: Represent live alias arguments as bitsets) - rust-lang/rust#161394 (fix `is_homogeneous_aggregate`: use unit size, not total size ) - rust-lang/rust#161400 (Improve diagnostics for references to closures) - rust-lang/rust#161656 (Suggest mutable references for FnMut closure arguments) - rust-lang/rust#161711 (Add more splat fn type tests) - rust-lang/rust#161786 (Make `tcx.def_id_partial_cmp` public) - rust-lang/rust#161953 (sanitizers: Implicitly disable mutually exclusive sanitizers) - rust-lang/rust#162155 (add suggestion for `rustc_allowed_through_unstable_modules` attribute) - rust-lang/rust#162212 (Implement `Rng` for `Box`) - rust-lang/rust#162246 (Fix incorrect meta span) - rust-lang/rust#162266 (std: fix typo) - rust-lang/rust#162291 (Add regression test from 1.98.1) - rust-lang/rust#162303 (Fix a minor spelling error in `library/std/src/sys/fs/unix.rs`)
…uwer Rollup of 25 pull requests Successful merges: - rust-lang/rust#162072 (Add new Tier-3 target: `powerpc64-sony-ps3`) - rust-lang/rust#162179 (type system const items via direct rhs) - rust-lang/rust#162255 (let people change rustc-dev-guide in-tree) - rust-lang/rust#162262 (Avoid manually instantiating some binders in error reporting with `-Znext-solver`) - rust-lang/rust#162277 (Introduce `rustc_middle::middel::resolve`) - rust-lang/rust#162285 (box: fixup map/try_map deallocate calls) - rust-lang/rust#162286 (string: don't unwind prematurely) - rust-lang/rust#162289 (alloc: a bunch of safety comments) - rust-lang/rust#162290 (abby test DSL: AliasTyOutlivesViaEnv) - rust-lang/rust#162292 (Update `askama` version to `0.16.1`) - rust-lang/rust#160509 (Remove `RegionExt`; move methods to `Region` in `rustc_type_ir`) - rust-lang/rust#160906 (Suggest usize instead of placeholder type for array length constants) - rust-lang/rust#160936 (traits: Represent live alias arguments as bitsets) - rust-lang/rust#161394 (fix `is_homogeneous_aggregate`: use unit size, not total size ) - rust-lang/rust#161400 (Improve diagnostics for references to closures) - rust-lang/rust#161656 (Suggest mutable references for FnMut closure arguments) - rust-lang/rust#161711 (Add more splat fn type tests) - rust-lang/rust#161786 (Make `tcx.def_id_partial_cmp` public) - rust-lang/rust#161953 (sanitizers: Implicitly disable mutually exclusive sanitizers) - rust-lang/rust#162155 (add suggestion for `rustc_allowed_through_unstable_modules` attribute) - rust-lang/rust#162212 (Implement `Rng` for `Box`) - rust-lang/rust#162246 (Fix incorrect meta span) - rust-lang/rust#162266 (std: fix typo) - rust-lang/rust#162291 (Add regression test from 1.98.1) - rust-lang/rust#162303 (Fix a minor spelling error in `library/std/src/sys/fs/unix.rs`)
This attempts to match clang's behavior of implicitly disabling sanitizers that are incompatible. Specifically, if a set of default sanitizers would be incompatible with ones provided by -Zsanitize=..., then clang (and now rust) will opt for keeping the ones specified via flags over the ones used as platform defaults. This helps maintain build consistency where we can just enable sanitizers via flags for both rust and c++ code without needing to manually disable others.
The driving reason for this is asan and safestack where we'd like to enable safestack by default for x86_64 fuchsia but disable it if -Zsanitize=address is passed (matching clang's behavior).
This commit also refactors all uses of
self.opts.unstable_opts.sanitizerto go through the updatedsanitizer()method.AI: Gemini was used to help review the code and write some tests, but it did not generate the whole patch. I edited and reviewed this PR to the best of my ability before pushing for review.