[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/FuzzerUtil.h:47-54
+  HandlerT sigalrm_cb;
+  HandlerT sigsegv_cb;
+  HandlerT sigbus_cb;
+  HandlerT sigabrt_cb;
+  HandlerT sigill_cb;
+  HandlerT sigfpe_cb;
+  HandlerT sigint_cb;
----------------
zturner wrote:
> How about getting rid of the constructor and initializing all these inline?
> 
> ```
> int timeout = 0;
> HandlerT sigalrm_cb = nullptr;
> // etc
> ```
@zturner , I think using a constructor is clearer. Do you mean to avoid the static constructor? In that case, I can remove it and initialize that members inline inside `SetSignalHandler()`.


================
Comment at: lib/Fuzzer/FuzzerUtilWindows.cpp:30
 
-LONG WINAPI SEGVHandler(PEXCEPTION_POINTERS ExceptionInfo) {
+static SignalHandlerOptions HandlerOpt;
+
----------------
zturner wrote:
> @kcc: Are there any issues with using static global constructors in libfuzzer?
@zturner , the Code Standards mentions that we should avoid static constructors: [[1]](http://llvm.org/docs/CodingStandards.html#do-not-use-static-constructors). In this case, I included it because it is a very simple constructor. I can remove it and initialize that struct inside `SetSignalHandler()`.


================
Comment at: lib/Fuzzer/FuzzerUtilWindows.cpp:89-90
+  ~TimerQ() {
+    if (TimerQueue)
+      DeleteTimerQueueEx(TimerQueue, NULL);
+  };
----------------
zturner wrote:
> Shouldn't use all delete the timer as well?  You store it locally in `SetTimer` but then let the handle leak.  So every time we call `SetTimer` it will leak a handle.
According to the documentation, the timers which were added to the `TimerQueue` are automatically deleted when deleting the `TimerQueue`.


Repository:
  rL LLVM

https://reviews.llvm.org/D27179