Kernel/LibC: Cleanup, fix and implement a lot of signal code

This patch implements sigsets and some of their usages

kernel/include/kernel/Process.h

@@ -174,8 +174,10 @@ namespace Kernel
 
 		BAN::ErrorOr<long> sys_tty_ctrl(int fildes, int command, int flags);
 
-		BAN::ErrorOr<long> sys_signal(int, void (*)(int));
 		BAN::ErrorOr<long> sys_kill(pid_t pid, int signal);
+		BAN::ErrorOr<long> sys_sigaction(int signal, const struct sigaction* act, struct sigaction* oact);
+		BAN::ErrorOr<long> sys_sigpending(sigset_t* set);
+		BAN::ErrorOr<long> sys_sigprocmask(int how, const sigset_t* set, sigset_t* oset);
 
 		BAN::ErrorOr<long> sys_tcsetpgrp(int fd, pid_t pgid);
 
@@ -220,7 +222,8 @@ namespace Kernel
 
 		uint64_t signal_pending_mask() const
 		{
-			return ((uint64_t)m_signal_pending_mask[1].load() << 32) | m_signal_pending_mask[0].load();
+			SpinLockGuard _(m_signal_lock);
+			return m_signal_pending_mask;
 		}
 
 		void add_pending_signal(uint8_t signal)
@@ -228,15 +231,13 @@ namespace Kernel
 			ASSERT(signal >= _SIGMIN);
 			ASSERT(signal <= _SIGMAX);
 			ASSERT(signal < 64);
-			vaddr_t handler = m_signal_handlers[signal];
+			SpinLockGuard _(m_signal_lock);
-			if (handler == (vaddr_t)SIG_IGN)
+			auto handler = m_signal_handlers[signal].sa_handler;
+			if (handler == SIG_IGN)
 				return;
-			if (handler == (vaddr_t)SIG_DFL && (signal == SIGCHLD || signal == SIGURG))
+			if (handler == SIG_DFL && (signal == SIGCHLD || signal == SIGURG))
 				return;
-			if (signal < 32)
+			m_signal_pending_mask |= 1ull << signal;
-				m_signal_pending_mask[0] |= (uint32_t)1 << signal;
-			else
-				m_signal_pending_mask[1] |= (uint32_t)1 << (signal - 32);
 		}
 
 		void remove_pending_signal(uint8_t signal)
@@ -244,10 +245,8 @@ namespace Kernel
 			ASSERT(signal >= _SIGMIN);
 			ASSERT(signal <= _SIGMAX);
 			ASSERT(signal < 64);
-			if (signal < 32)
+			SpinLockGuard _(m_signal_lock);
-				m_signal_pending_mask[0] &= ~((uint32_t)1 << signal);
+			m_signal_pending_mask &= ~(1ull << signal);
-			else
-				m_signal_pending_mask[1] &= ~((uint32_t)1 << (signal - 32));
 		}
 
 	private:
@@ -276,9 +275,9 @@ namespace Kernel
 		BAN::String m_working_directory;
 		BAN::Vector<Thread*> m_threads;
 
-		BAN::Atomic<vaddr_t> m_signal_handlers[_SIGMAX + 1] { };
+		mutable SpinLock m_signal_lock;
-		// This is 2 32 bit values to allow atomicity on 32 targets
+		struct sigaction m_signal_handlers[_SIGMAX + 1] { };
-		BAN::Atomic<uint32_t> m_signal_pending_mask[2] { 0, 0 };
+		uint64_t m_signal_pending_mask { 0 };
 
 		BAN::Vector<BAN::String> m_cmdline;
 		BAN::Vector<BAN::String> m_environ;

kernel/kernel/IDT.cpp

@@ -338,6 +338,10 @@ done:
 		ASSERT(InterruptController::get().is_in_service(IRQ_TIMER));
 		InterruptController::get().eoi(IRQ_TIMER);
 		Processor::scheduler().timer_interrupt();
+
+		auto& current_thread = Thread::current();
+		if (current_thread.can_add_signal_to_execute())
+			current_thread.handle_signal();
 	}
 
 	extern "C" void cpp_irq_handler(uint32_t irq)

kernel/kernel/Process.cpp

@@ -173,7 +173,10 @@ namespace Kernel
 		, m_parent(parent)
 	{
 		for (size_t i = 0; i < sizeof(m_signal_handlers) / sizeof(*m_signal_handlers); i++)
-			m_signal_handlers[i] = (vaddr_t)SIG_DFL;
+		{
+			m_signal_handlers[i].sa_handler = SIG_DFL;
+			m_signal_handlers[i].sa_flags = 0;
+		}
 	}
 
 	Process::~Process()
@@ -472,7 +475,10 @@ namespace Kernel
 			m_userspace_info.entry = m_loadable_elf->entry_point();
 
 			for (size_t i = 0; i < sizeof(m_signal_handlers) / sizeof(*m_signal_handlers); i++)
