From 01eb6e146c929b559de2709dcb1138b2bc914e17 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Thu, 6 Nov 2025 11:10:44 +0100 Subject: [PATCH 01/75] fix: split inproc with a handler thread --- CMakeLists.txt | 14 +- src/CMakeLists.txt | 7 + src/backends/sentry_backend_inproc.c | 510 +++++++++++++++---- src/sentry_sync.c | 103 +++- src/sentry_sync.h | 1 + src/unwinder/sentry_unwinder.c | 4 + src/unwinder/sentry_unwinder_libunwind_mac.c | 92 ++++ tests/assertions.py | 12 +- 8 files changed, 633 insertions(+), 110 deletions(-) create mode 100644 src/unwinder/sentry_unwinder_libunwind_mac.c diff --git a/CMakeLists.txt b/CMakeLists.txt index 50beda333..b7639b7e5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -144,15 +144,6 @@ if(LINUX) set(CMAKE_ASM_FLAGS "${CMAKE_ASM_FLAGS} -m32 -D_FILE_OFFSET_BITS=64 -D_LARGEFILE64_SOURCE") set_property(GLOBAL PROPERTY FIND_LIBRARY_USE_LIB64_PATHS OFF) endif() - - execute_process( - COMMAND ${CMAKE_C_COMPILER} -dumpmachine - OUTPUT_VARIABLE TARGET_TRIPLET - OUTPUT_STRIP_TRAILING_WHITESPACE - ) - if(TARGET_TRIPLET MATCHES "musl") - set(MUSL TRUE) - endif() endif() # CMAKE_POSITION_INDEPENDENT_CODE must be set BEFORE adding any libraries (including subprojects) @@ -266,9 +257,12 @@ endif() if(ANDROID) set(SENTRY_WITH_LIBUNWINDSTACK TRUE) -elseif(MUSL) +elseif(LINUX) set(SENTRY_WITH_LIBUNWIND TRUE) +elseif(APPLE) + set(SENTRY_WITH_LIBUNWIND_MAC TRUE) elseif(NOT WIN32 AND NOT PROSPERO) + # this should never be true, but we keep it as fallback set(SENTRY_WITH_LIBBACKTRACE TRUE) endif() diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 738d1c110..e212c702c 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -167,6 +167,13 @@ if(SENTRY_WITH_LIBUNWIND) ) endif() +if(SENTRY_WITH_LIBUNWIND_MAC) + target_compile_definitions(sentry PRIVATE SENTRY_WITH_UNWINDER_LIBUNWIND_MAC) + sentry_target_sources_cwd(sentry + unwinder/sentry_unwinder_libunwind_mac.c + ) +endif() + if(SENTRY_WITH_LIBUNWINDSTACK) target_compile_definitions(sentry PRIVATE SENTRY_WITH_UNWINDER_LIBUNWINDSTACK) sentry_target_sources_cwd(sentry diff --git a/src/backends/sentry_backend_inproc.c b/src/backends/sentry_backend_inproc.c index 8a8755a75..89ef37628 100644 --- a/src/backends/sentry_backend_inproc.c +++ b/src/backends/sentry_backend_inproc.c @@ -4,6 +4,7 @@ #include "sentry_alloc.h" #include "sentry_backend.h" #include "sentry_core.h" +#include "sentry_cpu_relax.h" #include "sentry_database.h" #include "sentry_envelope.h" #include "sentry_logger.h" @@ -18,13 +19,56 @@ #include "sentry_transport.h" #include "sentry_unix_pageallocator.h" #include "transports/sentry_disk_transport.h" +#include +#include #include #define SIGNAL_DEF(Sig, Desc) { Sig, #Sig, Desc } #define MAX_FRAMES 128 +// the data exchange between the signal handler and the handler thread +typedef struct sentry_inproc_handler_state_s { + sentry_ucontext_t uctx; +#ifdef SENTRY_PLATFORM_UNIX + siginfo_t siginfo_storage; + ucontext_t user_context_storage; +#endif + const struct signal_slot *sig_slot; + sentry_handler_strategy_t strategy; +} sentry_inproc_handler_state_t; + +// "data" struct containing options to prevent mutex access in signal handler +typedef struct sentry_inproc_backend_config_s { + bool enable_logging_when_crashed; + sentry_handler_strategy_t handler_strategy; +} sentry_inproc_backend_config_t; + +// global instance for data-exchange between signal handler and handler thread +static sentry_inproc_handler_state_t g_handler_state; +// global instance for backend configuration state +static sentry_inproc_backend_config_t g_backend_config; + +// handler thread state and synchronization variables +static sentry_threadid_t g_handler_thread; +// true once the handler thread starts waiting +static volatile long g_handler_thread_ready = 0; +// shutdown loop invariant +static volatile long g_handler_should_exit = 0; +// signal handler tells handler thread to start working +static volatile long g_handler_has_work = 0; +// handler thread wakes signal handler from suspension after finishing the work +static volatile long g_handler_work_done = 0; + +// trigger/schedule primitives that block the handler thread until we need it +#ifdef SENTRY_PLATFORM_UNIX +static int g_handler_pipe[2] = { -1, -1 }; +#elif defined(SENTRY_PLATFORM_WINDOWS) +static HANDLE g_handler_semaphore = NULL; +#endif + #ifdef SENTRY_PLATFORM_UNIX +# include struct signal_slot { int signum; const char *signame; @@ -47,6 +91,8 @@ static const struct signal_slot SIGNAL_DEFINITIONS[SIGNAL_COUNT] = { }; static void handle_signal(int signum, siginfo_t *info, void *user_context); +static int start_handler_thread(void); +static void stop_handler_thread(void); static void reset_signal_handlers(void) @@ -77,8 +123,22 @@ invoke_signal_handler(int signum, siginfo_t *info, void *user_context) static int startup_inproc_backend( - sentry_backend_t *UNUSED(backend), const sentry_options_t *UNUSED(options)) + sentry_backend_t *backend, const sentry_options_t *options) { + // get option state so we don't need to sync read during signal handling + g_backend_config.enable_logging_when_crashed + = options ? options->enable_logging_when_crashed : true; + g_backend_config.handler_strategy = +# if defined(SENTRY_PLATFORM_LINUX) + options ? sentry_options_get_handler_strategy(options) : +# endif + SENTRY_HANDLER_STRATEGY_DEFAULT; + if (backend) { + backend->data = &g_backend_config; + } + + start_handler_thread(); + // save the old signal handlers memset(g_previous_handlers, 0, sizeof(g_previous_handlers)); for (size_t i = 0; i < SIGNAL_COUNT; ++i) { @@ -119,8 +179,10 @@ startup_inproc_backend( } static void -shutdown_inproc_backend(sentry_backend_t *UNUSED(backend)) +shutdown_inproc_backend(sentry_backend_t *backend) { + stop_handler_thread(); + if (g_signal_stack.ss_sp) { g_signal_stack.ss_flags = SS_DISABLE; sigaltstack(&g_signal_stack, 0); @@ -128,6 +190,9 @@ shutdown_inproc_backend(sentry_backend_t *UNUSED(backend)) g_signal_stack.ss_sp = NULL; } reset_signal_handlers(); + if (backend) { + backend->data = NULL; + } } #elif defined(SENTRY_PLATFORM_WINDOWS) @@ -169,25 +234,40 @@ static LONG WINAPI handle_exception(EXCEPTION_POINTERS *); static int startup_inproc_backend( - sentry_backend_t *UNUSED(backend), const sentry_options_t *UNUSED(options)) + sentry_backend_t *backend, const sentry_options_t *options) { + g_backend_config.enable_logging_when_crashed + = options ? options->enable_logging_when_crashed : true; + g_backend_config.handler_strategy = options + ? sentry_options_get_handler_strategy(options) + : SENTRY_HANDLER_STRATEGY_DEFAULT; + if (backend) { + backend->data = &g_backend_config; + } + # if !defined(SENTRY_BUILD_SHARED) \ && defined(SENTRY_THREAD_STACK_GUARANTEE_AUTO_INIT) sentry__set_default_thread_stack_guarantee(); # endif + start_handler_thread(); g_previous_handler = SetUnhandledExceptionFilter(&handle_exception); SetErrorMode(SEM_FAILCRITICALERRORS); return 0; } static void -shutdown_inproc_backend(sentry_backend_t *UNUSED(backend)) +shutdown_inproc_backend(sentry_backend_t *backend) { + stop_handler_thread(); + LPTOP_LEVEL_EXCEPTION_FILTER current_handler = SetUnhandledExceptionFilter(g_previous_handler); if (current_handler != &handle_exception) { SetUnhandledExceptionFilter(current_handler); } + if (backend) { + backend->data = NULL; + } } #endif @@ -517,6 +597,7 @@ make_signal_event(const struct signal_slot *sig_slot, signal_meta, "name", sentry_value_new_string(sig_slot->signame)); // at least on windows, the signum is a true u32 which we can't // otherwise represent. + // TODO: does that still match reality? sentry_value_set_by_key(signal_meta, "number", sentry_value_new_double((double)sig_slot->signum)); } @@ -563,97 +644,32 @@ make_signal_event(const struct signal_slot *sig_slot, return event; } +/** + * This is the signal-unsafe part of the inproc handler. Everything that + * requires stdio, time-formatting/-capture or serialization must happen here. + * + * Although we can use signal-unsafe functions here, this should still be + * written with care. Don't overly rely on thread synchronization since the + * program is in a crashed state. At least one thread no longer progresses and + * memory can be corrupted. + */ static void -handle_ucontext(const sentry_ucontext_t *uctx) +process_ucontext_deferred( + const sentry_ucontext_t *uctx, const struct signal_slot *sig_slot) { - // Disable logging during crash handling if the option is set - SENTRY_WITH_OPTIONS (options) { - if (!options->enable_logging_when_crashed) { - sentry__logger_disable(); - } - } - SENTRY_INFO("entering signal handler"); - sentry_handler_strategy_t strategy = SENTRY_HANDLER_STRATEGY_DEFAULT; -#ifdef SENTRY_PLATFORM_UNIX - // inform the sentry_sync system that we're in a signal handler. This will - // make mutexes spin on a spinlock instead as it's no longer safe to use a - // pthread mutex. - sentry__enter_signal_handler(); -#endif - SENTRY_WITH_OPTIONS (options) { -#ifdef SENTRY_PLATFORM_LINUX - // On Linux (and thus Android) CLR/Mono converts signals provoked by - // AOT/JIT-generated native code into managed code exceptions. In these - // cases, we shouldn't react to the signal at all and let their handler - // discontinue the signal chain by invoking the runtime handler before - // we process the signal. - strategy = sentry_options_get_handler_strategy(options); - if (strategy == SENTRY_HANDLER_STRATEGY_CHAIN_AT_START) { - SENTRY_DEBUG("defer to runtime signal handler at start"); - // there is a good chance that we won't return from the previous - // handler and that would mean we couldn't enter this handler with - // the next signal coming in if we didn't "leave" here. - sentry__leave_signal_handler(); - if (!options->enable_logging_when_crashed) { - sentry__logger_enable(); - } - - uintptr_t ip = get_instruction_pointer(uctx); - uintptr_t sp = get_stack_pointer(uctx); - - // invoke the previous handler (typically the CLR/Mono - // signal-to-managed-exception handler) - invoke_signal_handler( - uctx->signum, uctx->siginfo, (void *)uctx->user_context); - - // If the execution returns here in AOT mode, and the instruction - // or stack pointer were changed, it means CLR/Mono converted the - // signal into a managed exception and transferred execution to a - // managed exception handler. - // https://github.com/dotnet/runtime/blob/6d96e28597e7da0d790d495ba834cc4908e442cd/src/mono/mono/mini/exceptions-arm64.c#L538 - if (ip != get_instruction_pointer(uctx) - || sp != get_stack_pointer(uctx)) { - SENTRY_DEBUG("runtime converted the signal to a managed " - "exception, we do not handle the signal"); - return; - } - - // let's re-enter because it means this was an actual native crash - if (!options->enable_logging_when_crashed) { - sentry__logger_disable(); - } - sentry__enter_signal_handler(); - SENTRY_DEBUG( - "return from runtime signal handler, we handle the signal"); - } -#endif - - const struct signal_slot *sig_slot = NULL; - for (int i = 0; i < SIGNAL_COUNT; ++i) { -#ifdef SENTRY_PLATFORM_UNIX - if (SIGNAL_DEFINITIONS[i].signum == uctx->signum) { -#elif defined SENTRY_PLATFORM_WINDOWS - if (SIGNAL_DEFINITIONS[i].signum - == uctx->exception_ptrs.ExceptionRecord->ExceptionCode) { -#else -# error Unsupported platform -#endif - sig_slot = &SIGNAL_DEFINITIONS[i]; - } - } - -#ifdef SENTRY_PLATFORM_UNIX - // use a signal-safe allocator before we tear down. - sentry__page_allocator_enable(); -#endif // Flush logs in a crash-safe manner before crash handling if (options->enable_logs) { sentry__logs_flush_crash_safe(); } + sentry_handler_strategy_t strategy = +#if defined(SENTRY_PLATFORM_LINUX) + options ? sentry_options_get_handler_strategy(options) : +#endif + SENTRY_HANDLER_STRATEGY_DEFAULT; sentry_value_t event = make_signal_event(sig_slot, uctx, strategy); bool should_handle = true; sentry__write_crash_marker(options); @@ -699,9 +715,325 @@ handle_ucontext(const sentry_ucontext_t *uctx) // after capturing the crash event, dump all the envelopes to disk sentry__transport_dump_queue(options->transport, options->run); + + SENTRY_INFO("crash has been captured"); + } +} + +SENTRY_THREAD_FN +handler_thread_main(void *UNUSED(data)) +{ + sentry__atomic_store(&g_handler_thread_ready, 1); + + while (!sentry__atomic_fetch(&g_handler_should_exit)) { +#ifdef SENTRY_PLATFORM_UNIX + char command = 0; + ssize_t rv = read(g_handler_pipe[0], &command, 1); + if (rv == -1 && errno == EINTR) { + continue; + } + if (rv <= 0) { + if (sentry__atomic_fetch(&g_handler_should_exit)) { + break; + } + continue; + } +#elif defined(SENTRY_PLATFORM_WINDOWS) + DWORD wait_result = WaitForSingleObject(g_handler_semaphore, INFINITE); + if (wait_result != WAIT_OBJECT_0) { + continue; + } + if (sentry__atomic_fetch(&g_handler_should_exit)) { + break; + } +#endif + + if (!sentry__atomic_fetch(&g_handler_has_work)) { + continue; + } + +#ifdef SENTRY_PLATFORM_UNIX + sentry__switch_handler_thread(); +#endif + process_ucontext_deferred( + &g_handler_state.uctx, g_handler_state.sig_slot); + sentry__atomic_store(&g_handler_has_work, 0); + sentry__atomic_store(&g_handler_work_done, 1); + } + +#ifdef SENTRY_PLATFORM_WINDOWS + return 0; +#else + return NULL; +#endif +} + +static int +start_handler_thread(void) +{ + if (sentry__atomic_fetch(&g_handler_thread_ready)) { + return 0; + } + + sentry__thread_init(&g_handler_thread); + sentry__atomic_store(&g_handler_should_exit, 0); + sentry__atomic_store(&g_handler_has_work, 0); + sentry__atomic_store(&g_handler_work_done, 0); + +#ifdef SENTRY_PLATFORM_UNIX + if (pipe(g_handler_pipe) != 0) { + SENTRY_WARNF("failed to create handler pipe: %s", strerror(errno)); + return 1; + } +#elif defined(SENTRY_PLATFORM_WINDOWS) + g_handler_semaphore = CreateSemaphoreW(NULL, 0, LONG_MAX, NULL); + if (!g_handler_semaphore) { + SENTRY_WARN("failed to create handler semaphore"); + return 1; + } +#endif + + if (sentry__thread_spawn(&g_handler_thread, handler_thread_main, NULL) + != 0) { + SENTRY_WARN("failed to spawn handler thread"); +#ifdef SENTRY_PLATFORM_UNIX + close(g_handler_pipe[0]); + close(g_handler_pipe[1]); + g_handler_pipe[0] = -1; + g_handler_pipe[1] = -1; +#elif defined(SENTRY_PLATFORM_WINDOWS) + CloseHandle(g_handler_semaphore); + g_handler_semaphore = NULL; +#endif + return 1; } - SENTRY_INFO("crash has been captured"); + return 0; +} + +static void +stop_handler_thread(void) +{ + if (!sentry__atomic_fetch(&g_handler_thread_ready)) { + return; + } + + sentry__atomic_store(&g_handler_should_exit, 1); + +#ifdef SENTRY_PLATFORM_UNIX + if (g_handler_pipe[1] >= 0) { + char c = 0; + ssize_t rv; + do { + rv = write(g_handler_pipe[1], &c, 1); + } while (rv == -1 && errno == EINTR); + } +#elif defined(SENTRY_PLATFORM_WINDOWS) + if (g_handler_semaphore) { + ReleaseSemaphore(g_handler_semaphore, 1, NULL); + } +#endif + + sentry__thread_join(g_handler_thread); + sentry__thread_free(&g_handler_thread); + +#ifdef SENTRY_PLATFORM_UNIX + if (g_handler_pipe[0] >= 0) { + close(g_handler_pipe[0]); + g_handler_pipe[0] = -1; + } + if (g_handler_pipe[1] >= 0) { + close(g_handler_pipe[1]); + g_handler_pipe[1] = -1; + } +#elif defined(SENTRY_PLATFORM_WINDOWS) + if (g_handler_semaphore) { + CloseHandle(g_handler_semaphore); + g_handler_semaphore = NULL; + } +#endif + + sentry__atomic_store(&g_handler_thread_ready, 0); + sentry__atomic_store(&g_handler_should_exit, 0); +} + +static void +dispatch_ucontext(const sentry_ucontext_t *uctx, + const struct signal_slot *sig_slot, sentry_handler_strategy_t strategy) +{ + if (!sentry__atomic_fetch(&g_handler_thread_ready)) { + process_ucontext_deferred(uctx, sig_slot); + return; + } + + g_handler_state.uctx = *uctx; + g_handler_state.sig_slot = sig_slot; + +#ifdef SENTRY_PLATFORM_UNIX + if (uctx->siginfo) { + memcpy(&g_handler_state.siginfo_storage, uctx->siginfo, + sizeof(g_handler_state.siginfo_storage)); + g_handler_state.uctx.siginfo = &g_handler_state.siginfo_storage; + } else { + g_handler_state.uctx.siginfo = NULL; + } + + if (uctx->user_context) { + memcpy(&g_handler_state.user_context_storage, uctx->user_context, + sizeof(g_handler_state.user_context_storage)); + g_handler_state.uctx.user_context + = &g_handler_state.user_context_storage; + } else { + g_handler_state.uctx.user_context = NULL; + } +#endif + + sentry__atomic_store(&g_handler_work_done, 0); + sentry__atomic_store(&g_handler_has_work, 1); + + // signal the handler thread to start working +#ifdef SENTRY_PLATFORM_UNIX + if (g_handler_pipe[1] >= 0) { + char c = 1; + ssize_t rv; + do { + rv = write(g_handler_pipe[1], &c, 1); + } while (rv == -1 && errno == EINTR); + } +#elif defined(SENTRY_PLATFORM_WINDOWS) + if (g_handler_semaphore) { + ReleaseSemaphore(g_handler_semaphore, 1, NULL); + } +#endif + + // wait until the handler has done its work + while (!sentry__atomic_fetch(&g_handler_work_done)) { + sentry__cpu_relax(); + } + +#ifdef SENTRY_PLATFORM_UNIX + sentry__switch_handler_thread(); +#endif +} + +/** + * This is the signal-safe part of the inproc handler. Everything in here should + * not defer to more than the set of functions listed in: + * https://www.man7.org/linux/man-pages/man7/signal-safety.7.html + * + * That means: + * - no heap allocations except for sentry_malloc() (page allocator enabled!!!) + * - no stdio or any kind of libc string formatting + * - no logging (at least not with the printf-based default logger) + * - no pthread synchronization (SENTRY_WITH_OPTIONS will terminate with a log) + * - in particular, don't access sentry interfaces that could request + * access to options or the scope, those should go to the handler thread + * - sentry_value_* and sentry_malloc are generally fine, because we use a safe + * allocator, but keep in mind that some constructors create timestampy and + * similar stringy and thus formatted values (and those are forbidden here). + * + * If you are unsure about a particular function on a given target platform + * please consult the signal-safety man page. + * + * Another decision marker of whether code should go in here: do you must run + * on the preempted crashed thread? Do you need to run before anything else? + */ +static void +process_ucontext(const sentry_ucontext_t *uctx) +{ +#ifdef SENTRY_PLATFORM_UNIX + sentry__enter_signal_handler(); +#endif + + if (!g_backend_config.enable_logging_when_crashed) { + sentry__logger_disable(); + } + + sentry_handler_strategy_t strategy = g_backend_config.handler_strategy; + +#ifdef SENTRY_PLATFORM_LINUX + if (strategy == SENTRY_HANDLER_STRATEGY_CHAIN_AT_START) { + // On Linux (and thus Android) CLR/Mono converts signals provoked by + // AOT/JIT-generated native code into managed code exceptions. In these + // cases, we shouldn't react to the signal at all and let their handler + // discontinue the signal chain by invoking the runtime handler before + // we process the signal. + // there is a good chance that we won't return from the previous + // handler and that would mean we couldn't enter this handler with + // the next signal coming in if we didn't "leave" here. + sentry__leave_signal_handler(); + if (!g_backend_config.enable_logging_when_crashed) { + sentry__logger_enable(); + } + + uintptr_t ip = get_instruction_pointer(uctx); + uintptr_t sp = get_stack_pointer(uctx); + + // invoke the previous handler (typically the CLR/Mono + // signal-to-managed-exception handler) + invoke_signal_handler( + uctx->signum, uctx->siginfo, (void *)uctx->user_context); + + // If the execution returns here in AOT mode, and the instruction + // or stack pointer were changed, it means CLR/Mono converted the + // signal into a managed exception and transferred execution to a + // managed exception handler. + // https://github.com/dotnet/runtime/blob/6d96e28597e7da0d790d495ba834cc4908e442cd/src/mono/mono/mini/exceptions-arm64.c#L538 + if (ip != get_instruction_pointer(uctx) + || sp != get_stack_pointer(uctx)) { + return; + } + + // let's re-enter because it means this was an actual native crash + if (!g_backend_config.enable_logging_when_crashed) { + sentry__logger_disable(); + } + sentry__enter_signal_handler(); + // return from runtime handler; continue processing the crash on the + // signal thread until the worker takes over + } +#endif + + const struct signal_slot *sig_slot = NULL; + for (int i = 0; i < SIGNAL_COUNT; ++i) { +#ifdef SENTRY_PLATFORM_UNIX + if (SIGNAL_DEFINITIONS[i].signum == uctx->signum) { +#elif defined SENTRY_PLATFORM_WINDOWS + if (SIGNAL_DEFINITIONS[i].signum + == uctx->exception_ptrs.ExceptionRecord->ExceptionCode) { +#else +# error Unsupported platform +#endif + sig_slot = &SIGNAL_DEFINITIONS[i]; + } + } + +#ifdef SENTRY_PLATFORM_UNIX + // use a signal-safe allocator before we tear down. + sentry__page_allocator_enable(); +#endif + + const sentry_threadid_t current_thread = sentry__current_thread(); + if (g_handler_thread_ready + && sentry__threadid_equal(current_thread, g_handler_thread)) { + // This means our handler thread crashed, there is no safe way out: + // make an async-signal-safe log and defer to previous + static const char msg[] = "[sentry] FATAL crash in handler thread, " + "falling back to previous handler\n"; +#ifdef SENTRY_PLATFORM_UNIX + (void)write(STDERR_FILENO, msg, sizeof(msg) - 1); +#else + OutputDebugStringA(msg); + HANDLE stderr_handle = GetStdHandle(STD_ERROR_HANDLE); + if (stderr_handle && stderr_handle != INVALID_HANDLE_VALUE) { + DWORD written; + WriteFile(stderr_handle, msg, (DWORD)strlen(msg), &written, NULL); + } +#endif + } else { + // invoke the handler thread for signal unsafe actions + dispatch_ucontext(uctx, sig_slot, strategy); + } #ifdef SENTRY_PLATFORM_UNIX // reset signal handlers and invoke the original ones. This will then tear @@ -725,7 +1057,7 @@ handle_signal(int signum, siginfo_t *info, void *user_context) uctx.signum = signum; uctx.siginfo = info; uctx.user_context = (ucontext_t *)user_context; - handle_ucontext(&uctx); + process_ucontext(&uctx); } #elif defined SENTRY_PLATFORM_WINDOWS static LONG WINAPI @@ -740,7 +1072,7 @@ handle_exception(EXCEPTION_POINTERS *ExceptionInfo) sentry_ucontext_t uctx; memset(&uctx, 0, sizeof(uctx)); uctx.exception_ptrs = *ExceptionInfo; - handle_ucontext(&uctx); + process_ucontext(&uctx); return EXCEPTION_CONTINUE_SEARCH; } #endif @@ -748,7 +1080,7 @@ handle_exception(EXCEPTION_POINTERS *ExceptionInfo) static void handle_except(sentry_backend_t *UNUSED(backend), const sentry_ucontext_t *uctx) { - handle_ucontext(uctx); + process_ucontext(uctx); } sentry_backend_t * diff --git a/src/sentry_sync.c b/src/sentry_sync.c index 7766a6970..18d66c2a9 100644 --- a/src/sentry_sync.c +++ b/src/sentry_sync.c @@ -505,34 +505,117 @@ sentry__bgworker_get_thread_name(sentry_bgworker_t *bgw) #if defined(SENTRY_PLATFORM_UNIX) || defined(SENTRY_PLATFORM_NX) # include "sentry_cpu_relax.h" +# include -static sig_atomic_t g_in_signal_handler = 0; +static sig_atomic_t g_in_signal_handler __attribute__((aligned(64))) = 0; static sentry_threadid_t g_signal_handling_thread = { 0 }; +# ifdef SENTRY_BACKEND_INPROC +static sig_atomic_t g_signal_handler_can_lock __attribute__((aligned(64))) = 0; + +static void +fatal_signal_lock_violation(void) +{ + static const char msg[] + = "[sentry] FATAL attempted to acquire mutex inside signal handler\n"; + (void)write(STDERR_FILENO, msg, sizeof(msg) - 1); + _exit(1); +} +# endif + bool sentry__block_for_signal_handler(void) { - while (__sync_fetch_and_add(&g_in_signal_handler, 0)) { - if (sentry__threadid_equal( - sentry__current_thread(), g_signal_handling_thread)) { - return false; + for (;;) { + // if there is no signal handler active, we don't need to block + if (!__atomic_load_n(&g_in_signal_handler, __ATOMIC_RELAXED)) { + return true; } + + sentry_threadid_t current = sentry__current_thread(); + sentry_threadid_t handling + = __atomic_load_n(&g_signal_handling_thread, __ATOMIC_ACQUIRE); + + if (sentry__threadid_equal(current, handling)) { +# ifdef SENTRY_BACKEND_INPROC + if (!__atomic_load_n( + &g_signal_handler_can_lock, __ATOMIC_ACQUIRE)) { + fatal_signal_lock_violation(); + } +# endif + return true; + } + + // otherwise, spin sentry__cpu_relax(); } - return true; +} + +static bool +is_handling_thread(void) +{ + sentry_threadid_t handling + = __atomic_load_n(&g_signal_handling_thread, __ATOMIC_ACQUIRE); + return sentry__threadid_equal(handling, sentry__current_thread()); } void sentry__enter_signal_handler(void) { - sentry__block_for_signal_handler(); - g_signal_handling_thread = sentry__current_thread(); - __sync_fetch_and_or(&g_in_signal_handler, 1); + for (;;) { + // entering a signal handler while another runs, should block us + while (__atomic_load_n(&g_in_signal_handler, __ATOMIC_RELAXED)) { + // however, if we re-enter most likely a signal was raised from + // within the handler and then we should proceed. + // TODO: maybe pass in the signum and check for SIGABRT loops here + if (is_handling_thread()) { + return; + } + } + + // RMW that both tests AND sets atomically so we know we won the race + if (__sync_lock_test_and_set(&g_in_signal_handler, 1) == 0) { + sentry_threadid_t current = sentry__current_thread(); + // update the thread, now that no one else can and leave + __atomic_store_n( + &g_signal_handling_thread, current, __ATOMIC_RELEASE); +# ifdef SENTRY_BACKEND_INPROC + __atomic_store_n(&g_signal_handler_can_lock, 0, __ATOMIC_RELEASE); +# endif + return; + } + + // otherwise, spin + } +} + +bool +sentry__switch_handler_thread(void) +{ + if (!__atomic_load_n(&g_in_signal_handler, __ATOMIC_ACQUIRE)) { + return false; + } + + sentry_threadid_t current = sentry__current_thread(); + __atomic_store_n(&g_signal_handling_thread, current, __ATOMIC_RELEASE); + // TODO: this is still insufficient as a safe-guard when crashing in the + // handler thread +# ifdef SENTRY_BACKEND_INPROC + __atomic_store_n(&g_signal_handler_can_lock, 1, __ATOMIC_RELEASE); +# endif + + return true; } void sentry__leave_signal_handler(void) { - __sync_fetch_and_and(&g_in_signal_handler, 0); + // clean up the thread-id and drop the reentrancy guard + __atomic_store_n( + &g_signal_handling_thread, (sentry_threadid_t) { 0 }, __ATOMIC_RELAXED); +# ifdef SENTRY_BACKEND_INPROC + __atomic_store_n(&g_signal_handler_can_lock, 0, __ATOMIC_RELAXED); +# endif + __sync_lock_release(&g_in_signal_handler); } #endif diff --git a/src/sentry_sync.h b/src/sentry_sync.h index 5516443b9..8761cd6ab 100644 --- a/src/sentry_sync.h +++ b/src/sentry_sync.h @@ -227,6 +227,7 @@ typedef CONDITION_VARIABLE sentry_cond_t; bool sentry__block_for_signal_handler(void); void sentry__enter_signal_handler(void); void sentry__leave_signal_handler(void); +bool sentry__switch_handler_thread(void); typedef pthread_t sentry_threadid_t; typedef pthread_mutex_t sentry_mutex_t; diff --git a/src/unwinder/sentry_unwinder.c b/src/unwinder/sentry_unwinder.c index 19affe8fe..09664d015 100644 --- a/src/unwinder/sentry_unwinder.c +++ b/src/unwinder/sentry_unwinder.c @@ -16,6 +16,7 @@ DEFINE_UNWINDER(libunwindstack); DEFINE_UNWINDER(libbacktrace); DEFINE_UNWINDER(dbghelp); DEFINE_UNWINDER(libunwind); +DEFINE_UNWINDER(libunwind_mac); DEFINE_UNWINDER(psunwind); static size_t @@ -34,6 +35,9 @@ unwind_stack( #ifdef SENTRY_WITH_UNWINDER_LIBUNWIND TRY_UNWINDER(libunwind); #endif +#ifdef SENTRY_WITH_UNWINDER_LIBUNWIND_MAC + TRY_UNWINDER(libunwind_mac); +#endif #ifdef SENTRY_WITH_UNWINDER_PS TRY_UNWINDER(psunwind); #endif diff --git a/src/unwinder/sentry_unwinder_libunwind_mac.c b/src/unwinder/sentry_unwinder_libunwind_mac.c new file mode 100644 index 000000000..176fb287b --- /dev/null +++ b/src/unwinder/sentry_unwinder_libunwind_mac.c @@ -0,0 +1,92 @@ +#include "sentry_boot.h" +#include "sentry_logger.h" +#include + +// a very cheap pointer validation for starters +static bool +valid_ptr(uintptr_t p) +{ + return p && (p % sizeof(uintptr_t) == 0); +} + +size_t +sentry__unwind_stack_libunwind_mac( + void *addr, const sentry_ucontext_t *uctx, void **ptrs, size_t max_frames) +{ + if (addr) { + return 0; + } + size_t frame_idx = 0; + + if (uctx) { + // TODO: this is a working ARM64 from ucontext unwinder using FP that + // doesn't have the issues we see backtrace() (which isn't even + // signal-safe) but also the system provided libunwind() on macOS + // - clean this up + // - implement an x86_64 ucontext unwinder + + struct __darwin_mcontext64 *mctx = uctx->user_context->uc_mcontext; + uintptr_t pc = (uintptr_t)mctx->__ss.__pc; + uintptr_t fp = (uintptr_t)mctx->__ss.__fp; + uintptr_t lr = (uintptr_t)mctx->__ss.__lr; + + // top frame: adjust pc−1 so it symbolizes inside the function + if (pc) { + ptrs[frame_idx++] = (void *)(pc - 1); + } + + // next frame is from saved LR at current FP record + if (lr) { + ptrs[frame_idx++] = (void *)(lr - 1); + } + + for (size_t i = 0; i < max_frames; ++i) { + if (!valid_ptr(fp)) { + break; + } + + // arm64 frame record layout: [prev_fp, saved_lr] at fp and fp+8 + uintptr_t *record = (uintptr_t *)fp; + uintptr_t next_fp = record[0]; + uintptr_t retaddr = record[1]; + if (!valid_ptr(next_fp) || !retaddr) { + break; + } + + ptrs[frame_idx++] = (void *)(retaddr - 1); + if (next_fp <= fp) { + break; // prevent loops + } + fp = next_fp; + } + } else { + unw_context_t uc; + int ret = unw_getcontext(&uc); + if (ret != 0) { + SENTRY_WARN("Failed to retrieve context with libunwind"); + return 0; + } + + unw_cursor_t cursor; + ret = unw_init_local(&cursor, &uc); + if (ret != 0) { + SENTRY_WARN("Failed to initialize libunwind with local context"); + return 0; + } + while (unw_step(&cursor) > 0 && frame_idx < max_frames - 1) { + unw_word_t ip = 0; + SENTRY_INFOF("ip: %p", ip); + unw_get_reg(&cursor, UNW_REG_IP, &ip); +#if defined(__arm64__) + // Strip pointer authentication, for some reason ptrauth_strip() not + // working + // https://developer.apple.com/documentation/security/preparing_your_app_to_work_with_pointer_authentication + ip &= 0x7fffffffffffull; +#endif + ptrs[frame_idx] = (void *)ip; + frame_idx++; + } + } + + return frame_idx + 1; +} diff --git a/tests/assertions.py b/tests/assertions.py index 05d3ea320..62e5a2cb0 100644 --- a/tests/assertions.py +++ b/tests/assertions.py @@ -170,6 +170,16 @@ def assert_event_meta( ) +def is_valid_hex(s): + if not s.lower().startswith("0x"): + return False + try: + int(s, 0) + return True + except ValueError: + return False + + def assert_stacktrace( envelope, inside_exception=False, check_size=True, check_package=False ): @@ -181,7 +191,7 @@ def assert_stacktrace( if check_size: assert len(frames) > 0 - assert all(frame["instruction_addr"].startswith("0x") for frame in frames) + assert all(is_valid_hex(frame["instruction_addr"]) for frame in frames) assert any( frame.get("function") is not None and frame.get("package") is not None for frame in frames From 7eb3ca859a9e114568f6833e0ef09df2f30b3a53 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Thu, 6 Nov 2025 14:15:25 +0100 Subject: [PATCH 02/75] prevent warning on write() return value --- src/sentry_sync.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/sentry_sync.c b/src/sentry_sync.c index 18d66c2a9..1af5979b7 100644 --- a/src/sentry_sync.c +++ b/src/sentry_sync.c @@ -518,7 +518,8 @@ fatal_signal_lock_violation(void) { static const char msg[] = "[sentry] FATAL attempted to acquire mutex inside signal handler\n"; - (void)write(STDERR_FILENO, msg, sizeof(msg) - 1); + const ssize_t rv = write(STDERR_FILENO, msg, sizeof(msg) - 1); + (void)rv; _exit(1); } # endif From 0cff127e525b422a04bf8eadc7dd80d999dfc0ff Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Thu, 6 Nov 2025 14:16:17 +0100 Subject: [PATCH 03/75] get rid of unsused dispatch parameter --- src/backends/sentry_backend_inproc.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/backends/sentry_backend_inproc.c b/src/backends/sentry_backend_inproc.c index 89ef37628..54681486a 100644 --- a/src/backends/sentry_backend_inproc.c +++ b/src/backends/sentry_backend_inproc.c @@ -35,7 +35,6 @@ typedef struct sentry_inproc_handler_state_s { ucontext_t user_context_storage; #endif const struct signal_slot *sig_slot; - sentry_handler_strategy_t strategy; } sentry_inproc_handler_state_t; // "data" struct containing options to prevent mutex access in signal handler @@ -858,8 +857,8 @@ stop_handler_thread(void) } static void -dispatch_ucontext(const sentry_ucontext_t *uctx, - const struct signal_slot *sig_slot, sentry_handler_strategy_t strategy) +dispatch_ucontext( + const sentry_ucontext_t *uctx, const struct signal_slot *sig_slot) { if (!sentry__atomic_fetch(&g_handler_thread_ready)) { process_ucontext_deferred(uctx, sig_slot); @@ -1032,7 +1031,7 @@ process_ucontext(const sentry_ucontext_t *uctx) #endif } else { // invoke the handler thread for signal unsafe actions - dispatch_ucontext(uctx, sig_slot, strategy); + dispatch_ucontext(uctx, sig_slot); } #ifdef SENTRY_PLATFORM_UNIX From 12650070f7f614c72039cb8952fbab48fad3e8c7 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Thu, 6 Nov 2025 14:18:59 +0100 Subject: [PATCH 04/75] and another unused return value for write() --- src/backends/sentry_backend_inproc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/backends/sentry_backend_inproc.c b/src/backends/sentry_backend_inproc.c index 54681486a..25fb3d03b 100644 --- a/src/backends/sentry_backend_inproc.c +++ b/src/backends/sentry_backend_inproc.c @@ -1020,7 +1020,8 @@ process_ucontext(const sentry_ucontext_t *uctx) static const char msg[] = "[sentry] FATAL crash in handler thread, " "falling back to previous handler\n"; #ifdef SENTRY_PLATFORM_UNIX - (void)write(STDERR_FILENO, msg, sizeof(msg) - 1); + const ssize_t rv = write(STDERR_FILENO, msg, sizeof(msg) - 1); + (void)rv; #else OutputDebugStringA(msg); HANDLE stderr_handle = GetStdHandle(STD_ERROR_HANDLE); From 5a11ea24c59a0d92a2e650659348736e5de66171 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Fri, 7 Nov 2025 11:54:21 +0100 Subject: [PATCH 05/75] make tests a bit less brittle --- tests/assertions.py | 8 ++++---- tests/test_dotnet_signals.py | 6 ++++++ tests/unit/test_process.c | 17 +++++++++++++++-- 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/tests/assertions.py b/tests/assertions.py index 62e5a2cb0..d40311c18 100644 --- a/tests/assertions.py +++ b/tests/assertions.py @@ -127,11 +127,11 @@ def assert_event_meta( match = VERSION_RE.match(version) version = match.group(1) build = match.group(2) + expected_os_context = {"name": "Linux", "version": version} + if build: + expected_os_context["build"] = build - assert_matches( - event["contexts"]["os"], - {"name": "Linux", "version": version, "build": build}, - ) + assert_matches(event["contexts"]["os"], expected_os_context) assert "distribution_name" in event["contexts"]["os"] assert "distribution_version" in event["contexts"]["os"] elif sys.platform == "darwin": diff --git a/tests/test_dotnet_signals.py b/tests/test_dotnet_signals.py index 9d7fe07c9..7c4e2a70d 100644 --- a/tests/test_dotnet_signals.py +++ b/tests/test_dotnet_signals.py @@ -65,6 +65,9 @@ def run_dotnet_native_crash(tmp_path): reason="dotnet signal handling is currently only supported on 64-bit Linux without sanitizers", ) def test_dotnet_signals_inproc(cmake): + if shutil.which("dotnet") is None: + pytest.skip("dotnet is not installed") + try: # build native client library with inproc and the example for crash dumping tmp_path = cmake( @@ -166,6 +169,9 @@ def run_aot_native_crash(tmp_path): reason="dotnet AOT signal handling is currently only supported on 64-bit Linux without sanitizers", ) def test_aot_signals_inproc(cmake): + if shutil.which("dotnet") is None: + pytest.skip("dotnet is not installed") + try: # build native client library with inproc and the example for crash dumping tmp_path = cmake( diff --git a/tests/unit/test_process.c b/tests/unit/test_process.c index baeff7b20..71d6565b5 100644 --- a/tests/unit/test_process.c +++ b/tests/unit/test_process.c @@ -24,6 +24,17 @@ SENTRY_TEST(process_invalid) sentry__path_free(nul); } +void find_cp_path(char* buf, size_t buf_len) +{ + FILE *fp = popen("command -v cp 2>/dev/null", "r"); + if (fp && fgets(buf, buf_len, fp)) { + buf[strcspn(buf, "\n")] = 0; // strip newline + } + if (fp) { + pclose(fp); + } +} + SENTRY_TEST(process_spawn) { #if defined(SENTRY_PLATFORM_WINDOWS) || defined(SENTRY_PLATFORM_MACOS) \ @@ -46,8 +57,10 @@ SENTRY_TEST(process_spawn) sentry__process_spawn(cmd, "/C", "copy", exe->path, dst->path, NULL); sentry__path_free(cmd); # else - // /bin/cp - sentry_path_t *cp = sentry__path_from_str("/bin/cp"); + char cp_path[512] = "/bin/cp"; + find_cp_path(cp_path, sizeof(cp_path)); + // cp + sentry_path_t *cp = sentry__path_from_str(cp_path); TEST_ASSERT(!!cp); sentry__process_spawn(cp, exe->path, dst->path, NULL); sentry__path_free(cp); From b83fc370773ae14f78c18cfe78567f886ae1b011 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Fri, 7 Nov 2025 11:27:53 +0100 Subject: [PATCH 06/75] ensure that find_cp_path isn't built on windows. --- tests/unit/test_process.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_process.c b/tests/unit/test_process.c index 71d6565b5..8fed7546e 100644 --- a/tests/unit/test_process.c +++ b/tests/unit/test_process.c @@ -24,7 +24,9 @@ SENTRY_TEST(process_invalid) sentry__path_free(nul); } -void find_cp_path(char* buf, size_t buf_len) +#ifndef SENTRY_PLATFORM_WINDOWS +void +find_cp_path(char *buf, size_t buf_len) { FILE *fp = popen("command -v cp 2>/dev/null", "r"); if (fp && fgets(buf, buf_len, fp)) { @@ -34,6 +36,7 @@ void find_cp_path(char* buf, size_t buf_len) pclose(fp); } } +#endif SENTRY_TEST(process_spawn) { From b57995ca3637fcc10440eba6d8693212c01bbfed Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Fri, 7 Nov 2025 11:45:10 +0100 Subject: [PATCH 07/75] fix trivial windows compile def issues to run tests locally --- src/backends/sentry_backend_inproc.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/backends/sentry_backend_inproc.c b/src/backends/sentry_backend_inproc.c index 25fb3d03b..d12294ad1 100644 --- a/src/backends/sentry_backend_inproc.c +++ b/src/backends/sentry_backend_inproc.c @@ -66,6 +66,9 @@ static int g_handler_pipe[2] = { -1, -1 }; static HANDLE g_handler_semaphore = NULL; #endif +static int start_handler_thread(void); +static void stop_handler_thread(void); + #ifdef SENTRY_PLATFORM_UNIX # include struct signal_slot { @@ -90,8 +93,6 @@ static const struct signal_slot SIGNAL_DEFINITIONS[SIGNAL_COUNT] = { }; static void handle_signal(int signum, siginfo_t *info, void *user_context); -static int start_handler_thread(void); -static void stop_handler_thread(void); static void reset_signal_handlers(void) @@ -237,9 +238,7 @@ startup_inproc_backend( { g_backend_config.enable_logging_when_crashed = options ? options->enable_logging_when_crashed : true; - g_backend_config.handler_strategy = options - ? sentry_options_get_handler_strategy(options) - : SENTRY_HANDLER_STRATEGY_DEFAULT; + g_backend_config.handler_strategy = SENTRY_HANDLER_STRATEGY_DEFAULT; if (backend) { backend->data = &g_backend_config; } @@ -948,9 +947,9 @@ process_ucontext(const sentry_ucontext_t *uctx) sentry__logger_disable(); } - sentry_handler_strategy_t strategy = g_backend_config.handler_strategy; #ifdef SENTRY_PLATFORM_LINUX + sentry_handler_strategy_t strategy = g_backend_config.handler_strategy; if (strategy == SENTRY_HANDLER_STRATEGY_CHAIN_AT_START) { // On Linux (and thus Android) CLR/Mono converts signals provoked by // AOT/JIT-generated native code into managed code exceptions. In these From ea527caa8cf9d6cc5486ab7822f20ee6f96722f4 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Fri, 7 Nov 2025 14:06:52 +0100 Subject: [PATCH 08/75] clean up backends * use `std::nothrow` `new` consistently to keep exception-free semantics for allocation * rename static crashpad_handler to have no module-public prefix * use `nullptr` for arguments where we previously used 0 to clarify that those are pointers * eliminate the `memset()` of the `crashpad_state_t` initialization since it now contains non-trivially constructable fields (`std::atomic`) and replace it with `new` and an empty value initializer. --- src/backends/sentry_backend_breakpad.cpp | 8 ++++---- src/backends/sentry_backend_crashpad.cpp | 22 ++++++++++------------ src/backends/sentry_backend_inproc.c | 1 - 3 files changed, 14 insertions(+), 17 deletions(-) diff --git a/src/backends/sentry_backend_breakpad.cpp b/src/backends/sentry_backend_breakpad.cpp index 1c737ce55..a312e69c9 100644 --- a/src/backends/sentry_backend_breakpad.cpp +++ b/src/backends/sentry_backend_breakpad.cpp @@ -261,22 +261,22 @@ breakpad_backend_startup( && defined(SENTRY_THREAD_STACK_GUARANTEE_AUTO_INIT) sentry__set_default_thread_stack_guarantee(); # endif - backend->data = new google_breakpad::ExceptionHandler( + backend->data = new (std::nothrow) google_breakpad::ExceptionHandler( current_run_folder->path_w, nullptr, breakpad_backend_callback, nullptr, google_breakpad::ExceptionHandler::HANDLER_EXCEPTION); #elif defined(SENTRY_PLATFORM_MACOS) // If process is being debugged and there are breakpoints set it will cause // task_set_exception_ports to crash the whole process and debugger - backend->data = new google_breakpad::ExceptionHandler( + backend->data = new (std::nothrow) google_breakpad::ExceptionHandler( current_run_folder->path, nullptr, breakpad_backend_callback, nullptr, !IsDebuggerActive(), nullptr); #elif defined(SENTRY_PLATFORM_IOS) backend->data - = new google_breakpad::ExceptionHandler(current_run_folder->path, + = new (std::nothrow) google_breakpad::ExceptionHandler(current_run_folder->path, nullptr, breakpad_backend_callback, nullptr, true, nullptr); #else google_breakpad::MinidumpDescriptor descriptor(current_run_folder->path); - backend->data = new google_breakpad::ExceptionHandler( + backend->data = new (std::nothrow) google_breakpad::ExceptionHandler( descriptor, nullptr, breakpad_backend_callback, nullptr, true, -1); #endif return backend->data == nullptr; diff --git a/src/backends/sentry_backend_crashpad.cpp b/src/backends/sentry_backend_crashpad.cpp index 181e3d6c4..9570170c7 100644 --- a/src/backends/sentry_backend_crashpad.cpp +++ b/src/backends/sentry_backend_crashpad.cpp @@ -310,11 +310,11 @@ flush_scope_from_handler( # ifdef SENTRY_PLATFORM_WINDOWS static bool -sentry__crashpad_handler(EXCEPTION_POINTERS *ExceptionInfo) +crashpad_handler(EXCEPTION_POINTERS *ExceptionInfo) { # else static bool -sentry__crashpad_handler(int signum, siginfo_t *info, ucontext_t *user_context) +crashpad_handler(int signum, siginfo_t *info, ucontext_t *user_context) { sentry__page_allocator_enable(); sentry__enter_signal_handler(); @@ -540,7 +540,7 @@ crashpad_backend_startup( // Initialize database first, flushing the consent later on as part of // `sentry_init` will persist the upload flag. data->db = crashpad::CrashReportDatabase::Initialize(database).release(); - data->client = new crashpad::CrashpadClient; + data->client = new (std::nothrow) crashpad::CrashpadClient; char *minidump_url = sentry__dsn_get_minidump_url(options->dsn, options->user_agent); if (minidump_url) { @@ -581,8 +581,7 @@ crashpad_backend_startup( } #if defined(SENTRY_PLATFORM_LINUX) || defined(SENTRY_PLATFORM_WINDOWS) - crashpad::CrashpadClient::SetFirstChanceExceptionHandler( - &sentry__crashpad_handler); + crashpad::CrashpadClient::SetFirstChanceExceptionHandler(&crashpad_handler); #endif #ifdef SENTRY_PLATFORM_LINUX // Crashpad was recently changed to register its own signal stack, which for @@ -593,7 +592,7 @@ crashpad_backend_startup( if (g_signal_stack.ss_sp) { g_signal_stack.ss_size = SIGNAL_STACK_SIZE; g_signal_stack.ss_flags = 0; - sigaltstack(&g_signal_stack, 0); + sigaltstack(&g_signal_stack, nullptr); } #endif @@ -632,9 +631,9 @@ crashpad_backend_shutdown(sentry_backend_t *backend) #ifdef SENTRY_PLATFORM_LINUX g_signal_stack.ss_flags = SS_DISABLE; - sigaltstack(&g_signal_stack, 0); + sigaltstack(&g_signal_stack, nullptr); sentry_free(g_signal_stack.ss_sp); - g_signal_stack.ss_sp = NULL; + g_signal_stack.ss_sp = nullptr; #endif } @@ -745,8 +744,8 @@ crashpad_backend_prune_database(sentry_backend_t *backend) // large. data->db->CleanDatabase(60 * 60 * 24 * 2); crashpad::BinaryPruneCondition condition(crashpad::BinaryPruneCondition::OR, - new crashpad::DatabaseSizePruneCondition(1024 * 8), - new crashpad::AgePruneCondition(2)); + new (std::nothrow) crashpad::DatabaseSizePruneCondition(1024 * 8), + new (std::nothrow) crashpad::AgePruneCondition(2)); crashpad::PruneCrashReportDatabase(data->db, &condition); } @@ -825,12 +824,11 @@ sentry__backend_new(void) } memset(backend, 0, sizeof(sentry_backend_t)); - auto *data = SENTRY_MAKE(crashpad_state_t); + auto *data = new (std::nothrow) crashpad_state_t {}; if (!data) { sentry_free(backend); return nullptr; } - memset(data, 0, sizeof(crashpad_state_t)); data->scope_flush = false; data->crashed = false; diff --git a/src/backends/sentry_backend_inproc.c b/src/backends/sentry_backend_inproc.c index d12294ad1..3f9ced4f4 100644 --- a/src/backends/sentry_backend_inproc.c +++ b/src/backends/sentry_backend_inproc.c @@ -947,7 +947,6 @@ process_ucontext(const sentry_ucontext_t *uctx) sentry__logger_disable(); } - #ifdef SENTRY_PLATFORM_LINUX sentry_handler_strategy_t strategy = g_backend_config.handler_strategy; if (strategy == SENTRY_HANDLER_STRATEGY_CHAIN_AT_START) { From c78f4250dc7b21632432240be793622846b52269 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Mon, 10 Nov 2025 09:12:19 +0100 Subject: [PATCH 09/75] eliminate local handler strategy declaration in signal handler --- src/backends/sentry_backend_inproc.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/backends/sentry_backend_inproc.c b/src/backends/sentry_backend_inproc.c index 3f9ced4f4..db911a151 100644 --- a/src/backends/sentry_backend_inproc.c +++ b/src/backends/sentry_backend_inproc.c @@ -948,8 +948,8 @@ process_ucontext(const sentry_ucontext_t *uctx) } #ifdef SENTRY_PLATFORM_LINUX - sentry_handler_strategy_t strategy = g_backend_config.handler_strategy; - if (strategy == SENTRY_HANDLER_STRATEGY_CHAIN_AT_START) { + if (g_backend_config.handler_strategy + == SENTRY_HANDLER_STRATEGY_CHAIN_AT_START) { // On Linux (and thus Android) CLR/Mono converts signals provoked by // AOT/JIT-generated native code into managed code exceptions. In these // cases, we shouldn't react to the signal at all and let their handler @@ -1040,7 +1040,8 @@ process_ucontext(const sentry_ucontext_t *uctx) // forward as we're not restoring the page allocator. reset_signal_handlers(); sentry__leave_signal_handler(); - if (strategy != SENTRY_HANDLER_STRATEGY_CHAIN_AT_START) { + if (g_backend_config.handler_strategy + != SENTRY_HANDLER_STRATEGY_CHAIN_AT_START) { invoke_signal_handler( uctx->signum, uctx->siginfo, (void *)uctx->user_context); } From 116c4a52463e0f745a933fafef9e44230ee9589e Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Mon, 10 Nov 2025 09:52:59 +0100 Subject: [PATCH 10/75] turn off painful warning noise --- CMakeLists.txt | 9 +++++++++ src/backends/sentry_backend_crashpad.cpp | 2 -- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index b7639b7e5..236b11f62 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -679,6 +679,15 @@ if(SENTRY_BACKEND_CRASHPAD) $ $ ) + + # ignore #include_next warning being a GCC extension via pedantic + if (LINUX) + target_include_directories(sentry + SYSTEM PRIVATE + external/crashpad/compat/linux + ) + endif() + install(EXPORT crashpad_export NAMESPACE sentry_crashpad:: FILE sentry_crashpad-targets.cmake DESTINATION "${CMAKE_INSTALL_CMAKEDIR}" ) diff --git a/src/backends/sentry_backend_crashpad.cpp b/src/backends/sentry_backend_crashpad.cpp index 9570170c7..17ca48fa3 100644 --- a/src/backends/sentry_backend_crashpad.cpp +++ b/src/backends/sentry_backend_crashpad.cpp @@ -32,8 +32,6 @@ extern "C" { #if defined(__GNUC__) # pragma GCC diagnostic push # pragma GCC diagnostic ignored "-Wunused-parameter" -# pragma GCC diagnostic ignored "-Wgnu-zero-variadic-macro-arguments" -# pragma GCC diagnostic ignored "-Wfour-char-constants" #elif defined(_MSC_VER) # pragma warning(push) # pragma warning(disable : 4100) // unreferenced formal parameter From b4747c7b0ee739f5681b4a0bdb8e394943da778c Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Mon, 10 Nov 2025 10:13:40 +0100 Subject: [PATCH 11/75] add libunwind to the CI dependencies --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b2af65145..140384c1f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -200,7 +200,7 @@ jobs: if: ${{ runner.os == 'Linux' && matrix.os != 'ubuntu-22.04' && !env['TEST_X86'] && !matrix.container }} run: | sudo apt update - sudo apt install cmake clang-19 llvm g++-12 valgrind zlib1g-dev libcurl4-openssl-dev + sudo apt install cmake clang-19 llvm g++-12 valgrind zlib1g-dev libcurl4-openssl-dev libunwind-dev # Install kcov from source sudo apt-get install binutils-dev libssl-dev libelf-dev libstdc++-12-dev libdw-dev libiberty-dev git clone https://github.com/SimonKagstrom/kcov.git From d21c180f05c0ab2d484207c8c70018fbac7e3cf1 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Mon, 10 Nov 2025 10:19:56 +0100 Subject: [PATCH 12/75] ensure we build the example with PIE disabled when doing a static build, since libraries like libunwind.a might be packaged without PIC. --- CMakeLists.txt | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 236b11f62..41f46f9d5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -816,6 +816,14 @@ if(SENTRY_BUILD_EXAMPLES) # Disable all optimizations for the `sentry_example` in gcc/clang. This allows us to keep crash triggers simple. # The effects besides reproducible code-gen across compiler versions, will be negligible for build- and runtime. target_compile_options(sentry_example PRIVATE $) + + # Most Linux distros will build executables with PIE. However, if we build a static library, not all static + # dependencies being packaged with the same distro, are built with PIC enabled. This will lead `ld` to error on + # relocation data, thus failing the build/tests. There is no need to build the example with PIE enabled. + if (LINUX AND NOT SENTRY_BUILD_SHARED_LIBS) + target_link_options(sentry_example PRIVATE -no-pie) + target_compile_options(sentry_example PRIVATE -fno-pie) + endif() endif() # set static runtime if enabled From b0822d3f4476b93af51691ddeef2324bcdd1d2ae Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Mon, 10 Nov 2025 10:53:29 +0100 Subject: [PATCH 13/75] extend handler crash to windows --- src/backends/sentry_backend_breakpad.cpp | 12 +++---- src/backends/sentry_backend_inproc.c | 46 ++++++++++++++---------- src/sentry_sync.h | 12 +++++++ 3 files changed, 45 insertions(+), 25 deletions(-) diff --git a/src/backends/sentry_backend_breakpad.cpp b/src/backends/sentry_backend_breakpad.cpp index a312e69c9..528a3c00a 100644 --- a/src/backends/sentry_backend_breakpad.cpp +++ b/src/backends/sentry_backend_breakpad.cpp @@ -267,13 +267,13 @@ breakpad_backend_startup( #elif defined(SENTRY_PLATFORM_MACOS) // If process is being debugged and there are breakpoints set it will cause // task_set_exception_ports to crash the whole process and debugger - backend->data = new (std::nothrow) google_breakpad::ExceptionHandler( - current_run_folder->path, nullptr, breakpad_backend_callback, nullptr, - !IsDebuggerActive(), nullptr); + backend->data = new (std::nothrow) + google_breakpad::ExceptionHandler(current_run_folder->path, nullptr, + breakpad_backend_callback, nullptr, !IsDebuggerActive(), nullptr); #elif defined(SENTRY_PLATFORM_IOS) - backend->data - = new (std::nothrow) google_breakpad::ExceptionHandler(current_run_folder->path, - nullptr, breakpad_backend_callback, nullptr, true, nullptr); + backend->data = new (std::nothrow) + google_breakpad::ExceptionHandler(current_run_folder->path, nullptr, + breakpad_backend_callback, nullptr, true, nullptr); #else google_breakpad::MinidumpDescriptor descriptor(current_run_folder->path); backend->data = new (std::nothrow) google_breakpad::ExceptionHandler( diff --git a/src/backends/sentry_backend_inproc.c b/src/backends/sentry_backend_inproc.c index db911a151..270c65aa8 100644 --- a/src/backends/sentry_backend_inproc.c +++ b/src/backends/sentry_backend_inproc.c @@ -855,6 +855,32 @@ stop_handler_thread(void) sentry__atomic_store(&g_handler_should_exit, 0); } +static bool +did_handler_thread_crash(void) +{ + const sentry_threadid_t current_thread = sentry__current_thread(); + if (g_handler_thread_ready + && sentry__threadid_equal(current_thread, g_handler_thread)) { + // This means our handler thread crashed, there is no safe way out: + // fall back to previous handler + static const char msg[] = "[sentry] FATAL crash in handler thread, " + "falling back to previous handler\n"; +#ifdef SENTRY_PLATFORM_UNIX + const ssize_t rv = write(STDERR_FILENO, msg, sizeof(msg) - 1); + (void)rv; +#else + OutputDebugStringA(msg); + HANDLE stderr_handle = GetStdHandle(STD_ERROR_HANDLE); + if (stderr_handle && stderr_handle != INVALID_HANDLE_VALUE) { + DWORD written; + WriteFile(stderr_handle, msg, (DWORD)strlen(msg), &written, NULL); + } +#endif + return true; + } + return false; +} + static void dispatch_ucontext( const sentry_ucontext_t *uctx, const struct signal_slot *sig_slot) @@ -1010,25 +1036,7 @@ process_ucontext(const sentry_ucontext_t *uctx) sentry__page_allocator_enable(); #endif - const sentry_threadid_t current_thread = sentry__current_thread(); - if (g_handler_thread_ready - && sentry__threadid_equal(current_thread, g_handler_thread)) { - // This means our handler thread crashed, there is no safe way out: - // make an async-signal-safe log and defer to previous - static const char msg[] = "[sentry] FATAL crash in handler thread, " - "falling back to previous handler\n"; -#ifdef SENTRY_PLATFORM_UNIX - const ssize_t rv = write(STDERR_FILENO, msg, sizeof(msg) - 1); - (void)rv; -#else - OutputDebugStringA(msg); - HANDLE stderr_handle = GetStdHandle(STD_ERROR_HANDLE); - if (stderr_handle && stderr_handle != INVALID_HANDLE_VALUE) { - DWORD written; - WriteFile(stderr_handle, msg, (DWORD)strlen(msg), &written, NULL); - } -#endif - } else { + if (!did_handler_thread_crash()) { // invoke the handler thread for signal unsafe actions dispatch_ucontext(uctx, sig_slot); } diff --git a/src/sentry_sync.h b/src/sentry_sync.h index 8761cd6ab..6157e5fb8 100644 --- a/src/sentry_sync.h +++ b/src/sentry_sync.h @@ -207,6 +207,18 @@ typedef CONDITION_VARIABLE sentry_cond_t; # define sentry__cond_wait(CondVar, Lock) \ sentry__cond_wait_timeout(CondVar, Lock, INFINITE) +static inline sentry_threadid_t +sentry__current_thread(void) +{ + return GetCurrentThread(); +} + +static inline int +sentry__threadid_equal(sentry_threadid_t a, sentry_threadid_t b) +{ + return GetThreadId(a) == GetThreadId(b); +} + #else # include # include From 8b93ccac3764881ccef6f9b241ddc0f0b167620e Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Mon, 10 Nov 2025 11:04:36 +0100 Subject: [PATCH 14/75] further clean of inproc --- src/backends/sentry_backend_inproc.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/backends/sentry_backend_inproc.c b/src/backends/sentry_backend_inproc.c index 270c65aa8..63aab60be 100644 --- a/src/backends/sentry_backend_inproc.c +++ b/src/backends/sentry_backend_inproc.c @@ -647,9 +647,9 @@ make_signal_event(const struct signal_slot *sig_slot, * requires stdio, time-formatting/-capture or serialization must happen here. * * Although we can use signal-unsafe functions here, this should still be - * written with care. Don't overly rely on thread synchronization since the - * program is in a crashed state. At least one thread no longer progresses and - * memory can be corrupted. + * written with care. Don't rely on thread synchronization or the system + * allocator since the program is in a crashed state. At least one thread no + * longer progresses and memory can be corrupted. */ static void process_ucontext_deferred( @@ -861,14 +861,15 @@ did_handler_thread_crash(void) const sentry_threadid_t current_thread = sentry__current_thread(); if (g_handler_thread_ready && sentry__threadid_equal(current_thread, g_handler_thread)) { - // This means our handler thread crashed, there is no safe way out: - // fall back to previous handler - static const char msg[] = "[sentry] FATAL crash in handler thread, " - "falling back to previous handler\n"; #ifdef SENTRY_PLATFORM_UNIX + static const char msg[] + = "[sentry] FATAL crash in handler thread, " + "falling back to previous handler\n"; const ssize_t rv = write(STDERR_FILENO, msg, sizeof(msg) - 1); (void)rv; #else + static const char msg[] = "[sentry] FATAL crash in handler thread, " + "UEF continues search\n"; OutputDebugStringA(msg); HANDLE stderr_handle = GetStdHandle(STD_ERROR_HANDLE); if (stderr_handle && stderr_handle != INVALID_HANDLE_VALUE) { @@ -944,12 +945,14 @@ dispatch_ucontext( * This is the signal-safe part of the inproc handler. Everything in here should * not defer to more than the set of functions listed in: * https://www.man7.org/linux/man-pages/man7/signal-safety.7.html + * Since this function runs as an UnhandledExceptionFilter on Windows, the rules + * are less strict, but similar in nature. * * That means: * - no heap allocations except for sentry_malloc() (page allocator enabled!!!) * - no stdio or any kind of libc string formatting * - no logging (at least not with the printf-based default logger) - * - no pthread synchronization (SENTRY_WITH_OPTIONS will terminate with a log) + * - no thread synchronization (SENTRY_WITH_OPTIONS will terminate with a log) * - in particular, don't access sentry interfaces that could request * access to options or the scope, those should go to the handler thread * - sentry_value_* and sentry_malloc are generally fine, because we use a safe @@ -1076,8 +1079,7 @@ handle_exception(EXCEPTION_POINTERS *ExceptionInfo) return EXCEPTION_CONTINUE_SEARCH; } - sentry_ucontext_t uctx; - memset(&uctx, 0, sizeof(uctx)); + sentry_ucontext_t uctx = { 0 }; uctx.exception_ptrs = *ExceptionInfo; process_ucontext(&uctx); return EXCEPTION_CONTINUE_SEARCH; From c4f662a191d62ca19c47577e11ff23864f8ea682 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Mon, 10 Nov 2025 11:19:54 +0100 Subject: [PATCH 15/75] review and fix/remove remaining TODOs from branch changes --- src/backends/sentry_backend_inproc.c | 15 +++++++-------- src/sentry_sync.c | 5 +---- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/src/backends/sentry_backend_inproc.c b/src/backends/sentry_backend_inproc.c index 63aab60be..849956177 100644 --- a/src/backends/sentry_backend_inproc.c +++ b/src/backends/sentry_backend_inproc.c @@ -593,11 +593,11 @@ make_signal_event(const struct signal_slot *sig_slot, if (sig_slot) { sentry_value_set_by_key( signal_meta, "name", sentry_value_new_string(sig_slot->signame)); - // at least on windows, the signum is a true u32 which we can't - // otherwise represent. - // TODO: does that still match reality? - sentry_value_set_by_key(signal_meta, "number", - sentry_value_new_double((double)sig_slot->signum)); + // relay interprets the signal number as an i64: + // https://github.com/getsentry/relay/blob/e96e4b037cfddaa7b0fb97a0909d100dde034f8e/relay-event-schema/src/protocol/mechanism.rs#L52-L53 + // This covers the signal number ranges of all supported platforms. + sentry_value_set_by_key( + signal_meta, "number", sentry_value_new_int64(sig_slot->signum)); } sentry_value_set_by_key(mechanism_meta, "signal", signal_meta); sentry_value_set_by_key( @@ -862,9 +862,8 @@ did_handler_thread_crash(void) if (g_handler_thread_ready && sentry__threadid_equal(current_thread, g_handler_thread)) { #ifdef SENTRY_PLATFORM_UNIX - static const char msg[] - = "[sentry] FATAL crash in handler thread, " - "falling back to previous handler\n"; + static const char msg[] = "[sentry] FATAL crash in handler thread, " + "falling back to previous handler\n"; const ssize_t rv = write(STDERR_FILENO, msg, sizeof(msg) - 1); (void)rv; #else diff --git a/src/sentry_sync.c b/src/sentry_sync.c index 1af5979b7..5e7bfe118 100644 --- a/src/sentry_sync.c +++ b/src/sentry_sync.c @@ -567,8 +567,7 @@ sentry__enter_signal_handler(void) // entering a signal handler while another runs, should block us while (__atomic_load_n(&g_in_signal_handler, __ATOMIC_RELAXED)) { // however, if we re-enter most likely a signal was raised from - // within the handler and then we should proceed. - // TODO: maybe pass in the signum and check for SIGABRT loops here + // within the signal handler and then we should proceed. if (is_handling_thread()) { return; } @@ -599,8 +598,6 @@ sentry__switch_handler_thread(void) sentry_threadid_t current = sentry__current_thread(); __atomic_store_n(&g_signal_handling_thread, current, __ATOMIC_RELEASE); - // TODO: this is still insufficient as a safe-guard when crashing in the - // handler thread # ifdef SENTRY_BACKEND_INPROC __atomic_store_n(&g_signal_handler_can_lock, 1, __ATOMIC_RELEASE); # endif From 84b3a7a812141a924cf2f23546ab3fe43c9c721e Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Mon, 10 Nov 2025 12:07:51 +0100 Subject: [PATCH 16/75] eliminate multichar warning in crashpad backend when using GCC --- src/backends/sentry_backend_crashpad.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/backends/sentry_backend_crashpad.cpp b/src/backends/sentry_backend_crashpad.cpp index 17ca48fa3..72de92078 100644 --- a/src/backends/sentry_backend_crashpad.cpp +++ b/src/backends/sentry_backend_crashpad.cpp @@ -32,6 +32,7 @@ extern "C" { #if defined(__GNUC__) # pragma GCC diagnostic push # pragma GCC diagnostic ignored "-Wunused-parameter" +# pragma GCC diagnostic ignored "-Wmultichar" #elif defined(_MSC_VER) # pragma warning(push) # pragma warning(disable : 4100) // unreferenced formal parameter From ef7b03b4f4c88ee34cc098c80aaf69403b549599 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Mon, 10 Nov 2025 12:09:17 +0100 Subject: [PATCH 17/75] ensure libunwind in benchmark and codeql workflows --- .github/workflows/benchmark.yml | 2 +- .github/workflows/codeql.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml index e4b73d0b8..024d6fd74 100644 --- a/.github/workflows/benchmark.yml +++ b/.github/workflows/benchmark.yml @@ -44,7 +44,7 @@ jobs: if: ${{ runner.os == 'Linux' }} run: | sudo apt update - sudo apt install -y libcurl4-openssl-dev + sudo apt install -y libcurl4-openssl-dev libunwind-dev - name: Benchmark shell: bash diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index fb5468074..e6fd5681d 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -37,7 +37,7 @@ jobs: - name: Installing Linux Dependencies run: | sudo apt update - sudo apt install cmake clang-14 clang-tools llvm kcov g++-12 valgrind zlib1g-dev libcurl4-openssl-dev + sudo apt install cmake clang-14 clang-tools llvm kcov g++-12 valgrind zlib1g-dev libcurl4-openssl-dev libunwind-dev - if: matrix.language == 'java' name: Setup Java Version From 6ae25c3b9079e794f49e9969774df6505911b3e6 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Mon, 10 Nov 2025 12:25:51 +0100 Subject: [PATCH 18/75] fix Linux ARM64 build (warning in libunwind) --- src/unwinder/sentry_unwinder_libunwind.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/unwinder/sentry_unwinder_libunwind.c b/src/unwinder/sentry_unwinder_libunwind.c index 844558dd7..3445cd172 100644 --- a/src/unwinder/sentry_unwinder_libunwind.c +++ b/src/unwinder/sentry_unwinder_libunwind.c @@ -21,7 +21,12 @@ sentry__unwind_stack_libunwind( } } else { unw_context_t uc; +// This pragma is required to build with Werror on ARM64 Ubuntu +#pragma clang diagnostic push +#pragma clang diagnostic ignored \ + "-Wgnu-statement-expression-from-macro-expansion" int ret = unw_getcontext(&uc); +#pragma clang diagnostic pop if (ret != 0) { SENTRY_WARN("Failed to retrieve context with libunwind"); return 0; From a0195dbcb87b15914f37f786a52ddf24e00da912 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Mon, 10 Nov 2025 14:09:13 +0100 Subject: [PATCH 19/75] provide an x86_64 ucontext stackwalker for macOS --- CMakeLists.txt | 10 +- src/unwinder/sentry_unwinder_libunwind_mac.c | 154 +++++++++++-------- 2 files changed, 102 insertions(+), 62 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 41f46f9d5..0f72823e1 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -835,11 +835,17 @@ if(SENTRY_BUILD_EXAMPLES) set_target_properties(sentry_example PROPERTIES FOLDER ${SENTRY_FOLDER}) endif() + if(CMAKE_GENERATOR STREQUAL "Xcode") + set(SENTRY_FIXTURE_OUTPUT_DIR "$/..") + else() + set(SENTRY_FIXTURE_OUTPUT_DIR "$") + endif() + add_custom_command(TARGET sentry_example POST_BUILD - COMMAND ${CMAKE_COMMAND} -E copy_if_different "${CMAKE_CURRENT_SOURCE_DIR}/tests/fixtures/minidump.dmp" "$/minidump.dmp") + COMMAND ${CMAKE_COMMAND} -E copy_if_different "${CMAKE_CURRENT_SOURCE_DIR}/tests/fixtures/minidump.dmp" "${SENTRY_FIXTURE_OUTPUT_DIR}/minidump.dmp") add_custom_command(TARGET sentry_example POST_BUILD - COMMAND ${CMAKE_COMMAND} -E copy_if_different "${CMAKE_CURRENT_SOURCE_DIR}/tests/fixtures/view-hierarchy.json" "$/view-hierarchy.json") + COMMAND ${CMAKE_COMMAND} -E copy_if_different "${CMAKE_CURRENT_SOURCE_DIR}/tests/fixtures/view-hierarchy.json" "${SENTRY_FIXTURE_OUTPUT_DIR}/view-hierarchy.json") endif() # Limit the exported symbols when sentry is built as a shared library to those with a "sentry_" prefix: diff --git a/src/unwinder/sentry_unwinder_libunwind_mac.c b/src/unwinder/sentry_unwinder_libunwind_mac.c index 176fb287b..b228300a0 100644 --- a/src/unwinder/sentry_unwinder_libunwind_mac.c +++ b/src/unwinder/sentry_unwinder_libunwind_mac.c @@ -10,82 +10,116 @@ valid_ptr(uintptr_t p) } size_t -sentry__unwind_stack_libunwind_mac( - void *addr, const sentry_ucontext_t *uctx, void **ptrs, size_t max_frames) +fp_walk_from_uctx(const sentry_ucontext_t *uctx, void **ptrs, size_t max_frames) { - if (addr) { - return 0; - } size_t frame_idx = 0; + struct __darwin_mcontext64 *mctx = uctx->user_context->uc_mcontext; +#if defined(__arm64__) + uintptr_t pc = (uintptr_t)mctx->__ss.__pc; + uintptr_t fp = (uintptr_t)mctx->__ss.__fp; + uintptr_t lr = (uintptr_t)mctx->__ss.__lr; - if (uctx) { - // TODO: this is a working ARM64 from ucontext unwinder using FP that - // doesn't have the issues we see backtrace() (which isn't even - // signal-safe) but also the system provided libunwind() on macOS - // - clean this up - // - implement an x86_64 ucontext unwinder + // top frame: adjust pc−1 so it symbolizes inside the function + if (pc) { + ptrs[frame_idx++] = (void *)(pc - 1); + } - struct __darwin_mcontext64 *mctx = uctx->user_context->uc_mcontext; - uintptr_t pc = (uintptr_t)mctx->__ss.__pc; - uintptr_t fp = (uintptr_t)mctx->__ss.__fp; - uintptr_t lr = (uintptr_t)mctx->__ss.__lr; + // next frame is from saved LR at current FP record + if (lr) { + ptrs[frame_idx++] = (void *)(lr - 1); + } - // top frame: adjust pc−1 so it symbolizes inside the function - if (pc) { - ptrs[frame_idx++] = (void *)(pc - 1); + for (size_t i = 0; i < max_frames; ++i) { + if (!valid_ptr(fp)) { + break; } - // next frame is from saved LR at current FP record - if (lr) { - ptrs[frame_idx++] = (void *)(lr - 1); + // arm64 frame record layout: [prev_fp, saved_lr] at fp and fp+8 + uintptr_t *record = (uintptr_t *)fp; + uintptr_t next_fp = record[0]; + uintptr_t ret_addr = record[1]; + if (!valid_ptr(next_fp) || !ret_addr) { + break; } - for (size_t i = 0; i < max_frames; ++i) { - if (!valid_ptr(fp)) { - break; - } + ptrs[frame_idx++] = (void *)(ret_addr - 1); + if (next_fp <= fp) { + break; // prevent loops + } + fp = next_fp; + } +#elif defined(__x86_64__) + uintptr_t ip = (uintptr_t)mctx->__ss.__rip; + uintptr_t bp = (uintptr_t)mctx->__ss.__rbp; - // arm64 frame record layout: [prev_fp, saved_lr] at fp and fp+8 - uintptr_t *record = (uintptr_t *)fp; - uintptr_t next_fp = record[0]; - uintptr_t retaddr = record[1]; - if (!valid_ptr(next_fp) || !retaddr) { - break; - } + // top frame: adjust ip−1 so it symbolizes inside the function + if (ip) { + ptrs[frame_idx++] = (void *)(ip - 1); + } - ptrs[frame_idx++] = (void *)(retaddr - 1); - if (next_fp <= fp) { - break; // prevent loops - } - fp = next_fp; + for (size_t i = 0; i < max_frames; ++i) { + if (!valid_ptr(bp)) { + break; } - } else { - unw_context_t uc; - int ret = unw_getcontext(&uc); - if (ret != 0) { - SENTRY_WARN("Failed to retrieve context with libunwind"); - return 0; + // x86_64 frame record layout: [prev_rbp, saved_retaddr] at bp and bp+8 + uintptr_t *record = (uintptr_t *)bp; + uintptr_t next_bp = record[0]; + uintptr_t ret_addr = record[1]; + if (!valid_ptr(next_bp) || !ret_addr) { + break; } - - unw_cursor_t cursor; - ret = unw_init_local(&cursor, &uc); - if (ret != 0) { - SENTRY_WARN("Failed to initialize libunwind with local context"); - return 0; + ptrs[frame_idx++] = (void *)(ret_addr - 1); + if (next_bp <= bp) { + break; } - while (unw_step(&cursor) > 0 && frame_idx < max_frames - 1) { - unw_word_t ip = 0; - SENTRY_INFOF("ip: %p", ip); - unw_get_reg(&cursor, UNW_REG_IP, &ip); + bp = next_bp; + } +#else +# error "Unsupported CPU architecture for macOS stackwalker" +#endif + return frame_idx + 1; +} + +size_t +sentry__unwind_stack_libunwind_mac( + void *addr, const sentry_ucontext_t *uctx, void **ptrs, size_t max_frames) +{ + if (addr) { + // we don't support stack walks from arbitrary addresses + return 0; + } + + if (uctx) { + return fp_walk_from_uctx(uctx, ptrs, max_frames); + } + + // fall back on the system `libunwind` for stack-traces "from call-site" + unw_context_t uc; + int ret = unw_getcontext(&uc); + if (ret != 0) { + SENTRY_WARN("Failed to retrieve context with libunwind"); + return 0; + } + + unw_cursor_t cursor; + ret = unw_init_local(&cursor, &uc); + if (ret != 0) { + SENTRY_WARN("Failed to initialize libunwind with local context"); + return 0; + } + size_t frame_idx = 0; + while (unw_step(&cursor) > 0 && frame_idx < max_frames - 1) { + unw_word_t ip = 0; + SENTRY_INFOF("ip: %p", ip); + unw_get_reg(&cursor, UNW_REG_IP, &ip); #if defined(__arm64__) - // Strip pointer authentication, for some reason ptrauth_strip() not - // working - // https://developer.apple.com/documentation/security/preparing_your_app_to_work_with_pointer_authentication - ip &= 0x7fffffffffffull; + // Strip pointer authentication, for some reason ptrauth_strip() not + // working + // https://developer.apple.com/documentation/security/preparing_your_app_to_work_with_pointer_authentication + ip &= 0x7fffffffffffull; #endif - ptrs[frame_idx] = (void *)ip; - frame_idx++; - } + ptrs[frame_idx] = (void *)ip; + frame_idx++; } return frame_idx + 1; From 8a63e64a6a46f7a0b2bd35092eede623456c682b Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Mon, 10 Nov 2025 14:17:15 +0100 Subject: [PATCH 20/75] bump lower-end GCC to 10.5.0 --- .github/workflows/ci.yml | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 140384c1f..343eb90ba 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -39,15 +39,15 @@ jobs: fail-fast: false matrix: include: - - name: Linux (GCC 9.5.0, 32-bit) + - name: Linux (GCC 10.5.0, 32-bit) os: ubuntu-22.04 - CC: gcc-9 - CXX: g++-9 + CC: gcc-10 + CXX: g++-10 TEST_X86: 1 - - name: Linux (GCC 9.5.0) + - name: Linux (GCC 10.5.0) os: ubuntu-22.04 - CC: gcc-9 - CXX: g++-9 + CC: gcc-10 + CXX: g++-10 # ERROR_ON_WARNINGS: 1 - name: Linux (GCC 12.3.0) os: ubuntu-24.04 @@ -213,7 +213,7 @@ jobs: make sudo make install - - name: Installing Linux GCC 9.4.0 Dependencies + - name: Installing Linux GCC 10.5.0 Dependencies if: ${{ runner.os == 'Linux' && matrix.os == 'ubuntu-22.04' && !env['TEST_X86'] && !matrix.container }} run: | sudo apt update @@ -224,7 +224,7 @@ jobs: run: | sudo dpkg --add-architecture i386 sudo apt update - sudo apt install cmake gcc-9-multilib g++-9-multilib zlib1g-dev:i386 libssl-dev:i386 libcurl4-openssl-dev:i386 + sudo apt install cmake gcc-10-multilib g++-10-multilib zlib1g-dev:i386 libssl-dev:i386 libcurl4-openssl-dev:i386 - name: Installing Alpine Linux Dependencies if: ${{ contains(matrix.container, 'alpine') }} From c1fd0a870248dd333fca1bc95f1c04ea48a5a378 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Mon, 10 Nov 2025 14:18:06 +0100 Subject: [PATCH 21/75] remove obsolete ASAN patch from CI --- .github/workflows/ci.yml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 343eb90ba..77db85925 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -232,11 +232,6 @@ jobs: apk update apk add curl-dev libunwind-dev libunwind-static xz-dev - # https://github.com/actions/runner-images/issues/9491 - - name: Decrease vm.mmap_rnd_bit to prevent ASLR ASAN issues - if: ${{ runner.os == 'Linux' && contains(env['RUN_ANALYZER'], 'asan') }} - run: sudo sysctl vm.mmap_rnd_bits=28 - - name: Installing CodeChecker if: ${{ contains(env['RUN_ANALYZER'], 'code-checker') }} run: sudo snap install codechecker --classic From 7c97dbb02f185ba298e29ae08672668fee522729 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Mon, 10 Nov 2025 14:36:08 +0100 Subject: [PATCH 22/75] ensure lower-end GCC also finds libunwind --- .github/workflows/ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 77db85925..f67cf3f29 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -217,14 +217,14 @@ jobs: if: ${{ runner.os == 'Linux' && matrix.os == 'ubuntu-22.04' && !env['TEST_X86'] && !matrix.container }} run: | sudo apt update - sudo apt install cmake llvm kcov g++ valgrind zlib1g-dev libcurl4-openssl-dev + sudo apt install cmake llvm kcov g++ valgrind zlib1g-dev libcurl4-openssl-dev libunwind-dev - name: Installing Linux 32-bit Dependencies if: ${{ runner.os == 'Linux' && env['TEST_X86'] && !matrix.container }} run: | sudo dpkg --add-architecture i386 sudo apt update - sudo apt install cmake gcc-10-multilib g++-10-multilib zlib1g-dev:i386 libssl-dev:i386 libcurl4-openssl-dev:i386 + sudo apt install cmake gcc-10-multilib g++-10-multilib zlib1g-dev:i386 libssl-dev:i386 libcurl4-openssl-dev:i386 libunwind-dev - name: Installing Alpine Linux Dependencies if: ${{ contains(matrix.container, 'alpine') }} From 524d8b4c17c55cbad0cb96c58c84a6fa8a35df90 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Mon, 10 Nov 2025 14:48:19 +0100 Subject: [PATCH 23/75] disable LTO for the example too --- CMakeLists.txt | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 0f72823e1..f31f1f6a5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -820,9 +820,17 @@ if(SENTRY_BUILD_EXAMPLES) # Most Linux distros will build executables with PIE. However, if we build a static library, not all static # dependencies being packaged with the same distro, are built with PIC enabled. This will lead `ld` to error on # relocation data, thus failing the build/tests. There is no need to build the example with PIE enabled. + # Similarly, LTO must be disabled since the static library archive might have different LTO bytecode version. if (LINUX AND NOT SENTRY_BUILD_SHARED_LIBS) - target_link_options(sentry_example PRIVATE -no-pie) - target_compile_options(sentry_example PRIVATE -fno-pie) + target_link_options(sentry_example PRIVATE + -no-pie + -no-lto + -Wl,--disable-lto + ) + target_compile_options(sentry_example PRIVATE + -fno-pie + -fno-lto + ) endif() endif() From 1c378c3af09d22ea67bd071793f102b4584ae8df Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Mon, 10 Nov 2025 14:49:00 +0100 Subject: [PATCH 24/75] use clang diagnostics only for __clang__ --- src/unwinder/sentry_unwinder_libunwind.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/unwinder/sentry_unwinder_libunwind.c b/src/unwinder/sentry_unwinder_libunwind.c index 3445cd172..6ddf3c7df 100644 --- a/src/unwinder/sentry_unwinder_libunwind.c +++ b/src/unwinder/sentry_unwinder_libunwind.c @@ -21,12 +21,16 @@ sentry__unwind_stack_libunwind( } } else { unw_context_t uc; +#ifdef __clang__ // This pragma is required to build with Werror on ARM64 Ubuntu -#pragma clang diagnostic push -#pragma clang diagnostic ignored \ - "-Wgnu-statement-expression-from-macro-expansion" +# pragma clang diagnostic push +# pragma clang diagnostic ignored \ + "-Wgnu-statement-expression-from-macro-expansion" +#endif int ret = unw_getcontext(&uc); -#pragma clang diagnostic pop +#ifdef __clang__ +# pragma clang diagnostic pop +#endif if (ret != 0) { SENTRY_WARN("Failed to retrieve context with libunwind"); return 0; From 4912a824091e407e5e85abddc5e942679293447f Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Mon, 10 Nov 2025 15:00:39 +0100 Subject: [PATCH 25/75] install 32-bit libunwind package for 32-bit Linux CI test config --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f67cf3f29..18de82c8d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -224,7 +224,7 @@ jobs: run: | sudo dpkg --add-architecture i386 sudo apt update - sudo apt install cmake gcc-10-multilib g++-10-multilib zlib1g-dev:i386 libssl-dev:i386 libcurl4-openssl-dev:i386 libunwind-dev + sudo apt install cmake gcc-10-multilib g++-10-multilib zlib1g-dev:i386 libssl-dev:i386 libcurl4-openssl-dev:i386 libunwind-dev:i386 - name: Installing Alpine Linux Dependencies if: ${{ contains(matrix.container, 'alpine') }} From 0c24c4fb2a4db02e2137fe41125f1e8de66ae9fb Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Mon, 10 Nov 2025 15:32:14 +0100 Subject: [PATCH 26/75] fix weird linker asymmetry --- CMakeLists.txt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index f31f1f6a5..0a6fd0109 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -824,8 +824,7 @@ if(SENTRY_BUILD_EXAMPLES) if (LINUX AND NOT SENTRY_BUILD_SHARED_LIBS) target_link_options(sentry_example PRIVATE -no-pie - -no-lto - -Wl,--disable-lto + -fno-lto ) target_compile_options(sentry_example PRIVATE -fno-pie From 70dd979e8c09897065e4e716a8e81980a4b739c6 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Mon, 10 Nov 2025 15:51:03 +0100 Subject: [PATCH 27/75] provide empty path-suffix so that CMake can find libunwind on platforms with architecture prefixes (32-bit Linux) --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 0a6fd0109..4da7f7c8c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -615,7 +615,7 @@ endif() if(SENTRY_WITH_LIBUNWIND) if(LINUX) option(SENTRY_LIBUNWIND_SHARED "Link to shared libunwind" ${SENTRY_BUILD_SHARED_LIBS}) - find_path(LIBUNWIND_INCLUDE_DIR libunwind.h PATH_SUFFIXES libunwind REQUIRED) + find_path(LIBUNWIND_INCLUDE_DIR libunwind.h PATH_SUFFIXES "" libunwind REQUIRED) if(SENTRY_LIBUNWIND_SHARED) find_library(LIBUNWIND_LIBRARIES unwind REQUIRED) else() From 4449859801c31371eb0a04448eaf60831d8117b2 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Mon, 10 Nov 2025 20:28:42 +0100 Subject: [PATCH 28/75] remove can_lock mechanism from the handler block API --- src/sentry_sync.c | 29 ----------------------------- 1 file changed, 29 deletions(-) diff --git a/src/sentry_sync.c b/src/sentry_sync.c index 5e7bfe118..a46acc2ff 100644 --- a/src/sentry_sync.c +++ b/src/sentry_sync.c @@ -510,20 +510,6 @@ sentry__bgworker_get_thread_name(sentry_bgworker_t *bgw) static sig_atomic_t g_in_signal_handler __attribute__((aligned(64))) = 0; static sentry_threadid_t g_signal_handling_thread = { 0 }; -# ifdef SENTRY_BACKEND_INPROC -static sig_atomic_t g_signal_handler_can_lock __attribute__((aligned(64))) = 0; - -static void -fatal_signal_lock_violation(void) -{ - static const char msg[] - = "[sentry] FATAL attempted to acquire mutex inside signal handler\n"; - const ssize_t rv = write(STDERR_FILENO, msg, sizeof(msg) - 1); - (void)rv; - _exit(1); -} -# endif - bool sentry__block_for_signal_handler(void) { @@ -538,12 +524,6 @@ sentry__block_for_signal_handler(void) = __atomic_load_n(&g_signal_handling_thread, __ATOMIC_ACQUIRE); if (sentry__threadid_equal(current, handling)) { -# ifdef SENTRY_BACKEND_INPROC - if (!__atomic_load_n( - &g_signal_handler_can_lock, __ATOMIC_ACQUIRE)) { - fatal_signal_lock_violation(); - } -# endif return true; } @@ -579,9 +559,6 @@ sentry__enter_signal_handler(void) // update the thread, now that no one else can and leave __atomic_store_n( &g_signal_handling_thread, current, __ATOMIC_RELEASE); -# ifdef SENTRY_BACKEND_INPROC - __atomic_store_n(&g_signal_handler_can_lock, 0, __ATOMIC_RELEASE); -# endif return; } @@ -598,9 +575,6 @@ sentry__switch_handler_thread(void) sentry_threadid_t current = sentry__current_thread(); __atomic_store_n(&g_signal_handling_thread, current, __ATOMIC_RELEASE); -# ifdef SENTRY_BACKEND_INPROC - __atomic_store_n(&g_signal_handler_can_lock, 1, __ATOMIC_RELEASE); -# endif return true; } @@ -611,9 +585,6 @@ sentry__leave_signal_handler(void) // clean up the thread-id and drop the reentrancy guard __atomic_store_n( &g_signal_handling_thread, (sentry_threadid_t) { 0 }, __ATOMIC_RELAXED); -# ifdef SENTRY_BACKEND_INPROC - __atomic_store_n(&g_signal_handler_can_lock, 0, __ATOMIC_RELAXED); -# endif __sync_lock_release(&g_in_signal_handler); } #endif From 4ce794095e6d9e9776d6c17db5b0d8f9f05fbe78 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Mon, 10 Nov 2025 23:11:40 +0100 Subject: [PATCH 29/75] use PRIx64 without ull cast in sentry__value_new_addr() --- src/sentry_value.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/sentry_value.c b/src/sentry_value.c index af4c8709b..7e4da8d94 100644 --- a/src/sentry_value.c +++ b/src/sentry_value.c @@ -1191,8 +1191,7 @@ sentry_value_t sentry__value_new_addr(uint64_t addr) { char buf[32]; - size_t written = (size_t)snprintf( - buf, sizeof(buf), "0x%llx", (unsigned long long)addr); + size_t written = (size_t)snprintf(buf, sizeof(buf), "0x%" PRIx64, addr); if (written >= sizeof(buf)) { return sentry_value_new_null(); } From 895492836e4618d8b0c223686898afde417d34ba Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Mon, 10 Nov 2025 23:17:42 +0100 Subject: [PATCH 30/75] fix off-by-one bug when returning the trace length after walking the stack also ensure to get the first frame harmonize libunwind usage --- src/unwinder/sentry_unwinder_libunwind.c | 38 ++++++++++--- src/unwinder/sentry_unwinder_libunwind_mac.c | 57 +++++++++++++++----- 2 files changed, 73 insertions(+), 22 deletions(-) diff --git a/src/unwinder/sentry_unwinder_libunwind.c b/src/unwinder/sentry_unwinder_libunwind.c index 6ddf3c7df..bd6ae8dd4 100644 --- a/src/unwinder/sentry_unwinder_libunwind.c +++ b/src/unwinder/sentry_unwinder_libunwind.c @@ -43,14 +43,36 @@ sentry__unwind_stack_libunwind( } } - size_t frame_idx = 0; - while (unw_step(&cursor) > 0 && frame_idx < max_frames - 1) { + size_t n = 0; + // get the first frame + if (n < max_frames) { unw_word_t ip = 0; - unw_get_reg(&cursor, UNW_REG_IP, &ip); - ptrs[frame_idx] = (void *)ip; - unw_word_t sp = 0; - unw_get_reg(&cursor, UNW_REG_SP, &sp); - frame_idx++; + if (unw_get_reg(&cursor, UNW_REG_IP, &ip) == 0) { + return n; + } + ptrs[n++] = (void *)ip; + } + // walk the callers + unw_word_t prev_ip = (unw_word_t)ptrs[0]; + unw_word_t prev_sp = 0; + (void)unw_get_reg(&cursor, UNW_REG_SP, &prev_sp); + + while (n < max_frames && unw_step(&cursor) > 0) { + unw_word_t ip = 0, sp = 0; + // stop the walk if we fail to read IP + if (unw_get_reg(&cursor, UNW_REG_IP, &ip) < 0) { + break; + } + // SP is optional for progress + (void)unw_get_reg(&cursor, UNW_REG_SP, &sp); + + // stop the walk if there is _no_ progress + if (ip == prev_ip && sp == prev_sp) { + break; + } + prev_ip = ip; + prev_sp = sp; + ptrs[n++] = (void *)ip; } - return frame_idx + 1; + return n; } diff --git a/src/unwinder/sentry_unwinder_libunwind_mac.c b/src/unwinder/sentry_unwinder_libunwind_mac.c index b228300a0..f9bc2a0d2 100644 --- a/src/unwinder/sentry_unwinder_libunwind_mac.c +++ b/src/unwinder/sentry_unwinder_libunwind_mac.c @@ -12,7 +12,7 @@ valid_ptr(uintptr_t p) size_t fp_walk_from_uctx(const sentry_ucontext_t *uctx, void **ptrs, size_t max_frames) { - size_t frame_idx = 0; + size_t n = 0; struct __darwin_mcontext64 *mctx = uctx->user_context->uc_mcontext; #if defined(__arm64__) uintptr_t pc = (uintptr_t)mctx->__ss.__pc; @@ -21,12 +21,12 @@ fp_walk_from_uctx(const sentry_ucontext_t *uctx, void **ptrs, size_t max_frames) // top frame: adjust pc−1 so it symbolizes inside the function if (pc) { - ptrs[frame_idx++] = (void *)(pc - 1); + ptrs[n++] = (void *)(pc - 1); } // next frame is from saved LR at current FP record if (lr) { - ptrs[frame_idx++] = (void *)(lr - 1); + ptrs[n++] = (void *)(lr - 1); } for (size_t i = 0; i < max_frames; ++i) { @@ -42,7 +42,7 @@ fp_walk_from_uctx(const sentry_ucontext_t *uctx, void **ptrs, size_t max_frames) break; } - ptrs[frame_idx++] = (void *)(ret_addr - 1); + ptrs[n++] = (void *)(ret_addr - 1); if (next_fp <= fp) { break; // prevent loops } @@ -54,7 +54,7 @@ fp_walk_from_uctx(const sentry_ucontext_t *uctx, void **ptrs, size_t max_frames) // top frame: adjust ip−1 so it symbolizes inside the function if (ip) { - ptrs[frame_idx++] = (void *)(ip - 1); + ptrs[n++] = (void *)(ip - 1); } for (size_t i = 0; i < max_frames; ++i) { @@ -68,7 +68,7 @@ fp_walk_from_uctx(const sentry_ucontext_t *uctx, void **ptrs, size_t max_frames) if (!valid_ptr(next_bp) || !ret_addr) { break; } - ptrs[frame_idx++] = (void *)(ret_addr - 1); + ptrs[n++] = (void *)(ret_addr - 1); if (next_bp <= bp) { break; } @@ -77,7 +77,7 @@ fp_walk_from_uctx(const sentry_ucontext_t *uctx, void **ptrs, size_t max_frames) #else # error "Unsupported CPU architecture for macOS stackwalker" #endif - return frame_idx + 1; + return n; } size_t @@ -107,20 +107,49 @@ sentry__unwind_stack_libunwind_mac( SENTRY_WARN("Failed to initialize libunwind with local context"); return 0; } - size_t frame_idx = 0; - while (unw_step(&cursor) > 0 && frame_idx < max_frames - 1) { + size_t n = 0; + // get the first frame + if (n < max_frames) { unw_word_t ip = 0; - SENTRY_INFOF("ip: %p", ip); - unw_get_reg(&cursor, UNW_REG_IP, &ip); + if (unw_get_reg(&cursor, UNW_REG_IP, &ip) >= 0) { +#if defined(__arm64__) + // Strip pointer authentication, for some reason ptrauth_strip() not + // working + // https://developer.apple.com/documentation/security/preparing_your_app_to_work_with_pointer_authentication + ip &= 0x7fffffffffffull; +#endif + ptrs[n++] = (void *)ip; + } else { + return 0; + } + } + // walk the callers + unw_word_t prev_ip = (uintptr_t)ptrs[0]; + unw_word_t prev_sp = 0; + (void)unw_get_reg(&cursor, UNW_REG_SP, &prev_sp); + while (n < max_frames && unw_step(&cursor) > 0) { + unw_word_t ip = 0, sp = 0; + // stop the walk if we fail to read IP + if (unw_get_reg(&cursor, UNW_REG_IP, &ip) < 0) { + break; + } + // SP is optional for progress + (void)unw_get_reg(&cursor, UNW_REG_SP, &sp); + + // stop the walk if there is _no_ progress + if (ip == prev_ip && sp == prev_sp) { + break; + } #if defined(__arm64__) // Strip pointer authentication, for some reason ptrauth_strip() not // working // https://developer.apple.com/documentation/security/preparing_your_app_to_work_with_pointer_authentication ip &= 0x7fffffffffffull; #endif - ptrs[frame_idx] = (void *)ip; - frame_idx++; + prev_ip = ip; + prev_sp = sp; + ptrs[n++] = (void *)ip; } - return frame_idx + 1; + return n; } From e633f2640da97e8739b07a046e6d5fc998014180 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Tue, 11 Nov 2025 09:35:08 +0100 Subject: [PATCH 31/75] clean up cmake configure stage in pytest --- tests/cmake.py | 106 +++++++++++++++++++++++++++---------------------- 1 file changed, 59 insertions(+), 47 deletions(-) diff --git a/tests/cmake.py b/tests/cmake.py index d49d0243f..082141de8 100644 --- a/tests/cmake.py +++ b/tests/cmake.py @@ -154,9 +154,7 @@ def cmake(cwd, targets, options=None, cflags=None): config_cmd = cmake.copy() if os.environ.get("VS_GENERATOR_TOOLSET") == "ClangCL": - config_cmd.append("-G Visual Studio 17 2022") - config_cmd.append("-A x64") - config_cmd.append("-T ClangCL") + configure_clang_cl(config_cmd) for key, value in options.items(): config_cmd.append("-D{}={}".format(key, value)) @@ -167,17 +165,7 @@ def cmake(cwd, targets, options=None, cflags=None): if "asan" in os.environ.get("RUN_ANALYZER", ""): config_cmd.append("-DWITH_ASAN_OPTION=ON") if "tsan" in os.environ.get("RUN_ANALYZER", ""): - module_dir = Path(__file__).resolve().parent - tsan_options = { - "verbosity": 2, - "detect_deadlocks": 1, - "second_deadlock_stack": 1, - "suppressions": module_dir / "tsan.supp", - } - os.environ["TSAN_OPTIONS"] = ":".join( - f"{key}={value}" for key, value in tsan_options.items() - ) - config_cmd.append("-DWITH_TSAN_OPTION=ON") + configure_tsan(config_cmd) # we have to set `-Werror` for this cmake invocation only, otherwise # completely unrelated things will break @@ -190,39 +178,7 @@ def cmake(cwd, targets, options=None, cflags=None): if "gcc" in os.environ.get("RUN_ANALYZER", ""): cflags.append("-fanalyzer") if "llvm-cov" in os.environ.get("RUN_ANALYZER", ""): - if False and os.environ.get("VS_GENERATOR_TOOLSET") == "ClangCL": - # for clang-cl in CI we have to use `--coverage` rather than `fprofile-instr-generate` and provide an - # architecture-specific profiling library for it work: - # TODO: This currently doesn't work due to https://gitlab.kitware.com/cmake/cmake/-/issues/24025 - # The issue describes a behavior where generated object-name is specified via `/fo` using a target - # directory, rather than a file-name (this is CMake behavior). While the `clang-cl` suggest that this - # is supported the flag produces `.gcda` and `.gcno` files, which have no relation with the object- - # file and which leads to failure to accumulate coverage data. - # This would have to be fixed in clang-cl: https://github.com/llvm/llvm-project/issues/87304 - # Let's leave this in here for posterity, it would be great to get coverage analysis on Windows. - flags = "--coverage" - profile_lib = "clang_rt.profile-x86_64.lib" - config_cmd.append(f"-DCMAKE_EXE_LINKER_FLAGS='{profile_lib}'") - config_cmd.append(f"-DCMAKE_SHARED_LINKER_FLAGS='{profile_lib}'") - config_cmd.append(f"-DCMAKE_MODULE_LINKER_FLAGS='{profile_lib}'") - else: - flags = "-fprofile-instr-generate -fcoverage-mapping" - config_cmd.append("-DCMAKE_C_FLAGS='{}'".format(flags)) - - # Since we overwrite `CXXFLAGS` below, we must add the experimental library here for the GHA runner that builds - # sentry-native with LLVM clang for macOS (to run ASAN on macOS) rather than the version coming with XCode. - # TODO: remove this if the GHA runner image for macOS ever updates beyond llvm15. - if ( - sys.platform == "darwin" - and os.environ.get("CC", "") == "clang" - and shutil.which("clang") == "/usr/local/opt/llvm@15/bin/clang" - ): - flags = ( - flags - + " -L/usr/local/opt/llvm@15/lib/c++ -fexperimental-library -Wno-unused-command-line-argument" - ) - - config_cmd.append("-DCMAKE_CXX_FLAGS='{}'".format(flags)) + configure_llvm_cov(config_cmd) if "CMAKE_DEFINES" in os.environ: config_cmd.extend(os.environ.get("CMAKE_DEFINES").split()) env = dict(os.environ) @@ -317,3 +273,59 @@ def cmake(cwd, targets, options=None, cflags=None): cwd=cwd, check=True, ) + + +def configure_clang_cl(config_cmd: list[str]): + config_cmd.append("-G Visual Studio 17 2022") + config_cmd.append("-A x64") + config_cmd.append("-T ClangCL") + + +def configure_tsan(config_cmd: list[str]): + module_dir = Path(__file__).resolve().parent + tsan_options = { + "verbosity": 2, + "detect_deadlocks": 1, + "second_deadlock_stack": 1, + "suppressions": module_dir / "tsan.supp", + } + os.environ["TSAN_OPTIONS"] = ":".join( + f"{key}={value}" for key, value in tsan_options.items() + ) + config_cmd.append("-DWITH_TSAN_OPTION=ON") + + +def configure_llvm_cov(config_cmd: list[str]): + if False and os.environ.get("VS_GENERATOR_TOOLSET") == "ClangCL": + # for clang-cl in CI we have to use `--coverage` rather than `fprofile-instr-generate` and provide an + # architecture-specific profiling library for it work: + # TODO: This currently doesn't work due to https://gitlab.kitware.com/cmake/cmake/-/issues/24025 + # The issue describes a behavior where generated object-name is specified via `/fo` using a target + # directory, rather than a file-name (this is CMake behavior). While the `clang-cl` suggest that this + # is supported the flag produces `.gcda` and `.gcno` files, which have no relation with the object- + # file and which leads to failure to accumulate coverage data. + # This would have to be fixed in clang-cl: https://github.com/llvm/llvm-project/issues/87304 + # Let's leave this in here for posterity, it would be great to get coverage analysis on Windows. + flags = "--coverage" + profile_lib = "clang_rt.profile-x86_64.lib" + config_cmd.append(f"-DCMAKE_EXE_LINKER_FLAGS='{profile_lib}'") + config_cmd.append(f"-DCMAKE_SHARED_LINKER_FLAGS='{profile_lib}'") + config_cmd.append(f"-DCMAKE_MODULE_LINKER_FLAGS='{profile_lib}'") + else: + flags = "-fprofile-instr-generate -fcoverage-mapping" + config_cmd.append("-DCMAKE_C_FLAGS='{}'".format(flags)) + + # Since we overwrite `CXXFLAGS` below, we must add the experimental library here for the GHA runner that builds + # sentry-native with LLVM clang for macOS (to run ASAN on macOS) rather than the version coming with XCode. + # TODO: remove this if the GHA runner image for macOS ever updates beyond llvm15. + if ( + sys.platform == "darwin" + and os.environ.get("CC", "") == "clang" + and shutil.which("clang") == "/usr/local/opt/llvm@15/bin/clang" + ): + flags = ( + flags + + " -L/usr/local/opt/llvm@15/lib/c++ -fexperimental-library -Wno-unused-command-line-argument" + ) + + config_cmd.append("-DCMAKE_CXX_FLAGS='{}'".format(flags)) From f7f4b4ebe124925a8709671f303688559591b1a2 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Tue, 11 Nov 2025 09:36:41 +0100 Subject: [PATCH 32/75] skip tests if zlib wasn't found during cmake configure stage --- tests/cmake.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/tests/cmake.py b/tests/cmake.py index 082141de8..62484910a 100644 --- a/tests/cmake.py +++ b/tests/cmake.py @@ -188,9 +188,18 @@ def cmake(cwd, targets, options=None, cflags=None): print("\n{} > {}".format(cwd, " ".join(config_cmd)), flush=True) try: - subprocess.run(config_cmd, cwd=cwd, env=env, check=True) - except subprocess.CalledProcessError: - raise pytest.fail.Exception("cmake configure failed") from None + result = subprocess.run( + config_cmd, cwd=cwd, env=env, check=True, capture_output=True, text=True + ) + sys.stdout.write(result.stdout) + sys.stderr.write(result.stderr or "") + except subprocess.CalledProcessError as e: + if "Could NOT find ZLIB" in e.stderr: + pytest.skip("ZLIB not found") + else: + sys.stdout.write(e.stdout) + sys.stderr.write(e.stderr or "") + raise pytest.fail.Exception("cmake configure failed") from None # CodeChecker invocations and options are documented here: # https://github.com/Ericsson/codechecker/blob/master/docs/analyzer/user_guide.md From 66618cdd4522851550a7980e92ff108b26a79e36 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Tue, 11 Nov 2025 09:46:28 +0100 Subject: [PATCH 33/75] fix stupid comparison error that led to all failing Linux tests --- src/unwinder/sentry_unwinder_libunwind.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/unwinder/sentry_unwinder_libunwind.c b/src/unwinder/sentry_unwinder_libunwind.c index bd6ae8dd4..a47d4ca9b 100644 --- a/src/unwinder/sentry_unwinder_libunwind.c +++ b/src/unwinder/sentry_unwinder_libunwind.c @@ -47,7 +47,7 @@ sentry__unwind_stack_libunwind( // get the first frame if (n < max_frames) { unw_word_t ip = 0; - if (unw_get_reg(&cursor, UNW_REG_IP, &ip) == 0) { + if (unw_get_reg(&cursor, UNW_REG_IP, &ip) < 0) { return n; } ptrs[n++] = (void *)ip; From 3432dea63fd5513e3e7e1e6b9ca48e5707c47ec7 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Tue, 11 Nov 2025 10:01:54 +0100 Subject: [PATCH 34/75] exclude `logger_level` from transport unit tests. --- tests/test_unit.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/test_unit.py b/tests/test_unit.py index 7ce28d04f..c38fffb91 100644 --- a/tests/test_unit.py +++ b/tests/test_unit.py @@ -17,7 +17,11 @@ def test_unit(cmake, unittest): @pytest.mark.skipif(not has_http, reason="tests need http transport") def test_unit_transport(cmake, unittest): - if unittest in ["custom_logger", "logger_enable_disable_functionality"]: + if unittest in [ + "custom_logger", + "logger_enable_disable_functionality", + "logger_level", + ]: pytest.skip("excluded from transport test-suite") cwd = cmake( From c0aa3995dce3408a87a3f65bd392bec9f3c58d78 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Tue, 11 Nov 2025 10:56:21 +0100 Subject: [PATCH 35/75] let find_path search the library architecture --- CMakeLists.txt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 4da7f7c8c..bd89f20eb 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -615,7 +615,11 @@ endif() if(SENTRY_WITH_LIBUNWIND) if(LINUX) option(SENTRY_LIBUNWIND_SHARED "Link to shared libunwind" ${SENTRY_BUILD_SHARED_LIBS}) - find_path(LIBUNWIND_INCLUDE_DIR libunwind.h PATH_SUFFIXES "" libunwind REQUIRED) + find_path(LIBUNWIND_INCLUDE_DIR libunwind.h + PATH_SUFFIXES + ${CMAKE_LIBRARY_ARCHITECTURE} + libunwind + REQUIRED) if(SENTRY_LIBUNWIND_SHARED) find_library(LIBUNWIND_LIBRARIES unwind REQUIRED) else() From 96afd3d551883dba9fd901a0efefe48b7044580c Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Tue, 11 Nov 2025 12:02:23 +0100 Subject: [PATCH 36/75] support i386 multilib on Ubuntu/Debian --- CMakeLists.txt | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index bd89f20eb..0e2a798e7 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -614,17 +614,21 @@ endif() if(SENTRY_WITH_LIBUNWIND) if(LINUX) + set(_multiarch_suffixes + i386-linux-gnu # Ubuntu/Debian i386 multilib + ${CMAKE_LIBRARY_ARCHITECTURE} # e.g. x86_64-linux-gnu, aarch64-linux-gnu + ) option(SENTRY_LIBUNWIND_SHARED "Link to shared libunwind" ${SENTRY_BUILD_SHARED_LIBS}) find_path(LIBUNWIND_INCLUDE_DIR libunwind.h PATH_SUFFIXES - ${CMAKE_LIBRARY_ARCHITECTURE} - libunwind + ${_multiarch_suffixes} + libunwind # some distros put the libunwind headers into a subdirectory REQUIRED) if(SENTRY_LIBUNWIND_SHARED) - find_library(LIBUNWIND_LIBRARIES unwind REQUIRED) + find_library(LIBUNWIND_LIBRARIES unwind PATH_SUFFIXES ${_multiarch_suffixes} REQUIRED) else() - find_library(LIBUNWIND_LIBRARIES libunwind.a REQUIRED) - find_library(LZMA_LIBRARY lzma) + find_library(LIBUNWIND_LIBRARIES libunwind.a PATH_SUFFIXES ${_multiarch_suffixes} REQUIRED) + find_library(LZMA_LIBRARY lzma PATH_SUFFIXES ${_multiarch_suffixes}) if(LZMA_LIBRARY) list(APPEND LIBUNWIND_LIBRARIES ${LZMA_LIBRARY}) endif() From bea2b65524281a749311218521bcc08011b5b2cb Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Tue, 11 Nov 2025 14:03:53 +0100 Subject: [PATCH 37/75] add 32-bit lzma package to Linux 32-bit build --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 18de82c8d..660ebbabc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -224,7 +224,7 @@ jobs: run: | sudo dpkg --add-architecture i386 sudo apt update - sudo apt install cmake gcc-10-multilib g++-10-multilib zlib1g-dev:i386 libssl-dev:i386 libcurl4-openssl-dev:i386 libunwind-dev:i386 + sudo apt install cmake gcc-10-multilib g++-10-multilib zlib1g-dev:i386 libssl-dev:i386 libcurl4-openssl-dev:i386 libunwind-dev:i386 lzma-dev:i386 - name: Installing Alpine Linux Dependencies if: ${{ contains(matrix.container, 'alpine') }} From dc117e51291e71a621ef7a542d52db04d789d4df Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Tue, 11 Nov 2025 14:49:07 +0100 Subject: [PATCH 38/75] typo --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 660ebbabc..c12b4e80f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -224,7 +224,7 @@ jobs: run: | sudo dpkg --add-architecture i386 sudo apt update - sudo apt install cmake gcc-10-multilib g++-10-multilib zlib1g-dev:i386 libssl-dev:i386 libcurl4-openssl-dev:i386 libunwind-dev:i386 lzma-dev:i386 + sudo apt install cmake gcc-10-multilib g++-10-multilib zlib1g-dev:i386 libssl-dev:i386 libcurl4-openssl-dev:i386 libunwind-dev:i386 liblzma-dev:i386 - name: Installing Alpine Linux Dependencies if: ${{ contains(matrix.container, 'alpine') }} From 14af96d575c5eb63c0bc4387b5b9e92df2e30bb5 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Tue, 11 Nov 2025 15:44:51 +0100 Subject: [PATCH 39/75] make even more elaborate multilib triplet and suffice handling based on FORCE32 --- CMakeLists.txt | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 0e2a798e7..0abb29572 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -614,21 +614,31 @@ endif() if(SENTRY_WITH_LIBUNWIND) if(LINUX) - set(_multiarch_suffixes - i386-linux-gnu # Ubuntu/Debian i386 multilib - ${CMAKE_LIBRARY_ARCHITECTURE} # e.g. x86_64-linux-gnu, aarch64-linux-gnu - ) option(SENTRY_LIBUNWIND_SHARED "Link to shared libunwind" ${SENTRY_BUILD_SHARED_LIBS}) + + # ensure suffix and search paths for include and library search (specifically for multilib) + if(SENTRY_BUILD_FORCE32) + set(_unwind_triplet "i386-linux-gnu") + set(_i386_libpaths /usr/lib/i386-linux-gnu /lib/i386-linux-gnu /usr/lib32 /lib32) + else() + set(_unwind_triplet "${CMAKE_LIBRARY_ARCHITECTURE}") + endif() + find_path(LIBUNWIND_INCLUDE_DIR libunwind.h PATH_SUFFIXES - ${_multiarch_suffixes} + ${_unwind_triplet} libunwind # some distros put the libunwind headers into a subdirectory REQUIRED) if(SENTRY_LIBUNWIND_SHARED) - find_library(LIBUNWIND_LIBRARIES unwind PATH_SUFFIXES ${_multiarch_suffixes} REQUIRED) + find_library(LIBUNWIND_LIBRARIES unwind PATH_SUFFIXES ${_unwind_triplet} REQUIRED) else() - find_library(LIBUNWIND_LIBRARIES libunwind.a PATH_SUFFIXES ${_multiarch_suffixes} REQUIRED) - find_library(LZMA_LIBRARY lzma PATH_SUFFIXES ${_multiarch_suffixes}) + find_library(LIBUNWIND_LIBRARIES libunwind.a PATH_SUFFIXES ${_unwind_triplet} REQUIRED) + if(SENTRY_BUILD_FORCE32) + find_library(LZMA_LIBRARY lzma PATHS ${_i386_libpaths} NO_DEFAULT_PATH) + else() + find_library(LZMA_LIBRARY lzma PATH_SUFFIXES ${_unwind_triplet}) + endif() + if(LZMA_LIBRARY) list(APPEND LIBUNWIND_LIBRARIES ${LZMA_LIBRARY}) endif() From 2e921a003b2f0bbba40b5caa5c7fc50217c8a9cc Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Tue, 11 Nov 2025 15:47:02 +0100 Subject: [PATCH 40/75] be even more careful wrt to max_frames in the fp walker and check on every access for overrun (+ track with while instead for-looping) --- src/unwinder/sentry_unwinder_libunwind_mac.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/unwinder/sentry_unwinder_libunwind_mac.c b/src/unwinder/sentry_unwinder_libunwind_mac.c index f9bc2a0d2..2843b0727 100644 --- a/src/unwinder/sentry_unwinder_libunwind_mac.c +++ b/src/unwinder/sentry_unwinder_libunwind_mac.c @@ -20,16 +20,16 @@ fp_walk_from_uctx(const sentry_ucontext_t *uctx, void **ptrs, size_t max_frames) uintptr_t lr = (uintptr_t)mctx->__ss.__lr; // top frame: adjust pc−1 so it symbolizes inside the function - if (pc) { + if (pc && n < max_frames) { ptrs[n++] = (void *)(pc - 1); } // next frame is from saved LR at current FP record - if (lr) { + if (lr && n < max_frames) { ptrs[n++] = (void *)(lr - 1); } - for (size_t i = 0; i < max_frames; ++i) { + while (n < max_frames) { if (!valid_ptr(fp)) { break; } @@ -53,11 +53,11 @@ fp_walk_from_uctx(const sentry_ucontext_t *uctx, void **ptrs, size_t max_frames) uintptr_t bp = (uintptr_t)mctx->__ss.__rbp; // top frame: adjust ip−1 so it symbolizes inside the function - if (ip) { + if (ip && n < max_frames) { ptrs[n++] = (void *)(ip - 1); } - for (size_t i = 0; i < max_frames; ++i) { + while (n < max_frames) { if (!valid_ptr(bp)) { break; } From e3643c7c8998495e7675fbfc5f5b750505049094 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Tue, 11 Nov 2025 16:17:38 +0100 Subject: [PATCH 41/75] eliminate the non-atomic access to g_handler_thread_ready from has_handler_thread_crashed --- src/backends/sentry_backend_inproc.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/backends/sentry_backend_inproc.c b/src/backends/sentry_backend_inproc.c index 849956177..5bb82eed0 100644 --- a/src/backends/sentry_backend_inproc.c +++ b/src/backends/sentry_backend_inproc.c @@ -856,10 +856,10 @@ stop_handler_thread(void) } static bool -did_handler_thread_crash(void) +has_handler_thread_crashed(void) { const sentry_threadid_t current_thread = sentry__current_thread(); - if (g_handler_thread_ready + if (sentry__atomic_fetch(&g_handler_thread_ready) && sentry__threadid_equal(current_thread, g_handler_thread)) { #ifdef SENTRY_PLATFORM_UNIX static const char msg[] = "[sentry] FATAL crash in handler thread, " @@ -1038,7 +1038,7 @@ process_ucontext(const sentry_ucontext_t *uctx) sentry__page_allocator_enable(); #endif - if (!did_handler_thread_crash()) { + if (!has_handler_thread_crashed()) { // invoke the handler thread for signal unsafe actions dispatch_ucontext(uctx, sig_slot); } From 3ffe195319331c8012f72cfdefc2b8c540c73f9b Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Tue, 11 Nov 2025 17:05:56 +0100 Subject: [PATCH 42/75] bring spinlock and pageallocator into the atomic fold --- src/sentry_unix_pageallocator.c | 22 ++++++++++++++-------- src/sentry_unix_spinlock.h | 12 +++++++++--- 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/src/sentry_unix_pageallocator.c b/src/sentry_unix_pageallocator.c index 0226c9b09..7c0e4ec40 100644 --- a/src/sentry_unix_pageallocator.c +++ b/src/sentry_unix_pageallocator.c @@ -33,20 +33,26 @@ static sentry_spinlock_t g_lock = SENTRY__SPINLOCK_INIT; bool sentry__page_allocator_enabled(void) { - return !!g_alloc; + return __atomic_load_n(&g_alloc, __ATOMIC_ACQUIRE) != NULL; } void sentry__page_allocator_enable(void) { + if (sentry__page_allocator_enabled()) { + return; + } sentry__spinlock_lock(&g_lock); - if (!g_alloc) { - g_alloc = &g_page_allocator_backing; - g_alloc->page_size = getpagesize(); - g_alloc->last_page = NULL; - g_alloc->current_page = NULL; - g_alloc->page_offset = 0; - g_alloc->pages_allocated = 0; + if (__atomic_load_n(&g_alloc, __ATOMIC_RELAXED) == NULL) { + struct page_allocator_s *p = &g_page_allocator_backing; + + p->page_size = getpagesize(); + p->last_page = NULL; + p->current_page = NULL; + p->page_offset = 0; + p->pages_allocated = 0; + + __atomic_store_n(&g_alloc, p, __ATOMIC_RELEASE); } sentry__spinlock_unlock(&g_lock); } diff --git a/src/sentry_unix_spinlock.h b/src/sentry_unix_spinlock.h index 575f86d67..b0b9f2b46 100644 --- a/src/sentry_unix_spinlock.h +++ b/src/sentry_unix_spinlock.h @@ -14,9 +14,15 @@ typedef volatile sig_atomic_t sentry_spinlock_t; #define SENTRY__SPINLOCK_INIT 0 #define sentry__spinlock_lock(spinlock_ref) \ - while (!__sync_bool_compare_and_swap(spinlock_ref, 0, 1)) { \ - sentry__cpu_relax(); \ + for (;;) { \ + while (__atomic_load_n(spinlock_ref, __ATOMIC_RELAXED)) { \ + sentry__cpu_relax(); \ + } \ + if (__atomic_exchange_n(spinlock_ref, 1, __ATOMIC_ACQUIRE) == 0) { \ + break; \ + } \ } -#define sentry__spinlock_unlock(spinlock_ref) (*spinlock_ref = 0) +#define sentry__spinlock_unlock(spinlock_ref) \ + (__atomic_store_n(spinlock_ref, 0, __ATOMIC_RELEASE)) #endif From 7eff2d1288b1befeb08b26347d77a37a28c95ff1 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Tue, 11 Nov 2025 17:19:15 +0100 Subject: [PATCH 43/75] fix: add GNU-stack note to crashpad_info_note.S --- external/crashpad | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/external/crashpad b/external/crashpad index d8990d2f6..2c2ae3c8a 160000 --- a/external/crashpad +++ b/external/crashpad @@ -1 +1 @@ -Subproject commit d8990d2f686b8827a21532748c6c42add21c21ea +Subproject commit 2c2ae3c8acc4fe1bf193d5b8979dd5fab232ee7c From 01061fdcd67a11511727305d3806351219767e0e Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Tue, 11 Nov 2025 17:29:56 +0100 Subject: [PATCH 44/75] update crashpad --- external/crashpad | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/external/crashpad b/external/crashpad index 2c2ae3c8a..a64405e09 160000 --- a/external/crashpad +++ b/external/crashpad @@ -1 +1 @@ -Subproject commit 2c2ae3c8acc4fe1bf193d5b8979dd5fab232ee7c +Subproject commit a64405e09ca9bb98ab169e594e1c17771414ec1b From fd584038b50321e3d16ab99411dccb105d146f54 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Tue, 11 Nov 2025 17:30:46 +0100 Subject: [PATCH 45/75] remove deprecated macOS 13 and replace it with macOS 26 --- .github/workflows/ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c12b4e80f..0126b5693 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -122,8 +122,8 @@ jobs: os: macos-14 ERROR_ON_WARNINGS: 1 SYSTEM_VERSION_COMPAT: 0 - - name: macOS 13 (xcode llvm) - os: macos-13 + - name: macOS 26 (xcode llvm) + os: macos-26 ERROR_ON_WARNINGS: 1 SYSTEM_VERSION_COMPAT: 0 - name: macOS 14 (xcode llvm + universal) From e8a393e1432b5bf1d67e9dd43bdaaa17784ae847 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Wed, 12 Nov 2025 11:33:42 +0100 Subject: [PATCH 46/75] update crashpad after merging to getsentry --- external/crashpad | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/external/crashpad b/external/crashpad index a64405e09..19d9d8a06 160000 --- a/external/crashpad +++ b/external/crashpad @@ -1 +1 @@ -Subproject commit a64405e09ca9bb98ab169e594e1c17771414ec1b +Subproject commit 19d9d8a06f6bbf3837db2464675e4545fff411f6 From b197e1ea64dedcb3b7284b151d40780e8547e10e Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Wed, 12 Nov 2025 11:35:04 +0100 Subject: [PATCH 47/75] simplify signal handler blocker by leaving and entering around the dispatch to the handler thread --- src/backends/sentry_backend_inproc.c | 5 ++- src/sentry_sync.c | 55 ++++------------------------ src/sentry_sync.h | 1 - 3 files changed, 12 insertions(+), 49 deletions(-) diff --git a/src/backends/sentry_backend_inproc.c b/src/backends/sentry_backend_inproc.c index 5bb82eed0..573d6c88c 100644 --- a/src/backends/sentry_backend_inproc.c +++ b/src/backends/sentry_backend_inproc.c @@ -910,6 +910,9 @@ dispatch_ucontext( } else { g_handler_state.uctx.user_context = NULL; } + + // we leave the handler + sentry__leave_signal_handler(); #endif sentry__atomic_store(&g_handler_work_done, 0); @@ -936,7 +939,7 @@ dispatch_ucontext( } #ifdef SENTRY_PLATFORM_UNIX - sentry__switch_handler_thread(); + sentry__enter_signal_handler(); #endif } diff --git a/src/sentry_sync.c b/src/sentry_sync.c index a46acc2ff..5c71e6b9c 100644 --- a/src/sentry_sync.c +++ b/src/sentry_sync.c @@ -508,22 +508,13 @@ sentry__bgworker_get_thread_name(sentry_bgworker_t *bgw) # include static sig_atomic_t g_in_signal_handler __attribute__((aligned(64))) = 0; -static sentry_threadid_t g_signal_handling_thread = { 0 }; bool sentry__block_for_signal_handler(void) { for (;;) { // if there is no signal handler active, we don't need to block - if (!__atomic_load_n(&g_in_signal_handler, __ATOMIC_RELAXED)) { - return true; - } - - sentry_threadid_t current = sentry__current_thread(); - sentry_threadid_t handling - = __atomic_load_n(&g_signal_handling_thread, __ATOMIC_ACQUIRE); - - if (sentry__threadid_equal(current, handling)) { + if (!__atomic_load_n(&g_in_signal_handler, __ATOMIC_ACQUIRE)) { return true; } @@ -532,59 +523,29 @@ sentry__block_for_signal_handler(void) } } -static bool -is_handling_thread(void) -{ - sentry_threadid_t handling - = __atomic_load_n(&g_signal_handling_thread, __ATOMIC_ACQUIRE); - return sentry__threadid_equal(handling, sentry__current_thread()); -} - void sentry__enter_signal_handler(void) { for (;;) { // entering a signal handler while another runs, should block us while (__atomic_load_n(&g_in_signal_handler, __ATOMIC_RELAXED)) { - // however, if we re-enter most likely a signal was raised from - // within the signal handler and then we should proceed. - if (is_handling_thread()) { - return; - } + sentry__cpu_relax(); } - // RMW that both tests AND sets atomically so we know we won the race - if (__sync_lock_test_and_set(&g_in_signal_handler, 1) == 0) { - sentry_threadid_t current = sentry__current_thread(); - // update the thread, now that no one else can and leave - __atomic_store_n( - &g_signal_handling_thread, current, __ATOMIC_RELEASE); + // atomically try to take ownership + sig_atomic_t prev + = __atomic_exchange_n(&g_in_signal_handler, 1, __ATOMIC_ACQ_REL); + if (prev == 0) { return; } - // otherwise, spin - } -} - -bool -sentry__switch_handler_thread(void) -{ - if (!__atomic_load_n(&g_in_signal_handler, __ATOMIC_ACQUIRE)) { - return false; + // otherwise we've been raced, spin } - - sentry_threadid_t current = sentry__current_thread(); - __atomic_store_n(&g_signal_handling_thread, current, __ATOMIC_RELEASE); - - return true; } void sentry__leave_signal_handler(void) { - // clean up the thread-id and drop the reentrancy guard - __atomic_store_n( - &g_signal_handling_thread, (sentry_threadid_t) { 0 }, __ATOMIC_RELAXED); - __sync_lock_release(&g_in_signal_handler); + __atomic_store_n(&g_in_signal_handler, 0, __ATOMIC_RELEASE); } #endif diff --git a/src/sentry_sync.h b/src/sentry_sync.h index 6157e5fb8..964822e6f 100644 --- a/src/sentry_sync.h +++ b/src/sentry_sync.h @@ -239,7 +239,6 @@ sentry__threadid_equal(sentry_threadid_t a, sentry_threadid_t b) bool sentry__block_for_signal_handler(void); void sentry__enter_signal_handler(void); void sentry__leave_signal_handler(void); -bool sentry__switch_handler_thread(void); typedef pthread_t sentry_threadid_t; typedef pthread_mutex_t sentry_mutex_t; From 4ff5a6e0fc239cba114cc6a55c5eae9afd45a215 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Wed, 12 Nov 2025 11:38:15 +0100 Subject: [PATCH 48/75] remove left-over handler switch --- src/backends/sentry_backend_inproc.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/backends/sentry_backend_inproc.c b/src/backends/sentry_backend_inproc.c index 573d6c88c..a5caf745a 100644 --- a/src/backends/sentry_backend_inproc.c +++ b/src/backends/sentry_backend_inproc.c @@ -750,9 +750,6 @@ handler_thread_main(void *UNUSED(data)) continue; } -#ifdef SENTRY_PLATFORM_UNIX - sentry__switch_handler_thread(); -#endif process_ucontext_deferred( &g_handler_state.uctx, g_handler_state.sig_slot); sentry__atomic_store(&g_handler_has_work, 0); From 192e0338cf69a738b93492066f2847be782183ea Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Wed, 12 Nov 2025 12:17:52 +0100 Subject: [PATCH 49/75] reintroduce handling thread into cleaned up signal blocker for the remaining backends --- src/sentry_sync.c | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/src/sentry_sync.c b/src/sentry_sync.c index 5c71e6b9c..1b91bf310 100644 --- a/src/sentry_sync.c +++ b/src/sentry_sync.c @@ -507,6 +507,7 @@ sentry__bgworker_get_thread_name(sentry_bgworker_t *bgw) # include "sentry_cpu_relax.h" # include +static sentry_threadid_t g_signal_handling_thread = { 0 }; static sig_atomic_t g_in_signal_handler __attribute__((aligned(64))) = 0; bool @@ -514,11 +515,18 @@ sentry__block_for_signal_handler(void) { for (;;) { // if there is no signal handler active, we don't need to block - if (!__atomic_load_n(&g_in_signal_handler, __ATOMIC_ACQUIRE)) { - return true; + // we can spin cheaply, but for the return we must acquire + if (!__atomic_load_n(&g_in_signal_handler, __ATOMIC_RELAXED)) { + return __atomic_load_n(&g_in_signal_handler, __ATOMIC_ACQUIRE) == 0; } - // otherwise, spin + // if we are on the signal handler thread we can also leave + if (sentry__threadid_equal(sentry__current_thread(), + __atomic_load_n(&g_signal_handling_thread, __ATOMIC_ACQUIRE))) { + return false; + } + + // otherwise, we spin sentry__cpu_relax(); } } @@ -533,9 +541,11 @@ sentry__enter_signal_handler(void) } // atomically try to take ownership - sig_atomic_t prev - = __atomic_exchange_n(&g_in_signal_handler, 1, __ATOMIC_ACQ_REL); - if (prev == 0) { + if (__atomic_exchange_n(&g_in_signal_handler, 1, __ATOMIC_ACQ_REL) + == 0) { + // once we have, publish the handling thread too + sentry_threadid_t me = sentry__current_thread(); + __atomic_store_n(&g_signal_handling_thread, me, __ATOMIC_RELEASE); return; } @@ -546,6 +556,11 @@ sentry__enter_signal_handler(void) void sentry__leave_signal_handler(void) { + // reset handling thread + __atomic_store_n( + &g_signal_handling_thread, (sentry_threadid_t) { 0 }, __ATOMIC_RELAXED); + + // reset handler flag __atomic_store_n(&g_in_signal_handler, 0, __ATOMIC_RELEASE); } #endif From 2ef97e100cde67ccc71533b26caf4d31c428ee86 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Wed, 12 Nov 2025 14:33:24 +0100 Subject: [PATCH 50/75] add valgrind suppression for false-positive deep in libunwind --- tests/valgrind.txt | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/valgrind.txt b/tests/valgrind.txt index 92b756580..955977747 100644 --- a/tests/valgrind.txt +++ b/tests/valgrind.txt @@ -37,3 +37,10 @@ fun:sentry__bgworker_start fun:test_sentry_task_queue } +{ + # writes of uninit buffers with padding or foreign-stack pointers lead to false-positive during stack unwinding + libunwind writes buffers we do not manage + Memcheck:Param + write(buf) + obj:*/libunwind.so* +} \ No newline at end of file From d72fb8729fcbc9014ec379c037cfae2a0f1822bd Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Wed, 12 Nov 2025 14:58:56 +0100 Subject: [PATCH 51/75] track failing ARM64/Linux clang-tsan build across available toolchain versions --- .github/workflows/ci.yml | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0126b5693..cb3e40b6b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -89,12 +89,36 @@ jobs: CXX: clang++-19 ERROR_ON_WARNINGS: 1 RUN_ANALYZER: tsan + - name: Linux Arm64 (clang 16 + tsan) + os: ubuntu-24.04-arm + CC: clang-16 + CXX: clang++-16 + ERROR_ON_WARNINGS: 1 + RUN_ANALYZER: tsan + - name: Linux Arm64 (clang 17 + tsan) + os: ubuntu-24.04-arm + CC: clang-17 + CXX: clang++-17 + ERROR_ON_WARNINGS: 1 + RUN_ANALYZER: tsan + - name: Linux Arm64 (clang 18 + tsan) + os: ubuntu-24.04-arm + CC: clang-18 + CXX: clang++-18 + ERROR_ON_WARNINGS: 1 + RUN_ANALYZER: tsan - name: Linux Arm64 (clang 19 + tsan) os: ubuntu-24.04-arm CC: clang-19 CXX: clang++-19 ERROR_ON_WARNINGS: 1 RUN_ANALYZER: tsan + - name: Linux Arm64 (clang 20 + tsan) + os: ubuntu-24.04-arm + CC: clang-20 + CXX: clang++-20 + ERROR_ON_WARNINGS: 1 + RUN_ANALYZER: tsan - name: Linux (clang 19 + kcov) os: ubuntu-24.04 CC: clang-19 @@ -200,7 +224,7 @@ jobs: if: ${{ runner.os == 'Linux' && matrix.os != 'ubuntu-22.04' && !env['TEST_X86'] && !matrix.container }} run: | sudo apt update - sudo apt install cmake clang-19 llvm g++-12 valgrind zlib1g-dev libcurl4-openssl-dev libunwind-dev + sudo apt install cmake clang-16 clang-17 clang-18 clang-19 clang-20 llvm g++-12 valgrind zlib1g-dev libcurl4-openssl-dev libunwind-dev # Install kcov from source sudo apt-get install binutils-dev libssl-dev libelf-dev libstdc++-12-dev libdw-dev libiberty-dev git clone https://github.com/SimonKagstrom/kcov.git From 3e51843f529875bf610cb5dca1fde26e9011ab39 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Thu, 13 Nov 2025 14:49:43 +0100 Subject: [PATCH 52/75] let valgrind match any frame between the libunwind obj matcher and the error site. --- tests/valgrind.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/valgrind.txt b/tests/valgrind.txt index 955977747..aaf7a10b5 100644 --- a/tests/valgrind.txt +++ b/tests/valgrind.txt @@ -42,5 +42,6 @@ libunwind writes buffers we do not manage Memcheck:Param write(buf) + ... obj:*/libunwind.so* } \ No newline at end of file From 21697c946c5068beb89065f0dd14252ab12c47d6 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Thu, 13 Nov 2025 15:47:20 +0100 Subject: [PATCH 53/75] move the check for a crashed handler thread into dispatch_ucontext() where we execute the unsafe part directly in the signal-handler if the handler thread is dead as a last chance report --- src/backends/sentry_backend_inproc.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/backends/sentry_backend_inproc.c b/src/backends/sentry_backend_inproc.c index a5caf745a..623c92a48 100644 --- a/src/backends/sentry_backend_inproc.c +++ b/src/backends/sentry_backend_inproc.c @@ -882,7 +882,10 @@ static void dispatch_ucontext( const sentry_ucontext_t *uctx, const struct signal_slot *sig_slot) { - if (!sentry__atomic_fetch(&g_handler_thread_ready)) { + if (!sentry__atomic_fetch(&g_handler_thread_ready) + || has_handler_thread_crashed()) { + // directly execute unsafe part in signal handler as a last chance to + // report an error when the handler thread is unavailable. process_ucontext_deferred(uctx, sig_slot); return; } @@ -1038,10 +1041,8 @@ process_ucontext(const sentry_ucontext_t *uctx) sentry__page_allocator_enable(); #endif - if (!has_handler_thread_crashed()) { - // invoke the handler thread for signal unsafe actions - dispatch_ucontext(uctx, sig_slot); - } + // invoke the handler thread for signal unsafe actions + dispatch_ucontext(uctx, sig_slot); #ifdef SENTRY_PLATFORM_UNIX // reset signal handlers and invoke the original ones. This will then tear From 9388b49ac0897562d23a36fdcb4d69857aacf92a Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Thu, 13 Nov 2025 15:49:50 +0100 Subject: [PATCH 54/75] isolate the libunwind walker as a tsan cause in CI --- src/unwinder/sentry_unwinder_libunwind.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/unwinder/sentry_unwinder_libunwind.c b/src/unwinder/sentry_unwinder_libunwind.c index a47d4ca9b..645cf55fb 100644 --- a/src/unwinder/sentry_unwinder_libunwind.c +++ b/src/unwinder/sentry_unwinder_libunwind.c @@ -57,6 +57,7 @@ sentry__unwind_stack_libunwind( unw_word_t prev_sp = 0; (void)unw_get_reg(&cursor, UNW_REG_SP, &prev_sp); +#ifdef TSAN_CI_ISSUE_ISOLATION while (n < max_frames && unw_step(&cursor) > 0) { unw_word_t ip = 0, sp = 0; // stop the walk if we fail to read IP @@ -74,5 +75,6 @@ sentry__unwind_stack_libunwind( prev_sp = sp; ptrs[n++] = (void *)ip; } +#endif return n; } From 37477baf1616a4b7861cb1be135c76cd85b453ee Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Thu, 13 Nov 2025 16:01:47 +0100 Subject: [PATCH 55/75] exclude walker variables to prevent unused variables Werror --- src/unwinder/sentry_unwinder_libunwind.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/unwinder/sentry_unwinder_libunwind.c b/src/unwinder/sentry_unwinder_libunwind.c index 645cf55fb..74b038199 100644 --- a/src/unwinder/sentry_unwinder_libunwind.c +++ b/src/unwinder/sentry_unwinder_libunwind.c @@ -53,11 +53,11 @@ sentry__unwind_stack_libunwind( ptrs[n++] = (void *)ip; } // walk the callers +#ifdef TSAN_CI_ISSUE_ISOLATION unw_word_t prev_ip = (unw_word_t)ptrs[0]; unw_word_t prev_sp = 0; (void)unw_get_reg(&cursor, UNW_REG_SP, &prev_sp); -#ifdef TSAN_CI_ISSUE_ISOLATION while (n < max_frames && unw_step(&cursor) > 0) { unw_word_t ip = 0, sp = 0; // stop the walk if we fail to read IP From fcee5a511c084900acd32dbc4360d738e293fc31 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Thu, 13 Nov 2025 16:22:55 +0100 Subject: [PATCH 56/75] isolation pinpoint fine, but only exclude when using ucontext. --- src/unwinder/sentry_unwinder_libunwind.c | 38 ++++++++++++------------ 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/src/unwinder/sentry_unwinder_libunwind.c b/src/unwinder/sentry_unwinder_libunwind.c index 74b038199..c2e770007 100644 --- a/src/unwinder/sentry_unwinder_libunwind.c +++ b/src/unwinder/sentry_unwinder_libunwind.c @@ -53,28 +53,28 @@ sentry__unwind_stack_libunwind( ptrs[n++] = (void *)ip; } // walk the callers -#ifdef TSAN_CI_ISSUE_ISOLATION - unw_word_t prev_ip = (unw_word_t)ptrs[0]; - unw_word_t prev_sp = 0; - (void)unw_get_reg(&cursor, UNW_REG_SP, &prev_sp); + if (!uctx) { + unw_word_t prev_ip = (unw_word_t)ptrs[0]; + unw_word_t prev_sp = 0; + (void)unw_get_reg(&cursor, UNW_REG_SP, &prev_sp); - while (n < max_frames && unw_step(&cursor) > 0) { - unw_word_t ip = 0, sp = 0; - // stop the walk if we fail to read IP - if (unw_get_reg(&cursor, UNW_REG_IP, &ip) < 0) { - break; - } - // SP is optional for progress - (void)unw_get_reg(&cursor, UNW_REG_SP, &sp); + while (n < max_frames && unw_step(&cursor) > 0) { + unw_word_t ip = 0, sp = 0; + // stop the walk if we fail to read IP + if (unw_get_reg(&cursor, UNW_REG_IP, &ip) < 0) { + break; + } + // SP is optional for progress + (void)unw_get_reg(&cursor, UNW_REG_SP, &sp); - // stop the walk if there is _no_ progress - if (ip == prev_ip && sp == prev_sp) { - break; + // stop the walk if there is _no_ progress + if (ip == prev_ip && sp == prev_sp) { + break; + } + prev_ip = ip; + prev_sp = sp; + ptrs[n++] = (void *)ip; } - prev_ip = ip; - prev_sp = sp; - ptrs[n++] = (void *)ip; } -#endif return n; } From eda314b4b31676606ebf36de8794faf57c361b52 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Thu, 13 Nov 2025 18:34:43 +0100 Subject: [PATCH 57/75] silence clang-20 warning for crashpad --- external/crashpad | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/external/crashpad b/external/crashpad index 19d9d8a06..fbb5badde 160000 --- a/external/crashpad +++ b/external/crashpad @@ -1 +1 @@ -Subproject commit 19d9d8a06f6bbf3837db2464675e4545fff411f6 +Subproject commit fbb5badde25e9602b9fe44be329ddd005be7e637 From 89d95d21ab94fed630094094dff2cd48c53e3b40 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Fri, 14 Nov 2025 17:01:17 +0100 Subject: [PATCH 58/75] update `crashpad` after merging to `getsentry` --- external/crashpad | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/external/crashpad b/external/crashpad index fbb5badde..60dd8995c 160000 --- a/external/crashpad +++ b/external/crashpad @@ -1 +1 @@ -Subproject commit fbb5badde25e9602b9fe44be329ddd005be7e637 +Subproject commit 60dd8995c6a8539718c878f9b41063604abe737c From 4d5114badfb43e6ad0c52e134303d35cae200cf2 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Fri, 14 Nov 2025 18:50:52 +0100 Subject: [PATCH 59/75] we know the walk crashes in aarch64 tsan, now introduce stack bound reader in the libunwind walker for Linux and log as much as possible to understand where the actual crash happens --- src/unwinder/sentry_unwinder_libunwind.c | 119 ++++++++++++++++++----- tests/test_integration_stdout.py | 2 +- 2 files changed, 97 insertions(+), 24 deletions(-) diff --git a/src/unwinder/sentry_unwinder_libunwind.c b/src/unwinder/sentry_unwinder_libunwind.c index c2e770007..e75ef841e 100644 --- a/src/unwinder/sentry_unwinder_libunwind.c +++ b/src/unwinder/sentry_unwinder_libunwind.c @@ -2,6 +2,49 @@ #include "sentry_logger.h" #define UNW_LOCAL_ONLY #include +#include + +typedef struct { + uintptr_t lo, hi; +} mem_range_t; + +/** + * Looks up the memory range for a given pointer in /proc/self/maps. + * `range` is an output parameter. Returns 0 on success. + * Note: it is safe to use this function as long as we are running in a healthy + * thread or in the handler thread. The function is unsafe for a signal handler. + */ +static int +find_mem_range(uintptr_t ptr, mem_range_t *range) +{ + FILE *fp = fopen("/proc/self/maps", "r"); + if (!fp) { + return 1; + } + int result = 1; + char line[512]; + while (fgets(line, sizeof line, fp)) { + unsigned long lo, hi; + SENTRY_INFOF("%s", line); + if (sscanf(line, "%lx-%lx", &lo, &hi) == 2) { + // our bounds are [lo, hi) + if (ptr >= lo && ptr < hi) { + range->lo = (uintptr_t)lo; + range->hi = (uintptr_t)hi; + SENTRY_INFOF("Found ptr (0x%" PRIx64 + ") in the range %0x%" PRIx64 "-%0x%" PRIx64, + ptr, lo, hi); + result = 0; + break; + } + } + } + fclose(fp); + if (result) { + SENTRY_WARNF("Failed to find range for ptr (0x%" PRIx64 ")", ptr); + } + return result; +} size_t sentry__unwind_stack_libunwind( @@ -45,36 +88,66 @@ sentry__unwind_stack_libunwind( size_t n = 0; // get the first frame + unw_word_t ip = 0; + if (unw_get_reg(&cursor, UNW_REG_IP, &ip) < 0) { + return n; + } if (n < max_frames) { - unw_word_t ip = 0; - if (unw_get_reg(&cursor, UNW_REG_IP, &ip) < 0) { - return n; - } ptrs[n++] = (void *)ip; } - // walk the callers - if (!uctx) { - unw_word_t prev_ip = (unw_word_t)ptrs[0]; - unw_word_t prev_sp = 0; - (void)unw_get_reg(&cursor, UNW_REG_SP, &prev_sp); + unw_word_t sp = 0; + (void)unw_get_reg(&cursor, UNW_REG_SP, &sp); - while (n < max_frames && unw_step(&cursor) > 0) { - unw_word_t ip = 0, sp = 0; - // stop the walk if we fail to read IP - if (unw_get_reg(&cursor, UNW_REG_IP, &ip) < 0) { - break; + mem_range_t stack = { 0, 0 }; + int have_bounds = 0; + long page_size = 0; + if (uctx) { + if (find_mem_range((uintptr_t)sp, &stack) == 0) { + have_bounds = 1; + page_size = sysconf(_SC_PAGESIZE); + if (page_size <= 0) { + page_size = 4096; } - // SP is optional for progress - (void)unw_get_reg(&cursor, UNW_REG_SP, &sp); + } + } - // stop the walk if there is _no_ progress - if (ip == prev_ip && sp == prev_sp) { - break; - } - prev_ip = ip; - prev_sp = sp; - ptrs[n++] = (void *)ip; + // walk the callers + unw_word_t prev_ip = ip; + unw_word_t prev_sp = sp; + size_t step_idx = 0; + while (n < max_frames) { + SENTRY_DEBUGF("unwind: about to unw_step, step=%zu, prev_ip=%p prev_sp=%p", + step_idx, (void*)prev_ip, (void*)prev_sp); + if (unw_step(&cursor) <= 0) { + SENTRY_DEBUGF("unwind: unw_step failed at step=%zu", step_idx); + break; } + SENTRY_DEBUGF("unwind: unw_step success at step=%zu", step_idx); + + // stop the walk if we fail to read IP + if (unw_get_reg(&cursor, UNW_REG_IP, &ip) < 0) { + SENTRY_DEBUGF("unwind: no progress at step=%zu", step_idx); + break; + } + // SP is optional for progress + (void)unw_get_reg(&cursor, UNW_REG_SP, &sp); + + // stop the walk if there is _no_ progress + if (ip == prev_ip && sp == prev_sp) { + break; + } + + if (have_bounds) { + intptr_t d_lo = (intptr_t)((uintptr_t)sp - stack.lo); + intptr_t d_hi = (intptr_t)((uintptr_t)stack.hi - sp); + SENTRY_DEBUGF("unwind: unw_step %zu: ip=%p, sp=%p, d_lo=%zd, d_hi=%zd", n, + (void *)ip, (void *)sp, d_lo, d_hi); + } + + prev_ip = ip; + prev_sp = sp; + ptrs[n++] = (void *)ip; + step_idx++; } return n; } diff --git a/tests/test_integration_stdout.py b/tests/test_integration_stdout.py index fdded435a..d7ed20ccb 100644 --- a/tests/test_integration_stdout.py +++ b/tests/test_integration_stdout.py @@ -240,7 +240,7 @@ def test_inproc_crash_stdout_before_send_and_on_crash(cmake): ) def test_inproc_stack_overflow_stdout(cmake, build_args): tmp_path, output = run_stdout_for( - "inproc", cmake, ["attachment", "stack-overflow"], build_args + "inproc", cmake, ["log", "attachment", "stack-overflow"], build_args ) envelope = Envelope.deserialize(output) From 13dfdbcc358d37b5c49fd130bf93f0f6419d68b3 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Fri, 14 Nov 2025 18:51:19 +0100 Subject: [PATCH 60/75] format --- src/unwinder/sentry_unwinder_libunwind.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/unwinder/sentry_unwinder_libunwind.c b/src/unwinder/sentry_unwinder_libunwind.c index e75ef841e..596d859bc 100644 --- a/src/unwinder/sentry_unwinder_libunwind.c +++ b/src/unwinder/sentry_unwinder_libunwind.c @@ -116,8 +116,9 @@ sentry__unwind_stack_libunwind( unw_word_t prev_sp = sp; size_t step_idx = 0; while (n < max_frames) { - SENTRY_DEBUGF("unwind: about to unw_step, step=%zu, prev_ip=%p prev_sp=%p", - step_idx, (void*)prev_ip, (void*)prev_sp); + SENTRY_DEBUGF( + "unwind: about to unw_step, step=%zu, prev_ip=%p prev_sp=%p", + step_idx, (void *)prev_ip, (void *)prev_sp); if (unw_step(&cursor) <= 0) { SENTRY_DEBUGF("unwind: unw_step failed at step=%zu", step_idx); break; @@ -140,7 +141,8 @@ sentry__unwind_stack_libunwind( if (have_bounds) { intptr_t d_lo = (intptr_t)((uintptr_t)sp - stack.lo); intptr_t d_hi = (intptr_t)((uintptr_t)stack.hi - sp); - SENTRY_DEBUGF("unwind: unw_step %zu: ip=%p, sp=%p, d_lo=%zd, d_hi=%zd", n, + SENTRY_DEBUGF( + "unwind: unw_step %zu: ip=%p, sp=%p, d_lo=%zd, d_hi=%zd", n, (void *)ip, (void *)sp, d_lo, d_hi); } From 72612cfaf8b3bdfd2cec602a90ddd6efc5dc6b2a Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Fri, 14 Nov 2025 19:22:18 +0100 Subject: [PATCH 61/75] add unistd.h for musl --- src/unwinder/sentry_unwinder_libunwind.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/unwinder/sentry_unwinder_libunwind.c b/src/unwinder/sentry_unwinder_libunwind.c index 596d859bc..74a3b5f7b 100644 --- a/src/unwinder/sentry_unwinder_libunwind.c +++ b/src/unwinder/sentry_unwinder_libunwind.c @@ -3,6 +3,7 @@ #define UNW_LOCAL_ONLY #include #include +#include typedef struct { uintptr_t lo, hi; From df9272853311ae5fbd9bba0299e3f76ffc8221ad Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Fri, 14 Nov 2025 20:35:09 +0100 Subject: [PATCH 62/75] eliminate the logs and ensure we never walk the callers if the SP is in unmapped memory --- src/unwinder/sentry_unwinder_libunwind.c | 59 ++++++------------------ 1 file changed, 13 insertions(+), 46 deletions(-) diff --git a/src/unwinder/sentry_unwinder_libunwind.c b/src/unwinder/sentry_unwinder_libunwind.c index 74a3b5f7b..d2c817ce9 100644 --- a/src/unwinder/sentry_unwinder_libunwind.c +++ b/src/unwinder/sentry_unwinder_libunwind.c @@ -3,7 +3,6 @@ #define UNW_LOCAL_ONLY #include #include -#include typedef struct { uintptr_t lo, hi; @@ -11,40 +10,33 @@ typedef struct { /** * Looks up the memory range for a given pointer in /proc/self/maps. - * `range` is an output parameter. Returns 0 on success. + * `range` is an output parameter. Returns `true` if a range was found. * Note: it is safe to use this function as long as we are running in a healthy * thread or in the handler thread. The function is unsafe for a signal handler. */ -static int +static bool find_mem_range(uintptr_t ptr, mem_range_t *range) { + bool found = false; FILE *fp = fopen("/proc/self/maps", "r"); if (!fp) { - return 1; + return found; } - int result = 1; char line[512]; while (fgets(line, sizeof line, fp)) { unsigned long lo, hi; - SENTRY_INFOF("%s", line); if (sscanf(line, "%lx-%lx", &lo, &hi) == 2) { // our bounds are [lo, hi) if (ptr >= lo && ptr < hi) { range->lo = (uintptr_t)lo; range->hi = (uintptr_t)hi; - SENTRY_INFOF("Found ptr (0x%" PRIx64 - ") in the range %0x%" PRIx64 "-%0x%" PRIx64, - ptr, lo, hi); - result = 0; + found = true; break; } } } fclose(fp); - if (result) { - SENTRY_WARNF("Failed to find range for ptr (0x%" PRIx64 ")", ptr); - } - return result; + return found; } size_t @@ -88,7 +80,7 @@ sentry__unwind_stack_libunwind( } size_t n = 0; - // get the first frame + // get the first frame and stack pointer unw_word_t ip = 0; if (unw_get_reg(&cursor, UNW_REG_IP, &ip) < 0) { return n; @@ -99,36 +91,20 @@ sentry__unwind_stack_libunwind( unw_word_t sp = 0; (void)unw_get_reg(&cursor, UNW_REG_SP, &sp); + // ensure we have a valid stack pointer otherwise we only send the top frame mem_range_t stack = { 0, 0 }; - int have_bounds = 0; - long page_size = 0; - if (uctx) { - if (find_mem_range((uintptr_t)sp, &stack) == 0) { - have_bounds = 1; - page_size = sysconf(_SC_PAGESIZE); - if (page_size <= 0) { - page_size = 4096; - } - } + if (uctx && !find_mem_range((uintptr_t)sp, &stack) == 0) { + SENTRY_WARNF("unwinder: SP (%p) is in unmapped memory likely due to stack overflow", + (void *)sp); + return n; } // walk the callers unw_word_t prev_ip = ip; unw_word_t prev_sp = sp; - size_t step_idx = 0; - while (n < max_frames) { - SENTRY_DEBUGF( - "unwind: about to unw_step, step=%zu, prev_ip=%p prev_sp=%p", - step_idx, (void *)prev_ip, (void *)prev_sp); - if (unw_step(&cursor) <= 0) { - SENTRY_DEBUGF("unwind: unw_step failed at step=%zu", step_idx); - break; - } - SENTRY_DEBUGF("unwind: unw_step success at step=%zu", step_idx); - + while (n < max_frames && unw_step(&cursor) > 0) { // stop the walk if we fail to read IP if (unw_get_reg(&cursor, UNW_REG_IP, &ip) < 0) { - SENTRY_DEBUGF("unwind: no progress at step=%zu", step_idx); break; } // SP is optional for progress @@ -139,18 +115,9 @@ sentry__unwind_stack_libunwind( break; } - if (have_bounds) { - intptr_t d_lo = (intptr_t)((uintptr_t)sp - stack.lo); - intptr_t d_hi = (intptr_t)((uintptr_t)stack.hi - sp); - SENTRY_DEBUGF( - "unwind: unw_step %zu: ip=%p, sp=%p, d_lo=%zd, d_hi=%zd", n, - (void *)ip, (void *)sp, d_lo, d_hi); - } - prev_ip = ip; prev_sp = sp; ptrs[n++] = (void *)ip; - step_idx++; } return n; } From bdf6b2573146d889487cf3689cb23aa3f80b8d91 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Fri, 14 Nov 2025 20:36:13 +0100 Subject: [PATCH 63/75] format --- src/unwinder/sentry_unwinder_libunwind.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/unwinder/sentry_unwinder_libunwind.c b/src/unwinder/sentry_unwinder_libunwind.c index d2c817ce9..a9d086392 100644 --- a/src/unwinder/sentry_unwinder_libunwind.c +++ b/src/unwinder/sentry_unwinder_libunwind.c @@ -94,7 +94,8 @@ sentry__unwind_stack_libunwind( // ensure we have a valid stack pointer otherwise we only send the top frame mem_range_t stack = { 0, 0 }; if (uctx && !find_mem_range((uintptr_t)sp, &stack) == 0) { - SENTRY_WARNF("unwinder: SP (%p) is in unmapped memory likely due to stack overflow", + SENTRY_WARNF("unwinder: SP (%p) is in unmapped memory likely due to " + "stack overflow", (void *)sp); return n; } From 7e4e1fa71dadde76dfd75d42f148634d3f3b67a9 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Fri, 14 Nov 2025 21:07:50 +0100 Subject: [PATCH 64/75] fix invalid `find_mem_range()` return value check --- src/unwinder/sentry_unwinder_libunwind.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/unwinder/sentry_unwinder_libunwind.c b/src/unwinder/sentry_unwinder_libunwind.c index a9d086392..563228e9d 100644 --- a/src/unwinder/sentry_unwinder_libunwind.c +++ b/src/unwinder/sentry_unwinder_libunwind.c @@ -93,7 +93,7 @@ sentry__unwind_stack_libunwind( // ensure we have a valid stack pointer otherwise we only send the top frame mem_range_t stack = { 0, 0 }; - if (uctx && !find_mem_range((uintptr_t)sp, &stack) == 0) { + if (uctx && !find_mem_range((uintptr_t)sp, &stack)) { SENTRY_WARNF("unwinder: SP (%p) is in unmapped memory likely due to " "stack overflow", (void *)sp); From 0966118fa88e7da880615e2ddaed42125c26ea8e Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Fri, 14 Nov 2025 21:08:44 +0100 Subject: [PATCH 65/75] remove macOS left-overs in the `libbacktrace` unwinder module --- src/unwinder/sentry_unwinder_libbacktrace.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/unwinder/sentry_unwinder_libbacktrace.c b/src/unwinder/sentry_unwinder_libbacktrace.c index dd671a958..fbfb0f895 100644 --- a/src/unwinder/sentry_unwinder_libbacktrace.c +++ b/src/unwinder/sentry_unwinder_libbacktrace.c @@ -2,7 +2,7 @@ // XXX: Make into a CMake check // XXX: IBM i PASE offers libbacktrace in libutil, but not available in AIX -#if defined(SENTRY_PLATFORM_DARWIN) || defined(__GLIBC__) || defined(__PASE__) +#if defined(__GLIBC__) || defined(__PASE__) # define HAS_EXECINFO_H #endif @@ -14,15 +14,7 @@ size_t sentry__unwind_stack_libbacktrace( void *addr, const sentry_ucontext_t *uctx, void **ptrs, size_t max_frames) { - if (addr) { -#if defined(SENTRY_PLATFORM_MACOS) && defined(MAC_OS_X_VERSION_10_14) \ - && __has_builtin(__builtin_available) - if (__builtin_available(macOS 10.14, *)) { - return (size_t)backtrace_from_fp(addr, ptrs, (int)max_frames); - } -#endif - return 0; - } else if (uctx) { + if (addr || uctx) { return 0; } #ifdef HAS_EXECINFO_H From 549293a2ef5af17168ad16631eee2715c6a06937 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Fri, 14 Nov 2025 21:26:16 +0100 Subject: [PATCH 66/75] extract fp_walk for the macOS unwinder so we can also provide a stack-trace from an arbitrary frame-pointer --- src/unwinder/sentry_unwinder_libunwind_mac.c | 72 +++++++++----------- 1 file changed, 34 insertions(+), 38 deletions(-) diff --git a/src/unwinder/sentry_unwinder_libunwind_mac.c b/src/unwinder/sentry_unwinder_libunwind_mac.c index 2843b0727..6e87b7d8b 100644 --- a/src/unwinder/sentry_unwinder_libunwind_mac.c +++ b/src/unwinder/sentry_unwinder_libunwind_mac.c @@ -9,7 +9,36 @@ valid_ptr(uintptr_t p) return p && (p % sizeof(uintptr_t) == 0); } -size_t +/** + * This does the same frame-pointer walk for arm64 and x86_64, with the only + * difference being which registers value is used as frame-pointer (fp vs rbp) + */ +static void +fp_walk(uintptr_t fp, size_t *n, void **ptrs, size_t max_frames) +{ + while (*n < max_frames) { + if (!valid_ptr(fp)) { + break; + } + + // arm64 frame record layout: [prev_fp, saved_lr] at fp and fp+8 + // x86_64 frame record layout: [prev_rbp, saved_retaddr] at bp and bp+8 + const uintptr_t *record = (uintptr_t *)fp; + const uintptr_t next_fp = record[0]; + const uintptr_t ret_addr = record[1]; + if (!valid_ptr(next_fp) || !ret_addr) { + break; + } + + ptrs[(*n)++] = (void *)(ret_addr - 1); + if (next_fp <= fp) { + break; // prevent loops + } + fp = next_fp; + } +} + +static size_t fp_walk_from_uctx(const sentry_ucontext_t *uctx, void **ptrs, size_t max_frames) { size_t n = 0; @@ -29,25 +58,7 @@ fp_walk_from_uctx(const sentry_ucontext_t *uctx, void **ptrs, size_t max_frames) ptrs[n++] = (void *)(lr - 1); } - while (n < max_frames) { - if (!valid_ptr(fp)) { - break; - } - - // arm64 frame record layout: [prev_fp, saved_lr] at fp and fp+8 - uintptr_t *record = (uintptr_t *)fp; - uintptr_t next_fp = record[0]; - uintptr_t ret_addr = record[1]; - if (!valid_ptr(next_fp) || !ret_addr) { - break; - } - - ptrs[n++] = (void *)(ret_addr - 1); - if (next_fp <= fp) { - break; // prevent loops - } - fp = next_fp; - } + fp_walk(fp, &n, ptrs, max_frames); #elif defined(__x86_64__) uintptr_t ip = (uintptr_t)mctx->__ss.__rip; uintptr_t bp = (uintptr_t)mctx->__ss.__rbp; @@ -57,23 +68,7 @@ fp_walk_from_uctx(const sentry_ucontext_t *uctx, void **ptrs, size_t max_frames) ptrs[n++] = (void *)(ip - 1); } - while (n < max_frames) { - if (!valid_ptr(bp)) { - break; - } - // x86_64 frame record layout: [prev_rbp, saved_retaddr] at bp and bp+8 - uintptr_t *record = (uintptr_t *)bp; - uintptr_t next_bp = record[0]; - uintptr_t ret_addr = record[1]; - if (!valid_ptr(next_bp) || !ret_addr) { - break; - } - ptrs[n++] = (void *)(ret_addr - 1); - if (next_bp <= bp) { - break; - } - bp = next_bp; - } + fp_walk(bp, &n, ptrs, max_frames); #else # error "Unsupported CPU architecture for macOS stackwalker" #endif @@ -85,7 +80,8 @@ sentry__unwind_stack_libunwind_mac( void *addr, const sentry_ucontext_t *uctx, void **ptrs, size_t max_frames) { if (addr) { - // we don't support stack walks from arbitrary addresses + size_t n = 0; + fp_walk((uintptr_t)addr, &n, ptrs, max_frames); return 0; } From be0b2458c616ca6cf9c21b0a3c99f3ccaf41a112 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Fri, 14 Nov 2025 21:34:24 +0100 Subject: [PATCH 67/75] for targets that must use `backtrace()` as an unwinder we fall back und running the deferred code directly inside the signal handler. Nothing changes for them. --- src/backends/sentry_backend_inproc.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/backends/sentry_backend_inproc.c b/src/backends/sentry_backend_inproc.c index 623c92a48..d911ae7d0 100644 --- a/src/backends/sentry_backend_inproc.c +++ b/src/backends/sentry_backend_inproc.c @@ -882,6 +882,12 @@ static void dispatch_ucontext( const sentry_ucontext_t *uctx, const struct signal_slot *sig_slot) { +#ifdef SENTRY_WITH_UNWINDER_LIBBACKTRACE + // For targets that still use `backtrace()` as the sole unwinder we must + // run the signal-unsafe part in the signal handler like we did before. + process_ucontext_deferred(uctx, sig_slot); + return; +#else if (!sentry__atomic_fetch(&g_handler_thread_ready) || has_handler_thread_crashed()) { // directly execute unsafe part in signal handler as a last chance to @@ -893,7 +899,7 @@ dispatch_ucontext( g_handler_state.uctx = *uctx; g_handler_state.sig_slot = sig_slot; -#ifdef SENTRY_PLATFORM_UNIX +# ifdef SENTRY_PLATFORM_UNIX if (uctx->siginfo) { memcpy(&g_handler_state.siginfo_storage, uctx->siginfo, sizeof(g_handler_state.siginfo_storage)); @@ -913,13 +919,13 @@ dispatch_ucontext( // we leave the handler sentry__leave_signal_handler(); -#endif +# endif sentry__atomic_store(&g_handler_work_done, 0); sentry__atomic_store(&g_handler_has_work, 1); // signal the handler thread to start working -#ifdef SENTRY_PLATFORM_UNIX +# ifdef SENTRY_PLATFORM_UNIX if (g_handler_pipe[1] >= 0) { char c = 1; ssize_t rv; @@ -927,19 +933,21 @@ dispatch_ucontext( rv = write(g_handler_pipe[1], &c, 1); } while (rv == -1 && errno == EINTR); } -#elif defined(SENTRY_PLATFORM_WINDOWS) +# elif defined(SENTRY_PLATFORM_WINDOWS) if (g_handler_semaphore) { ReleaseSemaphore(g_handler_semaphore, 1, NULL); } -#endif +# endif // wait until the handler has done its work while (!sentry__atomic_fetch(&g_handler_work_done)) { sentry__cpu_relax(); } -#ifdef SENTRY_PLATFORM_UNIX +# ifdef SENTRY_PLATFORM_UNIX sentry__enter_signal_handler(); +# endif + #endif } From 1c8dbf9dfe0ecfc75e2cc39323c09faa5fc95769 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Mon, 17 Nov 2025 20:20:32 +0100 Subject: [PATCH 68/75] instead of spinning on atomic in the signal-handler add ACK pipe/semaphore on the return channel and let the OS block and wait. Also check the return value of startup_handler_thread in the initialization and propagate the failure. --- src/backends/sentry_backend_inproc.c | 77 ++++++++++++++++++++++++---- 1 file changed, 67 insertions(+), 10 deletions(-) diff --git a/src/backends/sentry_backend_inproc.c b/src/backends/sentry_backend_inproc.c index d911ae7d0..c51c56fe1 100644 --- a/src/backends/sentry_backend_inproc.c +++ b/src/backends/sentry_backend_inproc.c @@ -56,14 +56,14 @@ static volatile long g_handler_thread_ready = 0; static volatile long g_handler_should_exit = 0; // signal handler tells handler thread to start working static volatile long g_handler_has_work = 0; -// handler thread wakes signal handler from suspension after finishing the work -static volatile long g_handler_work_done = 0; -// trigger/schedule primitives that block the handler thread until we need it +// trigger/schedule primitives that block the other side until this side is done #ifdef SENTRY_PLATFORM_UNIX static int g_handler_pipe[2] = { -1, -1 }; +static int g_handler_ack_pipe[2] = { -1, -1 }; #elif defined(SENTRY_PLATFORM_WINDOWS) static HANDLE g_handler_semaphore = NULL; +static HANDLE g_handler_ack_semaphore = NULL; #endif static int start_handler_thread(void); @@ -137,7 +137,9 @@ startup_inproc_backend( backend->data = &g_backend_config; } - start_handler_thread(); + if (start_handler_thread() != 0) { + return 1; + } // save the old signal handlers memset(g_previous_handlers, 0, sizeof(g_previous_handlers)); @@ -247,7 +249,9 @@ startup_inproc_backend( && defined(SENTRY_THREAD_STACK_GUARANTEE_AUTO_INIT) sentry__set_default_thread_stack_guarantee(); # endif - start_handler_thread(); + if (start_handler_thread() != 0) { + return 1; + } g_previous_handler = SetUnhandledExceptionFilter(&handle_exception); SetErrorMode(SEM_FAILCRITICALERRORS); return 0; @@ -753,7 +757,19 @@ handler_thread_main(void *UNUSED(data)) process_ucontext_deferred( &g_handler_state.uctx, g_handler_state.sig_slot); sentry__atomic_store(&g_handler_has_work, 0); - sentry__atomic_store(&g_handler_work_done, 1); +#ifdef SENTRY_PLATFORM_UNIX + if (g_handler_ack_pipe[1] >= 0) { + char c = 1; + ssize_t rv; + do { + rv = write(g_handler_ack_pipe[1], &c, 1); + } while (rv == -1 && errno == EINTR); + } +#elif defined(SENTRY_PLATFORM_WINDOWS) + if (g_handler_ack_semaphore) { + ReleaseSemaphore(g_handler_ack_semaphore, 1, NULL); + } +#endif } #ifdef SENTRY_PLATFORM_WINDOWS @@ -773,19 +789,33 @@ start_handler_thread(void) sentry__thread_init(&g_handler_thread); sentry__atomic_store(&g_handler_should_exit, 0); sentry__atomic_store(&g_handler_has_work, 0); - sentry__atomic_store(&g_handler_work_done, 0); #ifdef SENTRY_PLATFORM_UNIX if (pipe(g_handler_pipe) != 0) { SENTRY_WARNF("failed to create handler pipe: %s", strerror(errno)); return 1; } + if (pipe(g_handler_ack_pipe) != 0) { + SENTRY_WARNF("failed to create handler ack pipe: %s", strerror(errno)); + close(g_handler_pipe[0]); + close(g_handler_pipe[1]); + g_handler_pipe[0] = -1; + g_handler_pipe[1] = -1; + return 1; + } #elif defined(SENTRY_PLATFORM_WINDOWS) g_handler_semaphore = CreateSemaphoreW(NULL, 0, LONG_MAX, NULL); if (!g_handler_semaphore) { SENTRY_WARN("failed to create handler semaphore"); return 1; } + g_handler_ack_semaphore = CreateSemaphoreW(NULL, 0, LONG_MAX, NULL); + if (!g_handler_ack_semaphore) { + SENTRY_WARN("failed to create handler ack semaphore"); + CloseHandle(g_handler_semaphore); + g_handler_semaphore = NULL; + return 1; + } #endif if (sentry__thread_spawn(&g_handler_thread, handler_thread_main, NULL) @@ -796,9 +826,15 @@ start_handler_thread(void) close(g_handler_pipe[1]); g_handler_pipe[0] = -1; g_handler_pipe[1] = -1; + close(g_handler_ack_pipe[0]); + close(g_handler_ack_pipe[1]); + g_handler_ack_pipe[0] = -1; + g_handler_ack_pipe[1] = -1; #elif defined(SENTRY_PLATFORM_WINDOWS) CloseHandle(g_handler_semaphore); g_handler_semaphore = NULL; + CloseHandle(g_handler_ack_semaphore); + g_handler_ack_semaphore = NULL; #endif return 1; } @@ -841,11 +877,23 @@ stop_handler_thread(void) close(g_handler_pipe[1]); g_handler_pipe[1] = -1; } + if (g_handler_ack_pipe[0] >= 0) { + close(g_handler_ack_pipe[0]); + g_handler_ack_pipe[0] = -1; + } + if (g_handler_ack_pipe[1] >= 0) { + close(g_handler_ack_pipe[1]); + g_handler_ack_pipe[1] = -1; + } #elif defined(SENTRY_PLATFORM_WINDOWS) if (g_handler_semaphore) { CloseHandle(g_handler_semaphore); g_handler_semaphore = NULL; } + if (g_handler_ack_semaphore) { + CloseHandle(g_handler_ack_semaphore); + g_handler_ack_semaphore = NULL; + } #endif sentry__atomic_store(&g_handler_thread_ready, 0); @@ -921,7 +969,6 @@ dispatch_ucontext( sentry__leave_signal_handler(); # endif - sentry__atomic_store(&g_handler_work_done, 0); sentry__atomic_store(&g_handler_has_work, 1); // signal the handler thread to start working @@ -940,9 +987,19 @@ dispatch_ucontext( # endif // wait until the handler has done its work - while (!sentry__atomic_fetch(&g_handler_work_done)) { - sentry__cpu_relax(); +# ifdef SENTRY_PLATFORM_UNIX + if (g_handler_ack_pipe[0] >= 0) { + char c = 0; + ssize_t rv; + do { + rv = read(g_handler_ack_pipe[0], &c, 1); + } while (rv == -1 && errno == EINTR); } +# elif defined(SENTRY_PLATFORM_WINDOWS) + if (g_handler_ack_semaphore) { + WaitForSingleObject(g_handler_ack_semaphore, INFINITE); + } +# endif # ifdef SENTRY_PLATFORM_UNIX sentry__enter_signal_handler(); From 1d85149fa0d46ffd859943c13b76b2a2e6565bbb Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Tue, 18 Nov 2025 00:11:52 +0100 Subject: [PATCH 69/75] actually check the FP against mach_vm_region bounds in the validation --- src/CMakeLists.txt | 3 ++ src/unwinder/sentry_unwinder.h | 8 +++ src/unwinder/sentry_unwinder_libunwind.c | 5 +- src/unwinder/sentry_unwinder_libunwind_mac.c | 51 +++++++++++++++++++- 4 files changed, 61 insertions(+), 6 deletions(-) create mode 100644 src/unwinder/sentry_unwinder.h diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index e212c702c..2aff0b054 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -153,6 +153,9 @@ elseif(SENTRY_BACKEND_NONE) endif() # unwinder +sentry_target_sources_cwd(sentry + unwinder/sentry_unwinder.h +) if(SENTRY_WITH_LIBBACKTRACE) target_compile_definitions(sentry PRIVATE SENTRY_WITH_UNWINDER_LIBBACKTRACE) sentry_target_sources_cwd(sentry diff --git a/src/unwinder/sentry_unwinder.h b/src/unwinder/sentry_unwinder.h new file mode 100644 index 000000000..889d20ca1 --- /dev/null +++ b/src/unwinder/sentry_unwinder.h @@ -0,0 +1,8 @@ +#ifndef SENTRY_UNWINDER_H_INCLUDED +#define SENTRY_UNWINDER_H_INCLUDED + +typedef struct { + uintptr_t lo, hi; +} mem_range_t; + +#endif // SENTRY_UNWINDER_H_INCLUDED diff --git a/src/unwinder/sentry_unwinder_libunwind.c b/src/unwinder/sentry_unwinder_libunwind.c index 563228e9d..27439c20c 100644 --- a/src/unwinder/sentry_unwinder_libunwind.c +++ b/src/unwinder/sentry_unwinder_libunwind.c @@ -1,13 +1,10 @@ #include "sentry_boot.h" #include "sentry_logger.h" +#include "sentry_unwinder.h" #define UNW_LOCAL_ONLY #include #include -typedef struct { - uintptr_t lo, hi; -} mem_range_t; - /** * Looks up the memory range for a given pointer in /proc/self/maps. * `range` is an output parameter. Returns `true` if a range was found. diff --git a/src/unwinder/sentry_unwinder_libunwind_mac.c b/src/unwinder/sentry_unwinder_libunwind_mac.c index 6e87b7d8b..0190821de 100644 --- a/src/unwinder/sentry_unwinder_libunwind_mac.c +++ b/src/unwinder/sentry_unwinder_libunwind_mac.c @@ -1,12 +1,59 @@ #include "sentry_boot.h" #include "sentry_logger.h" +#include "sentry_unwinder.h" #include +#include +#include + +// Basic pointer validation to make sure we stay inside mapped memory. +static bool +is_readable_ptr(uintptr_t p, size_t size) +{ + if (!p) { + return false; + } + + mach_vm_address_t address = (mach_vm_address_t)p; + mach_vm_size_t region_size = 0; + vm_region_basic_info_data_64_t info; + mach_msg_type_number_t count = VM_REGION_BASIC_INFO_COUNT_64; + mach_port_t object = MACH_PORT_NULL; + + kern_return_t kr = mach_vm_region(mach_task_self(), &address, ®ion_size, + VM_REGION_BASIC_INFO_64, (vm_region_info_t)&info, &count, &object); + if (object != MACH_PORT_NULL) { + mach_port_deallocate(mach_task_self(), object); + } + if (kr != KERN_SUCCESS) { + return false; + } + + if (!(info.protection & VM_PROT_READ)) { + return false; + } + + mem_range_t vm_region + = { (uintptr_t)address, (uintptr_t)address + (uintptr_t)region_size }; + if (vm_region.hi < vm_region.lo) { + return false; + } + + uintptr_t end = p + (uintptr_t)size; + if (end < p) { + return false; + } + + return p >= vm_region.lo && end <= vm_region.hi; +} -// a very cheap pointer validation for starters static bool valid_ptr(uintptr_t p) { - return p && (p % sizeof(uintptr_t) == 0); + if (!p || (p % sizeof(uintptr_t)) != 0) { + return false; + } + + return is_readable_ptr(p, sizeof(uintptr_t) * 2); } /** From cd130d37cbc1eb323242f62957d148ac6a43e854 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Tue, 18 Nov 2025 00:23:02 +0100 Subject: [PATCH 70/75] check mach_vm_region bounds only on macOS builds --- src/unwinder/sentry_unwinder_libunwind_mac.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/unwinder/sentry_unwinder_libunwind_mac.c b/src/unwinder/sentry_unwinder_libunwind_mac.c index 0190821de..91ceda367 100644 --- a/src/unwinder/sentry_unwinder_libunwind_mac.c +++ b/src/unwinder/sentry_unwinder_libunwind_mac.c @@ -2,9 +2,13 @@ #include "sentry_logger.h" #include "sentry_unwinder.h" #include -#include -#include +#if defined(SENTRY_PLATFORM_MACOS) +# include +# include +#endif + +#if defined(SENTRY_PLATFORM_MACOS) // Basic pointer validation to make sure we stay inside mapped memory. static bool is_readable_ptr(uintptr_t p, size_t size) @@ -45,6 +49,7 @@ is_readable_ptr(uintptr_t p, size_t size) return p >= vm_region.lo && end <= vm_region.hi; } +#endif static bool valid_ptr(uintptr_t p) @@ -53,7 +58,11 @@ valid_ptr(uintptr_t p) return false; } +#if defined(SENTRY_PLATFORM_MACOS) return is_readable_ptr(p, sizeof(uintptr_t) * 2); +#else + return true; +#endif } /** From 9e9d933bdf8301414defc0148327d77a700e39b7 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Tue, 18 Nov 2025 08:39:44 +0100 Subject: [PATCH 71/75] ensure we conditionally return on acquire in block_for_signal_handler --- src/sentry_sync.c | 4 +++- tests/unit/tests.inc | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/sentry_sync.c b/src/sentry_sync.c index 1b91bf310..299e444a7 100644 --- a/src/sentry_sync.c +++ b/src/sentry_sync.c @@ -517,7 +517,9 @@ sentry__block_for_signal_handler(void) // if there is no signal handler active, we don't need to block // we can spin cheaply, but for the return we must acquire if (!__atomic_load_n(&g_in_signal_handler, __ATOMIC_RELAXED)) { - return __atomic_load_n(&g_in_signal_handler, __ATOMIC_ACQUIRE) == 0; + if (__atomic_load_n(&g_in_signal_handler, __ATOMIC_ACQUIRE) == 0) { + return true; + } } // if we are on the signal handler thread we can also leave diff --git a/tests/unit/tests.inc b/tests/unit/tests.inc index f8fa3c920..d09250663 100644 --- a/tests/unit/tests.inc +++ b/tests/unit/tests.inc @@ -8,7 +8,6 @@ XX(attachments_bytes) XX(attachments_extend) XX(background_worker) XX(basic_consent_tracking) -XX(query_consent_requirement) XX(basic_function_transport) XX(basic_function_transport_transaction) XX(basic_function_transport_transaction_ts) @@ -122,6 +121,7 @@ XX(process_invalid) XX(process_spawn) XX(procmaps_parser) XX(propagation_context_init) +XX(query_consent_requirement) XX(rate_limit_parsing) XX(read_envelope_from_file) XX(read_write_envelope_to_file_null) From 6b6e5457ef9d8e8d819028562503b7d7923baa9b Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Tue, 18 Nov 2025 09:23:58 +0100 Subject: [PATCH 72/75] move chain-first strategy completely outside the signal handler reentrancy guard * up to now, we've been serializing signal handling even though we didn't know whether it was a runtime signal or one we should be handling * this meant that we blocked all our critical sections during a managed exception * it also meant that we blocked any concurrent managed exceptions * it also meant that we introduced a race window during the time when we chained, because incoming signal on other threads would have gotten next in line, before we even completed the current signal handler by moving it completely outside our synchronization we truly chain at start and don't interfere until we know we must. --- src/backends/sentry_backend_inproc.c | 29 ++++++++-------------------- 1 file changed, 8 insertions(+), 21 deletions(-) diff --git a/src/backends/sentry_backend_inproc.c b/src/backends/sentry_backend_inproc.c index c51c56fe1..8a08dad4b 100644 --- a/src/backends/sentry_backend_inproc.c +++ b/src/backends/sentry_backend_inproc.c @@ -1035,14 +1035,6 @@ dispatch_ucontext( static void process_ucontext(const sentry_ucontext_t *uctx) { -#ifdef SENTRY_PLATFORM_UNIX - sentry__enter_signal_handler(); -#endif - - if (!g_backend_config.enable_logging_when_crashed) { - sentry__logger_disable(); - } - #ifdef SENTRY_PLATFORM_LINUX if (g_backend_config.handler_strategy == SENTRY_HANDLER_STRATEGY_CHAIN_AT_START) { @@ -1051,14 +1043,6 @@ process_ucontext(const sentry_ucontext_t *uctx) // cases, we shouldn't react to the signal at all and let their handler // discontinue the signal chain by invoking the runtime handler before // we process the signal. - // there is a good chance that we won't return from the previous - // handler and that would mean we couldn't enter this handler with - // the next signal coming in if we didn't "leave" here. - sentry__leave_signal_handler(); - if (!g_backend_config.enable_logging_when_crashed) { - sentry__logger_enable(); - } - uintptr_t ip = get_instruction_pointer(uctx); uintptr_t sp = get_stack_pointer(uctx); @@ -1077,16 +1061,19 @@ process_ucontext(const sentry_ucontext_t *uctx) return; } - // let's re-enter because it means this was an actual native crash - if (!g_backend_config.enable_logging_when_crashed) { - sentry__logger_disable(); - } - sentry__enter_signal_handler(); // return from runtime handler; continue processing the crash on the // signal thread until the worker takes over } #endif +#ifdef SENTRY_PLATFORM_UNIX + sentry__enter_signal_handler(); +#endif + + if (!g_backend_config.enable_logging_when_crashed) { + sentry__logger_disable(); + } + const struct signal_slot *sig_slot = NULL; for (int i = 0; i < SIGNAL_COUNT; ++i) { #ifdef SENTRY_PLATFORM_UNIX From a011327964940adf9c3958ac06e351f6aa01068a Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Tue, 18 Nov 2025 13:30:12 +0100 Subject: [PATCH 73/75] update changelog --- CHANGELOG.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7eaa1b765..a3d2228cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,12 +2,23 @@ ## Unreleased +**Breaking**: + +- inproc(Linux): the `inproc` backend on Linux now depends on "nognu" `libunwind`. ([#1446](https://github.com/getsentry/sentry-native/pull/1446)) +- inproc: since we split `inproc` into signal-handler/UEF part and a separate handler thread, `before_send` and `on_crash` could be called from other threads than the one that crashed. While this was never part of the contract, if your code relies on this, it will no longer work. ([#1446](https://github.com/getsentry/sentry-native/pull/1446)) + **Features**: - Add custom attributes API for logs. When `logs_with_attributes` is set to `true`, treats the first `varg` passed into `sentry_logs_X(message,...)` as a `sentry_value_t` object of attributes. ([#1435](https://github.com/getsentry/sentry-native/pull/1435)) - Add runtime API to query user consent requirement. ([#1443](https://github.com/getsentry/sentry-native/pull/1443)) - Add logs flush on `sentry_flush()`. ([#1434](https://github.com/getsentry/sentry-native/pull/1434)) +**Fixes**: + +- Make the signal-handler synchronization fully atomic to fix rare race scenarios. ([#1446](https://github.com/getsentry/sentry-native/pull/1446)) +- Reintroduce an FP-based stack-walker for macOS that can start from a user context. This also makes `inproc` backend functional again on macOS 13+. ([#1446](https://github.com/getsentry/sentry-native/pull/1446)) +- Split the `inproc` signal handler (and UEF on Windows) into a safe handler part and an "unsafe" handler thread. This minimizes exposure to undefined behavior inside the signal handler. ([#1446](https://github.com/getsentry/sentry-native/pull/1446)) + ## 0.12.1 **Fixes**: From 4a62c6e6e0585a9d9533f0f17c1b23dce7e2d022 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Thu, 20 Nov 2025 11:51:50 +0100 Subject: [PATCH 74/75] add inproc module-level docs for developers --- src/backends/sentry_backend_inproc.c | 97 +++++++++++++++++++++++++++- 1 file changed, 96 insertions(+), 1 deletion(-) diff --git a/src/backends/sentry_backend_inproc.c b/src/backends/sentry_backend_inproc.c index 8a08dad4b..ec65cf0e1 100644 --- a/src/backends/sentry_backend_inproc.c +++ b/src/backends/sentry_backend_inproc.c @@ -23,8 +23,103 @@ #include #include -#define SIGNAL_DEF(Sig, Desc) { Sig, #Sig, Desc } +/** + * Inproc Backend Introduction + * + * As the name suggests the inproc backend runs the crash handling entirely + * inside the process and thus is the right choice for platforms that + * are limited in process creation/spawning/cloning (or even deploying a + * separate release artifact like with `crashpad`). It is also very lightweight + * in terms of toolchain dependencies because it does not require a C++ standard + * library. + * + * It targets UNIX and Windows (effectively supporting all target platforms of + * the Native SDK) and uses POSIX signal handling on UNIX and unhandled + * exception filters (UEF) on Windows. Whenever a signal handler is mentioned + * in the code or comments, one can replace that with UEF on Windows. + * + * In its current implementation it only gathers the crash context for the + * crashed thread and does not attempt to stop any other threads. While this + * can be considered a downside for some users, it allows additional handlers + * to process the crashed process again, which the other backends currenlty + * can't guarantee to work. Additional crash signals coming from other threads + * will be blocked indefinitely until previous handler takes over. + * + * The inproc backend splits the handler in two parts: + * - a signal handler/unhandled exception filter that severely limits what we + * can do, focusing on response to the OS mechanism and almost zero policy. + * - a separate handler thread that does most of the typical sentry error + * handling and policy implementation, with a bit more freedom. + * + * Only if the handler thread has crashed or is otherwise unavailable, will we + * execute the unsafe part inside the signal-handler itself, as a last chance + * fallback for report creation. The signal-handler part should not use any + * synchronization or signal-unsafe function from `libc` (see function-level + * comment), even access to options is ideally done before during + * initailization. If access to option or scope (or any other global context) + * is required this should happen in the handler thread. + * + * The handler thread is started during backend initialization and will be + * triggered by the signal handler via a POSIX pipe on which the handler thread + * blocks from the start (similarly on Windows, which uses a Semaphore). While + * the handler thread handles a crash, the signal handler (or UEF) blocks itself + * on an ACK pipe/semaphore. Once the handler thread is done processing the + * crash, it will unblock the signal handler which resets the synchronization + * during crash handling and invokes the handler chain. + * + * The most important functions and their meaning: + * + * - `handle_signal`/`handle_exception`: top-level entry points called directly + * from the operating system. They pack sentry_ucontext_t and call... + * - `process_ucontext`: the actual signal-handler/UEF, primarily manages the + * interaction with the OS and other handlers and calls.. + * - `dispatch_ucontext`: this is the place that decides on where to run the + * sentry error event creation and that does the synchronization with... + * - `handler_thread_main`: implements the handler thread loop, blocks until + * unblocked by the signal handler and finally calls... + * - `process_ucontext_deferred`: the implementation of sentry specific + * handler policy leading to crash event construction, it defers to... + * - `make_signal_event`: that is purely about making a crash event object and + * filling the context data + * + * The `on_crash` and `before_send` hook usually run on the handler thread + * during `process_ucontext_deferred` but users cannot rely on any particular + * thread to call their callbacks. However, they can be sure that the crashed + * thread won't run during their the execution of their callback code. + * + * Note on unwinders: + * + * The backend relies on an unwinder that can backtrace from a user context. + * This is important because the unwinder usually runs in the context of the + * handler thread, where a direct backtrace makes no longer any sense (even if + * it was signal safe). We do not dispatch to the handler thread for targets + * that still use `libbacktrace`, and instead run the unsafe part directly in + * the signal handler. This is primarily to not break these target, but in + * general the `libbacktrace`-based unwinder should be considered deprecated. + * + * Notes on signal handling in other runtimes: + * + * The .net runtimes currently rely on signal handling to deliver managed + * exceptions caused from the generated native code. Due to the initialization + * order the inproc backend will receive those signals which it should not + * process. On setups like these it offers a handler strategy that chains the + * previous signal handler first, allowing the .net runtime handler to either + * immediately jump back into runtime code or reset IP/SP so that the returning + * signal handler continues from the managed exception rather then the crashed + * instruction. + * + * The Android runtime (ART) otoh, while also relying heavily on signal handling + * to communicate between generated code and the garbage collector entirely + * shields the signals from us (via `libsigchain` special handler ordering) and + * only forwards signals that are not relevant to runtime. However, it relies on + * each thread having a specific sigaltstack setup, which can lead to crashes if + * overriden. For this reason, we do not set the sigaltstack of any thread if + * one was already configured even if the size is smaller than we'd want. Since + * most of the handler runs in a separate thread the size limitation of any pre- + * configured `sigaltstack` is not a problem to our more complex handler code. + */ +#define SIGNAL_DEF(Sig, Desc) { Sig, #Sig, Desc } #define MAX_FRAMES 128 // the data exchange between the signal handler and the handler thread From 7d6142f4e9d87903831f7b89f0bfb6efd974d714 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Thu, 20 Nov 2025 12:16:53 +0100 Subject: [PATCH 75/75] fix: return the number of frames when doing a stack walk from an FP directly. --- src/unwinder/sentry_unwinder_libunwind_mac.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/unwinder/sentry_unwinder_libunwind_mac.c b/src/unwinder/sentry_unwinder_libunwind_mac.c index 91ceda367..b9a909828 100644 --- a/src/unwinder/sentry_unwinder_libunwind_mac.c +++ b/src/unwinder/sentry_unwinder_libunwind_mac.c @@ -138,7 +138,7 @@ sentry__unwind_stack_libunwind_mac( if (addr) { size_t n = 0; fp_walk((uintptr_t)addr, &n, ptrs, max_frames); - return 0; + return n; } if (uctx) {