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

Make the BUG_REPORT_URL configurable by tools #110989

Merged
merged 2 commits into from
May 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 57 additions & 46 deletions compiler/rustc_driver_impl/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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"];
Expand Down Expand Up @@ -1195,43 +1195,66 @@ pub fn catch_with_exit_code(f: impl FnOnce() -> interface::Result<()>) -> i32 {
}
}

static DEFAULT_HOOK: LazyLock<Box<dyn Fn(&panic::PanicInfo<'_>) + 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::<String>() {
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<Box<dyn Fn(&panic::PanicInfo<'_>) + 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::<rustc_errors::DelayedBugPanic>() {
(*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::<String>() {
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::<rustc_errors::DelayedBugPanic>() {
(*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.
///
/// The message will point the user at `bug_report_url` to report the ICE.
///
/// 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(
Expand Down Expand Up @@ -1276,29 +1299,17 @@ 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
unsafe { windows::Win32::System::Diagnostics::Debug::DebugBreak() };
}
}

/// 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() {
Expand Down Expand Up @@ -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()
Expand Down
12 changes: 8 additions & 4 deletions src/librustdoc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,15 +154,19 @@ 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_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();
Expand Down
70 changes: 9 additions & 61 deletions src/tools/clippy/src/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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`.
Expand Down Expand Up @@ -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<Box<PanicCallback>> = 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::<rustc_errors::ExplicitBug>() {
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<Cow<'static, str>> = 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<String> = env::args().collect();
let has_sysroot_arg = arg_value(&orig_args, "--sysroot", |_| true).is_some();
Expand Down
8 changes: 5 additions & 3 deletions src/tools/miri/src/bin/miri.rs
Original file line number Diff line number Diff line change
Expand Up @@ -286,11 +286,10 @@ fn main() {
// (`install_ice_hook` might change `RUST_BACKTRACE`.)
let env_snapshot = env::vars_os().collect::<Vec<_>>();

// 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" {
Expand All @@ -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();

Expand Down
9 changes: 9 additions & 0 deletions src/tools/rustfmt/src/bin/main.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#![feature(rustc_private)]

use anyhow::{format_err, Result};

use io::Error as IoError;
Expand All @@ -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();

Expand Down
14 changes: 14 additions & 0 deletions tests/rustdoc-ui/ice-bug-report-url.rs
Original file line number Diff line number Diff line change
@@ -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
16 changes: 16 additions & 0 deletions tests/rustdoc-ui/ice-bug-report-url.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
error: expected one of `->`, `where`, or `{`, found `<eof>`
--> $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