all: Parse agent arguments in Opt

Centralizes all options parsing in Opt.

5 files changed, 48 insertions, 74 deletions

diff --git a/README.md b/README.md
index ff5875f2..12137ae7 100644
--- a/README.md
+++ b/README.md
@@ -429,7 +429,7 @@ The particular instrumentation types to apply can be specified using the `--trac
 * `indir`: call through `Method#invoke`
 * `all`: shorthand to apply all available instrumentations (except `gep`)
 
-Multiple instrumentation types can be combined with a colon.
+Multiple instrumentation types can be combined with a colon (Linux, macOS) or a semicolon (Windows).
 
 ### Value Profile
 
diff --git a/agent/src/main/java/com/code_intelligence/jazzer/agent/Agent.kt b/agent/src/main/java/com/code_intelligence/jazzer/agent/Agent.kt
index 3690c5cf..f5fee3a2 100644
--- a/agent/src/main/java/com/code_intelligence/jazzer/agent/Agent.kt
+++ b/agent/src/main/java/com/code_intelligence/jazzer/agent/Agent.kt
@@ -36,17 +36,6 @@ import kotlin.io.path.ExperimentalPathApi
 import kotlin.io.path.exists
 import kotlin.io.path.isDirectory
 
-private val KNOWN_ARGUMENTS = listOf(
-    "instrumentation_includes",
-    "instrumentation_excludes",
-    "custom_hook_includes",
-    "custom_hook_excludes",
-    "trace",
-    "custom_hooks",
-    "disabled_hooks",
-    "dump_classes_dir",
-)
-
 // To be accessible by the agent classes the native library has to be loaded by the same class loader.
 // premain is executed in the context of the system class loader. At the beginning of premain the agent jar is added to
 // the bootstrap class loader and all subsequently required agent classes are loaded by it. Hence, it's not possible to
@@ -68,10 +57,8 @@ fun jarUriForClass(clazz: Class<*>): URI? {
     return clazz.protectionDomain?.codeSource?.location?.toURI()
 }
 
-private val argumentDelimiter =
-    if (System.getProperty("os.name").startsWith("Windows")) ";" else ":"
-
 @OptIn(ExperimentalPathApi::class)
