[PATCH] D27179: LibFuzzer - Implement timers for Windows and improve synchronization.
Marcos Pividori via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Nov 29 11:37:33 PST 2016
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
More information about the llvm-commits
mailing list