[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:30
+// I'm not really worried.
+const char valid_chars[] = {
+    ' ', '\t', '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'a',
----------------
`constexpr` and `VALID_CHARS`.


================
Comment at: libc/fuzzing/stdlib/strtointeger_differential_fuzz.cpp:37
+
+extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
+  uint8_t *container = new uint8_t[size + 1];
----------------
Add a comment briefly explaining the fuzz strategy. Like, give the information that the first byte of of `data` will be treated as the target base for `strto*` functions.


================
Comment at: libc/fuzzing/stdlib/strtointeger_differential_fuzz.cpp:48
+    container[i] = data[i];
+    cleaner[i] = valid_chars[data[i] % sizeof(valid_chars)];
+  }
----------------
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.


================
Comment at: libc/fuzzing/stdlib/strtointeger_differential_fuzz.cpp:53
+  cleaner[0] = container[0]; // the first character is interpreted as the base,
+                             // so it hsould be fully random.
+
----------------
should


================
Comment at: libc/src/stdlib/atoi.cpp:16
 LLVM_LIBC_FUNCTION(int, atoi, (const char *str)) {
-  auto result = internal::strtointeger<int>(str, 10);
+  auto result = internal::strtointeger<long>(str, 10);
   if (result.has_error())
----------------
I think this change is trying to match the standard prescribed behavior. Can you please do this separately and add unit tests with inputs which after affected by this behavior if any?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140178