From: Gabor Melis Date: Mon, 16 Feb 2009 22:19:27 +0000 (+0000) Subject: 1.0.25.46: restore errno in signal handlers X-Git-Url: http://repo.macrolet.net/gitweb/?a=commitdiff_plain;h=3dd90b64c37103d9c86d32b6c36277a6cea4098a;p=sbcl.git 1.0.25.46: restore errno in signal handlers --- diff --git a/NEWS b/NEWS index c3511f3..3d91a48 100644 --- a/NEWS +++ b/NEWS @@ -36,6 +36,7 @@ changes in sbcl-1.0.26 relative to 1.0.25: * bug fix: fix random memory faults related to interrupts on alpha * bug fix: fix deadlocks related to starting threads * bug fix: fix deadlines on locks on futex platforms + * bug fix: restore errno in signal handlers changes in sbcl-1.0.25 relative to 1.0.24: * incompatible change: SB-INTROSPECT:FUNCTION-ARGLIST is deprecated, to be diff --git a/doc/internals/signals.texinfo b/doc/internals/signals.texinfo index e5cfceb..acd47b7 100644 --- a/doc/internals/signals.texinfo +++ b/doc/internals/signals.texinfo @@ -75,8 +75,8 @@ is deferred by pseudo atomic and @code{WITHOUT-GCING}. @subsection Miscellaneous issues -Signal handlers should automatically restore errno and fp -state. Currently, this is not the case. +Signal handlers automatically restore errno and fp state, but +arrange_return_to_lisp_function does not restore errno. @subsection POSIX -- Letter and Spirit diff --git a/src/runtime/interrupt.c b/src/runtime/interrupt.c index 3c6f716..f651742 100644 --- a/src/runtime/interrupt.c +++ b/src/runtime/interrupt.c @@ -68,6 +68,26 @@ #include "genesis/simple-fun.h" #include "genesis/cons.h" +/* These are to be used in signal handlers. Currently all handlers are + * called from one of: + * + * interrupt_handle_now_handler + * maybe_now_maybe_later + * unblock_me_trampoline + * low_level_handle_now_handler + * low_level_maybe_now_maybe_later + * low_level_unblock_me_trampoline + */ +#define SAVE_ERRNO() \ + { \ + int _saved_errno = errno; \ + { + +#define RESTORE_ERRNO \ + } \ + errno = _saved_errno; \ + } + static void run_deferred_handler(struct interrupt_data *data, void *v_context); #ifndef LISP_FEATURE_WIN32 static void store_signal_data_for_later (struct interrupt_data *data, @@ -1024,6 +1044,7 @@ store_signal_data_for_later (struct interrupt_data *data, void *handler, static void maybe_now_maybe_later(int signal, siginfo_t *info, void *void_context) { + SAVE_ERRNO(); os_context_t *context = arch_os_get_context(&void_context); struct thread *thread = arch_os_get_current_thread(); struct interrupt_data *data = thread->interrupt_data; @@ -1034,6 +1055,7 @@ maybe_now_maybe_later(int signal, siginfo_t *info, void *void_context) if(!maybe_defer_handler(interrupt_handle_now,data,signal,info,context)) interrupt_handle_now(signal, info, context); + RESTORE_ERRNO; } static void @@ -1050,6 +1072,7 @@ low_level_interrupt_handle_now(int signal, siginfo_t *info, static void low_level_maybe_now_maybe_later(int signal, siginfo_t *info, void *void_context) { + SAVE_ERRNO(); os_context_t *context = arch_os_get_context(&void_context); struct thread *thread = arch_os_get_current_thread(); struct interrupt_data *data = thread->interrupt_data; @@ -1061,6 +1084,7 @@ low_level_maybe_now_maybe_later(int signal, siginfo_t *info, void *void_context) if(!maybe_defer_handler(low_level_interrupt_handle_now,data, signal,info,context)) low_level_interrupt_handle_now(signal, info, context); + RESTORE_ERRNO; } #endif @@ -1129,6 +1153,7 @@ sig_stop_for_gc_handler(int signal, siginfo_t *info, void *void_context) void interrupt_handle_now_handler(int signal, siginfo_t *info, void *void_context) { + SAVE_ERRNO(); os_context_t *context = arch_os_get_context(&void_context); #if defined(LISP_FEATURE_LINUX) || defined(RESTORE_FP_CONTROL_FROM_CONTEXT) os_restore_fp_control(context); @@ -1142,6 +1167,7 @@ interrupt_handle_now_handler(int signal, siginfo_t *info, void *void_context) #endif #endif interrupt_handle_now(signal, info, context); + RESTORE_ERRNO; } /* manipulate the signal context and stack such that when the handler @@ -1170,7 +1196,13 @@ arrange_return_to_lisp_function(os_context_t *context, lispobj function) * user's backtrace makes (as much) sense (as usual) */ /* FIXME: what about restoring fp state? */ - /* FIXME: what about restoring errno? */ + /* FIXME: errno is not restored, but since current uses of this + * function only call Lisp code that signals an error, it's not + * much of a problem. In other words, running out of the control + * stack between a syscall and (GET-ERRNO) may clobber errno if + * something fails during signalling or in the handler. But I + * can't see what can go wrong as long as there is no CONTINUE + * like restart on them. */ #ifdef LISP_FEATURE_X86 /* Suppose the existence of some function that saved all * registers, called call_into_lisp, then restored GP registers and @@ -1463,23 +1495,36 @@ see_if_sigaction_nodefer_works(void) static void unblock_me_trampoline(int signal, siginfo_t *info, void *void_context) { + SAVE_ERRNO(); + os_context_t *context = arch_os_get_context(&void_context); sigset_t unblock; sigemptyset(&unblock); sigaddset(&unblock, signal); thread_sigmask(SIG_UNBLOCK, &unblock, 0); - interrupt_handle_now_handler(signal, info, void_context); + interrupt_handle_now(signal, info, context); + RESTORE_ERRNO; } static void low_level_unblock_me_trampoline(int signal, siginfo_t *info, void *void_context) { + SAVE_ERRNO(); sigset_t unblock; sigemptyset(&unblock); sigaddset(&unblock, signal); thread_sigmask(SIG_UNBLOCK, &unblock, 0); (*interrupt_low_level_handlers[signal])(signal, info, void_context); + RESTORE_ERRNO; +} + +static void +low_level_handle_now_handler(int signal, siginfo_t *info, void *void_context) +{ + SAVE_ERRNO(); + (*interrupt_low_level_handlers[signal])(signal, info, void_context); + RESTORE_ERRNO; } void @@ -1506,7 +1551,7 @@ undoably_install_low_level_interrupt_handler (int signal, sa.sa_sigaction = low_level_unblock_me_trampoline; #endif else - sa.sa_sigaction = handler; + sa.sa_sigaction = low_level_handle_now_handler; sigcopyset(&sa.sa_mask, &blockable_sigset); sa.sa_flags = SA_SIGINFO | SA_RESTART diff --git a/tests/signals.impure.lisp b/tests/signals.impure.lisp index 16b9767..675fd8a 100644 --- a/tests/signals.impure.lisp +++ b/tests/signals.impure.lisp @@ -35,3 +35,24 @@ (princ '*) (force-output)) (terpri))) + +(require :sb-posix) + +(with-test (:name (:signal :errno)) + (let* (saved-errno + (returning nil) + (timer (make-timer (lambda () + (sb-unix:unix-open "~!@#$%^&*[]()/\\" 0 0) + (assert (= sb-unix:enoent + (sb-unix::get-errno))) + (setq returning t))))) + (schedule-timer timer 0.2) + ;; Fail and set errno. + (sb-unix:nanosleep -1 -1) + (setq saved-errno (sb-unix::get-errno)) + (assert (= saved-errno sb-posix:einval)) + ;; Wait, but not with sleep because that will be interrupted and + ;; we get EINTR. + (loop until returning) + (loop repeat 1000000000) + (assert (= saved-errno (sb-unix::get-errno))))) diff --git a/version.lisp-expr b/version.lisp-expr index 4bbaf0c..c1e27f0 100644 --- a/version.lisp-expr +++ b/version.lisp-expr @@ -17,4 +17,4 @@ ;;; checkins which aren't released. (And occasionally for internal ;;; versions, especially for internal versions off the main CVS ;;; branch, it gets hairier, e.g. "0.pre7.14.flaky4.13".) -"1.0.25.45" +"1.0.25.46"