From 37d3828773e2f847bb1ed7522b0af4fb8e736fc8 Mon Sep 17 00:00:00 2001 From: David Lichteblau Date: Wed, 5 Dec 2012 19:08:23 +0100 Subject: [PATCH] Support building without PSEUDO-ATOMIC on POSIX safepoints - Mark Lisp signal handlers with a flag `synchronous' to indicate whether we can (and must) handle them immediately. Conversely, we understand this flag to imply a guarantee that the signal does not occur during allocation. - Any signal with a Lisp handler that is not synchronous is implemented in the runtime using a trampoline, which (instead of invoking Lisp code directly) first spawns a new pthread, which only then calls back into Lisp to invoke the handler function (with a fake signal context). - Used in particular for SIGINT. - For SIGPROF, introduce a second per-thread allocation region, which gets swapped with the usual region around the call into SIGPROF-HANDLER. This handler is a special case, because it is careful not to trigger GC nor non-local unwinds, and we can safely return to the original region afterwards. - Add a new subclass SIGNAL-HANDLER-THREAD for this purpose, making it easy to identify these threads (e.g. in the test driver). - Run sprof tests while building the contrib. Add a test stressing time profiling of allocation sequences. Enable using :SB-SAFEPOINT-STRICTLY on features. Quite usable already on x86 and x86-64; PPC still has more prominent issues, e.g. in threads.impure.lisp. --- contrib/sb-sprof/Makefile | 2 +- contrib/sb-sprof/sb-sprof.lisp | 21 ++++++++- contrib/sb-sprof/test.lisp | 13 ++++++ make-config.sh | 1 + package-data-list.lisp-expr | 2 + src/code/target-signal.lisp | 49 +++++++++++++++++--- src/code/thread.lisp | 8 ++++ src/compiler/generic/objdef.lisp | 2 + src/compiler/generic/parms.lisp | 4 +- src/compiler/ppc/macros.lisp | 3 ++ src/compiler/x86-64/macros.lisp | 4 +- src/compiler/x86/macros.lisp | 4 +- src/runtime/gencgc.c | 12 ++--- src/runtime/interrupt.c | 92 +++++++++++++++++++++++++++++++++++++- src/runtime/interrupt.h | 3 +- src/runtime/runtime.c | 2 +- src/runtime/safepoint.c | 20 +++++++++ src/runtime/thread.c | 9 ++++ src/runtime/thread.h | 5 +++ tests/signals.impure.lisp | 12 +++-- tests/test-util.lisp | 4 ++ 21 files changed, 247 insertions(+), 25 deletions(-) create mode 100644 contrib/sb-sprof/test.lisp diff --git a/contrib/sb-sprof/Makefile b/contrib/sb-sprof/Makefile index 463ae52..7373c72 100644 --- a/contrib/sb-sprof/Makefile +++ b/contrib/sb-sprof/Makefile @@ -2,4 +2,4 @@ MODULE=sb-sprof include ../vanilla-module.mk test:: - true + $(SBCL) --eval '(load (format nil "SYS:CONTRIB;~:@(~A~);TEST.LISP" "$(MODULE)"))' > $ltf printf ' :sb-thread :sb-safepoint :sb-thruption :sb-wtimer' >> $ltf + printf ' :sb-safepoint-strictly' >> $ltf # link_or_copy Config.$sbcl_arch-win32 Config link_or_copy $sbcl_arch-win32-os.h target-arch-os.h diff --git a/package-data-list.lisp-expr b/package-data-list.lisp-expr index 03ab6ee..691a040 100644 --- a/package-data-list.lisp-expr +++ b/package-data-list.lisp-expr @@ -2023,6 +2023,8 @@ is a good idea, but see SB-SYS re. blurring of boundaries." "THREAD-NAME" "THREAD-YIELD" "FOREIGN-THREAD" + #!+(and sb-safepoint-strictly (not win32)) + "SIGNAL-HANDLING-THREAD" ;; Memory barrier "BARRIER" ;; Mutexes diff --git a/src/code/target-signal.lisp b/src/code/target-signal.lisp index 9fab9d1..3fd22f3 100644 --- a/src/code/target-signal.lisp +++ b/src/code/target-signal.lisp @@ -117,11 +117,25 @@ (sb!alien:define-alien-routine ("install_handler" install-handler) sb!alien:unsigned-long (signal sb!alien:int) - (handler sb!alien:unsigned-long)) + (handler sb!alien:unsigned-long) + (synchronous boolean)) ;;;; interface to enabling and disabling signal handlers -(defun enable-interrupt (signal handler) +;;; Note on the SYNCHRONOUS argument: On builds without pseudo-atomic, +;;; we have no way of knowing whether interrupted code was in an +;;; allocation sequence, and cannot delay signals until after +;;; allocation. Any signal that can occur asynchronously must be +;;; considered unsafe for immediate execution, and the invocation of its +;;; lisp handler will get delayed into a newly spawned signal handler +;;; thread. However, there are signals which we must handle +;;; immediately, because they occur synchonously (hence the boolean flag +;;; SYNCHRONOUS to this function), luckily implying that the signal +;;; happens only in specific places (illegal instructions, floating +;;; point instructions, certain system calls), hopefully ruling out the +;;; possibility that we would trigger it during allocation. + +(defun enable-interrupt (signal handler &key synchronous) (declare (type (or function fixnum (member :default :ignore)) handler)) (/show0 "enable-interrupt") (flet ((run-handler (&rest args) @@ -135,7 +149,8 @@ (:ignore sig-ign) (t (sb!kernel:get-lisp-obj-address - #'run-handler)))))) + #'run-handler))) + synchronous))) (cond ((= result sig-dfl) :default) ((= result sig-ign) :ignore) (t (the (or function fixnum) @@ -147,6 +162,26 @@ (defun ignore-interrupt (signal) (enable-interrupt signal :ignore)) +;;;; Support for signal handlers which aren't. +;;;; +;;;; On safepoint builds, user-defined Lisp signal handlers do not run +;;;; in the handler for their signal, because we have no pseudo atomic +;;;; mechanism to prevent handlers from hitting during allocation. +;;;; Rather, the signal spawns off a fresh native thread, which calls +;;;; into lisp with a fake context through this callback: + +#!+(and sb-safepoint-strictly (not win32)) +(defun signal-handler-callback (run-handler signal args) + (sb!thread::initial-thread-function-trampoline + (sb!thread::make-signal-handling-thread :name "signal handler" + :signal-number signal) + nil (lambda () + (let* ((info (sb!sys:sap-ref-sap args 0)) + (context (sb!sys:sap-ref-sap args sb!vm:n-word-bytes))) + (funcall run-handler signal info context))) + nil nil nil nil)) + + ;;;; default LISP signal handlers ;;;; ;;;; Most of these just call ERROR to report the presence of the signal. @@ -237,13 +272,13 @@ "Enable all the default signals that Lisp knows how to deal with." (enable-interrupt sigint #'sigint-handler) (enable-interrupt sigterm #'sigterm-handler) - (enable-interrupt sigill #'sigill-handler) + (enable-interrupt sigill #'sigill-handler :synchronous t) #!-linux (enable-interrupt sigemt #'sigemt-handler) - (enable-interrupt sigfpe #'sb!vm:sigfpe-handler) - (enable-interrupt sigbus #'sigbus-handler) + (enable-interrupt sigfpe #'sb!vm:sigfpe-handler :synchronous t) + (enable-interrupt sigbus #'sigbus-handler :synchronous t) #!-linux - (enable-interrupt sigsys #'sigsys-handler) + (enable-interrupt sigsys #'sigsys-handler :synchronous t) #!-sb-wtimer (enable-interrupt sigalrm #'sigalrm-handler) #!-sb-thruption diff --git a/src/code/thread.lisp b/src/code/thread.lisp index 2cfd567..3d1ecd4 100644 --- a/src/code/thread.lisp +++ b/src/code/thread.lisp @@ -42,6 +42,14 @@ in future versions." "Type of native threads which are attached to the runtime as Lisp threads temporarily.") +#!+(and sb-safepoint-strictly (not win32)) +(def!struct (signal-handling-thread + (:include foreign-thread) + (:conc-name "THREAD-")) + #!+sb-doc + "Asynchronous signal handling thread." + (signal-number nil :type integer)) + (def!struct mutex #!+sb-doc "Mutex type." diff --git a/src/compiler/generic/objdef.lisp b/src/compiler/generic/objdef.lisp index fda2758..262f472 100644 --- a/src/compiler/generic/objdef.lisp +++ b/src/compiler/generic/objdef.lisp @@ -434,6 +434,8 @@ #!+sb-safepoint (csp-around-foreign-call :c-type "lispobj *") #!+sb-safepoint (pc-around-foreign-call :c-type "lispobj *") #!+win32 (synchronous-io-handle-and-flag :c-type "HANDLE" :length 1) + #!+(and sb-safepoint-strictly (not win32)) + (sprof-alloc-region :c-type "struct alloc_region" :length 5) ;; KLUDGE: On alpha, until STEPPING we have been lucky and the 32 ;; bit slots came in pairs. However the C compiler will align ;; interrupt_contexts on a double word boundary. This logic should diff --git a/src/compiler/generic/parms.lisp b/src/compiler/generic/parms.lisp index ca556c2..4661321 100644 --- a/src/compiler/generic/parms.lisp +++ b/src/compiler/generic/parms.lisp @@ -124,7 +124,9 @@ fdefinition-object #!+win32 sb!kernel::handle-win32-exception #!+sb-thruption sb!thread::run-interruption - #!+sb-safepoint sb!thread::enter-foreign-callback)) + #!+sb-safepoint sb!thread::enter-foreign-callback + #!+(and sb-safepoint-strictly (not win32)) + sb!unix::signal-handler-callback)) (defparameter *common-static-symbols* '(t diff --git a/src/compiler/ppc/macros.lisp b/src/compiler/ppc/macros.lisp index 2fdfb7e..262cc10 100644 --- a/src/compiler/ppc/macros.lisp +++ b/src/compiler/ppc/macros.lisp @@ -336,6 +336,9 @@ ;;; trap if ALLOC-TN's negative (handling the deferred interrupt) and ;;; using FLAG-TN - minus the large constant - to correct ALLOC-TN. (defmacro pseudo-atomic ((flag-tn) &body forms) + #!+sb-safepoint-strictly + `(progn ,flag-tn ,@forms (emit-safepoint)) + #!-sb-safepoint-strictly `(progn (without-scheduling () ;; Extra debugging stuff: diff --git a/src/compiler/x86-64/macros.lisp b/src/compiler/x86-64/macros.lisp index 11a06fc..a397573 100644 --- a/src/compiler/x86-64/macros.lisp +++ b/src/compiler/x86-64/macros.lisp @@ -303,9 +303,9 @@ #!+sb-thread (defmacro pseudo-atomic (&rest forms) - #!+win32 + #!+sb-safepoint-strictly `(progn ,@forms (emit-safepoint)) - #!-win32 + #!-sb-safepoint-strictly (with-unique-names (label) `(let ((,label (gen-label))) (inst mov (make-ea :qword diff --git a/src/compiler/x86/macros.lisp b/src/compiler/x86/macros.lisp index d7b6bc2..4050640 100644 --- a/src/compiler/x86/macros.lisp +++ b/src/compiler/x86/macros.lisp @@ -408,9 +408,9 @@ #!+sb-thread (defmacro pseudo-atomic (&rest forms) - #!+win32 + #!+sb-safepoint-strictly `(progn ,@forms (emit-safepoint)) - #!-win32 + #!-sb-safepoint-strictly (with-unique-names (label) `(let ((,label (gen-label))) (inst mov (make-ea :dword :disp (* 4 thread-pseudo-atomic-bits-slot)) diff --git a/src/runtime/gencgc.c b/src/runtime/gencgc.c index 5829b5a..f4ffd2f 100644 --- a/src/runtime/gencgc.c +++ b/src/runtime/gencgc.c @@ -4306,9 +4306,7 @@ general_alloc(sword_t nbytes, int page_type_flag) lispobj AMD64_SYSV_ABI * alloc(long nbytes) { -#ifdef LISP_FEATURE_WIN32 - /* WIN32 is currently the only platform where inline allocation is - * not pseudo atomic. */ +#ifdef LISP_FEATURE_SB_SAFEPOINT_STRICTLY struct thread *self = arch_os_get_current_thread(); int was_pseudo_atomic = get_pseudo_atomic_atomic(self); if (!was_pseudo_atomic) @@ -4319,7 +4317,7 @@ alloc(long nbytes) lispobj *result = general_alloc(nbytes, BOXED_PAGE_FLAG); -#ifdef LISP_FEATURE_WIN32 +#ifdef LISP_FEATURE_SB_SAFEPOINT_STRICTLY if (!was_pseudo_atomic) clear_pseudo_atomic_atomic(self); #endif @@ -4434,8 +4432,12 @@ void gc_alloc_update_all_page_tables(void) { /* Flush the alloc regions updating the tables. */ struct thread *th; - for_each_thread(th) + for_each_thread(th) { gc_alloc_update_page_tables(BOXED_PAGE_FLAG, &th->alloc_region); +#if defined(LISP_FEATURE_SB_SAFEPOINT_STRICTLY) && !defined(LISP_FEATURE_WIN32) + gc_alloc_update_page_tables(BOXED_PAGE_FLAG, &th->sprof_alloc_region); +#endif + } gc_alloc_update_page_tables(UNBOXED_PAGE_FLAG, &unboxed_region); gc_alloc_update_page_tables(BOXED_PAGE_FLAG, &boxed_region); } diff --git a/src/runtime/interrupt.c b/src/runtime/interrupt.c index d23dbd7..882e2bb 100644 --- a/src/runtime/interrupt.c +++ b/src/runtime/interrupt.c @@ -1779,6 +1779,89 @@ see_if_sigaction_nodefer_works(void) #undef SA_NODEFER_TEST_BLOCK_SIGNAL #undef SA_NODEFER_TEST_KILL_SIGNAL +#if defined(LISP_FEATURE_SB_SAFEPOINT_STRICTLY) && !defined(LISP_FEATURE_WIN32) + +static void * +signal_thread_trampoline(void *pthread_arg) +{ + int signo = (int) pthread_arg; + os_context_t fake_context; + siginfo_t fake_info; +#ifdef LISP_FEATURE_PPC + mcontext_t uc_regs; +#endif + + memset(&fake_info, 0, sizeof(fake_info)); + memset(&fake_context, 0, sizeof(fake_context)); +#ifdef LISP_FEATURE_PPC + memset(&uc_regs, 0, sizeof(uc_regs)); + fake_context.uc_mcontext.uc_regs = &uc_regs; +#endif + + *os_context_pc_addr(&fake_context) = &signal_thread_trampoline; +#ifdef ARCH_HAS_STACK_POINTER /* aka x86(-64) */ + *os_context_sp_addr(&fake_context) = __builtin_frame_address(0); +#endif + + signal_handler_callback(interrupt_handlers[signo].lisp, + signo, &fake_info, &fake_context); + return 0; +} + +static void +sigprof_handler_trampoline(int signal, siginfo_t *info, void *void_context) +{ + SAVE_ERRNO(signal,context,void_context); + struct thread *self = arch_os_get_current_thread(); + + /* alloc() is not re-entrant and still uses pseudo atomic (even though + * inline allocation does not). In this case, give up. */ + if (get_pseudo_atomic_atomic(self)) + goto cleanup; + + struct alloc_region tmp = self->alloc_region; + self->alloc_region = self->sprof_alloc_region; + self->sprof_alloc_region = tmp; + + interrupt_handle_now_handler(signal, info, void_context); + + /* And we're back. We know that the SIGPROF handler never unwinds + * non-locally, and can simply swap things back: */ + + tmp = self->alloc_region; + self->alloc_region = self->sprof_alloc_region; + self->sprof_alloc_region = tmp; + +cleanup: + ; /* Dear C compiler, it's OK to have a label here. */ + RESTORE_ERRNO; +} + +static void +spawn_signal_thread_handler(int signal, siginfo_t *info, void *void_context) +{ + SAVE_ERRNO(signal,context,void_context); + + pthread_attr_t attr; + pthread_t th; + + if (pthread_attr_init(&attr)) + goto lost; + if (pthread_attr_setstacksize(&attr, thread_control_stack_size)) + goto lost; + if (pthread_create(&th, &attr, &signal_thread_trampoline, (void*) signal)) + goto lost; + if (pthread_attr_destroy(&attr)) + goto lost; + + RESTORE_ERRNO; + return; + +lost: + lose("spawn_signal_thread_handler"); +} +#endif + static void unblock_me_trampoline(int signal, siginfo_t *info, void *void_context) { @@ -1863,7 +1946,8 @@ undoably_install_low_level_interrupt_handler (int signal, /* This is called from Lisp. */ uword_t -install_handler(int signal, void handler(int, siginfo_t*, os_context_t*)) +install_handler(int signal, void handler(int, siginfo_t*, os_context_t*), + int synchronous) { #ifndef LISP_FEATURE_WIN32 struct sigaction sa; @@ -1880,6 +1964,12 @@ install_handler(int signal, void handler(int, siginfo_t*, os_context_t*)) if (ARE_SAME_HANDLER(handler, SIG_DFL) || ARE_SAME_HANDLER(handler, SIG_IGN)) sa.sa_sigaction = (void (*)(int, siginfo_t*, void*))handler; +#ifdef LISP_FEATURE_SB_SAFEPOINT_STRICTLY + else if (signal == SIGPROF) + sa.sa_sigaction = sigprof_handler_trampoline; + else if (!synchronous) + sa.sa_sigaction = spawn_signal_thread_handler; +#endif else if (sigismember(&deferrable_sigset, signal)) sa.sa_sigaction = maybe_now_maybe_later; else if (!sigaction_nodefer_works && diff --git a/src/runtime/interrupt.h b/src/runtime/interrupt.h index a27eb08..07b4a2d 100644 --- a/src/runtime/interrupt.h +++ b/src/runtime/interrupt.h @@ -158,7 +158,8 @@ extern void undoably_install_low_level_interrupt_handler ( int signal, interrupt_handler_t handler); extern uword_t install_handler(int signal, - interrupt_handler_t handler); + interrupt_handler_t handler, + int synchronous); extern union interrupt_handler interrupt_handlers[NSIG]; diff --git a/src/runtime/runtime.c b/src/runtime/runtime.c index 3bc18ee..49c007b 100644 --- a/src/runtime/runtime.c +++ b/src/runtime/runtime.c @@ -95,7 +95,7 @@ void sigint_init(void) { SHOW("entering sigint_init()"); - install_handler(SIGINT, sigint_handler); + install_handler(SIGINT, sigint_handler, 1); SHOW("leaving sigint_init()"); } diff --git a/src/runtime/safepoint.c b/src/runtime/safepoint.c index a9e578c..45af50e 100644 --- a/src/runtime/safepoint.c +++ b/src/runtime/safepoint.c @@ -966,6 +966,26 @@ handle_safepoint_violation(os_context_t *ctx, os_vm_address_t fault_address) } #endif /* LISP_FEATURE_WIN32 */ +#if defined(LISP_FEATURE_SB_SAFEPOINT_STRICTLY) && !defined(LISP_FEATURE_WIN32) +void +signal_handler_callback(lispobj run_handler, int signo, void *info, void *ctx) +{ + init_thread_data scribble; + void *args[2]; + args[0] = info; + args[1] = ctx; + + attach_os_thread(&scribble); + + odxprint(misc, "callback from signal handler thread for: %d\n", signo); + funcall3(StaticSymbolFunction(SIGNAL_HANDLER_CALLBACK), + run_handler, make_fixnum(signo), alloc_sap(args)); + + detach_os_thread(&scribble); + return; +} +#endif + void callback_wrapper_trampoline( #if !(defined(LISP_FEATURE_X86) || defined(LISP_FEATURE_X86_64)) diff --git a/src/runtime/thread.c b/src/runtime/thread.c index 4d20d0f..46f8f7d 100644 --- a/src/runtime/thread.c +++ b/src/runtime/thread.c @@ -401,6 +401,9 @@ undo_init_new_thread(struct thread *th, init_thread_data *scribble) #ifdef LISP_FEATURE_SB_SAFEPOINT block_blockable_signals(0, 0); gc_alloc_update_page_tables(BOXED_PAGE_FLAG, &th->alloc_region); +#if defined(LISP_FEATURE_SB_SAFEPOINT_STRICTLY) && !defined(LISP_FEATURE_WIN32) + gc_alloc_update_page_tables(BOXED_PAGE_FLAG, &th->sprof_alloc_region); +#endif pop_gcing_safety(&scribble->safety); lock_ret = pthread_mutex_lock(&all_threads_lock); gc_assert(lock_ret == 0); @@ -418,6 +421,9 @@ undo_init_new_thread(struct thread *th, init_thread_data *scribble) gc_assert(lock_ret == 0); gc_alloc_update_page_tables(BOXED_PAGE_FLAG, &th->alloc_region); +#if defined(LISP_FEATURE_SB_SAFEPOINT_STRICTLY) && !defined(LISP_FEATURE_WIN32) + gc_alloc_update_page_tables(BOXED_PAGE_FLAG, &th->sprof_alloc_region); +#endif unlink_thread(th); pthread_mutex_unlock(&all_threads_lock); gc_assert(lock_ret == 0); @@ -700,6 +706,9 @@ create_thread_struct(lispobj initial_function) { #endif #ifdef LISP_FEATURE_GENCGC gc_set_region_empty(&th->alloc_region); +# if defined(LISP_FEATURE_SB_SAFEPOINT_STRICTLY) && !defined(LISP_FEATURE_WIN32) + gc_set_region_empty(&th->sprof_alloc_region); +# endif #endif #ifdef LISP_FEATURE_SB_THREAD /* This parallels the same logic in globals.c for the diff --git a/src/runtime/thread.h b/src/runtime/thread.h index 8bde9ba..1a004c0 100644 --- a/src/runtime/thread.h +++ b/src/runtime/thread.h @@ -422,6 +422,11 @@ int check_pending_thruptions(os_context_t *ctx); void attach_os_thread(init_thread_data *); void detach_os_thread(init_thread_data *); +# if defined(LISP_FEATURE_SB_SAFEPOINT_STRICTLY) && !defined(LISP_FEATURE_WIN32) + +void signal_handler_callback(lispobj, int, void *, void *); +# endif + #endif extern void create_initial_thread(lispobj); diff --git a/tests/signals.impure.lisp b/tests/signals.impure.lisp index 716580c..c4f347e 100644 --- a/tests/signals.impure.lisp +++ b/tests/signals.impure.lisp @@ -70,9 +70,15 @@ :skipped-on :win32) (assert (eq :condition (handler-case - (sb-thread::kill-safely - (sb-thread::thread-os-thread sb-thread::*current-thread*) - sb-unix:sigint) + (progn + (sb-thread::kill-safely + (sb-thread::thread-os-thread sb-thread::*current-thread*) + sb-unix:sigint) + #+sb-safepoint-strictly + ;; In this case, the signals handler gets invoked + ;; indirectly through an INTERRUPT-THREAD. Give it + ;; enough time to hit. + (sleep 1)) (sb-sys:interactive-interrupt () :condition))))) diff --git a/tests/test-util.lisp b/tests/test-util.lisp index 1d44174..20b2c54 100644 --- a/tests/test-util.lisp +++ b/tests/test-util.lisp @@ -69,6 +69,10 @@ (setf ,threads (union (union *threads-to-kill* *threads-to-join*) ,threads)) + #+(and sb-safepoint-strictly (not win32)) + (dolist (thread (sb-thread:list-all-threads)) + (when (typep thread 'sb-thread:signal-handling-thread) + (ignore-errors (sb-thread:join-thread thread)))) (dolist (thread (sb-thread:list-all-threads)) (unless (or (not (sb-thread:thread-alive-p thread)) (eql thread sb-thread:*current-thread*) -- 1.7.10.4