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

[Daniel Berlin via llvm-dev]

On Wed, Jul 12, 2017 at 12:48 PM, Kostya Serebryany <kcc at google.com> wrote:

> + Chandler, Danny,
>
> We are considering to move the libFuzzer code from llvm to compiler-rt,
> and that implies a license change.
> Will it be sufficient to do the following?
>   * e-mail to all contributors (a short list, below) asking for their
> consent
>   * remove any code for which we did not get consent in, say, 1 week.
>
> (BTW, this list is actually much shorter, I recognize many of these as
> belonging to googlers or applers)
>

This process works legally, but i can't speak to whether the foundation
would be okay with it, as it may result in bad press, etc, if you rip code
out.


>
> 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/a1cd2470/attachment.html>