From c12f4fb40f2b92cd7685c3724c03e7739e094562 Mon Sep 17 00:00:00 2001 From: Bananymous Date: Fri, 21 Jul 2023 19:54:37 +0300 Subject: [PATCH] Kernel: Make signals more POSIX --- kernel/include/kernel/Process.h | 4 ++++ kernel/include/kernel/Thread.h | 7 ++++--- kernel/kernel/Process.cpp | 19 ++++++++++++++++++- kernel/kernel/Scheduler.cpp | 2 +- kernel/kernel/Thread.cpp | 26 +++++++++++++++++++++----- libc/include/signal.h | 8 ++++---- 6 files changed, 52 insertions(+), 14 deletions(-) diff --git a/kernel/include/kernel/Process.h b/kernel/include/kernel/Process.h index 9f2ae3a8aa..1f2a629c05 100644 --- a/kernel/include/kernel/Process.h +++ b/kernel/include/kernel/Process.h @@ -155,11 +155,15 @@ namespace Kernel BAN::Vector> m_fixed_width_allocators; BAN::UniqPtr m_general_allocator; + vaddr_t m_signal_handlers[_SIGMAX + 1] { }; + userspace_info_t m_userspace_info; ExitStatus m_exit_status; BAN::UniqPtr m_page_table; BAN::RefPtr m_tty; + + friend class Thread; }; } \ No newline at end of file diff --git a/kernel/include/kernel/Thread.h b/kernel/include/kernel/Thread.h index 5d4605cfce..4bf01aa951 100644 --- a/kernel/include/kernel/Thread.h +++ b/kernel/include/kernel/Thread.h @@ -37,6 +37,7 @@ namespace Kernel BAN::ErrorOr clone(Process*, uintptr_t rsp, uintptr_t rip); void setup_exec(); + bool has_signal_to_execute() const; void handle_next_signal(); void handle_signal(int signal, uintptr_t& return_rsp, uintptr_t& return_rip); @@ -88,9 +89,9 @@ namespace Kernel bool m_in_syscall { false }; bool m_is_userspace { false }; - BAN::CircularQueue m_signal_queue; - vaddr_t m_signal_handlers[_SIGMAX + 1] { }; - uint64_t m_signal_mask { (1ull << SIGCHLD) | (1ull << SIGURG) }; + BAN::CircularQueue m_signal_queue; + uint64_t m_signal_mask { 0 }; + bool m_handling_signal { false }; static_assert(_SIGMAX < 64); friend class Process; diff --git a/kernel/kernel/Process.cpp b/kernel/kernel/Process.cpp index b346ca7eb4..d91fff62df 100644 --- a/kernel/kernel/Process.cpp +++ b/kernel/kernel/Process.cpp @@ -97,7 +97,10 @@ namespace Kernel , m_open_file_descriptors(m_credentials) , m_pid(pid) , m_tty(TTY::current()) - { } + { + for (size_t i = 0; i < sizeof(m_signal_handlers) / sizeof(*m_signal_handlers); i++) + m_signal_handlers[i] = (vaddr_t)SIG_DFL; + } Process::~Process() { @@ -303,6 +306,7 @@ namespace Kernel forked->m_fixed_width_allocators = BAN::move(fixed_width_allocators); forked->m_general_allocator = BAN::move(general_allocator); forked->m_userspace_info = m_userspace_info; + memcpy(forked->m_signal_handlers, m_signal_handlers, sizeof(m_signal_handlers)); ASSERT(this == &Process::current()); // FIXME: this should be able to fail @@ -352,6 +356,9 @@ namespace Kernel m_userspace_info.entry = elf->file_header_native().e_entry; + for (size_t i = 0; i < sizeof(m_signal_handlers) / sizeof(*m_signal_handlers); i++) + m_signal_handlers[i] = (vaddr_t)SIG_DFL; + // NOTE: we clear the elf since we don't need the memory anymore elf.clear(); @@ -789,6 +796,16 @@ namespace Kernel return 0; } + BAN::ErrorOr Process::sys_signal(int signal, void (*handler)(int)) + { + if (signal < _SIGMIN || signal > _SIGMAX) + return BAN::Error::from_errno(EINVAL); + + CriticalScope _; + m_signal_handlers[signal] = (vaddr_t)handler; + return 0; + } + BAN::ErrorOr Process::sys_kill(pid_t pid, int signal, uintptr_t& return_rsp, uintptr_t& return_rip) { if (pid <= 0) diff --git a/kernel/kernel/Scheduler.cpp b/kernel/kernel/Scheduler.cpp index eb5b209249..9b3d1d45c2 100644 --- a/kernel/kernel/Scheduler.cpp +++ b/kernel/kernel/Scheduler.cpp @@ -212,7 +212,7 @@ namespace Kernel current.set_started(); start_thread(current.rsp(), current.rip()); case Thread::State::Executing: - while (!current.m_signal_queue.empty()) + while (current.has_signal_to_execute()) current.handle_next_signal(); continue_thread(current.rsp(), current.rip()); case Thread::State::Terminating: diff --git a/kernel/kernel/Thread.cpp b/kernel/kernel/Thread.cpp index 858d486e28..b1a5091ee6 100644 --- a/kernel/kernel/Thread.cpp +++ b/kernel/kernel/Thread.cpp @@ -133,6 +133,8 @@ namespace Kernel m_rsp = stack_base() + stack_size(); m_rip = (uintptr_t)entry_trampoline; + // Signal mask is inherited + // Setup stack for returning { // FIXME: don't use PageTableScope @@ -143,6 +145,11 @@ namespace Kernel } } + bool Thread::has_signal_to_execute() const + { + return !m_signal_queue.empty() && !m_handling_signal; + } + void Thread::handle_next_signal() { ASSERT(!interrupts_enabled()); @@ -158,16 +165,24 @@ namespace Kernel ASSERT(!m_signal_queue.empty()); ASSERT(m_signal_queue.front() == signal); - // Skip masked (ignored) signals - if (m_signal_mask & (1ull << signal)) - return; + vaddr_t signal_handler = process().m_signal_handlers[signal]; - if (m_signal_handlers[signal]) + m_handling_signal = true; + + // Skip masked and ignored signals + if (m_signal_mask & (1ull << signal)) + ; + else if (signal_handler == (vaddr_t)SIG_IGN) + ; + else if (signal_handler != (vaddr_t)SIG_DFL) { write_to_stack(return_rsp, return_rip); write_to_stack(return_rsp, signal); - write_to_stack(return_rsp, m_signal_handlers[signal]); + write_to_stack(return_rsp, signal_handler); return_rip = (uintptr_t)signal_trampoline; + + // FIXME: we should only mark this signal as done when + // handler returns } else { @@ -226,6 +241,7 @@ namespace Kernel } m_signal_queue.pop(); + m_handling_signal = false; } void Thread::validate_stack() const diff --git a/libc/include/signal.h b/libc/include/signal.h index a1a042a4f6..f555c7426a 100644 --- a/libc/include/signal.h +++ b/libc/include/signal.h @@ -9,10 +9,10 @@ __BEGIN_DECLS #include -#define SIG_DFL 1 -#define SIG_ERR 2 -#define SIG_HOLD 3 -#define SIG_IGN 4 +#define SIG_DFL ((void (*)(int))0) +#define SIG_ERR ((void (*)(int))1) +#define SIG_HOLD ((void (*)(int))2) +#define SIG_IGN ((void (*)(int))3) #define __need_pthread_t #define __need_size_t