diff --git a/brush-builtins/src/kill.rs b/brush-builtins/src/kill.rs index f69b9fa82..1d3af4bbb 100644 --- a/brush-builtins/src/kill.rs +++ b/brush-builtins/src/kill.rs @@ -145,9 +145,16 @@ fn print_signals( let signal = if let Ok(n) = s.parse::() { // bash compatibility. `SIGHUP` -> `HUP` - TrapSignal::try_from(n).map(|s| { - PrintSignal::Name(s.as_str().strip_prefix("SIG").unwrap_or(s.as_str())) - }) + TrapSignal::try_from(n) + .or_else(|error| { + n.checked_sub(128) + .filter(|signal| *signal > 0) + .ok_or(error) + .and_then(TrapSignal::try_from) + }) + .map(|s| { + PrintSignal::Name(s.as_str().strip_prefix("SIG").unwrap_or(s.as_str())) + }) } else { TrapSignal::try_from(s.as_str()).map(|sig| { i32::try_from(sig).map_or(PrintSignal::Name(sig.as_str()), PrintSignal::Num) diff --git a/brush-core/src/shell/traps.rs b/brush-core/src/shell/traps.rs index 83c4400d0..7a49a809e 100644 --- a/brush-core/src/shell/traps.rs +++ b/brush-core/src/shell/traps.rs @@ -99,7 +99,7 @@ impl crate::Shell { // EXIT and system signals are always inherited — i.e. their visibility is // not gated by errtrace/functrace options. (The actual trap *state* for // subshells is managed separately via `Shell::clone`.) - TrapSignal::Exit | TrapSignal::Signal(_) => true, + TrapSignal::Exit | TrapSignal::Signal(_) | TrapSignal::RealtimeSignal(_) => true, } } } diff --git a/brush-core/src/sys.rs b/brush-core/src/sys.rs index 8e8f41ed8..4b9746aa3 100644 --- a/brush-core/src/sys.rs +++ b/brush-core/src/sys.rs @@ -17,7 +17,7 @@ pub(crate) mod wasm; #[cfg(target_family = "wasm")] pub(crate) use wasm as platform; -#[cfg(not(unix))] +#[cfg(any(not(unix), test))] pub(crate) mod stubs; #[cfg(any(unix, windows))] diff --git a/brush-core/src/sys/stubs/signal.rs b/brush-core/src/sys/stubs/signal.rs index 5fd58bff4..09afd32ed 100644 --- a/brush-core/src/sys/stubs/signal.rs +++ b/brush-core/src/sys/stubs/signal.rs @@ -18,8 +18,16 @@ impl Signal { "" } - /// Creates a `Signal` from a string representation. - pub fn from_str(s: &str) -> Result { + /// Returns the platform signal number. + pub const fn number(self) -> i32 { + match self {} + } +} + +impl std::str::FromStr for Signal { + type Err = error::Error; + + fn from_str(s: &str) -> Result { Err(error::ErrorKind::InvalidSignal(s.into()).into()) } } @@ -32,7 +40,19 @@ impl TryFrom for Signal { } } -pub(crate) fn ignored_signals() -> &'static [(i32, String)] { +pub(crate) const fn realtime_signal_definitions() -> &'static [(i32, String)] { + &[] +} + +pub(crate) const fn realtime_signal_from_name(_name: &str) -> Option { + None +} + +pub(crate) const fn realtime_signal_name(_number: i32) -> Option<&'static str> { + None +} + +pub(crate) const fn ignored_signals() -> &'static [(i32, String)] { &[] } diff --git a/brush-core/src/sys/unix/signal.rs b/brush-core/src/sys/unix/signal.rs index 2bd1c9791..7a43fe784 100644 --- a/brush-core/src/sys/unix/signal.rs +++ b/brush-core/src/sys/unix/signal.rs @@ -27,12 +27,13 @@ pub(crate) fn ignored_signals() -> &'static [(i32, String)] { } fn query_ignored_signals() -> Vec<(i32, String)> { - let signals = Signal::iterator().map(|signal| (signal as i32, signal.as_str().to_owned())); - - #[cfg(target_os = "linux")] - let signals = signals.chain(realtime_signals()); - - signals + Signal::iterator() + .map(|signal| (signal as i32, signal.as_str().to_owned())) + .chain( + realtime_signal_definitions() + .iter() + .map(|(number, name)| (*number, name.clone())), + ) .filter_map(|(number, name)| match signal_is_ignored(number) { Ok(true) => Some((number, name)), Ok(false) | Err(_) => None, @@ -55,23 +56,71 @@ fn signal_is_ignored(signal: i32) -> Result { } #[cfg(target_os = "linux")] -fn realtime_signals() -> impl Iterator { +pub(crate) fn realtime_signal_definitions() -> &'static [(i32, String)] { + static SIGNALS: std::sync::OnceLock> = std::sync::OnceLock::new(); + + SIGNALS.get_or_init(|| { + let min = nix::libc::SIGRTMIN(); + let max = nix::libc::SIGRTMAX(); + let midpoint = min + (max - min) / 2; + + (min..=max) + .map(|signal| { + let name = if signal == min { + "SIGRTMIN".to_owned() + } else if signal <= midpoint { + format!("SIGRTMIN+{}", signal - min) + } else if signal == max { + "SIGRTMAX".to_owned() + } else { + format!("SIGRTMAX-{}", max - signal) + }; + (signal, name) + }) + .collect() + }) +} + +#[cfg(not(target_os = "linux"))] +pub(crate) const fn realtime_signal_definitions() -> &'static [(i32, String)] { + &[] +} + +#[cfg(target_os = "linux")] +pub(crate) fn realtime_signal_from_name(name: &str) -> Option { let min = nix::libc::SIGRTMIN(); let max = nix::libc::SIGRTMAX(); let midpoint = min + (max - min) / 2; - (min..=max).map(move |signal| { - let name = if signal == min { - "SIGRTMIN".to_owned() - } else if signal <= midpoint { - format!("SIGRTMIN+{}", signal - min) - } else if signal == max { - "SIGRTMAX".to_owned() - } else { - format!("SIGRTMAX-{}", max - signal) - }; - (signal, name) - }) + if name == "SIGRTMIN" { + min + } else if name == "SIGRTMAX" { + max + } else if let Some(offset) = name + .strip_prefix("SIGRTMIN+") + .and_then(|offset| offset.parse::().ok()) + { + min.checked_add(offset) + .filter(|number| (min..=max).contains(number))? + } else { + let offset = name + .strip_prefix("SIGRTMAX-") + .and_then(|offset| offset.parse::().ok())?; + max.checked_sub(offset) + .filter(|number| (midpoint < *number) && (*number < max))? + } + .into() +} + +#[cfg(not(target_os = "linux"))] +pub(crate) const fn realtime_signal_from_name(_name: &str) -> Option { + None +} + +pub(crate) fn realtime_signal_name(number: i32) -> Option<&'static str> { + realtime_signal_definitions() + .iter() + .find_map(|(candidate, name)| (*candidate == number).then_some(name.as_str())) } pub(crate) fn continue_process(pid: sys::process::ProcessId) -> Result<(), error::Error> { @@ -90,7 +139,8 @@ pub fn kill_process( signal: traps::TrapSignal, ) -> Result<(), error::Error> { let translated_signal = match signal { - traps::TrapSignal::Signal(signal) => signal, + traps::TrapSignal::Signal(signal) => signal as i32, + traps::TrapSignal::RealtimeSignal(signal) => signal.number(), traps::TrapSignal::Debug | traps::TrapSignal::Err | traps::TrapSignal::Exit @@ -99,7 +149,8 @@ pub fn kill_process( } }; - nix::sys::signal::kill(nix::unistd::Pid::from_raw(pid), translated_signal) + // SAFETY: `kill` does not dereference pointers; both arguments are plain integers. + nix::errno::Errno::result(unsafe { nix::libc::kill(pid, translated_signal) }) .map_err(|_errno| error::ErrorKind::FailedToSendSignal)?; Ok(()) @@ -237,3 +288,81 @@ fn siginfo_to_wait_status( Ok(status) } + +#[cfg(test)] +mod tests { + #[cfg(target_os = "linux")] + mod linux { + use super::super::{ + realtime_signal_definitions, realtime_signal_from_name, realtime_signal_name, + }; + + #[test] + fn realtime_signal_names_use_runtime_bounds() { + let min = nix::libc::SIGRTMIN(); + let max = nix::libc::SIGRTMAX(); + let midpoint = min + (max - min) / 2; + + assert_eq!(realtime_signal_name(min), Some("SIGRTMIN")); + assert_eq!(realtime_signal_name(max), Some("SIGRTMAX")); + assert_eq!( + realtime_signal_name(midpoint), + Some(format!("SIGRTMIN+{}", midpoint - min).as_str()) + ); + assert_eq!( + realtime_signal_name(midpoint + 1), + Some(format!("SIGRTMAX-{}", max - midpoint - 1).as_str()) + ); + } + + #[test] + fn realtime_signal_names_and_numbers_are_parsed() { + let min = nix::libc::SIGRTMIN(); + let max = nix::libc::SIGRTMAX(); + let span = max - min; + + assert_eq!(realtime_signal_from_name("SIGRTMIN"), Some(min)); + assert_eq!(realtime_signal_from_name("SIGRTMIN+0"), Some(min)); + assert_eq!( + realtime_signal_from_name(format!("SIGRTMIN+{span}").as_str()), + Some(max) + ); + assert_eq!(realtime_signal_from_name("SIGRTMAX-1"), Some(max - 1)); + assert!(realtime_signal_name(min).is_some()); + assert!(realtime_signal_name(max).is_some()); + } + + #[test] + fn invalid_realtime_signal_offsets_are_rejected() { + let min = nix::libc::SIGRTMIN(); + let max = nix::libc::SIGRTMAX(); + let span = max - min; + + assert!(realtime_signal_from_name(format!("SIGRTMIN+{}", span + 1).as_str()).is_none()); + assert!(realtime_signal_from_name("SIGRTMAX-0").is_none()); + assert!(realtime_signal_from_name(format!("SIGRTMAX-{span}").as_str()).is_none()); + assert!(realtime_signal_name(min - 1).is_none()); + assert!(realtime_signal_name(max + 1).is_none()); + } + + #[test] + fn iterator_uses_canonical_realtime_names() { + let min = nix::libc::SIGRTMIN(); + let max = nix::libc::SIGRTMAX(); + let realtime = realtime_signal_definitions(); + + assert_eq!( + realtime.len(), + usize::try_from(max - min + 1).unwrap_or_default() + ); + assert_eq!( + realtime.first().map(|(_, name)| name.as_str()), + Some("SIGRTMIN") + ); + assert_eq!( + realtime.last().map(|(_, name)| name.as_str()), + Some("SIGRTMAX") + ); + } + } +} diff --git a/brush-core/src/traps.rs b/brush-core/src/traps.rs index 1e63c6d6c..7ba01c9f0 100644 --- a/brush-core/src/traps.rs +++ b/brush-core/src/traps.rs @@ -12,6 +12,8 @@ use crate::{error, sys}; pub enum TrapSignal { /// A system signal. Signal(sys::signal::Signal), + /// A real-time system signal. + RealtimeSignal(RealtimeSignal), /// The `DEBUG` trap. Debug, /// The `ERR` trap. @@ -22,6 +24,24 @@ pub enum TrapSignal { Return, } +/// A real-time system signal. +#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] +pub struct RealtimeSignal { + number: i32, + name: &'static str, +} + +impl RealtimeSignal { + /// Returns the platform signal number. + pub const fn number(self) -> i32 { + self.number + } + + const fn as_str(self) -> &'static str { + self.name + } +} + #[cfg(feature = "serde")] impl serde::Serialize for TrapSignal { fn serialize(&self, serializer: S) -> Result @@ -56,7 +76,15 @@ impl TrapSignal { let iter = itertools::chain!( SIGNALS.iter().copied(), - sys::signal::Signal::iterator().map(TrapSignal::Signal) + sys::signal::Signal::iterator().map(TrapSignal::Signal), + sys::signal::realtime_signal_definitions() + .iter() + .map(|(number, name)| { + Self::RealtimeSignal(RealtimeSignal { + number: *number, + name: name.as_str(), + }) + }) ); iter @@ -66,6 +94,7 @@ impl TrapSignal { pub const fn as_str(self) -> &'static str { match self { Self::Signal(s) => s.as_str(), + Self::RealtimeSignal(s) => s.as_str(), Self::Debug => "DEBUG", Self::Err => "ERR", Self::Exit => "EXIT", @@ -113,10 +142,18 @@ impl TryFrom for TrapSignal { // and don't have persistent numbers across platforms, so we skip them here. Ok(match value { 0 => Self::Exit, - value => Self::Signal( - sys::signal::Signal::try_from(value) - .map_err(|_| error::ErrorKind::InvalidSignal(value.to_string()))?, - ), + value => { + if let Ok(signal) = sys::signal::Signal::try_from(value) { + Self::Signal(signal) + } else if sys::signal::realtime_signal_name(value).is_some() { + Self::RealtimeSignal(RealtimeSignal { + number: value, + name: sys::signal::realtime_signal_name(value).unwrap_or_default(), + }) + } else { + return Err(error::ErrorKind::InvalidSignal(value.to_string()).into()); + } + } }) } } @@ -139,9 +176,16 @@ impl TryFrom<&str> for TrapSignal { if !s.starts_with("SIG") { s.insert_str(0, "SIG"); } - sys::signal::Signal::from_str(s.as_str()) - .map(TrapSignal::Signal) - .map_err(|_| error::ErrorKind::InvalidSignal(value.into()))? + if let Ok(signal) = sys::signal::Signal::from_str(s.as_str()) { + Self::Signal(signal) + } else if let Some(number) = sys::signal::realtime_signal_from_name(s.as_str()) { + Self::RealtimeSignal(RealtimeSignal { + number, + name: sys::signal::realtime_signal_name(number).unwrap_or_default(), + }) + } else { + return Err(error::ErrorKind::InvalidSignal(value.into()).into()); + } } }) } @@ -156,6 +200,7 @@ impl TryFrom for i32 { fn try_from(value: TrapSignal) -> Result { Ok(match value { TrapSignal::Signal(s) => s as Self, + TrapSignal::RealtimeSignal(s) => s.number(), TrapSignal::Exit => 0, _ => return Err(TrapSignalNumberError), }) @@ -267,6 +312,14 @@ impl TrapHandlerConfig { mod tests { use super::{TrapHandlerConfig, TrapSignal}; + #[cfg(unix)] + #[test] + fn signal_variant_preserves_nix_signal_payload() { + const SIGNAL: TrapSignal = TrapSignal::Signal(nix::sys::signal::Signal::SIGTERM); + + assert_eq!(SIGNAL.as_str(), "SIGTERM"); + } + #[test] fn ignored_signal_can_be_looked_up_by_trap_signal() { let Some(signal) = crate::sys::signal::Signal::iterator().next() else { diff --git a/brush-shell/tests/cases/compat/builtins/kill.yaml b/brush-shell/tests/cases/compat/builtins/kill.yaml index abbf7bece..db00b8133 100644 --- a/brush-shell/tests/cases/compat/builtins/kill.yaml +++ b/brush-shell/tests/cases/compat/builtins/kill.yaml @@ -4,8 +4,7 @@ cases: ignore_stderr: true stdin: | for i in $(seq 1 31); do kill -l $i; done - # limit the number of signals to 31. Realtime signals are not implemented yet. - for i in $(kill -l | sed -e "s/[[:digit:]]*)//g"); do echo $i; done | head -31 + for i in $(kill -l | sed -e "s/[[:digit:]]*)//g"); do echo $i; done # invalid option kill -l 9999 kill -l HUP @@ -14,6 +13,50 @@ cases: kill -l SIGHUP kill -l EXIT + - name: "kill -l signal-derived exit statuses" + ignore_stderr: true + stdin: | + for status in 129 137; do + printf '%s=%s\n' "$status" "$(kill -l "$status")" + done + kill -l 128 + printf '128:%s\n' "$?" + + if test "$(uname -s)" = Linux; then + min=$(kill -l RTMIN) + max=$(kill -l RTMAX) + for number in "$min" "$((min + 1))" "$((max - 1))" "$max"; do + status=$((128 + number)) + printf '%s=%s\n' "$status" "$(kill -l "$status")" + done + fi + + - name: "kill -l realtime signals" + ignore_stderr: true + stdin: | + if test "$(uname -s)" != Linux; then + echo "realtime signals unsupported" + exit + fi + + min=$(kill -l RTMIN) + max=$(kill -l RTMAX) + span=$((max - min)) + + for signal in RTMIN RTMIN+0 RTMIN+1 RTMIN+$span RTMAX-1 RTMAX; do + printf '%s=%s\n' "$signal" "$(kill -l "$signal")" + done + for number in "$min" "$((min + 1))" "$((max - 1))" "$max"; do + printf '%s=%s\n' "$number" "$(kill -l "$number")" + done + + for signal in RTMIN+$((span + 1)) RTMAX-0 RTMAX-$span; do + kill -l "$signal" >/dev/null + printf '%s:%s\n' "$signal" "$?" + done + kill -l "$((max + 1))" >/dev/null + printf '%s:%s\n' "$((max + 1))" "$?" + - name: "kill -s" stdin: | kill -s USR1 $$ diff --git a/brush-shell/tests/cases/compat/builtins/trap.yaml b/brush-shell/tests/cases/compat/builtins/trap.yaml index 1b07f1c7b..df43e27a4 100644 --- a/brush-shell/tests/cases/compat/builtins/trap.yaml +++ b/brush-shell/tests/cases/compat/builtins/trap.yaml @@ -25,6 +25,34 @@ cases: trap "echo 4" 2 trap -p INT + - name: "trap realtime signal registration and validation" + ignore_stderr: true + stdin: | + if test "$(uname -s)" != Linux; then + echo "realtime signals unsupported" + exit + fi + + min=$(kill -l RTMIN) + max=$(kill -l RTMAX) + span=$((max - min)) + + for signal in RTMIN RTMIN+0 RTMIN+1 RTMIN+$span RTMAX-1 RTMAX; do + trap 'echo realtime' "$signal" + trap -p "$signal" + trap - "$signal" + done + for number in "$min" "$((min + 1))" "$((max - 1))" "$max"; do + trap 'echo numeric' "$number" + trap -p "$number" + trap - "$number" + done + + for signal in RTMIN+$((span + 1)) RTMAX-0 RTMAX-$span "$((max + 1))"; do + trap true "$signal" + printf '%s:%s\n' "$signal" "$?" + done + - name: "trap unregistering" stdin: | echo "[Case 1]"