diff --git a/src/header/time/mod.rs b/src/header/time/mod.rs index f0f69e9903..06d5ccc8e3 100644 --- a/src/header/time/mod.rs +++ b/src/header/time/mod.rs @@ -60,7 +60,6 @@ pub(crate) struct timer_internal_t { pub eventfd: usize, pub evp: sigevent, pub thread: platform::types::pthread_t, - pub caller_thread: crate::pthread::OsTid, /// relibc handles it_interval, not the kernel pub next_wake_time: itimerspec, /// kernel does not support unregistering timer @@ -72,8 +71,8 @@ pub(crate) struct timer_internal_t { #[cfg(target_os = "redox")] impl timer_internal_t { - pub unsafe fn from_raw(timerid: timer_t) -> &'static Mutex { - unsafe { &*(timerid as *const Mutex) } + pub unsafe fn from_raw(timerid: timer_t) -> &'static mut Self { + unsafe { &mut *(timerid as *mut Self) } } } @@ -584,9 +583,6 @@ pub unsafe extern "C" fn timer_create( /// See . #[unsafe(no_mangle)] pub unsafe extern "C" fn timer_delete(timerid: timer_t) -> c_int { - if timerid.is_null() { - return Err(Errno(EFAULT)).or_minus_one_errno(); - } Sys::timer_delete(timerid).map(|()| 0).or_minus_one_errno() } @@ -599,7 +595,7 @@ pub extern "C" fn timer_getoverrun(timerid: timer_t) -> c_int { /// See . #[unsafe(no_mangle)] pub unsafe extern "C" fn timer_gettime(timerid: timer_t, value: *mut itimerspec) -> c_int { - if timerid.is_null() || value.is_null() { + if value.is_null() { return Err(Errno(EFAULT)).or_minus_one_errno(); } let value = unsafe { Out::nonnull(value) }; @@ -616,7 +612,7 @@ pub unsafe extern "C" fn timer_settime( value: *const itimerspec, ovalue: *mut itimerspec, ) -> c_int { - if timerid.is_null() || value.is_null() { + if value.is_null() { return Err(Errno(EFAULT)).or_minus_one_errno(); } let (value, ovalue) = unsafe { (&*value, Out::nullable(ovalue)) }; diff --git a/src/header/unistd/alarm.rs b/src/header/unistd/alarm.rs index 8a999b5724..b0e1f6fd77 100644 --- a/src/header/unistd/alarm.rs +++ b/src/header/unistd/alarm.rs @@ -1,5 +1,3 @@ -#[cfg(target_os = "redox")] -use crate::header::time::timer_internal_t; use crate::{ header::{ bits_timespec::timespec, @@ -80,15 +78,6 @@ pub fn alarm_timespec(duration: timespec) -> c_uint { return remaining; } - // Enable process-wide signal delivery instead of thread-specific - #[cfg(target_os = "redox")] - { - let timer_ptr = unsafe { timer_internal_t::from_raw(timer_id) }; - let mut timer_st = timer_ptr.lock(); - timer_st.process_pid = Sys::getpid(); - drop(timer_st); - } - *guard = Some(AlarmTimer(timer_id)); } diff --git a/src/platform/linux/mod.rs b/src/platform/linux/mod.rs index fe127a213b..1b002995c9 100644 --- a/src/platform/linux/mod.rs +++ b/src/platform/linux/mod.rs @@ -721,7 +721,7 @@ impl Pal for Sys { syscall!( TIMER_CREATE, clock_id, - evp as *const _, + &raw const *evp, timerid.as_mut_ptr() ) }) @@ -747,7 +747,7 @@ impl Pal for Sys { TIMER_SETTIME, timerid, flags, - value as *const _, + &raw const *value, match ovalue { None => ptr::null_mut(), Some(mut o) => o.as_mut_ptr(), diff --git a/src/platform/redox/mod.rs b/src/platform/redox/mod.rs index 000f742b15..16c90f4595 100644 --- a/src/platform/redox/mod.rs +++ b/src/platform/redox/mod.rs @@ -2,7 +2,6 @@ use core::{ convert::TryFrom, mem::{self, size_of}, num::NonZeroU64, - ops::DerefMut, ptr, slice, str, }; use object::bytes_of_slice_mut; @@ -39,6 +38,7 @@ use crate::{ pthread::{pthread_cancel, pthread_create}, signal::{NSIG, SIGEV_NONE, SIGEV_SIGNAL, SIGEV_THREAD, SIGRTMIN, sigevent}, stdio::RENAME_NOREPLACE, + stdlib::posix_memalign, sys_file, sys_mman::{MAP_ANONYMOUS, PROT_READ, PROT_WRITE}, sys_random, @@ -56,8 +56,11 @@ use crate::{ io::{self, BufReader, prelude::*}, ld_so::tcb::OsSpecific, out::Out, - platform::sys::timer::{timer_routine, timer_update_wake_time}, - sync::{Mutex, rwlock::RwLock}, + platform::{ + free, + sys::timer::{TIMERS, timer_routine, timer_update_wake_time}, + }, + sync::rwlock::RwLock, }; pub use redox_rt::proc::FdGuard; @@ -1247,65 +1250,77 @@ impl Pal for Sys { event::redox_event_queue_create_v1(0) })?) .to_upper()?; - let caller_thread = Self::current_os_tid(); - let timer_buf = unsafe { - let timer_buf = Self::mmap( - ptr::null_mut(), - size_of::>(), - PROT_READ | PROT_WRITE, - MAP_ANONYMOUS, - 0, - 0, - )?; - - let timer_ptr = timer_internal_t::from_raw(timer_buf); - let mut timer_st = timer_ptr.lock(); - - timer_st.clockid = clock_id; - timer_st.timerfd = timerfd.take(); - timer_st.eventfd = eventfd.take(); - timer_st.evp = (*evp).clone(); - timer_st.next_wake_time = itimerspec::default(); - timer_st.thread = ptr::null_mut(); - timer_st.caller_thread = caller_thread; - timer_st.process_pid = 0; - timer_st.next_wake_version = 0; - drop(timer_st); - - timer_buf + let timer_st = timer_internal_t { + clockid: clock_id, + timerfd: timerfd.take(), + eventfd: eventfd.take(), + evp: (*evp).clone(), + thread: ptr::null_mut(), + next_wake_time: itimerspec::default(), + next_wake_version: 0, + process_pid: Sys::getpid(), + }; + let timers = &mut TIMERS.lock().0; + // allocate enough memory on the heap to store one timer_internal_t + let mut memory_pointer: *mut timer_internal_t = ptr::null_mut(); + unsafe { + let result = posix_memalign( + (&mut memory_pointer as *mut *mut timer_internal_t).cast(), + align_of::(), + size_of::(), + ); + assert_eq!(result, 0, "Failed to allocate or invalid alignment"); }; - timerid.write(timer_buf); + let pointer = { + ptr::NonNull::new(memory_pointer) + .expect("Pointer is guaranteed to not be null if posix_memalign returns 0") + }; + + // move value from the stack to the location we allocated on the heap + unsafe { + // Safety: If non-null, posix_memalign gives us a pointer that is valid + // for writes and properly aligned. + pointer.as_ptr().write(timer_st); + } + let timer_ptr = pointer.as_ptr() as timer_t; + timers.insert(timer_ptr); + + timerid.write(timer_ptr); Ok(()) } fn timer_delete(timerid: timer_t) -> Result<()> { - unsafe { - let timer_ptr = timer_internal_t::from_raw(timerid); - let timer_st = timer_ptr.lock(); - let _ = syscall::close(timer_st.timerfd); - let _ = syscall::close(timer_st.eventfd); - if !timer_st.thread.is_null() { - let _ = pthread_cancel(timer_st.thread); - } - drop(timer_st); - Self::munmap(timerid, size_of::>())?; + let timers = &mut TIMERS.lock().0; + let removed = timers.remove(&timerid); + if !removed { + return Err(Errno(EINVAL)); } + let timer_st = unsafe { timer_internal_t::from_raw(timerid) }; + let _ = syscall::close(timer_st.timerfd); + let _ = syscall::close(timer_st.eventfd); + if !timer_st.thread.is_null() { + let _ = unsafe { pthread_cancel(timer_st.thread) }; + } + unsafe { free(timerid) }; Ok(()) } fn timer_gettime(timerid: timer_t, mut value: Out) -> Result<()> { - let timer_ptr = unsafe { timer_internal_t::from_raw(timerid) }; - let mut timer_st = timer_ptr.lock(); + let timers = &mut TIMERS.lock().0; + if !timers.contains(&timerid) { + return Err(Errno(EINVAL)); + } + let timer_st = unsafe { timer_internal_t::from_raw(timerid) }; let mut now = timespec::default(); Self::clock_gettime(timer_st.clockid, Out::from_mut(&mut now))?; if timer_st.evp.sigev_notify == SIGEV_NONE { if timespec::subtract(&timer_st.next_wake_time.it_value, &now).is_none() { // error here means the timer is disarmed - let _ = timer_update_wake_time(timer_st.deref_mut()); + let _ = timer_update_wake_time(timer_st); } } let remaining = &timer_st.next_wake_time.it_value; @@ -1328,13 +1343,16 @@ impl Pal for Sys { value: &itimerspec, ovalue: Option>, ) -> Result<()> { - let timer_ptr = unsafe { timer_internal_t::from_raw(timerid) }; - let mut timer_st = timer_ptr.lock(); - if let Some(ovalue) = ovalue { Self::timer_gettime(timerid, ovalue)?; } + let timers = &mut TIMERS.lock().0; + if !timers.contains(&timerid) { + return Err(Errno(EINVAL)); + } + let timer_st = unsafe { timer_internal_t::from_raw(timerid) }; + if value.it_value.is_zero() { timer_st.next_wake_version += 1; return Ok(()); diff --git a/src/platform/redox/timer.rs b/src/platform/redox/timer.rs index 34ec81b168..585ab0aa42 100644 --- a/src/platform/redox/timer.rs +++ b/src/platform/redox/timer.rs @@ -8,18 +8,37 @@ use crate::{ time::{timer_internal_t, timespec}, }, out::Out, - platform::{Pal, Sys, sys::event, types::c_void}, + platform::{ + Pal, PalSignal, Sys, + sys::event, + types::{c_void, timer_t}, + }, + sync::Mutex, }; +use alloc::collections::BTreeSet; use core::{ mem::{MaybeUninit, size_of}, ops::DerefMut, ptr, }; +pub static TIMERS: Mutex>> = + Mutex::new(ForceSendSync(BTreeSet::new())); + +unsafe impl Send for timer_internal_t {} +unsafe impl Sync for timer_internal_t {} +#[derive(Clone, Copy, Eq, Ord, PartialEq, PartialOrd)] +pub struct ForceSendSync(pub(crate) T); +unsafe impl Send for ForceSendSync {} +unsafe impl Sync for ForceSendSync {} + pub extern "C" fn timer_routine(arg: *mut c_void) -> *mut c_void { - let timer_ptr = unsafe { timer_internal_t::from_raw(arg) }; let (mut timer_version, eventfd) = { - let timer_st = timer_ptr.lock(); + let timers = &mut TIMERS.lock().0; + if !timers.contains(&arg) { + return ptr::null_mut(); + } + let timer_st = unsafe { timer_internal_t::from_raw(arg) }; (timer_st.next_wake_version, timer_st.eventfd) }; loop { @@ -41,18 +60,23 @@ pub extern "C" fn timer_routine(arg: *mut c_void) -> *mut c_void { break; } - let mut timer_st = timer_ptr.lock(); + let timers = &mut TIMERS.lock().0; + if !timers.contains(&arg) { + return ptr::null_mut(); + } + let mut timer_st = unsafe { timer_internal_t::from_raw(arg) }; if timer_version == timer_st.next_wake_version { if timer_st.evp.sigev_notify == SIGEV_THREAD { if let Some(fun) = timer_st.evp.sigev_notify_function { fun(timer_st.evp.sigev_value); } } else if timer_st.evp.sigev_notify == SIGEV_SIGNAL { - // TODO: This will deliver signal to process, which is required for alarm() - // Until it can bypass the exec() boundary, do not uncomment this code - // if timer_st.process_pid != 0 && Sys::kill(timer_st.process_pid, timer_st.evp.sigev_signo).is_err() { break; } else - if unsafe { Sys::rlct_kill(timer_st.caller_thread, timer_st.evp.sigev_signo as _) } - .is_err() + if Sys::sigqueue( + timer_st.process_pid, + timer_st.evp.sigev_signo as _, + timer_st.evp.sigev_value, + ) + .is_err() { break; } diff --git a/tests/Makefile.tests.mk b/tests/Makefile.tests.mk index 5264e65a21..634884d7c2 100644 --- a/tests/Makefile.tests.mk +++ b/tests/Makefile.tests.mk @@ -163,6 +163,7 @@ EXPECT_NAMES=\ time/strptime \ time/time \ time/timegm \ + time/timer \ time/tzset \ unistd/access \ unistd/alarm \ diff --git a/tests/expected/bins_dynamic/time/timer.stdout b/tests/expected/bins_dynamic/time/timer.stdout new file mode 100644 index 0000000000..750a6c012d --- /dev/null +++ b/tests/expected/bins_dynamic/time/timer.stdout @@ -0,0 +1,5 @@ +invalid_timer: ok +timer_create: ok +timer_gettime: ok +timer_settime: ok +timer_delete: ok diff --git a/tests/expected/bins_static/time/timer.stdout b/tests/expected/bins_static/time/timer.stdout new file mode 100644 index 0000000000..750a6c012d --- /dev/null +++ b/tests/expected/bins_static/time/timer.stdout @@ -0,0 +1,5 @@ +invalid_timer: ok +timer_create: ok +timer_gettime: ok +timer_settime: ok +timer_delete: ok diff --git a/tests/expected/time/timer.stdout b/tests/expected/time/timer.stdout new file mode 100644 index 0000000000..750a6c012d --- /dev/null +++ b/tests/expected/time/timer.stdout @@ -0,0 +1,5 @@ +invalid_timer: ok +timer_create: ok +timer_gettime: ok +timer_settime: ok +timer_delete: ok diff --git a/tests/time/timer.c b/tests/time/timer.c new file mode 100644 index 0000000000..29ca47294c --- /dev/null +++ b/tests/time/timer.c @@ -0,0 +1,99 @@ +#include +#include +#include +#include +#include + +#include "test_helpers.h" + +static volatile sig_atomic_t alarm_count = 0; + +static void handler(int sig) { + (void)sig; + alarm_count++; +} + +int main(void) { + long COUNTDOWN_MILLISECONDS = 100; + unsigned int SLEEP_MILLISECONDS = 110; + + struct sigaction sa; + sa.sa_handler = handler; + sa.sa_flags = 0; + sigemptyset(&sa.sa_mask); + int r = sigaction(SIGALRM, &sa, NULL); + ERROR_IF(sigaction, r, == -1); + + struct sigevent signal_event = {0}; + signal_event.sigev_signo = SIGALRM; + signal_event.sigev_notify = SIGEV_SIGNAL; + + timer_t timerid = {0}; + int status = 0; + struct itimerspec current_timer_spec = {0}; + struct itimerspec new_timer_spec = {0}; + new_timer_spec.it_value.tv_sec = 0; + new_timer_spec.it_value.tv_nsec = COUNTDOWN_MILLISECONDS * 1000000; + + + // use an invalid timer + status = timer_gettime(timerid, ¤t_timer_spec); + ERROR_IF(timer_gettime, status, == 0); + status = timer_settime(timerid, 0, &new_timer_spec, NULL); + ERROR_IF(timer_settime, status, == 0); + printf("invalid_timer: ok\n"); + + // create a timer + status = timer_create(CLOCK_MONOTONIC, &signal_event, &timerid); + ERROR_IF(timer_create, status, == -1); + + printf("timer_create: ok\n"); + + // check that no timer has been configured yet + status = timer_gettime(timerid, ¤t_timer_spec); + ERROR_IF(timer_gettime, status, == -1); + assert(current_timer_spec.it_value.tv_sec == 0); + assert(current_timer_spec.it_value.tv_nsec == 0); + + printf("timer_gettime: ok\n"); + + // start a timer + status = timer_settime(timerid, 0, &new_timer_spec, ¤t_timer_spec); + ERROR_IF(timer_settime, status, == -1); + // check that there has been no previous timer + assert(current_timer_spec.it_value.tv_sec == 0); + assert(current_timer_spec.it_value.tv_nsec == 0); + + // timer_gettime reports the timer + status = timer_gettime(timerid, ¤t_timer_spec); + assert(current_timer_spec.it_value.tv_sec == 0); + assert(current_timer_spec.it_value.tv_nsec > 0); + assert(current_timer_spec.it_value.tv_nsec <= COUNTDOWN_MILLISECONDS * 1000000); + + // timer fires + usleep(SLEEP_MILLISECONDS * 1000); + assert(alarm_count == 1); + + // timer_gettime reports no timer any more + status = timer_gettime(timerid, ¤t_timer_spec); + assert(current_timer_spec.it_value.tv_sec == 0); + assert(current_timer_spec.it_value.tv_nsec == 0); + + printf("timer_settime: ok\n"); + + // delete the timer + status = timer_delete(timerid); + ERROR_IF(timer_delete, status, == -1); + + // any attempts to use the timerid should report EINVAL + status = timer_gettime(timerid, ¤t_timer_spec); // must fail + ERROR_IF(timer_delete, status, == 0); + assert(errno == EINVAL); + status = timer_settime(timerid, 0, &new_timer_spec, ¤t_timer_spec); + ERROR_IF(timer_delete, status, == 0); + assert(errno == EINVAL); + + printf("timer_delete: ok\n", status); + + return EXIT_SUCCESS; +}