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

Use panic::set_hook to print the ICE message #60584

Merged
merged 8 commits into from
Sep 15, 2019
Merged
Show file tree
Hide file tree
Changes from 7 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
2 changes: 1 addition & 1 deletion Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2844,7 +2844,6 @@ dependencies = [
"fmt_macros",
"graphviz",
"jobserver",
"lazy_static 1.3.0",
"log",
"measureme",
"num_cpus",
Expand Down Expand Up @@ -3204,6 +3203,7 @@ version = "0.0.0"
dependencies = [
"env_logger 0.5.13",
"graphviz",
"lazy_static 1.3.0",
"log",
"rustc",
"rustc_ast_borrowck",
Expand Down
1 change: 0 additions & 1 deletion src/librustc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ bitflags = "1.0"
fmt_macros = { path = "../libfmt_macros" }
graphviz = { path = "../libgraphviz" }
jobserver = "0.1"
lazy_static = "1.0.0"
num_cpus = "1.0"
scoped-tls = "1.0"
log = { version = "0.4", features = ["release_max_level_info", "std"] }
Expand Down
1 change: 0 additions & 1 deletion src/librustc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@

#[macro_use] extern crate bitflags;
extern crate getopts;
#[macro_use] extern crate lazy_static;
#[macro_use] extern crate scoped_tls;
#[cfg(windows)]
extern crate libc;
Expand Down
37 changes: 0 additions & 37 deletions src/librustc/util/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,13 @@ use rustc_data_structures::{fx::FxHashMap, sync::Lock};
use std::cell::{RefCell, Cell};
use std::fmt::Debug;
use std::hash::Hash;
use std::panic;
use std::env;
use std::time::{Duration, Instant};

use std::sync::mpsc::{Sender};
use syntax_pos::{SpanData};
use syntax::symbol::{Symbol, sym};
use rustc_macros::HashStable;
use crate::ty::TyCtxt;
use crate::dep_graph::{DepNode};
use lazy_static;
use crate::session::Session;

#[cfg(test)]
Expand All @@ -31,39 +27,6 @@ pub struct ErrorReported;

thread_local!(static TIME_DEPTH: Cell<usize> = Cell::new(0));

lazy_static! {
static ref DEFAULT_HOOK: Box<dyn Fn(&panic::PanicInfo<'_>) + Sync + Send + 'static> = {
let hook = panic::take_hook();
panic::set_hook(Box::new(panic_hook));
hook
};
}

fn panic_hook(info: &panic::PanicInfo<'_>) {
(*DEFAULT_HOOK)(info);

let backtrace = env::var_os("RUST_BACKTRACE").map(|x| &x != "0").unwrap_or(false);

if backtrace {
TyCtxt::try_print_query_stack();
}

#[cfg(windows)]
unsafe {
if env::var("RUSTC_BREAK_ON_ICE").is_ok() {
extern "system" {
fn DebugBreak();
}
// Trigger a debugger if we crashed during bootstrap.
DebugBreak();
}
}
}

pub fn install_panic_hook() {
lazy_static::initialize(&DEFAULT_HOOK);
}

/// Parameters to the `Dump` variant of type `ProfileQueriesMsg`.
#[derive(Clone,Debug)]
pub struct ProfQDumpParams {
Expand Down
1 change: 1 addition & 0 deletions src/librustc_driver/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ crate-type = ["dylib"]

[dependencies]
graphviz = { path = "../libgraphviz" }
lazy_static = "1.0"
log = "0.4"
env_logger = { version = "0.5", default-features = false }
rustc = { path = "../librustc" }
Expand Down
143 changes: 94 additions & 49 deletions src/librustc_driver/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ pub extern crate getopts;
extern crate libc;
#[macro_use]
extern crate log;
#[macro_use]
extern crate lazy_static;

pub extern crate rustc_plugin_impl as plugin;

Expand All @@ -36,8 +38,8 @@ use rustc::session::{early_error, early_warn};
use rustc::lint::Lint;
use rustc::lint;
use rustc::hir::def_id::LOCAL_CRATE;
use rustc::util::common::{ErrorReported, install_panic_hook, print_time_passes_entry};
use rustc::util::common::{set_time_depth, time};
use rustc::ty::TyCtxt;
use rustc::util::common::{set_time_depth, time, print_time_passes_entry, ErrorReported};
use rustc_metadata::locator;
use rustc_metadata::cstore::CStore;
use rustc_codegen_utils::codegen_backend::CodegenBackend;
Expand Down Expand Up @@ -162,8 +164,6 @@ pub fn run_compiler(
None => return Ok(()),
};

install_panic_hook();

let (sopts, cfg) = config::build_session_options_and_crate_config(&matches);

let mut dummy_config = |sopts, cfg, diagnostic_output| {
Expand Down Expand Up @@ -1143,61 +1143,105 @@ fn extra_compiler_flags() -> Option<(Vec<String>, bool)> {
}
}

/// Runs a procedure which will detect panics in the compiler and print nicer
/// error messages rather than just failing the test.
/// Runs a closure and catches unwinds triggered by fatal errors.
///
/// The diagnostic emitter yielded to the procedure should be used for reporting
/// errors of the compiler.
pub fn report_ices_to_stderr_if_any<F: FnOnce() -> R, R>(f: F) -> Result<R, ErrorReported> {
/// The compiler currently unwinds with a special sentinel value to abort
/// compilation on fatal errors. This function catches that sentinel and turns
/// the panic into a `Result` instead.
pub fn catch_fatal_errors<F: FnOnce() -> R, R>(f: F) -> Result<R, ErrorReported> {
catch_unwind(panic::AssertUnwindSafe(f)).map_err(|value| {
if value.is::<errors::FatalErrorMarker>() {
ErrorReported
} else {
// Thread panicked without emitting a fatal diagnostic
eprintln!("");

let emitter = Box::new(errors::emitter::EmitterWriter::stderr(
errors::ColorConfig::Auto,
None,
false,
false,
None,
));
let handler = errors::Handler::with_emitter(true, None, emitter);

// a .span_bug or .bug call has already printed what
// it wants to print.
if !value.is::<errors::ExplicitBug>() {
handler.emit(&MultiSpan::new(),
"unexpected panic",
errors::Level::Bug);
}
panic::resume_unwind(value);
}
})
}

let mut 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!("rustc {} running on {}",
option_env!("CFG_VERSION").unwrap_or("unknown_version"),
config::host_triple()).into(),
];
lazy_static! {
static ref DEFAULT_HOOK: Box<dyn Fn(&panic::PanicInfo<'_>) + Sync + Send + 'static> = {
let hook = panic::take_hook();
panic::set_hook(Box::new(|info| report_ice(info, BUG_REPORT_URL)));
hook
};
}

if let Some((flags, excluded_cargo_defaults)) = extra_compiler_flags() {
xs.push(format!("compiler flags: {}", flags.join(" ")).into());
/// Prints the ICE message, including backtrace and query stack.
///
/// 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) {
// Invoke the default handler, which prints the actual panic message and optionally a backtrace
(*DEFAULT_HOOK)(info);

// Separate the output with an empty line
eprintln!();
jonas-schievink marked this conversation as resolved.
Show resolved Hide resolved

let emitter = Box::new(errors::emitter::EmitterWriter::stderr(
errors::ColorConfig::Auto,
None,
false,
false,
None,
));
let handler = 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::<errors::ExplicitBug>() {
handler.emit(&MultiSpan::new(),
"unexpected panic",
errors::Level::Bug);
}

if excluded_cargo_defaults {
xs.push("some of the compiler flags provided by cargo are hidden".into());
}
}
let mut 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!("rustc {} running on {}",
option_env!("CFG_VERSION").unwrap_or("unknown_version"),
config::host_triple()).into(),
];

for note in &xs {
handler.emit(&MultiSpan::new(),
note,
errors::Level::Note);
}
if let Some((flags, excluded_cargo_defaults)) = extra_compiler_flags() {
xs.push(format!("compiler flags: {}", flags.join(" ")).into());

panic::resume_unwind(Box::new(errors::FatalErrorMarker));
if excluded_cargo_defaults {
xs.push("some of the compiler flags provided by cargo are hidden".into());
}
})
}

for note in &xs {
handler.emit(&MultiSpan::new(),
note,
errors::Level::Note);
}

// If backtraces are enabled, also print the query stack
let backtrace = env::var_os("RUST_BACKTRACE").map(|x| &x != "0").unwrap_or(false);

if backtrace {
TyCtxt::try_print_query_stack();
}

#[cfg(windows)]
unsafe {
if env::var("RUSTC_BREAK_ON_ICE").is_ok() {
extern "system" {
fn DebugBreak();
}
// Trigger a debugger if we crashed during bootstrap
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() {
lazy_static::initialize(&DEFAULT_HOOK);
}

/// This allows tools to enable rust logging without having to magically match rustc's
Expand All @@ -1210,7 +1254,8 @@ pub fn main() {
let start = Instant::now();
init_rustc_env_logger();
let mut callbacks = TimePassesCallbacks::default();
let result = report_ices_to_stderr_if_any(|| {
install_ice_hook();
let result = catch_fatal_errors(|| {
let args = env::args_os().enumerate()
.map(|(i, arg)| arg.into_string().unwrap_or_else(|arg| {
early_error(ErrorOutputType::default(),
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,7 @@ where R: 'static + Send,
// First, parse the crate and extract all relevant information.
info!("starting to run rustc");

let result = rustc_driver::report_ices_to_stderr_if_any(move || {
let result = rustc_driver::catch_fatal_errors(move || {
let crate_name = options.crate_name.clone();
let crate_version = options.crate_version.clone();
let (mut krate, renderinfo, renderopts) = core::run_core(options);
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui-fulldeps/compiler-calls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ impl rustc_driver::Callbacks for TestCalls<'_> {
fn main() {
let mut count = 1;
let args = vec!["compiler-calls".to_string(), "foo.rs".to_string()];
rustc_driver::report_ices_to_stderr_if_any(|| {
rustc_driver::catch_fatal_errors(|| {
rustc_driver::run_compiler(&args, &mut TestCalls { count: &mut count }, None, None).ok();
}).ok();
assert_eq!(count, 2);
Expand Down
5 changes: 5 additions & 0 deletions src/test/ui/proc-macro/invalid-punct-ident-1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@
// FIXME https://github.com/rust-lang/rust/issues/59998
// normalize-stderr-test "thread.*panicked.*proc_macro_server.rs.*\n" -> ""
// normalize-stderr-test "note:.*RUST_BACKTRACE=1.*\n" -> ""
// normalize-stderr-test "error: internal compiler error.*\n" -> ""
Copy link
Contributor

Choose a reason for hiding this comment

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

So if I'm reading this right, we're now emitting an ICE message if a proc macro panics? We should not do that. People should not report broken proc macros on the rustc repo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This isn't caused by broken proc macros, it's a legitimate ICE: #59998

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To expand on this, there's a bug in rustc that causes the ICE to print, and it's triggered by certain cases of proc macros doing invalid things (that are documented to panic). I don't think it is very easy to trigger it, it requires doing something like this: https://github.com/rust-lang/rust/blob/61dced18277a8cd55c963502db5a4cdf837858f3/src/test/ui/proc-macro/auxiliary/invalid-punct-ident.rs

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the explanation!

// normalize-stderr-test "note:.*unexpectedly panicked.*\n" -> ""
// normalize-stderr-test "note: we would appreciate a bug report.*\n" -> ""
// normalize-stderr-test "note: compiler flags.*\n" -> ""
// normalize-stderr-test "note: rustc.*running on.*\n" -> ""

#[macro_use]
extern crate invalid_punct_ident;
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/proc-macro/invalid-punct-ident-1.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: proc macro panicked
--> $DIR/invalid-punct-ident-1.rs:10:1
--> $DIR/invalid-punct-ident-1.rs:15:1
|
LL | invalid_punct!();
| ^^^^^^^^^^^^^^^^^
Expand Down
5 changes: 5 additions & 0 deletions src/test/ui/proc-macro/invalid-punct-ident-2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@
// FIXME https://github.com/rust-lang/rust/issues/59998
// normalize-stderr-test "thread.*panicked.*proc_macro_server.rs.*\n" -> ""
// normalize-stderr-test "note:.*RUST_BACKTRACE=1.*\n" -> ""
// normalize-stderr-test "error: internal compiler error.*\n" -> ""
// normalize-stderr-test "note:.*unexpectedly panicked.*\n" -> ""
// normalize-stderr-test "note: we would appreciate a bug report.*\n" -> ""
// normalize-stderr-test "note: compiler flags.*\n" -> ""
// normalize-stderr-test "note: rustc.*running on.*\n" -> ""

#[macro_use]
extern crate invalid_punct_ident;
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/proc-macro/invalid-punct-ident-2.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: proc macro panicked
--> $DIR/invalid-punct-ident-2.rs:10:1
--> $DIR/invalid-punct-ident-2.rs:15:1
|
LL | invalid_ident!();
| ^^^^^^^^^^^^^^^^^
Expand Down
5 changes: 5 additions & 0 deletions src/test/ui/proc-macro/invalid-punct-ident-3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@
// FIXME https://github.com/rust-lang/rust/issues/59998
// normalize-stderr-test "thread.*panicked.*proc_macro_server.rs.*\n" -> ""
// normalize-stderr-test "note:.*RUST_BACKTRACE=1.*\n" -> ""
// normalize-stderr-test "error: internal compiler error.*\n" -> ""
// normalize-stderr-test "note:.*unexpectedly panicked.*\n" -> ""
// normalize-stderr-test "note: we would appreciate a bug report.*\n" -> ""
// normalize-stderr-test "note: compiler flags.*\n" -> ""
// normalize-stderr-test "note: rustc.*running on.*\n" -> ""

#[macro_use]
extern crate invalid_punct_ident;
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/proc-macro/invalid-punct-ident-3.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: proc macro panicked
--> $DIR/invalid-punct-ident-3.rs:10:1
--> $DIR/invalid-punct-ident-3.rs:15:1
|
LL | invalid_raw_ident!();
| ^^^^^^^^^^^^^^^^^^^^^
Expand Down