Don't include libarchive fuzz tests by default

[arichardson]

Summary

These tests are basic fuzz tests that permute input to trigger crashes
rather than regression or unit tests. Additionally, some of them take a
rather long time to run and should probably be run on a dedicated fuzzing
job instead. Moreover, these simple tests use rand() instead of a real
fuzzing tool that generates interesting inputs (e.g. LLVM libFuzzer) so are
unlikely to find anything interesting when run in CI.

This allows removing one BROKEN_TESTS case due to timeouts and speeds up
running tests on emulated platforms such as QEMU.

[emaste]

I think this is fine in principle, but ideally would be a run-time instead of build-time check.

We should also find a way to ensure we add good fuzz testing via CI at some point (i.e., don't just commit this and never revisit).

[arichardson]

In D27153#606211, @emaste wrote:

I think this is fine in principle, but ideally would be a run-time instead of build-time check.

We should also find a way to ensure we add good fuzz testing via CI at some point (i.e., don't just commit this and never revisit).

There is a SKIP_TEST_FUZZ variable that could be set in functional_test.sh: https://github.com/freebsd/freebsd/blob/4e9412c9eec32da012f06fd9553b1d14232227ae/contrib/libarchive/libarchive/test/test_fuzz.c#L63

I just thought we might as well skip listing those test if we aren't going to run them.

[lwhsu]

In D27153#606212, @arichardson wrote:

In D27153#606211, @emaste wrote:

I think this is fine in principle, but ideally would be a run-time instead of build-time check.

We should also find a way to ensure we add good fuzz testing via CI at some point (i.e., don't just commit this and never revisit).

There is a SKIP_TEST_FUZZ variable that could be set in functional_test.sh: https://github.com/freebsd/freebsd/blob/4e9412c9eec32da012f06fd9553b1d14232227ae/contrib/libarchive/libarchive/test/test_fuzz.c#L63

I just thought we might as well skip listing those test if we aren't going to run them.

I also think this is fine. There are some buzz tests in libarchive weren't stable, for example, https://bugs.freebsd.org/241662 (They recently become more stable in both ci.freebsd.org and upstream.)

The only comment from me is we're using LIBARCHIVE_INCLUDE_TEST_FUZZ vaiable here, and for GoogleTest we use a toggle with another naming scheme. Is there a way to easily build, install and default to run all the tests in the code base? Say, all can be enabled by a toggle like WITH_EXTRA_TESTS that will be useful to check all the available test codes in the tree, and do improvement and categorizing, for the purpose that we want to create lists of tests with different purposes.

I do want eventually having a periodic job running fuzz tests, like syzkaller, over the user space programs and libraries, but if the current implementation coming from upstream isn't very useful to us, skipping them them is fine. For running fuzz tests, I think we need implement a fuzzer in the system (with libFuzzer) or enabling these fuzz test from upstream, or both, and create a standalone job for them.

[arichardson]

In D27153#606346, @lwhsu wrote:

In D27153#606212, @arichardson wrote:

In D27153#606211, @emaste wrote:

I think this is fine in principle, but ideally would be a run-time instead of build-time check.

We should also find a way to ensure we add good fuzz testing via CI at some point (i.e., don't just commit this and never revisit).

There is a SKIP_TEST_FUZZ variable that could be set in functional_test.sh: https://github.com/freebsd/freebsd/blob/4e9412c9eec32da012f06fd9553b1d14232227ae/contrib/libarchive/libarchive/test/test_fuzz.c#L63

I just thought we might as well skip listing those test if we aren't going to run them.

I also think this is fine. There are some buzz tests in libarchive weren't stable, for example, https://bugs.freebsd.org/241662 (They recently become more stable in both ci.freebsd.org and upstream.)

The only comment from me is we're using LIBARCHIVE_INCLUDE_TEST_FUZZ vaiable here, and for GoogleTest we use a toggle with another naming scheme. Is there a way to easily build, install and default to run all the tests in the code base? Say, all can be enabled by a toggle like WITH_EXTRA_TESTS that will be useful to check all the available test codes in the tree, and do improvement and categorizing, for the purpose that we want to create lists of tests with different purposes.

My intent was that this should only be compiled if you really want it to, so it shouldn't be discoverable. I could also change it to .if 0? There's not much difference between setting a magic variable and editing the makefile.

I do want eventually having a periodic job running fuzz tests, like syzkaller, over the user space programs and libraries, but if the current implementation coming from upstream isn't very useful to us, skipping them them is fine. For running fuzz tests, I think we need implement a fuzzer in the system (with libFuzzer) or enabling these fuzz test from upstream, or both, and create a standalone job for them.

That sounds like a good plan to me.

[lwhsu]

In D27153#606351, @arichardson wrote:

In D27153#606346, @lwhsu wrote:

The only comment from me is we're using LIBARCHIVE_INCLUDE_TEST_FUZZ vaiable here, and for GoogleTest we use a toggle with another naming scheme. Is there a way to easily build, install and default to run all the tests in the code base? Say, all can be enabled by a toggle like WITH_EXTRA_TESTS that will be useful to check all the available test codes in the tree, and do improvement and categorizing, for the purpose that we want to create lists of tests with different purposes.

My intent was that this should only be compiled if you really want it to, so it shouldn't be discoverable. I could also change it to .if 0? There's not much difference between setting a magic variable and editing the makefile.

If that is the intent, then it's fine to me. I added userspace fuzzing in https://hackmd.io/@FreeBSD-CI/freebsd-ci-todo so at least this idea won't get forgot.

BTW, don't forget to notify the maintainer, Martin Matuska. Give him some time to response.

[arichardson]

Use .if 0

[arichardson]

@mm Does this look okay to you?

[arichardson]

In D27153#606355, @lwhsu wrote:

In D27153#606351, @arichardson wrote:

In D27153#606346, @lwhsu wrote:

The only comment from me is we're using LIBARCHIVE_INCLUDE_TEST_FUZZ vaiable here, and for GoogleTest we use a toggle with another naming scheme. Is there a way to easily build, install and default to run all the tests in the code base? Say, all can be enabled by a toggle like WITH_EXTRA_TESTS that will be useful to check all the available test codes in the tree, and do improvement and categorizing, for the purpose that we want to create lists of tests with different purposes.

My intent was that this should only be compiled if you really want it to, so it shouldn't be discoverable. I could also change it to .if 0? There's not much difference between setting a magic variable and editing the makefile.

If that is the intent, then it's fine to me. I added userspace fuzzing in https://hackmd.io/@FreeBSD-CI/freebsd-ci-todo so at least this idea won't get forgot.

BTW, don't forget to notify the maintainer, Martin Matuska. Give him some time to response.

I tried to notify Martin Matuska but have not heard anything back, so I'll go ahead and commit this early next week.

[mm]

We are currently being fuzzed by oss-fuzz which is way better than our little fuzzing in the tests. I have no exceptions in removing the fuzz tests.