From a728651f04320792272030105dbdb10061a2278a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Axel=20D=C3=B6rfler?= Date: Fri, 17 Aug 2007 11:01:09 +0000 Subject: [PATCH] Some more work on the signal code: * get_signal_stack() checked the wrong sig_action field (index is signal-1), also, it had an off-by-one error in the stack range check. * factored out a restart_syscall() function to avoid code duplication * arch_setup_signal_frame() relied on the fact that vregs and the signal stack code is a multiple of 4 bytes in size. * Fixed sigaction(): it did return the error code directly instead of setting errno. * signal() actually had a work-around for the broken sigaction()... * Replaced the sig_func_t typedef with a sighandler_t typedef - this is non-standard anyway, but now we're at least compatible with the GNU world instead of introducing our own solution (BSD seems to use sig_t here, BTW). * Removed now unused sigval structure from the header; it should be added again as soon as we start supporting it. * SA_RESETHAND and SA_ONESHOT are the same thing; the former did not work before. * Made the non-standard SA_* flags refer to the standard ones instead of the other way around. * Added a test application for various signal features - works fine under Haiku, tested also under Linux and BeOS (the latter fails as it does not support SA_RESTART). More tests should be added, though. * Cleanup. git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@21999 a95241bf-73f2-0310-859d-f6bbb57e9c96 --- headers/posix/signal.h | 96 ++++++--------- src/system/kernel/arch/x86/arch_thread.c | 64 +++++----- src/system/libroot/posix/signal/sigaction.c | 20 +++- src/system/libroot/posix/signal/signal.c | 23 ++-- src/system/libroot/posix/signal/sigset.c | 31 ++--- src/system/libroot/posix/signal/strsignal.c | 29 ++--- src/tests/system/libroot/posix/Jamfile | 4 + .../system/libroot/posix/signal_test.cpp | 111 ++++++++++++++++++ 8 files changed, 234 insertions(+), 144 deletions(-) create mode 100644 src/tests/system/libroot/posix/signal_test.cpp diff --git a/headers/posix/signal.h b/headers/posix/signal.h index d000688c55..6fea23f70d 100644 --- a/headers/posix/signal.h +++ b/headers/posix/signal.h @@ -12,17 +12,20 @@ typedef int sig_atomic_t; typedef long sigset_t; -typedef void (*sig_func_t)(int); -typedef void (*__signal_func_ptr)(int); /* deprecated, for compatibility with BeOS only */ +typedef void (*sighandler_t)(int); + /* GNU-like signal handler typedef */ + +typedef void (*__signal_func_ptr)(int); + /* deprecated, for compatibility with BeOS only */ -typedef union sigval { - int sival_int; - void *sival_ptr; -} sigval_t; +/* macros defining the standard signal handling behavior */ +#define SIG_DFL ((sighandler_t)0) /* "default" signal behaviour */ +#define SIG_IGN ((sighandler_t)1) /* ignore signal */ +#define SIG_ERR ((sighandler_t)-1) /* an error occurred during signal processing */ +#define SIG_HOLD ((sighandler_t)3) /* the signal was hold */ - -// ToDo: actually make use of this structure! +// TODO: support this structure! typedef struct { int si_signo; /* signal number */ int si_code; /* signal code */ @@ -32,45 +35,32 @@ typedef struct { void *si_addr; /* address of faulting instruction */ int si_status; /* exit value or signal */ long si_band; /* band event for SIGPOLL */ - union sigval si_value; /* signal value */ } siginfo_t; - -/* - * macros defining the standard signal handling behavior - */ -#define SIG_DFL ((sig_func_t) 0) /* the signal was treated in the "default" manner */ -#define SIG_IGN ((sig_func_t) 1) /* the signal was ignored */ -#define SIG_ERR ((sig_func_t)-1) /* an error occurred during signal processing */ -#define SIG_HOLD ((sig_func_t) 3) /* the signal was hold */ - - /* * structure used by sigaction() * - * Note: the 'sa_userdata' field is a non-Posix extension. - * See the SPECIAL NOTES below for an explanation of this. - * + * Note: the 'sa_userdata' field is a non-POSIX extension. + * See the documentation for more info on this. */ struct sigaction { - sig_func_t sa_handler; + sighandler_t sa_handler; sigset_t sa_mask; int sa_flags; void *sa_userdata; /* will be passed to the signal handler */ }; - /* values for sa_flags */ #define SA_NOCLDSTOP 0x01 -#define SA_ONESHOT 0x02 -#define SA_NOMASK 0x04 -#define SA_NODEFER SA_NOMASK -#define SA_RESTART 0x08 -#define SA_STACK 0x10 -#define SA_ONSTACK SA_STACK -#define SA_RESETHAND 0x20 +#define SA_NOCLDWAIT 0x02 +#define SA_RESETHAND 0x04 +#define SA_NODEFER 0x08 +#define SA_RESTART 0x10 +#define SA_ONSTACK 0x20 #define SA_SIGINFO 0x40 -#define SA_NOCLDWAIT 0x80 +#define SA_NOMASK SA_NODEFER +#define SA_STACK SA_ONSTACK +#define SA_ONESHOT SA_RESETHAND /* values for ss_flags */ #define SS_ONSTACK 0x1 @@ -94,14 +84,14 @@ typedef struct sigstack { } sigstack; /* for the 'how' arg of sigprocmask() */ -#define SIG_BLOCK 1 -#define SIG_UNBLOCK 2 -#define SIG_SETMASK 3 +#define SIG_BLOCK 1 +#define SIG_UNBLOCK 2 +#define SIG_SETMASK 3 /* * The list of all defined signals: * - * The numbering of signals for OpenBeOS attempts to maintain + * The numbering of signals for Haiku attempts to maintain * some consistency with UN*X conventions so that things * like "kill -9" do what you expect. */ @@ -142,9 +132,10 @@ typedef struct sigstack { * releases. Use them at your own peril (if you do use them, at least * be smart and use them backwards from signal 32). */ -#define MAX_SIGNO 32 /* the most signals that a single thread can reference */ -#define __signal_max 29 /* the largest signal number that is actually defined */ -#define NSIG (__signal_max+1) /* the number of defined signals */ +#define MAX_SIGNO 32 /* the most signals that a single thread can reference */ +#define __signal_max 29 /* the largest signal number that is actually defined */ +#define NSIG (__signal_max+1) + /* the number of defined signals */ /* the global table of text strings containing descriptions for each signal */ @@ -155,7 +146,7 @@ extern const char * const sys_siglist[NSIG]; extern "C" { #endif -sig_func_t signal(int sig, sig_func_t signal_handler); +sighandler_t signal(int sig, sighandler_t signalHandler); int raise(int sig); int kill(pid_t pid, int sig); int send_signal(pid_t tid, unsigned int sig); @@ -205,7 +196,7 @@ sigdelset(sigset_t *set, int sig) } #endif -/* +/* TODO: move this into the documentation! * ================================================== * !!! SPECIAL NOTES CONCERNING NON-POSIX EXTENSIONS: * ================================================== @@ -226,7 +217,7 @@ sigdelset(sigset_t *set, int sig) * handling. It also allows an opportunity, via the 'sigaction' struct, to * enable additional data to be passed to the handler. For example: * void - * my_signal_handler(int sig, char *somedata, vregs regs) + * my_signal_handler(int sig, char *userData, vregs regs) * { * . . . * } @@ -234,10 +225,9 @@ sigdelset(sigset_t *set, int sig) * struct sigaction sa; * char data_buffer[32]; * - * sa.sa_handler = (sig_func_t) my_signal_handler; + * sa.sa_handler = (sighandler_t)my_signal_handler; * sigemptyset(&sa.sa_mask); - * sigaddset(&sa.sa_mask, SIGINT); - * sa.sa_userdata = data_buffer; + * sa.sa_userdata = userData; * * // install the handler * sigaction(SIGINT, &sa, NULL); @@ -245,29 +235,21 @@ sigdelset(sigset_t *set, int sig) * The two additional arguments available to the signal handler are extensions * to the Posix standard. This feature was introduced by the BeOS and retained * by Haiku. However, to remain compatible with Posix and ANSI C, the type - * of the sa_handler field is defined as 'sig_func_t'. This requires the handler + * of the sa_handler field is defined as 'sighandler_t'. This requires the handler * to be cast when assigned to the sa_handler field, as in the example above. * - * NOTE: C++ member functions can not be signal handlers! - * This is because they expect a "this" pointer as the first argument. - * - * * The 3 arguments that Haiku provides to signal handlers are as follows: + * 1) The first argument is the (usual) signal number. * - * - The first argument is the (usual) signal number. - * - * - The second argument is whatever value is put in the sa_userdata field + * 2) The second argument is whatever value is put in the sa_userdata field * of the sigaction struct. * - * - The third argument is a pointer to a vregs struct (defined below). + * 3) The third argument is a pointer to a vregs struct (defined below). * The vregs struct contains the contents of the volatile registers at * the time the signal was delivered to your thread. You can change the fields * of the structure. After your signal handler completes, the OS uses this struct * to reload the registers for your thread (privileged registers are not loaded * of course). The vregs struct is of course terribly machine dependent. - * If you use it, you should expect to have to re-work your code when new - * processors come out. Nonetheless, the ability to change the registers does - * open some interesting programming possibilities. */ /* diff --git a/src/system/kernel/arch/x86/arch_thread.c b/src/system/kernel/arch/x86/arch_thread.c index a32b46fcbc..09a9dcde7b 100644 --- a/src/system/kernel/arch/x86/arch_thread.c +++ b/src/system/kernel/arch/x86/arch_thread.c @@ -156,14 +156,25 @@ set_tls_context(struct thread *thread) } +static inline void +restart_syscall(struct iframe *frame) +{ + frame->eax = frame->orig_eax; + frame->edx = frame->orig_edx; + frame->eip -= 2; + // undos the "int $99" syscall interrupt + // (so that it'll be called again) +} + + static uint32 * get_signal_stack(struct thread *thread, struct iframe *frame, int signal) { // use the alternate signal stack if we should and can if (thread->signal_stack_enabled - && (thread->sig_action[signal].sa_flags & SA_ONSTACK) != 0 + && (thread->sig_action[signal - 1].sa_flags & SA_ONSTACK) != 0 && (frame->user_esp < thread->signal_stack_base - || frame->user_esp > thread->signal_stack_base + || frame->user_esp >= thread->signal_stack_base + thread->signal_stack_size)) { return (uint32 *)(thread->signal_stack_base + thread->signal_stack_size); @@ -193,7 +204,8 @@ arch_thread_init_thread_struct(struct thread *thread) status_t -arch_thread_init_kthread_stack(struct thread *t, int (*start_func)(void), void (*entry_func)(void), void (*exit_func)(void)) +arch_thread_init_kthread_stack(struct thread *t, int (*start_func)(void), + void (*entry_func)(void), void (*exit_func)(void)) { addr_t *kstack = (addr_t *)t->kernel_stack_base; addr_t *kstack_top = kstack + KERNEL_STACK_SIZE / sizeof(addr_t); @@ -222,7 +234,8 @@ arch_thread_init_kthread_stack(struct thread *t, int (*start_func)(void), void ( kstack_top--; *kstack_top = (unsigned int)start_func; - // set the return address to be the start of the entry (thread setup) function + // set the return address to be the start of the entry (thread setup) + // function kstack_top--; *kstack_top = (unsigned int)entry_func; @@ -256,7 +269,8 @@ arch_thread_init_tls(struct thread *thread) uint32 tls[TLS_THREAD_ID_SLOT + 1]; int32 i; - thread->user_local_storage = thread->user_stack_base + thread->user_stack_size; + thread->user_local_storage = thread->user_stack_base + + thread->user_stack_size; // initialize default TLS fields tls[TLS_BASE_ADDRESS_SLOT] = thread->user_local_storage; @@ -267,7 +281,8 @@ arch_thread_init_tls(struct thread *thread) void -arch_thread_switch_kstack_and_call(struct thread *t, addr_t new_kstack, void (*func)(void *), void *arg) +arch_thread_switch_kstack_and_call(struct thread *t, addr_t new_kstack, + void (*func)(void *), void *arg) { i386_switch_stack_and_call(new_kstack, func, arg); } @@ -328,7 +343,8 @@ arch_thread_dump_info(void *info) */ status_t -arch_thread_enter_userspace(struct thread *t, addr_t entry, void *args1, void *args2) +arch_thread_enter_userspace(struct thread *t, addr_t entry, void *args1, + void *args2) { addr_t stackTop = t->user_stack_base + t->user_stack_size; uint32 codeSize = (addr_t)x86_end_userspace_thread_exit @@ -375,7 +391,7 @@ bool arch_on_signal_stack(struct thread *thread) { struct iframe *frame = get_current_iframe(); - + return frame->user_esp >= thread->signal_stack_base && frame->user_esp < thread->signal_stack_base + thread->signal_stack_size; @@ -383,7 +399,7 @@ arch_on_signal_stack(struct thread *thread) status_t -arch_setup_signal_frame(struct thread *thread, struct sigaction *sa, +arch_setup_signal_frame(struct thread *thread, struct sigaction *action, int signal, int signalMask) { struct iframe *frame = get_current_iframe(); @@ -397,14 +413,10 @@ arch_setup_signal_frame(struct thread *thread, struct sigaction *sa, if (frame->orig_eax >= 0) { // we're coming from a syscall if ((status_t)frame->eax == EINTR - && (sa->sa_flags & SA_RESTART) != 0) { + && (action->sa_flags & SA_RESTART) != 0) { TRACE(("### restarting syscall %d after signal %d\n", frame->orig_eax, sig)); - frame->eax = frame->orig_eax; - frame->edx = frame->orig_edx; - frame->eip -= 2; - // undos the "int $99" syscall interrupt - // (so that it'll be called again) + restart_syscall(frame); } } @@ -412,8 +424,6 @@ arch_setup_signal_frame(struct thread *thread, struct sigaction *sa, userStack = get_signal_stack(thread, frame, signal); // store the saved regs onto the user stack - userStack -= ROUNDUP(sizeof(struct vregs) / 4, 4); - userRegs = userStack; regs.eip = frame->eip; regs.eflags = frame->flags; regs.eax = frame->eax; @@ -426,12 +436,14 @@ arch_setup_signal_frame(struct thread *thread, struct sigaction *sa, regs._reserved_2[2] = frame->ebp; i386_fnsave((void *)(®s.xregs)); + userStack -= ROUNDUP((sizeof(struct vregs) + 3) / 4, 4); + userRegs = userStack; status = user_memcpy(userRegs, ®s, sizeof(regs)); if (status < B_OK) return status; // now store a code snippet on the stack - userStack -= ((uint32)i386_end_return_from_signal + userStack -= ((uint32)i386_end_return_from_signal + 3 - (uint32)i386_return_from_signal) / 4; signalCode = userStack; status = user_memcpy(signalCode, i386_return_from_signal, @@ -442,9 +454,9 @@ arch_setup_signal_frame(struct thread *thread, struct sigaction *sa, // now set up the final part buffer[0] = (uint32)signalCode; // return address when sa_handler done - buffer[1] = signal; // first argument to sa_handler - buffer[2] = (uint32)sa->sa_userdata; // second argument to sa_handler - buffer[3] = (uint32)userRegs; // third argument to sa_handler + buffer[1] = signal; // arguments to sa_handler + buffer[2] = (uint32)action->sa_userdata; + buffer[3] = (uint32)userRegs; buffer[4] = signalMask; // Old signal mask to restore buffer[5] = (uint32)userRegs; // Int frame + extra regs to restore @@ -456,7 +468,7 @@ arch_setup_signal_frame(struct thread *thread, struct sigaction *sa, return status; frame->user_esp = (uint32)userStack; - frame->eip = (uint32)sa->sa_handler; + frame->eip = (uint32)action->sa_handler; return B_OK; } @@ -511,12 +523,8 @@ arch_check_syscall_restart(struct thread *thread) return; } - if ((status_t)frame->orig_eax >= 0 && (status_t)frame->eax == EINTR) { - frame->eax = frame->orig_eax; - frame->edx = frame->orig_edx; - frame->eip -= 2; - // undoes the "int $99" syscall interrupt (so that it'll be called again) - } + if ((status_t)frame->orig_eax >= 0 && (status_t)frame->eax == EINTR) + restart_syscall(frame); } diff --git a/src/system/libroot/posix/signal/sigaction.c b/src/system/libroot/posix/signal/sigaction.c index ff6b4fb18a..716ff8139f 100644 --- a/src/system/libroot/posix/signal/sigaction.c +++ b/src/system/libroot/posix/signal/sigaction.c @@ -1,19 +1,27 @@ /* - * Copyright (c) 2002-2004, Haiku Project. All rights reserved. - * Distributed under the terms of the Haiku license. + * Copyright 2002-2007, Haiku Inc. All rights reserved. + * Distributed under the terms of the MIT license. * - * Author(s): - * Daniel Reinhold (danielre@users.sf.net) + * Author: + * Daniel Reinhold (danielre@users.sf.net) */ -#include +#include #include +#include + int sigaction(int sig, const struct sigaction *action, struct sigaction *oldAction) { - return _kern_sigaction(sig, action, oldAction); + status_t status = _kern_sigaction(sig, action, oldAction); + if (status < B_OK) { + errno = status; + return -1; + } + + return 0; } diff --git a/src/system/libroot/posix/signal/signal.c b/src/system/libroot/posix/signal/signal.c index 07dc4cc8ea..3e58bffa53 100644 --- a/src/system/libroot/posix/signal/signal.c +++ b/src/system/libroot/posix/signal/signal.c @@ -1,35 +1,28 @@ /* - * Copyright (c) 2002-2004, Haiku Project. All rights reserved. - * Distributed under the terms of the Haiku license. + * Copyright 2002-2007, Haiku Inc. All rights reserved. + * Distributed under the terms of the MIT license. * - * Author(s): - * Daniel Reinhold (danielre@users.sf.net) + * Author: + * Daniel Reinhold (danielre@users.sf.net) */ -#include #include -#include +#include -sig_func_t -signal(int sig, sig_func_t signalHandler) +sighandler_t +signal(int sig, sighandler_t signalHandler) { struct sigaction newAction, oldAction; - int err; // setup the replacement sigaction newAction.sa_handler = signalHandler; newAction.sa_mask = 0; newAction.sa_flags = 0; - // install the replacement sigaction for the signal 'sig' into - // the current thread (the original is copied into the last param) - err = sigaction(sig, &newAction, &oldAction); - if (err < B_OK) { - errno = err; + if (sigaction(sig, &newAction, &oldAction) != 0) return SIG_ERR; - } // success, return the original handler return oldAction.sa_handler; diff --git a/src/system/libroot/posix/signal/sigset.c b/src/system/libroot/posix/signal/sigset.c index 9382209bfe..eab8c15b02 100644 --- a/src/system/libroot/posix/signal/sigset.c +++ b/src/system/libroot/posix/signal/sigset.c @@ -1,26 +1,15 @@ -#include +/* + * Copyright 2002-2007, Haiku Inc. All rights reserved. + * Distributed under the terms of the MIT license. + * + * Author: + * Daniel Reinhold (danielre@users.sf.net) + */ + + #include -/* - * Copyright (c) 2002, OpenBeOS Project. All rights reserved. - * Distributed under the terms of the OpenBeOS license. - * - * - * sigset.c: - * implements two sigset functions: - * sigemptyset() and sigfillset() - * - * the other three sigset functions - * sigaddset(), sigdelset(), and sigismember() - * are inlined in the header file - * (I have no idea why there is a distinction between - * some being inlined and others not) - * - * - * Author(s): - * Daniel Reinhold (danielre@users.sf.net) - * - */ +#include int diff --git a/src/system/libroot/posix/signal/strsignal.c b/src/system/libroot/posix/signal/strsignal.c index 6bd3571674..f7e7d2a827 100644 --- a/src/system/libroot/posix/signal/strsignal.c +++ b/src/system/libroot/posix/signal/strsignal.c @@ -1,22 +1,17 @@ -#include -#include -#include - /* - * Copyright (c) 2002, OpenBeOS Project. All rights reserved. - * Distributed under the terms of the OpenBeOS license. - * - * - * strsignal.c: - * implements the signal function strsignal() - * - * - * Author(s): - * Daniel Reinhold (danielre@users.sf.net) + * Copyright 2002-2007, Haiku Inc. All rights reserved. + * Distributed under the terms of the MIT license. * + * Author: + * Daniel Reinhold (danielre@users.sf.net) */ +#include +#include +#include + + const char * const sys_siglist[NSIG] = { /* 0 */ "Signal 0", @@ -56,13 +51,13 @@ const char * strsignal(int sig) { const char *s = NULL; - + if (sig < 0 || sig > MAX_SIGNO) return "Bogus signal number"; - + if (sig < NSIG) s = sys_siglist[sig]; - + if (s) return s; else { diff --git a/src/tests/system/libroot/posix/Jamfile b/src/tests/system/libroot/posix/Jamfile index 4fc453e9bd..92d9aaec61 100644 --- a/src/tests/system/libroot/posix/Jamfile +++ b/src/tests/system/libroot/posix/Jamfile @@ -10,6 +10,10 @@ SimpleTest flock_test : flock_test.cpp ; +SimpleTest signal_test + : signal_test.cpp +; + SimpleTest setjmp_test : setjmp_test.c ; diff --git a/src/tests/system/libroot/posix/signal_test.cpp b/src/tests/system/libroot/posix/signal_test.cpp new file mode 100644 index 0000000000..809bb94020 --- /dev/null +++ b/src/tests/system/libroot/posix/signal_test.cpp @@ -0,0 +1,111 @@ +/* + * Copyright 2007, Axel Dörfler, axeld@pinc-software.de. + * Distributed under the terms of the MIT License. + */ + + +#include +#include +#include +#include +#include + + +#if defined(__BEOS__) && !defined(__HAIKU__) +typedef void (*sighandler_t)(int); +# define SIGSTKSZ 16384 +# define SA_ONESHOT 0 +# define SA_ONSTACK 0 +# define SA_RESTART 0 +# define ualarm(usec, interval) alarm(1) +#endif + +const void* kUserDataMagic = (void *)0x12345678; + +static char sAlternateStack[SIGSTKSZ]; + + +bool +is_alternate(void* pointer) +{ + return (char*)pointer > &sAlternateStack[0] + && (char*)pointer <= &sAlternateStack[0] + SIGSTKSZ; +} + + +void +sigHandler(int signal, void *userData, vregs *regs) +{ +#ifdef __BEOS__ + if (userData != kUserDataMagic) + fprintf(stderr, "FAILED: user data not correct: %p\n", userData); +#endif + + printf("signal handler called with signal %i on %s stack\n", + signal, is_alternate(&signal) ? "alternate" : "standard"); +} + + +void +wait_for_key() +{ + char buffer[100]; + if (fgets(buffer, sizeof(buffer), stdin) == NULL + || buffer[0] == 'q') { + if (errno == EINTR) + puts("interrupted"); + else + exit(0); + } +} + +int +main() +{ + puts("-- 1 (should block) --"); + + struct sigaction newAction; + newAction.sa_handler = (sighandler_t)sigHandler; + newAction.sa_mask = 0; + newAction.sa_flags = SA_ONESHOT | SA_ONSTACK | SA_RESTART; +#ifdef __BEOS__ + newAction.sa_userdata = (void*)kUserDataMagic; +#endif + sigaction(SIGALRM, &newAction, NULL); + + ualarm(10000, 0); + wait_for_key(); + + puts("-- 2 (does not block, should call handler twice) --"); + + newAction.sa_flags = 0; + sigaction(SIGALRM, &newAction, NULL); + + ualarm(0, 50000); + wait_for_key(); + wait_for_key(); + + ualarm(0, 0); + + puts("-- 3 (alternate stack, should block) --"); + +#if defined(__BEOS__) && !defined(__HAIKU__) + set_signal_stack(sAlternateStack, SIGSTKSZ); +#else + stack_t newStack; + newStack.ss_sp = sAlternateStack; + newStack.ss_size = SIGSTKSZ; + newStack.ss_flags = 0; + if (sigaltstack(&newStack, NULL) != 0) + fprintf(stderr, "sigaltstack() failed: %s\n", strerror(errno)); +#endif + + newAction.sa_flags = SA_RESTART | SA_ONSTACK; + sigaction(SIGALRM, &newAction, NULL); + + ualarm(10000, 0); + wait_for_key(); + + puts("-- end --"); + return 0; +}