[PATCH] D27238: [libFuzzer] Diff 7 - Improve Signal Handler interface.

[Marcos Pividori via Phabricator via llvm-commits]

mpividori added a comment.

Hi, thanks for your comments. I think the 3 reasons I mentioned before justify this change. At least from my point of view:

1. This way we make explicit which callback function will be registered for a specific signal. In previous implementation, when calling functions like SetSigSegvHandler(), it's not clear which callback will be called for that given signal.
2. With these changes we decouple the implementation of FuzzerUtils from the rest of the Fuzzer's implementation. In fact, you can see I remove the includes: #include "FuzzerInternal.h" from the FuzzerUtil*.cpp implementations.
3. This changes simplify the implementation of Windows's exception handling.

+ I created a new struct `SignalHandlerOptions` to expose an independent interface for the signal handling, independent from the fuzzer implementation. Another option is to pass the Flags struct from Fuzzer's implementation. Then we can analyse the flags: `flags.handle_segv`, `falgs.handle_ill`, etc. (With the disadvantage that we are not decoupling the implementation of FuzzerUtils from the rest of the Fuzzer's implementation (1 and 2), and we are passing far more information that needed...).

+ Another option is to expose this interface:

  SetSigSegvHandler(void (*cb)(void));
  SetSigIll(void (*cb)(void));
  .....

And then we register the callbacks like:

  SetSigSegvHandler(Fuzzer::StaticCrashCallback);

This way we achieve the mentioned points 1) and 2), but not 3)

If you think the reasons I mentioned above are not very important, I can remove this diff. :)

Thanks


Repository:
  rL LLVM

https://reviews.llvm.org/D27238