[PATCH] D56194: [gn build] Add fuzzers in llvm/tools that are needed for check-llvm

[Petr Hosek via Phabricator via llvm-commits]

phosek accepted this revision.
phosek added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: llvm/utils/gn/build/fuzzer.gni:29
+template("fuzzer") {
+  assert(defined(invoker.dummy_main))
+  assert(defined(invoker.sources))
----------------
thakis wrote:
> phosek wrote:
> > I checked the LLVM `add_llvm_fuzzer` function and `DUMMY_MAIN` is just an optional argument which is only included in `LLVM_OPTIONAL_SOURCES` for IDE but otherwise ignored if `llvm_use_sanitize_coverage` or `llvm_use_sanitize_coverage` is set. Shouldn't this behave the same?
> This template also only uses dummy_main if `llvm_lib_fuzzing_engine` and `llvm_use_sanitize_coverage` both aren't set  (see the two `not_needed`s) -- it tries to do the same. Maybe I got it wrong?
If I'm reading `add_llvm_fuzzer`, it's not going to give you an error if you don't pass `DUMMY_MAIN` while this template will. I think it's fine since you don't want the behavior of this target differ based on whether `llvm_use_sanitize_coverage` or `llvm_use_sanitize_coverage` is set or not, but I want to point out the difference. In this case it'd be probably better to change CMake to make those arguments required as well.

Can you please also provide an error message for this assertion and the one below?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56194/new/

https://reviews.llvm.org/D56194