From d603c2b3e3c4f1d80829f4dc16f03c78614c1b1e Mon Sep 17 00:00:00 2001 From: Derek Bruening Date: Tue, 29 Dec 2020 01:10:18 -0500 Subject: [PATCH] i#1921 native sig: Account for sigaltstack when placing native frames The native frame adjustments to handle the DR handler being on the native stack previously failed to handle an application with a sigaltstack. We correct that here. Adds sigaltstack testing to the api.detach_signal test. With a smaller stack, this exercises the stack-too-small error message path from DR. Issue: #1921 --- core/unix/signal.c | 107 +++++++++++++++++------------- core/win32/events.mc | 2 +- suite/tests/api/detach_signal.cpp | 28 ++++++-- 3 files changed, 86 insertions(+), 51 deletions(-) diff --git a/core/unix/signal.c b/core/unix/signal.c index 1b44c61c4e0..835f52edb28 100644 --- a/core/unix/signal.c +++ b/core/unix/signal.c @@ -443,6 +443,21 @@ handler_alloc(dcontext_t *dcontext, size_t size) return global_heap_alloc(size HEAPACCT(ACCT_OTHER)); } +/* Does not return. */ +static void +report_unhandleable_signal_and_exit(int sig, const char *sub_message) +{ + /* TODO i#1921: Add more info such as the PC and disasm of surrounding instrs. */ + char signum_str[8]; + snprintf(signum_str, BUFFER_SIZE_ELEMENTS(signum_str), "%d", sig); + NULL_TERMINATE_BUFFER(signum_str); + char tid_str[16]; + snprintf(tid_str, BUFFER_SIZE_ELEMENTS(tid_str), TIDFMT, get_sys_thread_id()); + NULL_TERMINATE_BUFFER(tid_str); + REPORT_FATAL_ERROR_AND_EXIT(FAILED_TO_HANDLE_SIGNAL, 5, get_application_name(), + get_application_pid(), signum_str, tid_str, sub_message); +} + /**** top-level routines ***********************************************/ static bool @@ -2957,11 +2972,39 @@ get_sigstack_frame_ptr(dcontext_t *dcontext, thread_sig_info_t *info, int sig, if (frame != NULL) { /* Signal happened natively or while in the cache: grab interrupted xsp. */ sp = (byte *)sc->SC_XSP; + LOG(THREAD, LOG_ASYNCH, 3, "get_sigstack_frame_ptr: using frame's xsp " PFX "\n", + sp); + } else { + /* signal happened while in DR, use stored xsp */ + sp = (byte *)get_mcontext(dcontext)->xsp; + LOG(THREAD, LOG_ASYNCH, 3, "get_sigstack_frame_ptr: using app xsp " PFX "\n", sp); + } + + if (USE_APP_SIGSTACK(info, sig)) { + /* app has own signal stack which is enabled for this handler */ + LOG(THREAD, LOG_ASYNCH, 3, "get_sigstack_frame_ptr: app has own stack " PFX "\n", + info->app_sigstack.ss_sp); + LOG(THREAD, LOG_ASYNCH, 3, "\tcur sp=" PFX " vs app stack " PFX "-" PFX "\n", sp, + info->app_sigstack.ss_sp, + info->app_sigstack.ss_sp + info->app_sigstack.ss_size); + if (sp > (byte *)info->app_sigstack.ss_sp && + sp - (byte *)info->app_sigstack.ss_sp < info->app_sigstack.ss_size) { + /* we're currently in the alt stack, so use current xsp */ + LOG(THREAD, LOG_ASYNCH, 3, + "\tinside alt stack, so using current xsp " PFX "\n", sp); + } else { + /* need to go to top, stack grows down */ + sp = info->app_sigstack.ss_sp + info->app_sigstack.ss_size; + LOG(THREAD, LOG_ASYNCH, 3, + "\tnot inside alt stack, so using base xsp " PFX "\n", sp); + } + } + + if (frame != NULL) { /* Handle DR's frame already being on the app stack. For native delivery we * could try to re-use this frame, but that doesn't work with plain vs rt. * Instead we move below and live with the downsides of a potential stack - * overflow and confusing the app over why it's so low: but this is an app - * with no sigaltstack so it should have plenty of room. + * overflow. */ size_t frame_sz_max = sizeof(sigframe_rt_t) + REDZONE_SIZE + IF_LINUX(IF_X86((sc->fpstate == NULL ? 0 @@ -2975,43 +3018,30 @@ get_sigstack_frame_ptr(dcontext_t *dcontext, thread_sig_info_t *info, int sig, /* We have to copy the frame below our own stack usage high watermark * in the rest of the code we'll execute from execute_native_handler(). * Kind of a mess. For now we estimate two pages which should be plenty - * and still not be egregious usage for most app stacks. + * and still not be egregious usage for most app stacks. For the altstack + * we check the size below. */ #define EXECUTE_NATIVE_STACK_USAGE (4096 * 2) if (sp > cur_sp && sp - frame_sz_max - EXECUTE_NATIVE_STACK_USAGE < cur_sp) { sp = cur_sp - frame_sz_max - EXECUTE_NATIVE_STACK_USAGE; } + if (USE_APP_SIGSTACK(info, sig)) { +#define APP_ALTSTACK_USAGE (SIGSTKSZ / 2) + if (sp - APP_ALTSTACK_USAGE < (byte *)info->app_sigstack.ss_sp) { + /* There's not enough stack space. The only solution would be to + * re-use DR's frame and limit our own stack usage here, which + * gets complex. We bail. + */ + report_unhandleable_signal_and_exit( + sig, "sigaltstack too small in native thread"); + ASSERT_NOT_REACHED(); + } + } LOG(THREAD, LOG_ASYNCH, 3, "get_sigstack_frame_ptr: moving beyond same-stack frame to %p\n", sp); - } else { - LOG(THREAD, LOG_ASYNCH, 3, - "get_sigstack_frame_ptr: using frame's xsp " PFX "\n", sp); } - } else { - /* signal happened while in DR, use stored xsp */ - sp = (byte *)get_mcontext(dcontext)->xsp; - LOG(THREAD, LOG_ASYNCH, 3, "get_sigstack_frame_ptr: using app xsp " PFX "\n", sp); } - if (USE_APP_SIGSTACK(info, sig)) { - /* app has own signal stack which is enabled for this handler */ - LOG(THREAD, LOG_ASYNCH, 3, "get_sigstack_frame_ptr: app has own stack " PFX "\n", - info->app_sigstack.ss_sp); - LOG(THREAD, LOG_ASYNCH, 3, "\tcur sp=" PFX " vs app stack " PFX "-" PFX "\n", sp, - info->app_sigstack.ss_sp, - info->app_sigstack.ss_sp + info->app_sigstack.ss_size); - if (sp > (byte *)info->app_sigstack.ss_sp && - sp - (byte *)info->app_sigstack.ss_sp < info->app_sigstack.ss_size) { - /* we're currently in the alt stack, so use current xsp */ - LOG(THREAD, LOG_ASYNCH, 3, - "\tinside alt stack, so using current xsp " PFX "\n", sp); - } else { - /* need to go to top, stack grows down */ - sp = info->app_sigstack.ss_sp + info->app_sigstack.ss_size; - LOG(THREAD, LOG_ASYNCH, 3, - "\tnot inside alt stack, so using base xsp " PFX "\n", sp); - } - } /* now get frame pointer: need to go down to first field of frame */ sp -= get_app_frame_size(info, sig); #if defined(LINUX) && defined(X86) @@ -5814,21 +5844,6 @@ execute_handler_from_dispatch(dcontext_t *dcontext, int sig) return true; } -/* Does not return. */ -static void -report_unhandleable_signal_and_exit(int sig) -{ - /* TODO i#1921: Add more info such as the PC and disasm of surrounding instrs. */ - char signum_str[8]; - snprintf(signum_str, BUFFER_SIZE_ELEMENTS(signum_str), "%d", sig); - NULL_TERMINATE_BUFFER(signum_str); - char tid_str[16]; - snprintf(tid_str, BUFFER_SIZE_ELEMENTS(tid_str), TIDFMT, get_sys_thread_id()); - NULL_TERMINATE_BUFFER(tid_str); - REPORT_FATAL_ERROR_AND_EXIT(FAILED_TO_HANDLE_SIGNAL, 4, get_application_name(), - get_application_pid(), signum_str, tid_str); -} - /* Sends a signal to a currently-native thread. dcontext can be NULL. */ static void execute_native_handler(dcontext_t *dcontext, int sig, sigframe_rt_t *our_frame) @@ -5848,7 +5863,7 @@ execute_native_handler(dcontext_t *dcontext, int sig, sigframe_rt_t *our_frame) } else { if (atomic_read_bool(&multiple_handlers_present)) { /* See i#1921 comment up top: we don't handle this. */ - report_unhandleable_signal_and_exit(sig); + report_unhandleable_signal_and_exit(sig, "multiple native handlers"); ASSERT_NOT_REACHED(); } info = &synthetic; @@ -5909,7 +5924,7 @@ execute_native_handler(dcontext_t *dcontext, int sig, sigframe_rt_t *our_frame) } if (default_action[sig] == DEFAULT_IGNORE) return; - report_unhandleable_signal_and_exit(sig); + report_unhandleable_signal_and_exit(sig, "default action in native thread"); ASSERT_NOT_REACHED(); return; } diff --git a/core/win32/events.mc b/core/win32/events.mc index a519846db0c..ebeb86d6a65 100644 --- a/core/win32/events.mc +++ b/core/win32/events.mc @@ -632,7 +632,7 @@ Severity = Error Facility = DRCore SymbolicName = MSG_FAILED_TO_HANDLE_SIGNAL Language=English -Application %1!s! (%2!s!). Cannot correctly handle received signal %3!s! in thread %4!s!. +Application %1!s! (%2!s!). Cannot correctly handle received signal %3!s! in thread %4!s!: %5!s!. . ;#endif diff --git a/suite/tests/api/detach_signal.cpp b/suite/tests/api/detach_signal.cpp index ec81008f3fa..7d5b47ec6b7 100644 --- a/suite/tests/api/detach_signal.cpp +++ b/suite/tests/api/detach_signal.cpp @@ -58,6 +58,9 @@ # define VPRINT(...) /* nothing */ #endif +/* SIGSTKSZ*2 results in a fatal error from DR on fitting the copied frame. */ +#define ALT_STACK_SIZE (SIGSTKSZ * 4) + static volatile bool sideline_exit = false; static void *sideline_continue; static void *sideline_ready[NUM_THREADS]; @@ -103,6 +106,13 @@ sideline_spinner(void *arg) VPRINT("%d signaling sideline_ready\n", idx); signal_cond_var(sideline_ready[idx]); + stack_t sigstack; + sigstack.ss_sp = (char *)malloc(ALT_STACK_SIZE); + sigstack.ss_size = ALT_STACK_SIZE; + sigstack.ss_flags = 0; + int res = sigaltstack(&sigstack, NULL); + assert(res == 0); + /* Block some signals to test mask preservation. */ sigset_t mask = { 0, /* Set padding to 0 so we can use memcmp */ @@ -110,7 +120,7 @@ sideline_spinner(void *arg) sigemptyset(&mask); sigaddset(&mask, SIGUSR1); sigaddset(&mask, SIGURG); - int res = sigprocmask(SIG_SETMASK, &mask, NULL); + res = sigprocmask(SIG_SETMASK, &mask, NULL); assert(res == 0); /* Now sit in a signal-generating loop. */ @@ -135,6 +145,16 @@ sideline_spinner(void *arg) assert(res == 0 && memcmp(&mask, &check_mask, sizeof(mask)) == 0); } + stack_t check_stack; + res = sigaltstack(NULL, &check_stack); + assert(res == 0 && check_stack.ss_sp == sigstack.ss_sp && + check_stack.ss_size == sigstack.ss_size && + check_stack.ss_flags == sigstack.ss_flags); + sigstack.ss_flags = SS_DISABLE; + res = sigaltstack(&sigstack, NULL); + assert(res == 0); + free(sigstack.ss_sp); + return THREAD_FUNC_RETURN_ZERO; } @@ -166,10 +186,10 @@ main(void) res = sigprocmask(SIG_SETMASK, &prior_mask, NULL); assert(res == 0); - intercept_signal_with_mask(SIGSEGV, (handler_3_t)&handle_signal, false, - &handler_mask); + /* We request an alt stack for some signals but not all to test both types. */ + intercept_signal_with_mask(SIGSEGV, (handler_3_t)&handle_signal, true, &handler_mask); intercept_signal_with_mask(SIGBUS, (handler_3_t)&handle_signal, false, &handler_mask); - intercept_signal_with_mask(SIGURG, (handler_3_t)&handle_signal, false, &handler_mask); + intercept_signal_with_mask(SIGURG, (handler_3_t)&handle_signal, true, &handler_mask); intercept_signal_with_mask(SIGALRM, (handler_3_t)&handle_signal, false, &handler_mask);