squiid/rust
1
0
Fork 0
Code Issues Pull requests Projects Activity

Auto merge of #42938 - est31:col_number, r=eddyb

Browse source 
Output column number info when panicking

Outputs the column number when panicking. Useful when you e.g. have code like `foo[i] = bar[k] + bar[l]` and you get a panic with index out of bounds, or when you have an expression like `a = b + c + d + e` and the addition overflows. Now you know which operation to blame!

The format is `file:line:column`, just like for compiler errors. Example output with the patch:

```
thread 'main' panicked at 'index out of bounds: the len is 5 but the index is 8', src/main.rs:3:8
```

As some of the API between the compiler and the library landscape gets broken, this is a bit hackier than I'd originally wanted it to be.

* `panic` and `panic_bounds_check` lang items got an additional column param, on stage0 I still have to use the previous version. After a SNAP this should be resolved.
* For `#[derive(RustcDeserialze)]`, stage0 requires a fixed signature for `std::rt::begin_panic`, so we can't change it right away. What we need to do instead is to keep the signature, and add a `begin_panic_new` function that we use in later stages instead. After a SNAP we can change the `begin_panic` function and rely on it instead of `begin_panic_new`, and one SNAP later we can remove `begin_panic_new`.
* Fortunately I didn't have to break anything about the panic hook API, I could easily extend it.

Note that debuginfo remains unchanged, so RUST_BACKTRACE output won't contain any column info. See issue #42921 for discussion on including the column in debuginfo.
This commit is contained in:
bors 2017-07-02 15:40:26 +00:00
parent 2a992167c5 3b91f9406c
commit 0679711398
11 changed files with 151 additions and 61 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
src/Cargo.lock generated
View file

