InauguralSystems · Nitjsefnie · Aug 2, 2026
@@ -373,14 +373,30 @@ const char *op_name(uint8_t op) {
         [OP_DEFAULT_PARAM] = "DEFAULT_PARAM",
         [OP_DESTRUCTURE_UNPACK] = "DESTRUCTURE_UNPACK",
         [OP_SLICE_GET] = "SLICE_GET",
+        /* #737: these four drifted out of the table and disassembled as
+         * "???" — their siblings (REPORT_VALUE_*, OBSERVE_ASSIGN*) were
+         * present, which is exactly how the omission went unnoticed. */
+        [OP_REPORT_SLOT] = "REPORT_SLOT",
+        [OP_REPORT_NAME] = "REPORT_NAME",
+        [OP_OBSERVE_VALUE_SLOT] = "OBSERVE_VALUE_SLOT",
+        [OP_OBSERVE_VALUE_NAME] = "OBSERVE_VALUE_NAME",
     };
     if (op < OP_COUNT && names[op]) return names[op];
     return "???";
 }
 
-/* Returns 1 if opcode has a 16-bit operand */
-static int op_has_u16(uint8_t op) {
+/* Number of 16-bit operands an opcode carries (0..3). Mirrors the operand
+ * layout the VM decodes in vm.c — keep in lockstep if an opcode changes.
+ * (Was op_has_u16, a boolean — which structurally could not express the
+ * multi-operand superinstructions, so chunk_disassemble desynchronized on
+ * them and on the operand-carrying opcodes missing here entirely, #737.)
+ *
+ * Deliberately switched on OpCode with NO default arm: -Werror=switch
+ * (Makefile:9) then makes a missing case — e.g. a newly added opcode — a
+ * BUILD ERROR instead of a silently wrong operand width. */
+static int op_u16_operand_count(OpCode op) {
     switch (op) {
+    /* One u16 operand */
     case OP_CONST: case OP_GET_LOCAL: case OP_SET_LOCAL:
     case OP_GET_NAME: case OP_SET_NAME: case OP_SET_NAME_LOCAL:
     case OP_SET_FN_NAME_LOCAL:
@@ -394,16 +410,50 @@ static int op_has_u16(uint8_t op) {
     case OP_TRY_BEGIN: case OP_LOOP_STALL_CHECK: case OP_LOOP_CAP_CHECK:
     case OP_OBSERVE_ASSIGN: case OP_OBSERVE_ASSIGN_LOCAL:
     case OP_IMPORT: case OP_MATCH:
+    case OP_DESTRUCTURE_UNPACK:
+    case OP_REPORT_SLOT: case OP_REPORT_NAME:
+    case OP_OBSERVE_VALUE_SLOT: case OP_OBSERVE_VALUE_NAME:
+    case OP_OBSERVE_NAME_POST:
     case OP_REPORT_VALUE_SLOT: case OP_REPORT_VALUE_NAME:
     case OP_TRAJECTORY_SLOT: case OP_TRAJECTORY_NAME:
         return 1;
     case OP_INTERROGATE: case OP_PREDICATE:
         return 1; /* kind:8 but padded to 16 for uniformity */
+    /* Two u16 operands */
+    case OP_PREDICATE_SLOT: case OP_PREDICATE_NAME:
+    case OP_DEFAULT_PARAM:
+    case OP_LOCAL_DOT_GET: case OP_LOCAL_DOT_SET:
+    case OP_LOCAL_IDX_GET:
+    case OP_INTERROGATE_NAMED: case OP_INTERROGATE_NAMED_AT:
+        return 2;
+    /* Three u16 operands */
+    case OP_LOCAL_IDX_DOT_GET: case OP_LOCAL_IDX_DOT_SET:
+        return 3;
     /* OP_LINE has a 32-bit operand (#630) — handled separately by the
-     * disassembler and verifier, never through the u16 path. */
-    default:
+     * disassembler and verifier, never through the u16 path. OP_WIDE is an
+     * unimplemented placeholder, never emitted. The rest carry no operand. */
+    case OP_LINE: case OP_WIDE:
+    case OP_NULL: case OP_NUM_ZERO: case OP_NUM_ONE:
+    case OP_ADD: case OP_SUB: case OP_MUL: case OP_DIV: case OP_MOD:
+    case OP_BAND: case OP_BOR: case OP_BXOR: case OP_SHL: case OP_SHR:
+    case OP_NEG: case OP_NOT: case OP_BNOT:
+    case OP_EQ: case OP_NE: case OP_LT: case OP_GT: case OP_LE: case OP_GE:
+    case OP_POP: case OP_DUP: case OP_DUP2:
+    case OP_RETURN: case OP_RETURN_NULL:
+    case OP_INDEX_GET: case OP_INDEX_SET:
+    case OP_ITER_SETUP:
+    case OP_LOOP_ENV_FRESH: case OP_LOOP_ENV_END: case OP_LOOP_ENV_CLEAR:
+    case OP_BREAK: case OP_CONTINUE:
+    case OP_TRY_END:
+    case OP_UNOBSERVED_BEGIN: case OP_UNOBSERVED_END:
+    case OP_LISTCOMP_BEGIN: case OP_LISTCOMP_APPEND:
+    case OP_DISPATCH: case OP_SLICE_GET:
+    case OP_COUNT:   /* sentinel, never an instruction */
         return 0;
     }
+    /* Unreachable for any valid enumerator; guards an out-of-enum byte
+     * (NOT a default arm — -Werror=switch still fires on a missing case). */
+    return 0;
 }
 
 void chunk_disassemble(EigsChunk *chunk, const char *label) {
@@ -423,17 +473,26 @@ void chunk_disassemble(EigsChunk *chunk, const char *label) {
                            ((uint32_t)chunk->code[i + 3] << 24);
             fprintf(stderr, " %u", arg);
             i += 4;
-        } else if (op_has_u16(op) && i + 1 < chunk->code_len) {
-            uint16_t arg = chunk->code[i] | (chunk->code[i + 1] << 8);
-            fprintf(stderr, " %d", arg);
-            if (op == OP_CONST && arg < (uint16_t)chunk->const_count) {
-                Value *v = chunk->constants[arg];
-                if (v->type == VAL_NUM)
-                    fprintf(stderr, " (%.6g)", v->data.num);
-                else if (v->type == VAL_STR)
-                    fprintf(stderr, " (\"%s\")", v->data.str);
+        } else if (op < OP_COUNT) {
+            /* #737: skip ALL of the opcode's u16 operands, not just one —
+             * the multi-operand superinstructions desynchronized every
+             * following byte under the old boolean op_has_u16. The op <
+             * OP_COUNT guard keeps a crafted out-of-range byte (which
+             * op_name above prints as "???") out of the enum-typed switch:
+             * it has no default arm, by design. */
+            int nops = op_u16_operand_count(op);
+            for (int k = 0; k < nops && i + 1 < chunk->code_len; k++) {
+                uint16_t arg = chunk->code[i] | (chunk->code[i + 1] << 8);
+                fprintf(stderr, " %d", arg);
+                if (k == 0 && op == OP_CONST && arg < (uint16_t)chunk->const_count) {
+                    Value *v = chunk->constants[arg];
+                    if (v->type == VAL_NUM)
+                        fprintf(stderr, " (%.6g)", v->data.num);
+                    else if (v->type == VAL_STR)
+                        fprintf(stderr, " (\"%s\")", v->data.str);
+                }
+                i += 2;
             }
-            i += 2;
         }
         fprintf(stderr, "\n");
     }
@@ -470,8 +529,17 @@ typedef enum {
 } VerifyRole;
 
 /* Fill roles[] for op; return its operand count (0..3). Mirrors the operand
- * layout the VM decodes in vm.c — keep in lockstep if an opcode changes. */
-static int op_verify_operands(uint8_t op, VerifyRole roles[3]) {
+ * layout the VM decodes in vm.c — keep in lockstep if an opcode changes.
+ *
+ * Deliberately switched on OpCode with NO default arm: -Werror=switch
+ * (Makefile:9) then makes a missing case a BUILD ERROR. The old
+ * `default: return 0` silently absorbed OP_TRAJECTORY_SLOT (#737): its
+ * 3-byte instruction was walked as 1 byte, so is_start[] marked the
+ * operand's bytes as instruction boundaries and a crafted jump could land
+ * mid-instruction and still pass pass 2 — in the untrusted-chunk sandbox
+ * gate. The caller (chunk_verify) rejects op >= OP_COUNT before calling,
+ * so every value reaching here is a valid enumerator. */
+static int op_verify_operands(OpCode op, VerifyRole roles[3]) {
     switch (op) {
     case OP_CONST:
         roles[0] = VR_CONST; return 1;
@@ -494,12 +562,10 @@ static int op_verify_operands(uint8_t op, VerifyRole roles[3]) {
     case OP_LIST: case OP_DICT:
     case OP_OBSERVE_ASSIGN: case OP_OBSERVE_ASSIGN_LOCAL:
     case OP_REPORT_SLOT: case OP_OBSERVE_VALUE_SLOT:
-    case OP_REPORT_VALUE_SLOT:
+    case OP_REPORT_VALUE_SLOT: case OP_TRAJECTORY_SLOT:
     case OP_INTERROGATE: case OP_PREDICATE:
     case OP_MATCH: case OP_DESTRUCTURE_UNPACK:
         roles[0] = VR_RAW; return 1;
-    /* OP_LINE's operand is 32-bit (#630); chunk_verify walks it specially,
-     * so it never reaches this per-operand (u16-strided) role machinery. */
     case OP_LOCAL_DOT_GET: case OP_LOCAL_DOT_SET:
         roles[0] = VR_RAW; roles[1] = VR_NAME; return 2;    /* slot, name */
     case OP_LOCAL_IDX_GET:
@@ -514,9 +580,32 @@ static int op_verify_operands(uint8_t op, VerifyRole roles[3]) {
         roles[0] = VR_RAW; roles[1] = VR_JFWD; return 2;    /* slot, skip */
     case OP_LOCAL_IDX_DOT_GET: case OP_LOCAL_IDX_DOT_SET:
         roles[0] = VR_RAW; roles[1] = VR_RAW; roles[2] = VR_NAME; return 3;
-    default:
+    /* OP_LINE's operand is 32-bit (#630); chunk_verify walks it specially and
+     * never calls here — the case exists only to keep the switch exhaustive.
+     * OP_WIDE is an unimplemented placeholder, never emitted. The rest carry
+     * no operand. */
+    case OP_LINE: case OP_WIDE:
+    case OP_NULL: case OP_NUM_ZERO: case OP_NUM_ONE:
+    case OP_ADD: case OP_SUB: case OP_MUL: case OP_DIV: case OP_MOD:
+    case OP_BAND: case OP_BOR: case OP_BXOR: case OP_SHL: case OP_SHR:
+    case OP_NEG: case OP_NOT: case OP_BNOT:
+    case OP_EQ: case OP_NE: case OP_LT: case OP_GT: case OP_LE: case OP_GE:
+    case OP_POP: case OP_DUP: case OP_DUP2:
+    case OP_RETURN: case OP_RETURN_NULL:
+    case OP_INDEX_GET: case OP_INDEX_SET:
+    case OP_ITER_SETUP:
+    case OP_LOOP_ENV_FRESH: case OP_LOOP_ENV_END: case OP_LOOP_ENV_CLEAR:
+    case OP_BREAK: case OP_CONTINUE:
+    case OP_TRY_END:
+    case OP_UNOBSERVED_BEGIN: case OP_UNOBSERVED_END:
+    case OP_LISTCOMP_BEGIN: case OP_LISTCOMP_APPEND:
+    case OP_DISPATCH: case OP_SLICE_GET:
+    case OP_COUNT:   /* sentinel, never an instruction */
         return 0;   /* no operand */
     }
+    /* Unreachable — chunk_verify rejects op >= OP_COUNT before calling; not
+     * a default arm, so -Werror=switch still fires on a missing case. */
+    return 0;
 }
 
 int chunk_verify(EigsChunk *chunk) {

@@ -241,8 +241,12 @@ static void adjust_stack(Compiler *c, int delta) {
 
 /* ---- Emit helpers ---- */
 
-/* Stack effect of each opcode */
-static int op_stack_effect(uint8_t op) {
+/* Stack effect of each opcode.
+ *
+ * Deliberately switched on OpCode with NO default arm: -Werror=switch
+ * (Makefile:9) then makes a missing case — e.g. a newly added opcode — a
+ * BUILD ERROR instead of a silently wrong stack depth (#737). */
+static int op_stack_effect(OpCode op) {
     switch (op) {
     /* Push 1 */
     case OP_CONST: case OP_NULL: case OP_NUM_ZERO: case OP_NUM_ONE:
@@ -346,10 +350,26 @@ static int op_stack_effect(uint8_t op) {
     /* DISPATCH: pop 3 (table, key, arg), push 1 = -2 */
     case OP_DISPATCH:
         return -2;
-    /* CALL, LIST, DICT: dynamic — handled separately */
-    default:
+    /* MATCH: vm.c decodes it as push-null (net +1), but the compiler never
+     * emits it (match is compiled as DUP+compare+jump) — the case exists so
+     * the switch stays exhaustive, not because it runs. */
+    case OP_MATCH:
+        return 1;
+    /* Loop guards: iteration checks only, no stack change */
+    case OP_LOOP_STALL_CHECK: case OP_LOOP_CAP_CHECK:
+        return 0;
+    /* WIDE: unimplemented placeholder, never emitted */
+    case OP_WIDE:
+        return 0;
+    /* CALL, LIST, DICT: dynamic — handled separately by the emit sites */
+    case OP_CALL: case OP_LIST: case OP_DICT:
+        return 0;
+    case OP_COUNT:   /* sentinel, never an instruction */
         return 0;
     }
+    /* Unreachable for any valid enumerator; not a default arm, so
+     * -Werror=switch still fires on a missing case. */
+    return 0;
 }
 
 static void emit(Compiler *c, uint8_t op, int line) {

@@ -4094,6 +4094,17 @@ fi
 rm -rf "$CONT_DIR"
 echo ""
 
+echo "[99i] Opcode layout tables complete (#737)"
+TOTAL=$((TOTAL + 1))
+if bash "$TESTS_DIR/../tools/opcode_layout_check.sh" && bash "$TESTS_DIR/../tools/opcode_layout_check.sh" --selftest >/dev/null; then
+    PASS=$((PASS + 1))
+    echo "  PASS: all four opcode metadata tables cover every opcode (gate self-test green)"
+else
+    FAIL=$((FAIL + 1))
+    echo "  FAIL: opcode metadata table drift, or gate self-test broke (see lines above)"
+fi
+echo ""
+
 # Final guard (#681): if the binary changed during the last block, results are invalid.
 check_binary_fingerprint
 

@@ -295,4 +295,18 @@ assert_eq of [vm_run_bytecode of [ABI, iter_neg, [[10, 20, 30], 1, "EXITED"]], 2
 try9 is [TRY_BEGIN,27,0, TRY_BEGIN,24,0, TRY_BEGIN,21,0, TRY_BEGIN,18,0, TRY_BEGIN,15,0, TRY_BEGIN,12,0, TRY_BEGIN,9,0, TRY_BEGIN,6,0, TRY_BEGIN,3,0, JUMP,4,0, CONST,1,0, RETURN, CONST,0,0, RETURN]
 assert_eq of [vm_run_bytecode of [ABI, try9, ["NOERR", "CAUGHT8"]], "CAUGHT8", "#726: the 9th TRY_BEGIN raises instead of silently mis-pairing"]
 
+# ---- #737: op_verify_operands was missing OP_TRAJECTORY_SLOT (91) — the
+# default arm walked its 3-byte instruction as 1 byte, so the operand's bytes
+# were marked instruction starts and a crafted jump could land mid-instruction
+# and still pass pass 2. A legit TRAJECTORY_SLOT chunk must now verify and run
+# (unobserved slot snapshots as an empty trajectory dict), and a jump into its
+# operand bytes must be REJECTED. The crafted chunk is exploit-shaped: under
+# the old 1-byte walk its operand bytes re-decode as a valid stream
+# (NUM_ZERO; CONST 0; RETURN), so the pre-fix verifier ACCEPTED it and
+# executed it (returned "x"); the post-fix verifier rejects it outright.
+TRAJ_SLOT is 91
+NUM_ZERO is 2
+assert_eq of [type of (vm_run_bytecode of [ABI, [TRAJ_SLOT,0,0, RETURN], []]), "dict", "#737: TRAJECTORY_SLOT verifies as a 3-byte instruction"]
+assert_eq of [vm_run_bytecode of [ABI, [JUMP,1,0, TRAJ_SLOT,NUM_ZERO,0, 0,0, RETURN], ["x"]], null, "#737: jump into TRAJECTORY_SLOT's operand is rejected"]
+
 test_summary of null
@@ -0,0 +1,79 @@
+#!/bin/bash
+# Opcode layout-table drift gate (#737).
+#
+# Adding one opcode requires editing several hand-maintained metadata sites,
+# and three of them had ALREADY drifted when #737 was filed: op_verify_operands
+# (the untrusted-chunk sandbox gate) was missing OP_TRAJECTORY_SLOT, op_name
+# printed 4 opcodes as "???", and the disassembler's operand-width table was
+# missing 16 opcodes. The compile-time leg of the fix types the three layout
+# switches as OpCode with no default arm so -Werror=switch (Makefile:9)
+# makes a missing case a build error — but that covers only the switch-based
+# tables, and only under a compiler that honors it. op_name is a lookup TABLE
+# (a switch cannot enforce a designated-initializer array), so this grep gate
+# is the table-agnostic backstop: every opcode in the OpCode enum must be
+# named in ALL FOUR metadata sites:
+#
+#   1. op_name()               — src/chunk.c   (disassembler names)
+#   2. op_u16_operand_count()  — src/chunk.c   (disassembler operand widths)
+#   3. op_verify_operands()    — src/chunk.c   (untrusted-chunk verifier)
+#   4. op_stack_effect()       — src/compiler.c (stack-depth accounting)
+#
+# Usage: tools/opcode_layout_check.sh [--selftest]
+#   --selftest : inject a fake opcode and confirm the gate flags it (proves
+#                the checker isn't vacuously green).
+# Exit 0 = all four tables cover every opcode; 1 = drift (or selftest failure).
+
+set -u
+cd "$(dirname "$0")/.."
+
+# --- every opcode in the OpCode enum (src/vm.h), minus the OP_COUNT sentinel ---
+enum_opcodes() {
+    awk '/^typedef enum \{/,/^\} OpCode;/' src/vm.h \
+        | grep -oE '^[[:space:]]+OP_[A-Z_0-9]+' | tr -d ' ' \
+        | grep -v '^OP_COUNT$' | sort -u
+}
+
+# --- OP_ tokens mentioned inside one function body (col-0 start to col-0 }) ---
+function_opcodes() {  # args: file, function-start-regex
+    awk "/$2/,/^\\}/" "$1" | grep -oE 'OP_[A-Z_0-9]+' | grep -v '^OP_COUNT$' | sort -u
+}
+
+check_table() {  # args: label, file, function-start-regex, required-set
+    local label="$1" file="$2" fnre="$3" required="$4"
+    local have missing
+    have=$(function_opcodes "$file" "$fnre")
+    missing=$(comm -23 <(printf '%s\n' "$required") <(printf '%s\n' "$have"))
+    if [ -n "$missing" ]; then
+        echo "OPCODE TABLE DRIFT in $label ($file): opcode(s) absent from the table:"
+        echo "$missing" | sed 's/^/  - /'
+        return 1
+    fi
+    return 0
+}
+
+check_all() {  # arg: optional extra (fake) opcode for selftest
+    local extra="${1:-}" required rc=0
+    required=$(enum_opcodes)
+    [ -n "$extra" ] && required=$(printf '%s\n%s\n' "$required" "$extra" | sort -u)
+    check_table "op_name"              src/chunk.c    '^const char \*op_name'          "$required" || rc=1
+    check_table "op_u16_operand_count" src/chunk.c    '^static int op_u16_operand_count' "$required" || rc=1
+    check_table "op_verify_operands"   src/chunk.c    '^static int op_verify_operands'   "$required" || rc=1
+    check_table "op_stack_effect"      src/compiler.c '^static int op_stack_effect'      "$required" || rc=1
+    return $rc
+}
+
+# --- selftest: the gate MUST catch a deliberately-injected fake opcode ---
+if [ "${1:-}" = "--selftest" ]; then
+    if check_all "OP_ZZ_SELFTEST_PROBE" >/dev/null 2>&1; then
+        echo "SELFTEST FAILED: gate did not flag an injected fake opcode"
+        exit 1
+    fi
+    echo "SELFTEST OK: gate flags an injected fake opcode"
+    exit 0
+fi
+
+if check_all; then
+    echo "opcode layout OK: $(enum_opcodes | wc -l | tr -d ' ') opcodes covered by all 4 metadata tables"
+    exit 0
+fi
+exit 1