Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Launch a non-unwinding panic for misaligned pointer deref #112599

Merged
merged 1 commit into from
Jun 18, 2023
Merged

Launch a non-unwinding panic for misaligned pointer deref #112599

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 4 additions & 6 deletions compiler/rustc_mir_transform/src/check_alignment.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use rustc_middle::mir::{
};
use rustc_middle::ty::{Ty, TyCtxt, TypeAndMut};
use rustc_session::Session;
use rustc_target::spec::PanicStrategy;

pub struct CheckAlignment;

Expand Down Expand Up @@ -237,11 +236,10 @@ fn insert_alignment_check<'tcx>(
required: Operand::Copy(alignment),
found: Operand::Copy(addr),
}),
unwind: if tcx.sess.panic_strategy() == PanicStrategy::Unwind {
UnwindAction::Terminate
} else {
UnwindAction::Unreachable
},
// The panic symbol that this calls is #[rustc_nounwind]. We never want to insert an
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to mention panic_misaligned_pointer_dereference here so one can grep for what that symbol is. (I was not aware/forgot that different AssertKind dispatch to completely different code on assertion failure.)

// unwind into unsafe code, because unwinding could make a failing UB check turn into
// much worse UB when we start unwinding.
unwind: UnwindAction::Unreachable,
},
});
}
5 changes: 3 additions & 2 deletions library/core/src/panicking.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,14 +166,15 @@ fn panic_bounds_check(index: usize, len: usize) -> ! {
#[cfg_attr(not(feature = "panic_immediate_abort"), inline(never))]
#[track_caller]
#[lang = "panic_misaligned_pointer_dereference"] // needed by codegen for panic on misaligned pointer deref
#[rustc_nounwind] // `CheckAlignment` MIR pass requires this function to never unwind
fn panic_misaligned_pointer_dereference(required: usize, found: usize) -> ! {
if cfg!(feature = "panic_immediate_abort") {
super::intrinsics::abort()
}

panic!(
panic_nounwind_fmt(format_args!(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also make this function #[rustc_nounwind] and document that this function MUST NOT unwind.

I wouldn't worry about non-local dependency that you mentioned at #112403 (comment), since this is a lang item and it's expected that lang item may need special guarantees. A #[rustc_nounwind] just near the lang item attribute with a comment should be clear enough to prevent mistakes.

"misaligned pointer dereference: address must be a multiple of {required:#x} but is {found:#x}"
)
))
}

/// Panic because we cannot unwind out of a function.
Expand Down