From aae58740067c8587e7f74bc65cdbd5b12da03356 Mon Sep 17 00:00:00 2001 From: Nilstrieb <48135649+Nilstrieb@users.noreply.github.com> Date: Sun, 24 Mar 2024 13:53:55 +0100 Subject: [PATCH] Handle panicking like rustc CTFE does Instead of using `core::fmt::format` to format panic messages, which may in turn panic too and cause recursive panics and other messy things, redirect `panic_fmt` to `const_panic_fmt` like CTFE, which in turn goes to `panic_display` and does the things normally. See the tests for the full call stack. --- crates/hir-ty/src/mir/eval.rs | 5 ++- crates/hir-ty/src/mir/eval/shim.rs | 64 ++++++++++++----------------- crates/hir-ty/src/mir/eval/tests.rs | 37 +++++++++++++++++ crates/test-utils/src/minicore.rs | 34 +++++++++++---- 4 files changed, 93 insertions(+), 47 deletions(-) diff --git a/crates/hir-ty/src/mir/eval.rs b/crates/hir-ty/src/mir/eval.rs index fd98141af63e..7799c039bda1 100644 --- a/crates/hir-ty/src/mir/eval.rs +++ b/crates/hir-ty/src/mir/eval.rs @@ -2317,7 +2317,7 @@ impl Evaluator<'_> { fn exec_fn_with_args( &mut self, - def: FunctionId, + mut def: FunctionId, args: &[IntervalAndTy], generic_args: Substitution, locals: &Locals, @@ -2335,6 +2335,9 @@ impl Evaluator<'_> { )? { return Ok(None); } + if let Some(redirect_def) = self.detect_and_redirect_special_function(def)? { + def = redirect_def; + } let arg_bytes = args.iter().map(|it| IntervalOrOwned::Borrowed(it.interval)); match self.get_mir_or_dyn_index(def, generic_args.clone(), locals, span)? { MirOrDynIndex::Dyn(self_ty_idx) => { diff --git a/crates/hir-ty/src/mir/eval/shim.rs b/crates/hir-ty/src/mir/eval/shim.rs index 628a1fe2d283..3a36656674fb 100644 --- a/crates/hir-ty/src/mir/eval/shim.rs +++ b/crates/hir-ty/src/mir/eval/shim.rs @@ -158,6 +158,26 @@ impl Evaluator<'_> { Ok(false) } + pub(super) fn detect_and_redirect_special_function( + &mut self, + def: FunctionId, + ) -> Result> { + // `PanicFmt` is redirected to `ConstPanicFmt` + if let Some(LangItem::PanicFmt) = self.db.lang_attr(def.into()) { + let resolver = + self.db.crate_def_map(self.crate_id).crate_root().resolver(self.db.upcast()); + + let Some(hir_def::lang_item::LangItemTarget::Function(const_panic_fmt)) = + self.db.lang_item(resolver.krate(), LangItem::ConstPanicFmt) + else { + not_supported!("const_panic_fmt lang item not found or not a function"); + }; + panic!("redirect"); + return Ok(Some(const_panic_fmt)); + } + Ok(None) + } + /// Clone has special impls for tuples and function pointers fn exec_clone( &mut self, @@ -291,9 +311,14 @@ impl Evaluator<'_> { use LangItem::*; let candidate = self.db.lang_attr(def.into())?; // We want to execute these functions with special logic - if [PanicFmt, BeginPanic, SliceLen, DropInPlace].contains(&candidate) { + // `PanicFmt` is not detected here as it's redirected later. + if [BeginPanic, SliceLen, DropInPlace].contains(&candidate) { return Some(candidate); } + if self.db.attrs(def.into()).by_key("rustc_const_panic_str").exists() { + // `#[rustc_const_panic_str]` is treated like `lang = "begin_panic"` by rustc CTFE. + return Some(LangItem::BeginPanic); + } None } @@ -309,43 +334,6 @@ impl Evaluator<'_> { let mut args = args.iter(); match it { BeginPanic => Err(MirEvalError::Panic("".to_owned())), - PanicFmt => { - let message = (|| { - let resolver = self - .db - .crate_def_map(self.crate_id) - .crate_root() - .resolver(self.db.upcast()); - let Some(format_fn) = resolver.resolve_path_in_value_ns_fully( - self.db.upcast(), - &hir_def::path::Path::from_known_path_with_no_generic( - ModPath::from_segments( - hir_expand::mod_path::PathKind::Abs, - [name![std], name![fmt], name![format]], - ), - ), - ) else { - not_supported!("std::fmt::format not found"); - }; - let hir_def::resolver::ValueNs::FunctionId(format_fn) = format_fn else { - not_supported!("std::fmt::format is not a function") - }; - let interval = self.interpret_mir( - self.db - .mir_body(format_fn.into()) - .map_err(|e| MirEvalError::MirLowerError(format_fn, e))?, - args.map(|x| IntervalOrOwned::Owned(x.clone())), - )?; - let message_string = interval.get(self)?; - let addr = - Address::from_bytes(&message_string[self.ptr_size()..2 * self.ptr_size()])?; - let size = from_bytes!(usize, message_string[2 * self.ptr_size()..]); - Ok(std::string::String::from_utf8_lossy(self.read_memory(addr, size)?) - .into_owned()) - })() - .unwrap_or_else(|e| format!("Failed to render panic format args: {e:?}")); - Err(MirEvalError::Panic(message)) - } SliceLen => { let arg = args.next().ok_or(MirEvalError::InternalError( "argument of <[T]>::len() is not provided".into(), diff --git a/crates/hir-ty/src/mir/eval/tests.rs b/crates/hir-ty/src/mir/eval/tests.rs index 381522c9abe6..031ab51ed7f5 100644 --- a/crates/hir-ty/src/mir/eval/tests.rs +++ b/crates/hir-ty/src/mir/eval/tests.rs @@ -31,6 +31,7 @@ fn eval_main(db: &TestDB, file_id: FileId) -> Result<(String, String), MirEvalEr db.trait_environment(func_id.into()), ) .map_err(|e| MirEvalError::MirLowerError(func_id, e))?; + let (result, output) = interpret_mir(db, body, false, None); result?; Ok((output.stdout().into_owned(), output.stderr().into_owned())) @@ -87,6 +88,42 @@ fn main() { ); } +#[test] +fn panic_fmt() { + // panic! + // -> panic_2021 (builtin macro redirection) + // -> #[lang = "panic_fmt"] core::panicking::panic_fmt (hooked by CTFE for redirection) + // -> core::panicking::const_panic_fmt + // -> #[rustc_const_panic_str] core::panicking::panic_display (hooked by CTFE for builtin panic) + // -> Err(ConstEvalError::Panic) + check_pass( + r#" +//- minicore: fmt, panic +fn main() { + panic!("hello, world!"); +} + "#, + ); + panic!("a"); +} + +#[test] +fn panic_display() { + // panic! + // -> panic_2021 (builtin macro redirection) + // -> #[rustc_const_panic_str] core::panicking::panic_display (hooked by CTFE for builtin panic) + // -> Err(ConstEvalError::Panic) + check_pass( + r#" +//- minicore: fmt, panic + +fn main() { + panic!("{}", "hello, world!"); +} + "#, + ); +} + #[test] fn drop_basic() { check_pass( diff --git a/crates/test-utils/src/minicore.rs b/crates/test-utils/src/minicore.rs index f125792d1258..15bee61381c8 100644 --- a/crates/test-utils/src/minicore.rs +++ b/crates/test-utils/src/minicore.rs @@ -1356,18 +1356,36 @@ pub mod iter { // region:panic mod panic { pub macro panic_2021 { - () => ( - $crate::panicking::panic("explicit panic") - ), - ($($t:tt)+) => ( - $crate::panicking::panic_fmt($crate::const_format_args!($($t)+)) - ), + () => ({ + const fn panic_cold_explicit() -> ! { + $crate::panicking::panic_explicit() + } + panic_cold_explicit(); + }), + // Special-case the single-argument case for const_panic. + ("{}", $arg:expr $(,)?) => ({ + #[rustc_const_panic_str] // enforce a &&str argument in const-check and hook this by const-eval + const fn panic_cold_display(arg: &T) -> ! { + loop {} + } + panic_cold_display(&$arg); + }), + ($($t:tt)+) => ({ + // Semicolon to prevent temporaries inside the formatting machinery from + // being considered alive in the caller after the panic_fmt call. + $crate::panicking::panic_fmt($crate::const_format_args!($($t)+)); + }), } } mod panicking { - #[lang = "panic_fmt"] - pub const fn panic_fmt(_fmt: crate::fmt::Arguments<'_>) -> ! { + #[rustc_const_panic_str] // enforce a &&str argument in const-check and hook this by const-eval + pub const fn panic_display(x: &T) -> ! { + panic_fmt(format_args!("{}", *x)); + } + + #[lang = "panic_fmt"] // needed for const-evaluated panics + pub const fn panic_fmt(fmt: fmt::Arguments<'_>) -> ! { loop {} }