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

[Paula Toth via Phabricator via libc-commits]

PaulkaToast added inline comments.


================
Comment at: libc/fuzzing/string/strcpy_fuzz.cpp:7
+
+extern "C" int LLVMFuzzerTestOneInput(uint8_t *data, size_t size) {
+  if (size == 0) {
----------------
sivachandra wrote:
> abrachet wrote:
> > Does `oss-fuzz` require this to not be mangled?
> Just a few high level comments for now. Might have more later.
> 
> Avoid using malloc/memcpy/abort:
> - Return a non-zero value instead of abort.
> - Instead of malloc/memcpy/free, split the input data into two parts deterministic-ally. Say, use the first N bytes to determine the size of the first part.
> - If you think a generic data provider makes sense, then we should probably build one for our use. For example, like this: https://github.com/llvm/llvm-project/blob/master/compiler-rt/include/fuzzer/FuzzedDataProvider.h
> 
> 
Yes, [[ https://llvm.org/docs/LibFuzzer.html#id22 | LibFuzzer ]] and indirectly oss-fuzz requires symbols to be unmangled.


================
Comment at: libc/fuzzing/string/strcpy_fuzz.cpp:7
+
+extern "C" int LLVMFuzzerTestOneInput(uint8_t *data, size_t size) {
+  if (size == 0) {
----------------
PaulkaToast wrote:
> sivachandra wrote:
> > abrachet wrote:
> > > Does `oss-fuzz` require this to not be mangled?
> > Just a few high level comments for now. Might have more later.
> > 
> > Avoid using malloc/memcpy/abort:
> > - Return a non-zero value instead of abort.
> > - Instead of malloc/memcpy/free, split the input data into two parts deterministic-ally. Say, use the first N bytes to determine the size of the first part.
> > - If you think a generic data provider makes sense, then we should probably build one for our use. For example, like this: https://github.com/llvm/llvm-project/blob/master/compiler-rt/include/fuzzer/FuzzedDataProvider.h
> > 
> > 
> Yes, [[ https://llvm.org/docs/LibFuzzer.html#id22 | LibFuzzer ]] and indirectly oss-fuzz requires symbols to be unmangled.
 Just to address the first comment. [[ https://llvm.org/docs/LibFuzzer.html#id22 | Non-zero returns are reserved ]] by LibFuzzer. The usage to indicate fault is to crash the program.


================
Comment at: libc/fuzzing/string/strcpy_fuzz.cpp:25-27
+  if (strcmp(dest, src) != 0) {
+    abort();
+  }
----------------
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. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74091