-
Notifications
You must be signed in to change notification settings - Fork 922
Attempt to prevent crashes during exit due to deregistering frames on exit #5893
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.
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -4,6 +4,7 @@ | |||||||||
| //! Module for System V ABI unwind registry. | ||||||||||
|
|
||||||||||
| use core::sync::atomic::{AtomicUsize, Ordering::Relaxed}; | ||||||||||
| use std::sync::Mutex; | ||||||||||
|
|
||||||||||
| use crate::types::unwind::CompiledFunctionUnwindInfoReference; | ||||||||||
|
|
||||||||||
|
|
@@ -21,6 +22,16 @@ unsafe extern "C" { | |||||||||
| fn __deregister_frame(fde: *const u8); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /// The GLOBAL_UNREGISTRY fullfills two purposes: | ||||||||||
| /// 1. It keeps track of all the frames that should be deregistered. We don't want to | ||||||||||
| /// deregister frames in UnwindRegistry::Drop as that could be called during | ||||||||||
| /// program shutdown and can collide with release_registered_frames and lead to | ||||||||||
| /// crashes. | ||||||||||
| /// 2. It serves as a lock to ensure that registrations and deregistrations are not | ||||||||||
| /// interleaved when multiple threads are registering/deregistering frames at the | ||||||||||
| /// same time because that can also lead to crashes. | ||||||||||
| static GLOBAL_UNREGISTRY: Mutex<Vec<usize>> = Mutex::new(Vec::new()); | ||||||||||
|
|
||||||||||
| // Apple-specific unwind functions - the following is taken from LLVM's libunwind itself. | ||||||||||
| #[cfg(all(target_os = "macos", target_arch = "aarch64"))] | ||||||||||
| mod compact_unwind; | ||||||||||
|
|
@@ -141,12 +152,26 @@ impl UnwindRegistry { | |||||||||
| unsafe fn register_frames(&mut self, eh_frame: &[u8]) { | ||||||||||
| #[cfg(all(target_os = "macos", target_arch = "aarch64"))] | ||||||||||
| { | ||||||||||
| // Aquire the unregistry lock to avoid interleaved registrations/deregistrations. | ||||||||||
zebreus marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||
| let mut unregistry = GLOBAL_UNREGISTRY.lock().unwrap(); | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here we would iterate the |
||||||||||
|
|
||||||||||
| for registration in unregistry.iter_mut() { | ||||||||||
| unsafe { | ||||||||||
| compact_unwind::__unw_remove_dynamic_eh_frame_section(*registration); | ||||||||||
| } | ||||||||||
| self.compact_unwind_mgr.deregister(); | ||||||||||
| } | ||||||||||
|
||||||||||
| self.compact_unwind_mgr.deregister(); | |
| } | |
| } | |
| self.compact_unwind_mgr.deregister(); |
Copilot
AI
Nov 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On macOS aarch64, the code registers frames via __unw_add_dynamic_eh_frame_section at line 170, but unlike the non-macOS path (line 245), it doesn't add the registration to self.registrations. This means when Drop is called (line 294), self.registrations will be empty on macOS, so nothing gets queued in the unregistry for later deregistration. This could lead to the registered frames never being deregistered. Consider adding the registration to self.registrations after line 170, similar to how it's done at line 245 for non-macOS platforms.
| compact_unwind::__unw_add_dynamic_eh_frame_section(eh_frame.as_ptr() as usize); | |
| let registration = eh_frame.as_ptr() as usize; | |
| compact_unwind::__unw_add_dynamic_eh_frame_section(registration); | |
| self.registrations.push(registration); |
zebreus marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
zebreus marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
zebreus marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and this will simply push to the GLOBAL_UNREGISTRY vector.
Uh oh!
There was an error while loading. Please reload this page.