From 3c38b3a4acbb57b145b2d10fdb1d762c80882c1d Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sun, 17 Mar 2024 22:26:39 -0400 Subject: [PATCH] Codegen const panic messages as function calls This skips emitting extra arguments at every callsite (of which there can be many). --- compiler/rustc_codegen_cranelift/src/base.rs | 24 +++---- compiler/rustc_codegen_ssa/src/mir/block.rs | 6 +- compiler/rustc_hir/src/lang_items.rs | 19 ++++++ compiler/rustc_middle/src/mir/terminator.rs | 69 +++++++++++++------- compiler/rustc_monomorphize/src/collector.rs | 21 +++--- compiler/rustc_span/src/symbol.rs | 18 +++++ library/core/src/panicking.rs | 51 +++++++++++++++ src/tools/miri/src/shims/panic.rs | 12 +++- 8 files changed, 166 insertions(+), 54 deletions(-) diff --git a/compiler/rustc_codegen_cranelift/src/base.rs b/compiler/rustc_codegen_cranelift/src/base.rs index 2415c2c90b22c..238ff8d931ae1 100644 --- a/compiler/rustc_codegen_cranelift/src/base.rs +++ b/compiler/rustc_codegen_cranelift/src/base.rs @@ -369,8 +369,14 @@ fn codegen_fn_body(fx: &mut FunctionCx<'_, '_, '_>, start_block: Block) { ); } _ => { - let msg_str = msg.description(); - codegen_panic(fx, msg_str, source_info); + let location = fx.get_caller_location(source_info).load_scalar(fx); + + codegen_panic_inner( + fx, + msg.description(), + &[location], + Some(source_info.span), + ); } } } @@ -954,20 +960,6 @@ pub(crate) fn codegen_operand<'tcx>( } } -pub(crate) fn codegen_panic<'tcx>( - fx: &mut FunctionCx<'_, '_, 'tcx>, - msg_str: &str, - source_info: mir::SourceInfo, -) { - let location = fx.get_caller_location(source_info).load_scalar(fx); - - let msg_ptr = fx.anonymous_str(msg_str); - let msg_len = fx.bcx.ins().iconst(fx.pointer_type, i64::try_from(msg_str.len()).unwrap()); - let args = [msg_ptr, msg_len, location]; - - codegen_panic_inner(fx, rustc_hir::LangItem::Panic, &args, Some(source_info.span)); -} - pub(crate) fn codegen_panic_nounwind<'tcx>( fx: &mut FunctionCx<'_, '_, 'tcx>, msg_str: &str, diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs index 9bb2a52826585..04c0dc5cec77d 100644 --- a/compiler/rustc_codegen_ssa/src/mir/block.rs +++ b/compiler/rustc_codegen_ssa/src/mir/block.rs @@ -651,10 +651,8 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { (LangItem::PanicMisalignedPointerDereference, vec![required, found, location]) } _ => { - let msg = bx.const_str(msg.description()); - // It's `pub fn panic(expr: &str)`, with the wide reference being passed - // as two arguments, and `#[track_caller]` adds an implicit third argument. - (LangItem::Panic, vec![msg.0, msg.1, location]) + // It's `pub fn panic_...()` and `#[track_caller]` adds an implicit argument. + (msg.description(), vec![location]) } }; diff --git a/compiler/rustc_hir/src/lang_items.rs b/compiler/rustc_hir/src/lang_items.rs index 5118bf5c3b7ab..f1a2b4d916401 100644 --- a/compiler/rustc_hir/src/lang_items.rs +++ b/compiler/rustc_hir/src/lang_items.rs @@ -246,6 +246,25 @@ language_item_table! { PanicImpl, sym::panic_impl, panic_impl, Target::Fn, GenericRequirement::None; PanicCannotUnwind, sym::panic_cannot_unwind, panic_cannot_unwind, Target::Fn, GenericRequirement::Exact(0); PanicInCleanup, sym::panic_in_cleanup, panic_in_cleanup, Target::Fn, GenericRequirement::Exact(0); + /// Constant panic messages, used for codegen of MIR asserts. + PanicAddOverflow, sym::panic_const_add_overflow, panic_const_add_overflow, Target::Fn, GenericRequirement::None; + PanicSubOverflow, sym::panic_const_sub_overflow, panic_const_sub_overflow, Target::Fn, GenericRequirement::None; + PanicMulOverflow, sym::panic_const_mul_overflow, panic_const_mul_overflow, Target::Fn, GenericRequirement::None; + PanicDivOverflow, sym::panic_const_div_overflow, panic_const_div_overflow, Target::Fn, GenericRequirement::None; + PanicRemOverflow, sym::panic_const_rem_overflow, panic_const_rem_overflow, Target::Fn, GenericRequirement::None; + PanicNegOverflow, sym::panic_const_neg_overflow, panic_const_neg_overflow, Target::Fn, GenericRequirement::None; + PanicShrOverflow, sym::panic_const_shr_overflow, panic_const_shr_overflow, Target::Fn, GenericRequirement::None; + PanicShlOverflow, sym::panic_const_shl_overflow, panic_const_shl_overflow, Target::Fn, GenericRequirement::None; + PanicDivZero, sym::panic_const_div_by_zero, panic_const_div_by_zero, Target::Fn, GenericRequirement::None; + PanicRemZero, sym::panic_const_rem_by_zero, panic_const_rem_by_zero, Target::Fn, GenericRequirement::None; + PanicCoroutineResumed, sym::panic_const_coroutine_resumed, panic_const_coroutine_resumed, Target::Fn, GenericRequirement::None; + PanicAsyncFnResumed, sym::panic_const_async_fn_resumed, panic_const_async_fn_resumed, Target::Fn, GenericRequirement::None; + PanicAsyncGenFnResumed, sym::panic_const_async_gen_fn_resumed, panic_const_async_gen_fn_resumed, Target::Fn, GenericRequirement::None; + PanicGenFnNone, sym::panic_const_gen_fn_none, panic_const_gen_fn_none, Target::Fn, GenericRequirement::None; + PanicCoroutineResumedPanic, sym::panic_const_coroutine_resumed_panic, panic_const_coroutine_resumed_panic, Target::Fn, GenericRequirement::None; + PanicAsyncFnResumedPanic, sym::panic_const_async_fn_resumed_panic, panic_const_async_fn_resumed_panic, Target::Fn, GenericRequirement::None; + PanicAsyncGenFnResumedPanic, sym::panic_const_async_gen_fn_resumed_panic, panic_const_async_gen_fn_resumed_panic, Target::Fn, GenericRequirement::None; + PanicGenFnNonePanic, sym::panic_const_gen_fn_none_panic, panic_const_gen_fn_none_panic, Target::Fn, GenericRequirement::None; /// libstd panic entry point. Necessary for const eval to be able to catch it BeginPanic, sym::begin_panic, begin_panic_fn, Target::Fn, GenericRequirement::None; diff --git a/compiler/rustc_middle/src/mir/terminator.rs b/compiler/rustc_middle/src/mir/terminator.rs index 94f8cba6fb578..05450beaf95c8 100644 --- a/compiler/rustc_middle/src/mir/terminator.rs +++ b/compiler/rustc_middle/src/mir/terminator.rs @@ -149,44 +149,45 @@ impl AssertKind { matches!(self, OverflowNeg(..) | Overflow(Add | Sub | Mul | Shl | Shr, ..)) } - /// Get the message that is printed at runtime when this assertion fails. + /// Get the lang item that is invoked to print a static message when this assert fires. /// /// The caller is expected to handle `BoundsCheck` and `MisalignedPointerDereference` by /// invoking the appropriate lang item (panic_bounds_check/panic_misaligned_pointer_dereference) - /// instead of printing a static message. - pub fn description(&self) -> &'static str { + /// instead of printing a static message. Those have dynamic arguments that aren't present for + /// the rest of the messages here. + pub fn description(&self) -> LangItem { use AssertKind::*; match self { - Overflow(BinOp::Add, _, _) => "attempt to add with overflow", - Overflow(BinOp::Sub, _, _) => "attempt to subtract with overflow", - Overflow(BinOp::Mul, _, _) => "attempt to multiply with overflow", - Overflow(BinOp::Div, _, _) => "attempt to divide with overflow", - Overflow(BinOp::Rem, _, _) => "attempt to calculate the remainder with overflow", - OverflowNeg(_) => "attempt to negate with overflow", - Overflow(BinOp::Shr, _, _) => "attempt to shift right with overflow", - Overflow(BinOp::Shl, _, _) => "attempt to shift left with overflow", + Overflow(BinOp::Add, _, _) => LangItem::PanicAddOverflow, + Overflow(BinOp::Sub, _, _) => LangItem::PanicSubOverflow, + Overflow(BinOp::Mul, _, _) => LangItem::PanicMulOverflow, + Overflow(BinOp::Div, _, _) => LangItem::PanicDivOverflow, + Overflow(BinOp::Rem, _, _) => LangItem::PanicRemOverflow, + OverflowNeg(_) => LangItem::PanicNegOverflow, + Overflow(BinOp::Shr, _, _) => LangItem::PanicShrOverflow, + Overflow(BinOp::Shl, _, _) => LangItem::PanicShlOverflow, Overflow(op, _, _) => bug!("{:?} cannot overflow", op), - DivisionByZero(_) => "attempt to divide by zero", - RemainderByZero(_) => "attempt to calculate the remainder with a divisor of zero", - ResumedAfterReturn(CoroutineKind::Coroutine(_)) => "coroutine resumed after completion", + DivisionByZero(_) => LangItem::PanicDivZero, + RemainderByZero(_) => LangItem::PanicRemZero, + ResumedAfterReturn(CoroutineKind::Coroutine(_)) => LangItem::PanicCoroutineResumed, ResumedAfterReturn(CoroutineKind::Desugared(CoroutineDesugaring::Async, _)) => { - "`async fn` resumed after completion" + LangItem::PanicAsyncFnResumed } ResumedAfterReturn(CoroutineKind::Desugared(CoroutineDesugaring::AsyncGen, _)) => { - "`async gen fn` resumed after completion" + LangItem::PanicAsyncGenFnResumed } ResumedAfterReturn(CoroutineKind::Desugared(CoroutineDesugaring::Gen, _)) => { - "`gen fn` should just keep returning `None` after completion" + LangItem::PanicGenFnNone } - ResumedAfterPanic(CoroutineKind::Coroutine(_)) => "coroutine resumed after panicking", + ResumedAfterPanic(CoroutineKind::Coroutine(_)) => LangItem::PanicCoroutineResumedPanic, ResumedAfterPanic(CoroutineKind::Desugared(CoroutineDesugaring::Async, _)) => { - "`async fn` resumed after panicking" + LangItem::PanicAsyncFnResumedPanic } ResumedAfterPanic(CoroutineKind::Desugared(CoroutineDesugaring::AsyncGen, _)) => { - "`async gen fn` resumed after panicking" + LangItem::PanicAsyncGenFnResumedPanic } ResumedAfterPanic(CoroutineKind::Desugared(CoroutineDesugaring::Gen, _)) => { - "`gen fn` should just keep returning `None` after panicking" + LangItem::PanicGenFnNonePanic } BoundsCheck { .. } | MisalignedPointerDereference { .. } => { @@ -246,13 +247,37 @@ impl AssertKind { Overflow(BinOp::Shl, _, r) => { write!(f, "\"attempt to shift left by `{{}}`, which would overflow\", {r:?}") } + Overflow(op, _, _) => bug!("{:?} cannot overflow", op), MisalignedPointerDereference { required, found } => { write!( f, "\"misaligned pointer dereference: address must be a multiple of {{}} but is {{}}\", {required:?}, {found:?}" ) } - _ => write!(f, "\"{}\"", self.description()), + ResumedAfterReturn(CoroutineKind::Coroutine(_)) => { + write!(f, "\"coroutine resumed after completion\"") + } + ResumedAfterReturn(CoroutineKind::Desugared(CoroutineDesugaring::Async, _)) => { + write!(f, "\"`async fn` resumed after completion\"") + } + ResumedAfterReturn(CoroutineKind::Desugared(CoroutineDesugaring::AsyncGen, _)) => { + write!(f, "\"`async gen fn` resumed after completion\"") + } + ResumedAfterReturn(CoroutineKind::Desugared(CoroutineDesugaring::Gen, _)) => { + write!(f, "\"`gen fn` should just keep returning `None` after completion\"") + } + ResumedAfterPanic(CoroutineKind::Coroutine(_)) => { + write!(f, "\"coroutine resumed after panicking\"") + } + ResumedAfterPanic(CoroutineKind::Desugared(CoroutineDesugaring::Async, _)) => { + write!(f, "\"`async fn` resumed after panicking\"") + } + ResumedAfterPanic(CoroutineKind::Desugared(CoroutineDesugaring::AsyncGen, _)) => { + write!(f, "\"`async gen fn` resumed after panicking\"") + } + ResumedAfterPanic(CoroutineKind::Desugared(CoroutineDesugaring::Gen, _)) => { + write!(f, "\"`gen fn` should just keep returning `None` after panicking\"") + } } } diff --git a/compiler/rustc_monomorphize/src/collector.rs b/compiler/rustc_monomorphize/src/collector.rs index cd9eb4916ce5d..a6ddc40001bc2 100644 --- a/compiler/rustc_monomorphize/src/collector.rs +++ b/compiler/rustc_monomorphize/src/collector.rs @@ -886,16 +886,17 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirUsedCollector<'a, 'tcx> { } } } - mir::TerminatorKind::Assert { ref msg, .. } => { - let lang_item = match &**msg { - mir::AssertKind::BoundsCheck { .. } => LangItem::PanicBoundsCheck, - mir::AssertKind::MisalignedPointerDereference { .. } => { - LangItem::PanicMisalignedPointerDereference - } - _ => LangItem::Panic, - }; - push_mono_lang_item(self, lang_item); - } + mir::TerminatorKind::Assert { ref msg, .. } => match &**msg { + mir::AssertKind::BoundsCheck { .. } => { + push_mono_lang_item(self, LangItem::PanicBoundsCheck); + } + mir::AssertKind::MisalignedPointerDereference { .. } => { + push_mono_lang_item(self, LangItem::PanicMisalignedPointerDereference); + } + _ => { + push_mono_lang_item(self, msg.description()); + } + }, mir::TerminatorKind::UnwindTerminate(reason) => { push_mono_lang_item(self, reason.lang_item()); } diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index e43c9533382e1..6d2829853397b 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -1297,6 +1297,24 @@ symbols! { panic_abort, panic_bounds_check, panic_cannot_unwind, + panic_const_add_overflow, + panic_const_async_fn_resumed, + panic_const_async_fn_resumed_panic, + panic_const_async_gen_fn_resumed, + panic_const_async_gen_fn_resumed_panic, + panic_const_coroutine_resumed, + panic_const_coroutine_resumed_panic, + panic_const_div_by_zero, + panic_const_div_overflow, + panic_const_gen_fn_none, + panic_const_gen_fn_none_panic, + panic_const_mul_overflow, + panic_const_neg_overflow, + panic_const_rem_by_zero, + panic_const_rem_overflow, + panic_const_shl_overflow, + panic_const_shr_overflow, + panic_const_sub_overflow, panic_fmt, panic_handler, panic_impl, diff --git a/library/core/src/panicking.rs b/library/core/src/panicking.rs index 9e1184c8f5b88..52457d142da15 100644 --- a/library/core/src/panicking.rs +++ b/library/core/src/panicking.rs @@ -145,6 +145,57 @@ pub const fn panic(expr: &'static str) -> ! { panic_fmt(fmt::Arguments::new_const(&[expr])); } +macro_rules! panic_const { + ($($lang:ident = $message:expr,)+) => { + #[cfg(not(bootstrap))] + pub mod panic_const { + use super::*; + + $( + /// This is a panic called with a message that's a result of a MIR-produced Assert. + // + // 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), cold)] + #[cfg_attr(feature = "panic_immediate_abort", inline)] + #[track_caller] + #[rustc_const_unstable(feature = "panic_internals", issue = "none")] + #[lang = stringify!($lang)] // needed by codegen for panic on overflow and other `Assert` MIR terminators + pub const fn $lang() -> ! { + // 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. + panic_fmt(fmt::Arguments::new_const(&[$message])); + } + )+ + } + } +} + +panic_const! { + panic_const_add_overflow = "attempt to add with overflow", + panic_const_sub_overflow = "attempt to subtract with overflow", + panic_const_mul_overflow = "attempt to multiply with overflow", + panic_const_div_overflow = "attempt to divide with overflow", + panic_const_rem_overflow = "attempt to calculate the remainder with overflow", + panic_const_neg_overflow = "attempt to negate with overflow", + panic_const_shr_overflow = "attempt to shift right with overflow", + panic_const_shl_overflow = "attempt to shift left with overflow", + panic_const_div_by_zero = "attempt to divide by zero", + panic_const_rem_by_zero = "attempt to calculate the remainder with a divisor of zero", + panic_const_coroutine_resumed = "coroutine resumed after completion", + panic_const_async_fn_resumed = "`async fn` resumed after completion", + panic_const_async_gen_fn_resumed = "`async gen fn` resumed after completion", + panic_const_gen_fn_none = "`gen fn` should just keep returning `None` after completion", + panic_const_coroutine_resumed_panic = "coroutine resumed after panicking", + panic_const_async_fn_resumed_panic = "`async fn` resumed after panicking", + panic_const_async_gen_fn_resumed_panic = "`async gen fn` resumed after panicking", + panic_const_gen_fn_none_panic = "`gen fn` should just keep returning `None` after panicking", +} + /// Like `panic`, but without unwinding and track_caller to reduce the impact on codesize on the caller. /// If you want `#[track_caller]` for nicer errors, call `panic_nounwind_fmt` directly. #[cfg_attr(not(feature = "panic_immediate_abort"), inline(never), cold)] diff --git a/src/tools/miri/src/shims/panic.rs b/src/tools/miri/src/shims/panic.rs index 34b6481dd387d..8cf1de955d3a4 100644 --- a/src/tools/miri/src/shims/panic.rs +++ b/src/tools/miri/src/shims/panic.rs @@ -256,8 +256,16 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } _ => { - // Forward everything else to `panic` lang item. - this.start_panic(msg.description(), unwind)?; + // Call the lang item associated with this message. + let fn_item = this.tcx.require_lang_item(msg.description(), None); + let instance = ty::Instance::mono(this.tcx.tcx, fn_item); + this.call_function( + instance, + Abi::Rust, + &[], + None, + StackPopCleanup::Goto { ret: None, unwind }, + )?; } } Ok(())