[llvm-dev] moving libfuzzer to compiler-rt?

Wed Jul 12 11:01:00 PDT 2017

On Tue, Jul 11, 2017 at 7:02 PM, George Karpenkov <ekarpenkov at apple.com>
wrote:

> I’ve submitted a WIP PR: https://reviews.llvm.org/D35288
>

Thanks for working in this!

One question: will it make sense to *copy* the code to the new location,
work on it, then delete the code from the old location,
instead of doing a move in a single commit?
I don't expect any dramatic changes in the code structure during a few
weeks, so 'copy' might be much simpler than 'move'.

>
> There remains a number of unsolved issues:
>
> 1) Naming: “runtimes” enforces LLVM naming convention.
> The project name is formed by removing the “lib” prefix, and then that is
> the main ninja target,
> and check-PROJ_NAME is used for tests.
> Thus for libfuzzer the choices for folder/target name/tests target name
> become
> `libfuzzer`/`fuzzer`/`check-fuzzer`.
>
> That is not optimal, but the situation is even worse if the folder is
> called `libFuzzer`,
> as targets rarely start with capital letters.
>
> 2) Dependencies: libFuzzer tests depend on ASAN and UBSAN.
> It’s not possible to directly depend on other projects from “runtimes”, as
> the code is compiled using
> a recursive CMake invocation.
> Those are also runtimes, and thus apparently it’s not possible to clearly
> depend on them
> in `runtimes/CMakeListst.txt` (though maybe the target list can be lazily
> parsed for tests?)
> The relevant problem was highlighted in a comment in
> https://reviews.llvm.org/D33048
>

I already forgot why we decided not to move the code to compiler-rt.
This would solve at least this problem.
Since we now have -fsanitize=fuzzer it will actually be pretty natural.

