From 1955ede43bd7ced3d16c6a11fce5a4467c7d8eeb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=A0=D1=83=D1=81=D0=BB=D0=B0=D0=BD=20=D0=98=D0=B6=D0=B1?= =?UTF-8?q?=D1=83=D0=BB=D0=B0=D1=82=D0=BE=D0=B2?= Date: Tue, 6 Apr 2021 07:40:23 +0000 Subject: [PATCH 1/6] W32: Don't use g_getenv() in crash handler or during initialization The first is to avoid any non-trivial code in the crash handler. The second is to avoid the use of quarks and hash tables (brought in by g_getenv()) during GLib initialization. --- glib/gwin32.c | 52 ++++++++++++++++++++++++++++----------------------- 1 file changed, 29 insertions(+), 23 deletions(-) diff --git a/glib/gwin32.c b/glib/gwin32.c index b62f19461..fd3b561c1 100644 --- a/glib/gwin32.c +++ b/glib/gwin32.c @@ -1068,20 +1068,29 @@ static void *WinVEH_handle = NULL; * or for control flow. * * This function deliberately avoids calling any GLib code. + * This is done on purpose. This function can be called when the program + * is in a bad state (crashing). It can also be called very early, as soon + * as the handler is installed. Therefore, it's imperative that + * it does as little as possible. Preferably, all the work that can be + * done in advance (when the program is not crashing yet) should be done + * in advance. And this function certainly does not need to use Unicode. */ static LONG __stdcall g_win32_veh_handler (PEXCEPTION_POINTERS ExceptionInfo) { EXCEPTION_RECORD *er; - char debugger[MAX_PATH + 1]; - WCHAR *debugger_utf16; - const char *debugger_env = NULL; - const char *catch_list; + const gsize debugger_buffer_size = MAX_PATH + 1; + char debugger[debugger_buffer_size]; + char debugger_env[debugger_buffer_size]; + const gsize catch_buffer_size = 1024; + char catch_buffer[catch_buffer_size]; + char *catch_list; gboolean catch = FALSE; - STARTUPINFOW si; + STARTUPINFOA si; PROCESS_INFORMATION pi; HANDLE event; SECURITY_ATTRIBUTES sa; + DWORD flags; if (ExceptionInfo == NULL || ExceptionInfo->ExceptionRecord == NULL || @@ -1097,7 +1106,11 @@ g_win32_veh_handler (PEXCEPTION_POINTERS ExceptionInfo) case EXCEPTION_ILLEGAL_INSTRUCTION: break; default: - catch_list = g_getenv ("G_VEH_CATCH"); + catch_buffer[0] = 0; + if (!GetEnvironmentVariableA ("G_VEH_CATCH", catch_buffer, catch_buffer_size)) + break; + + catch_list = catch_buffer; while (!catch && catch_list != NULL && @@ -1156,9 +1169,8 @@ g_win32_veh_handler (PEXCEPTION_POINTERS ExceptionInfo) fflush (stderr); - debugger_env = g_getenv ("G_DEBUGGER"); - - if (debugger_env == NULL) + debugger_env[0] = 0; + if (!GetEnvironmentVariableA ("G_DEBUGGER", debugger_env, debugger_buffer_size)) return EXCEPTION_CONTINUE_SEARCH; /* Create an inheritable event */ @@ -1180,19 +1192,13 @@ g_win32_veh_handler (PEXCEPTION_POINTERS ExceptionInfo) } debugger[MAX_PATH] = '\0'; - debugger_utf16 = g_utf8_to_utf16 (debugger, -1, NULL, NULL, NULL); + if (GetEnvironmentVariableA ("G_DEBUGGER_OLD_CONSOLE", (char *) &flags, 1)) + flags = 0; + else + flags = CREATE_NEW_CONSOLE; /* Run the debugger */ - if (0 != CreateProcessW (NULL, - debugger_utf16, - NULL, - NULL, - TRUE, - g_getenv ("G_DEBUGGER_OLD_CONSOLE") != NULL ? 0 : CREATE_NEW_CONSOLE, - NULL, - NULL, - &si, - &pi)) + if (0 != CreateProcessA (NULL, debugger, NULL, NULL, TRUE, flags, NULL, NULL, &si, &pi)) { CloseHandle (pi.hProcess); CloseHandle (pi.hThread); @@ -1205,8 +1211,6 @@ g_win32_veh_handler (PEXCEPTION_POINTERS ExceptionInfo) WaitForSingleObject (event, 60000); } - g_free (debugger_utf16); - CloseHandle (event); /* Now the debugger is present, and we can try @@ -1222,6 +1226,8 @@ g_win32_veh_handler (PEXCEPTION_POINTERS ExceptionInfo) void g_crash_handler_win32_init (void) { + char tmp; + if (WinVEH_handle != NULL) return; @@ -1230,7 +1236,7 @@ g_crash_handler_win32_init (void) * break advanced exception handling such as in CLRs like C# or other managed * code. See: https://blogs.msdn.microsoft.com/jmstall/2006/05/24/beware-of-the-vectored-exception-handler-and-managed-code/ */ - if (g_getenv ("G_DEBUGGER") == NULL && g_getenv("G_VEH_CATCH") == NULL) + if (!GetEnvironmentVariableA ("G_DEBUGGER", (char *) &tmp, 1)) return; WinVEH_handle = AddVectoredExceptionHandler (0, &g_win32_veh_handler); From 891e3a0bba8d67c64e247e57b00c71b97fa16197 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=A0=D1=83=D1=81=D0=BB=D0=B0=D0=BD=20=D0=98=D0=B6=D0=B1?= =?UTF-8?q?=D1=83=D0=BB=D0=B0=D1=82=D0=BE=D0=B2?= Date: Tue, 6 Apr 2021 09:35:39 +0000 Subject: [PATCH 2/6] W32: Initialize debugger stuff in advance Since VEH is invoked when an exception occurs (which, for us, is mostly when the program is already crashing), we should try to avoid doing much processing at that point. Since these things (debugger commandline, a list of extra exceptions to catch) are known in advance, set them up during initialization. --- glib/gwin32.c | 155 +++++++++++++++++++++++++++++--------------------- 1 file changed, 90 insertions(+), 65 deletions(-) diff --git a/glib/gwin32.c b/glib/gwin32.c index fd3b561c1..657b18e3e 100644 --- a/glib/gwin32.c +++ b/glib/gwin32.c @@ -1035,7 +1035,17 @@ g_console_win32_init (void) * it will be non-NULL. Only used to later de-install the handler * on library de-initialization. */ -static void *WinVEH_handle = NULL; +static void *WinVEH_handle = NULL; + +#define DEBUGGER_BUFFER_SIZE (MAX_PATH + 1) +/* This is the debugger that we'll run on crash */ +static char debugger[DEBUGGER_BUFFER_SIZE]; + +static gsize number_of_exceptions_to_catch = 0; +static DWORD *exceptions_to_catch = NULL; + +static HANDLE debugger_wakeup_event = 0; +static DWORD debugger_spawn_flags = 0; #include "gwin32-private.c" @@ -1079,18 +1089,9 @@ static LONG __stdcall g_win32_veh_handler (PEXCEPTION_POINTERS ExceptionInfo) { EXCEPTION_RECORD *er; - const gsize debugger_buffer_size = MAX_PATH + 1; - char debugger[debugger_buffer_size]; - char debugger_env[debugger_buffer_size]; - const gsize catch_buffer_size = 1024; - char catch_buffer[catch_buffer_size]; - char *catch_list; - gboolean catch = FALSE; + gsize i; STARTUPINFOA si; PROCESS_INFORMATION pi; - HANDLE event; - SECURITY_ATTRIBUTES sa; - DWORD flags; if (ExceptionInfo == NULL || ExceptionInfo->ExceptionRecord == NULL || @@ -1106,33 +1107,14 @@ g_win32_veh_handler (PEXCEPTION_POINTERS ExceptionInfo) case EXCEPTION_ILLEGAL_INSTRUCTION: break; default: - catch_buffer[0] = 0; - if (!GetEnvironmentVariableA ("G_VEH_CATCH", catch_buffer, catch_buffer_size)) - break; + for (i = 0; i < number_of_exceptions_to_catch; i++) + if (exceptions_to_catch[i] == er->ExceptionCode) + break; - catch_list = catch_buffer; + if (i == number_of_exceptions_to_catch) + return EXCEPTION_CONTINUE_SEARCH; - while (!catch && - catch_list != NULL && - catch_list[0] != 0) - { - unsigned long catch_code; - char *end; - errno = 0; - catch_code = strtoul (catch_list, &end, 16); - if (errno != NO_ERROR) - break; - catch_list = end; - if (catch_list != NULL && catch_list[0] == ',') - catch_list++; - if (catch_code == er->ExceptionCode) - catch = TRUE; - } - - if (catch) - break; - - return EXCEPTION_CONTINUE_SEARCH; + break; } fprintf_s (stderr, @@ -1169,36 +1151,15 @@ g_win32_veh_handler (PEXCEPTION_POINTERS ExceptionInfo) fflush (stderr); - debugger_env[0] = 0; - if (!GetEnvironmentVariableA ("G_DEBUGGER", debugger_env, debugger_buffer_size)) + if (debugger[0] == 0) return EXCEPTION_CONTINUE_SEARCH; - /* Create an inheritable event */ memset (&si, 0, sizeof (si)); memset (&pi, 0, sizeof (pi)); - memset (&sa, 0, sizeof (sa)); si.cb = sizeof (si); - sa.nLength = sizeof (sa); - sa.bInheritHandle = TRUE; - event = CreateEvent (&sa, FALSE, FALSE, NULL); - - /* Put process ID and event handle into debugger commandline */ - if (!_g_win32_subst_pid_and_event (debugger, G_N_ELEMENTS (debugger), - debugger_env, GetCurrentProcessId (), - (guintptr) event)) - { - CloseHandle (event); - return EXCEPTION_CONTINUE_SEARCH; - } - debugger[MAX_PATH] = '\0'; - - if (GetEnvironmentVariableA ("G_DEBUGGER_OLD_CONSOLE", (char *) &flags, 1)) - flags = 0; - else - flags = CREATE_NEW_CONSOLE; /* Run the debugger */ - if (0 != CreateProcessA (NULL, debugger, NULL, NULL, TRUE, flags, NULL, NULL, &si, &pi)) + if (0 != CreateProcessA (NULL, debugger, NULL, NULL, TRUE, debugger_spawn_flags, NULL, NULL, &si, &pi)) { CloseHandle (pi.hProcess); CloseHandle (pi.hThread); @@ -1208,11 +1169,9 @@ g_win32_veh_handler (PEXCEPTION_POINTERS ExceptionInfo) * up forever in case the debugger does not support * event signalling. */ - WaitForSingleObject (event, 60000); + WaitForSingleObject (debugger_wakeup_event, 60000); } - CloseHandle (event); - /* Now the debugger is present, and we can try * resuming execution, re-triggering the exception, * which will be caught by debugger this time around. @@ -1223,10 +1182,41 @@ g_win32_veh_handler (PEXCEPTION_POINTERS ExceptionInfo) return EXCEPTION_CONTINUE_SEARCH; } +static gsize +parse_catch_list (const char *catch_buffer, + DWORD *exceptions, + gsize num_exceptions) +{ + const char *catch_list = catch_buffer; + gsize result = 0; + gsize i = 0; + + while (catch_list != NULL && + catch_list[0] != 0) + { + unsigned long catch_code; + char *end; + errno = 0; + catch_code = strtoul (catch_list, &end, 16); + if (errno != NO_ERROR) + break; + catch_list = end; + if (catch_list != NULL && catch_list[0] == ',') + catch_list++; + if (exceptions && i < num_exceptions) + exceptions[i++] = catch_code; + } + + return result; +} + void g_crash_handler_win32_init (void) { - char tmp; + char debugger_env[DEBUGGER_BUFFER_SIZE]; +#define CATCH_BUFFER_SIZE 1024 + char catch_buffer[CATCH_BUFFER_SIZE]; + SECURITY_ATTRIBUTES sa; if (WinVEH_handle != NULL) return; @@ -1234,11 +1224,46 @@ g_crash_handler_win32_init (void) /* Do not register an exception handler if we're not supposed to catch any * exceptions. Exception handlers are considered dangerous to use, and can * break advanced exception handling such as in CLRs like C# or other managed - * code. See: https://blogs.msdn.microsoft.com/jmstall/2006/05/24/beware-of-the-vectored-exception-handler-and-managed-code/ + * code. See: http://www.windows-tech.info/13/785f590867bd6316.php */ - if (!GetEnvironmentVariableA ("G_DEBUGGER", (char *) &tmp, 1)) + debugger_env[0] = 0; + if (!GetEnvironmentVariableA ("G_DEBUGGER", debugger_env, DEBUGGER_BUFFER_SIZE)) return; + /* Create an inheritable event */ + memset (&sa, 0, sizeof (sa)); + sa.nLength = sizeof (sa); + sa.bInheritHandle = TRUE; + debugger_wakeup_event = CreateEvent (&sa, FALSE, FALSE, NULL); + + /* Put process ID and event handle into debugger commandline */ + if (!_g_win32_subst_pid_and_event (debugger, G_N_ELEMENTS (debugger), + debugger_env, GetCurrentProcessId (), + (guintptr) debugger_wakeup_event)) + { + CloseHandle (debugger_wakeup_event); + debugger_wakeup_event = 0; + debugger[0] = 0; + return; + } + debugger[MAX_PATH] = '\0'; + + catch_buffer[0] = 0; + if (GetEnvironmentVariableA ("G_VEH_CATCH", catch_buffer, CATCH_BUFFER_SIZE)) + { + number_of_exceptions_to_catch = parse_catch_list (catch_buffer, NULL, 0); + if (number_of_exceptions_to_catch > 0) + { + exceptions_to_catch = g_new0 (DWORD, number_of_exceptions_to_catch); + parse_catch_list (catch_buffer, exceptions_to_catch, number_of_exceptions_to_catch); + } + } + + if (GetEnvironmentVariableA ("G_DEBUGGER_OLD_CONSOLE", (char *) &debugger_spawn_flags, 1)) + debugger_spawn_flags = 0; + else + debugger_spawn_flags = CREATE_NEW_CONSOLE; + WinVEH_handle = AddVectoredExceptionHandler (0, &g_win32_veh_handler); } From 6d9c3e3226c8953c90bb72befe6b5d4845e14c98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=A0=D1=83=D1=81=D0=BB=D0=B0=D0=BD=20=D0=98=D0=B6=D0=B1?= =?UTF-8?q?=D1=83=D0=BB=D0=B0=D1=82=D0=BE=D0=B2?= Date: Tue, 6 Apr 2021 11:03:45 +0000 Subject: [PATCH 3/6] W32: Remove allocations from the crash handler Use OutputDebugStringA() instead of fprintf. The goal for this code is to inform the person running the debugger about the exception that caused the debugger to be attached. This is useful for debugging with gdb, because gdb does not catch Windows exception information (it just displays "Segmentation fault"). OutputDebugStringA() ensures that the output goes to the debugger, and the (ab)use of strcpy() with a stack-allocated buffer ensures that we do not allocate anything while the crash handler is running, nor to we call CRT functions that can be reasinably expected to allocate anything. --- glib/gwin32.c | 122 ++++++++++++++++++++++++++++++++++---------------- 1 file changed, 84 insertions(+), 38 deletions(-) diff --git a/glib/gwin32.c b/glib/gwin32.c index 657b18e3e..8d29f376a 100644 --- a/glib/gwin32.c +++ b/glib/gwin32.c @@ -1049,6 +1049,24 @@ static DWORD debugger_spawn_flags = 0; #include "gwin32-private.c" +static char * +copy_chars (char *buffer, + gsize *buffer_size, + const char *to_copy) +{ + gsize copy_count = strlen (to_copy); + if (copy_count <= *buffer_size) + memset (buffer, 0x20, copy_count); + else + memset (buffer, 0x20, *buffer_size); + strncpy_s (buffer, *buffer_size, to_copy, copy_count); + if (*buffer_size >= copy_count) + *buffer_size -= copy_count; + else + *buffer_size = 0; + return &buffer[copy_count]; +} + /* Handles exceptions (useful for debugging). * Issues a DebugBreak() call if the process is being debugged (not really * useful - if the process is being debugged, this handler won't be invoked @@ -1092,10 +1110,17 @@ g_win32_veh_handler (PEXCEPTION_POINTERS ExceptionInfo) gsize i; STARTUPINFOA si; PROCESS_INFORMATION pi; +#define ITOA_BUFFER_SIZE 100 + char itoa_buffer[ITOA_BUFFER_SIZE]; +#define DEBUG_STRING_SIZE 1024 + gsize dbgs = DEBUG_STRING_SIZE; + char debug_string[DEBUG_STRING_SIZE]; + char *dbgp; if (ExceptionInfo == NULL || ExceptionInfo->ExceptionRecord == NULL || - IsDebuggerPresent ()) + IsDebuggerPresent () || + debugger[0] == 0) return EXCEPTION_CONTINUE_SEARCH; er = ExceptionInfo->ExceptionRecord; @@ -1117,43 +1142,6 @@ g_win32_veh_handler (PEXCEPTION_POINTERS ExceptionInfo) break; } - fprintf_s (stderr, - "Exception code=0x%lx flags=0x%lx at 0x%p", - er->ExceptionCode, - er->ExceptionFlags, - er->ExceptionAddress); - - switch (er->ExceptionCode) - { - case EXCEPTION_ACCESS_VIOLATION: - fprintf_s (stderr, - ". Access violation - attempting to %s at address 0x%p\n", - er->ExceptionInformation[0] == 0 ? "read data" : - er->ExceptionInformation[0] == 1 ? "write data" : - er->ExceptionInformation[0] == 8 ? "execute data" : - "do something bad", - (void *) er->ExceptionInformation[1]); - break; - case EXCEPTION_IN_PAGE_ERROR: - fprintf_s (stderr, - ". Page access violation - attempting to %s at address 0x%p with status %Ix\n", - er->ExceptionInformation[0] == 0 ? "read from an inaccessible page" : - er->ExceptionInformation[0] == 1 ? "write to an inaccessible page" : - er->ExceptionInformation[0] == 8 ? "execute data in page" : - "do something bad with a page", - (void *) er->ExceptionInformation[1], - er->ExceptionInformation[2]); - break; - default: - fprintf_s (stderr, "\n"); - break; - } - - fflush (stderr); - - if (debugger[0] == 0) - return EXCEPTION_CONTINUE_SEARCH; - memset (&si, 0, sizeof (si)); memset (&pi, 0, sizeof (pi)); si.cb = sizeof (si); @@ -1170,6 +1158,64 @@ g_win32_veh_handler (PEXCEPTION_POINTERS ExceptionInfo) * event signalling. */ WaitForSingleObject (debugger_wakeup_event, 60000); + + dbgp = &debug_string[0]; + + dbgp = copy_chars (dbgp, &dbgs, "Exception code=0x"); + itoa_buffer[0] = 0; + _ui64toa_s (er->ExceptionCode, itoa_buffer, ITOA_BUFFER_SIZE, 16); + dbgp = copy_chars (dbgp, &dbgs, itoa_buffer); + dbgp = copy_chars (dbgp, &dbgs, " flags=0x"); + itoa_buffer[0] = 0; + _ui64toa_s (er->ExceptionFlags, itoa_buffer, ITOA_BUFFER_SIZE, 16); + dbgp = copy_chars (dbgp, &dbgs, itoa_buffer); + dbgp = copy_chars (dbgp, &dbgs, " at 0x"); + itoa_buffer[0] = 0; + _ui64toa_s ((guintptr) er->ExceptionAddress, itoa_buffer, ITOA_BUFFER_SIZE, 16); + dbgp = copy_chars (dbgp, &dbgs, itoa_buffer); + + switch (er->ExceptionCode) + { + case EXCEPTION_ACCESS_VIOLATION: + dbgp = copy_chars (dbgp, &dbgs, ". Access violation - attempting to "); + if (er->ExceptionInformation[0] == 0) + dbgp = copy_chars (dbgp, &dbgs, "read data"); + else if (er->ExceptionInformation[0] == 1) + dbgp = copy_chars (dbgp, &dbgs, "write data"); + else if (er->ExceptionInformation[0] == 8) + dbgp = copy_chars (dbgp, &dbgs, "execute data"); + else + dbgp = copy_chars (dbgp, &dbgs, "do something bad"); + dbgp = copy_chars (dbgp, &dbgs, " at address 0x"); + itoa_buffer[0] = 0; + _ui64toa_s (er->ExceptionInformation[1], itoa_buffer, ITOA_BUFFER_SIZE, 16); + dbgp = copy_chars (dbgp, &dbgs, itoa_buffer); + break; + case EXCEPTION_IN_PAGE_ERROR: + dbgp = copy_chars (dbgp, &dbgs, ". Page access violation - attempting to "); + if (er->ExceptionInformation[0] == 0) + dbgp = copy_chars (dbgp, &dbgs, "read from an inaccessible page"); + else if (er->ExceptionInformation[0] == 1) + dbgp = copy_chars (dbgp, &dbgs, "write to an inaccessible page"); + else if (er->ExceptionInformation[0] == 8) + dbgp = copy_chars (dbgp, &dbgs, "execute data in page"); + else + dbgp = copy_chars (dbgp, &dbgs, "do something bad with a page"); + dbgp = copy_chars (dbgp, &dbgs, " at address 0x"); + itoa_buffer[0] = 0; + _ui64toa_s (er->ExceptionInformation[1], itoa_buffer, ITOA_BUFFER_SIZE, 16); + dbgp = copy_chars (dbgp, &dbgs, itoa_buffer); + dbgp = copy_chars (dbgp, &dbgs, " with status "); + itoa_buffer[0] = 0; + _ui64toa_s (er->ExceptionInformation[2], itoa_buffer, ITOA_BUFFER_SIZE, 16); + dbgp = copy_chars (dbgp, &dbgs, itoa_buffer); + break; + default: + break; + } + + dbgp = copy_chars (dbgp, &dbgs, "\n"); + OutputDebugStringA (debug_string); } /* Now the debugger is present, and we can try From 5c187b9385a6681b4d6b3bdd214de828f252f73a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=A0=D1=83=D1=81=D0=BB=D0=B0=D0=BD=20=D0=98=D0=B6=D0=B1?= =?UTF-8?q?=D1=83=D0=BB=D0=B0=D1=82=D0=BE=D0=B2?= Date: Sat, 24 Apr 2021 20:50:10 +0000 Subject: [PATCH 4/6] Convert the crash handler to UTF-16, mostly --- glib/gwin32-private.c | 30 ++++++++++++++--------------- glib/gwin32.c | 44 +++++++++++++++++++++---------------------- 2 files changed, 37 insertions(+), 37 deletions(-) diff --git a/glib/gwin32-private.c b/glib/gwin32-private.c index f7913b553..c28e92baa 100644 --- a/glib/gwin32-private.c +++ b/glib/gwin32-private.c @@ -18,16 +18,16 @@ /* Copy @cmdline into @debugger, and substitute @pid for `%p` * and @event for `%e`. - * If @debugger_size (in bytes) is overflowed, return %FALSE. + * If @debugger_size (in wchar_ts) is overflowed, return %FALSE. * Also returns %FALSE when `%` is followed by anything other * than `e` or `p`. */ static gboolean -_g_win32_subst_pid_and_event (char *debugger, - gsize debugger_size, - const char *cmdline, - DWORD pid, - guintptr event) +_g_win32_subst_pid_and_event_w (wchar_t *debugger, + gsize debugger_size, + const wchar_t *cmdline, + DWORD pid, + guintptr event) { gsize i = 0, dbg_i = 0; /* These are integers, and they can't be longer than 20 characters @@ -35,31 +35,31 @@ _g_win32_subst_pid_and_event (char *debugger, * Use 30 just to be sure. */ #define STR_BUFFER_SIZE 30 - char pid_str[STR_BUFFER_SIZE] = {0}; + wchar_t pid_str[STR_BUFFER_SIZE] = {0}; gsize pid_str_len; - char event_str[STR_BUFFER_SIZE] = {0}; + wchar_t event_str[STR_BUFFER_SIZE] = {0}; gsize event_str_len; - _snprintf_s (pid_str, STR_BUFFER_SIZE, G_N_ELEMENTS (pid_str), "%lu", pid); + _snwprintf_s (pid_str, STR_BUFFER_SIZE, G_N_ELEMENTS (pid_str), L"%lu", pid); pid_str[G_N_ELEMENTS (pid_str) - 1] = 0; - pid_str_len = strlen (pid_str); - _snprintf_s (event_str, STR_BUFFER_SIZE, G_N_ELEMENTS (pid_str), "%Iu", event); + pid_str_len = wcslen (pid_str); + _snwprintf_s (event_str, STR_BUFFER_SIZE, G_N_ELEMENTS (pid_str), L"%Iu", event); event_str[G_N_ELEMENTS (pid_str) - 1] = 0; - event_str_len = strlen (event_str); + event_str_len = wcslen (event_str); #undef STR_BUFFER_SIZE while (cmdline[i] != 0 && dbg_i < debugger_size) { - if (cmdline[i] != '%') + if (cmdline[i] != L'%') debugger[dbg_i++] = cmdline[i++]; - else if (cmdline[i + 1] == 'p') + else if (cmdline[i + 1] == L'p') { gsize j = 0; while (j < pid_str_len && dbg_i < debugger_size) debugger[dbg_i++] = pid_str[j++]; i += 2; } - else if (cmdline[i + 1] == 'e') + else if (cmdline[i + 1] == L'e') { gsize j = 0; while (j < event_str_len && dbg_i < debugger_size) diff --git a/glib/gwin32.c b/glib/gwin32.c index 8d29f376a..c710ec603 100644 --- a/glib/gwin32.c +++ b/glib/gwin32.c @@ -1039,7 +1039,7 @@ static void *WinVEH_handle = NULL; #define DEBUGGER_BUFFER_SIZE (MAX_PATH + 1) /* This is the debugger that we'll run on crash */ -static char debugger[DEBUGGER_BUFFER_SIZE]; +static wchar_t debugger[DEBUGGER_BUFFER_SIZE]; static gsize number_of_exceptions_to_catch = 0; static DWORD *exceptions_to_catch = NULL; @@ -1101,14 +1101,14 @@ copy_chars (char *buffer, * as the handler is installed. Therefore, it's imperative that * it does as little as possible. Preferably, all the work that can be * done in advance (when the program is not crashing yet) should be done - * in advance. And this function certainly does not need to use Unicode. + * in advance. */ static LONG __stdcall g_win32_veh_handler (PEXCEPTION_POINTERS ExceptionInfo) { EXCEPTION_RECORD *er; gsize i; - STARTUPINFOA si; + STARTUPINFOW si; PROCESS_INFORMATION pi; #define ITOA_BUFFER_SIZE 100 char itoa_buffer[ITOA_BUFFER_SIZE]; @@ -1147,7 +1147,7 @@ g_win32_veh_handler (PEXCEPTION_POINTERS ExceptionInfo) si.cb = sizeof (si); /* Run the debugger */ - if (0 != CreateProcessA (NULL, debugger, NULL, NULL, TRUE, debugger_spawn_flags, NULL, NULL, &si, &pi)) + if (0 != CreateProcessW (NULL, debugger, NULL, NULL, TRUE, debugger_spawn_flags, NULL, NULL, &si, &pi)) { CloseHandle (pi.hProcess); CloseHandle (pi.hThread); @@ -1229,25 +1229,25 @@ g_win32_veh_handler (PEXCEPTION_POINTERS ExceptionInfo) } static gsize -parse_catch_list (const char *catch_buffer, - DWORD *exceptions, - gsize num_exceptions) +parse_catch_list (const wchar_t *catch_buffer, + DWORD *exceptions, + gsize num_exceptions) { - const char *catch_list = catch_buffer; - gsize result = 0; - gsize i = 0; + const wchar_t *catch_list = catch_buffer; + gsize result = 0; + gsize i = 0; while (catch_list != NULL && catch_list[0] != 0) { unsigned long catch_code; - char *end; + wchar_t *end; errno = 0; - catch_code = strtoul (catch_list, &end, 16); + catch_code = wcstoul (catch_list, &end, 16); if (errno != NO_ERROR) break; catch_list = end; - if (catch_list != NULL && catch_list[0] == ',') + if (catch_list != NULL && catch_list[0] == L',') catch_list++; if (exceptions && i < num_exceptions) exceptions[i++] = catch_code; @@ -1259,9 +1259,9 @@ parse_catch_list (const char *catch_buffer, void g_crash_handler_win32_init (void) { - char debugger_env[DEBUGGER_BUFFER_SIZE]; + wchar_t debugger_env[DEBUGGER_BUFFER_SIZE]; #define CATCH_BUFFER_SIZE 1024 - char catch_buffer[CATCH_BUFFER_SIZE]; + wchar_t catch_buffer[CATCH_BUFFER_SIZE]; SECURITY_ATTRIBUTES sa; if (WinVEH_handle != NULL) @@ -1273,7 +1273,7 @@ g_crash_handler_win32_init (void) * code. See: http://www.windows-tech.info/13/785f590867bd6316.php */ debugger_env[0] = 0; - if (!GetEnvironmentVariableA ("G_DEBUGGER", debugger_env, DEBUGGER_BUFFER_SIZE)) + if (!GetEnvironmentVariableW (L"G_DEBUGGER", debugger_env, DEBUGGER_BUFFER_SIZE)) return; /* Create an inheritable event */ @@ -1283,19 +1283,19 @@ g_crash_handler_win32_init (void) debugger_wakeup_event = CreateEvent (&sa, FALSE, FALSE, NULL); /* Put process ID and event handle into debugger commandline */ - if (!_g_win32_subst_pid_and_event (debugger, G_N_ELEMENTS (debugger), - debugger_env, GetCurrentProcessId (), - (guintptr) debugger_wakeup_event)) + if (!_g_win32_subst_pid_and_event_w (debugger, G_N_ELEMENTS (debugger), + debugger_env, GetCurrentProcessId (), + (guintptr) debugger_wakeup_event)) { CloseHandle (debugger_wakeup_event); debugger_wakeup_event = 0; debugger[0] = 0; return; } - debugger[MAX_PATH] = '\0'; + debugger[MAX_PATH] = L'\0'; catch_buffer[0] = 0; - if (GetEnvironmentVariableA ("G_VEH_CATCH", catch_buffer, CATCH_BUFFER_SIZE)) + if (GetEnvironmentVariableW (L"G_VEH_CATCH", catch_buffer, CATCH_BUFFER_SIZE)) { number_of_exceptions_to_catch = parse_catch_list (catch_buffer, NULL, 0); if (number_of_exceptions_to_catch > 0) @@ -1305,7 +1305,7 @@ g_crash_handler_win32_init (void) } } - if (GetEnvironmentVariableA ("G_DEBUGGER_OLD_CONSOLE", (char *) &debugger_spawn_flags, 1)) + if (GetEnvironmentVariableW (L"G_DEBUGGER_OLD_CONSOLE", (wchar_t *) &debugger_spawn_flags, 1)) debugger_spawn_flags = 0; else debugger_spawn_flags = CREATE_NEW_CONSOLE; From fbd7a37e1a126bdfe638048bf5dcbd718e645fd7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=A0=D1=83=D1=81=D0=BB=D0=B0=D0=BD=20=D0=98=D0=B6=D0=B1?= =?UTF-8?q?=D1=83=D0=BB=D0=B0=D1=82=D0=BE=D0=B2?= Date: Sat, 24 Apr 2021 20:58:37 +0000 Subject: [PATCH 5/6] Test the wchar_t version of pid-event subst routine Also move env setup earlier in the test, to ensure that the child gets the envvars during initialization. Also, don't look for exception codes in stderr, since OutputDebugStringA() doesn't dump stuff there. --- glib/tests/win32.c | 60 ++++++++++++++++++++++++---------------------- 1 file changed, 32 insertions(+), 28 deletions(-) diff --git a/glib/tests/win32.c b/glib/tests/win32.c index 97e5887cf..121997311 100644 --- a/glib/tests/win32.c +++ b/glib/tests/win32.c @@ -33,34 +33,40 @@ static char *argv0 = NULL; static void test_subst_pid_and_event (void) { - const gchar not_enough[] = "too long when %e and %p are substituted"; - gchar debugger_3[3]; - gchar debugger_not_enough[G_N_ELEMENTS (not_enough)]; - gchar debugger_enough[G_N_ELEMENTS (not_enough) + 1]; - gchar debugger_big[65535] = {0}; + const wchar_t not_enough[] = L"too long when %e and %p are substituted"; + wchar_t debugger_3[3]; + wchar_t debugger_not_enough[G_N_ELEMENTS (not_enough)]; + wchar_t debugger_enough[G_N_ELEMENTS (not_enough) + 1]; + char *debugger_enough_utf8; + wchar_t debugger_big[65535] = {0}; + char *debugger_big_utf8; gchar *output; guintptr be = (guintptr) 0xFFFFFFFF; DWORD bp = G_MAXSIZE; /* %f is not valid */ - g_assert_false (_g_win32_subst_pid_and_event (debugger_3, G_N_ELEMENTS (debugger_3), - "%f", 0, 0)); + g_assert_false (_g_win32_subst_pid_and_event_w (debugger_3, G_N_ELEMENTS (debugger_3), + L"%f", 0, 0)); - g_assert_false (_g_win32_subst_pid_and_event (debugger_3, G_N_ELEMENTS (debugger_3), - "string longer than 10", 0, 0)); + g_assert_false (_g_win32_subst_pid_and_event_w (debugger_3, G_N_ELEMENTS (debugger_3), + L"string longer than 10", 0, 0)); /* 200 is longer than %e, so the string doesn't fit by 1 byte */ - g_assert_false (_g_win32_subst_pid_and_event (debugger_not_enough, G_N_ELEMENTS (debugger_not_enough), - "too long when %e and %p are substituted", 10, 200)); + g_assert_false (_g_win32_subst_pid_and_event_w (debugger_not_enough, G_N_ELEMENTS (debugger_not_enough), + not_enough, 10, 200)); /* This should fit */ - g_assert_true (_g_win32_subst_pid_and_event (debugger_enough, G_N_ELEMENTS (debugger_enough), - "too long when %e and %p are substituted", 10, 200)); - g_assert_cmpstr (debugger_enough, ==, "too long when 200 and 10 are substituted"); + g_assert_true (_g_win32_subst_pid_and_event_w (debugger_enough, G_N_ELEMENTS (debugger_enough), + not_enough, 10, 200)); + debugger_enough_utf8 = g_utf16_to_utf8 (debugger_enough, -1, NULL, NULL, NULL); + g_assert_cmpstr (debugger_enough_utf8, ==, "too long when 200 and 10 are substituted"); + g_free (debugger_enough_utf8); - g_assert_true (_g_win32_subst_pid_and_event (debugger_big, G_N_ELEMENTS (debugger_big), - "multipl%e big %e %entries and %pids are %provided here", bp, be)); + g_assert_true (_g_win32_subst_pid_and_event_w (debugger_big, G_N_ELEMENTS (debugger_big), + L"multipl%e big %e %entries and %pids are %provided here", bp, be)); + debugger_big_utf8 = g_utf16_to_utf8 (debugger_big, -1, NULL, NULL, NULL); output = g_strdup_printf ("multipl%llu big %llu %lluntries and %luids are %lurovided here", (guint64) be, (guint64) be, (guint64) be, bp, bp); - g_assert_cmpstr (debugger_big, ==, output); + g_assert_cmpstr (debugger_big_utf8, ==, output); + g_free (debugger_big_utf8); g_free (output); } @@ -95,7 +101,6 @@ test_veh_crash_access_violation (void) /* Run a test that crashes */ g_test_trap_subprocess ("/win32/subprocess/access_violation", 0, 0); g_test_trap_assert_failed (); - g_test_trap_assert_stderr ("Exception code=0xc0000005*"); } static void @@ -105,20 +110,10 @@ test_veh_crash_illegal_instruction (void) /* Run a test that crashes */ g_test_trap_subprocess ("/win32/subprocess/illegal_instruction", 0, 0); g_test_trap_assert_failed (); - g_test_trap_assert_stderr ("Exception code=0xc000001d*"); } static void test_veh_debug (void) -{ - /* Run a test that crashes and runs a debugger */ - g_test_trap_subprocess ("/win32/subprocess/debuggee", 0, 0); - g_test_trap_assert_failed (); - g_test_trap_assert_stderr ("Exception code=0xc0000005*Debugger invoked, attaching to*"); -} - -static void -test_veh_debuggee (void) { /* Set up a debugger to be run on crash */ gchar *command = g_strdup_printf ("%s %s", argv0, "%p %e"); @@ -129,6 +124,15 @@ test_veh_debuggee (void) */ g_setenv ("G_DEBUGGER_OLD_CONSOLE", "1", TRUE); g_free (command); + /* Run a test that crashes and runs a debugger */ + g_test_trap_subprocess ("/win32/subprocess/debuggee", 0, 0); + g_test_trap_assert_failed (); + g_test_trap_assert_stderr ("Debugger invoked, attaching to*"); +} + +static void +test_veh_debuggee (void) +{ /* Crash */ test_access_violation (); } From 0908e6a8e7370be6ebb8caf79d8bfe9920cd8c11 Mon Sep 17 00:00:00 2001 From: LRN Date: Sat, 15 May 2021 04:21:37 +0000 Subject: [PATCH 6/6] Fix the math in copy_chars Now we end up returning a pointer to the end of the buffer after we run out of space. On subsequent calls copy_count will end up being 0. --- glib/gwin32.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/glib/gwin32.c b/glib/gwin32.c index c710ec603..f4590916f 100644 --- a/glib/gwin32.c +++ b/glib/gwin32.c @@ -1054,16 +1054,10 @@ copy_chars (char *buffer, gsize *buffer_size, const char *to_copy) { - gsize copy_count = strlen (to_copy); - if (copy_count <= *buffer_size) - memset (buffer, 0x20, copy_count); - else - memset (buffer, 0x20, *buffer_size); - strncpy_s (buffer, *buffer_size, to_copy, copy_count); - if (*buffer_size >= copy_count) - *buffer_size -= copy_count; - else - *buffer_size = 0; + gsize copy_count = MIN (strlen (to_copy), *buffer_size - 1); + memset (buffer, 0x20, copy_count); + strncpy_s (buffer, *buffer_size, to_copy, _TRUNCATE); + *buffer_size -= copy_count; return &buffer[copy_count]; }