Skip to content

Commit

Permalink
i#5145: Fix arm-vs-thumb signal transitions
Browse files Browse the repository at this point in the history
Removes a too-early-and-thus-incorrect call to set_pc_mode_in_cpsr()
in execute_handler_from_cache()
(transfer_from_sig_handler_to_fcache_return() does this for us at the
right time).

Removes an incorrect call to dr_set_isa_mode from the cpsr in
transfer_from_sig_handler_to_fcache_return(): we want to only set the
mode from the target, not the interruption point.

Works around QEMU bugs with signals 63 and 64 by using 62 instead in
the linux.signalNNNN tests.  This allows adding them to the list of
tests that work under QEMU.

Augments the linux.signalNNNN tests to vary whether the main code and
the signal handler are arm or thumb, helping to catch and test signal
transition issues.

Issue: #4719, #5145
Fixes #5145
  • Loading branch information
derekbruening committed Dec 6, 2021
1 parent 5e27a98 commit 3ee8649
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 9 deletions.
5 changes: 0 additions & 5 deletions core/unix/signal.c
Original file line number Diff line number Diff line change
Expand Up @@ -3665,7 +3665,6 @@ transfer_from_sig_handler_to_fcache_return(dcontext_t *dcontext, kernel_ucontext
sc_interrupted->SC_XIP = official_xl8;
}
dcontext->next_tag = canonicalize_pc_target(dcontext, next_pc);
IF_ARM(dr_set_isa_mode(dcontext, get_pc_mode_from_cpsr(sc), NULL));

/* Set our sigreturn context to point to fcache_return!
* Then we'll go back through kernel, appear in fcache_return,
Expand Down Expand Up @@ -5713,10 +5712,6 @@ execute_handler_from_cache(dcontext_t *dcontext, int sig, sigframe_rt_t *our_fra
sc->SC_LR = (reg_t)info->app_sigaction[sig]->restorer;
else
sc->SC_LR = (reg_t)dynamorio_sigreturn;
# ifndef AARCH64
/* We're going to our fcache_return gencode which uses DEFAULT_ISA_MODE */
set_pc_mode_in_cpsr(sc, DEFAULT_ISA_MODE);
# endif
#endif
/* Set our sigreturn context (NOT for the app: we already copied the
* translated context to the app stack) to point to fcache_return!
Expand Down
16 changes: 16 additions & 0 deletions suite/tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4916,6 +4916,22 @@ if (NOT ANDROID AND AARCHXX)
code_api|linux.alarm
code_api|linux.fork-sleep
code_api|linux.signal_race
code_api|linux.signal0000
code_api|linux.signal0001
code_api|linux.signal0010
code_api|linux.signal0011
code_api|linux.signal0100
code_api|linux.signal0101
code_api|linux.signal0110
code_api|linux.signal0111
code_api|linux.signal1000
code_api|linux.signal1001
code_api|linux.signal1010
code_api|linux.signal1011
code_api|linux.signal1100
code_api|linux.signal1101
code_api|linux.signal1110
code_api|linux.signal1111
code_api|linux.sigplain000
code_api|linux.sigplain001
code_api|linux.sigplain010
Expand Down
27 changes: 23 additions & 4 deletions suite/tests/linux/signal-base.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* **********************************************************
* Copyright (c) 2011-2018 Google, Inc. All rights reserved.
* Copyright (c) 2011-2021 Google, Inc. All rights reserved.
* Copyright (c) 2003-2010 VMware, Inc. All rights reserved.
* **********************************************************/

Expand Down Expand Up @@ -77,6 +77,14 @@ static sigjmp_buf env;
# define ITERS 500000
#endif

#ifdef AARCHXX
/* i#4719: Work around QEMU bugs where QEMU can't handle signals 63 or 64. */
# undef SIGRTMAX
# define SIGRTMAX 62
# undef __SIGRTMAX
# define __SIGRTMAX SIGRTMAX
#endif

static int a[ITERS];

/* strategy: anything that won't be the same across multiple runs,
Expand All @@ -90,7 +98,11 @@ static int timer_hits = 0;
#include <errno.h>

static void
signal_handler(int sig, siginfo_t *siginfo, ucontext_t *ucxt)
#if defined(ARM) && !defined(USE_SIGSTACK)
/* Test a variety of ISA transitions by tying this to USE_SIGSTACK. */
__attribute__((target("arm")))
#endif
signal_handler(int sig, siginfo_t *siginfo, ucontext_t *ucxt)
{
#if VERBOSE
print("signal_handler: sig=%d, retaddr=0x%08x, ucxt=0x%08x\n", sig, *(&sig - 1),
Expand Down Expand Up @@ -145,7 +157,10 @@ signal_handler(int sig, siginfo_t *siginfo, ucontext_t *ucxt)
void *pc = (void *)sc->SC_XIP;
/* SIGRTMAX has been 64 on Linux since kernel 2.1, from looking at glibc
* sources. */
assert(__SIGRTMAX == 64 && __SIGRTMAX == SIGRTMAX);
# ifndef AARCHXX /* i#4719: Work around QEMU bugs handling signals 63,64. */
assert(__SIGRTMAX == 64);
# endif
assert(__SIGRTMAX == SIGRTMAX);
# if VERBOSE
print("Got SIGRTMAX @ 0x%08x\n", pc);
# else
Expand Down Expand Up @@ -193,7 +208,11 @@ custom_intercept_signal(int sig, handler_t handler)
}

int
main(int argc, char *argv[])
#if defined(ARM) && !defined(BLOCK_IN_HANDLER)
/* Test a variety of ISA transitions by tying this to BLOCK_IN_HANDLER. */
__attribute__((target("arm")))
#endif
main(int argc, char *argv[])
{
double res = 0.;
int i, j, rc;
Expand Down

0 comments on commit 3ee8649

Please sign in to comment.