This is an archive of the discontinued LLVM Phabricator instance.

Port libFuzzer tests to LIT. Do not require two-stage build for check-fuzzer.
ClosedPublic

Authored by george.karpenkov on Aug 3 2017, 4:32 PM.

Details

Summary

This revision ports all libFuzzer tests apart from the unittest to LIT.
The advantages of doing so were already discussed, and include:

  • Tests being self-contained
  • Much easier debugging of a single test
  • No need to use two-stage compilation

The unit-test is still compiled using CMake, but it does not need a freshly built compiler.

Disadvantages require multiple test recompilation, but on my machine the final result is actually faster then the previous configuration (probably due to the fact that tests are compiled without optimizations).

NOTE: Now we can simply run check-fuzzer to run tests! However, the previous configuration will NOT work, as in the second stage build LLVM_USE_SANITIZER is set, which disables ASAN from being built. Thus bots would need to be reconfigured (but the new configuration is super-simple compared to the previous one).

Diff Detail

Repository
rL LLVM

Event Timeline

No need to change flags now that tests are ported to LIT.

Add asan to libfuzzer tests dependencies.

fix up isysroot flag

kcc edited edge metadata.Aug 3 2017, 4:47 PM

Lovely!

Once finalized and submitted, I'll take care of the linux buildbot.

lib/Fuzzer/test/lit.cfg
63 ↗(On Diff #109654)

Are these 4 lines correct?
Looks like the conditions should be reversed

apply Kostya's fix

george.karpenkov marked an inline comment as done.Aug 3 2017, 4:55 PM
george.karpenkov added inline comments.
lib/Fuzzer/test/lit.cfg
63 ↗(On Diff #109654)

Good catch. Now I'm confused how the tests were passing before that, but seems passing now as well.

kcc added a comment.Aug 3 2017, 5:03 PM

Now, something is fishy here.
check-fuzzer indeed passes, but if I crippled the merge functionality (replace OUTER with OTTER in FuzzerMerge.cpp) it still passes, i.e. the tests don't work any more.

That's bad. If we silently disable tests we will break half of libFuzzer within a month before we notice.

BTW, please also disable check-fuzzer on windows ASAP -- it's causing us troubles

george.karpenkov marked an inline comment as done.

add dependency on libFuzzer

If we silently disable tests

there was simply a missing dependency on libFuzzer, so that it wasn't recompiled. Seems to work now.

please also disable check-fuzzer on windows

OK

kcc added a comment.Aug 3 2017, 5:14 PM

Yep, this is ok now. Looking further.

kcc accepted this revision.Aug 3 2017, 5:24 PM

LGTM with a nit, and thanks again for doing this.
I guess you may want to submit this tomorrow morning, so that we don't spoil the tonight's LLVM social with whatever this is going to break.

lib/Fuzzer/test/lit.cfg
59 ↗(On Diff #109661)

Do you ever set asan_enabled to false?
If not, I guess you can remove this flag.

This revision is now accepted and ready to land.Aug 3 2017, 5:24 PM

Applying Kostya's suggestion

This revision was automatically updated to reflect the committed changes.

@alekseyshl I would assume the bot would need to be reconfigured. Previous CMake configuration was only building libFuzzer when LLVM_USE_SANITIZE_COVERAGE was set to true. In the current configuration it should be sufficient to add -DLIBFUZZER_ENABLE=ON to CMake invocation.
@kcc Would you be willing to reconfigure the bot, or should I do that?
Additionally, if we still want to be able to build libFuzzer correctly when LLVM_USE_SANITIZE_COVERAGE is on? If yes, I would need to add a command line flag which would avoid instrumenting libFuzzer itself.

llvm/trunk/lib/Fuzzer/test/trace-pc/CMakeLists.txt