[libc-commits] [PATCH] D143784: [libc] Add basic fuzz target for the printf parser

[Michael Jones via Phabricator via libc-commits]

michaelrj added inline comments.


================
Comment at: libc/fuzzing/stdio/printf_parser_fuzz.cpp:40
+
+  auto parser = printf_core::Parser(in_str, mock_arg_list);
+
----------------
sivachandra wrote:
> Other than passing it to the `Parser` constructor, I don't see `mock_arg_list` being used anywhere. What am I missing?
You're not missing anything. The mock arg list is just there so that the program doesn't crash when the parser starts requesting arguments. It needs to respond with some value, but what that value actually is doesn't matter right now. Later I'll hopefully add a separate fuzz test that does check that the arguments were read in the right order, but this works for now.


================
Comment at: libc/src/__support/arg_list.h:37
 
+#else  // not defined LIBC_COPT_MOCK_ARG_LIST
+
----------------
sivachandra wrote:
> I am assuming that the use of this class is to get the number of args read out from the list. So, my suggestion is as follows. I would really not add a comment referencing printf or fuzz testing here. Also, you can add a mock class here unconditionally. The condition should be at the usage-site, which is the `Parser` class and the fuzzer, something like:
> 
> ```
> #ifdef LIBC_COPT_PRINTF_PARSER_WITH_MOCK_ARGS // Or a better name
> using ArgProvider = MockArgList;
> #else
> using ArgProvider = ArgList;
> #endif
> ```
> 
> Update `Parser` to take `ArgProvider`.
This seemed to be getting larger than should be in this patch, so I've split the ArgList changes into this other patch: https://reviews.llvm.org/D144145


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143784