Skip to content

Commit

Permalink
i#1921 native sig: Account for sigaltstack when placing native frames
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@derekbruening
derekbruening committed Dec 30, 2020
1 parent b3bc4df commit d603c2b
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 51 deletions.
107 changes: 61 additions & 46 deletions core/unix/signal.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion core/win32/events.mc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
28 changes: 24 additions & 4 deletions suite/tests/api/detach_signal.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down Expand Up @@ -103,14 +106,21 @@ 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 */
};
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. */
Expand All @@ -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;
}

Expand Down Expand Up @@ -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);

Expand Down

0 comments on commit d603c2b

Please sign in to comment.