Rollup merge of #67137 - anp:tracked-panic-internals, r=eddyb

Dylan-DPC · web-flow · commit cce055daef93 · 2020-01-04T23:52:44.000+05:30
libstd uses `core::panic::Location` where possible. cc @eddyb
diff --git a/src/libcore/macros/mod.rs b/src/libcore/macros/mod.rs
@@ -1,27 +1,21 @@
 #[doc(include = "panic.md")]
 #[macro_export]
-#[allow_internal_unstable(core_panic,
-    // FIXME(anp, eddyb) `core_intrinsics` is used here to allow calling
-    // the `caller_location` intrinsic, but once  `#[track_caller]` is implemented,
-    // `panicking::{panic, panic_fmt}` can use that instead of a `Location` argument.
-    core_intrinsics,
-    const_caller_location,
-)]
+#[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, $crate::intrinsics::caller_location())
+        $crate::panicking::panic($msg)
     );
     ($msg:expr,) => (
         $crate::panic!($msg)
     );
     ($fmt:expr, $($arg:tt)+) => (
         $crate::panicking::panic_fmt(
             $crate::format_args!($fmt, $($arg)+),
-            $crate::intrinsics::caller_location(),
+            $crate::panic::Location::caller(),
         )
     );
 }
diff --git a/src/libcore/panicking.rs b/src/libcore/panicking.rs
@@ -36,8 +36,9 @@ use crate::panic::{Location, PanicInfo};
 // never inline unless panic_immediate_abort to avoid code
 // bloat at the call sites as much as possible
 #[cfg_attr(not(feature = "panic_immediate_abort"), inline(never))]
+#[track_caller]
 #[lang = "panic"] // needed by codegen for panic on overflow and other `Assert` MIR terminators
