Changing thread_local to __thread in libFuzzer

Authored by george.karpenkov on Apr 20 2017, 2:53 PM.



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

Diff Detail

kcc added a comment.Apr 20 2017, 2:55 PM

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 edited the summary of this revision. (Show Details)

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 added a comment.Apr 20 2017, 6:29 PM

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

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

kcc accepted this revision.Apr 20 2017, 8:38 PM


This revision is now accepted and ready to land.Apr 20 2017, 8:38 PM
This revision was automatically updated to reflect the committed changes.
devernay added a subscriber: devernay.EditedOct 4 2017, 5:49 AM

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 >

@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.

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?