From 9f241b3a26b9344e061094052ecd67a3bde4b2e1 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 21 Dec 2022 13:49:48 +0100 Subject: [PATCH] abort immediately on bad mem::zeroed/uninit --- compiler/rustc_codegen_ssa/src/mir/block.rs | 9 ++-- compiler/rustc_hir/src/lang_items.rs | 5 +- compiler/rustc_monomorphize/src/collector.rs | 2 +- compiler/rustc_span/src/symbol.rs | 3 +- library/core/src/intrinsics.rs | 2 +- library/core/src/panicking.rs | 14 +++--- .../unwind-abis/c-unwind-abi-panic-abort.rs | 2 +- src/test/codegen/unwind-and-panic-abort.rs | 2 +- .../intrinsics/panic-uninitialized-zeroed.rs | 50 +++++++++++++------ 9 files changed, 55 insertions(+), 34 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs index cff72048ead08..978aff511bfa7 100644 --- a/compiler/rustc_codegen_ssa/src/mir/block.rs +++ b/compiler/rustc_codegen_ssa/src/mir/block.rs @@ -637,7 +637,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { self.set_debug_loc(bx, terminator.source_info); // Obtain the panic entry point. - let (fn_abi, llfn) = common::build_langcall(bx, Some(span), LangItem::PanicNoUnwind); + let (fn_abi, llfn) = common::build_langcall(bx, Some(span), LangItem::PanicCannotUnwind); // Codegen the actual panic invoke/call. let merging_succ = helper.do_call(self, bx, fn_abi, llfn, &[], None, None, &[], false); @@ -698,11 +698,10 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { }) }); let msg = bx.const_str(&msg_str); - let location = self.get_caller_location(bx, source_info).immediate(); // Obtain the panic entry point. let (fn_abi, llfn) = - common::build_langcall(bx, Some(source_info.span), LangItem::Panic); + common::build_langcall(bx, Some(source_info.span), LangItem::PanicNounwind); // Codegen the actual panic invoke/call. helper.do_call( @@ -710,7 +709,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { bx, fn_abi, llfn, - &[msg.0, msg.1, location], + &[msg.0, msg.1], target.as_ref().map(|bb| (ReturnDest::Nothing, *bb)), cleanup, &[], @@ -1665,7 +1664,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { let llpersonality = self.cx.eh_personality(); bx.cleanup_landing_pad(llpersonality); - let (fn_abi, fn_ptr) = common::build_langcall(&bx, None, LangItem::PanicNoUnwind); + let (fn_abi, fn_ptr) = common::build_langcall(&bx, None, LangItem::PanicCannotUnwind); let fn_ty = bx.fn_decl_backend_type(&fn_abi); let llret = bx.call(fn_ty, Some(&fn_abi), fn_ptr, &[], None); diff --git a/compiler/rustc_hir/src/lang_items.rs b/compiler/rustc_hir/src/lang_items.rs index 038509031b180..3474fab34f00b 100644 --- a/compiler/rustc_hir/src/lang_items.rs +++ b/compiler/rustc_hir/src/lang_items.rs @@ -146,7 +146,7 @@ pub fn extract(attrs: &[ast::Attribute]) -> Option<(Symbol, Span)> { } language_item_table! { -// Variant name, Name, Method name, Target Generic requirements; +// Variant name, Name, Getter method name, Target Generic requirements; Sized, sym::sized, sized_trait, Target::Trait, GenericRequirement::Exact(0); Unsize, sym::unsize, unsize_trait, Target::Trait, GenericRequirement::Minimum(1); /// Trait injected by `#[derive(PartialEq)]`, (i.e. "Partial EQ"). @@ -232,6 +232,7 @@ language_item_table! { // is required to define it somewhere. Additionally, there are restrictions on crates that use // a weak lang item, but do not have it defined. Panic, sym::panic, panic_fn, Target::Fn, GenericRequirement::Exact(0); + PanicNounwind, sym::panic_nounwind, panic_nounwind, Target::Fn, GenericRequirement::Exact(0); PanicFmt, sym::panic_fmt, panic_fmt, Target::Fn, GenericRequirement::None; PanicDisplay, sym::panic_display, panic_display, Target::Fn, GenericRequirement::None; ConstPanicFmt, sym::const_panic_fmt, const_panic_fmt, Target::Fn, GenericRequirement::None; @@ -239,7 +240,7 @@ language_item_table! { PanicInfo, sym::panic_info, panic_info, Target::Struct, GenericRequirement::None; PanicLocation, sym::panic_location, panic_location, Target::Struct, GenericRequirement::None; PanicImpl, sym::panic_impl, panic_impl, Target::Fn, GenericRequirement::None; - PanicNoUnwind, sym::panic_no_unwind, panic_no_unwind, Target::Fn, GenericRequirement::Exact(0); + PanicCannotUnwind, sym::panic_cannot_unwind, panic_cannot_unwind, Target::Fn, GenericRequirement::Exact(0); /// 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_monomorphize/src/collector.rs b/compiler/rustc_monomorphize/src/collector.rs index deeeb9af4eb92..59cc500a99da7 100644 --- a/compiler/rustc_monomorphize/src/collector.rs +++ b/compiler/rustc_monomorphize/src/collector.rs @@ -843,7 +843,7 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirNeighborCollector<'a, 'tcx> { mir::TerminatorKind::Abort { .. } => { let instance = Instance::mono( tcx, - tcx.require_lang_item(LangItem::PanicNoUnwind, Some(source)), + tcx.require_lang_item(LangItem::PanicCannotUnwind, Some(source)), ); if should_codegen_locally(tcx, &instance) { self.output.push(create_fn_mono_item(tcx, instance, source)); diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index e802663471341..90f654c68ecbf 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -1041,6 +1041,7 @@ symbols! { panic_2021, panic_abort, panic_bounds_check, + panic_cannot_unwind, panic_display, panic_fmt, panic_handler, @@ -1048,7 +1049,7 @@ symbols! { panic_implementation, panic_info, panic_location, - panic_no_unwind, + panic_nounwind, panic_runtime, panic_str, panic_unwind, diff --git a/library/core/src/intrinsics.rs b/library/core/src/intrinsics.rs index a521905a9e735..6eeaef5ab0129 100644 --- a/library/core/src/intrinsics.rs +++ b/library/core/src/intrinsics.rs @@ -2281,7 +2281,7 @@ macro_rules! assert_unsafe_precondition { fn runtime$(<$($tt)*>)?($($i:$ty),*) { if !$e { // don't unwind to reduce impact on code size - ::core::panicking::panic_str_nounwind( + ::core::panicking::panic_nounwind( concat!("unsafe precondition(s) violated: ", $name) ); } diff --git a/library/core/src/panicking.rs b/library/core/src/panicking.rs index a704a00faaafa..9fce78d076bf3 100644 --- a/library/core/src/panicking.rs +++ b/library/core/src/panicking.rs @@ -64,12 +64,13 @@ pub const fn panic_fmt(fmt: fmt::Arguments<'_>) -> ! { unsafe { panic_impl(&pi) } } -/// Like panic_fmt, but without unwinding and track_caller to reduce the impact on codesize. -/// Also just works on `str`, as a `fmt::Arguments` needs more space to be passed. +/// Like `panic`, but without unwinding and track_caller to reduce the impact on codesize. +/// (No `fmt` variant as a `fmt::Arguments` needs more space to be passed.) #[cfg_attr(not(feature = "panic_immediate_abort"), inline(never), cold)] #[cfg_attr(feature = "panic_immediate_abort", inline)] +#[cfg_attr(not(bootstrap), lang = "panic_nounwind")] // needed by codegen for non-unwinding panics #[rustc_nounwind] -pub fn panic_str_nounwind(msg: &'static str) -> ! { +pub fn panic_nounwind(msg: &'static str) -> ! { if cfg!(feature = "panic_immediate_abort") { super::intrinsics::abort() } @@ -153,10 +154,11 @@ fn panic_bounds_check(index: usize, len: usize) -> ! { /// any extra arguments (including those synthesized by track_caller). #[cfg_attr(not(feature = "panic_immediate_abort"), inline(never), cold)] #[cfg_attr(feature = "panic_immediate_abort", inline)] -#[lang = "panic_no_unwind"] // needed by codegen for panic in nounwind function +#[cfg_attr(bootstrap, lang = "panic_no_unwind")] // needed by codegen for panic in nounwind function +#[cfg_attr(not(bootstrap), lang = "panic_cannot_unwind")] // needed by codegen for panic in nounwind function #[rustc_nounwind] -fn panic_no_unwind() -> ! { - panic_str_nounwind("panic in a function that cannot unwind") +fn panic_cannot_unwind() -> ! { + panic_nounwind("panic in a function that cannot unwind") } /// This function is used instead of panic_fmt in const eval. diff --git a/src/test/codegen/unwind-abis/c-unwind-abi-panic-abort.rs b/src/test/codegen/unwind-abis/c-unwind-abi-panic-abort.rs index 34fd401f9e42b..ea5bae18e2337 100644 --- a/src/test/codegen/unwind-abis/c-unwind-abi-panic-abort.rs +++ b/src/test/codegen/unwind-abis/c-unwind-abi-panic-abort.rs @@ -10,7 +10,7 @@ #[no_mangle] pub unsafe extern "C-unwind" fn rust_item_that_can_unwind() { // Handle both legacy and v0 symbol mangling. - // CHECK: call void @{{.*core9panicking15panic_no_unwind}} + // CHECK: call void @{{.*core9panicking19panic_cannot_unwind}} may_unwind(); } diff --git a/src/test/codegen/unwind-and-panic-abort.rs b/src/test/codegen/unwind-and-panic-abort.rs index b370191bf8c82..e43e73b96b996 100644 --- a/src/test/codegen/unwind-and-panic-abort.rs +++ b/src/test/codegen/unwind-and-panic-abort.rs @@ -10,7 +10,7 @@ extern "C-unwind" { // CHECK: Function Attrs:{{.*}}nounwind // CHECK-NEXT: define{{.*}}void @foo // Handle both legacy and v0 symbol mangling. -// CHECK: call void @{{.*core9panicking15panic_no_unwind}} +// CHECK: call void @{{.*core9panicking19panic_cannot_unwind}} #[no_mangle] pub unsafe extern "C" fn foo() { bar(); diff --git a/src/test/ui/intrinsics/panic-uninitialized-zeroed.rs b/src/test/ui/intrinsics/panic-uninitialized-zeroed.rs index ec3860a322fc5..142c0fc81b4ab 100644 --- a/src/test/ui/intrinsics/panic-uninitialized-zeroed.rs +++ b/src/test/ui/intrinsics/panic-uninitialized-zeroed.rs @@ -1,7 +1,5 @@ // run-pass -// needs-unwind -// revisions: mir thir strict -// [thir]compile-flags: -Zthir-unsafeck +// revisions: default strict // [strict]compile-flags: -Zstrict-init-checks // ignore-tidy-linelength @@ -12,7 +10,6 @@ use std::{ mem::{self, MaybeUninit, ManuallyDrop}, - panic, ptr::NonNull, num, }; @@ -70,21 +67,42 @@ enum ZeroIsValid { } #[track_caller] -fn test_panic_msg(op: impl (FnOnce() -> T) + panic::UnwindSafe, msg: &str) { - let err = panic::catch_unwind(op).err(); - assert_eq!( - err.as_ref().and_then(|a| a.downcast_ref::<&str>()), - Some(&msg) - ); +fn test_panic_msg T) + 'static>(op: F, msg: &str) { + use std::{panic, env, process}; + + // The tricky part is that we can't just run `op`, as that would *abort* the process. + // So instead, we reinvoke this process with the caller location as argument. + // For the purpose of this test, the line number is unique enough. + // If we are running in such a re-invocation, we skip all the tests *except* for the one with that type name. + let our_loc = panic::Location::caller().line().to_string(); + let mut args = env::args(); + let this = args.next().unwrap(); + if let Some(loc) = args.next() { + if loc == our_loc { + op(); + panic!("we did not abort"); + } else { + // Nothing, we are running another test. + } + } else { + // Invoke new process for actual test, and check result. + let mut cmd = process::Command::new(this); + cmd.arg(our_loc); + let res = cmd.output().unwrap(); + assert!(!res.status.success(), "test did not fail"); + let stderr = String::from_utf8_lossy(&res.stderr); + assert!(stderr.contains(msg), "test did not contain expected output: looking for {:?}, output:\n{}", msg, stderr); + } } #[track_caller] -fn test_panic_msg_only_if_strict(op: impl (FnOnce() -> T) + panic::UnwindSafe, msg: &str) { - let err = panic::catch_unwind(op).err(); - assert_eq!( - err.as_ref().and_then(|a| a.downcast_ref::<&str>()), - if cfg!(strict) { Some(&msg) } else { None }, - ); +fn test_panic_msg_only_if_strict(op: impl (FnOnce() -> T) + 'static, msg: &str) { + if !cfg!(strict) { + // Just run it. + op(); + } else { + test_panic_msg(op, msg); + } } fn main() {