Skip to content

Commit

Permalink
i#3336: increase determinism of linux.syscall_pwait test. (#3337)
Browse files Browse the repository at this point in the history 
Stabilize linux.syscall_pwait test that currently sporadically fails on Android.

Checks for pending signals in emulation of p* versions of system calls and aborts call if necessary. Supports restoring pre-syscall sigmask in signal frame.

Changes linux.syscall_pwait to using pthreads and cond vars. Adds subtest to check signal in "pending" state as well as signals arriving during system call execution.

Fixes #3336, #3344
  • Loading branch information
Hendrik Greving authored Jan 16, 2019
1 parent f9dc5f5 commit aedafbf
Show file tree
Hide file tree
Showing 8 changed files with 213 additions and 138 deletions.
56 changes: 45 additions & 11 deletions core/unix/os.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7441,14 +7441,25 @@ pre_system_call(dcontext_t *dcontext)
*/
dcontext->sys_param3 = (reg_t)sigmask;
set_syscall_param(dcontext, 3, (reg_t)NULL);
if (!handle_pre_extended_syscall_sigmasks(dcontext, sigmask, sizemask)) {
bool sig_pending = false;
if (!handle_pre_extended_syscall_sigmasks(dcontext, sigmask, sizemask,
&sig_pending)) {
/* In old kernels with sizeof(kernel_sigset_t) != sizemask, we're forcing
* failure. We're already violating app transparency in other places in DR.
*/
set_failure_return_val(dcontext, EINVAL);
DODEBUG({ dcontext->expect_last_syscall_to_fail = true; });
execute_syscall = false;
}
if (sig_pending) {
/* If there had been pending signals, we revert re-writing the app's
* parameter, but we leave the modified signal mask.
*/
set_syscall_param(dcontext, 3, dcontext->sys_param3);
set_failure_return_val(dcontext, EINTR);
DODEBUG({ dcontext->expect_last_syscall_to_fail = true; });
execute_syscall = false;
}
break;
}
case SYS_pselect6: {
Expand All @@ -7467,20 +7478,35 @@ pre_system_call(dcontext_t *dcontext)
set_failure_return_val(dcontext, EFAULT);
DODEBUG({ dcontext->expect_last_syscall_to_fail = true; });
execute_syscall = false;
} else {
dcontext->sys_param4 = (reg_t)data.sigmask;
kernel_sigset_t *nullsigmaskptr = NULL;
break;
}
dcontext->sys_param4 = (reg_t)data.sigmask;
kernel_sigset_t *nullsigmaskptr = NULL;
if (!safe_write_ex((void *)&data_param->sigmask, sizeof(data_param->sigmask),
&nullsigmaskptr, NULL)) {
set_failure_return_val(dcontext, EFAULT);
DODEBUG({ dcontext->expect_last_syscall_to_fail = true; });
execute_syscall = false;
break;
}
bool sig_pending = false;
if (!handle_pre_extended_syscall_sigmasks(dcontext, data.sigmask, data.sizemask,
&sig_pending)) {
set_failure_return_val(dcontext, EINVAL);
DODEBUG({ dcontext->expect_last_syscall_to_fail = true; });
execute_syscall = false;
}
if (sig_pending) {
if (!safe_write_ex((void *)&data_param->sigmask, sizeof(data_param->sigmask),
&nullsigmaskptr, NULL)) {
&dcontext->sys_param4, NULL)) {
set_failure_return_val(dcontext, EFAULT);
DODEBUG({ dcontext->expect_last_syscall_to_fail = true; });
execute_syscall = false;
} else if (!handle_pre_extended_syscall_sigmasks(dcontext, data.sigmask,
data.sizemask)) {
set_failure_return_val(dcontext, EINVAL);
DODEBUG({ dcontext->expect_last_syscall_to_fail = true; });
execute_syscall = false;
break;
}
set_failure_return_val(dcontext, EINTR);
DODEBUG({ dcontext->expect_last_syscall_to_fail = true; });
execute_syscall = false;
}
break;
}
Expand All @@ -7490,11 +7516,19 @@ pre_system_call(dcontext_t *dcontext)
/* Refer to comments in SYS_ppoll above. */
dcontext->sys_param4 = (reg_t)sigmask;
set_syscall_param(dcontext, 4, (reg_t)NULL);
if (!handle_pre_extended_syscall_sigmasks(dcontext, sigmask, sizemask)) {
bool sig_pending = false;
if (!handle_pre_extended_syscall_sigmasks(dcontext, sigmask, sizemask,
&sig_pending)) {
set_failure_return_val(dcontext, EINVAL);
DODEBUG({ dcontext->expect_last_syscall_to_fail = true; });
execute_syscall = false;
}
if (sig_pending) {
set_syscall_param(dcontext, 4, dcontext->sys_param4);
set_failure_return_val(dcontext, EINTR);
DODEBUG({ dcontext->expect_last_syscall_to_fail = true; });
execute_syscall = false;
}
break;
}
# endif
Expand Down
2 changes: 1 addition & 1 deletion core/unix/os_private.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ handle_sigreturn(dcontext_t *dcontext, void *ucxt, int style);
#ifdef LINUX
bool
handle_pre_extended_syscall_sigmasks(dcontext_t *dcontext, kernel_sigset_t *sigmask,
size_t sizemask);
size_t sizemask, bool *pending);

