From 33f0df3b27bc47cb37101668730b10f05c353f46 Mon Sep 17 00:00:00 2001 From: 4lDO2 <4lDO2@protonmail.com> Date: Sun, 4 Aug 2024 18:12:59 +0200 Subject: [PATCH] Deduplicate and simplify sigprocmask code. --- redox-rt/src/signal.rs | 88 +++++++++++++++++------------------------- 1 file changed, 36 insertions(+), 52 deletions(-) diff --git a/redox-rt/src/signal.rs b/redox-rt/src/signal.rs index 938f8ce3d..73e26ba02 100644 --- a/redox-rt/src/signal.rs +++ b/redox-rt/src/signal.rs @@ -160,25 +160,14 @@ unsafe fn inner(stack: &mut SigStack) { }; // Set sigmask to sa_mask and unmark the signal as pending. - let prev_sigallow_lo = os.control.word[0].load(Ordering::Relaxed) >> 32; - let prev_sigallow_hi = os.control.word[1].load(Ordering::Relaxed) >> 32; - let prev_sigallow = prev_sigallow_lo | (prev_sigallow_hi << 32); + let prev_sigallow = get_mask_raw(&os.control.word); let mut sigallow_inside = !sigaction.mask & prev_sigallow; if !sigaction.flags.contains(SigactionFlags::NODEFER) { sigallow_inside &= !sig_bit(stack.sig_num); } - let sigallow_inside_lo = sigallow_inside & 0xffff_ffff; - let sigallow_inside_hi = sigallow_inside >> 32; - let prev_w0 = os.control.word[0].fetch_add( - (sigallow_inside_lo << 32).wrapping_sub(prev_sigallow_lo << 32), - Ordering::Relaxed, - ); - let prev_w1 = os.control.word[1].fetch_add( - (sigallow_inside_hi << 32).wrapping_sub(prev_sigallow_hi << 32), - Ordering::Relaxed, - ); + let _pending_when_sa_mask = set_mask_raw(&os.control.word, prev_sigallow, sigallow_inside); // TODO: If sa_mask caused signals to be unblocked, deliver one or all of those first? @@ -190,7 +179,7 @@ unsafe fn inner(stack: &mut SigStack) { ); core::sync::atomic::compiler_fence(Ordering::Acquire); - stack.old_mask = ((prev_w1 >> 32) << 32) | (prev_w0 >> 32); + stack.old_mask = prev_sigallow; // Call handler, either sa_handler or sa_siginfo depending on flag. if sigaction.flags.contains(SigactionFlags::SIGINFO) @@ -225,17 +214,9 @@ unsafe fn inner(stack: &mut SigStack) { // Update allowset again. let new_mask = stack.old_mask; - let old_mask = (os.control.word[0].load(Ordering::Relaxed) >> 32) - | ((os.control.word[1].load(Ordering::Relaxed) >> 32) << 32); + let old_mask = get_mask_raw(&os.control.word); - let _prev_w0 = os.control.word[0].fetch_add( - ((new_mask & 0xffff_ffff) << 32).wrapping_sub((old_mask & 0xffff_ffff) << 32), - Ordering::Relaxed, - ); - let _prev_w1 = os.control.word[1].fetch_add( - ((new_mask >> 32) << 32).wrapping_sub((old_mask >> 32) << 32), - Ordering::Relaxed, - ); + let _pending_when_restored_mask = set_mask_raw(&os.control.word, old_mask, new_mask); // TODO: If resetting the sigmask caused signals to be unblocked, then should they be delivered // here? And would it be possible to tail-call-optimize that? @@ -267,61 +248,64 @@ pub(crate) unsafe extern "fastcall" fn inner_fastcall(stack: usize) { pub fn get_sigmask() -> Result<u64> { let mut mask = 0; - modify_sigmask(Some(&mut mask), Option::<fn(u32, bool) -> u32>::None)?; + modify_sigmask(Some(&mut mask), Option::<fn(u64) -> u64>::None)?; Ok(mask) } pub fn set_sigmask(new: Option<u64>, old: Option<&mut u64>) -> Result<()> { - modify_sigmask( - old, - new.map(move |newmask| move |_, upper| if upper { newmask >> 32 } else { newmask } as u32), - ) + modify_sigmask(old, new.map(move |newmask| move |_| newmask)) } pub fn or_sigmask(new: Option<u64>, old: Option<&mut u64>) -> Result<()> { // Parsing nightmare... :) modify_sigmask( old, - new.map(move |newmask| { - move |oldmask, upper| oldmask | if upper { newmask >> 32 } else { newmask } as u32 - }), + new.map(move |newmask| move |oldmask| oldmask | newmask), ) } pub fn andn_sigmask(new: Option<u64>, old: Option<&mut u64>) -> Result<()> { modify_sigmask( old, - new.map(move |newmask| { - move |oldmask, upper| oldmask & !if upper { newmask >> 32 } else { newmask } as u32 - }), + new.map(move |newmask| move |oldmask| oldmask & !newmask), ) } -fn modify_sigmask(old: Option<&mut u64>, op: Option<impl FnMut(u32, bool) -> u32>) -> Result<()> { +fn get_mask_raw(words: &[AtomicU64; 2]) -> u64 { + (words[0].load(Ordering::Relaxed) >> 32) | ((words[1].load(Ordering::Relaxed) >> 32) << 32) +} +/// Sets mask from old to new, returning what was pending at the time. +fn set_mask_raw(words: &[AtomicU64; 2], old: u64, new: u64) -> u64 { + // This assumes *only this thread* can change the allowset. If this rule is broken, the use of + // fetch_add will corrupt the words entirely. fetch_add is very efficient on x86, being + // generated as LOCK XADD which is the fastest RMW instruction AFAIK. + let prev_w0 = words[0].fetch_add( + ((new & 0xffff_ffff) << 32).wrapping_sub((old & 0xffff_ffff) << 32), + Ordering::Relaxed, + ) & 0xffff_ffff; + let prev_w1 = words[1].fetch_add( + ((new >> 32) << 32).wrapping_sub((old >> 32) << 32), + Ordering::Relaxed, + ) & 0xffff_ffff; + + prev_w0 | (prev_w1 << 32) +} +fn modify_sigmask(old: Option<&mut u64>, op: Option<impl FnOnce(u64) -> u64>) -> Result<()> { let _guard = tmp_disable_signals(); let ctl = current_sigctl(); - let words = ctl.word.each_ref().map(|w| w.load(Ordering::Relaxed)); + let prev = get_mask_raw(&ctl.word); if let Some(old) = old { - *old = !combine_allowset(words); + *old = !prev; } - let Some(mut op) = op else { + let Some(op) = op else { return Ok(()); }; - let mut can_raise = 0; - - for i in 0..2 { - let old_allow_bits = words[i] & 0xffff_ffff_0000_0000; - let new_allow_bits = u64::from(!op(!((old_allow_bits >> 32) as u32), i == 1)) << 32; + let next = !op(!prev); - let old_word = ctl.word[i].fetch_add( - new_allow_bits.wrapping_sub(old_allow_bits), - Ordering::Relaxed, - ); - can_raise |= ((old_word & 0xffff_ffff) & (new_allow_bits >> 32)) << (i * 32); - } + let pending = set_mask_raw(&ctl.word, prev, next); // POSIX requires that at least one pending unblocked signal be delivered before - // pthread_sigmask returns, if there is one. Deliver the lowest-numbered one. - if can_raise != 0 { + // pthread_sigmask returns, if there is one. + if pending != 0 { unsafe { manually_enter_trampoline(); } -- GitLab