[llvm] r310075 - Port libFuzzer tests to LIT. Do not require two-stage build for check-fuzzer.

[Justin Bogner via llvm-commits]

George Karpenkov <ekarpenkov at apple.com> writes:
>> On Aug 18, 2017, at 2:36 PM, Justin Bogner <mail at justinbogner.com> wrote:
>> 
>> George Karpenkov <ekarpenkov at apple.com <mailto:ekarpenkov at apple.com>> writes:
>>>> 
>>>> Yes, that's awkward on its own, but sometimes you're doing that
>>>> *anyway*. In particular, when I'm building an LLVM fuzzer tool (like the
>>>> forthcoming llvm-isel-fuzzer) on a branch of LLVM I'm obviously building
>>>> with coverage. I'd like to be able to run the fuzzer tests to verify
>>>> that the libFuzzer I'm linking that against works.
>>>> 
>>>>> After offline conversation:
>>>>> - seems that libFuzzer is correctly built without coverage, and
>>>>>  slowdown was probably caused by something else.
>>>>> - the aforementioned problems should disappear once libFuzzer is
>>>>>  moved to compiler-rt and becomes a part of clang's toolchain.
>>>> 
>>>> Yes, once libFuzzer is distributed with the compiler these issues go
>>>> away - at that point the libFuzzer and the clang that you're using are
>>>> tested together.
>>>> 
>>>> If that's happening in the next couple of weeks I'm comfortable with
>>>> waiting for it, but if it's likely to be months I think we need a way to
>>>> say "test with the host compiler". A CMake variable would work, but I
>>>> actually think doing that by default if LLVM_USE_SANITIZE_COVERAGE is
>>>> true would be even better.
>>> 
>>> I think https://reviews.llvm.org/D36883 should solve your problems.
>> 
>> I think you forgot to add llvm-commits to that review - I don't see an
>> email for it on the list.
>
> Right, is that the standard procedure? Actually, I have never done so.

Yes, the commits lists are the system of record for code review:

  http://llvm.org/docs/Phabricator.html

If -commits isn't on the review, nobody sees it (except the people you
add manually to the review, I guess).

There's also a known bug where adding -commits to a review after the
fact doesn't send the patch to the list (only a phabricator comment), so
it's best to "abandon" the review and create a new one if -commits is
missing and nobody's commented on phab yet.

>>> Additionally, I think relying on LLVM_USE_SANITIZE_COVERAGE is not a
>>> good idea: libFuzzer is a tool in itself, and using it to fuzz LLVM is
>>> just one of possible applications, which definitely should not change
>>> how the library is compiled (e.g. it would be better to simply always
>>> append a flag which specifies that libFuzzer is compiled with no
>>> coverage, regardless of what LLVM_USE_SANITIZE_COVERAGE is set to).
>> 
>> This doesn't change how libFuzzer is compiled - it changes how its tests
>> are.
>> 
>> My thought process here is that, since LLVM_USE_SANITIZER_COVERAGE is
>> how we gate whether or not we're building tools that link libFuzzer (and
>> we're building those tools with sanitizer runtimes from the host
>> compiler), it also makes sense to gate what configuration of libFuzzer
>> we're testing.
>
> But this doesn’t make much sense when your workflow is different (that
> is, you are doing anything other than fuzzing LLVM itself).  And I
> would say you are not even testing libFuzzer: you’re testing whether
> your compiler is compatible with ti. 

Exactly, you're testing whether the compiler you're literally using with
libFuzzer is compatible with libFuzzer. I don't know when you would
decide to build LLVM with LLVM_USE_SANITIZER_COVERAGE if your goal is to
"use libFuzzer for something else", but I suspect that use case is a lot
less common than "I'm building llvm-as-fuzzer and I want to know that
the fuzzer works at all with the compiler I'm building llvm-as-fuzzer
with".

> And if we are also moving libFuzzer to compiler-rt it wouldn’t be even
> possible.

If and when libFuzzer is distributed with the toolchain (just being in
compiler-rt isn't relevant, other than as a way to get there), then this
is no longer necessary, I agree. But as long as we're building tools
that use libFuzzer in tree in the same build that we're building
libFuzzer, we need a way to test that libFuzzer with the compiler it's
being used with.