-				m_signal_handlers[i] = (vaddr_t)SIG_DFL;
+			{
+				m_signal_handlers[i].sa_handler = SIG_DFL;
+				m_signal_handlers[i].sa_flags = 0;
+			}
 
 			ASSERT(m_threads.size() == 1);
 			ASSERT(&Process::current() == this);
@@ -1640,20 +1646,6 @@ namespace Kernel
 		return 0;
 	}
 
-	BAN::ErrorOr<long> Process::sys_signal(int signal, void (*handler)(int))
-	{
-		if (signal < _SIGMIN || signal > _SIGMAX)
-			return BAN::Error::from_errno(EINVAL);
-
-		{
-			LockGuard _(m_process_lock);
-			TRY(validate_pointer_access((void*)handler, sizeof(handler)));
-		}
-
-		m_signal_handlers[signal] = (vaddr_t)handler;
-		return 0;
-	}
-
 	BAN::ErrorOr<long> Process::sys_kill(pid_t pid, int signal)
 	{
 		if (pid == 0 || pid == -1)
@@ -1690,6 +1682,81 @@ namespace Kernel
 		return BAN::Error::from_errno(ESRCH);
 	}
 
+	BAN::ErrorOr<long> Process::sys_sigaction(int signal, const struct sigaction* act, struct sigaction* oact)
+	{
+		if (signal < _SIGMIN || signal > _SIGMAX)
+			return BAN::Error::from_errno(EINVAL);
+
+		LockGuard _(m_process_lock);
+		if (act)
+			TRY(validate_pointer_access(act, sizeof(struct sigaction)));
+		if (oact)
+			TRY(validate_pointer_access(oact, sizeof(struct sigaction)));
+
+		SpinLockGuard signal_lock_guard(m_signal_lock);
+
+		if (oact)
+			*oact = m_signal_handlers[signal];
+
+		if (act)
+		{
+			if (act->sa_flags)
+				return BAN::Error::from_errno(ENOTSUP);
+			m_signal_handlers[signal] = *act;
+		}
+
+		return 0;
+	}
+
+	BAN::ErrorOr<long> Process::sys_sigpending(sigset_t* set)
+	{
+		LockGuard _(m_process_lock);
+		TRY(validate_pointer_access(set, sizeof(sigset_t)));
+		*set = (signal_pending_mask() | Thread::current().m_signal_pending_mask) & Thread::current().m_signal_block_mask;
+		return 0;
+	}
+
+	BAN::ErrorOr<long> Process::sys_sigprocmask(int how, const sigset_t* set, sigset_t* oset)
+	{
+		switch (how)
+		{
+			case SIG_BLOCK:
+			case SIG_SETMASK:
+			case SIG_UNBLOCK:
+				break;
+			default:
+				return BAN::Error::from_errno(EINVAL);
+		}
+
+		LockGuard _(m_process_lock);
+		if (set)
+			TRY(validate_pointer_access(set, sizeof(sigset_t)));
+		if (oset)
+			TRY(validate_pointer_access(oset, sizeof(sigset_t)));
+
+		if (oset)
+			*oset = Thread::current().m_signal_block_mask;
+
+		if (set)
+		{
+			const sigset_t mask = *set & ~(SIGKILL | SIGSTOP);
+			switch (how)
+			{
+				case SIG_BLOCK:
+					Thread::current().m_signal_block_mask |= mask;
+					break;
+				case SIG_SETMASK:
+					Thread::current().m_signal_block_mask  = mask;
+					break;
+				case SIG_UNBLOCK:
+					Thread::current().m_signal_block_mask &= mask;
+					break;
+			}
+		}
+
+		return 0;
+	}
+
 	BAN::ErrorOr<long> Process::sys_tcsetpgrp(int fd, pid_t pgrp)
 	{
 		LockGuard _(m_process_lock);

kernel/kernel/Thread.cpp

@@ -266,10 +266,16 @@ namespace Kernel
 		{
 			if (!(signals & ((uint64_t)1 << i)))
 				continue;
-			vaddr_t handler = m_process->m_signal_handlers[i];
+
-			if (handler == (vaddr_t)SIG_IGN)
+			vaddr_t signal_handler;
+			{
+				SpinLockGuard _(m_process->m_signal_lock);
+				ASSERT(!(m_process->m_signal_handlers[i].sa_flags & SA_SIGINFO));
+				signal_handler = (vaddr_t)m_process->m_signal_handlers[i].sa_handler;
+			}
+			if (signal_handler == (vaddr_t)SIG_IGN)
 				continue;
-			if (handler == (vaddr_t)SIG_DFL && (i == SIGCHLD || i == SIGURG))
+			if (signal_handler == (vaddr_t)SIG_DFL && (i == SIGCHLD || i == SIGURG))
 				continue;
 			return true;
 		}
@@ -316,7 +322,12 @@ namespace Kernel
 			ASSERT(signal <= _SIGMAX);
 		}
 
