Auto merge of #105997 - RalfJung:immediate-abort, r=eholk

abort immediately on bad mem::zeroed/uninit

Now that we have non-unwinding panics, let's use them for these assertions. This re-establishes the property that `mem::uninitialized` and `mem::zeroed` will never unwind -- the earlier approach of causing panics here sometimes led to hard-to-debug segfaults when the surrounding code was not able to cope with the unexpected unwinding.

Cc `@bjorn3` I did not touch cranelift but I assume it needs a similar patch. However it has a `codegen_panic` abstraction that I did not want to touch since I didn't know how else it is used.
This commit is contained in:
bors 2022-12-25 20:51:37 +00:00
commit 8dfb339541
10 changed files with 65 additions and 42 deletions

View File

@ -637,7 +637,7 @@ fn codegen_abort_terminator(
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 @@ enum AssertIntrinsic {
})
});
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 @@ enum AssertIntrinsic {
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 @@ fn double_unwind_guard(&mut self) -> Bx::BasicBlock {
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);

View File

@ -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 @@ pub fn extract(attrs: &[ast::Attribute]) -> Option<(Symbol, Span)> {
// 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 @@ pub fn extract(attrs: &[ast::Attribute]) -> Option<(Symbol, Span)> {
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;

View File

@ -843,7 +843,7 @@ fn visit_terminator(&mut self, terminator: &mir::Terminator<'tcx>, location: Loc
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));

View File

@ -1041,6 +1041,7 @@
panic_2021,
panic_abort,
panic_bounds_check,
panic_cannot_unwind,
panic_display,
panic_fmt,
panic_handler,
@ -1048,7 +1049,7 @@
panic_implementation,
panic_info,
panic_location,
panic_no_unwind,
panic_nounwind,
panic_runtime,
panic_str,
panic_unwind,

View File

@ -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)
);
}

View File

@ -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.

View File

@ -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();
}

View File

@ -10,7 +10,7 @@
// 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();

View File

@ -18,11 +18,11 @@ pub fn shrink_to_fit(vec: &mut Vec<u32>) {
pub fn issue71861(vec: Vec<u32>) -> Box<[u32]> {
// CHECK-NOT: panic
// Call to panic_no_unwind in case of double-panic is expected,
// Call to panic_cannot_unwind in case of double-panic is expected,
// but other panics are not.
// CHECK: cleanup
// CHECK-NEXT: ; call core::panicking::panic_no_unwind
// CHECK-NEXT: panic_no_unwind
// CHECK-NEXT: ; call core::panicking::panic_cannot_unwind
// CHECK-NEXT: panic_cannot_unwind
// CHECK-NOT: panic
vec.into_boxed_slice()
@ -33,15 +33,15 @@ pub fn issue71861(vec: Vec<u32>) -> Box<[u32]> {
pub fn issue75636<'a>(iter: &[&'a str]) -> Box<[&'a str]> {
// CHECK-NOT: panic
// Call to panic_no_unwind in case of double-panic is expected,
// Call to panic_cannot_unwind in case of double-panic is expected,
// but other panics are not.
// CHECK: cleanup
// CHECK-NEXT: ; call core::panicking::panic_no_unwind
// CHECK-NEXT: panic_no_unwind
// CHECK-NEXT: ; call core::panicking::panic_cannot_unwind
// CHECK-NEXT: panic_cannot_unwind
// CHECK-NOT: panic
iter.iter().copied().collect()
}
// CHECK: ; core::panicking::panic_no_unwind
// CHECK: declare void @{{.*}}panic_no_unwind
// CHECK: ; core::panicking::panic_cannot_unwind
// CHECK: declare void @{{.*}}panic_cannot_unwind

View File

@ -1,9 +1,9 @@
// 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
// ignore-emscripten spawning processes is not supported
// ignore-sgx no processes
// This test checks panic emitted from `mem::{uninitialized,zeroed}`.
@ -12,7 +12,6 @@
use std::{
mem::{self, MaybeUninit, ManuallyDrop},
panic,
ptr::NonNull,
num,
};
@ -70,21 +69,42 @@ enum ZeroIsValid {
}
#[track_caller]
fn test_panic_msg<T>(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, F: (FnOnce() -> 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<T>(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<T>(op: impl (FnOnce() -> T) + 'static, msg: &str) {
if !cfg!(strict) {
// Just run it.
op();
} else {
test_panic_msg(op, msg);
}
}
fn main() {