From d60ca9aa383a604660d7a8e0bd19d27181cb5afc Mon Sep 17 00:00:00 2001 From: Luca Bacci Date: Mon, 4 Sep 2023 13:57:48 +0200 Subject: [PATCH 1/5] gthread-win32: Register fallback handler for thread name as last Many ASAN implementations install Vectored Exception Handlers that must be the first in the sequence. As we don't really care about ordering, let's place our handler as last. References: - https://developercommunity.visualstudio.com/t/asan-problems-when-manual-exception-handling/1242524 - https://source.chromium.org/chromium/chromium/src/+/main:components/browser_watcher/extended_crash_reporting.cc;l=150;drc=4ced8912 --- glib/gthread-win32.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/glib/gthread-win32.c b/glib/gthread-win32.c index 58e244ebe..afb3e5215 100644 --- a/glib/gthread-win32.c +++ b/glib/gthread-win32.c @@ -681,7 +681,12 @@ g_thread_win32_init (void) InitializeCriticalSection (&g_private_lock); #ifndef _MSC_VER - SetThreadName_VEH_handle = AddVectoredExceptionHandler (1, &SetThreadName_VEH); + /* Set the handler as last to not interfere with ASAN runtimes. + * Many ASAN implementations (currently all three of GCC, CLANG + * and MSVC) install a Vectored Exception Handler that must be + * first in the sequence to work well + */ + SetThreadName_VEH_handle = AddVectoredExceptionHandler (0, &SetThreadName_VEH); if (SetThreadName_VEH_handle == NULL) { /* This is bad, but what can we do? */ From fb3f1733c39bb6ac588ecd5ed6f99dc3b58f62f3 Mon Sep 17 00:00:00 2001 From: Luca Bacci Date: Mon, 4 Sep 2023 16:12:55 +0200 Subject: [PATCH 2/5] gthread-win32: Print critical if we cannot register fallback handler for the thread name exception --- glib/gthread-win32.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/glib/gthread-win32.c b/glib/gthread-win32.c index afb3e5215..e8a79be52 100644 --- a/glib/gthread-win32.c +++ b/glib/gthread-win32.c @@ -688,9 +688,8 @@ g_thread_win32_init (void) */ SetThreadName_VEH_handle = AddVectoredExceptionHandler (0, &SetThreadName_VEH); if (SetThreadName_VEH_handle == NULL) - { - /* This is bad, but what can we do? */ - } + g_critical ("%s failed with error code %u", + "AddVectoredExceptionHandler", (unsigned int) GetLastError ()); #endif } From f4b53cacbfa79eb507c210b757e0e0bced6cd1b9 Mon Sep 17 00:00:00 2001 From: Luca Bacci Date: Mon, 4 Sep 2023 16:14:30 +0200 Subject: [PATCH 3/5] gthread-win32: No need to raise thread name exception if the debugger is not present As outlined in [1], raising the exception without a debugger attached isn't of any use, as the thread name won't be stored anywhere. The system doesn't trap such exception, only debuggers. This also fixes a TOCTTOU issue between the IsDebuggerPresent() check and the RaiseException() call. References: [1] - https://learn.microsoft.com/en-us/visualstudio/debugger/how-to-set-a-thread-name-in-native-code?view=vs-2022#set-a-thread-name-by-throwing-an-exception --- glib/gthread-win32.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/glib/gthread-win32.c b/glib/gthread-win32.c index e8a79be52..be0b4d2d0 100644 --- a/glib/gthread-win32.c +++ b/glib/gthread-win32.c @@ -605,10 +605,7 @@ SetThreadName (DWORD dwThreadID, { } #else - /* Without a debugger we *must* have an exception handler, - * otherwise raising an exception will crash the process. - */ - if ((!IsDebuggerPresent ()) && (SetThreadName_VEH_handle == NULL)) + if ((!IsDebuggerPresent ()) || (SetThreadName_VEH_handle == NULL)) return; RaiseException (EXCEPTION_SET_THREAD_NAME, 0, infosize, (const ULONG_PTR *) &info); From 19d06340cf9599e5eeb713b4af0f25c679b7fc17 Mon Sep 17 00:00:00 2001 From: Luca Bacci Date: Mon, 4 Sep 2023 16:26:54 +0200 Subject: [PATCH 4/5] gthread-win32: Check for specific exception code https://learn.microsoft.com/en-us/cpp/code-quality/c6320?view=msvc-170 --- glib/gthread-win32.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/glib/gthread-win32.c b/glib/gthread-win32.c index be0b4d2d0..7daccaea9 100644 --- a/glib/gthread-win32.c +++ b/glib/gthread-win32.c @@ -601,7 +601,8 @@ SetThreadName (DWORD dwThreadID, RaiseException (EXCEPTION_SET_THREAD_NAME, 0, infosize, (const ULONG_PTR *) &info); } - __except (EXCEPTION_EXECUTE_HANDLER) + __except (GetExceptionCode () == EXCEPTION_SET_THREAD_NAME ? + EXCEPTION_EXECUTE_HANDLER : EXCEPTION_CONTINUE_SEARCH) { } #else From 13c5256fd73560e689227f454586baba01928b44 Mon Sep 17 00:00:00 2001 From: Luca Bacci Date: Tue, 5 Sep 2023 12:33:32 +0200 Subject: [PATCH 5/5] gthread-win32: Fix size argument passed to RaiseException https://learn.microsoft.com/en-us/visualstudio/debugger/how-to-set-a-thread-name-in-native-code?view=vs-2022#set-a-thread-name-by-throwing-an-exception --- glib/gthread-win32.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/glib/gthread-win32.c b/glib/gthread-win32.c index 7daccaea9..cd1cfff87 100644 --- a/glib/gthread-win32.c +++ b/glib/gthread-win32.c @@ -593,7 +593,7 @@ SetThreadName (DWORD dwThreadID, info.dwThreadID = dwThreadID; info.dwFlags = 0; - infosize = sizeof (info) / sizeof (DWORD); + infosize = sizeof (info) / sizeof (ULONG_PTR); #ifdef _MSC_VER __try