-		vaddr_t signal_handler = process().m_signal_handlers[signal];
+		vaddr_t signal_handler;
+		{
+			SpinLockGuard _(m_process->m_signal_lock);
+			ASSERT(!(m_process->m_signal_handlers[signal].sa_flags & SA_SIGINFO));
+			signal_handler = (vaddr_t)m_process->m_signal_handlers[signal].sa_handler;
+		}
 
 		m_signal_pending_mask &= ~(1ull << signal);
 		process().remove_pending_signal(signal);
@@ -390,10 +401,15 @@ namespace Kernel
 		SpinLockGuard _(m_signal_lock);
 		if (m_process)
 		{
-			vaddr_t handler = m_process->m_signal_handlers[signal];
+			vaddr_t signal_handler;
-			if (handler == (vaddr_t)SIG_IGN)
+			{
+				SpinLockGuard _(m_process->m_signal_lock);
+				ASSERT(!(m_process->m_signal_handlers[signal].sa_flags & SA_SIGINFO));
+				signal_handler = (vaddr_t)m_process->m_signal_handlers[signal].sa_handler;
+			}
+			if (signal_handler == (vaddr_t)SIG_IGN)
 				return false;
-			if (handler == (vaddr_t)SIG_DFL && (signal == SIGCHLD || signal == SIGURG))
+			if (signal_handler == (vaddr_t)SIG_DFL && (signal == SIGCHLD || signal == SIGURG))
 				return false;
 		}
 		uint64_t mask = 1ull << signal;

userspace/libraries/LibC/include/signal.h

@@ -22,7 +22,7 @@ __BEGIN_DECLS
 #include <sys/types.h>
 
 typedef int sig_atomic_t;
-typedef void* sigset_t;
+typedef unsigned long long sigset_t;
 
 union sigval
 {

userspace/libraries/LibC/include/sys/syscall.h

@@ -40,7 +40,6 @@ __BEGIN_DECLS
 	O(SYS_DUP,				dup)			\
 	O(SYS_DUP2,				dup2)			\
 	O(SYS_KILL,				kill)			\
-	O(SYS_SIGNAL,			signal)			\
 	O(SYS_TCSETPGRP,		tcsetpgrp)		\
 	O(SYS_GET_PID,			getpid)			\
 	O(SYS_GET_PGID,			getpgid)		\
@@ -84,6 +83,9 @@ __BEGIN_DECLS
 	O(SYS_REALPATH,			realpath)		\
 	O(SYS_TTYNAME,			ttyname)		\
 	O(SYS_ACCESS,			access)			\
+	O(SYS_SIGACTION,		sigaction)		\
+	O(SYS_SIGPENDING,		sigpending)		\
+	O(SYS_SIGPROCMASK,		sigprocmask)	\
 
 enum Syscall
 {

userspace/libraries/LibC/signal.cpp

@@ -1,20 +1,67 @@
+#include <BAN/Assert.h>
 #include <signal.h>
 #include <sys/syscall.h>
 #include <unistd.h>
 
+static_assert(sizeof(sigset_t) * 8 >= _SIGMAX);
+
+int kill(pid_t pid, int sig)
+{
+	return syscall(SYS_KILL, pid, sig);
+}
+
 int raise(int sig)
 {
 	// FIXME: won't work after multithreaded
 	return kill(getpid(), sig);
 }
 
-int kill(pid_t pid, int sig)
+int sigaction(int sig, const struct sigaction* __restrict act, struct sigaction* __restrict oact)
 {
-	return syscall(SYS_KILL, pid, sig);
+	return syscall(SYS_SIGACTION, sig, act, oact);
+}
+
+int sigaddset(sigset_t* set, int signo)
+{
+	*set |= 1ull << signo;
+	return 0;
+}
+
+int sigemptyset(sigset_t* set)
+{
+	*set = 0;
+	return 0;
+}
+
+int sigfillset(sigset_t* set)
+{
+	*set = (1ull << _SIGMAX) - 1;
+	return 0;
+}
+
+int sigismember(const sigset_t* set, int signo)
+{
+	return (*set >> signo) & 1;
 }
 
 void (*signal(int sig, void (*func)(int)))(int)
 {
-	long ret = syscall(SYS_SIGNAL, sig, func);
+	struct sigaction act;
-	return (void (*)(int))ret;
+	act.sa_handler = func;
+	act.sa_flags = 0;
+
+	int ret = sigaction(sig, &act, nullptr);
+	if (ret == -1)
+		return SIG_ERR;
+	return func;
+}
+
+int sigpending(sigset_t* set)
+{
+	return syscall(SYS_SIGPENDING, set);
+}
+
+int sigprocmask(int how, const sigset_t* __restrict set, sigset_t* __restrict oset)
+{
+	return syscall(SYS_SIGPROCMASK, how, set, oset);
 }