Skip to content

Commit

Permalink
fix: disable sigaltstack on Android (#901)
Browse files Browse the repository at this point in the history
  • Loading branch information
supervacuus authored Nov 10, 2023
1 parent 1040b51 commit 02fe0c4
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

**Fixes**:

- Disable sigaltstack on Android ([#901](https://github.com/getsentry/sentry-native/pull/901))
- Prevent stuck crashpad-client on Windows ([#902](https://github.com/getsentry/sentry-native/pull/902), [crashpad#89](https://github.com/getsentry/crashpad/pull/89))

## 0.6.6
Expand Down
35 changes: 33 additions & 2 deletions src/backends/sentry_backend_inproc.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,31 @@
#include "transports/sentry_disk_transport.h"
#include <string.h>

/**
* Android's bionic libc seems to allocate alternate signal handler stacks for
* every thread and also references them from their internal maintenance
* structs.
*
* The way we currently set up our sigaltstack seems to interfere with this
* setup and causes crashes whenever an ART signal handler touches the thread
* that called `sentry_init()`.
*
* In addition to this problem, it also means there is no need for our own
* sigaltstack on Android since our signal handler will always be running on
* an alternate stack managed by bionic.
*
* Note: In bionic the sigaltstacks for 32-bit devices have a size of 16KiB and
* on 64-bit devices they have 32KiB. The size of our own was set to 64KiB
* independent of the device. If this is a problem, we need figure out
* together with Google if there is a way in which our configs can coexist.
*
* Both breakpad and crashpad are way more defensive in the setup of their
* signal stacks and take existing stacks into account (or reuse them).
*/
#ifndef SENTRY_PLATFORM_ANDROID
# define SETUP_SIGALTSTACK
#endif

#define SIGNAL_DEF(Sig, Desc) \
{ \
Sig, #Sig, Desc \
Expand All @@ -32,8 +57,9 @@ struct signal_slot {
# define SIGNAL_STACK_SIZE 65536
static struct sigaction g_sigaction;
static struct sigaction g_previous_handlers[SIGNAL_COUNT];
# ifdef SETUP_SIGALTSTACK
static stack_t g_signal_stack;

# endif
static const struct signal_slot SIGNAL_DEFINITIONS[SIGNAL_COUNT] = {
SIGNAL_DEF(SIGILL, "IllegalInstruction"),
SIGNAL_DEF(SIGTRAP, "Trap"),
Expand Down Expand Up @@ -87,14 +113,15 @@ startup_inproc_backend(
}

// install our own signal handler
# ifdef SETUP_SIGALTSTACK
g_signal_stack.ss_sp = sentry_malloc(SIGNAL_STACK_SIZE);
if (!g_signal_stack.ss_sp) {
return 1;
}
g_signal_stack.ss_size = SIGNAL_STACK_SIZE;
g_signal_stack.ss_flags = 0;
sigaltstack(&g_signal_stack, 0);

# endif
sigemptyset(&g_sigaction.sa_mask);
g_sigaction.sa_sigaction = handle_signal;
g_sigaction.sa_flags = SA_SIGINFO | SA_ONSTACK;
Expand All @@ -107,10 +134,12 @@ startup_inproc_backend(
static void
shutdown_inproc_backend(sentry_backend_t *UNUSED(backend))
{
# ifdef SETUP_SIGALTSTACK
g_signal_stack.ss_flags = SS_DISABLE;
sigaltstack(&g_signal_stack, 0);
sentry_free(g_signal_stack.ss_sp);
g_signal_stack.ss_sp = NULL;
# endif
reset_signal_handlers();
}

Expand Down Expand Up @@ -472,6 +501,8 @@ make_signal_event(
void *backtrace[MAX_FRAMES];
size_t frame_count
= sentry_unwind_stack_from_ucontext(uctx, &backtrace[0], MAX_FRAMES);
SENTRY_TRACEF(
"captured backtrace from ucontext with %lu frames", frame_count);
// if unwinding from a ucontext didn't yield any results, try again with a
// direct unwind. this is most likely the case when using `libbacktrace`,
// since that does not allow to unwind from a ucontext at all.
Expand Down

0 comments on commit 02fe0c4

Please sign in to comment.