From 7ce080cf72c7d7348c306d9b307642b06692693a Mon Sep 17 00:00:00 2001 From: Germano Cavalcante Date: Fri, 8 Nov 2024 23:17:36 +0100 Subject: [PATCH] Refactor: prevent storing dangling pointers The `_EXCEPTION_POINTERS` structure is valid only within the context of the `UnhandledExceptionFilter` function. After the function exits, the memory referenced by the pointer might no longer be valid or the exception information. The solution for this was to create `BLI_system_backtrace_with_os_info` and passing a system-specific data as the second argument. `BLI_system_backtrace` calls `BLI_system_backtrace_with_os_info` with a null second argument internally. Pull Request: https://projects.blender.org/blender/blender/pulls/129999 --- source/blender/blenlib/BLI_system.h | 1 + source/blender/blenlib/intern/system.c | 9 ++++-- source/blender/blenlib/intern/system_win32.cc | 30 +++++++++---------- source/creator/creator_signals.cc | 20 ++++++------- 4 files changed, 32 insertions(+), 28 deletions(-) diff --git a/source/blender/blenlib/BLI_system.h b/source/blender/blenlib/BLI_system.h index 4d78b425500..5bfa7eed02f 100644 --- a/source/blender/blenlib/BLI_system.h +++ b/source/blender/blenlib/BLI_system.h @@ -16,6 +16,7 @@ extern "C" { int BLI_cpu_support_sse2(void); int BLI_cpu_support_sse42(void); +void BLI_system_backtrace_with_os_info(FILE *fp, const void *os_info); void BLI_system_backtrace(FILE *fp); /** Get CPU brand, result is to be MEM_freeN()-ed. */ diff --git a/source/blender/blenlib/intern/system.c b/source/blender/blenlib/intern/system.c index ba4e7a22b4d..7d0c8d6e11e 100644 --- a/source/blender/blenlib/intern/system.c +++ b/source/blender/blenlib/intern/system.c @@ -60,7 +60,7 @@ int BLI_cpu_support_sse2(void) /* Windows stack-walk lives in system_win32.cc */ #if !defined(_MSC_VER) -void BLI_system_backtrace(FILE *fp) +void BLI_system_backtrace_with_os_info(FILE *fp, const void * /*os_info*/) { /* ----------------------- */ /* If system as execinfo.h */ @@ -97,7 +97,12 @@ void BLI_system_backtrace(FILE *fp) # endif } #endif -/* end BLI_system_backtrace */ +/* end BLI_system_backtrace_with_os_info */ + +void BLI_system_backtrace(FILE *fp) +{ + BLI_system_backtrace_with_os_info(fp, NULL); +} /* NOTE: The code for CPU brand string is adopted from Cycles. */ diff --git a/source/blender/blenlib/intern/system_win32.cc b/source/blender/blenlib/intern/system_win32.cc index 4ceb9e1cc97..a718a8db6d7 100644 --- a/source/blender/blenlib/intern/system_win32.cc +++ b/source/blender/blenlib/intern/system_win32.cc @@ -18,8 +18,6 @@ #include "BLI_system.h" /* Own include. */ -static EXCEPTION_POINTERS *current_exception = nullptr; - static const char *bli_windows_get_exception_description(const DWORD exceptioncode) { switch (exceptioncode) { @@ -310,14 +308,14 @@ static void bli_windows_system_backtrace_threads(FILE *fp) CloseHandle(hThreadSnap); } -static bool BLI_windows_system_backtrace_stack(FILE *fp) +static bool bli_windows_system_backtrace_stack(FILE *fp, const EXCEPTION_POINTERS *exception_info) { fprintf(fp, "Stack trace:\n"); /* If we are handling an exception use the context record from that. */ - if (current_exception && current_exception->ExceptionRecord->ExceptionAddress) { + if (exception_info && exception_info->ExceptionRecord->ExceptionAddress) { /* The back trace code will write to the context record, to protect the original record from * modifications give the backtrace a copy to work on. */ - CONTEXT TempContext = *current_exception->ContextRecord; + CONTEXT TempContext = *exception_info->ContextRecord; return BLI_windows_system_backtrace_run_trace(fp, GetCurrentThread(), &TempContext); } else { @@ -383,14 +381,15 @@ static void bli_load_symbols() /** * Write a backtrace into a file for systems which support it. */ -void BLI_system_backtrace(FILE *fp) +void BLI_system_backtrace_with_os_info(FILE *fp, const void *os_info) { + const EXCEPTION_POINTERS *exception_info = static_cast(os_info); SymInitialize(GetCurrentProcess(), nullptr, TRUE); bli_load_symbols(); - if (current_exception) { - bli_windows_system_backtrace_exception_record(fp, current_exception->ExceptionRecord); + if (exception_info) { + bli_windows_system_backtrace_exception_record(fp, exception_info->ExceptionRecord); } - if (BLI_windows_system_backtrace_stack(fp)) { + if (bli_windows_system_backtrace_stack(fp, exception_info)) { /* When the blender symbols are missing the stack traces will be unreliable * so only run if the previous step completed successfully. */ bli_windows_system_backtrace_threads(fp); @@ -400,15 +399,14 @@ void BLI_system_backtrace(FILE *fp) void BLI_windows_handle_exception(void *exception) { - current_exception = static_cast(exception); - if (current_exception) { - fprintf( - stderr, - "Error : %s\n", - bli_windows_get_exception_description(current_exception->ExceptionRecord->ExceptionCode)); + const EXCEPTION_POINTERS *exception_info = static_cast(exception); + if (exception_info) { + fprintf(stderr, + "Error : %s\n", + bli_windows_get_exception_description(exception_info->ExceptionRecord->ExceptionCode)); fflush(stderr); - LPVOID address = current_exception->ExceptionRecord->ExceptionAddress; + LPVOID address = exception_info->ExceptionRecord->ExceptionAddress; fprintf(stderr, "Address : 0x%p\n", address); CHAR modulename[MAX_PATH]; diff --git a/source/creator/creator_signals.cc b/source/creator/creator_signals.cc index bd0690691ac..ede76679ad9 100644 --- a/source/creator/creator_signals.cc +++ b/source/creator/creator_signals.cc @@ -81,13 +81,7 @@ static void sig_handle_blender_esc(int sig) } } -static void sig_handle_crash_backtrace(FILE *fp) -{ - fputs("\n# backtrace\n", fp); - BLI_system_backtrace(fp); -} - -static void sig_handle_crash(int signum) +static void sig_handle_crash_backtrace(int signum, const void *os_info) { /* Might be called after WM/Main exit, so needs to be careful about nullptr-checking before * de-referencing. */ @@ -136,7 +130,8 @@ static void sig_handle_crash(int signum) BKE_report_write_file_fp(fp, &wm->runtime->reports, header); } - sig_handle_crash_backtrace(fp); + fputs("\n# backtrace\n", fp); + BLI_system_backtrace_with_os_info(fp, os_info); # ifdef WITH_PYTHON /* Generate python back-trace if Python is currently active. */ @@ -158,6 +153,11 @@ static void sig_handle_crash(int signum) # endif } +static void sig_handle_crash_fn(int signum) +{ + sig_handle_crash_backtrace(signum, nullptr); +} + # ifdef WIN32 extern LONG WINAPI windows_exception_handler(EXCEPTION_POINTERS *ExceptionInfo) { @@ -177,7 +177,7 @@ extern LONG WINAPI windows_exception_handler(EXCEPTION_POINTERS *ExceptionInfo) } else { BLI_windows_handle_exception(ExceptionInfo); - sig_handle_crash(SIGSEGV); + sig_handle_crash_backtrace(SIGSEGV, ExceptionInfo); } return EXCEPTION_EXECUTE_HANDLER; @@ -197,7 +197,7 @@ void main_signal_setup() SetUnhandledExceptionFilter(windows_exception_handler); # else /* After parsing arguments. */ - signal(SIGSEGV, sig_handle_crash); + signal(SIGSEGV, sig_handle_crash_fn); # endif }