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/FuzzerInternal.h

private:
   system_clock::time_point UnitStartTime, UnitStopTime;
   long TimeOfLongestUnitInSeconds = 0;
   long EpochOfLastReadOfOutputCorpus = 0;
 
   size_t MaxInputLen = 0;
   size_t MaxMutationLen = 0;
 
   // Need to know our own thread.
-  static thread_local bool IsMyThread;
+  static __thread bool IsMyThread;
 };
 
 }; // namespace fuzzer
 
 #endif // LLVM_FUZZER_INTERNAL_H

lib/Fuzzer/FuzzerLoop.cpp

 #undef NO_SANITIZE_MEMORY
 #define NO_SANITIZE_MEMORY __attribute__((no_sanitize_memory))
 #endif
 #endif
 
 namespace fuzzer {
 static const size_t kMaxUnitSizeToPrint = 256;
 
-thread_local bool Fuzzer::IsMyThread;
+__thread bool Fuzzer::IsMyThread;
 
 SharedMemoryRegion SMR;
 
 // Only one Fuzzer per process.
 static Fuzzer *F;
 
 // Leak detection is expensive, so we first check if there were more mallocs
 // than frees (using the sanitizer malloc hooks) and only then try to call lsan.