From 8ad23794077b4380da05fc5dabb573f38bdb2718 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 7 Sep 2023 04:16:06 +0000 Subject: [PATCH 1/2] Don't modify libstd to dump rustc ICEs --- compiler/rustc_driver_impl/src/lib.rs | 63 ++++++++++++++---------- library/std/src/lib.rs | 3 -- library/std/src/panicking.rs | 36 +++----------- tests/run-make/dump-ice-to-disk/check.sh | 4 +- 4 files changed, 46 insertions(+), 60 deletions(-) diff --git a/compiler/rustc_driver_impl/src/lib.rs b/compiler/rustc_driver_impl/src/lib.rs index 1a16759d7f9a8..35f4ff7d964e2 100644 --- a/compiler/rustc_driver_impl/src/lib.rs +++ b/compiler/rustc_driver_impl/src/lib.rs @@ -7,7 +7,7 @@ #![doc(html_root_url = "https://doc.rust-lang.org/nightly/nightly-rustc/")] #![feature(lazy_cell)] #![feature(decl_macro)] -#![feature(ice_to_disk)] +#![feature(panic_update_hook)] #![feature(let_chains)] #![recursion_limit = "256"] #![allow(rustc::potential_query_instability)] @@ -50,9 +50,9 @@ use std::collections::BTreeMap; use std::env; use std::ffi::OsString; use std::fmt::Write as _; -use std::fs; +use std::fs::{self, File}; use std::io::{self, IsTerminal, Read, Write}; -use std::panic::{self, catch_unwind}; +use std::panic::{self, catch_unwind, PanicInfo}; use std::path::PathBuf; use std::process::{self, Command, Stdio}; use std::str; @@ -1323,31 +1323,42 @@ pub fn install_ice_hook(bug_report_url: &'static str, extra_info: fn(&Handler)) std::env::set_var("RUST_BACKTRACE", "full"); } - panic::set_hook(Box::new(move |info| { - // If the error was caused by a broken pipe then this is not a bug. - // Write the error and return immediately. See #98700. - #[cfg(windows)] - if let Some(msg) = info.payload().downcast_ref::() { - if msg.starts_with("failed printing to stdout: ") && msg.ends_with("(os error 232)") { - // the error code is already going to be reported when the panic unwinds up the stack - let handler = EarlyErrorHandler::new(ErrorOutputType::default()); - let _ = handler.early_error_no_abort(msg.clone()); - return; - } - }; - - // Invoke the default handler, which prints the actual panic message and optionally a backtrace - // Don't do this for delayed bugs, which already emit their own more useful backtrace. - if !info.payload().is::() { - std::panic_hook_with_disk_dump(info, ice_path().as_deref()); + panic::update_hook(Box::new( + move |default_hook: &(dyn Fn(&PanicInfo<'_>) + Send + Sync + 'static), + info: &PanicInfo<'_>| { + // If the error was caused by a broken pipe then this is not a bug. + // Write the error and return immediately. See #98700. + #[cfg(windows)] + if let Some(msg) = info.payload().downcast_ref::() { + if msg.starts_with("failed printing to stdout: ") && msg.ends_with("(os error 232)") + { + // the error code is already going to be reported when the panic unwinds up the stack + let handler = EarlyErrorHandler::new(ErrorOutputType::default()); + let _ = handler.early_error_no_abort(msg.clone()); + return; + } + }; - // Separate the output with an empty line - eprintln!(); - } + // Invoke the default handler, which prints the actual panic message and optionally a backtrace + // Don't do this for delayed bugs, which already emit their own more useful backtrace. + if !info.payload().is::() { + default_hook(info); + // Separate the output with an empty line + eprintln!(); + + if let Some(ice_path) = ice_path() + && let Ok(mut out) = + File::options().create(true).append(true).open(&ice_path) + { + let _ = + write!(&mut out, "{info}{:#}", std::backtrace::Backtrace::force_capture()); + } + } - // Print the ICE message - report_ice(info, bug_report_url, extra_info); - })); + // Print the ICE message + report_ice(info, bug_report_url, extra_info); + }, + )); } /// Prints the ICE message, including query stack, but without backtrace. diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index 1955ef815ff80..0e41b35ec1e84 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -613,9 +613,6 @@ pub mod alloc; // Private support modules mod panicking; -#[unstable(feature = "ice_to_disk", issue = "none")] -pub use panicking::panic_hook_with_disk_dump; - #[path = "../../backtrace/src/lib.rs"] #[allow(dead_code, unused_attributes, fuzzy_provenance_casts)] mod backtrace_rs; diff --git a/library/std/src/panicking.rs b/library/std/src/panicking.rs index b20e5464e6e01..9d066c82a89e6 100644 --- a/library/std/src/panicking.rs +++ b/library/std/src/panicking.rs @@ -236,14 +236,6 @@ where /// The default panic handler. fn default_hook(info: &PanicInfo<'_>) { - panic_hook_with_disk_dump(info, None) -} - -#[unstable(feature = "ice_to_disk", issue = "none")] -/// The implementation of the default panic handler. -/// -/// It can also write the backtrace to a given `path`. This functionality is used only by `rustc`. -pub fn panic_hook_with_disk_dump(info: &PanicInfo<'_>, path: Option<&crate::path::Path>) { // If this is a double panic, make sure that we print a backtrace // for this panic. Otherwise only print it if logging is enabled. let backtrace = if info.force_no_backtrace() { @@ -267,7 +259,7 @@ pub fn panic_hook_with_disk_dump(info: &PanicInfo<'_>, path: Option<&crate::path let thread = thread_info::current_thread(); let name = thread.as_ref().and_then(|t| t.name()).unwrap_or(""); - let write = |err: &mut dyn crate::io::Write, backtrace: Option| { + let write = |err: &mut dyn crate::io::Write| { let _ = writeln!(err, "thread '{name}' panicked at {location}:\n{msg}"); static FIRST_PANIC: AtomicBool = AtomicBool::new(true); @@ -281,19 +273,11 @@ pub fn panic_hook_with_disk_dump(info: &PanicInfo<'_>, path: Option<&crate::path } Some(BacktraceStyle::Off) => { if FIRST_PANIC.swap(false, Ordering::SeqCst) { - if let Some(path) = path { - let _ = writeln!( - err, - "note: a backtrace for this error was stored at `{}`", - path.display(), - ); - } else { - let _ = writeln!( - err, - "note: run with `RUST_BACKTRACE=1` environment variable to display a \ + let _ = writeln!( + err, + "note: run with `RUST_BACKTRACE=1` environment variable to display a \ backtrace" - ); - } + ); } } // If backtraces aren't supported or are forced-off, do nothing. @@ -301,17 +285,11 @@ pub fn panic_hook_with_disk_dump(info: &PanicInfo<'_>, path: Option<&crate::path } }; - if let Some(path) = path - && let Ok(mut out) = crate::fs::File::options().create(true).append(true).open(&path) - { - write(&mut out, BacktraceStyle::full()); - } - if let Some(local) = set_output_capture(None) { - write(&mut *local.lock().unwrap_or_else(|e| e.into_inner()), backtrace); + write(&mut *local.lock().unwrap_or_else(|e| e.into_inner())); set_output_capture(Some(local)); } else if let Some(mut out) = panic_output() { - write(&mut out, backtrace); + write(&mut out); } } diff --git a/tests/run-make/dump-ice-to-disk/check.sh b/tests/run-make/dump-ice-to-disk/check.sh index 91109596a451f..ab6f9ab60188d 100644 --- a/tests/run-make/dump-ice-to-disk/check.sh +++ b/tests/run-make/dump-ice-to-disk/check.sh @@ -22,8 +22,8 @@ rm $TMPDIR/rustc-ice-*.txt # Explicitly disabling ICE dump export RUSTC_ICE=0 $RUSTC src/lib.rs -Z treat-err-as-bug=1 1>$TMPDIR/rust-test-disabled.log 2>&1 -should_be_empty_tmp=$(ls -l $TMPDIR/rustc-ice-*.txt | wc -l) -should_be_empty_dot=$(ls -l ./rustc-ice-*.txt | wc -l) +should_be_empty_tmp=$(ls -l $TMPDIR/rustc-ice-*.txt 2>/dev/null | wc -l) +should_be_empty_dot=$(ls -l ./rustc-ice-*.txt 2>/dev/null | wc -l) echo "#### ICE Dump content:" echo $content From b59480784dce3e02c3ce804d3fd383e2ebede465 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 7 Sep 2023 05:33:36 +0000 Subject: [PATCH 2/2] Make ICE backtrace actually match the panic handler --- compiler/rustc_driver_impl/src/lib.rs | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_driver_impl/src/lib.rs b/compiler/rustc_driver_impl/src/lib.rs index 35f4ff7d964e2..9cfb599aff4cb 100644 --- a/compiler/rustc_driver_impl/src/lib.rs +++ b/compiler/rustc_driver_impl/src/lib.rs @@ -1350,8 +1350,25 @@ pub fn install_ice_hook(bug_report_url: &'static str, extra_info: fn(&Handler)) && let Ok(mut out) = File::options().create(true).append(true).open(&ice_path) { - let _ = - write!(&mut out, "{info}{:#}", std::backtrace::Backtrace::force_capture()); + // The current implementation always returns `Some`. + let location = info.location().unwrap(); + let msg = match info.payload().downcast_ref::<&'static str>() { + Some(s) => *s, + None => match info.payload().downcast_ref::() { + Some(s) => &s[..], + None => "Box", + }, + }; + let thread = std::thread::current(); + let name = thread.name().unwrap_or(""); + let _ = write!( + &mut out, + "thread '{name}' panicked at {location}:\n\ + {msg}\n\ + stack backtrace:\n\ + {:#}", + std::backtrace::Backtrace::force_capture() + ); } }