From f0be14565fd331e9c96123dda33bf1105f35201c Mon Sep 17 00:00:00 2001 From: jyn Date: Sat, 29 Apr 2023 06:30:21 -0500 Subject: [PATCH 1/2] drive-by cleanup of rustdoc comment --- src/librustdoc/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index c15afca22611b..70030e293344e 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -156,13 +156,13 @@ pub fn main() { rustc_driver::install_ice_hook(); - // When using CI artifacts (with `download_stage1 = true`), tracing is unconditionally built + // When using CI artifacts with `download-rustc`, tracing is unconditionally built // with `--features=static_max_level_info`, which disables almost all rustdoc logging. To avoid // this, compile our own version of `tracing` that logs all levels. // NOTE: this compiles both versions of tracing unconditionally, because // - The compile time hit is not that bad, especially compared to rustdoc's incremental times, and - // - Otherwise, there's no warning that logging is being ignored when `download_stage1 = true`. - // NOTE: The reason this doesn't show double logging when `download_stage1 = false` and + // - Otherwise, there's no warning that logging is being ignored when `download-rustc` is enabled + // NOTE: The reason this doesn't show double logging when `download-rustc = false` and // `debug_logging = true` is because all rustc logging goes to its version of tracing (the one // in the sysroot), and all of rustdoc's logging goes to its version (the one in Cargo.toml). init_logging(); From 2469afef1a9697e671f9e90971c342fa3e0006e6 Mon Sep 17 00:00:00 2001 From: jyn Date: Sat, 29 Apr 2023 06:29:07 -0500 Subject: [PATCH 2/2] Make the BUG_REPORT_URL configurable by tools This greatly simplifies how hard it is to set a custom bug report url; previously tools had to copy the entire hook implementation. - Switch clippy to the new hook This also adds a `extra_info` callback so clippy can include its own version number, which differs from rustc's. - Call `install_ice_hook` in rustfmt --- compiler/rustc_driver_impl/src/lib.rs | 103 ++++++++++++--------- src/librustdoc/lib.rs | 6 +- src/tools/clippy/src/driver.rs | 70 ++------------ src/tools/miri/src/bin/miri.rs | 8 +- src/tools/rustfmt/src/bin/main.rs | 9 ++ tests/rustdoc-ui/ice-bug-report-url.rs | 14 +++ tests/rustdoc-ui/ice-bug-report-url.stderr | 16 ++++ 7 files changed, 115 insertions(+), 111 deletions(-) create mode 100644 tests/rustdoc-ui/ice-bug-report-url.rs create mode 100644 tests/rustdoc-ui/ice-bug-report-url.stderr diff --git a/compiler/rustc_driver_impl/src/lib.rs b/compiler/rustc_driver_impl/src/lib.rs index 5fac485de6417..92153e91a277d 100644 --- a/compiler/rustc_driver_impl/src/lib.rs +++ b/compiler/rustc_driver_impl/src/lib.rs @@ -25,7 +25,7 @@ use rustc_data_structures::profiling::{ use rustc_data_structures::sync::SeqCst; use rustc_errors::registry::{InvalidErrorCode, Registry}; use rustc_errors::{ - DiagnosticMessage, ErrorGuaranteed, PResult, SubdiagnosticMessage, TerminalUrl, + DiagnosticMessage, ErrorGuaranteed, Handler, PResult, SubdiagnosticMessage, TerminalUrl, }; use rustc_feature::find_gated_cfg; use rustc_fluent_macro::fluent_messages; @@ -55,7 +55,7 @@ use std::panic::{self, catch_unwind}; use std::path::PathBuf; use std::process::{self, Command, Stdio}; use std::str; -use std::sync::LazyLock; +use std::sync::OnceLock; use std::time::Instant; // This import blocks the use of panicking `print` and `println` in all the code @@ -119,7 +119,7 @@ pub const EXIT_SUCCESS: i32 = 0; /// Exit status code used for compilation failures and invalid flags. pub const EXIT_FAILURE: i32 = 1; -const BUG_REPORT_URL: &str = "https://github.com/rust-lang/rust/issues/new\ +pub const DEFAULT_BUG_REPORT_URL: &str = "https://github.com/rust-lang/rust/issues/new\ ?labels=C-bug%2C+I-ICE%2C+T-compiler&template=ice.md"; const ICE_REPORT_COMPILER_FLAGS: &[&str] = &["-Z", "-C", "--crate-type"]; @@ -1195,35 +1195,58 @@ pub fn catch_with_exit_code(f: impl FnOnce() -> interface::Result<()>) -> i32 { } } -static DEFAULT_HOOK: LazyLock) + Sync + Send + 'static>> = - LazyLock::new(|| { - let hook = panic::take_hook(); - panic::set_hook(Box::new(|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)") - { - early_error_no_abort(ErrorOutputType::default(), &msg); - return; - } - }; +/// Stores the default panic hook, from before [`install_ice_hook`] was called. +static DEFAULT_HOOK: OnceLock) + Sync + Send + 'static>> = + OnceLock::new(); + +/// Installs a panic hook that will print the ICE message on unexpected panics. +/// +/// The hook is intended to be useable even by external tools. You can pass a custom +/// `bug_report_url`, or report arbitrary info in `extra_info`. Note that `extra_info` is called in +/// a context where *the thread is currently panicking*, so it must not panic or the process will +/// abort. +/// +/// If you have no extra info to report, pass the empty closure `|_| ()` as the argument to +/// extra_info. +/// +/// A custom rustc driver can skip calling this to set up a custom ICE hook. +pub fn install_ice_hook(bug_report_url: &'static str, extra_info: fn(&Handler)) { + // If the user has not explicitly overridden "RUST_BACKTRACE", then produce + // full backtraces. When a compiler ICE happens, we want to gather + // as much information as possible to present in the issue opened + // by the user. Compiler developers and other rustc users can + // opt in to less-verbose backtraces by manually setting "RUST_BACKTRACE" + // (e.g. `RUST_BACKTRACE=1`) + if std::env::var("RUST_BACKTRACE").is_err() { + std::env::set_var("RUST_BACKTRACE", "full"); + } - // 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); + let default_hook = DEFAULT_HOOK.get_or_init(panic::take_hook); - // Separate the output with an empty line - eprintln!(); + 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)") { + early_error_no_abort(ErrorOutputType::default(), &msg); + return; } + }; - // Print the ICE message - report_ice(info, BUG_REPORT_URL); - })); - hook - }); + // 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!(); + } + + // Print the ICE message + report_ice(info, bug_report_url, extra_info); + })); +} /// Prints the ICE message, including query stack, but without backtrace. /// @@ -1231,7 +1254,7 @@ static DEFAULT_HOOK: LazyLock) + Sync + Send + /// /// When `install_ice_hook` is called, this function will be called as the panic /// hook. -pub fn report_ice(info: &panic::PanicInfo<'_>, bug_report_url: &str) { +pub fn report_ice(info: &panic::PanicInfo<'_>, bug_report_url: &str, extra_info: fn(&Handler)) { let fallback_bundle = rustc_errors::fallback_fluent_bundle(crate::DEFAULT_LOCALE_RESOURCES.to_vec(), false); let emitter = Box::new(rustc_errors::emitter::EmitterWriter::stderr( @@ -1276,6 +1299,10 @@ pub fn report_ice(info: &panic::PanicInfo<'_>, bug_report_url: &str) { interface::try_print_query_stack(&handler, num_frames); + // We don't trust this callback not to panic itself, so run it at the end after we're sure we've + // printed all the relevant info. + extra_info(&handler); + #[cfg(windows)] if env::var("RUSTC_BREAK_ON_ICE").is_ok() { // Trigger a debugger if we crashed during bootstrap @@ -1283,22 +1310,6 @@ pub fn report_ice(info: &panic::PanicInfo<'_>, bug_report_url: &str) { } } -/// Installs a panic hook that will print the ICE message on unexpected panics. -/// -/// A custom rustc driver can skip calling this to set up a custom ICE hook. -pub fn install_ice_hook() { - // If the user has not explicitly overridden "RUST_BACKTRACE", then produce - // full backtraces. When a compiler ICE happens, we want to gather - // as much information as possible to present in the issue opened - // by the user. Compiler developers and other rustc users can - // opt in to less-verbose backtraces by manually setting "RUST_BACKTRACE" - // (e.g. `RUST_BACKTRACE=1`) - if std::env::var("RUST_BACKTRACE").is_err() { - std::env::set_var("RUST_BACKTRACE", "full"); - } - LazyLock::force(&DEFAULT_HOOK); -} - /// This allows tools to enable rust logging without having to magically match rustc's /// tracing crate version. pub fn init_rustc_env_logger() { @@ -1369,7 +1380,7 @@ pub fn main() -> ! { init_rustc_env_logger(); signal_handler::install(); let mut callbacks = TimePassesCallbacks::default(); - install_ice_hook(); + install_ice_hook(DEFAULT_BUG_REPORT_URL, |_| ()); let exit_code = catch_with_exit_code(|| { let args = env::args_os() .enumerate() diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index 70030e293344e..2746debbfab61 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -154,7 +154,11 @@ pub fn main() { } } - rustc_driver::install_ice_hook(); + rustc_driver::install_ice_hook( + "https://github.com/rust-lang/rust/issues/new\ + ?labels=C-bug%2C+I-ICE%2C+T-rustdoc&template=ice.md", + |_| (), + ); // When using CI artifacts with `download-rustc`, tracing is unconditionally built // with `--features=static_max_level_info`, which disables almost all rustdoc logging. To avoid diff --git a/src/tools/clippy/src/driver.rs b/src/tools/clippy/src/driver.rs index 205905d509135..59bf447a7cd07 100644 --- a/src/tools/clippy/src/driver.rs +++ b/src/tools/clippy/src/driver.rs @@ -11,7 +11,6 @@ // FIXME: switch to something more ergonomic here, once available. // (Currently there is no way to opt into sysroot crates without `extern crate`.) extern crate rustc_driver; -extern crate rustc_errors; extern crate rustc_interface; extern crate rustc_session; extern crate rustc_span; @@ -20,13 +19,10 @@ use rustc_interface::interface; use rustc_session::parse::ParseSess; use rustc_span::symbol::Symbol; -use std::borrow::Cow; use std::env; use std::ops::Deref; -use std::panic; use std::path::Path; use std::process::exit; -use std::sync::LazyLock; /// If a command-line option matches `find_arg`, then apply the predicate `pred` on its value. If /// true, then return it. The parameter is assumed to be either `--arg=value` or `--arg value`. @@ -198,66 +194,18 @@ You can use tool lints to allow or deny lints from your code, eg.: const BUG_REPORT_URL: &str = "https://github.com/rust-lang/rust-clippy/issues/new"; -type PanicCallback = dyn Fn(&panic::PanicInfo<'_>) + Sync + Send + 'static; -static ICE_HOOK: LazyLock> = LazyLock::new(|| { - let hook = panic::take_hook(); - panic::set_hook(Box::new(|info| report_clippy_ice(info, BUG_REPORT_URL))); - hook -}); - -fn report_clippy_ice(info: &panic::PanicInfo<'_>, bug_report_url: &str) { - // Invoke our ICE handler, which prints the actual panic message and optionally a backtrace - (*ICE_HOOK)(info); - - // Separate the output with an empty line - eprintln!(); - - let fallback_bundle = rustc_errors::fallback_fluent_bundle(rustc_driver::DEFAULT_LOCALE_RESOURCES.to_vec(), false); - let emitter = Box::new(rustc_errors::emitter::EmitterWriter::stderr( - rustc_errors::ColorConfig::Auto, - None, - None, - fallback_bundle, - false, - false, - None, - false, - false, - rustc_errors::TerminalUrl::No, - )); - let handler = rustc_errors::Handler::with_emitter(true, None, emitter); - - // a .span_bug or .bug call has already printed what - // it wants to print. - if !info.payload().is::() { - let mut d = rustc_errors::Diagnostic::new(rustc_errors::Level::Bug, "unexpected panic"); - handler.emit_diagnostic(&mut d); - } - - let version_info = rustc_tools_util::get_version_info!(); - - let xs: Vec> = vec![ - "the compiler unexpectedly panicked. this is a bug.".into(), - format!("we would appreciate a bug report: {bug_report_url}").into(), - format!("Clippy version: {version_info}").into(), - ]; - - for note in &xs { - handler.note_without_error(note.as_ref()); - } - - // If backtraces are enabled, also print the query stack - let backtrace = env::var_os("RUST_BACKTRACE").map_or(false, |x| &x != "0"); - - let num_frames = if backtrace { None } else { Some(2) }; - - interface::try_print_query_stack(&handler, num_frames); -} - #[allow(clippy::too_many_lines)] pub fn main() { rustc_driver::init_rustc_env_logger(); - LazyLock::force(&ICE_HOOK); + + rustc_driver::install_ice_hook(BUG_REPORT_URL, |handler| { + // FIXME: this macro calls unwrap internally but is called in a panicking context! It's not + // as simple as moving the call from the hook to main, because `install_ice_hook` doesn't + // accept a generic closure. + let version_info = rustc_tools_util::get_version_info!(); + handler.note_without_error(format!("Clippy version: {version_info}")); + }); + exit(rustc_driver::catch_with_exit_code(move || { let mut orig_args: Vec = env::args().collect(); let has_sysroot_arg = arg_value(&orig_args, "--sysroot", |_| true).is_some(); diff --git a/src/tools/miri/src/bin/miri.rs b/src/tools/miri/src/bin/miri.rs index 3aa71bb7e3c87..2c0074951d6e5 100644 --- a/src/tools/miri/src/bin/miri.rs +++ b/src/tools/miri/src/bin/miri.rs @@ -286,11 +286,10 @@ fn main() { // (`install_ice_hook` might change `RUST_BACKTRACE`.) let env_snapshot = env::vars_os().collect::>(); - // Earliest rustc setup. - rustc_driver::install_ice_hook(); - // If the environment asks us to actually be rustc, then do that. if let Some(crate_kind) = env::var_os("MIRI_BE_RUSTC") { + // Earliest rustc setup. + rustc_driver::install_ice_hook(rustc_driver::DEFAULT_BUG_REPORT_URL, |_| ()); rustc_driver::init_rustc_env_logger(); let target_crate = if crate_kind == "target" { @@ -309,6 +308,9 @@ fn main() { ) } + // Add an ICE bug report hook. + rustc_driver::install_ice_hook("https://github.com/rust-lang/miri/issues/new", |_| ()); + // Init loggers the Miri way. init_early_loggers(); diff --git a/src/tools/rustfmt/src/bin/main.rs b/src/tools/rustfmt/src/bin/main.rs index be64559e87745..47846424b06e4 100644 --- a/src/tools/rustfmt/src/bin/main.rs +++ b/src/tools/rustfmt/src/bin/main.rs @@ -1,3 +1,5 @@ +#![feature(rustc_private)] + use anyhow::{format_err, Result}; use io::Error as IoError; @@ -19,7 +21,14 @@ use crate::rustfmt::{ FormatReportFormatterBuilder, Input, Session, Verbosity, }; +const BUG_REPORT_URL: &str = "https://github.com/rust-lang/rustfmt/issues/new?labels=bug"; + +// N.B. these crates are loaded from the sysroot, so they need extern crate. +extern crate rustc_driver; + fn main() { + rustc_driver::install_ice_hook(BUG_REPORT_URL, |_| ()); + env_logger::Builder::from_env("RUSTFMT_LOG").init(); let opts = make_opts(); diff --git a/tests/rustdoc-ui/ice-bug-report-url.rs b/tests/rustdoc-ui/ice-bug-report-url.rs new file mode 100644 index 0000000000000..cc066447d313b --- /dev/null +++ b/tests/rustdoc-ui/ice-bug-report-url.rs @@ -0,0 +1,14 @@ +// compile-flags: -Ztreat-err-as-bug +// failure-status: 101 +// error-pattern: aborting due to +// error-pattern: we would appreciate a bug report: https://github.com/rust-lang/rust/issues/new?labels=C-bug%2C+I-ICE%2C+T-rustdoc&template=ice.md + +// normalize-stderr-test "note: compiler flags.*\n\n" -> "" +// normalize-stderr-test "note: rustc.*running on.*" -> "note: rustc {version} running on {platform}" +// normalize-stderr-test "thread.*panicked at .*, compiler.*" -> "thread panicked at 'aborting due to `-Z treat-err-as-bug`'" +// normalize-stderr-test "\s*\d{1,}: .*\n" -> "" +// normalize-stderr-test "\s at .*\n" -> "" +// normalize-stderr-test ".*note: Some details are omitted.*\n" -> "" + +fn wrong() +//~^ ERROR expected one of diff --git a/tests/rustdoc-ui/ice-bug-report-url.stderr b/tests/rustdoc-ui/ice-bug-report-url.stderr new file mode 100644 index 0000000000000..cfb73a9b9193f --- /dev/null +++ b/tests/rustdoc-ui/ice-bug-report-url.stderr @@ -0,0 +1,16 @@ +error: expected one of `->`, `where`, or `{`, found `` + --> $DIR/ice-bug-report-url.rs:13:10 + | +LL | fn wrong() + | ^ expected one of `->`, `where`, or `{` + +thread panicked at 'aborting due to `-Z treat-err-as-bug`' +stack backtrace: +error: the compiler unexpectedly panicked. this is a bug. + +note: we would appreciate a bug report: https://github.com/rust-lang/rust/issues/new?labels=C-bug%2C+I-ICE%2C+T-rustdoc&template=ice.md + +note: rustc {version} running on {platform} + +query stack during panic: +end of query stack