>
> 3) CMake files in LLVM root repository do a rather large amount of
> trickery:
> more warning flags are introduced, default linking commands for different
> platforms are changed,
> etc.
> None of this is picked up when running from the “runtimes” directory,
> which is often undesirable,
> as the runtime would be built very differently from the rest of LLVM,
> missing those desired warnings/optimizations.
>
> Additionally, working with “runtimes” introduces some inherent
> restrictions.
> Namely:
>
> 4) Extra arguments to CMake will not be propagated.
> E.g. invoking `cmake … -DLLVM_USE_SANITIZE_COVERAGE=1` will not have any
> effect on the compilation of any project
> in “runtimes”.
> It’s not clear how those flags can be explicitly propagated.
>
> 5) In a similar vein to (4), additional flags to `ninja` will not be
> propagated.
> E.g. running “ninja -v check-blah” will actually not show the commands
> required to execute the tests,
> as it goes through a CMake invocation.
> This issue has a workaround: ninja can be launched directly from
> “runtimes/runtime-bins”, but that is counterintuitive.
>
> 6) Recursive invocation required for “runtimes” breaks
> `compile_commands.json` construction, which is used
> by many editors and tools (e.g. Vim+Ale or rtags) for go-to-defintion and
> error highlight functionality.
> There is a workaround: a separate `compile_commands.json` is generated for
> the `runtimes` directory,
> and it might be possible to write some magic to inject it back into the
> parent one, but that would be yet-another-piece-of-build-infrastructure
> which would have to be maintained.
>
> 7) Similarly to (6), recursive invocation breaks XCode tooling: in a
> generated XCode project,
> no libfuzzer files are accessible. I would assume same would hold for
> other IDEs.
>
> Any thoughts or comments on these?
> While (1) is not really a problem, and I can probably find a workaround
> for (2), the issues
> listed in (3)-(7) seem inherent to recursive CMake invocation.
> I can think of a number of alternative suggestions, which would solve the
> problem of requiring a 2-stage build:
>
> a) We can move the compilation commands for libFuzzer tests from CMake
> into lit.
> This would have an added benefit of each lit test being self-contained: it
> would be sufficient to just run
> “lit” to reproduce everything, and it would pick up all changes to
> compiler/coverage instrumentation/sanitizers/etc.
> The first run would generate the tested binaries, and further tinkering
> could be done with binaries directly if desired.
>
> b) We can use the “tools” directory instead.
>
>
> Thanks,
> George
>
> On May 11, 2017, at 10:31 AM, Kostya Serebryany <kcc at google.com> wrote:
>
>
>
> On Wed, May 10, 2017 at 6:09 PM, Chris Bieneman via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
>>
>> On May 10, 2017, at 4:43 PM, George Karpenkov via llvm-dev <
>> llvm-dev at lists.llvm.org> wrote:
>>
>> Actually, there’s another problem we have missed: libraries under
>> `build/lib` are not installed into toolchain
>> on mac os (and neither on linux, I would suppose).
>>
>>
>> Actually that isn't accurate. By default we don't install the LLVM
>> libraries, but that is completely configurable in the build system. It
>> doesn't work for libFuzzer because the CMake build for libFuzzer is not
>> built using any of the LLVM CMake modules or following any of LLVM's
>> conventions.
>>
>> Thus installations of Clang would not contain libLLVMFuzzer, but we would
>> like them to, so that users would not have
>> to compile anything, and could just call `clang -fsanitize=fuzzer`.
>>
>> That could probably be done with another CMake change, but I have no idea
>> how to do that.
>>
>>
>> Yea, libFuzzer's CMake really needs a big overhaul, and probably an
>> almost complete rewrite.
>>
>
> no objections.
>
>
>>
>> -Chris
>>
>>
>> On May 9, 2017, at 4:04 PM, George Karpenkov via llvm-dev <
>> llvm-dev at lists.llvm.org> wrote:
>>
>>
>> On May 9, 2017, at 3:00 PM, Kostya Serebryany <kcc at google.com> wrote:
>>
>> Thanks for the explanations! (it was worth asking)
>>
>> I do want to build libFuzzer itself (and its tests) using the just-built
>> clang. So, llvm/runtimes then.
>> I'd name the directory llvm/runtimes/libFuzzer, if possible (the old
>> path was lib/Fuzzer which is how the tool got it's name, actually)
>> George, would you like to send the change for review?
>>
>> OK
>>
>>
>> --kcc
>>
>>
>>
>> On Tue, May 9, 2017 at 2:37 PM, Chris Bieneman <cbieneman at apple.com>
>> wrote:
>>
>>>
>>> On May 9, 2017, at 2:19 PM, George Karpenkov <ekarpenkov at apple.com>
>>> wrote:
>>>
>>> +Chris.
>>>
>>> My understanding was that it is technically impossible for things in
>>> “lib”, as they are built first, and there’s no way to tell them to do that
>>> before “clang”.
>>> I’m not a CMake expert, and I might be wrong.
>>>
>>>
>>> It is not impossible, it would just involve excessive hacks. Since it
>>> seems like this isn't a short-term solution we're talking about I am very
>>> opposed to throwing hacks into the build system. I'd rather we actually fix
>>> the problem(s). More below.
>>>
>>>
>>> On May 9, 2017, at 2:15 PM, Kostya Serebryany <kcc at google.com> wrote:
>>>
>>>
>>>
>>> On Tue, May 9, 2017 at 1:56 PM, George Karpenkov <ekarpenkov at apple.com>
>>> wrote:
>>>
>>>> Again, after offline conversation with Chris Bieneman:
>>>>
>>>>  - move to compiler-rt would be too complicated due to change in
>>>> licenses
>>>>  - it would make much more sense to move to “tools” folder instead, for
>>>> the following reasons:
>>>>     * conceptually, it’s a tool, not a library
>>>>     * all other projects in “lib” depend on LLVM and can not build
>>>> without LLVM, libFuzzer does not
>>>>     * practically speaking, CMake has no way of knowing whether Clang
>>>> is being built when
>>>>       “lib” is compiled, yet it does know for projects in tools.
>>>>
>>>> Using a freshly built clang for projects in “tools” is embarrassingly
>>>> easy and only requires a couple of lines
>>>> of configuration change.
>>>>
>>>> Kostya, what about moving to “tools” then?
>>>>
>>>
>>> Well, ok, this sounds cool.
>>> But can we make one more step and try to preserve the code where it is,
>>> for the sake of compatibility?
>>>
>>>
>>> Please no. This code doesn't actually belong in lib, it has never fit
>>> the model of an LLVM library, we really need to pull it out of there.
>>>
>>> E.g. can we have the CMake in tools while still keeping the code in lib?
>>>
>>>
>>> Could we contrive a hack in the build system to do it? Yes, but I will
>>> fight violently against allowing that change into the build system because
>>> the right answer here is to move the code.
>>>
>>> Or a link of some kind?
>>>
>>>
>>> Links are incredibly fragile on Windows, and they trip up a lot of SCM
>>> tools. We have one in LLDB's repo that causes me nothing but problems, so I
>>> am also strongly opposed to that.
>>>
>>>
>>> My worry is that there are already quite a few places that know where
>>> libFuzzer code is,
>>> and I don't control all of them.
>>>
>>>
>>> Downstream clients will have to update. That is kinda how these things
>>> work, I can't imagine re-pointing an SCM checkout being a huge burden.
>>>
>>>
>>> And, finally, I really don't get why we can do something in tools and
>>> can't do the same in lib.
>>> Or we simply don't want to do it to keep things simple?
>>>
>>>
>>> Not all functionality in CMake is order-independent. Specifically the
>>> detection of targets is not. In order to support what you're trying to do
>>> you are going to change behavior based on the presence of the clang target.
>>> Which means the clang target must be added before your CMake is processed.
>>>
>>> To support this our build system has strict ordering requirements such
>>> that things in lib cannot depend on things in tools. If you need to depend
>>> on clang, you need to not be in lib.
>>>
>>> Also, generally speaking Fuzzer is a library under lib that also has
>>> nested tests, which is *not* how the lib directory is supposed to be
>>> structured. It never should have been allowed to be structured like that.
>>> If you want the tests next to the library, it is a tool or a runtime, but
>>> not a lib.
>>>
>>> As I see there are two options to move forward with, and it really
>>> depends on how you intend to use the just-built clang.
>>>
>>> (1) If you want to use just-built clang to build libFuzzer and its
>>> tests, it should be a runtime.
>>> (2) If you want to use just-built clang to only build libFuzzer's tests,
>>> it should be a tool.
>>>
>>> I think that since it is a runtime library, it should be a runtime, and
>>> I expect it would mostly work to just copy the Fuzzer directory into
>>> llvm/runtimes.
>>>
>>> -Chris
>>>
>>>
>>> --kcc
>>>
>>>
>>>
>>>>
>>>> On May 9, 2017, at 11:07 AM, Dan Liew <dan at su-root.co.uk> wrote:
>>>>
>>>> On 9 May 2017 at 18:55, Kostya Serebryany <kcc at google.com> wrote:
>>>>
>>>>
>>>>
>>>> On Tue, May 9, 2017 at 10:23 AM, Dan Liew <dan at su-root.co.uk> wrote:
>>>>
>>>>
>>>> Does anyone see good reasons why libFuzzer should remain in llvm repo
>>>> (as
>>>> opposed to moving it to compiler-rt)?
>>>>
>>>>
>>>> Does moving LibFuzzer to compiler-rt imply that it is compiled as part
>>>> of compiler-rt and shipped with it?
>>>>
>>>> How does that fit with LibFuzzer's model of allowing the user to
>>>> provide their own `main()`.
>>>>
>>>>
>>>>
>>>> libFuzzer doesn't allow users to use their own main (not any more).
>>>> Although I am not sure how that's related to moving libFuzzer somewhere.
>>>>
>>>>
>>>> Oops. That shows how long it's been since I looked at the source code.
>>>>
>>>> It was related in that if LibFuzzer was shipped as part of compiler-rt
>>>> I presumed we would need to supply both libraries to end users.
>>>> Given that this feature was removed it is a non-issue.
>>>>
>>>>
>>>
>>>
>>
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
>>
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
>>
>>
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20170712/acd85c72/attachment-0001.html>