Kostya Serebryany via llvm-dev
2017-May-11 17:31 UTC
[llvm-dev] moving libfuzzer to compiler-rt?
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/20170511/a8db9936/attachment-0001.html>
Kostya Serebryany via llvm-dev
2017-Jul-12 18:01 UTC
[llvm-dev] moving libfuzzer to compiler-rt?
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>