Make fuzzerTestOneInput void

Java assertion errors are impossible to deduplicate and easily replaced
by an assert or a check and a custom exception.

This commit makes both variants of fuzzerTestOneInput void methods and
    adds a note about this change to the respective error message.

18 files changed, 49 insertions, 81 deletions

diff --git a/README.md b/README.md
index 9706db1a..94cd496f 100644
--- a/README.md
+++ b/README.md
@@ -136,25 +136,24 @@ may be as simple as the following Java example:
 package com.example.MyFirstFuzzTarget;
 
 class MyFirstFuzzTarget {
-    public static boolean fuzzerTestOneInput(byte[] input) {
+    public static void fuzzerTestOneInput(byte[] input) {
         ...
-        // call the function under test with arguments derived from input
+        // Call the function under test with arguments derived from input and
+        // throw an exception if something unwanted happens.
         ...
-        return false;
     }
 }
 ```
 
 A Java fuzz target class needs to define exactly one of the following functions:
 
-* `public static boolean fuzzerTestOneInput(byte[] input)`: Ideal for fuzz targets that naturally work on raw byte input (e.g.
+* `public static void fuzzerTestOneInput(byte[] input)`: Ideal for fuzz targets that naturally work on raw byte input (e.g.
   image parsers).
-* `public static boolean fuzzerTestOneInput(com.code_intelligence.api.FuzzedDataProvider data)`: A variety of types of "fuzzed
+* `public static void fuzzerTestOneInput(com.code_intelligence.api.FuzzedDataProvider data)`: A variety of types of "fuzzed
   data" is made available via the `FuzzedDataProvider` interface (see below for more information on this interface).
 
 The fuzzer will repeatedly call this function with generated inputs. All unhandled exceptions are caught and
-reported as errors. Alternatively, returning `true` from `fuzzerTestOneInput` triggers an assertion error without
-the need to throw an exception.
+reported as errors.
 
 The optional functions `public static void fuzzerInitialize()` or `public static void fuzzerInitialize(String[] args)`
 can be defined if initial setup is required. These functions will be called once before
@@ -187,7 +186,7 @@ to extract multiple values or convert the input into a valid `java.lang.String`.
 [atheris'](https://github.com/google/atheris) `FuzzedDataProvider` and libFuzzer's `FuzzedDataProvider.h` to simplify
 the task of writing JVM fuzz targets.
 
-If the function `public static boolean fuzzerTestOneInput(FuzzedDataProvider data)` is defined in the fuzz target, it will
+If the function `public static void fuzzerTestOneInput(FuzzedDataProvider data)` is defined in the fuzz target, it will
 be passed an object implementing `com.code_intelligence.jazzer.api.FuzzedDataProvider` that allows _consuming_ the raw fuzzer
 input as values of common types. This can look as follows:
 
@@ -201,7 +200,7 @@ class MySecondFuzzTarget {
         ...
     }
 
-    public static boolean fuzzerTestOneInput(FuzzedDataProvider data) {
+    public static void fuzzerTestOneInput(FuzzedDataProvider data) {
         callApi1(data.consumeInt(), data.consumeRemainingAsString());
         return false;
     }
diff --git a/driver/fuzz_target_runner.cpp b/driver/fuzz_target_runner.cpp
index eea58099..3943b5c8 100644
--- a/driver/fuzz_target_runner.cpp
+++ b/driver/fuzz_target_runner.cpp
@@ -93,13 +93,13 @@ FuzzTargetRunner::FuzzTargetRunner(
   }
   jclass_ = jvm.FindClass(FLAGS_target_class);
   // one of the following functions is required:
-  //    public static boolean fuzzerTestOneInput(byte[] input)
-  //    public static boolean fuzzerTestOneInput(FuzzedDataProvider data)
+  //    public static void fuzzerTestOneInput(byte[] input)
+  //    public static void fuzzerTestOneInput(FuzzedDataProvider data)
   fuzzer_test_one_input_bytes_ =
-      jvm.GetStaticMethodID(jclass_, "fuzzerTestOneInput", "([B)Z", false);
+      jvm.GetStaticMethodID(jclass_, "fuzzerTestOneInput", "([B)V", false);
   fuzzer_test_one_input_data_ = jvm.GetStaticMethodID(
       jclass_, "fuzzerTestOneInput",
-      "(Lcom/code_intelligence/jazzer/api/FuzzedDataProvider;)Z", false);
+      "(Lcom/code_intelligence/jazzer/api/FuzzedDataProvider;)V", false);
   bool using_bytes = fuzzer_test_one_input_bytes_ != nullptr;
   bool using_data = fuzzer_test_one_input_data_ != nullptr;
   // Fail if none ore both of the two possible fuzzerTestOneInput versions is
@@ -107,9 +107,12 @@ FuzzTargetRunner::FuzzTargetRunner(
   if (using_bytes == using_data) {
     LOG(ERROR) << FLAGS_target_class
                << " must define exactly one of the following two functions:";
-    LOG(ERROR) << "public static boolean fuzzerTestOneInput(byte[] ...)";
+    LOG(ERROR) << "public static void fuzzerTestOneInput(byte[] ...)";
     LOG(ERROR)
-        << "public static boolean fuzzerTestOneInput(FuzzedDataProvider ...)";
+        << "public static void fuzzerTestOneInput(FuzzedDataProvider ...)";
+    LOG(ERROR) << "Note: Fuzz targets returning boolean are no longer "
+                  "supported; exceptions should be thrown instead of "
+                  "returning true.";
     exit(1);
   }
 
@@ -177,12 +180,10 @@ FuzzTargetRunner::~FuzzTargetRunner() {
 
 RunResult FuzzTargetRunner::Run(const uint8_t *data, const std::size_t size) {
   auto &env = jvm_.GetEnv();
-  bool trigger_exit;
   if (fuzzer_test_one_input_data_ != nullptr) {
     FeedFuzzedDataProvider(data, size);
-    trigger_exit =
-        env.CallStaticBooleanMethod(jclass_, fuzzer_test_one_input_data_,
-                                    GetFuzzedDataProviderJavaObject(jvm_));
+    env.CallStaticVoidMethod(jclass_, fuzzer_test_one_input_data_,
+                             GetFuzzedDataProviderJavaObject(jvm_));
   } else {
     jbyteArray byte_array = env.NewByteArray(size);
     if (byte_array == nullptr) {
@@ -191,15 +192,11 @@ RunResult FuzzTargetRunner::Run(const uint8_t *data, const std::size_t size) {
     }
     env.SetByteArrayRegion(byte_array, 0, size,
                            reinterpret_cast<const jbyte *>(data));
-    trigger_exit = env.CallStaticBooleanMethod(
-        jclass_, fuzzer_test_one_input_bytes_, byte_array);
+    env.CallStaticVoidMethod(jclass_, fuzzer_test_one_input_bytes_, byte_array);
     env.DeleteLocalRef(byte_array);
   }
 
-  if (trigger_exit) {
-    std::cerr << "== Java Assertion Error" << std::endl;
-    return RunResult::kAssertion;
-  } else if (env.ExceptionOccurred()) {
+  if (env.ExceptionOccurred()) {
     jlong dedup_token = computeDedupToken();
     // Check whether this stack trace has been encountered before if
     // `--keep_going` has been supplied.
@@ -231,9 +228,8 @@ void FuzzTargetRunner::DumpReproducer(const uint8_t *data, std::size_t size) {
     // Java-only CannedFuzzedDataProvider in the reproducer.
     FeedFuzzedDataProvider(data, size);
     jobject recorder = GetRecordingFuzzedDataProviderJavaObject(jvm_);
-    bool result = env.CallStaticBooleanMethod(
-        jclass_, fuzzer_test_one_input_data_, recorder);
-    if (!result && !env.ExceptionOccurred()) {
+    env.CallStaticVoidMethod(jclass_, fuzzer_test_one_input_data_, recorder);
+    if (!env.ExceptionOccurred()) {
       LOG(ERROR) << "Failed to reproduce crash when rerunning with recorder";
       return;
     }
diff --git a/driver/fuzz_target_runner.h b/driver/fuzz_target_runner.h
index 194abc32..845acf32 100644
--- a/driver/fuzz_target_runner.h
+++ b/driver/fuzz_target_runner.h
@@ -26,7 +26,6 @@ namespace jazzer {
 
 enum class RunResult {
   kOk,
-  kAssertion,
   kException,
   kDumpAndContinue,
 };
@@ -37,9 +36,9 @@ enum class RunResult {
 //    OR
 //    - `public static void fuzzerInitialize(String[] args)`
 // 2. On every call of Run():
-//    - `public static boolean fuzzerTestOneInput(FuzzedDataProvider data)`
+//    - `public static void fuzzerTestOneInput(FuzzedDataProvider data)`
 //    OR
-//    - `public static boolean fuzzerTestOneInput(byte[] input)`
+//    - `public static void fuzzerTestOneInput(byte[] input)`
 // 3. On destruction:
 //    - `public static void fuzzerTearDown()`
 class FuzzTargetRunner : public ExceptionPrinter {
diff --git a/driver/jvm_tooling_test.cpp b/driver/jvm_tooling_test.cpp
index fa0ba747..a690be61 100644
--- a/driver/jvm_tooling_test.cpp
+++ b/driver/jvm_tooling_test.cpp
@@ -102,12 +102,6 @@ TEST_F(JvmToolingTest, SimpleFuzzTarget) {
   ASSERT_EQ(RunResult::kOk, fuzz_target_runner.Run(
                                 (const uint8_t *)input.c_str(), input.size()));
 
-  // fuzzerTestOneInput returns true - treated as an assertion failure
-  input = "true";
-  ASSERT_EQ(
-      RunResult::kAssertion,
-      fuzz_target_runner.Run((const uint8_t *)input.c_str(), input.size()));
-
   // exception is thrown in fuzzerTestOneInput
   input = "crash";
   ASSERT_EQ(
diff --git a/driver/testdata/test/FuzzTargetWithCoverage.java b/driver/testdata/test/FuzzTargetWithCoverage.java
index 7a50822a..599b1fa8 100644
--- a/driver/testdata/test/FuzzTargetWithCoverage.java
+++ b/driver/testdata/test/FuzzTargetWithCoverage.java
@@ -17,13 +17,12 @@ package test;
 import com.code_intelligence.jazzer.runtime.CoverageMap;
 
 public class FuzzTargetWithCoverage {
-  public static boolean fuzzerTestOneInput(byte[] input) {
+  public static void fuzzerTestOneInput(byte[] input) {
     // manually increase the first coverage counter
     byte counter = CoverageMap.mem.get(0);
     counter++;
     if (counter == 0)
       counter--;
     CoverageMap.mem.put(0, counter);
-    return false;
   }
 }
diff --git a/driver/testdata/test/FuzzTargetWithDataProvider.java b/driver/testdata/test/FuzzTargetWithDataProvider.java
index 7ab8d237..fc5bc1b0 100644
--- a/driver/testdata/test/FuzzTargetWithDataProvider.java
+++ b/driver/testdata/test/FuzzTargetWithDataProvider.java
@@ -26,7 +26,7 @@ class FuzzTargetWithDataProvider {
     }
   }
 
-  public strictfp static boolean fuzzerTestOneInput(FuzzedDataProvider data) {
+  public strictfp static void fuzzerTestOneInput(FuzzedDataProvider data) {
     assertEqual(true, data.consumeBoolean());
 
     assertEqual((byte) 0x7F, data.consumeByte());
@@ -110,6 +110,5 @@ class FuzzTargetWithDataProvider {
     assertEqual("", data.consumeRemainingAsString());
     assertEqual("", data.consumeAsciiString(100));
     assertEqual("", data.consumeString(100));
-    return false;
   }
 }
diff --git a/driver/testdata/test/FuzzTargetWithInit.java b/driver/testdata/test/FuzzTargetWithInit.java
index 601a4e3a..86aed82b 100644
--- a/driver/testdata/test/FuzzTargetWithInit.java
+++ b/driver/testdata/test/FuzzTargetWithInit.java
@@ -19,13 +19,12 @@ class FuzzTargetWithInit {
   public static void fuzzerInitialize(String[] args) {
     crashOnString = args;
   }
-  public static boolean fuzzerTestOneInput(byte[] input) {
+  public static void fuzzerTestOneInput(byte[] input) {
     String inputString = new String(input);
     for (String crashString : crashOnString) {
       if (inputString.equals(crashString)) {
         throw new RuntimeException("triggered the exception");
       }
     }
-    return false;
   }
 }
diff --git a/driver/testdata/test/SimpleFuzzTarget.java b/driver/testdata/test/SimpleFuzzTarget.java
index 520dba04..5657e416 100644
--- a/driver/testdata/test/SimpleFuzzTarget.java
+++ b/driver/testdata/test/SimpleFuzzTarget.java
@@ -15,12 +15,11 @@
 package test;
 
 class SimpleFuzzTarget {
-  public static boolean fuzzerTestOneInput(byte[] input) {
+  public static void fuzzerTestOneInput(byte[] input) {
     String inputString = new String(input);
     System.err.println("got input " + inputString);
     if (inputString.startsWith("crash")) {
       throw new RuntimeException("exception triggered in fuzz target");
-    } else
-      return inputString.startsWith("true");
+    }
   }
 }
diff --git a/examples/src/main/java/com/example/ExampleFuzzer.java b/examples/src/main/java/com/example/ExampleFuzzer.java
index 40dcb697..b41f9c77 100644
--- a/examples/src/main/java/com/example/ExampleFuzzer.java
+++ b/examples/src/main/java/com/example/ExampleFuzzer.java
@@ -22,7 +22,7 @@ public class ExampleFuzzer {
     // Optional initialization to be run before the first call to fuzzerTestOneInput.
   }
 
-  public static boolean fuzzerTestOneInput(FuzzedDataProvider data) {
+  public static void fuzzerTestOneInput(FuzzedDataProvider data) {
     String input = data.consumeRemainingAsString();
     // Without the hook in ExampleFuzzerHooks.java, the value of random would change on every
     // invocation, making it almost impossible to guess for the fuzzer.
@@ -31,7 +31,6 @@ public class ExampleFuzzer {
         && input.charAt(25) == 'C') {
       mustNeverBeCalled();
     }
-    return false;
   }
 
   private static void mustNeverBeCalled() {
diff --git a/examples/src/main/java/com/example/ExampleFuzzerWithNative.java b/examples/src/main/java/com/example/ExampleFuzzerWithNative.java
index 801e84ea..853501bf 100644
--- a/examples/src/main/java/com/example/ExampleFuzzerWithNative.java
+++ b/examples/src/main/java/com/example/ExampleFuzzerWithNative.java
@@ -21,14 +21,13 @@ public class ExampleFuzzerWithNative {
     System.loadLibrary("native");
   }
 
-  public static boolean fuzzerTestOneInput(FuzzedDataProvider data) {
+  public static void fuzzerTestOneInput(FuzzedDataProvider data) {
     int val = data.consumeInt();
     String stringData = data.consumeRemainingAsString();
     if (val == 17759716 && stringData.length() > 10 && stringData.contains("jazzer")) {
       // call native function which contains a crash
       new ExampleFuzzerWithNative().parse(stringData);
     }
-    return false;
   }
 
   private native boolean parse(String bytes);
diff --git a/examples/src/main/java/com/example/ExampleValueProfileFuzzer.java b/examples/src/main/java/com/example/ExampleValueProfileFuzzer.java
index 1eb55df0..1200c560 100644
--- a/examples/src/main/java/com/example/ExampleValueProfileFuzzer.java
+++ b/examples/src/main/java/com/example/ExampleValueProfileFuzzer.java
@@ -27,14 +27,14 @@ public class ExampleValueProfileFuzzer {
     return input ^ key;
   }
 
-  public static boolean fuzzerTestOneInput(FuzzedDataProvider data) {
+  public static void fuzzerTestOneInput(FuzzedDataProvider data) {
     // Without -use_value_profile=1, the fuzzer gets stuck here as there is no direct correspondence
     // between the input bytes and the compared string. With value profile, the fuzzer can guess the
     // expected input byte by byte, which takes linear rather than exponential time.
     if (base64(data.consumeBytes(6)).equals("SmF6emVy")) {
       long[] plaintextBlocks = data.consumeLongs(2);
       if (plaintextBlocks.length != 2)
-        return false;
+        return;
       if (insecureEncrypt(plaintextBlocks[0]) == 0x9fc48ee64d3dc090L) {
         // Without --fake_pcs (enabled by default with -use_value_profile=1), the fuzzer would get
         // stuck here as the value profile information for long comparisons would not be able to
@@ -44,7 +44,6 @@ public class ExampleValueProfileFuzzer {
         }
       }
     }
-    return false;
   }
 
   private static void mustNeverBeCalled() {
diff --git a/examples/src/main/java/com/example/FastJsonFuzzer.java b/examples/src/main/java/com/example/FastJsonFuzzer.java
index 5c203ffb..2e5d4797 100644
--- a/examples/src/main/java/com/example/FastJsonFuzzer.java
+++ b/examples/src/main/java/com/example/FastJsonFuzzer.java
@@ -21,12 +21,10 @@ import com.code_intelligence.jazzer.api.FuzzedDataProvider;
 // Found the issues described in
 // https://github.com/alibaba/fastjson/issues/3631
 public class FastJsonFuzzer {
-  public static boolean fuzzerTestOneInput(FuzzedDataProvider data) {
+  public static void fuzzerTestOneInput(FuzzedDataProvider data) {
     try {
       JSON.parse(data.consumeRemainingAsString());
-    } catch (JSONException e) {
-      return false;
+    } catch (JSONException ignored) {
     }
-    return false;
   }
 }
diff --git a/examples/src/main/java/com/example/GifImageParserFuzzer.java b/examples/src/main/java/com/example/GifImageParserFuzzer.java
index 276bdb84..ab7de907 100644
--- a/examples/src/main/java/com/example/GifImageParserFuzzer.java
+++ b/examples/src/main/java/com/example/GifImageParserFuzzer.java
@@ -23,12 +23,10 @@ import org.apache.commons.imaging.formats.gif.GifImageParser;
 // Found https://issues.apache.org/jira/browse/IMAGING-277 and
 // https://issues.apache.org/jira/browse/IMAGING-278.
 public class GifImageParserFuzzer {
-  public static boolean fuzzerTestOneInput(byte[] input) {
+  public static void fuzzerTestOneInput(byte[] input) {
     try {
       new GifImageParser().getBufferedImage(new ByteSourceArray(input), new HashMap<>());
-    } catch (IOException | ImageReadException e) {
-      return false;
+    } catch (IOException | ImageReadException ignored) {
     }
-    return false;
   }
 }
diff --git a/examples/src/main/java/com/example/JacksonCborFuzzer.java b/examples/src/main/java/com/example/JacksonCborFuzzer.java
index 3d847751..902c1d96 100644
--- a/examples/src/main/java/com/example/JacksonCborFuzzer.java
+++ b/examples/src/main/java/com/example/JacksonCborFuzzer.java
@@ -22,15 +22,13 @@ import java.io.IOException;
 // https://github.com/FasterXML/jackson-databind/pull/3032 if executed with
 // `--keep_going=3 -seed=2735196724`.
 public class JacksonCborFuzzer {
-  public static boolean fuzzerTestOneInput(byte[] input) {
+  public static void fuzzerTestOneInput(byte[] input) {
     CBORFactory factory = new CBORFactory();
     ObjectMapper mapper = new ObjectMapper(factory);
     mapper.enableDefaultTyping();
     try {
       mapper.readTree(input);
-    } catch (IOException e) {
-      return false;
+    } catch (IOException ignored) {
     }
-    return false;
   }
 }
diff --git a/examples/src/main/java/com/example/JpegImageParserFuzzer.java b/examples/src/main/java/com/example/JpegImageParserFuzzer.java
index 4040daee..ba3e7c81 100644
--- a/examples/src/main/java/com/example/JpegImageParserFuzzer.java
+++ b/examples/src/main/java/com/example/JpegImageParserFuzzer.java
@@ -22,12 +22,10 @@ import org.apache.commons.imaging.formats.jpeg.JpegImageParser;
 
 // Found https://issues.apache.org/jira/browse/IMAGING-275.
 public class JpegImageParserFuzzer {
-  public static boolean fuzzerTestOneInput(byte[] input) {
+  public static void fuzzerTestOneInput(byte[] input) {
     try {
       new JpegImageParser().getBufferedImage(new ByteSourceArray(input), new HashMap<>());
-    } catch (IOException | ImageReadException e) {
-      return false;
+    } catch (IOException | ImageReadException ignored) {
     }
-    return false;
   }
 }
diff --git a/examples/src/main/java/com/example/JsonSanitizerFuzzer.java b/examples/src/main/java/com/example/JsonSanitizerFuzzer.java
index ef13f369..31831616 100644
--- a/examples/src/main/java/com/example/JsonSanitizerFuzzer.java
+++ b/examples/src/main/java/com/example/JsonSanitizerFuzzer.java
@@ -20,7 +20,7 @@ import com.google.gson.JsonElement;
 import com.google.json.JsonSanitizer;
 
 public class JsonSanitizerFuzzer {
-  public static boolean fuzzerTestOneInput(FuzzedDataProvider data) {
+  public static void fuzzerTestOneInput(FuzzedDataProvider data) {
     String input = data.consumeRemainingAsString();
     String validJson;
     try {
@@ -28,15 +28,14 @@ public class JsonSanitizerFuzzer {
     } catch (ArrayIndexOutOfBoundsException e) {
       // ArrayIndexOutOfBoundsException is expected if nesting depth is
       // exceeded.
-      return false;
+      return;
     }
     Gson gson = new Gson();
     gson.fromJson(validJson, JsonElement.class);
     if (validJson.contains("</script>") || validJson.contains("<script")
         || validJson.contains("<!--") || validJson.contains("]]>")) {
       System.out.println(validJson);
-      return true;
+      throw new IllegalStateException("Output contains forbidden substring");
     }
-    return false;
   }
 }
diff --git a/examples/src/main/java/com/example/TiffImageParserFuzzer.java b/examples/src/main/java/com/example/TiffImageParserFuzzer.java
index 6dd127c2..5fa1af2d 100644
--- a/examples/src/main/java/com/example/TiffImageParserFuzzer.java
+++ b/examples/src/main/java/com/example/TiffImageParserFuzzer.java
@@ -22,12 +22,10 @@ import org.apache.commons.imaging.formats.tiff.TiffImageParser;
 
 // Found https://issues.apache.org/jira/browse/IMAGING-276.
 public class TiffImageParserFuzzer {
-  public static boolean fuzzerTestOneInput(byte[] input) {
+  public static void fuzzerTestOneInput(byte[] input) {
     try {
       new TiffImageParser().getBufferedImage(new ByteSourceArray(input), new HashMap<>());
-    } catch (IOException | ImageReadException e) {
-      return false;
+    } catch (IOException | ImageReadException ignored) {
     }
-    return false;
   }
 }
diff --git a/examples/src/main/java/com/example/TurboJpegFuzzer.java b/examples/src/main/java/com/example/TurboJpegFuzzer.java
index 0a0059e4..b9ea715b 100644
--- a/examples/src/main/java/com/example/TurboJpegFuzzer.java
+++ b/examples/src/main/java/com/example/TurboJpegFuzzer.java
@@ -29,7 +29,7 @@ public class TurboJpegFuzzer {
     new TJDecompressor();
   }
 
-  public static boolean fuzzerTestOneInput(FuzzedDataProvider data) {
+  public static void fuzzerTestOneInput(FuzzedDataProvider data) {
     try {
       int flagsDecompress = data.consumeInt();
       int flagsTransform = data.consumeInt();
@@ -52,10 +52,8 @@ public class TurboJpegFuzzer {
         tjd = new TJDecompressor(data.consumeRemainingAsBytes());
       }
       tjd.decompress(buffer, 0, 0, desiredWidth, 0, desiredHeight, pixelFormat, flagsDecompress);
-    } catch (Exception e) {
+    } catch (Exception ignored) {
       // We are not looking for Java exceptions, but segfaults and ASan reports.
-      return false;
     }
-    return false;
   }
 }