[compiler-rt] [fuzzer] Skip trying to set the thread name on MinGW #115167
Conversation
Since b4130be, we check for _LIBCPP_HAS_THREAD_API_PTHREAD to decide between using SetThreadDescription or pthread_setname_np for setting the thread name. c6f3b7b changed how libcxx defines their configuration macros - now they are always defined, but defined to 0 or 1, while they previously were either defined or undefined. As these libcxx defines used to be defined to an empty string (rather than expanding to 1) if enabled, we can't easily produce an expression that works both with older and newer libcxx. Additionally, these defines are libcxx internal config macros that aren't a detail that isn't supported and isn't meant to be relied upon. Simply skip trying to set thread name on MinGW as we can't easily know which kind of thread native handle we have. Setting the thread name is only a nice to have, quality of life improvement - things should work the same even without it. Additionally, libfuzzer isn't generally usable on MinGW targets yet (Clang doesn't include it in the getSupportedSanitizers() method for the MinGW target), so this shouldn't make any difference in practice anyway.
|
@llvm/pr-subscribers-compiler-rt-sanitizer Author: Martin Storsjö (mstorsjo) ChangesSince b4130be, we check for _LIBCPP_HAS_THREAD_API_PTHREAD to decide between using SetThreadDescription or pthread_setname_np for setting the thread name. c6f3b7b changed how libcxx defines their configuration macros - now they are always defined, but defined to 0 or 1, while they previously were either defined or undefined. As these libcxx defines used to be defined to an empty string (rather than expanding to 1) if enabled, we can't easily produce an expression that works both with older and newer libcxx. Additionally, these defines are libcxx internal config macros that aren't a detail that isn't supported and isn't meant to be relied upon. Simply skip trying to set thread name on MinGW as we can't easily know which kind of thread native handle we have. Setting the thread name is only a nice to have, quality of life improvement - things should work the same even without it. Additionally, libfuzzer isn't generally usable on MinGW targets yet (Clang doesn't include it in the getSupportedSanitizers() method for the MinGW target), so this shouldn't make any difference in practice anyway. Full diff: https://github.com/llvm/llvm-project/pull/115167.diff 1 Files Affected:
diff --git a/compiler-rt/lib/fuzzer/FuzzerUtilWindows.cpp b/compiler-rt/lib/fuzzer/FuzzerUtilWindows.cpp
index 37aecae7237ae9..2db2ea98d5c4f9 100644
--- a/compiler-rt/lib/fuzzer/FuzzerUtilWindows.cpp
+++ b/compiler-rt/lib/fuzzer/FuzzerUtilWindows.cpp
@@ -239,10 +239,11 @@ size_t PageSize() {
}
void SetThreadName(std::thread &thread, const std::string &name) {
-#if defined(_LIBCPP_HAS_THREAD_API_PTHREAD) || \
- defined(_GLIBCXX_GCC_GTHR_POSIX_H)
- (void)pthread_setname_np(thread.native_handle(), name.c_str());
-#else
+#ifndef __MINGW32__
+ // Not setting the thread name in MinGW environments. MinGW C++ standard
+ // libraries can either use native Windows threads or pthreads, so we
+ // don't know with certainty what kind of thread handle we're getting
+ // from thread.native_handle() here.
typedef HRESULT(WINAPI * proc)(HANDLE, PCWSTR);
HMODULE kbase = GetModuleHandleA("KernelBase.dll");
proc ThreadNameProc = reinterpret_cast<proc>(
|
|
CC @Zentrik See #112094 (comment) for discussion on how we can't easily check the thread type, and for why we shouldn't be checking libcxx internal macros. |
|
Thank you for fixing this. |
|
/cherry-pick 87f4bc0 |
|
/pull-request #115287 |
|
FYI, based on advice from @ldionne, I did manage to implement a way to properly select automatically between the two kinds of thread handles - see mstorsjo@compiler-rt-thread-name-select. I don't think it's worth merging this for now, though, as this code isn't really used on mingw targets right now anyway, but I'm putting it out there for future reference. |
…lvm#115167) Since b4130be, we check for _LIBCPP_HAS_THREAD_API_PTHREAD to decide between using SetThreadDescription or pthread_setname_np for setting the thread name. c6f3b7b changed how libcxx defines their configuration macros - now they are always defined, but defined to 0 or 1, while they previously were either defined or undefined. As these libcxx defines used to be defined to an empty string (rather than expanding to 1) if enabled, we can't easily produce an expression that works both with older and newer libcxx. Additionally, these defines are libcxx internal config macros that aren't a detail that isn't supported and isn't meant to be relied upon. Simply skip trying to set thread name on MinGW as we can't easily know which kind of thread native handle we have. Setting the thread name is only a nice to have, quality of life improvement - things should work the same even without it. Additionally, libfuzzer isn't generally usable on MinGW targets yet (Clang doesn't include it in the getSupportedSanitizers() method for the MinGW target), so this shouldn't make any difference in practice anyway. (cherry picked from commit 87f4bc0)
…lvm#115167) Since b4130be, we check for _LIBCPP_HAS_THREAD_API_PTHREAD to decide between using SetThreadDescription or pthread_setname_np for setting the thread name. c6f3b7b changed how libcxx defines their configuration macros - now they are always defined, but defined to 0 or 1, while they previously were either defined or undefined. As these libcxx defines used to be defined to an empty string (rather than expanding to 1) if enabled, we can't easily produce an expression that works both with older and newer libcxx. Additionally, these defines are libcxx internal config macros that aren't a detail that isn't supported and isn't meant to be relied upon. Simply skip trying to set thread name on MinGW as we can't easily know which kind of thread native handle we have. Setting the thread name is only a nice to have, quality of life improvement - things should work the same even without it. Additionally, libfuzzer isn't generally usable on MinGW targets yet (Clang doesn't include it in the getSupportedSanitizers() method for the MinGW target), so this shouldn't make any difference in practice anyway. (cherry picked from commit 87f4bc0)
Since b4130be, we check for _LIBCPP_HAS_THREAD_API_PTHREAD to decide between using SetThreadDescription or pthread_setname_np for setting the thread name.
c6f3b7b changed how libcxx defines their configuration macros - now they are always defined, but defined to 0 or 1, while they previously were either defined or undefined.
As these libcxx defines used to be defined to an empty string (rather than expanding to 1) if enabled, we can't easily produce an expression that works both with older and newer libcxx. Additionally, these defines are libcxx internal config macros that aren't a detail that isn't supported and isn't meant to be relied upon.
Simply skip trying to set thread name on MinGW as we can't easily know which kind of thread native handle we have. Setting the thread name is only a nice to have, quality of life improvement - things should work the same even without it.
Additionally, libfuzzer isn't generally usable on MinGW targets yet (Clang doesn't include it in the getSupportedSanitizers() method for the MinGW target), so this shouldn't make any difference in practice anyway.