This is an archive of the discontinued LLVM Phabricator instance.

[libFuzzer] AlrmHandler is executed in a different thread for Windows.
ClosedPublic

Authored by mpividori on Jan 13 2017, 8:00 PM.

Details

Summary

I don't check for InFuzzingThread() in Windows, since the AlarmHandler() is executed by a thread from a thread pool.
I continue thinking that it would be better to take the same approach for both Windows and Linux and execute that callbacks in a different thread in Linux too, instead of relying on asynchronous execution (as mentioned in https://reviews.llvm.org/D27240). We would avoid this kind of problems.
Thanks,

Diff Detail

Repository
rL LLVM

Event Timeline

mpividori updated this revision to Diff 84426.Jan 13 2017, 8:00 PM
mpividori retitled this revision from to [libFuzzer] AlrmHandler is executed in a different thread for Windows..
mpividori updated this object.
mpividori added reviewers: kcc, zturner.
mpividori set the repository for this revision to rL LLVM.
mpividori added a subscriber: llvm-commits.
zturner edited edge metadata.Jan 17 2017, 10:05 AM

Is the code already re-entrant for Windows so that it will work properly? No locks or guards are added, just a check that allows the code to continue even though we're in a different thread. Is this going to ahve unintended side effects that haven't been addressed?

@zturner Yes, I disable the check for InFuzzingThread() on Windows, because AlarmCallback() is always executed on a different thread. If I don't add these changes, the alarm handler will never execute.

As I mentioned in https://reviews.llvm.org/D27240, we could have some problems because we don't synchronize with the main thread. But we decided to ignore them.
In fact, we can't fix that because the same function AlarmCallback() will be called asynchronously for linux, so we can't use locks or anything like that.

@zturner Ok. Do you think this diff is ready to land?

zturner accepted this revision.Jan 17 2017, 8:52 PM
This revision is now accepted and ready to land.Jan 17 2017, 8:52 PM
This revision was automatically updated to reflect the committed changes.