Skip to content

Fix libFuzzer not building with pthreads on Windows #109525

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 24, 2024

Conversation

Zentrik
Copy link
Contributor

@Zentrik Zentrik commented Sep 21, 2024

Fixes #106871

@Zentrik
Copy link
Contributor Author

Zentrik commented Sep 21, 2024

@mstorsjo does this look good to you?

@llvmbot
Copy link
Member

llvmbot commented Sep 21, 2024

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: None (Zentrik)

Changes

Fixes #106871


Full diff: https://github.com/llvm/llvm-project/pull/109525.diff

1 Files Affected:

  • (modified) compiler-rt/lib/fuzzer/FuzzerUtilWindows.cpp (+4)
diff --git a/compiler-rt/lib/fuzzer/FuzzerUtilWindows.cpp b/compiler-rt/lib/fuzzer/FuzzerUtilWindows.cpp
index e0210aa0ac3651..ca0a2a9f958caa 100644
--- a/compiler-rt/lib/fuzzer/FuzzerUtilWindows.cpp
+++ b/compiler-rt/lib/fuzzer/FuzzerUtilWindows.cpp
@@ -239,6 +239,9 @@ 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
   typedef HRESULT(WINAPI * proc)(HANDLE, PCWSTR);
   HMODULE kbase = GetModuleHandleA("KernelBase.dll");
   proc ThreadNameProc = reinterpret_cast<proc>(
@@ -253,6 +256,7 @@ void SetThreadName(std::thread &thread, const std::string &name) {
       }
     }
   }
+#endif
 }
 
 } // namespace fuzzer

Copy link

github-actions bot commented Sep 21, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Member

@mstorsjo mstorsjo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, this works for me.

@mstorsjo mstorsjo merged commit b4130be into llvm:main Sep 24, 2024
7 checks passed
@mstorsjo mstorsjo added this to the LLVM 19.X Release milestone Oct 1, 2024
@mstorsjo
Copy link
Member

mstorsjo commented Oct 1, 2024

/cherry-pick b4130be

llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Oct 1, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 1, 2024

/pull-request #110663

giordano pushed a commit to JuliaLang/llvm-project that referenced this pull request Oct 8, 2024
giordano pushed a commit to JuliaLang/llvm-project that referenced this pull request Oct 9, 2024
tru pushed a commit to llvmbot/llvm-project that referenced this pull request Oct 11, 2024
tru pushed a commit to llvmbot/llvm-project that referenced this pull request Oct 11, 2024
@mstorsjo
Copy link
Member

mstorsjo commented Nov 6, 2024

CC @devnexen

This fix breaks with recent changes in libcxx, see #112094 (comment).

I guess we could change this to simply not check things for libcxx - but that also puts us in an awkward position - then we know for certain that this code wouldn't simply build on top of libcxx using pthreads.

Alternatively we could skip the whole setting of thread names in mingw build configurations. As far as I understand, setting the thread name is just a nice-to-have around here. As we can't really confidently know the configuration (without relying on C++ standard library internals) it is safer to simply skip it. (I'm not aware of anybody using libfuzzer in such mingw configurations - Clang doesn't even support enabling the fuzzer sanitizer in mingw configurations anyway.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

libFuzzer does not build on mingw with pthreads
3 participants