[PATCH] D38642: [clang-fuzzer] Allow building without coverage instrumentation.

2017-10-10 Thread Matt Morehouse via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL315336: [clang-fuzzer] Allow building without coverage instrumentation. (authored by morehouse). Changed prior to commit: https://reviews.llvm.org/D38642?vs=118097&id=118420#toc Repository: rL LLVM

[PATCH] D38642: [clang-fuzzer] Allow building without coverage instrumentation.

2017-10-06 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. In https://reviews.llvm.org/D38642#891125, @morehouse wrote: > In https://reviews.llvm.org/D38642#891074, @kcc wrote: > > > If you can *easily* share main() with the one in LLVM -- do it, otherwise > > don't bother. > > > Does the fuzzer main come from LLVM or compiler-rt no

[PATCH] D38642: [clang-fuzzer] Allow building without coverage instrumentation.

2017-10-06 Thread Matt Morehouse via Phabricator via cfe-commits
morehouse added a comment. In https://reviews.llvm.org/D38642#891074, @kcc wrote: > If you can *easily* share main() with the one in LLVM -- do it, otherwise > don't bother. Does the fuzzer main come from LLVM or compiler-rt now? There's still FuzzerMain.cpp, but I'm not sure if we should be

[PATCH] D38642: [clang-fuzzer] Allow building without coverage instrumentation.

2017-10-06 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a reviewer: vitalybuka. kcc added a comment. conceptually ok, but please let Vitaly review the cmake part. https://reviews.llvm.org/D38642 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listin

[PATCH] D38642: [clang-fuzzer] Allow building without coverage instrumentation.

2017-10-06 Thread Matt Morehouse via Phabricator via cfe-commits
morehouse updated this revision to Diff 118097. morehouse added a comment. - Revert "Remove dummy main and link with -fsantize=fuzzer." https://reviews.llvm.org/D38642 Files: clang/tools/clang-fuzzer/CMakeLists.txt clang/tools/clang-fuzzer/ClangFuzzer.cpp clang/tools/clang-fuzzer/DummyCla

[PATCH] D38642: [clang-fuzzer] Allow building without coverage instrumentation.

2017-10-06 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. grrr. I am sorry, I've just contradicted myself. :( The goal here is to build the fuzz targets always and use them as tests, which includes building with any toolchain, including toolchains that don't support -fsanitize=fuzzer your original change actually solved this.

[PATCH] D38642: [clang-fuzzer] Allow building without coverage instrumentation.

2017-10-06 Thread Matt Morehouse via Phabricator via cfe-commits
morehouse updated this revision to Diff 118087. morehouse added a comment. - Remove dummy main and link with -fsantize=fuzzer. https://reviews.llvm.org/D38642 Files: clang/tools/clang-fuzzer/CMakeLists.txt clang/tools/clang-fuzzer/proto-to-cxx/CMakeLists.txt Index: clang/tools/clang-fuzzer

[PATCH] D38642: [clang-fuzzer] Allow building without coverage instrumentation.

2017-10-06 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. We often suggest to code owners to implement their own dummy main to run fuzz targets as regression tests. But for ourselves (LLVM) this recommendations makes less sense since libFuzzer is part of LLVM and we can use it's main directly. https://reviews.llvm.org/D38642

[PATCH] D38642: [clang-fuzzer] Allow building without coverage instrumentation.

2017-10-06 Thread Matt Morehouse via Phabricator via cfe-commits
morehouse added a comment. In https://reviews.llvm.org/D38642#890969, @kcc wrote: > I'd like to know more. > At least simple cases work fine: You're right. I was trying to add `-fsanitize=fuzzer` to `CMAKE_CXX_FLAGS` right before the link command, which was causing a later compilation to gi

[PATCH] D38642: [clang-fuzzer] Allow building without coverage instrumentation.

2017-10-06 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. >> Will we be able to reuse some of Justin's code instead of creating one more >> main() function? > > This reuses the code that Justin moved to FuzzMutate/FuzzerCLI. That's why > the main is so short. But perhaps we could move the main itself into > FuzzerCLI? Yes, hav

[PATCH] D38642: [clang-fuzzer] Allow building without coverage instrumentation.

2017-10-06 Thread Matt Morehouse via Phabricator via cfe-commits
morehouse added a comment. In https://reviews.llvm.org/D38642#890963, @kcc wrote: > It's not about coverage instrumentation (not) being present, but about > libFuzzer's main() being present, right? Yes. > Will we be able to reuse some of Justin's code instead of creating one more > main() fu

[PATCH] D38642: [clang-fuzzer] Allow building without coverage instrumentation.

2017-10-06 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added a comment. It's not about coverage instrumentation (not) being present, but about libFuzzer's main() being present, right? Will we be able to reuse some of Justin's code instead of creating one more main() function? Or, why not link with libFuzzer (-fsanitize=fuzzer at link time) eve

[PATCH] D38642: [clang-fuzzer] Allow building without coverage instrumentation.

2017-10-06 Thread Matt Morehouse via Phabricator via cfe-commits
morehouse created this revision. Herald added a subscriber: mgorny. Build with DummyClangFuzzer.cpp as entry point when coverage instrumentation isn't present. https://reviews.llvm.org/D38642 Files: clang/tools/clang-fuzzer/CMakeLists.txt clang/tools/clang-fuzzer/ClangFuzzer.cpp clang/too