@ -157,7 +157,7 @@ dependencies = [
[[package]]
name = "cargo"
version = "0.21.0"
source = "git+https://github.com/rust-lang/cargo#854bc167bbf74053f821f65cb86d506033f3d3a7"
source = "git+https://github.com/rust-lang/cargo#eb6cf012a6cc23c9c89c4009564de9fccc38b9cb"
replace = "cargo 0.21.0"
[[package]]

6
src/doc/unstable-book/src/language-features/lang-items.md
View file

@ -143,7 +143,8 @@ pub extern fn rust_eh_unwind_resume() {
#[no_mangle]
pub extern fn rust_begin_panic(_msg: core::fmt::Arguments,
_file: &'static str,
_line: u32) -> ! {
_line: u32,
_column: u32) -> ! {
unsafe { intrinsics::abort() }
}
```
@ -187,7 +188,8 @@ pub extern fn rust_eh_unwind_resume() {
#[no_mangle]
pub extern fn rust_begin_panic(_msg: core::fmt::Arguments,
_file: &'static str,
_line: u32) -> ! {
_line: u32,
_column: u32) -> ! {
unsafe { intrinsics::abort() }
}
```

10
src/libcore/lib.rs
View file

@ -36,12 +36,12 @@
//! These functions are often provided by the system libc, but can also be
//! provided by the [rlibc crate](https://crates.io/crates/rlibc).
//!
//! * `rust_begin_panic` - This function takes three arguments, a
//! `fmt::Arguments`, a `&'static str`, and a `u32`. These three arguments
//! * `rust_begin_panic` - This function takes four arguments, a
//! `fmt::Arguments`, a `&'static str`, and two `u32`'s. These four arguments
//! dictate the panic message, the file at which panic was invoked, and the
//! line. It is up to consumers of this core library to define this panic
//! function; it is only required to never return. This requires a `lang`
//! attribute named `panic_fmt`.
//! line and column inside the file. It is up to consumers of this core
//! library to define this panic function; it is only required to never
//! return. This requires a `lang` attribute named `panic_fmt`.
//!
//! * `rust_eh_personality` - is used by the failure mechanisms of the
//! compiler. This is often mapped to GCC's personality function, but crates

10
src/libcore/macros.rs
View file

@ -17,16 +17,18 @@ macro_rules! panic {
panic!("explicit panic")
);
($msg:expr) => ({
static _MSG_FILE_LINE: (&'static str, &'static str, u32) = ($msg, file!(), line!());
$crate::panicking::panic(&_MSG_FILE_LINE)
static _MSG_FILE_LINE_COL: (&'static str, &'static str, u32, u32) =
($msg, file!(), line!(), column!());
$crate::panicking::panic(&_MSG_FILE_LINE_COL)
});
($fmt:expr, $($arg:tt)*) => ({
// The leading _'s are to avoid dead code warnings if this is
// used inside a dead function. Just `#[allow(dead_code)]` is
// insufficient, since the user may have
// `#[forbid(dead_code)]` and which cannot be overridden.
static _FILE_LINE: (&'static str, u32) = (file!(), line!());
$crate::panicking::panic_fmt(format_args!($fmt, $($arg)*), &_FILE_LINE)
static _MSG_FILE_LINE_COL: (&'static str, u32, u32) =
(file!(), line!(), column!());
$crate::panicking::panic_fmt(format_args!($fmt, $($arg)*), &_MSG_FILE_LINE_COL)
});
}

43
src/libcore/panicking.rs
View file

@ -17,7 +17,7 @@
//!
//! ```
//! # use std::fmt;
//! fn panic_impl(fmt: fmt::Arguments, file_line: &(&'static str, u32)) -> !
//! fn panic_impl(fmt: fmt::Arguments, file_line_col: &(&'static str, u32, u32)) -> !
//! # { loop {} }
//! ```
//!
@ -39,34 +39,55 @@
use fmt;
#[cold] #[inline(never)] // this is the slow path, always
#[lang = "panic"]
pub fn panic(expr_file_line: &(&'static str, &'static str, u32)) -> ! {
#[cfg_attr(not(stage0), lang = "panic")]
pub fn panic(expr_file_line_col: &(&'static str, &'static str, u32, u32)) -> ! {
// Use Arguments::new_v1 instead of format_args!("{}", expr) to potentially
// reduce size overhead. The format_args! macro uses str's Display trait to
// write expr, which calls Formatter::pad, which must accommodate string
// truncation and padding (even though none is used here). Using
// Arguments::new_v1 may allow the compiler to omit Formatter::pad from the
// output binary, saving up to a few kilobytes.
let (expr, file, line, col) = *expr_file_line_col;
panic_fmt(fmt::Arguments::new_v1(&[expr], &[]), &(file, line, col))
}
// FIXME: remove when SNAP
#[cold] #[inline(never)]
#[cfg(stage0)]
#[lang = "panic"]
pub fn panic_old(expr_file_line: &(&'static str, &'static str, u32)) -> ! {
let (expr, file, line) = *expr_file_line;
panic_fmt(fmt::Arguments::new_v1(&[expr], &[]), &(file, line))
let expr_file_line_col = (expr, file, line, 0);
panic(&expr_file_line_col)
}
#[cold] #[inline(never)]
#[lang = "panic_bounds_check"]
fn panic_bounds_check(file_line: &(&'static str, u32),
#[cfg_attr(not(stage0), lang = "panic_bounds_check")]
fn panic_bounds_check(file_line_col: &(&'static str, u32, u32),
index: usize, len: usize) -> ! {
panic_fmt(format_args!("index out of bounds: the len is {} but the index is {}",
len, index), file_line)
len, index), file_line_col)
}
// FIXME: remove when SNAP
#[cold] #[inline(never)]
#[cfg(stage0)]
#[lang = "panic_bounds_check"]
fn panic_bounds_check_old(file_line: &(&'static str, u32),
index: usize, len: usize) -> ! {
let (file, line) = *file_line;
panic_fmt(format_args!("index out of bounds: the len is {} but the index is {}",
len, index), &(file, line, 0))
}
#[cold] #[inline(never)]
pub fn panic_fmt(fmt: fmt::Arguments, file_line: &(&'static str, u32)) -> ! {
pub fn panic_fmt(fmt: fmt::Arguments, file_line_col: &(&'static str, u32, u32)) -> ! {
#[allow(improper_ctypes)]
extern {
#[lang = "panic_fmt"]
#[unwind]
fn panic_impl(fmt: fmt::Arguments, file: &'static str, line: u32) -> !;
fn panic_impl(fmt: fmt::Arguments, file: &'static str, line: u32, col: u32) -> !;
}
let (file, line) = *file_line;
unsafe { panic_impl(fmt, file, line) }
let (file, line, col) = *file_line_col;
unsafe { panic_impl(fmt, file, line, col) }
}

24
src/librustc_trans/mir/block.rs
View file

@ -28,6 +28,7 @@ use type_of;
use type_::Type;
use syntax::symbol::Symbol;
use syntax_pos::Pos;
use std::cmp;
@ -333,6 +334,7 @@ impl<'a, 'tcx> MirContext<'a, 'tcx> {
let filename = Symbol::intern(&loc.file.name).as_str();
let filename = C_str_slice(bcx.ccx, filename);
let line = C_u32(bcx.ccx, loc.line as u32);
let col = C_u32(bcx.ccx, loc.col.to_usize() as u32 + 1);
// Put together the arguments to the panic entry point.
let (lang_item, args, const_err) = match *msg {
@ -347,29 +349,29 @@ impl<'a, 'tcx> MirContext<'a, 'tcx> {
index: index as u64
}));
let file_line = C_struct(bcx.ccx, &[filename, line], false);
let align = llalign_of_min(bcx.ccx, common::val_ty(file_line));
let file_line = consts::addr_of(bcx.ccx,
file_line,
let file_line_col = C_struct(bcx.ccx, &[filename, line, col], false);
let align = llalign_of_min(bcx.ccx, common::val_ty(file_line_col));
let file_line_col = consts::addr_of(bcx.ccx,
file_line_col,
align,
"panic_bounds_check_loc");
(lang_items::PanicBoundsCheckFnLangItem,
vec![file_line, index, len],
vec![file_line_col, index, len],
const_err)
}
mir::AssertMessage::Math(ref err) => {
let msg_str = Symbol::intern(err.description()).as_str();
let msg_str = C_str_slice(bcx.ccx, msg_str);
let msg_file_line = C_struct(bcx.ccx,
&[msg_str, filename, line],
let msg_file_line_col = C_struct(bcx.ccx,
&[msg_str, filename, line, col],
false);
let align = llalign_of_min(bcx.ccx, common::val_ty(msg_file_line));
let msg_file_line = consts::addr_of(bcx.ccx,
msg_file_line,
let align = llalign_of_min(bcx.ccx, common::val_ty(msg_file_line_col));
let msg_file_line_col = consts::addr_of(bcx.ccx,
msg_file_line_col,
align,
"panic_loc");
(lang_items::PanicFnLangItem,
vec![msg_file_line],
vec![msg_file_line_col],
Some(ErrKind::Math(err.clone())))
}
};

10
src/libstd/macros.rs
View file

@ -41,10 +41,10 @@ macro_rules! panic {
panic!("explicit panic")
});
($msg:expr) => ({
$crate::rt::begin_panic($msg, {
$crate::rt::begin_panic_new($msg, {
// static requires less code at runtime, more constant data
static _FILE_LINE: (&'static str, u32) = (file!(), line!());
&_FILE_LINE
static _FILE_LINE_COL: (&'static str, u32, u32) = (file!(), line!(), column!());
&_FILE_LINE_COL
})
});
($fmt:expr, $($arg:tt)+) => ({
@ -53,8 +53,8 @@ macro_rules! panic {
// used inside a dead function. Just `#[allow(dead_code)]` is
// insufficient, since the user may have
// `#[forbid(dead_code)]` and which cannot be overridden.
static _FILE_LINE: (&'static str, u32) = (file!(), line!());
&_FILE_LINE
static _FILE_LINE_COL: (&'static str, u32, u32) = (file!(), line!(), column!());
&_FILE_LINE_COL
})
});
}

84
src/libstd/panicking.rs
View file

@ -262,6 +262,7 @@ impl<'a> PanicInfo<'a> {
pub struct Location<'a> {
file: &'a str,
line: u32,
col: u32,
}
impl<'a> Location<'a> {
@ -308,6 +309,29 @@ impl<'a> Location<'a> {
pub fn line(&self) -> u32 {
self.line
}
/// Returns the column from which the panic originated.
///
/// # Examples
///
/// ```should_panic
/// #![feature(panic_col)]
/// use std::panic;
///
/// panic::set_hook(Box::new(|panic_info| {
/// if let Some(location) = panic_info.location() {
/// println!("panic occured at column {}", location.column());
/// } else {
/// println!("panic occured but can't get location information...");
/// }
/// }));
///
/// panic!("Normal panic");
/// ```
#[unstable(feature = "panic_col", reason = "recently added", issue = "42939")]
pub fn column(&self) -> u32 {
self.col
}
}
fn default_hook(info: &PanicInfo) {
@ -329,6 +353,7 @@ fn default_hook(info: &PanicInfo) {
let file = info.location.file;
let line = info.location.line;
let col = info.location.col;
let msg = match info.payload.downcast_ref::<&'static str>() {
Some(s) => *s,
@ -342,8 +367,8 @@ fn default_hook(info: &PanicInfo) {
let name = thread.as_ref().and_then(|t| t.name()).unwrap_or("<unnamed>");
let write = |err: &mut ::io::Write| {
let _ = writeln!(err, "thread '{}' panicked at '{}', {}:{}",
name, msg, file, line);
let _ = writeln!(err, "thread '{}' panicked at '{}', {}:{}:{}",
name, msg, file, line, col);
#[cfg(feature = "backtrace")]
{
@ -467,8 +492,9 @@ pub fn panicking() -> bool {
#[unwind]
pub extern fn rust_begin_panic(msg: fmt::Arguments,
file: &'static str,
line: u32) -> ! {
begin_panic_fmt(&msg, &(file, line))
line: u32,
col: u32) -> ! {
begin_panic_fmt(&msg, &(file, line, col))
}
/// The entry point for panicking with a formatted message.
@ -482,7 +508,7 @@ pub extern fn rust_begin_panic(msg: fmt::Arguments,
issue = "0")]
#[inline(never)] #[cold]
pub fn begin_panic_fmt(msg: &fmt::Arguments,
file_line: &(&'static str, u32)) -> ! {
file_line_col: &(&'static str, u32, u32)) -> ! {
use fmt::Write;
// We do two allocations here, unfortunately. But (a) they're
@ -492,7 +518,39 @@ pub fn begin_panic_fmt(msg: &fmt::Arguments,
let mut s = String::new();
let _ = s.write_fmt(*msg);
begin_panic(s, file_line)
begin_panic_new(s, file_line_col)
}
// FIXME: In PR #42938, we have added the column as info passed to the panic
// handling code. For this, we want to break the ABI of begin_panic.
// This is not possible to do directly, as the stage0 compiler is hardcoded
// to emit a call to begin_panic in src/libsyntax/ext/build.rs, only
// with the file and line number being passed, but not the colum number.
// By changing the compiler source, we can only affect behaviour of higher
// stages. We need to perform the switch over two stage0 replacements, using
// a temporary function begin_panic_new while performing the switch:
// 0. Right now, we tell stage1 onward to emit a call to begin_panic_new.
// 1. In the first SNAP, stage0 calls begin_panic_new with the new ABI,
// begin_panic stops being used. Now we can change begin_panic to
// the new ABI, and start emitting calls to begin_panic in higher
// stages again, this time with the new ABI.
// 2. After the second SNAP, stage0 calls begin_panic with the new ABI,
// and we can remove the temporary begin_panic_new function.
/// This is the entry point of panicking for panic!() and assert!().
#[unstable(feature = "libstd_sys_internals",
reason = "used by the panic! macro",
issue = "0")]
#[inline(never)] #[cold] // avoid code bloat at the call sites as much as possible
pub fn begin_panic_new<M: Any + Send>(msg: M, file_line_col: &(&'static str, u32, u32)) -> ! {
// Note that this should be the only allocation performed in this code path.
// Currently this means that panic!() on OOM will invoke this code path,
// but then again we're not really ready for panic on OOM anyway. If
// we do start doing this, then we should propagate this allocation to
// be performed in the parent of this thread instead of the thread that's
// panicking.
rust_panic_with_hook(Box::new(msg), file_line_col)
}
/// This is the entry point of panicking for panic!() and assert!().
@ -508,7 +566,10 @@ pub fn begin_panic<M: Any + Send>(msg: M, file_line: &(&'static str, u32)) -> !
// be performed in the parent of this thread instead of the thread that's
// panicking.
rust_panic_with_hook(Box::new(msg), file_line)
let (file, line) = *file_line;
let file_line_col = (file, line, 0);
rust_panic_with_hook(Box::new(msg), &file_line_col)
}
/// Executes the primary logic for a panic, including checking for recursive
@ -520,8 +581,8 @@ pub fn begin_panic<M: Any + Send>(msg: M, file_line: &(&'static str, u32)) -> !
#[inline(never)]
#[cold]
fn rust_panic_with_hook(msg: Box<Any + Send>,
file_line: &(&'static str, u32)) -> ! {
let (file, line) = *file_line;
file_line_col: &(&'static str, u32, u32)) -> ! {
let (file, line, col) = *file_line_col;
let panics = update_panic_count(1);
@ -540,8 +601,9 @@ fn rust_panic_with_hook(msg: Box<Any + Send>,
let info = PanicInfo {
payload: &*msg,
location: Location {
file: file,
line: line,
file,
line,
col,
},
};
HOOK_LOCK.read();

2
src/libstd/rt.rs
View file

@ -25,7 +25,7 @@
// Reexport some of our utilities which are expected by other crates.
pub use panicking::{begin_panic, begin_panic_fmt, update_panic_count};
pub use panicking::{begin_panic_new, begin_panic, begin_panic_fmt, update_panic_count};
#[cfg(not(test))]
#[lang = "start"]

11
src/libsyntax/ext/build.rs
View file

@ -11,7 +11,7 @@
use abi::Abi;
use ast::{self, Ident, Generics, Expr, BlockCheckMode, UnOp, PatKind};
use attr;
use syntax_pos::{Span, DUMMY_SP};
use syntax_pos::{Pos, Span, DUMMY_SP};
use codemap::{dummy_spanned, respan, Spanned};
use ext::base::ExtCtxt;
use ptr::P;
@ -768,14 +768,15 @@ impl<'a> AstBuilder for ExtCtxt<'a> {
let loc = self.codemap().lookup_char_pos(span.lo);
let expr_file = self.expr_str(span, Symbol::intern(&loc.file.name));
let expr_line = self.expr_u32(span, loc.line as u32);
let expr_file_line_tuple = self.expr_tuple(span, vec![expr_file, expr_line]);
let expr_file_line_ptr = self.expr_addr_of(span, expr_file_line_tuple);
let expr_col = self.expr_u32(span, loc.col.to_usize() as u32 + 1);
let expr_loc_tuple = self.expr_tuple(span, vec![expr_file, expr_line, expr_col]);
let expr_loc_ptr = self.expr_addr_of(span, expr_loc_tuple);
self.expr_call_global(
span,
self.std_path(&["rt", "begin_panic"]),
self.std_path(&["rt", "begin_panic_new"]),
vec![
self.expr_str(span, msg),
expr_file_line_ptr])
expr_loc_ptr])
}
fn expr_unreachable(&self, span: Span) -> P<ast::Expr> {

2
src/tools/cargo

@ -1 +1 @@
Subproject commit d2d6e675fcb70cfb91c2cf2fad8cdd345c120316
Subproject commit eb6cf012a6cc23c9c89c4009564de9fccc38b9cb