all: Rework Opt value handling (#767)

all: Rework Opt value handling

This is a complete rewrite of the logic underlying `Opt`, with the
following notable changes:
* All options are now evaluated lazily, with safeguards ensuring that
  there can be no attempts to modify their defaults after they have been
  read once. This reduces the risk of accidentally locking in `Opt`
  and then reading stale values due to its previous reliance on static
  initializers.
* There are now clear precedence rules for all options and every option
  can be set from all possible sources: manifest entries, env variables,
  system properties, configuration parameters, command-line arguments.

In follow-up PRs, the new capabilities of `OptItem` (e.g. accumulating
settings and JUnit configuration parameters as sources) will be used in
Jazzer.

Co-authored-by: Brian Lewis <[email protected]>

Author: fmeum

docs/common.md

@@ -1,12 +1,26 @@
 ## Common options and workflows
 
+* [Passing arguments](#passing-arguments)
 * [Reproducing a finding](#reproducing-a-finding)
 * [Minimizing a crashing input](#minimizing-a-crashing-input)
 * [Parallel execution](#parallel-execution)
 * [Autofuzz mode](#autofuzz-mode)
 
 <!-- Created by https://github.com/ekalinin/github-markdown-toc -->
 
+### Passing arguments
+
+Jazzer provides many configuration settings. An up-to-date list can be found by running Jazzer with the `--help` flag.
+
+The value of a setting item `some_opt` is obtained from the following sources in increasing order of precedence:
+
+- the default value
+- `META-INF/MANIFEST.MF` attribute `Jazzer-Some-Opt` on the classpath
+- the `JAZZER_SOME_OPT` environment variable
+- the `jazzer.some_opt` system property
+- the `jazzer.some_opt` JUnit configuration parameter
+- the `--some_opt` CLI parameter
+
 ### Reproducing a finding
 
 When Jazzer manages to find an input that causes an uncaught exception or a failed assertion, it prints a Java stack trace and creates two files that aid in reproducing the crash without Jazzer:
@@ -55,4 +69,3 @@ Creating objects from fuzzer input can lead to many reported exceptions.
 Jazzer addresses this issue by ignoring exceptions that the target method declares to throw.
 In addition to that, you can provide a list of exceptions to be ignored during fuzzing via the `--autofuzz_ignore` flag in the form of a comma-separated list.
 You can specify concrete exceptions (e.g., `java.lang.NullPointerException`), in which case also subclasses of these exception classes will be ignored, or glob patterns to ignore all exceptions in a specific package (e.g. `java.lang.*` or `com.company.**`).
-

src/main/java/com/code_intelligence/jazzer/BUILD.bazel

@@ -118,6 +118,7 @@ java_library(
     deps = [
         "//src/main/java/com/code_intelligence/jazzer/android:android_runtime",
         "//src/main/java/com/code_intelligence/jazzer/driver",
+        "//src/main/java/com/code_intelligence/jazzer/driver:opt",
         "//src/main/java/com/code_intelligence/jazzer/runtime:constants",
         "//src/main/java/com/code_intelligence/jazzer/utils:log",
         "//src/main/java/com/code_intelligence/jazzer/utils:zip_utils",

src/main/java/com/code_intelligence/jazzer/Jazzer.java

@@ -26,6 +26,7 @@
 
 import com.code_intelligence.jazzer.android.AndroidRuntime;
 import com.code_intelligence.jazzer.driver.Driver;
+import com.code_intelligence.jazzer.driver.Opt;
 import com.code_intelligence.jazzer.utils.Log;
 import com.code_intelligence.jazzer.utils.ZipUtils;
 import com.github.fmeum.rules_jni.RulesJni;
@@ -42,7 +43,7 @@
 import java.nio.file.Paths;
 import java.nio.file.attribute.FileAttribute;
 import java.nio.file.attribute.PosixFilePermissions;
-import java.util.AbstractMap.SimpleEntry;
+import java.util.AbstractMap.SimpleImmutableEntry;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
@@ -80,16 +81,18 @@ private static void start(List<String> args) throws IOException, InterruptedExce
     // itself is "silenced" by redirecting System.out and/or System.err.
     Log.fixOutErr(System.out, System.err);
 
-    parseJazzerArgsToProperties(args);
+    Opt.registerAndValidateCommandLineArgs(parseJazzerArgs(args));
+    Opt.handleHelpAndVersionArgs();
 
     // --asan and --ubsan imply --native by default, but --native can also be used by itself to fuzz
     // native libraries without sanitizers (e.g. to quickly grow a corpus).
-    final boolean loadASan = Boolean.parseBoolean(System.getProperty("jazzer.asan", "false"));
-    final boolean loadUBSan = Boolean.parseBoolean(System.getProperty("jazzer.ubsan", "false"));
-    final boolean loadHWASan = Boolean.parseBoolean(System.getProperty("jazzer.hwasan", "false"));
-    final boolean fuzzNative = Boolean.parseBoolean(
-        System.getProperty("jazzer.native", Boolean.toString(loadASan || loadUBSan || loadHWASan)));
-    if ((loadASan || loadUBSan || loadHWASan) && !fuzzNative) {
+    final boolean loadASan = Opt.asan.get();
+    final boolean loadUBSan = Opt.ubsan.get();
+    final boolean loadHWASan = Opt.hwasan.get();
+    final boolean needsNative = loadASan || loadUBSan || loadHWASan;
+    Opt.fuzzNative.setIfDefault(needsNative);
+    final boolean fuzzNative = Opt.fuzzNative.get();
+    if (needsNative && !fuzzNative) {
       Log.error("--asan, --hwasan and --ubsan cannot be used without --native");
       exit(1);
     }
@@ -182,28 +185,28 @@ private static void start(List<String> args) throws IOException, InterruptedExce
     exit(processBuilder.start().waitFor());
   }
 
-  private static void parseJazzerArgsToProperties(List<String> args) {
-    args.stream()
+  private static List<Map.Entry<String, String>> parseJazzerArgs(List<String> args) {
+    return args.stream()
         .filter(arg -> arg.startsWith("--"))
         .map(arg -> arg.substring("--".length()))
         // Filter out "--", which can be used to declare that all further arguments aren't libFuzzer
         // arguments.
         .filter(arg -> !arg.isEmpty())
         .map(Jazzer::parseSingleArg)
-        .forEach(e -> System.setProperty("jazzer." + e.getKey(), e.getValue()));
+        .collect(toList());
   }
 
-  private static SimpleEntry<String, String> parseSingleArg(String arg) {
+  private static SimpleImmutableEntry<String, String> parseSingleArg(String arg) {
     String[] nameAndValue = arg.split("=", 2);
     if (nameAndValue.length == 2) {
       // Example: --keep_going=10 --> (keep_going, 10)
-      return new SimpleEntry<>(nameAndValue[0], nameAndValue[1]);
+      return new SimpleImmutableEntry<>(nameAndValue[0], nameAndValue[1]);
     } else if (nameAndValue[0].startsWith("no")) {
       // Example: --nohooks --> (hooks, "false")
-      return new SimpleEntry<>(nameAndValue[0].substring("no".length()), "false");
+      return new SimpleImmutableEntry<>(nameAndValue[0].substring("no".length()), "false");
     } else {
       // Example: --dedup --> (dedup, "true")
-      return new SimpleEntry<>(nameAndValue[0], "true");
+      return new SimpleImmutableEntry<>(nameAndValue[0], "true");
     }
   }
 
@@ -280,9 +283,8 @@ private static Stream<String> javaBinaryArgs() throws IOException {
       Path agentPath =
           RulesJni.extractLibrary("android_native_agent", "/com/code_intelligence/jazzer/android");
 
-      String jazzerAgentPath = System.getProperty("jazzer.agent_path");
-      String bootclassClassOverrides =
-          System.getProperty("jazzer.android_bootpath_classes_overrides");
+      String jazzerAgentPath = Opt.agentPath.get();
+      String bootclassClassOverrides = Opt.androidBootclassClassesOverrides.get();
 
       String jazzerBootstrapJarPath =
           "com/code_intelligence/jazzer/android/jazzer_bootstrap_android.jar";
@@ -484,7 +486,7 @@ private static boolean isPosix() {
 
   private static String getAndroidRuntimeOptions() {
     List<String> validInitOptions = Arrays.asList("use_platform_libs", "use_none", "");
-    String initOptString = System.getProperty("jazzer.android_init_options");
+    String initOptString = Opt.androidInitOptions.get();
     if (!validInitOptions.contains(initOptString)) {
       Log.error("Invalid android_init_options set for Android Runtime.");
       exit(1);

src/main/java/com/code_intelligence/jazzer/agent/Agent.kt

@@ -35,16 +35,16 @@ fun install(instrumentation: Instrumentation) {
 
 fun installInternal(
     instrumentation: Instrumentation,
-    userHookNames: List<String> = findManifestCustomHookNames() + Opt.customHooks,
-    disabledHookNames: List<String> = Opt.disabledHooks,
+    userHookNames: List<String> = findManifestCustomHookNames() + Opt.customHooks.get(),
+    disabledHookNames: List<String> = Opt.disabledHooks.get(),
     instrumentationIncludes: List<String> = Opt.instrumentationIncludes.get(),
     instrumentationExcludes: List<String> = Opt.instrumentationExcludes.get(),
     customHookIncludes: List<String> = Opt.customHookIncludes.get(),
     customHookExcludes: List<String> = Opt.customHookExcludes.get(),
-    trace: List<String> = Opt.trace,
-    idSyncFile: String? = Opt.idSyncFile,
-    dumpClassesDir: String = Opt.dumpClassesDir,
-    additionalClassesExcludes: List<String> = Opt.additionalClassesExcludes,
+    trace: List<String> = Opt.trace.get(),
+    idSyncFile: String = Opt.idSyncFile.get(),
+    dumpClassesDir: String = Opt.dumpClassesDir.get(),
+    additionalClassesExcludes: List<String> = Opt.additionalClassesExcludes.get(),
 ) {
     val allCustomHookNames = (Constants.SANITIZER_HOOK_NAMES + userHookNames).toSet()
     check(allCustomHookNames.isNotEmpty()) { "No hooks registered; expected at least the built-in hooks" }
@@ -79,7 +79,7 @@ fun installInternal(
         }
     }.toSet()
 
-    val idSyncFilePath = idSyncFile?.takeUnless { it.isEmpty() }?.let {
+    val idSyncFilePath = idSyncFile.takeUnless { it.isEmpty() }?.let {
         Paths.get(it).also { path ->
             Log.info("Synchronizing coverage IDs in ${path.toAbsolutePath()}")
         }

src/main/java/com/code_intelligence/jazzer/agent/RuntimeInstrumentor.kt

@@ -59,7 +59,7 @@ class RuntimeInstrumentor(
         classfileBuffer: ByteArray,
     ): ByteArray? {
         var pathPrefix = ""
-        if (!Opt.instrumentOnly.isEmpty() && protectionDomain != null) {
+        if (Opt.instrumentOnly.get().isNotEmpty() && protectionDomain != null) {
             var outputPathPrefix = protectionDomain.getCodeSource().getLocation().getFile().toString()
             if (outputPathPrefix.isNotEmpty()) {
                 if (outputPathPrefix.contains(File.separator)) {
@@ -208,7 +208,7 @@ class RuntimeInstrumentor(
     }
 
     private fun instrument(internalClassName: String, bytecode: ByteArray, fullInstrumentation: Boolean): ByteArray {
-        val classWithHooksEnabledField = if (Opt.conditionalHooks) {
+        val classWithHooksEnabledField = if (Opt.conditionalHooks.get()) {
             // Let the hook instrumentation emit additional logic that checks the value of the
             // hooksEnabled field on this class and skips the hook if it is false.
             "com/code_intelligence/jazzer/runtime/JazzerInternal"

src/main/java/com/code_intelligence/jazzer/android/AndroidRuntime.java

@@ -14,6 +14,7 @@
 
 package com.code_intelligence.jazzer.android;
 
+import com.code_intelligence.jazzer.driver.Opt;
 import com.code_intelligence.jazzer.utils.Log;
 import com.github.fmeum.rules_jni.RulesJni;
 
@@ -55,7 +56,7 @@ public static String getClassPathsCommand() {
    * @return The string for LD_LIBRARY_PATH.
    */
   public static String getLdLibraryPath() {
-    String initOptString = System.getProperty("jazzer.android_init_options");
+    String initOptString = Opt.androidInitOptions.get();
     if (initOptString.equals(DO_NOT_INITIALIZE) || initOptString.equals("")) {
       return FUZZ_DIR;
     }

src/main/java/com/code_intelligence/jazzer/android/BUILD.bazel

@@ -92,6 +92,7 @@ java_jni_library(
         "//src/main/native/com/code_intelligence/jazzer/driver:__subpackages__",
     ],
     deps = [
+        "//src/main/java/com/code_intelligence/jazzer/driver:opt",
         "//src/main/java/com/code_intelligence/jazzer/utils:log",
     ],
 )

src/main/java/com/code_intelligence/jazzer/driver/BUILD.bazel

@@ -148,17 +148,30 @@ java_library(
         "OptParser.java",
     ],
     visibility = [
+        # Do not add //src/main/java/com/code_intelligence/jazzer/api to this list - it must
+        # function even if it isn't running within Jazzer.
+        "//src/main/java/com/code_intelligence/jazzer:__pkg__",
         "//src/main/java/com/code_intelligence/jazzer/agent:__pkg__",
+        "//src/main/java/com/code_intelligence/jazzer/android:__pkg__",
         "//src/main/java/com/code_intelligence/jazzer/driver:__subpackages__",
         "//src/main/java/com/code_intelligence/jazzer/junit:__pkg__",
         "//src/test/java/com/code_intelligence/jazzer/driver:__subpackages__",
     ],
     deps = [
+        ":opt_item",
         "//src/main/java/com/code_intelligence/jazzer:constants",
         "//src/main/java/com/code_intelligence/jazzer/utils:log",
     ],
 )
 
+java_library(
+    name = "opt_item",
+    srcs = ["OptItem.java"],
+    visibility = [
+        "//src/test/java/com/code_intelligence/jazzer/driver:__pkg__",
+    ],
+)
+
 java_library(
     name = "recording_fuzzed_data_provider",
     srcs = ["RecordingFuzzedDataProvider.java"],

src/main/java/com/code_intelligence/jazzer/driver/Driver.java

@@ -34,31 +34,31 @@
 public class Driver {
   public static int start(List<String> args, boolean spawnsSubprocesses) throws IOException {
     if (IS_ANDROID) {
-      if (!System.getProperty("jazzer.autofuzz", "").isEmpty()) {
+      if (!Opt.autofuzz.get().isEmpty()) {
         Log.error("--autofuzz is not supported for Android");
         return 1;
       }
-      if (!System.getProperty("jazzer.coverage_report", "").isEmpty()) {
-        Log.warn("--coverage_report is not supported for Android and has been disabled");
-        System.clearProperty("jazzer.coverage_report");
+      if (!Opt.coverageReport.get().isEmpty()) {
+        Log.error("--coverage_report is not supported for Android and has been disabled");
+        return 1;
       }
-      if (!System.getProperty("jazzer.coverage_dump", "").isEmpty()) {
-        Log.warn("--coverage_dump is not supported for Android and has been disabled");
-        System.clearProperty("jazzer.coverage_dump");
+      if (!Opt.coverageDump.get().isEmpty()) {
+        Log.error("--coverage_dump is not supported for Android and has been disabled");
+        return 1;
       }
     }
 
     if (spawnsSubprocesses) {
-      if (!System.getProperty("jazzer.coverage_report", "").isEmpty()) {
+      if (!Opt.coverageReport.get().isEmpty()) {
         Log.warn("--coverage_report does not support parallel fuzzing and has been disabled");
-        System.clearProperty("jazzer.coverage_report");
+        return 1;
       }
-      if (!System.getProperty("jazzer.coverage_dump", "").isEmpty()) {
+      if (!Opt.coverageDump.get().isEmpty()) {
         Log.warn("--coverage_dump does not support parallel fuzzing and has been disabled");
-        System.clearProperty("jazzer.coverage_dump");
+        return 1;
       }
 
-      String idSyncFileArg = System.getProperty("jazzer.id_sync_file", "");
+      String idSyncFileArg = Opt.idSyncFile.get();
       Path idSyncFile;
       if (idSyncFileArg.isEmpty()) {
         // Create an empty temporary file used for coverage ID synchronization and
@@ -84,7 +84,7 @@ public static int start(List<String> args, boolean spawnsSubprocesses) throws IO
     }
 
     if (args.stream().anyMatch("-merge_inner=1" ::equals)) {
-      System.setProperty("jazzer.internal.merge_inner", "true");
+      Opt.mergeInner.setIfDefault(true);
     }
 
     // Jazzer's hooks use deterministic randomness and thus require a seed. Search for the last
@@ -108,9 +108,12 @@ public static int start(List<String> args, boolean spawnsSubprocesses) throws IO
       args.add(getDefaultRssLimitMbArg());
     }
 
-    // Do not modify properties beyond this point, loading Opt locks in their values.
-    if (!Opt.instrumentOnly.isEmpty()) {
-      boolean instrumentationSuccess = OfflineInstrumentor.instrumentJars(Opt.instrumentOnly);
+    if (!Opt.instrumentOnly.get().isEmpty()) {
+      if (Opt.dumpClassesDir.get().isEmpty()) {
+        Log.error("--dump_classes_dir must be set with --instrument_only");
+        exit(1);
+      }
+      boolean instrumentationSuccess = OfflineInstrumentor.instrumentJars(Opt.instrumentOnly.get());
       if (!instrumentationSuccess) {
         exit(1);
       }
@@ -119,8 +122,8 @@ public static int start(List<String> args, boolean spawnsSubprocesses) throws IO
 
     Driver.class.getClassLoader().setDefaultAssertionStatus(true);
 
-    if (!Opt.autofuzz.isEmpty()) {
-      AgentInstaller.install(Opt.hooks);
+    if (!Opt.autofuzz.get().isEmpty()) {
+      AgentInstaller.install(Opt.hooks.get());
       FuzzTargetHolder.fuzzTarget = FuzzTargetHolder.AUTOFUZZ_FUZZ_TARGET;
       return FuzzTargetRunner.startLibFuzzer(args);
     }
@@ -142,7 +145,7 @@ public static int start(List<String> args, boolean spawnsSubprocesses) throws IO
 
     // Installing the agent after the following "findFuzzTarget" leads to an asan error
     // in it on "Class.forName(targetClassName)", but only during native fuzzing.
-    AgentInstaller.install(Opt.hooks);
+    AgentInstaller.install(Opt.hooks.get());
     FuzzTargetHolder.fuzzTarget = FuzzTargetFinder.findFuzzTarget(targetClassName);
     return FuzzTargetRunner.startLibFuzzer(args);
   }

src/main/java/com/code_intelligence/jazzer/driver/FuzzTargetFinder.java

@@ -38,8 +38,8 @@ class FuzzTargetFinder {
   private static final String FUZZER_TEAR_DOWN = "fuzzerTearDown";
 
   static String findFuzzTargetClassName() {
-    if (!Opt.targetClass.isEmpty()) {
-      return Opt.targetClass;
+    if (!Opt.targetClass.get().isEmpty()) {
+      return Opt.targetClass.get();
     }
     if (IS_ANDROID) {
       // Fuzz target detection tools aren't supported on android
@@ -72,7 +72,7 @@ static FuzzTarget findFuzzTarget(String targetClassName) {
   // Finds the traditional static fuzzerTestOneInput fuzz target method.
   private static FuzzTarget findFuzzTargetByMethodName(Class<?> clazz) {
     Method fuzzTargetMethod;
-    if (Opt.experimentalMutator) {
+    if (Opt.experimentalMutator.get()) {
       List<Method> fuzzTargetMethods =
           Arrays.stream(clazz.getMethods())
               .filter(method -> "fuzzerTestOneInput".equals(method.getName()))
@@ -105,7 +105,7 @@ private static FuzzTarget findFuzzTargetByMethodName(Class<?> clazz) {
         Stream
             .of(targetPublicStaticMethod(clazz, FUZZER_INITIALIZE, String[].class)
                     .map(init -> (Callable<Object>) () -> {
-                      init.invoke(null, (Object) Opt.targetArgs.toArray(new String[] {}));
+                      init.invoke(null, (Object) Opt.targetArgs.get().toArray(new String[] {}));
                       return null;
                     }),
                 targetPublicStaticMethod(clazz, FUZZER_INITIALIZE)