void
handle_post_extended_syscall_sigmasks(dcontext_t *dcontext, bool success);
Expand Down
20 changes: 16 additions & 4 deletions core/unix/signal.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1060,6 +1060,8 @@ signal_thread_inherit(dcontext_t *dcontext, void *clone_record)
(record == NULL) ? NULL : record->pcprofile_info);
}

info->pre_syscall_app_sigprocmask_valid = false;

/* Assumed to be async safe. */
info->fully_initialized = true;
}
Expand Down Expand Up @@ -1161,6 +1163,8 @@ signal_fork_init(dcontext_t *dcontext)
pcprofile_fork_init(dcontext);
}

info->pre_syscall_app_sigprocmask_valid = false;

/* Assumed to be async safe. */
info->fully_initialized = true;
}
Expand Down Expand Up @@ -2064,7 +2068,7 @@ signal_swap_mask(dcontext_t *dcontext, bool to_app)
* signals, and sets dcontext->signals_pending if there are. Do this after
* modifying the set of signals blocked by the application.
*/
static void
void
check_signals_pending(dcontext_t *dcontext, thread_sig_info_t *info)
{
int i;
Expand Down Expand Up @@ -3158,6 +3162,14 @@ copy_frame_to_stack(dcontext_t *dcontext, int sig, sigframe_rt_t *frame, byte *s
ASSERT_NOT_REACHED();
}

kernel_sigset_t *mask_to_restore = NULL;
if (info->pre_syscall_app_sigprocmask_valid) {
mask_to_restore = &info->pre_syscall_app_sigprocmask;
info->pre_syscall_app_sigprocmask_valid = false;
} else {
mask_to_restore = &info->app_sigblocked;
}

/* if !has_restorer we do NOT add the restorer code to the exec list here,
* to avoid removal problems (if handler never returns) and consistency problems
* (would have to mark as selfmod right now if on stack).
Expand All @@ -3179,9 +3191,9 @@ copy_frame_to_stack(dcontext_t *dcontext, int sig, sigframe_rt_t *frame, byte *s
&f_new->uc.uc_stack, info->app_sigstack.ss_sp);
f_new->uc.uc_stack = info->app_sigstack;
#endif

/* Store the prior mask, for restoring in sigreturn. */
memcpy(&f_new->uc.uc_sigmask, &info->app_sigblocked,
sizeof(info->app_sigblocked));
memcpy(&f_new->uc.uc_sigmask, mask_to_restore, sizeof(info->app_sigblocked));
} else {
#ifdef X64
ASSERT_NOT_REACHED();
Expand Down Expand Up @@ -3224,7 +3236,7 @@ copy_frame_to_stack(dcontext_t *dcontext, int sig, sigframe_rt_t *frame, byte *s
# endif
# endif /* X86 */
/* Store the prior mask, for restoring in sigreturn. */
convert_rt_mask_to_nonrt(f_new, &info->app_sigblocked);
convert_rt_mask_to_nonrt(f_new, mask_to_restore);
#endif /* LINUX && !X64 */
}

Expand Down
8 changes: 7 additions & 1 deletion core/unix/signal_linux.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ signalfd_thread_exit(dcontext_t *dcontext, thread_sig_info_t *info)

bool
handle_pre_extended_syscall_sigmasks(dcontext_t *dcontext, kernel_sigset_t *sigmask,
size_t sizemask)
size_t sizemask, bool *pending)
{
thread_sig_info_t *info = (thread_sig_info_t *)dcontext->signal_field;

Expand All @@ -363,15 +363,21 @@ handle_pre_extended_syscall_sigmasks(dcontext_t *dcontext, kernel_sigset_t *sigm
*/
if (sizemask != sizeof(kernel_sigset_t))
return false;
ASSERT(!info->pre_syscall_app_sigprocmask_valid);
info->pre_syscall_app_sigprocmask_valid = true;
info->pre_syscall_app_sigprocmask = info->app_sigblocked;
signal_set_mask(dcontext, sigmask);
/* Make sure we deliver pending signals that are now unblocked. */
check_signals_pending(dcontext, info);
*pending = dcontext->signals_pending;
return true;
}

void
handle_post_extended_syscall_sigmasks(dcontext_t *dcontext, bool success)
{
thread_sig_info_t *info = (thread_sig_info_t *)dcontext->signal_field;
info->pre_syscall_app_sigprocmask_valid = false;
signal_set_mask(dcontext, &info->pre_syscall_app_sigprocmask);
}

Expand Down
5 changes: 5 additions & 0 deletions core/unix/signal_private.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,8 @@ typedef struct _thread_sig_info_t {
* mask supporting ppoll, epoll_pwait and pselect
*/
kernel_sigset_t pre_syscall_app_sigprocmask;
/* True if pre_syscall_app_sigprocmask holds a pre-syscall sigmask */
bool pre_syscall_app_sigprocmask_valid;
/* for alarm signals arriving in coarse units we only attempt to xl8
* every nth signal since coarse translation is expensive (PR 213040)
*/
Expand Down Expand Up @@ -615,6 +617,9 @@ signalfd_thread_exit(dcontext_t *dcontext, thread_sig_info_t *info);
bool
notify_signalfd(dcontext_t *dcontext, thread_sig_info_t *info, int sig,
sigframe_rt_t *frame);

void
check_signals_pending(dcontext_t *dcontext, thread_sig_info_t *info);
#endif

#endif /* _SIGNAL_PRIVATE_H_ */
7 changes: 5 additions & 2 deletions suite/tests/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,9 @@ function (set_cflags source)
endif ()
if ("${source}" MATCHES ".cpp$")
# Our C files need -std=gnu99, but that's an invalid flag for C++.
string(REGEX REPLACE "-std=gnu99" "" cflags "${cflags}")
# configure_DynamoRIO_global removes unfavorable options for clients,
# re-adding -std=c++11.
string(REGEX REPLACE "-std=gnu99" "-std=c++11" cflags "${cflags}")
if (WIN32)
set(cflags "${cflags} /EHsc")
endif (WIN32)
Expand Down Expand Up @@ -3121,7 +3123,8 @@ if (UNIX)
tobuild(linux.sigplain111 linux/sigplain111.c)
tobuild(linux.sigaction linux/sigaction.c)
if (LINUX)
tobuild(linux.syscall_pwait linux/syscall_pwait.c)
tobuild(linux.syscall_pwait linux/syscall_pwait.cpp)
link_with_pthread(linux.syscall_pwait)
endif ()
if (LINUX AND NOT ANDROID) # Only tests RT sigaction which is not supported on Android.
tobuild(linux.sigaction_nosignals linux/sigaction_nosignals.c)
Expand Down
Loading

0 comments on commit aedafbf

Please sign in to comment.