Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

i#1921 native sig: Account for sigaltstack when placing native frames #4645

Merged
merged 1 commit into from
Dec 31, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 61 additions & 46 deletions core/unix/signal.c
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
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
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