Merge pull request #777 from libretro/fix/742-harness-unknown-option

fix(tests): harness refuses an --option key the core never registered

.claude/skills/vj-debug/SKILL.md

@@ -87,8 +87,8 @@ the current directory when omitted.
   ```
   cc -O2 -Wall -std=c99 -I. -I./test/harness -I./libretro-common/include -o test/tools/frame_hash_ab \
      test/tools/frame_hash_ab.c test/harness/harness.c -ldl -lm
-  ./test/tools/frame_hash_ab ./virtualjaguar_libretro.dylib rom.jag --csv a.csv --frames 1800 --option virtualjaguar_dsp=enabled
-  ./test/tools/frame_hash_ab ./virtualjaguar_libretro.dylib rom.jag --csv b.csv --frames 1800 --option virtualjaguar_dsp=disabled
+  ./test/tools/frame_hash_ab ./virtualjaguar_libretro.dylib rom.jag --csv a.csv --frames 1800 --option virtualjaguar_usefastblitter=enabled
+  ./test/tools/frame_hash_ab ./virtualjaguar_libretro.dylib rom.jag --csv b.csv --frames 1800 --option virtualjaguar_usefastblitter=disabled
   ```
   Then diff whichever CSV/dump pair applies:
   - `field_diff` — generic row/column CSV diff (works on `frame_hash_ab`'s

test/harness/harness.c

@@ -410,8 +410,116 @@ static struct retro_microphone_interface g_synth_mic_iface = {
     synth_read_mic
 };
 
+/* Option keys the core registered (#742).  An --option key the core never
+ * registered used to be silently ignored, so a typo, a renamed option or an
+ * option missing on this branch measured the defaults and reported a
+ * confident null result (the --bios / risc_pc_histogram casualties). */
+#define HARNESS_MAX_REG_KEYS 512
+#define HARNESS_REG_KEY_LEN  64
+static char     reg_keys[HARNESS_MAX_REG_KEYS][HARNESS_REG_KEY_LEN];
+static unsigned reg_num_keys;
+
+static void reg_add_key(const char *key)
+{
+    unsigned i;
+    if (!key || !*key) return;
+    for (i = 0; i < reg_num_keys; i++)
+        if (strcmp(reg_keys[i], key) == 0) return;
+    if (reg_num_keys >= HARNESS_MAX_REG_KEYS) return;
+    snprintf(reg_keys[reg_num_keys], HARNESS_REG_KEY_LEN, "%s", key);
+    reg_num_keys++;
+}
+
+static void reg_record(unsigned cmd, const void *data)
+{
+    unsigned i;
+    if (!data) return;
+    switch (cmd) {
+    case RETRO_ENVIRONMENT_SET_VARIABLES: {
+        const struct retro_variable *v = (const struct retro_variable *)data;
+        for (i = 0; v[i].key; i++) reg_add_key(v[i].key);
+        break;
+    }
+    case RETRO_ENVIRONMENT_SET_CORE_OPTIONS: {
+        const struct retro_core_option_definition *d =
+            (const struct retro_core_option_definition *)data;
+        for (i = 0; d[i].key; i++) reg_add_key(d[i].key);
+        break;
+    }
+    case RETRO_ENVIRONMENT_SET_CORE_OPTIONS_INTL: {
+        const struct retro_core_options_intl *o =
+            (const struct retro_core_options_intl *)data;
+        if (o->us)
+            for (i = 0; o->us[i].key; i++) reg_add_key(o->us[i].key);
+        break;
+    }
+    case RETRO_ENVIRONMENT_SET_CORE_OPTIONS_V2: {
+        const struct retro_core_options_v2 *o =
+            (const struct retro_core_options_v2 *)data;
+        if (o->definitions)
+            for (i = 0; o->definitions[i].key; i++)
+                reg_add_key(o->definitions[i].key);
+        break;
+    }
+    case RETRO_ENVIRONMENT_SET_CORE_OPTIONS_V2_INTL: {
+        const struct retro_core_options_v2_intl *o =
+            (const struct retro_core_options_v2_intl *)data;
+        if (o->us && o->us->definitions)
+            for (i = 0; o->us->definitions[i].key; i++)
+                reg_add_key(o->us->definitions[i].key);
+        break;
+    }
+    default:
+        break;
+    }
+}
+
+/* Call after retro_set_environment + retro_init, before retro_load_game.
+ * An unregistered --option key is fatal (exit 2, naming the key) unless
+ * VJ_HARNESS_ALLOW_UNKNOWN_OPTIONS is set -- the escape hatch for an A/B
+ * against an older core that predates an option the tool always passes.
+ * Prints the resolved option set (stderr) so a log is self-describing. */
+static void validate_options(const harness_config *cfg)
+{
+    unsigned i, j;
+    int bad = 0;
+    const char *allow = getenv("VJ_HARNESS_ALLOW_UNKNOWN_OPTIONS");
+
+    if (cfg->num_options == 0) return;
+    if (reg_num_keys == 0) {
+        fprintf(stderr, "harness: %s core registered no option keys, so no "
+                        "--option can be validated\n",
+                (allow && *allow) ? "WARNING" : "FATAL");
+        if (!(allow && *allow)) exit(2);
+    } else {
+        for (i = 0; i < cfg->num_options; i++) {
+            for (j = 0; j < reg_num_keys; j++)
+                if (strcmp(cfg->options[i].key, reg_keys[j]) == 0) break;
+            if (j == reg_num_keys) {
+                fprintf(stderr, "harness: %s --option key '%s' is not an option "
+                                "this core registers\n",
+                        (allow && *allow) ? "WARNING" : "FATAL",
+                        cfg->options[i].key);
+                bad = 1;
+            }
+        }
+        if (bad && !(allow && *allow)) {
+            fprintf(stderr, "harness: refusing to run with an unknown option -- it "
+                            "would be ignored and the run would measure the default. "
+                            "Set VJ_HARNESS_ALLOW_UNKNOWN_OPTIONS=1 to override.\n");
+            exit(2);
+        }
+    }
+    /* Always, even under --quiet: several tools rewrite their own flags to
+     * "--quiet" placeholders, and this goes to stderr, not the data. */
+    for (i = 0; i < cfg->num_options; i++)
+        fprintf(stderr, "harness: option %s=%s\n",
+                cfg->options[i].key, cfg->options[i].value);
+}
+
 static bool cb_environment(unsigned cmd, void *data)
 {
+    reg_record(cmd, data);
     switch (cmd) {
     case RETRO_ENVIRONMENT_GET_LOG_INTERFACE:
         *(struct retro_log_callback *)data = log_cb_struct;
@@ -593,7 +701,14 @@ bool harness_init_from_args(harness_config *cfg, int argc, char **argv)
             if (eq) {
                 *eq = '\0';
                 harness_set_option(cfg, argv[i], eq + 1);
+            } else {
+                fprintf(stderr, "harness: FATAL --option '%s' is not KEY=VALUE\n",
+                        argv[i]);
+                exit(2);
             }
+        } else if (strcmp(argv[i], "--option") == 0) {
+            fprintf(stderr, "harness: FATAL --option needs a KEY=VALUE argument\n");
+            exit(2);
         } else if (argv[i][0] == '-') {
             /* Unknown flag — skip. Tools pre-parse their own flags
              * before calling harness_init_from_args. */
@@ -730,8 +845,10 @@ bool harness_load_rom(harness_config *cfg)
 
     active_cfg = cfg;
 
+    reg_num_keys = 0;   /* this core's keys only, not a previous load's */
     lr_set_environment(cb_environment);
     lr_init();
+    validate_options(cfg);
     lr_set_video_refresh(cb_video);
     lr_set_audio_sample(cb_audio_sample);
     lr_set_audio_sample_batch(cb_audio_batch);
@@ -772,8 +889,10 @@ bool harness_load_no_content(harness_config *cfg)
 {
     active_cfg = cfg;
 
+    reg_num_keys = 0;   /* this core's keys only, not a previous load's */
     lr_set_environment(cb_environment);
     lr_init();
+    validate_options(cfg);
     lr_set_video_refresh(cb_video);
     lr_set_audio_sample(cb_audio_sample);
     lr_set_audio_sample_batch(cb_audio_batch);
@@ -984,7 +1103,11 @@ void *harness_dlsym(harness_config *cfg, const char *name)
 
 void harness_set_option(harness_config *cfg, const char *key, const char *value)
 {
-    if (cfg->num_options >= HARNESS_MAX_OPTIONS) return;
+    if (cfg->num_options >= HARNESS_MAX_OPTIONS) {
+        fprintf(stderr, "harness: FATAL more than %d options; '%s' would be "
+                        "dropped\n", HARNESS_MAX_OPTIONS, key);
+        exit(2);
+    }
     cfg->options[cfg->num_options].key = key;
     cfg->options[cfg->num_options].value = value;
     cfg->num_options++;

test/harness/harness.h

@@ -40,7 +40,8 @@
  *     --json           Output machine-parseable JSON instead of human text
  *     --frames N       Override frame count to run
  *     --bios           Enable BIOS mode (default: HLE)
- *     --option K=V     Set core option (e.g. --option virtualjaguar_dsp=enabled)
+ *     --option K=V     Set core option (e.g. --option virtualjaguar_bios=enabled).
+ *                      An unregistered key is FATAL (#742)
  *     --quiet          Suppress per-frame output, only show final results
  *     --snapshot-interval N   Probe snapshot every N frames (default: 1)
  *     --load-state F   Restore a RetroArch .state after the game loads,