From 1a9fc18e830dc57731e2976edb5c5f0fabc5019c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 9 Mar 2020 11:16:23 +0100 Subject: [PATCH 1/4] panic_bounds_check: use caller_location, like PanicFnLangItem --- src/libcore/panicking.rs | 18 ++++++++++++++++++ src/librustc_codegen_ssa/mir/block.rs | 6 +++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/libcore/panicking.rs b/src/libcore/panicking.rs index 61b764f2d6206..d58a7cfec951b 100644 --- a/src/libcore/panicking.rs +++ b/src/libcore/panicking.rs @@ -52,6 +52,24 @@ pub fn panic(expr: &str) -> ! { 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_fmt( + format_args!("index out of bounds: the len is {} but the index is {}", len, index), + Location::caller(), + ) +} + +// For bootstrap, we need a variant with the old argument order. +#[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 diff --git a/src/librustc_codegen_ssa/mir/block.rs b/src/librustc_codegen_ssa/mir/block.rs index a1b54607b809e..f22c0216a78d1 100644 --- a/src/librustc_codegen_ssa/mir/block.rs +++ b/src/librustc_codegen_ssa/mir/block.rs @@ -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]) } }; From 51b60b75e79e39dcbb1fa96d72f4cbb82df7f2ae Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 9 Mar 2020 16:56:45 +0100 Subject: [PATCH 2/4] Improve readability --- src/librustc_codegen_ssa/mir/block.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc_codegen_ssa/mir/block.rs b/src/librustc_codegen_ssa/mir/block.rs index f22c0216a78d1..6963f1d811a9b 100644 --- a/src/librustc_codegen_ssa/mir/block.rs +++ b/src/librustc_codegen_ssa/mir/block.rs @@ -415,8 +415,8 @@ 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(); - // It's `fn panic_bounds_check(index: usize, len: usize)`, and - // `#[track_caller]` adds an implicit third argument. + // 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]) } _ => { From f34e0664a1fa63f89f2257dba7d0d13445f58c02 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 9 Mar 2020 19:58:58 +0100 Subject: [PATCH 3/4] remove no-longer needed span from Miri Machine hook --- src/librustc_mir/const_eval/machine.rs | 1 - src/librustc_mir/interpret/machine.rs | 1 - src/librustc_mir/interpret/terminator.rs | 2 +- src/librustc_mir/transform/const_prop.rs | 1 - 4 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/librustc_mir/const_eval/machine.rs b/src/librustc_mir/const_eval/machine.rs index ed8029834680c..bb661d3d2a30a 100644 --- a/src/librustc_mir/const_eval/machine.rs +++ b/src/librustc_mir/const_eval/machine.rs @@ -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, ) -> InterpResult<'tcx> { diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index 6615cc608fb54..087517ff4e31d 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -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, ) -> InterpResult<'tcx>; diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs index ea8378574a3e0..b5c34daf8a318 100644 --- a/src/librustc_mir/interpret/terminator.rs +++ b/src/librustc_mir/interpret/terminator.rs @@ -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)?; } } diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index 0560f77f5c99f..4c353d1ae22e7 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -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, ) -> InterpResult<'tcx> { From 0b2329da9a9a61dc70f9e607628977302f8144b4 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 10 Mar 2020 10:31:03 +0100 Subject: [PATCH 4/4] also make panic_fmt track_caller --- src/libcore/macros/mod.rs | 21 +++++++++++++++++++++ src/libcore/panicking.rs | 22 +++++++++++++++------- 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/src/libcore/macros/mod.rs b/src/libcore/macros/mod.rs index 76e58f0cc62bc..04af5b5f76828 100644 --- a/src/libcore/macros/mod.rs +++ b/src/libcore/macros/mod.rs @@ -1,3 +1,4 @@ +#[cfg(bootstrap)] #[doc(include = "panic.md")] #[macro_export] #[allow_internal_unstable(core_panic, track_caller)] @@ -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 diff --git a/src/libcore/panicking.rs b/src/libcore/panicking.rs index d58a7cfec951b..3587f3f0ebf56 100644 --- a/src/libcore/panicking.rs +++ b/src/libcore/panicking.rs @@ -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 @@ -49,7 +50,10 @@ 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))] @@ -62,13 +66,11 @@ fn panic_bounds_check(index: usize, len: usize) -> ! { unsafe { super::intrinsics::abort() } } - panic_fmt( - format_args!("index out of bounds: the len is {} but the index is {}", len, index), - Location::caller(), - ) + 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. +// 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))] @@ -84,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() } } @@ -99,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) } }