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

[Daniel Berlin via llvm-dev]

Sure, please do!


On Mon, Jul 17, 2017 at 2:23 PM, George Karpenkov <ekarpenkov at apple.com>
wrote:

>
> 2) Does it mean that we should reach out to board at llvm.org to okay the
>> issue?
>>
>> IMHO, yes.
>
>
> Thanks again for the quick reply!
> Would it be okay to email the contributors with the question before asking
> the board?
> Maybe everyone would be okay with it, and then we wouldn’t need to discuss
> the possible implications of not-being-okay.
>
>
>
>> Thanks,
>> George
>>
>>
>>
>>>
>>> On Wed, Jul 12, 2017 at 12:41 PM, George Karpenkov <ekarpenkov at apple.com
>>> > wrote:
>>>
>>>>
>>>> On Jul 12, 2017, at 11:58 AM, Kostya Serebryany <kcc at google.com> wrote:
>>>>
>>>>
>>>>
>>>> On Wed, Jul 12, 2017 at 11:54 AM, George Karpenkov <
>>>> ekarpenkov at apple.com> wrote:
>>>>
>>>>>
>>>>> On Jul 12, 2017, at 11:34 AM, Kostya Serebryany <kcc at google.com>
>>>>> wrote:
>>>>>
>>>>>
>>>>>
>>>>> On Wed, Jul 12, 2017 at 11:30 AM, George Karpenkov <
>>>>> ekarpenkov at apple.com> wrote:
>>>>>
>>>>>>
>>>>>> On Jul 12, 2017, at 11:01 AM, Kostya Serebryany <kcc at google.com>
>>>>>> wrote:
>>>>>> 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’.
>>>>>>
>>>>>>
>>>>>> I am not sure how we would handle CMake targets collision (I think we
>>>>>> would have one: check-fuzzer vs. check-fuzzer)
>>>>>> Of course, I can give it a different name for now,
>>>>>>
>>>>>
>>>>> Yep. 'check-fuzzer-temporary' or some such.
>>>>>
>>>>>
>>>>>> but that would potentially add even more confusion and complexity.
>>>>>>
>>>>>> Though it seems like a good idea to at least see what would happen to
>>>>>> buildbots.
>>>>>>
>>>>>> 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.
>>>>>>
>>>>>>
>>>>>> Licensing concerns, compiler-rt has a different license.
>>>>>>
>>>>>
>>>>> @%##%)*%
>>>>>
>>>>> But wait a sec, the sanitizers are ok with this license, why libFuzzer
>>>>> isn't?
>>>>> (Sorry, my memory has been flushed over the last month)
>>>>>
>>>>>
>>>>> Apparently because the code was already contributed under a different
>>>>> license.
>>>>> IANAL but apparently to do the change in a fully correct way one would
>>>>> need to chase
>>>>> down every single contributor and ask them to agree to a license
>>>>> change.
>>>>>
>>>>
>>>> This is a PITA, but we should do it.
>>>> licensing issues like this should not prevent us from making our own
>>>> code better.
>>>>
>>>>
>>>>
>>>> Should not be that hard:
>>>>
>>>> george@/Volumes/Transcend/code/llvm (master)≻ git log
>>>> --pretty=format:"%ae" lib/Fuzzer | egrep -v "chromium|apple|google" | sort
>>>> | uniq
>>>> aaron at aaronballman.com
>>>> ahmed.bougacha at gmail.com
>>>> chandlerc at gmail.com
>>>> craig.topper at gmail.com
>>>> dan at su-root.co.uk
>>>> eric at efcs.ca
>>>> hans at hanshq.net
>>>> juergen at ributzka.de
>>>> lenny at colorado.edu
>>>> mail at justinbogner.com
>>>> matze at braunis.de
>>>> mgrang at codeaurora.org
>>>> nicholas at mxc.ca
>>>> peter at pcc.me.uk
>>>> rafael.espindola at gmail.com
>>>> richard-llvm at metafoo.co.uk
>>>> sanjoy at playingwithpointers.com
>>>> tzuhsiang.chien at gmail.com
>>>> vonosmas at gmail.com
>>>> yaron.keren at gmail.com
>>>>
>>>> Would you know the formal process for a license change?
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>>
>>>>>
>>>>>
>>>>>>
>>>>>> BTW libFuzzer CMake has a crazy amount of hacks to work under
>>>>>> Windows, where logic in many parts
>>>>>> is entirely different, so any help on testing and fixing arising
>>>>>> issues would be much appreciated.
>>>>>>
>>>>>
>>>>> It's totally fine for me if we *copy* the code to the new location and
>>>>> ditch the windows support completely.
>>>>> Then, whoever cares about windows, will reinstate it in the new
>>>>> location once the dust settles.
>>>>>
>>>>>
>>>>>
>>>>>> All I can do is to make a commit and then try to understand the
>>>>>> response from a bots,
>>>>>> which is not very efficient.
>>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> 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-inf
>>>>>>> rastructure
>>>>>>> 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/20170717/51df06ec/attachment.html>