-
Notifications
You must be signed in to change notification settings - Fork 13.8k
Add panic=immediate-abort #146317
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.
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
Add panic=immediate-abort #146317
Conversation
3b57f28
to
3dbe512
Compare
This comment has been minimized.
This comment has been minimized.
3dbe512
to
aa16ee3
Compare
This comment has been minimized.
This comment has been minimized.
b7cb10a
to
f69c5cd
Compare
This comment has been minimized.
This comment has been minimized.
f69c5cd
to
fd30646
Compare
☔ The latest upstream changes (presumably #146499) made this pull request unmergeable. Please resolve the merge conflicts. |
49ae76e
to
301ff58
Compare
Some changes occurred in compiler/rustc_codegen_ssa Some changes occurred in compiler/rustc_codegen_gcc Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt Some changes occurred in src/tools/compiletest cc @jieyouxu These commits modify compiler targets. Some changes occurred in cfg and check-cfg configuration cc @Urgau |
r? @nnethercote rustbot has assigned @nnethercote. Use |
compiler/rustc_span/src/symbol.rs
Outdated
if_let_rescope, | ||
if_while_or_patterns, | ||
ignore, | ||
immediate_abort, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if the symbol shouldn't be immediate-abort
, to follow the CLI.
Which would make the cfgs be like #[cfg(panic = "immediate-abort")]
which seems nicer than #[cfg(panic = "immediate_abort")]
.
immediate_abort, | |
immediate_abort: "immediate-abort", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suspect that if this is ever stabilized we will need some kind of special handling for the underscore version; we have -Cdebug-assertions
which sets cfg(debug_assertions)
and many other such flags.
I agree that the -
is better, but there's a lot of precedent.
fec8fa4
to
2da7064
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basically looks fine. A couple of things I don't understand, and some of the tests need some light documentation.
@rustbot author
let minicore_path = self.build_minicore(); | ||
aux_rustc.arg("--extern"); | ||
aux_rustc.arg(&format!("minicore={}", minicore_path)); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's going on here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without this code added, the //@ add-core-stubs
attribute silently does nothing in an auxillary crate.
Argh, the test as I wrote it would need a cross-linker in order to actually work, but it looks like we are using a host-only linker to run the tests. I found several other uses of |
@bors r=nnethercote rollup=iffy |
☀️ Test successful - checks-actions |
What is this?This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.Comparing f6092f2 (parent) -> 4056082 (this PR) Test differencesShow 209 test diffsStage 0
Stage 1
Stage 2
(and 103 additional test diffs) Additionally, 6 doctest diffs were found. These are ignored, as they are noisy. Job group index
Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard 40560823602064f4c726aea3e15e104449e1a392 --output-dir test-dashboard And then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
Finished benchmarking commit (4056082): comparison URL. Overall result: ❌ regressions - no action needed@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 -2.5%, secondary -1.9%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary 1.5%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 471.715s -> 472.868s (0.24%) |
### What does this PR try to resolve? The test exercises a regression of extra `-Zbuild-std-features` overriding the entire default-features set. However, after rust-lang/rust#146317 panic_immediate_abort became a codegen flag not a Cargo feature anymore. We pick another `optimize_for_size` feature flag to ensure the regression case is still covered. ### How to test and review this PR? To verify `optimize_for_size` actually triggers the regression, git revert f004691 and changes to `-Zbuild-std-features=optimize_for_size`. Alternatively, we can just drop this test.
…ethercote Add panic=immediate-abort MCP: rust-lang/compiler-team#909 This adds a new panic strategy, `-Cpanic=immediate-abort`. This panic strategy essentially just codifies use of `-Zbuild-std-features=panic_immediate_abort`. This PR is intended to just set up infrastructure, and while it will change how the compiler is invoked for users of the feature, there should be no other impacts. In many parts of the compiler, `PanicStrategy::ImmediateAbort` behaves just like `PanicStrategy::Abort`, because actually most parts of the compiler just mean to ask "can this unwind?" so I've added a helper function so we can say `sess.panic_strategy().unwinds()`. The panic and unwind strategies have some level of compatibility, which mostly means that we can pre-compile the sysroot with unwinding panics then the sysroot can be linked with aborting panics later. The immediate-abort strategy is all-or-nothing, enforced by `compiler/rustc_metadata/src/dependency_format.rs` and this is tested for in `tests/ui/panic-runtime/`. We could _technically_ be more compatible with the other panic strategies, but immediately-aborting panics primarily exist for users who want to eliminate all the code size responsible for the panic runtime. I'm open to other use cases if people want to present them, but not right now. This PR is already large. `-Cpanic=immediate-abort` sets both `cfg(panic = "immediate-abort")` _and_ `cfg(panic = "abort")`. bjorn3 pointed out that people may be checking for the abort cfg to ask if panics will unwind, and also the sysroot feature this is replacing used to require `-Cpanic=abort` so this seems like a good back-compat step. At least for the moment. Unclear if this is a good idea indefinitely. I can imagine this being confusing. The changes to the standard library attributes are purely mechanical. Apart from that, I removed an `unsafe` we haven't needed for a while since the `abort` intrinsic became safe, and I've added a helpful diagnostic for people trying to use the old feature. To test that `-Cpanic=immediate-abort` conflicts with other panic strategies, I've beefed up the core-stubs infrastructure a bit. There is now a separate attribute to set flags on it. I've added a test that this produces the desired codegen, called `tests/run-make-cargo/panic-immediate-abort-codegen/` and also a separate run-make-cargo test that checks that we can build a binary.
…40-g4d4cb757-1 / 99317ef14d0be42fa4039eea7c5ce50cb4e9aee7-4 : 15283f6fe95e5b604273d13a428bab5fc0788f5a-1 https://chromium.googlesource.com/external/github.com/llvm/llvm-project/+log/2384a6a2..4d4cb757 https://chromium.googlesource.com/external/github.com/rust-lang/rust/+log/99317ef14d0b..15283f6fe95e Ran: ./tools/clang/scripts/upload_revision.py 4d4cb757f94470b95458fcbe3b88332b212feeee Also: * ran gnrt_stdlib.py * use immediate-abort for official builds for *all* crates (instead of just the stdlib) to deal with rust-lang/rust#146317. Bug: 444224976,447200172 Disable-Rts: True Cq-Include-Trybots: chromium/try:android-cronet-riscv64-dbg Cq-Include-Trybots: chromium/try:android-cronet-riscv64-rel Cq-Include-Trybots: chromium/try:chromeos-amd64-generic-cfi-thin-lto-rel Cq-Include-Trybots: chromium/try:dawn-win10-x86-deps-rel Cq-Include-Trybots: chromium/try:fuchsia-arm64-cast-receiver-rel Cq-Include-Trybots: chromium/try:ios-catalyst,win-asan,android-official Cq-Include-Trybots: chromium/try:linux-cast-x64-rel Cq-Include-Trybots: chromium/try:linux-chromeos-dbg Cq-Include-Trybots: chromium/try:linux-swangle-try-x64,win-swangle-try-x86 Cq-Include-Trybots: chromium/try:linux-v4l2-codec-rel Cq-Include-Trybots: chromium/try:linux-wayland-mutter-rel Cq-Include-Trybots: chromium/try:linux_chromium_cfi_rel_ng Cq-Include-Trybots: chromium/try:linux_chromium_msan_rel_ng Cq-Include-Trybots: chromium/try:mac-official,linux-official Cq-Include-Trybots: chromium/try:mac12-arm64-rel,mac_chromium_asan_rel_ng Cq-Include-Trybots: chromium/try:win-arm64-rel Cq-Include-Trybots: chromium/try:win-official,win32-official Cq-Include-Trybots: chrome/try:android-arm32-pgo,android-arm64-pgo Cq-Include-Trybots: chrome/try:android-x64-rel-ready Cq-Include-Trybots: chrome/try:chromeos-brya-chrome,chromeos-eve-chrome Cq-Include-Trybots: chrome/try:chromeos-volteer-chrome Cq-Include-Trybots: chrome/try:iphone-device Cq-Include-Trybots: chrome/try:linux-chromeos-chrome Cq-Include-Trybots: chrome/try:linux-pgo,mac-pgo,win32-pgo,win64-pgo Cq-Include-Trybots: chrome/try:win-chrome,win64-chrome,linux-chrome,mac-chrome Cq-Include-Trybots: chromium/try:android-rust-arm32-rel Cq-Include-Trybots: chromium/try:android-rust-arm64-dbg Cq-Include-Trybots: chromium/try:android-rust-arm64-rel Cq-Include-Trybots: chromium/try:linux-rust-x64-dbg Cq-Include-Trybots: chromium/try:linux-rust-x64-rel Cq-Include-Trybots: chromium/try:mac-rust-x64-dbg Cq-Include-Trybots: chromium/try:win-rust-x64-dbg Cq-Include-Trybots: chromium/try:win-rust-x64-rel Change-Id: I078392e5e8cd86b791906fc47aebafed25527097 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6980648 Commit-Queue: Nico Weber <[email protected]> Reviewed-by: Nico Weber <[email protected]> Auto-Submit: Alan Zhao <[email protected]> Cr-Commit-Position: refs/heads/main@{#1520848}
Update the Rust toolchain for query-engine-wasm to `nightly-2025-09-29`. Update the cargo and compiler flags in `build.sh` to use the new panic strategy introduced in <rust-lang/rust#146317>.
Update the Rust toolchain for query-engine-wasm to `nightly-2025-09-29`. Update the cargo and compiler flags in `build.sh` to use the new panic strategy introduced in <rust-lang/rust#146317>.
Update the Rust toolchain for query-engine-wasm to `nightly-2025-09-29`. Update the cargo and compiler flags in `build.sh` to use the new panic strategy introduced in <rust-lang/rust#146317>.
I recently turned `-Zbuild-std-features=panic_immediate_abort` into `-Cpanic=immediate-abort` in rust-lang/rust#146317. There was some discussion of the feature on Zulip here: [#t-compiler/major changes > Unstably add -Cpanic=immediate-abort compiler-team#909](https://rust-lang.zulipchat.com/#narrow/channel/233931-t-compiler.2Fmajor-changes/topic/Unstably.20add.20-Cpanic.3Dimmediate-abort.20compiler-team.23909/with/542845480) One of the outcomes of this shipping in nightly is that a few use patterns were broken, see rust-lang/rust#146974 and rust-lang/rust#146317 for examples. I think most of the users commenting on these issues are having trouble with how `RUSTFLAGS` is propagated through Cargo, and most likely they would have just gotten what they wanted if they could have set a profile option instead. So I'm trying to implement that.
MCP: rust-lang/compiler-team#909
This adds a new panic strategy,
-Cpanic=immediate-abort
. This panic strategy essentially just codifies use of-Zbuild-std-features=panic_immediate_abort
. This PR is intended to just set up infrastructure, and while it will change how the compiler is invoked for users of the feature, there should be no other impacts.In many parts of the compiler,
PanicStrategy::ImmediateAbort
behaves just likePanicStrategy::Abort
, because actually most parts of the compiler just mean to ask "can this unwind?" so I've added a helper function so we can saysess.panic_strategy().unwinds()
.The panic and unwind strategies have some level of compatibility, which mostly means that we can pre-compile the sysroot with unwinding panics then the sysroot can be linked with aborting panics later. The immediate-abort strategy is all-or-nothing, enforced by
compiler/rustc_metadata/src/dependency_format.rs
and this is tested for intests/ui/panic-runtime/
. We could technically be more compatible with the other panic strategies, but immediately-aborting panics primarily exist for users who want to eliminate all the code size responsible for the panic runtime. I'm open to other use cases if people want to present them, but not right now. This PR is already large.-Cpanic=immediate-abort
sets bothcfg(panic = "immediate-abort")
andcfg(panic = "abort")
. bjorn3 pointed out that people may be checking for the abort cfg to ask if panics will unwind, and also the sysroot feature this is replacing used to require-Cpanic=abort
so this seems like a good back-compat step. At least for the moment. Unclear if this is a good idea indefinitely. I can imagine this being confusing.The changes to the standard library attributes are purely mechanical. Apart from that, I removed an
unsafe
we haven't needed for a while since theabort
intrinsic became safe, and I've added a helpful diagnostic for people trying to use the old feature.To test that
-Cpanic=immediate-abort
conflicts with other panic strategies, I've beefed up the core-stubs infrastructure a bit. There is now a separate attribute to set flags on it.I've added a test that this produces the desired codegen, called
tests/run-make-cargo/panic-immediate-abort-codegen/
and also a separate run-make-cargo test that checks that we can build a binary.