[libc-commits] [PATCH] D140178: [libc] add fuzz target for strtointeger functions

[Siva Chandra via Phabricator via libc-commits]

sivachandra added inline comments.


================
Comment at: libc/fuzzing/stdlib/strtointeger_differential_fuzz.cpp:48
+    container[i] = data[i];
+    cleaner[i] = valid_chars[data[i] % sizeof(valid_chars)];
+  }
----------------
michaelrj wrote:
> sivachandra wrote:
> > Why should we make the input clean? It defeats the purpose of fuzzing in a way. Since this is a differential fuzz setup, I am not really worried about garbage. Also, valid ASCII characters make half of the `uint8_t` range anyway. So, we don't have to worry about most of the `data` inputs being just invalid numbers. Also, the above list of valid chars does not have the `+` and `-` signs.
> The reason I want to clean the input is because otherwise almost all of the inputs won't actually exercise the function at all. If the first character is a question mark (for example), then the function won't actually parse anything and it will return 0. Testing that isn't bad, but given that more than half of the ascii range falls into that category there will be a lot of cycles spent on those obvious cases instead of actually testing the branches.
Your argument is fair. My point is that it might not help in exercising all code paths. Take decimal numbers for example. The set of valid characters is only 10 out of the 62 there are in `VALID_CHARS` now. But, there are more valid numbers as you progress to higher bases. For base 36, I think there are no invalid inputs at all? You can choose to implement two modes - one for human testing and another for continuous testing. For human testing, clean the input like you do in this patch currently. For continuous testing, don't clean the input. You will have to add two targets with a distinguishing pre-processor macro.

You should add `-` and `+` to the set of valid chars. Or, you can prepend a `-` and `+` to each clean input in the human mode and test again.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140178