Changing thread_local to __thread in libFuzzer

[george.karpenkov]

Summary

Old apple compilers do not support thread_local keyword.
__thread is supported, and appears to have the same effect.

[kcc]

Yes, I don't like the change -- I really want to have portable modern C++ w/o any old stuff.
Can you do some cmake magic to add -Dthread_local=__thread for old compilers (and only for them)?

[george.karpenkov]

Detect whether thread_local is supported in CMake, and alternate between thread_local and __thread.
Files built using the built script would always have thread_local.

I understand that you would prefer not to have __thread at all, but I'm not sure whether it is possible, since thread_local is a keyword, not a define, and CMake operates by generating files which are later included by a compiler.

A system modifying the source code and replacing thread_local by __thread could be implemented, but I think it would cause far more trouble.

[kcc]

That's even worse.
Why can't you add -Dthread_local=__thread to compiler flags if the compiler is old?

[george.karpenkov]

Indeed, you are right, my assumption was that keywords could not be changed by the preprocessor, yet surprisingly that works.

[kcc]

LGTM

[devernay]

This does not fix building on older Mac OS X (10.6), because of missing support from the OS:

/opt/local-libc++/var/macports/build/_opt_local-libc++_var_macports_sources_rsync.macports.org_release_tarballs_ports_lang_llvm-5.0/llvm-5.0/work/llvm-5.0.0.src/lib/Fuzzer/FuzzerInternal.h:138:10: error: thread-local storage is not
      supported for the current target
  static thread_local bool IsMyThread;
         ^
<command line>:8:22: note: expanded from here
#define thread_local __thread

A proper fix could use llvm::sys::ThreadLocal< T >

[george.karpenkov]

@devernay do you actually need to use libFuzzer, or do you simply want to be able to build LLVM?
I think I've talked about this issue previously with a person who maintains macports for LLVM, and they've simply used a workaround which does not build libFuzzer.

A proper fix could use llvm::sys::ThreadLocal< T >

No. LibFuzzer can not depend on LLVM.

[devernay]

A more portable (and also less expensive) way to code this is to store the thread id instead of setting a thread_local bool to true, and to compare the current thread id with this id. Is is also perhaps more efficient.

What do you think of it?

lib/Fuzzer/CMakeLists.txt

+include(CheckCXXSourceCompiles)
+
+CHECK_CXX_SOURCE_COMPILES("
+    static thread_local234 int blah;
+    int main() {
+      return 0;
+    }
+    " HAS_THREAD_LOCAL)
+
+if ( NOT HAS_THREAD_LOCAL )
+    set (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Dthread_local=__thread" )
+endif()
+
 if( LLVM_USE_SANITIZE_COVERAGE )
   set(LIBFUZZER_FLAGS_BASE "${CMAKE_CXX_FLAGS}")
 
   # Disable the coverage and sanitizer instrumentation for the fuzzer itself.
   set(CMAKE_CXX_FLAGS "${LIBFUZZER_FLAGS_BASE} -fno-sanitize-coverage=trace-pc-guard,edge,trace-cmp,indirect-calls,8bit-counters -Werror")
 endif()
 
 add_library(LLVMFuzzerNoMainObjects OBJECT