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

Jump to bottom

make sure panic_nounwind_fmt can still be fully inlined (e.g. for panic_immediate_abort) #118362

Merged
merged 2 commits into from
Dec 5, 2023
Merged

make sure panic_nounwind_fmt can still be fully inlined (e.g. for panic_immediate_abort) #118362

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2a48a77
make sure panic_nounwind_fmt can still be fully inlined (e.g. for pan…
RalfJung Nov 27, 2023
8de5bd0
use the usual attributes for panic_misaligned_pointer_dereference
RalfJung Nov 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 6 additions & 3 deletions library/core/src/panicking.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ pub const fn panic_fmt(fmt: fmt::Arguments<'_>) -> ! {
#[rustc_nounwind]
#[rustc_const_unstable(feature = "core_panic", issue = "none")]
pub const fn panic_nounwind_fmt(fmt: fmt::Arguments<'_>, force_no_backtrace: bool) -> ! {
#[inline] // this should always be inlined into `panic_nounwind_fmt`
#[track_caller]
fn runtime(fmt: fmt::Arguments<'_>, force_no_backtrace: bool) -> ! {
if cfg!(feature = "panic_immediate_abort") {
Expand Down Expand Up @@ -112,6 +113,7 @@ pub const fn panic_nounwind_fmt(fmt: fmt::Arguments<'_>, force_no_backtrace: boo
#[inline]
#[track_caller]
const fn comptime(fmt: fmt::Arguments<'_>, _force_no_backtrace: bool) -> ! {
// We don't unwind anyway at compile-time so we can call the regular `panic_fmt`.
panic_fmt(fmt);
}

Expand Down Expand Up @@ -142,7 +144,8 @@ pub const fn panic(expr: &'static str) -> ! {
panic_fmt(fmt::Arguments::new_const(&[expr]));
}

/// Like `panic`, but without unwinding and track_caller to reduce the impact on codesize.
/// 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)]
#[cfg_attr(feature = "panic_immediate_abort", inline)]
#[lang = "panic_nounwind"] // needed by codegen for non-unwinding panics
Expand Down Expand Up @@ -205,8 +208,8 @@ fn panic_bounds_check(index: usize, len: usize) -> ! {
panic!("index out of bounds: the len is {len} but the index is {index}")
}

#[cold]
#[cfg_attr(not(feature = "panic_immediate_abort"), inline(never))]
#[cfg_attr(not(feature = "panic_immediate_abort"), inline(never), cold)]
#[cfg_attr(feature = "panic_immediate_abort", inline)]
Copy link
Member Author

Choose a reason for hiding this comment

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

@saethlin is there any reason you used different attributes for this function than what all the others in this file do?

Copy link
Member

Choose a reason for hiding this comment

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

Argh we should really have codegen tests for this feature. That would settle all this fiddling with attributes, or at least we could know that whatever is in the code works as intended. I'll open an issue later if I remember.

I think #[cold] discourages inlining but if the body has been optimized to just an unreachable terminator it still gets optimized away. Whether by the MIR inliner or LLVM I'm not sure.

Copy link
Member

Choose a reason for hiding this comment

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

#118393

#[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
Expand Down