[PATCH] D47880: [Fuzzer] Afl driver changing iterations handling

[Jonathan Metzman via Phabricator via llvm-commits]

metzman added a comment.

Reposting my comments here from https://reviews.llvm.org/D49141

I think this patch should be reverted. 
It breaks afl_driver's command line interface by causing invocations such as `./fuzzer inputfile` to fail.
I don't think this breakage was intentional since `./fuzzer inputfile1 inputfile2` still works and I don't think there is a reason to break this.
I've created a revert here: https://reviews.llvm.org/D49141



================
Comment at: lib/fuzzer/afl/afl_driver.cpp:335
-      N = atoi(argv[1] + 1);
-  else if(argc == 2 && (N = atoi(argv[1])) > 0)
-      fprintf(stderr, "WARNING: using the deprecated call style `%s %d`\n",
----------------
I don't think (N = atoi(argv[1])) > 0) should have been removed.
It allowed a single file to be passed to afl_driver.


================
Comment at: test/fuzzer/afl-driver.test:25
+RUN: not %run %t-AFLDriverTest %t.file3 2>&1 | FileCheck %s --check-prefix=CHECK4
+CHECK4: LLVMFuzzerInitialize called
 
----------------
devnexen wrote:
> morehouse wrote:
> > devnexen wrote:
> > > morehouse wrote:
> > > > devnexen wrote:
> > > > > morehouse wrote:
> > > > > > Why have you changed these two lines?
> > > > > Most likely because of the exit call. Without this, the previous test passes.
> > > > If I understand the code correctly, this should cause `N` to default to 1000.  So I don't think exit should be called.
> > > If I change the signature of set_iterations to return a value and bring back the else if (argc == 2 && ...) as it was the test passes somehow.
> > Ah, I see.  The driver doesn't support specifying `N` with the deprecated call style.  Let's move the deprecation warning to before the call to `set_iterations`, and change the above test to check for the deprecation warning + the iterations invalid warning.
> I changed it as asked and realised it indeed exited, i.e the second warning (i.e iterations invalid) displays the path of the file as argument making the conversion to number failing for sure.
I think changing this test was incorrect. The test verified that the command line interface worked a certain way. 
Thus the test WAI if this patch causes it to fail.


Repository:
  rL LLVM

https://reviews.llvm.org/D47880