[libc-commits] [PATCH] D74091: [libc] Lay out framework for fuzzing libc functions.

[Siva Chandra via Phabricator via libc-commits]

sivachandra added inline comments.


================
Comment at: libc/fuzzing/string/strcpy_fuzz.cpp:25-27
+  if (strcmp(dest, src) != 0) {
+    abort();
+  }
----------------
sivachandra wrote:
> abrachet wrote:
> > PaulkaToast wrote:
> > > abrachet wrote:
> > > > Is this not `assert(strcmp(dest, src))` because you think `NDEBUG` might be defined for this file?
> > > oss-fuzz compiles with optimization -o3 enabled. Does NDEBUG get defined with that level of optimization? If it does then assert will not crash the fuzzer as expected. 
> > @sivachandra said to avoid `abort`, so that would mean avoid `assert` too, we could change this `abort` to `__builtin_trap`.
> > 
> > FWIW https://godbolt.org/z/khdC4E
> I am not really against using `abort` but against including `stdlib.h`. So, we can create an abort wrapper in `utils/CPP` which allows us to avoid including `stdlib.h`.
> 
> But, if `exit` is disallowed in a fuzz target, is `abort` OK? FWIW, libcxx's fuzz tests seem to prefer `assert` as @abrachet suggested.
> 
> Also, asking for my own knowledge: Should one care about correctness in a fuzz test? Correctness is important of course, but is that the goal of a fuzz test?
Just correcting myself: If we conclude we need `abort`/`assert`, then we should put their wrappers in a fuzzing specific util-library and not in `utils/CPP`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74091