[PATCH] D27179: LibFuzzer - Implement timers for Windows and improve synchronization.

[Marcos Pividori via Phabricator via llvm-commits]

mpividori added inline comments.


================
Comment at: lib/Fuzzer/FuzzerInternal.h:146-147
   std::atomic<size_t> CurrentUnitSize;
+  bool RunningCB;
+  std::mutex RunningCBMtx;
   uint8_t BaseSha1[kSHA1NumBytes];  // Checksum of the base unit.
----------------
mpividori wrote:
> zturner wrote:
> > It seems like we could use `std::atomic_bool` here.  Does that not work for some reason?  The only thing I see that is questionable is that in `FuzzerCallback::CrashCallback` we check the value, and then run multiple lines of code before returning.
> > 
> > But on the other hand, in `Fuzzer::ExecuteCallback`, we always set it to true before executing any callback, and false after.  So maybe there is no race condition?
> > 
> > `atomic_bool` is more lightweight than grabbing a full blown mutex, so if we can be sure it's safe, then it seems better.
> @zturner, thanks for your comments.
> I can't use `std::atomic_bool` because not only do I need to be sure it is modified atomically, buy also I need to synchronize the main thread with the rest of the threads accessing the Fuzzer's data. Because of that I added the mutex `RunningCBMtx`.
> All the information related to the state of the fuzzer is only modified by the main thread, when it is not running the callback function.
> So, in order to consistently access to the Fuzzer's data, we should lock the `RunningCBMtx` and make sure `RunningCB` is true (the main thread is running the CB).
> By locking the `RunningCBMtx`, we can be sure that the main thread won't change its state in `RunningCB`.
> This is used to synchronize the thread which manages the timers, and the one which supervises rss limits, with the main thread.
In the callbacks: `Fuzzer::InterruptCallback()`, `Fuzzer::AlarmCallback()` and `Fuzzer::RssLimitCallback()`, I lock the mutex `RunningCBMtx`, before accessing the Fuzzer's data. I can do that because I am sure that callbacks are executed by a separate thread in Posix and Windows implementations.
For the special case of: `Fuzzer::CrashCallback()`, I can't use the mutex because the callback can be executed asynchronously by the main thread in Posix, where we use signal handlers for crash signals such as: SIGSEGV, SIGILL, etc. So, I do my best and only check for the `RunningCB` flag.


Repository:
  rL LLVM

https://reviews.llvm.org/D27179