diff --git a/src/lang/stacktrace/CMakeLists.txt b/src/lang/stacktrace/CMakeLists.txt index b807d7542..421ce9de5 100644 --- a/src/lang/stacktrace/CMakeLists.txt +++ b/src/lang/stacktrace/CMakeLists.txt @@ -6,7 +6,7 @@ if(UNIX) target_link_libraries(sourcemeta_core_stacktrace PRIVATE ${CMAKE_DL_LIBS}) target_link_options(sourcemeta_core_stacktrace INTERFACE "$<$:-rdynamic>" - "$<$:-rdynamic>") + "$<$:-Wl,-export_dynamic>") elseif(WIN32) target_link_libraries(sourcemeta_core_stacktrace PRIVATE dbghelp) endif() diff --git a/src/lang/stacktrace/stacktrace_posix.h b/src/lang/stacktrace/stacktrace_posix.h index 07bb250c9..f949f5115 100644 --- a/src/lang/stacktrace/stacktrace_posix.h +++ b/src/lang/stacktrace/stacktrace_posix.h @@ -86,9 +86,10 @@ auto write_frame(int file_descriptor, int frame_index, void *address) -> void { write_text(file_descriptor, symbol_name); if (resolved != 0 && information.dli_saddr != nullptr) { - const auto offset{static_cast( - reinterpret_cast(address) - - reinterpret_cast(information.dli_saddr))}; + // Subtract as integers. Pointer subtraction across unrelated objects is + // undefined behavior in C++ + const auto offset{reinterpret_cast(address) - + reinterpret_cast(information.dli_saddr)}; write_text(file_descriptor, " +"); write_hex(file_descriptor, offset); } @@ -149,7 +150,7 @@ auto extract_crash_pc(void *context) -> void * { program_counter = user_context->uc_mcontext.pc; #elif defined(__linux__) && defined(__x86_64__) program_counter = - static_cast(user_context->uc_mcontext.gregs[16]); + static_cast(user_context->uc_mcontext.gregs[REG_RIP]); #endif // NOLINTNEXTLINE(performance-no-int-to-ptr) return reinterpret_cast(program_counter); @@ -160,8 +161,22 @@ constexpr const char *separator{"========================================" std::atomic crash_handler_installed{false}; +} // namespace + +// NOTE: `backtrace`, `dladdr`, and `strlen` are not on POSIX's strict +// async-signal-safe list but are reentrant in practice for the synchronous +// faults we care about (null derefs, bad casts, divide-by-zero). We +// deliberately do not demangle. `__cxa_demangle` allocates, which is the one +// operation that would actually risk a deadlock. +// +// Outside any anonymous namespace so `dladdr` reliably resolves the symbol +// name across platforms. `extern "C"` inside an unnamed namespace does NOT +// grant external linkage per C++11 +// NOLINTNEXTLINE(misc-definitions-in-headers) extern "C" __attribute__((visibility("default"))) auto -crash_handler(int signal_number, siginfo_t * /*info*/, void *context) -> void { +sourcemeta_core_stacktrace_crash_handler(int signal_number, + siginfo_t * /*info*/, void *context) + -> void { const int file_descriptor{STDERR_FILENO}; write_text(file_descriptor, "\n"); write_text(file_descriptor, separator); @@ -204,8 +219,6 @@ crash_handler(int signal_number, siginfo_t * /*info*/, void *context) -> void { ::raise(signal_number); } -} // namespace - namespace sourcemeta::core { // NOLINTNEXTLINE(misc-definitions-in-headers) @@ -216,7 +229,7 @@ __attribute__((visibility("default"))) auto stacktrace_on_crash() -> void { } struct sigaction action{}; - action.sa_sigaction = &crash_handler; + action.sa_sigaction = &sourcemeta_core_stacktrace_crash_handler; action.sa_flags = static_cast(SA_SIGINFO | SA_RESETHAND | SA_NODEFER); sigemptyset(&action.sa_mask); diff --git a/src/lang/stacktrace/stacktrace_windows.h b/src/lang/stacktrace/stacktrace_windows.h index 9dc86b95f..ae152725d 100644 --- a/src/lang/stacktrace/stacktrace_windows.h +++ b/src/lang/stacktrace/stacktrace_windows.h @@ -12,6 +12,7 @@ #include // std::snprintf #include // std::strlen #include // _write +#include // std::mutex, std::lock_guard #include // _getpid #pragma comment(lib, "dbghelp.lib") @@ -24,12 +25,25 @@ constexpr const char *separator{"========================================" std::atomic crash_handler_installed{false}; std::atomic symbols_initialized{false}; - +// `dbghelp` is documented as single-threaded by Microsoft, so serialize +// symbol lookups across concurrent `stacktrace()` callers +std::mutex dbghelp_mutex; + +// Double-checked locking. The atomic check fast-paths once init has +// succeeded. The mutex serializes the actual `SymInitialize` call so a +// second thread cannot enter `write_frames` and call `SymFromAddr` while +// init is still in flight on the first thread auto ensure_symbols_initialized() -> void { - bool expected{false}; - if (symbols_initialized.compare_exchange_strong(expected, true)) { - ::SymSetOptions(SYMOPT_UNDNAME | SYMOPT_DEFERRED_LOADS); - ::SymInitialize(::GetCurrentProcess(), nullptr, TRUE); + if (symbols_initialized.load(std::memory_order_acquire)) { + return; + } + const std::lock_guard guard{dbghelp_mutex}; + if (symbols_initialized.load(std::memory_order_relaxed)) { + return; + } + ::SymSetOptions(SYMOPT_UNDNAME | SYMOPT_DEFERRED_LOADS); + if (::SymInitialize(::GetCurrentProcess(), nullptr, TRUE) != FALSE) { + symbols_initialized.store(true, std::memory_order_release); } } @@ -43,6 +57,7 @@ __declspec(noinline) auto write_frames(int file_descriptor, const USHORT captured{ ::CaptureStackBackTrace(frames_to_skip, maximum_frames, frames, nullptr)}; ensure_symbols_initialized(); + const std::lock_guard guard{dbghelp_mutex}; const HANDLE process{::GetCurrentProcess()}; alignas(SYMBOL_INFO) char symbol_buffer[sizeof(SYMBOL_INFO) + 512]{}; @@ -79,7 +94,8 @@ __declspec(noinline) auto write_frames(int file_descriptor, } // namespace extern "C" SOURCEMETA_CORE_STACKTRACE_EXPORT auto WINAPI -crash_handler(EXCEPTION_POINTERS *information) -> LONG { +sourcemeta_core_stacktrace_crash_handler(EXCEPTION_POINTERS *information) + -> LONG { const int file_descriptor{2}; write_text(file_descriptor, "\n"); write_text(file_descriptor, separator); @@ -106,7 +122,7 @@ auto stacktrace_on_crash() -> void { if (!crash_handler_installed.compare_exchange_strong(expected, true)) { return; } - ::SetUnhandledExceptionFilter(&crash_handler); + ::SetUnhandledExceptionFilter(&sourcemeta_core_stacktrace_crash_handler); } // NOLINTNEXTLINE(misc-definitions-in-headers) diff --git a/test/stacktrace/stacktrace_abort_test.sh b/test/stacktrace/stacktrace_abort_test.sh index 42d4cf0a8..fff5b80f3 100755 --- a/test/stacktrace/stacktrace_abort_test.sh +++ b/test/stacktrace/stacktrace_abort_test.sh @@ -9,8 +9,14 @@ STACKTRACE_LIBRARY="$3" SELF="$(basename "$STACKTRACE_ABORT_MAIN")" case "$STACKTRACE_LIBRARY" in - *.a|*.lib) LIBRARY_PATH="$STACKTRACE_ABORT_MAIN" ;; - *) LIBRARY_PATH="$(echo "$STACKTRACE_LIBRARY" | sed -E -e 's|\.so\.[0-9.]+|.so|g' -e 's|\.[0-9.]+\.dylib|.dylib|g')" ;; + *.a|*.lib) + LIBRARY_PATH="$STACKTRACE_ABORT_MAIN" + ;; + *) + LIBRARY_PATH="$(echo "$STACKTRACE_LIBRARY" | sed -E \ + -e 's|\.so\.[0-9.]+|.so|g' \ + -e 's|\.[0-9.]+\.dylib|.dylib|g')" + ;; esac "$STACKTRACE_ABORT_MAIN" > "$WORKDIR/$SELF.actual.txt" 2>&1 \ @@ -25,7 +31,13 @@ sed -E \ -e 's/\+0xADDR/+0xOFFSET/g' \ -e 's/^pid:[[:space:]]+[0-9]+/pid: /' \ -e 's/^#[0-9]+ /# /' \ - -e '/^# /{N;/_sigtramp|__restore_rt|__kernel_rt_sigreturn|__libc_start_main|libsystem_|libdyld|\/dyld|linux-vdso|libc\.so| pthread_kill | gsignal | raise | _?start \+/d;}' \ + -e '/^# /{ + N + /_sigtramp|__restore_rt|__kernel_rt_sigreturn/d + /__libc_start_main| _?start \+/d + /libsystem_|libdyld|\/dyld|linux-vdso|libc\.so/d + / pthread_kill | gsignal | raise /d + }' \ -e 's|\.so\.[0-9.]+|.so|g' \ -e 's|\.[0-9.]+\.dylib|.dylib|g' \ -e '/^Aborted/d' \ @@ -39,7 +51,7 @@ cat << EOF > "$WORKDIR/$SELF.expected.txt" signal: 6 (SIGABRT) pid: -# 0xADDR crash_handler +0xOFFSET +# 0xADDR sourcemeta_core_stacktrace_crash_handler +0xOFFSET in $LIBRARY_PATH # 0xADDR _ZN31sourcemeta_core_stacktrace_test13crash_deepestEv +0xOFFSET in $STACKTRACE_ABORT_MAIN diff --git a/test/stacktrace/stacktrace_on_demand_test.sh b/test/stacktrace/stacktrace_on_demand_test.sh index e078f7e94..62288eea0 100755 --- a/test/stacktrace/stacktrace_on_demand_test.sh +++ b/test/stacktrace/stacktrace_on_demand_test.sh @@ -9,8 +9,14 @@ STACKTRACE_LIBRARY="$3" SELF="$(basename "$STACKTRACE_ON_DEMAND_MAIN")" case "$STACKTRACE_LIBRARY" in - *.a|*.lib) LIBRARY_PATH="$STACKTRACE_ON_DEMAND_MAIN" ;; - *) LIBRARY_PATH="$(echo "$STACKTRACE_LIBRARY" | sed -E -e 's|\.so\.[0-9.]+|.so|g' -e 's|\.[0-9.]+\.dylib|.dylib|g')" ;; + *.a|*.lib) + LIBRARY_PATH="$STACKTRACE_ON_DEMAND_MAIN" + ;; + *) + LIBRARY_PATH="$(echo "$STACKTRACE_LIBRARY" | sed -E \ + -e 's|\.so\.[0-9.]+|.so|g' \ + -e 's|\.[0-9.]+\.dylib|.dylib|g')" + ;; esac "$STACKTRACE_ON_DEMAND_MAIN" > "$WORKDIR/$SELF.actual.txt" 2>&1 \ @@ -25,7 +31,13 @@ sed -E \ -e 's/\+0xADDR/+0xOFFSET/g' \ -e 's/^pid:[[:space:]]+[0-9]+/pid: /' \ -e 's/^#[0-9]+ /# /' \ - -e '/^# /{N;/_sigtramp|__restore_rt|__kernel_rt_sigreturn|__libc_start_main|libsystem_|libdyld|\/dyld|linux-vdso|libc\.so| pthread_kill | gsignal | raise | _?start \+/d;}' \ + -e '/^# /{ + N + /_sigtramp|__restore_rt|__kernel_rt_sigreturn/d + /__libc_start_main| _?start \+/d + /libsystem_|libdyld|\/dyld|linux-vdso|libc\.so/d + / pthread_kill | gsignal | raise /d + }' \ -e 's|\.so\.[0-9.]+|.so|g' \ -e 's|\.[0-9.]+\.dylib|.dylib|g' \ -e '/^Aborted/d' \ diff --git a/test/stacktrace/stacktrace_segfault_test.ps1 b/test/stacktrace/stacktrace_segfault_test.ps1 index 55f9dab7f..49ca078d3 100644 --- a/test/stacktrace/stacktrace_segfault_test.ps1 +++ b/test/stacktrace/stacktrace_segfault_test.ps1 @@ -52,7 +52,7 @@ $ExpectedContent = (@" signal: 0xADDR (SEH) pid: -# 0xADDR crash_handler +0xOFFSET +# 0xADDR sourcemeta_core_stacktrace_crash_handler +0xOFFSET in $LibraryPath # 0xADDR sourcemeta_core_stacktrace_test::crash_deepest +0xOFFSET in $StacktraceSegfaultMain diff --git a/test/stacktrace/stacktrace_segfault_test.sh b/test/stacktrace/stacktrace_segfault_test.sh index ecc1c10fb..70f846676 100755 --- a/test/stacktrace/stacktrace_segfault_test.sh +++ b/test/stacktrace/stacktrace_segfault_test.sh @@ -9,8 +9,14 @@ STACKTRACE_LIBRARY="$3" SELF="$(basename "$STACKTRACE_SEGFAULT_MAIN")" case "$STACKTRACE_LIBRARY" in - *.a|*.lib) LIBRARY_PATH="$STACKTRACE_SEGFAULT_MAIN" ;; - *) LIBRARY_PATH="$(echo "$STACKTRACE_LIBRARY" | sed -E -e 's|\.so\.[0-9.]+|.so|g' -e 's|\.[0-9.]+\.dylib|.dylib|g')" ;; + *.a|*.lib) + LIBRARY_PATH="$STACKTRACE_SEGFAULT_MAIN" + ;; + *) + LIBRARY_PATH="$(echo "$STACKTRACE_LIBRARY" | sed -E \ + -e 's|\.so\.[0-9.]+|.so|g' \ + -e 's|\.[0-9.]+\.dylib|.dylib|g')" + ;; esac "$STACKTRACE_SEGFAULT_MAIN" > "$WORKDIR/$SELF.actual.txt" 2>&1 \ @@ -25,7 +31,13 @@ sed -E \ -e 's/\+0xADDR/+0xOFFSET/g' \ -e 's/^pid:[[:space:]]+[0-9]+/pid: /' \ -e 's/^#[0-9]+ /# /' \ - -e '/^# /{N;/_sigtramp|__restore_rt|__kernel_rt_sigreturn|__libc_start_main|libsystem_|libdyld|\/dyld|linux-vdso|libc\.so| pthread_kill | gsignal | raise | _?start \+/d;}' \ + -e '/^# /{ + N + /_sigtramp|__restore_rt|__kernel_rt_sigreturn/d + /__libc_start_main| _?start \+/d + /libsystem_|libdyld|\/dyld|linux-vdso|libc\.so/d + / pthread_kill | gsignal | raise /d + }' \ -e 's|\.so\.[0-9.]+|.so|g' \ -e 's|\.[0-9.]+\.dylib|.dylib|g' \ -e '/^Aborted/d' \ @@ -41,7 +53,7 @@ pid: # 0xADDR _ZN31sourcemeta_core_stacktrace_test13crash_deepestEv +0xOFFSET in $STACKTRACE_SEGFAULT_MAIN -# 0xADDR crash_handler +0xOFFSET +# 0xADDR sourcemeta_core_stacktrace_crash_handler +0xOFFSET in $LIBRARY_PATH # 0xADDR _ZN31sourcemeta_core_stacktrace_test12crash_middleEv +0xOFFSET in $STACKTRACE_SEGFAULT_MAIN