Skip to content

Commit

Permalink
Rollup merge of #69850 - RalfJung:panic-bounds-check, r=eddyb
Browse files Browse the repository at this point in the history
panic_bounds_check: use caller_location, like PanicFnLangItem

The `PanicFnLangItem` got switched to using `#[caller_location]` at some point, but `PanicBoundsCheckFnLangItem` was kept in the old style. For consistency, switch that one over to use `#[caller_location]` as well.

This is also helpful for Miri as it means the `assert_panic` machine hook never needs to know the current `Span`.
  • Loading branch information
Centril authored Mar 11, 2020
2 parents 080d413 + 0b2329d commit 3853da7
Show file tree
Hide file tree
Showing 7 changed files with 55 additions and 7 deletions.
21 changes: 21 additions & 0 deletions src/libcore/macros/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#[cfg(bootstrap)]
#[doc(include = "panic.md")]
#[macro_export]
#[allow_internal_unstable(core_panic, track_caller)]
Expand All @@ -20,6 +21,26 @@ macro_rules! panic {
);
}

#[cfg(not(bootstrap))]
#[doc(include = "panic.md")]
#[macro_export]
#[allow_internal_unstable(core_panic, track_caller)]
#[stable(feature = "core", since = "1.6.0")]
macro_rules! panic {
() => (
$crate::panic!("explicit panic")
);
($msg:expr) => (
$crate::panicking::panic($msg)
);
($msg:expr,) => (
$crate::panic!($msg)
);
($fmt:expr, $($arg:tt)+) => (
$crate::panicking::panic_fmt($crate::format_args!($fmt, $($arg)+))
);
}

/// Asserts that two expressions are equal to each other (using [`PartialEq`]).
///
/// On panic, this macro will print the values of the expressions with their
Expand Down
30 changes: 28 additions & 2 deletions src/libcore/panicking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
use crate::fmt;
use crate::panic::{Location, PanicInfo};

/// The underlying implementation of libcore's `panic!` macro when no formatting is used.
#[cold]
// never inline unless panic_immediate_abort to avoid code
// bloat at the call sites as much as possible
Expand All @@ -49,9 +50,28 @@ pub fn panic(expr: &str) -> ! {
// 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.
panic_fmt(fmt::Arguments::new_v1(&[expr], &[]), Location::caller())
#[cfg(not(bootstrap))]
panic_fmt(fmt::Arguments::new_v1(&[expr], &[]));
#[cfg(bootstrap)]
panic_fmt(fmt::Arguments::new_v1(&[expr], &[]), Location::caller());
}

#[cfg(not(bootstrap))]
#[cold]
#[cfg_attr(not(feature = "panic_immediate_abort"), inline(never))]
#[track_caller]
#[lang = "panic_bounds_check"] // needed by codegen for panic on OOB array/slice access
fn panic_bounds_check(index: usize, len: usize) -> ! {
if cfg!(feature = "panic_immediate_abort") {
unsafe { super::intrinsics::abort() }
}

panic!("index out of bounds: the len is {} but the index is {}", len, index)
}

// For bootstrap, we need a variant with the old argument order, and a corresponding
// `panic_fmt`.
#[cfg(bootstrap)]
#[cold]
#[cfg_attr(not(feature = "panic_immediate_abort"), inline(never))]
#[lang = "panic_bounds_check"] // needed by codegen for panic on OOB array/slice access
Expand All @@ -66,10 +86,12 @@ fn panic_bounds_check(location: &Location<'_>, index: usize, len: usize) -> ! {
)
}

/// The underlying implementation of libcore's `panic!` macro when formatting is used.
#[cold]
#[cfg_attr(not(feature = "panic_immediate_abort"), inline(never))]
#[cfg_attr(feature = "panic_immediate_abort", inline)]
pub fn panic_fmt(fmt: fmt::Arguments<'_>, location: &Location<'_>) -> ! {
#[cfg_attr(not(bootstrap), track_caller)]
pub fn panic_fmt(fmt: fmt::Arguments<'_>, #[cfg(bootstrap)] location: &Location<'_>) -> ! {
if cfg!(feature = "panic_immediate_abort") {
unsafe { super::intrinsics::abort() }
}
Expand All @@ -81,6 +103,10 @@ pub fn panic_fmt(fmt: fmt::Arguments<'_>, location: &Location<'_>) -> ! {
fn panic_impl(pi: &PanicInfo<'_>) -> !;
}

#[cfg(bootstrap)]
let pi = PanicInfo::internal_constructor(Some(&fmt), location);
#[cfg(not(bootstrap))]
let pi = PanicInfo::internal_constructor(Some(&fmt), Location::caller());

unsafe { panic_impl(&pi) }
}
6 changes: 5 additions & 1 deletion src/librustc_codegen_ssa/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -415,11 +415,15 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
AssertKind::BoundsCheck { ref len, ref index } => {
let len = self.codegen_operand(&mut bx, len).immediate();
let index = self.codegen_operand(&mut bx, index).immediate();
(lang_items::PanicBoundsCheckFnLangItem, vec![location, index, len])
// It's `fn panic_bounds_check(index: usize, len: usize)`,
// and `#[track_caller]` adds an implicit third argument.
(lang_items::PanicBoundsCheckFnLangItem, vec![index, len, location])
}
_ => {
let msg_str = Symbol::intern(msg.description());
let msg = bx.const_str(msg_str);
// It's `pub fn panic(expr: &str)`, with the wide reference being passed
// as two arguments, and `#[track_caller]` adds an implicit third argument.
(lang_items::PanicFnLangItem, vec![msg.0, msg.1, location])
}
};
Expand Down
1 change: 0 additions & 1 deletion src/librustc_mir/const_eval/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,6 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,

fn assert_panic(
ecx: &mut InterpCx<'mir, 'tcx, Self>,
_span: Span,
msg: &AssertMessage<'tcx>,
_unwind: Option<mir::BasicBlock>,
) -> InterpResult<'tcx> {
Expand Down
1 change: 0 additions & 1 deletion src/librustc_mir/interpret/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,6 @@ pub trait Machine<'mir, 'tcx>: Sized {
/// Called to evaluate `Assert` MIR terminators that trigger a panic.
fn assert_panic(
ecx: &mut InterpCx<'mir, 'tcx, Self>,
span: Span,
msg: &mir::AssertMessage<'tcx>,
unwind: Option<mir::BasicBlock>,
) -> InterpResult<'tcx>;
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/interpret/terminator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
if expected == cond_val {
self.go_to_block(target);
} else {
M::assert_panic(self, terminator.source_info.span, msg, cleanup)?;
M::assert_panic(self, msg, cleanup)?;
}
}

Expand Down
1 change: 0 additions & 1 deletion src/librustc_mir/transform/const_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,6 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine {

fn assert_panic(
_ecx: &mut InterpCx<'mir, 'tcx, Self>,
_span: Span,
_msg: &rustc::mir::AssertMessage<'tcx>,
_unwind: Option<rustc::mir::BasicBlock>,
) -> InterpResult<'tcx> {
Expand Down

0 comments on commit 3853da7

Please sign in to comment.