Deduplicate and simplify sigprocmask code.

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;
+    let next = !op(!prev);
 
-    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 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();
         }