[PATCH] D27240: [libFuzzer] Diff 8 - Improve synchronization.

[Marcos Pividori via Phabricator via llvm-commits]

mpividori created this revision.
mpividori added reviewers: kcc, zturner.
mpividori added a subscriber: llvm-commits.
mpividori set the repository for this revision to rL LLVM.
Herald added a subscriber: aemerson.

+ Move the SIGTERM, SIGINT and SIGALRM handlers into a separate thread for Posix implemention (instead of asynchronous signal handler). This is more appropriate for 2 reasons:

- `InterruptCallback()` and `AlarmCallback()` use functions like printf, and access to global data, which is not appropriate for a signal handler. By using a separate thread, we are able to synchronize with the main thread.
- Is the same approach than Windows, which makes code simpler, since we are sure that `InterruptCallback()` and `AlarmCallback()` will be excecuted in a separate thread in both Windows and Posix systems. Under this assumption, we can safely use locks to synchronize that thread with the main thread. (We couldn't do that if the same code could be executed asynchronously by a signal handler in Posix systems and by a separated thread in Windows).

+ I added a new flag `RunningCB` to know if the Fuzzer's main thread is running the CB function, instead of using `! CurrentUnitSize`. `! CurrentUnitSize` doesn't work properly.

  For example, in FuzzerLoop.cpp, line 452, we execute the callback with size 0. Previous implementation failed to detect timeouts in that execution.

+ Add a mutex `RunningCBMtx` to synchronize the access to the Fuzzer's data between different threads.

  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).
  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, both in Posix and Windows implementations.
  For the special case of: `Fuzzer::CrashCallback()`, I can't use the mutex because the callback could 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/D27240

Files:
  lib/Fuzzer/FuzzerInternal.h
  lib/Fuzzer/FuzzerLoop.cpp
  lib/Fuzzer/FuzzerUtilPosix.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D27240.79672.patch
Type: text/x-patch
Size: 5033 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161130/19a1da1e/attachment.bin>