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

Fix panic when restricted-std is enabled. #104971

Closed
wants to merge 1 commit into from
Closed

Fix panic when restricted-std is enabled. #104971

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
16 changes: 11 additions & 5 deletions library/std/src/panicking.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,20 +336,26 @@ pub mod panic_count {
pub fn increase() -> (bool, usize) {
let global_count = GLOBAL_PANIC_COUNT.fetch_add(1, Ordering::Relaxed);
let must_abort = global_count & ALWAYS_ABORT_FLAG != 0;
let panics = if must_abort {
global_count & !ALWAYS_ABORT_FLAG
} else {
LOCAL_PANIC_COUNT.with(|c| {
let mut panics = global_count & !ALWAYS_ABORT_FLAG;

// In a restricted_std environment, we likely don't have thread_local.
// If we attempt to use LOCAL_PANIC_COUNT here, it is probable that another panic will
// occur, sending us into an infinite panic loop that never calls the panic handler.
#[cfg(not(feature = "restricted-std"))]
bjorn3 marked this conversation as resolved.
Show resolved Hide resolved
if !must_abort {
panics = LOCAL_PANIC_COUNT.with(|c| {
Comment on lines +341 to +346
Copy link
Member

Choose a reason for hiding this comment

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

What does "likely" mean here? Your PR description says

When restricted-std is in use, we do not have access to thread local storage, as sys/unsupported does not implement it.

Is that always the case when using restricted-std? and if not, when can the behavior be different?

Copy link
Author

Choose a reason for hiding this comment

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

Looking at it a bit more, it seems it's true most of the time, but not necessarily always. The restricted-std requirement is defined here if a platform match is not found, and since thread_local is unimplemented in sys/unsupported, a panic loop occurs.

There do appear to be some cases where this may not always be true - i.e. in the case of aarch64-apple-tvos, which is not allowed to use regular unrestricted std, despite being part of the unix family and thus using the unix system module which does properly implement thread-local support. I would imagine this same situation would also apply to any of the other targets which are called out in the same area in build.rs and also use a non-unsupported system module.

I'm not really sure how big of an impact this would be; since restricted-std is infectious, requiring itself to be in every std-dependent crate throughout the entire dependency tree in order to be used, it seems like a minor detail, especially since a double panic on these platforms will still trigger an abort via the global panic counter.

let next = c.get() + 1;
c.set(next);
next
})
});
};

(must_abort, panics)
}

pub fn decrease() {
GLOBAL_PANIC_COUNT.fetch_sub(1, Ordering::Relaxed);
#[cfg(not(feature = "restricted-std"))]
LOCAL_PANIC_COUNT.with(|c| {
let next = c.get() - 1;
c.set(next);
Expand Down