+@Suppress("UNUSED_PARAMETER")
 fun premain(agentArgs: String?, instrumentation: Instrumentation) {
     // Add the agent jar (i.e., the jar out of which we are currently executing) to the search path of the bootstrap
     // class loader to ensure that instrumented classes can find the CoverageMap class regardless of which ClassLoader
@@ -83,45 +70,24 @@ fun premain(agentArgs: String?, instrumentation: Instrumentation) {
     }
     NativeLibraryLoader.load()
 
-    val argumentMap = (agentArgs ?: "")
-        .split(',')
-        .mapNotNull {
-            val splitArg = it.split('=', limit = 2)
-            when {
-                splitArg.size != 2 -> {
-                    if (splitArg[0].isNotEmpty())
-                        println("WARN: Ignoring argument ${splitArg[0]} without value")
-                    null
-                }
-                splitArg[0] !in KNOWN_ARGUMENTS -> {
-                    println("WARN: Ignoring unknown argument ${splitArg[0]}")
-                    null
-                }
-                else -> splitArg[0] to splitArg[1].split(argumentDelimiter)
-            }
-        }.toMap()
     val manifestCustomHookNames =
         ManifestUtils.combineManifestValues(ManifestUtils.HOOK_CLASSES).flatMap {
             it.split(':')
         }.filter { it.isNotBlank() }
-    val allCustomHookNames = manifestCustomHookNames + (argumentMap["custom_hooks"] ?: emptySet())
-    val disabledCustomHookNames = argumentMap["disabled_hooks"]?.toSet() ?: emptySet()
+    val allCustomHookNames = (manifestCustomHookNames + Opt.customHooks).toSet()
+    val disabledCustomHookNames = Opt.disabledHooks.toSet()
     val customHookNames = allCustomHookNames - disabledCustomHookNames
     val disabledCustomHooksToPrint = allCustomHookNames - customHookNames.toSet()
     if (disabledCustomHooksToPrint.isNotEmpty()) {
         println("INFO: Not using the following disabled hooks: ${disabledCustomHooksToPrint.joinToString(", ")}")
     }
 
-    val classNameGlobber = ClassNameGlobber(
-        argumentMap["instrumentation_includes"] ?: emptyList(),
-        (argumentMap["instrumentation_excludes"] ?: emptyList()) + customHookNames
-    )
+    val classNameGlobber = ClassNameGlobber(Opt.instrumentationIncludes, Opt.instrumentationExcludes + customHookNames)
     CoverageRecorder.classNameGlobber = classNameGlobber
-    val customHookClassNameGlobber = ClassNameGlobber(
-        argumentMap["custom_hook_includes"] ?: emptyList(),
-        (argumentMap["custom_hook_excludes"] ?: emptyList()) + customHookNames
-    )
-    val instrumentationTypes = (argumentMap["trace"] ?: listOf("all")).flatMap {
+    val customHookClassNameGlobber = ClassNameGlobber(Opt.customHookIncludes, Opt.customHookExcludes + customHookNames)
+    // FIXME: Setting trace to the empty string explicitly results in all rather than no trace types
+    //  being applied - this is unintuitive.
+    val instrumentationTypes = (Opt.trace.takeIf { it.isNotEmpty() } ?: listOf("all")).flatMap {
         when (it) {
             "cmp" -> setOf(InstrumentationType.CMP)
             "cov" -> setOf(InstrumentationType.COV)
@@ -145,8 +111,8 @@ fun premain(agentArgs: String?, instrumentation: Instrumentation) {
             println("INFO: Synchronizing coverage IDs in ${path.toAbsolutePath()}")
         }
     }
-    val dumpClassesDir = argumentMap["dump_classes_dir"]?.let {
-        Paths.get(it.single()).toAbsolutePath().also { path ->
+    val dumpClassesDir = Opt.dumpClassesDir.takeUnless { it.isEmpty() }?.let {
+        Paths.get(it).toAbsolutePath().also { path ->
             if (path.exists() && path.isDirectory()) {
                 println("INFO: Dumping instrumented classes into $path")
             } else {
diff --git a/driver/jvm_tooling.cpp b/driver/jvm_tooling.cpp
index 6ac50405..9afeeb4b 100644
--- a/driver/jvm_tooling.cpp
+++ b/driver/jvm_tooling.cpp
@@ -72,7 +72,8 @@ DEFINE_string(disabled_hooks, "",
               "loaded (separator is ':' on Linux/macOS and ';' on Windows)");
 DEFINE_string(
     trace, "",
-    "list of instrumentation to perform separated by colon \":\". "
+    "list of instrumentation to perform separated by colon ':' on Linux/macOS "
+    "and ';' on Windows. "
     "Available options are cov, cmp, div, gep, all. These options "
     "correspond to the \"-fsanitize-coverage=trace-*\" flags in clang.");
 DEFINE_string(
@@ -192,27 +193,6 @@ std::string getInstrumentorAgentPath(std::string_view executable_path) {
   exit(1);
 }
 
-std::string agentArgsFromFlags() {
-  std::vector<std::string> args;
-  for (const auto &flag_pair :
-       std::vector<std::pair<std::string, const std::string &>>{
-           // {<agent option>, <ref to glog flag> }
-           {"instrumentation_includes", FLAGS_instrumentation_includes},
-           {"instrumentation_excludes", FLAGS_instrumentation_excludes},
-           {"custom_hooks", FLAGS_custom_hooks},
-           {"disabled_hooks", FLAGS_disabled_hooks},
-           {"custom_hook_includes", FLAGS_custom_hook_includes},
-           {"custom_hook_excludes", FLAGS_custom_hook_excludes},
-           {"trace", FLAGS_trace},
-           {"dump_classes_dir", FLAGS_dump_classes_dir},
-       }) {
-    if (!flag_pair.second.empty()) {
-      args.push_back(flag_pair.first + "=" + flag_pair.second);
-    }
-  }
-  return absl::StrJoin(args, ",");
-}
-
 std::vector<std::string> optsAsDefines() {
   return {
       absl::StrFormat("-Djazzer.target_class=%s", FLAGS_target_class),
@@ -226,8 +206,19 @@ std::vector<std::string> optsAsDefines() {
       absl::StrFormat("-Djazzer.autofuzz=%s", FLAGS_autofuzz),
       absl::StrFormat("-Djazzer.autofuzz_ignore=%s", FLAGS_autofuzz_ignore),
       absl::StrFormat("-Djazzer.hooks=%s", FLAGS_hooks ? "true" : "false"),
-      absl::StrFormat("-Djazzer.agent_args=%s", agentArgsFromFlags()),
       absl::StrFormat("-Djazzer.id_sync_file=%s", FLAGS_id_sync_file),
+      absl::StrFormat("-Djazzer.instrumentation_includes=%s",
+                      FLAGS_instrumentation_includes),
+      absl::StrFormat("-Djazzer.instrumentation_excludes=%s",
+                      FLAGS_instrumentation_excludes),
+      absl::StrFormat("-Djazzer.custom_hooks=%s", FLAGS_custom_hooks),
+      absl::StrFormat("-Djazzer.disabled_hooks=%s", FLAGS_disabled_hooks),
+      absl::StrFormat("-Djazzer.custom_hook_includes=%s",
+                      FLAGS_custom_hook_includes),
+      absl::StrFormat("-Djazzer.custom_hook_excludes=%s",
+                      FLAGS_custom_hook_excludes),
+      absl::StrFormat("-Djazzer.trace=%s", FLAGS_trace),
+      absl::StrFormat("-Djazzer.dump_classes_dir=%s", FLAGS_dump_classes_dir),
   };
 }
 
diff --git a/driver/src/main/java/com/code_intelligence/jazzer/driver/Driver.java b/driver/src/main/java/com/code_intelligence/jazzer/driver/Driver.java
index 5dd05d36..05e1a582 100644
--- a/driver/src/main/java/com/code_intelligence/jazzer/driver/Driver.java
+++ b/driver/src/main/java/com/code_intelligence/jazzer/driver/Driver.java
@@ -90,7 +90,7 @@ public class Driver {
     // effect.
 
     if (Opt.hooks) {
-      Agent.premain(Opt.agentArgs, ByteBuddyAgent.install());
+      Agent.premain(null, ByteBuddyAgent.install());
     }
 
     return FuzzTargetRunner.startLibFuzzer(args);
diff --git a/driver/src/main/java/com/code_intelligence/jazzer/driver/Opt.java b/driver/src/main/java/com/code_intelligence/jazzer/driver/Opt.java
index 509e5084..417bfd5e 100644
--- a/driver/src/main/java/com/code_intelligence/jazzer/driver/Opt.java
+++ b/driver/src/main/java/com/code_intelligence/jazzer/driver/Opt.java
@@ -34,23 +34,32 @@ import java.util.stream.Stream;
  * <p>Every public field should be deeply immutable.
  */
 public final class Opt {
-  public static final String agentArgs = stringSetting("agent_args", "");
+  private static final char SYSTEM_DELIMITER =
+      System.getProperty("os.name").startsWith("Windows") ? ';' : ':';
+
   public static final String autofuzz = stringSetting("autofuzz", "");
   public static final List<String> autofuzzIgnore = stringListSetting("autofuzz_ignore", ',');
   public static final String coverageDump = stringSetting("coverage_dump", "");
   public static final String coverageReport = stringSetting("coverage_report", "");
+  public static final List<String> customHookIncludes = stringListSetting("custom_hook_includes");
+  public static final List<String> customHookExcludes = stringListSetting("custom_hook_excludes");
+  public static final List<String> customHooks = stringListSetting("custom_hooks");
+  public static final List<String> disabledHooks = stringListSetting("disabled_hooks");
+  public static final String dumpClassesDir = stringSetting("dump_classes_dir", "");
   public static final boolean hooks = boolSetting("hooks", true);
-  // Default to false if hooks is false to mimic the original behavior of the native fuzz target
-  // runner, but still support hooks = false && dedup = true.
-  public static boolean dedup = boolSetting("dedup", hooks);
   public static final String idSyncFile = stringSetting("id_sync_file", null);
+  public static final List<String> instrumentationIncludes =
+      stringListSetting("instrumentation_includes");
+  public static final List<String> instrumentationExcludes =
+      stringListSetting("instrumentation_excludes");
   public static final Set<Long> ignore =
       Collections.unmodifiableSet(stringListSetting("ignore", ',')
                                       .stream()
                                       .map(Long::parseUnsignedLong)
                                       .collect(Collectors.toSet()));
-  public static final String targetClass = stringSetting("target_class", "");
   public static final String reproducerPath = stringSetting("reproducer_path", ".");
+  public static final String targetClass = stringSetting("target_class", "");
+  public static final List<String> trace = stringListSetting("trace");
 
   // The values of these settings depend on autofuzz.
   public static final List<String> targetArgs = autofuzz.isEmpty()
@@ -60,6 +69,10 @@ public final class Opt {
   public static final long keepGoing =
       uint32Setting("keep_going", autofuzz.isEmpty() ? 1 : Integer.MIN_VALUE);
 
+  // Default to false if hooks is false to mimic the original behavior of the native fuzz target
+  // runner, but still support hooks = false && dedup = true.
+  public static boolean dedup = boolSetting("dedup", hooks);
+
   static {
     if (!targetClass.isEmpty() && !autofuzz.isEmpty()) {
       err.println("--target_class and --autofuzz cannot be specified together");
@@ -86,6 +99,10 @@ public final class Opt {
     return System.getProperty(optionsPrefix + name, defaultValue);
   }
 
+  private static List<String> stringListSetting(String name) {
+    return stringListSetting(name, SYSTEM_DELIMITER);
+  }
+
   private static List<String> stringListSetting(String name, char separator) {
     String value = System.getProperty(optionsPrefix + name);
     if (value == null || value.isEmpty()) {