-pub fn panic(expr: &str, location: &Location<'_>) -> ! {
+pub fn panic(expr: &str) -> ! {
     if cfg!(feature = "panic_immediate_abort") {
         unsafe { super::intrinsics::abort() }
     }
@@ -48,7 +49,7 @@ pub fn panic(expr: &str, location: &Location<'_>) -> ! {
     // 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)
+    panic_fmt(fmt::Arguments::new_v1(&[expr], &[]), Location::caller())
 }
 
 #[cold]
diff --git a/src/librustc_expand/build.rs b/src/librustc_expand/build.rs
@@ -6,7 +6,7 @@ use syntax::ast::{self, AttrVec, BlockCheckMode, Expr, Ident, PatKind, UnOp};
 use syntax::attr;
 use syntax::ptr::P;
 
-use rustc_span::{Pos, Span};
+use rustc_span::Span;
 
 impl<'a> ExtCtxt<'a> {
     pub fn path(&self, span: Span, strs: Vec<ast::Ident>) -> ast::Path {
@@ -350,16 +350,10 @@ impl<'a> ExtCtxt<'a> {
     }
 
     pub fn expr_fail(&self, span: Span, msg: Symbol) -> P<ast::Expr> {
-        let loc = self.source_map().lookup_char_pos(span.lo());
-        let expr_file = self.expr_str(span, Symbol::intern(&loc.file.name.to_string()));
-        let expr_line = self.expr_u32(span, loc.line as u32);
-        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,
             [sym::std, sym::rt, sym::begin_panic].iter().map(|s| Ident::new(*s, span)).collect(),
-            vec![self.expr_str(span, msg), expr_loc_ptr],
+            vec![self.expr_str(span, msg)],
         )
     }
 
diff --git a/src/librustc_mir/const_eval/machine.rs b/src/librustc_mir/const_eval/machine.rs
@@ -177,6 +177,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
 
     fn find_mir_or_eval_fn(
         ecx: &mut InterpCx<'mir, 'tcx, Self>,
+        span: Span,
         instance: ty::Instance<'tcx>,
         args: &[OpTy<'tcx>],
         ret: Option<(PlaceTy<'tcx>, mir::BasicBlock)>,
@@ -199,7 +200,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
                 // Some functions we support even if they are non-const -- but avoid testing
                 // that for const fn!  We certainly do *not* want to actually call the fn
                 // though, so be sure we return here.
-                return if ecx.hook_panic_fn(instance, args, ret)? {
+                return if ecx.hook_panic_fn(span, instance, args)? {
                     Ok(None)
                 } else {
                     throw_unsup_format!("calling non-const function `{}`", instance)
diff --git a/src/librustc_mir/interpret/intrinsics.rs b/src/librustc_mir/interpret/intrinsics.rs
@@ -366,47 +366,21 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
     /// Returns `true` if an intercept happened.
     pub fn hook_panic_fn(
         &mut self,
+        span: Span,
         instance: ty::Instance<'tcx>,
         args: &[OpTy<'tcx, M::PointerTag>],
-        _ret: Option<(PlaceTy<'tcx, M::PointerTag>, mir::BasicBlock)>,
     ) -> InterpResult<'tcx, bool> {
         let def_id = instance.def_id();
-        if Some(def_id) == self.tcx.lang_items().panic_fn() {
-            // &'static str, &core::panic::Location { &'static str, u32, u32 }
-            assert!(args.len() == 2);
+        if Some(def_id) == self.tcx.lang_items().panic_fn()
+            || Some(def_id) == self.tcx.lang_items().begin_panic_fn()
+        {
+            // &'static str
+            assert!(args.len() == 1);
 
             let msg_place = self.deref_operand(args[0])?;
             let msg = Symbol::intern(self.read_str(msg_place)?);
-
-            let location = self.deref_operand(args[1])?;
-            let (file, line, col) = (
-                self.mplace_field(location, 0)?,
-                self.mplace_field(location, 1)?,
-                self.mplace_field(location, 2)?,
-            );
-
-            let file_place = self.deref_operand(file.into())?;
-            let file = Symbol::intern(self.read_str(file_place)?);
-            let line = self.read_scalar(line.into())?.to_u32()?;
-            let col = self.read_scalar(col.into())?.to_u32()?;
-            throw_panic!(Panic { msg, file, line, col })
-        } else if Some(def_id) == self.tcx.lang_items().begin_panic_fn() {
-            assert!(args.len() == 2);
-            // &'static str, &(&'static str, u32, u32)
-            let msg = args[0];
-            let place = self.deref_operand(args[1])?;
-            let (file, line, col) = (
-                self.mplace_field(place, 0)?,
-                self.mplace_field(place, 1)?,
-                self.mplace_field(place, 2)?,
-            );
-
-            let msg_place = self.deref_operand(msg.into())?;
-            let msg = Symbol::intern(self.read_str(msg_place)?);
-            let file_place = self.deref_operand(file.into())?;
-            let file = Symbol::intern(self.read_str(file_place)?);
-            let line = self.read_scalar(line.into())?.to_u32()?;
-            let col = self.read_scalar(col.into())?.to_u32()?;
+            let span = self.find_closest_untracked_caller_location().unwrap_or(span);
+            let (file, line, col) = self.location_triple_for_span(span);
             throw_panic!(Panic { msg, file, line, col })
         } else {
             return Ok(false);
diff --git a/src/librustc_mir/interpret/intrinsics/caller_location.rs b/src/librustc_mir/interpret/intrinsics/caller_location.rs
@@ -9,8 +9,9 @@ use crate::interpret::{
 };
 
 impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
-    /// Walks up the callstack from the intrinsic's callsite, searching for the first frame which is
-    /// not `#[track_caller]`.
+    /// Walks up the callstack from the intrinsic's callsite, searching for the first callsite in a
+    /// frame which is not `#[track_caller]`. If the first frame found lacks `#[track_caller]`, then
+    /// `None` is returned and the callsite of the function invocation itself should be used.
     crate fn find_closest_untracked_caller_location(&self) -> Option<Span> {
         let mut caller_span = None;
         for next_caller in self.stack.iter().rev() {
@@ -54,9 +55,14 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
     }
 
     pub fn alloc_caller_location_for_span(&mut self, span: Span) -> MPlaceTy<'tcx, M::PointerTag> {
+        let (file, line, column) = self.location_triple_for_span(span);
+        self.alloc_caller_location(file, line, column)
+    }
+
+    pub(super) fn location_triple_for_span(&self, span: Span) -> (Symbol, u32, u32) {
         let topmost = span.ctxt().outer_expn().expansion_cause().unwrap_or(span);
         let caller = self.tcx.sess.source_map().lookup_char_pos(topmost.lo());
-        self.alloc_caller_location(
+        (
             Symbol::intern(&caller.file.name.to_string()),
             caller.line as u32,
             caller.col_display as u32 + 1,
diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs
@@ -139,6 +139,7 @@ pub trait Machine<'mir, 'tcx>: Sized {
     /// was used.
     fn find_mir_or_eval_fn(
         ecx: &mut InterpCx<'mir, 'tcx, Self>,
+        span: Span,
         instance: ty::Instance<'tcx>,
         args: &[OpTy<'tcx, Self::PointerTag>],
         ret: Option<(PlaceTy<'tcx, Self::PointerTag>, mir::BasicBlock)>,
diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs
@@ -238,7 +238,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             | ty::InstanceDef::CloneShim(..)
             | ty::InstanceDef::Item(_) => {
                 // We need MIR for this fn
-                let body = match M::find_mir_or_eval_fn(self, instance, args, ret, unwind)? {
+                let body = match M::find_mir_or_eval_fn(self, span, instance, args, ret, unwind)? {
                     Some(body) => body,
                     None => return Ok(()),
                 };
diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs
@@ -125,6 +125,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine {
 
     fn find_mir_or_eval_fn(
         _ecx: &mut InterpCx<'mir, 'tcx, Self>,
+        _span: Span,
         _instance: ty::Instance<'tcx>,
         _args: &[OpTy<'tcx>],
         _ret: Option<(PlaceTy<'tcx>, BasicBlock)>,
diff --git a/src/libstd/lib.rs b/src/libstd/lib.rs
@@ -305,6 +305,7 @@
 #![feature(thread_local)]
 #![feature(toowned_clone_into)]
 #![feature(trace_macros)]
+#![feature(track_caller)]
 #![feature(try_reserve)]
 #![feature(unboxed_closures)]
 #![feature(untagged_unions)]
diff --git a/src/libstd/macros.rs b/src/libstd/macros.rs
@@ -4,6 +4,7 @@
 //! library. Each macro is available for use when linking against the standard
 //! library.
 
+#[cfg(bootstrap)]
 #[doc(include = "../libcore/macros/panic.md")]
 #[macro_export]
 #[stable(feature = "rust1", since = "1.0.0")]
@@ -19,8 +20,21 @@ macro_rules! panic {
         $crate::panic!($msg)
     });
     ($fmt:expr, $($arg:tt)+) => ({
-        $crate::rt::begin_panic_fmt(&$crate::format_args!($fmt, $($arg)+),
-                                    &($crate::file!(), $crate::line!(), $crate::column!()))
+        $crate::rt::begin_panic_fmt(&$crate::format_args!($fmt, $($arg)+))
+    });
+}
+
+#[cfg(not(bootstrap))]
+#[doc(include = "../libcore/macros/panic.md")]
+#[macro_export]
+#[stable(feature = "rust1", since = "1.0.0")]
+#[allow_internal_unstable(libstd_sys_internals)]
+macro_rules! panic {
+    () => ({ $crate::panic!("explicit panic") });
+    ($msg:expr) => ({ $crate::rt::begin_panic($msg) });
+    ($msg:expr,) => ({ $crate::panic!($msg) });
+    ($fmt:expr, $($arg:tt)+) => ({
+        $crate::rt::begin_panic_fmt(&$crate::format_args!($fmt, $($arg)+))
     });
 }
 
diff --git a/src/libstd/panicking.rs b/src/libstd/panicking.rs
@@ -313,17 +313,15 @@ pub fn panicking() -> bool {
 #[cold]
 // If panic_immediate_abort, inline the abort call,
 // otherwise avoid inlining because of it is cold path.
+#[cfg_attr(not(feature = "panic_immediate_abort"), track_caller)]
 #[cfg_attr(not(feature = "panic_immediate_abort"), inline(never))]
 #[cfg_attr(feature = "panic_immediate_abort", inline)]
-pub fn begin_panic_fmt(msg: &fmt::Arguments<'_>, file_line_col: &(&'static str, u32, u32)) -> ! {
+pub fn begin_panic_fmt(msg: &fmt::Arguments<'_>) -> ! {
     if cfg!(feature = "panic_immediate_abort") {
         unsafe { intrinsics::abort() }
     }
 
-    // Just package everything into a `PanicInfo` and continue like libcore panics.
-    let (file, line, col) = *file_line_col;
-    let location = Location::internal_constructor(file, line, col);
-    let info = PanicInfo::internal_constructor(Some(msg), &location);
+    let info = PanicInfo::internal_constructor(Some(msg), Location::caller());
     begin_panic_handler(&info)
 }
 
@@ -356,6 +354,9 @@ pub fn begin_panic_handler(info: &PanicInfo<'_>) -> ! {
 
     unsafe impl<'a> BoxMeUp for PanicPayload<'a> {
         fn take_box(&mut self) -> *mut (dyn Any + Send) {
+            // We do two allocations here, unfortunately. But (a) they're required with the current
+            // scheme, and (b) we don't handle panic + OOM properly anyway (see comment in
+            // begin_panic below).
             let contents = mem::take(self.fill());
             Box::into_raw(Box::new(contents))
         }
@@ -365,15 +366,9 @@ pub fn begin_panic_handler(info: &PanicInfo<'_>) -> ! {
         }
     }
 
-    // We do two allocations here, unfortunately. But (a) they're
-    // required with the current scheme, and (b) we don't handle
-    // panic + OOM properly anyway (see comment in begin_panic
-    // below).
-
     let loc = info.location().unwrap(); // The current implementation always returns Some
     let msg = info.message().unwrap(); // The current implementation always returns Some
-    let file_line_col = (loc.file(), loc.line(), loc.column());
-    rust_panic_with_hook(&mut PanicPayload::new(msg), info.message(), &file_line_col);
+    rust_panic_with_hook(&mut PanicPayload::new(msg), info.message(), loc);
 }
 
 /// This is the entry point of panicking for the non-format-string variants of
@@ -386,19 +381,13 @@ pub fn begin_panic_handler(info: &PanicInfo<'_>) -> ! {
 // bloat at the call sites as much as possible
 #[cfg_attr(not(feature = "panic_immediate_abort"), inline(never))]
 #[cold]
-pub fn begin_panic<M: Any + Send>(msg: M, file_line_col: &(&'static str, u32, u32)) -> ! {
+#[track_caller]
+pub fn begin_panic<M: Any + Send>(msg: M, #[cfg(bootstrap)] _: &(&str, u32, u32)) -> ! {
     if cfg!(feature = "panic_immediate_abort") {
         unsafe { intrinsics::abort() }
     }
 
-    // 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(&mut PanicPayload::new(msg), None, file_line_col);
+    rust_panic_with_hook(&mut PanicPayload::new(msg), None, Location::caller());
 
     struct PanicPayload<A> {
         inner: Option<A>,
@@ -412,6 +401,11 @@ pub fn begin_panic<M: Any + Send>(msg: M, file_line_col: &(&'static str, u32, u3
 
     unsafe impl<A: Send + 'static> BoxMeUp for PanicPayload<A> {
         fn take_box(&mut self) -> *mut (dyn Any + Send) {
+            // 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.
             let data = match self.inner.take() {
                 Some(a) => Box::new(a) as Box<dyn Any + Send>,
                 None => process::abort(),
@@ -436,10 +430,8 @@ pub fn begin_panic<M: Any + Send>(msg: M, file_line_col: &(&'static str, u32, u3
 fn rust_panic_with_hook(
     payload: &mut dyn BoxMeUp,
     message: Option<&fmt::Arguments<'_>>,
-    file_line_col: &(&str, u32, u32),
+    location: &Location<'_>,
 ) -> ! {
-    let (file, line, col) = *file_line_col;
-
     let panics = update_panic_count(1);
 
     // If this is the third nested call (e.g., panics == 2, this is 0-indexed),
@@ -456,8 +448,7 @@ fn rust_panic_with_hook(
     }
 
     unsafe {
-        let location = Location::internal_constructor(file, line, col);
-        let mut info = PanicInfo::internal_constructor(message, &location);
+        let mut info = PanicInfo::internal_constructor(message, location);
         HOOK_LOCK.read();
         match HOOK {
             // Some platforms (like wasm) know that printing to stderr won't ever actually
diff --git a/src/test/mir-opt/retain-never-const.rs b/src/test/mir-opt/retain-never-const.rs
@@ -6,6 +6,7 @@
 
 #![feature(const_panic)]
 #![feature(never_type)]
+#![warn(const_err)]
 
 struct PrintName<T>(T);
 
diff --git a/src/test/ui/extern/issue-64655-allow-unwind-when-calling-panic-directly.rs b/src/test/ui/extern/issue-64655-allow-unwind-when-calling-panic-directly.rs
@@ -22,7 +22,7 @@
 //[thin]compile-flags: -C lto=thin
 //[fat]compile-flags: -C lto=fat
 
-#![feature(core_panic, panic_internals)]
+#![feature(core_panic)]
 
 // (For some reason, reproducing the LTO issue requires pulling in std
 // explicitly this way.)
@@ -50,9 +50,7 @@ fn main() {
         }
 
         let _guard = Droppable;
-        let s = "issue-64655-allow-unwind-when-calling-panic-directly.rs";
-        let location = core::panic::Location::internal_constructor(s, 17, 4);
-        core::panicking::panic("???", &location);
+        core::panicking::panic("???");
     });
 
     let wait = handle.join();
diff --git a/src/test/ui/rfc-2091-track-caller/caller-location-intrinsic.rs b/src/test/ui/rfc-2091-track-caller/caller-location-intrinsic.rs
@@ -4,16 +4,16 @@
 
 #[inline(never)]
 #[track_caller]
-fn defeat_const_prop() -> &'static core::panic::Location<'static> {
+fn codegen_caller_loc() -> &'static core::panic::Location<'static> {
     core::panic::Location::caller()
 }
 
 macro_rules! caller_location_from_macro {
-    () => (defeat_const_prop());
+    () => (codegen_caller_loc());
 }
 
 fn main() {
-    let loc = defeat_const_prop();
+    let loc = codegen_caller_loc();
     assert_eq!(loc.file(), file!());
     assert_eq!(loc.line(), 16);
     assert_eq!(loc.column(), 15);
diff --git a/src/test/ui/rfc-2091-track-caller/diverging-caller-location.rs b/src/test/ui/rfc-2091-track-caller/diverging-caller-location.rs
diff --git a/src/test/ui/rfc-2091-track-caller/track-caller-attribute.rs b/src/test/ui/rfc-2091-track-caller/track-caller-attribute.rs
