From ffb8b3b952c9fa9fced3e30d7992796a69afa424 Mon Sep 17 00:00:00 2001 From: "shoney.arickathil" Date: Fri, 14 Aug 2026 17:47:45 +0200 Subject: [PATCH] fix: net.Conn is a scalar, `\r` escape, map[k] is optional, interp node ids MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found by driving the compiled log-watcher's third path — the MCP server. It now answers real JSON-RPC over HTTP: initialize returns protocolVersion/serverInfo, tools/list returns the full tool list (1049 bytes of generated JSON), and an unauthorized request gets 401 {"error":"unauthorized"}. corpus 71/0, woc 565/0, wovm gates green. - lexer: `\r` and `\0` escapes. Without `\r` a program cannot write CRLF at all — the server's `index_of(buf, "\r\n\r\n")` was searching for a literal backslash-r, so it never found a header terminator and hung on every request - parser: an interpolated sub-expression now mints node ids from the OUTER id space. A fresh sub-parser started at 1, so `${...}` nodes collided with the file's own nodes — and every side table (drops, moves, rc, masks, f_decl) is keyed by node id. Surfaced as WO-E404 "ownership table names `headers`, which has no register"; silent misattribution otherwise - types.ml: `net.Conn` is a reserved SCALAR type (a file descriptor). It was falling through as "some user class", i.e. WO_K_OWNED, so the frame would DROP an integer at scope end - types.ml: confident_typ knows `..` yields Text. Interpolation desugars to a Concat chain, so without it every interpolated value looked underivable — which is why the `+`-on-Text check missed two live sites in the workload - `m[k]` on a map is now the OPTIONAL read (nil for a missing key), while `get(m, k)` stays the asserting one that traps KEY. That is what makes `let v = m[k]; if v != nil` — the workload's header lookup — work. trap/missing-map-key now pins `get(...)`, and the surface doc records the split - json.encode of a `json.Value` emits it verbatim (kind 255): an echoed id was coming back as "1" instead of 1 - disasm: TRY/ENDTRY render instead of ?OP32/?OP33 - status board: the push-of-a-borrowed-Text gap is now recorded with the concrete failure it produces (tools/call tail_log), plus the leaked temporary-record shell found in the same disassembly Co-Authored-By: Claude Opus 5 (1M context) --- compiler/src/disasm.ml | 4 ++++ compiler/src/emit.ml | 16 ++++++++++++-- compiler/src/lexer.ml | 9 +++++++- compiler/src/parser.ml | 10 +++++++++ compiler/src/types.ml | 16 ++++++++++++++ docs/00-status.md | 22 ++++++++++++++++---- docs/examples/log-watcher/mcp.wo | 2 +- docs/plan/oop-vm/08-builtin-surface.md | 15 ++++++++++--- runtime/src/builtin.c | 14 ++++++++++--- runtime/src/json.c | 6 +++++- runtime/src/loader.c | 2 +- runtime/src/wob.h | 13 ++++++++++-- tests/corpus/trap/missing-map-key/fixture.wo | 9 +++++++- 13 files changed, 119 insertions(+), 19 deletions(-) diff --git a/compiler/src/disasm.ml b/compiler/src/disasm.ml index 22d3020..a30ca0c 100644 --- a/compiler/src/disasm.ml +++ b/compiler/src/disasm.ml @@ -147,6 +147,10 @@ let ins_str (i : int) (pc : int) : string = else Printf.sprintf "BUILTIN r%d, r%d, %s" a b (builtin_name c) | 30 -> "DB_STUB" | 31 -> Printf.sprintf "TRAP %d" bx + (* haxe-parity Task 5: try/catch. The handler target is rendered the way + jumps are — absolute, so a disassembly can be read against the pc column. *) + | 32 -> Printf.sprintf "TRY r%d, handler -> %04d" a target + | 33 -> "ENDTRY" | op -> Printf.sprintf "?OP%d" op (* ---- the dump ---- *) diff --git a/compiler/src/emit.ml b/compiler/src/emit.ml index 1483307..01e8c84 100644 --- a/compiler/src/emit.ml +++ b/compiler/src/emit.ml @@ -255,6 +255,7 @@ let b_map_remove = 36 let b_map_key_at = 37 let b_map_val_at = 38 let b_multi_set = 39 +let b_map_get_opt = 59 (* json (runtime/src/json.c): encode takes the value's static kind as its second argument, decode the class id to build as its second. *) @@ -1518,7 +1519,12 @@ let rec emit_expr (p : pctx) (f : fstate) (v : views) ~(dst : int) ?expected (e | Index (base, idx) -> ( let bid = match ty_of_expr p f base with - | Some bt -> ( match unwrap bt with Multi _ -> Some b_multi_get | Map _ -> Some b_map_get | _ -> None) + (* `m[k]` on a map is the OPTIONAL read — a missing key is nil, which is + what makes `let v = m[k]; if v != nil { ... }` the ordinary lookup + idiom. `get(m, k)` keeps asserting (WO_B_MAP_GET traps KEY). A `multi` + index still traps out of range: a bad index is a fault, not an + absence. *) + | Some bt -> ( match unwrap bt with Multi _ -> Some b_multi_get | Map _ -> Some b_map_get_opt | _ -> None) | None -> None in match bid with @@ -2446,7 +2452,13 @@ and emit_call (p : pctx) (f : fstate) (v : views) ~(dst : int) ?expected (e : As emit_expr p f v ~dst:base a; f.f_temp <- save; let kind = - match ty_of_expr p f a with Some t -> field_kind p t | None -> 3 (* Text *) + match ty_of_expr p f a with + (* a `json.Value` is raw JSON already: kind 255 tells the + builtin to emit it verbatim instead of quoting it *) + | Some t when (match unwrap t with Scalar n -> n = Types.json_value_type | _ -> false) + -> 255 + | Some t -> field_kind p t + | None -> 3 (* Text *) in put f (ins_abx op_loadk (base + 1) (check_bx p f e.pos "constant" (const_int p kind))); sync_mask p f v e.id; diff --git a/compiler/src/lexer.ml b/compiler/src/lexer.ml index 747b780..74cb335 100644 --- a/compiler/src/lexer.ml +++ b/compiler/src/lexer.ml @@ -10,7 +10,8 @@ newline; - single- or double-quoted strings support the same backslash escapes as rt: n, t, backslash, or either quote character, each - backslash-prefixed; anything else verbatim; + backslash-prefixed; anything else verbatim. `\r` and `\0` were added + 2026-08-14 (a program writing HTTP needs CRLF, and rt never had to); - integer literals are plain runs of ASCII digits; - identifiers may contain internal dashes, exactly like rt's read_ident_chars (crates/rt/src/lexer.rs) — so `foo-bar` lexes as @@ -291,6 +292,12 @@ let tokenize (collector : Diag.Collector.t) ~(file : string) (src : string) : match advance lx with | Some 'n' -> Buffer.add_char buf '\n' | Some 't' -> Buffer.add_char buf '\t' + (* `\r` — added 2026-08-14: without it a program cannot write CRLF + at all, and the driving workload's HTTP server needs it (its + `index_of(buf, "\r\n\r\n")` was searching for a literal + backslash-r, so it never found a header terminator). *) + | Some 'r' -> Buffer.add_char buf '\r' + | Some '0' -> Buffer.add_char buf '\000' | Some '\\' -> Buffer.add_char buf '\\' | Some '"' -> Buffer.add_char buf '"' | Some '\'' -> Buffer.add_char buf '\'' diff --git a/compiler/src/parser.ml b/compiler/src/parser.ml index 96beabd..c4b1695 100644 --- a/compiler/src/parser.ml +++ b/compiler/src/parser.ml @@ -1121,6 +1121,15 @@ and desugar_interp (st : state) (pos : Ast.pos) (segs : Token.str_part list) : A let sub_collector = Diag.Collector.create () in let sub_toks = Lexer.tokenize sub_collector ~file:st.file raw in let sub_st = make sub_collector ~file:st.file sub_toks in + (* The sub-parse must mint ids from the OUTER id space and hand back what + it used. A fresh state starts at 1, so every interpolation used to + produce nodes whose ids collided with unrelated nodes of the same file — + and every side table downstream (drops, moves, rc sites, masks, + f_decl) is keyed by node id, so a collision silently attributes one + construct's ownership table to another. That surfaced as a WO-E404 + ("ownership table names `headers`, which has no register") on a method + whose interpolation happened to collide with a later local's node. *) + sub_st.next_id <- st.next_id; let parsed = try let e = parse_expr sub_st in @@ -1128,6 +1137,7 @@ and desugar_interp (st : state) (pos : Ast.pos) (segs : Token.str_part list) : A else None with Parse_error -> None in + st.next_id <- sub_st.next_id; match parsed with | Some e -> e | None -> fail st pos syntax_code "malformed \"${...}\" interpolation expression" diff --git a/compiler/src/types.ml b/compiler/src/types.ml index 5ded857..61da425 100644 --- a/compiler/src/types.ml +++ b/compiler/src/types.ml @@ -179,6 +179,16 @@ let is_stdlib_module (name : string) : bool = List.mem name stdlib_modules dynamic value tree. *) let json_value_type = "json.Value" +(* The reserved stdlib TYPE names and their representations. `net.Conn` is a + file descriptor — a SCALAR — and getting this wrong is not cosmetic: an + unknown qualified name would fall through to "some user class", i.e. + WO_K_OWNED, and the frame would DROP an integer at scope end. Anything + qualified with a stdlib module and not listed here is an error at the use + site rather than a guess (types.ml's check_use_edges). *) +let stdlib_scalar_types = [ "net.Conn" ] + +let is_stdlib_scalar_type (name : string) : bool = List.mem name stdlib_scalar_types + (* haxe-parity Task 5: the record `catch (e)` binds — the VM's structured trap error, one shape forever (spec §6). Predeclared rather than written: no source declares it, every program that catches gets it, and @@ -357,6 +367,7 @@ let wob_kind_of_typ (syms : symbols) (t : typ) : wob_kind = came from, which json.encode emits back verbatim. Kinding it TEXT is what makes it drop correctly and pass through concatenation. *) if name = "Text" || name = json_value_type then WO_K_TEXT + else if is_stdlib_scalar_type name then WO_K_SCALAR else if is_builtin_scalar name then WO_K_SCALAR else if StringMap.mem name syms.unions then (* haxe-parity Task 4: an all-bare union value is a plain @@ -1130,6 +1141,11 @@ let typecheck_program ~file ~(module_of : string -> string) `a == 1 and b == 2` without a false "underivable" silence on the left-hand comparison. *) Some (TScalar "Bool") + (* `a .. b` is CONCAT, which always produces a fresh Text — and string + interpolation desugars to exactly such a chain (parser.ml), so without + this arm every interpolated value looked underivable and every check + built on confident types silently skipped it. *) + | Binary (Concat, _, _) -> Some (TScalar "Text") | Interp _ -> (* An interpolation always *produces* Text by construction (emit.ml decides, per-segment, whether the embedded value diff --git a/docs/00-status.md b/docs/00-status.md index d08c21c..8417268 100644 --- a/docs/00-status.md +++ b/docs/00-status.md @@ -181,10 +181,21 @@ recorded, not silently owed: (`extends`/`cast`/`Dynamic`/…) still have no doctrine-citing diagnostics — plan 8 Tasks 7–8's remainder. - **A borrowed non-constant Text pushed into a container is a double-free - hazard** — `push(m, v)`'s open gap, now shared by list literals (`[a, b]`) - and documented in `owner.ml`'s own comment. The workload's literals are - string constants or borrowed params handed straight to a stdlib call, so - nothing reachable today hits it. + hazard** — `push(m, v)`'s open gap, now shared by list literals (`[a, b]`). + **Reached for real on 2026-08-14**: the MCP server answers `initialize`, + `tools/list` and an unauthorized request correctly, but a `tools/call` of + `tail_log` returns `{"isError":true,"text":"tool failed: not a text value"}` + — `Mcp.allowed_paths` pushes `e.log_path` (a Text the entry record owns) + into a fresh `multi Text`, so two owners free one string and a later read + finds recycled memory. The fix is a coordinated pair, not a one-liner: + `multi_push`/`map_set` must COPY a TEXT element (as `slice` already does), + and `owner.ml` must then stop treating a pushed Text as escaping so the + caller's own fresh temporaries are still dropped. Until then, treat + `push`-of-a-borrowed-Text as unsound. +- **A temporary record whose field is iterated is never dropped** — + `for e in parse_dir(dir).entries` keeps the entries alive (good) but leaks + the `ParseResult` shell (its drop is recorded for no register). Found in the + same disassembly; a leak, not a corruption. - **json's two documented limits**: a `Bool` field encodes as `0`/`1` (the class-table kind byte does not distinguish it from an integer), and a JSON number with a fraction or exponent decodes by truncation. @@ -193,6 +204,9 @@ recorded, not silently owed: descriptors. That is the sample's bug to fix, not the runtime's. - **The workload has never run under ASan**, and iteration 4's `gc/held-cycle` leak (above) is still open. The corpus itself stays ASan-clean. +- **`json.encode` of a `Bool` and of a nil scalar are asymmetric**: a nullable + scalar encodes as `null` (the field metadata says so), a plain `Bool` still + encodes as `0`/`1`. - **No corpus fixtures cover the new surface.** By explicit direction (2026-08-14) the acceptance for this work is the log-watcher program itself, not fixture pairs; `tests/corpus/` still gates every pre-existing behavior diff --git a/docs/examples/log-watcher/mcp.wo b/docs/examples/log-watcher/mcp.wo index 8c1f5a1..5113706 100644 --- a/docs/examples/log-watcher/mcp.wo +++ b/docs/examples/log-watcher/mcp.wo @@ -126,7 +126,7 @@ class Mcp { let resp = self.handle(req); let head = "HTTP/1.1 ${resp.status} ${status_text(resp.status)}\r\n"; head = head .. "Content-Type: application/json\r\nConnection: close\r\n"; - head = head + "Content-Length: ${len(resp.body)}\r\n\r\n"; + head = head .. "Content-Length: ${len(resp.body)}\r\n\r\n"; try net.write(c, head .. resp.body) catch (e) {} } } diff --git a/docs/plan/oop-vm/08-builtin-surface.md b/docs/plan/oop-vm/08-builtin-surface.md index 918f038..aef1200 100644 --- a/docs/plan/oop-vm/08-builtin-surface.md +++ b/docs/plan/oop-vm/08-builtin-surface.md @@ -52,9 +52,18 @@ maps to one `BUILTIN` id of the format doc. given, so one source name covers the `multi` and `map` ids the runtime keeps apart. -**Sugar.** `c[i]` is exactly `get(c, i)` and `m[k] = v` is exactly -`set(m, k, v)` for a `map` and `multi_set(m, i, v)` for a `multi` (the -element it replaces is the container's, so the VM drops it). +**Sugar.** `m[k] = v` is exactly `set(m, k, v)` for a `map` and +`multi_set(m, i, v)` for a `multi` (the element it replaces is the container's, +so the VM drops it). + +Reads differ by container, deliberately (amended 2026-08-14): + +- `c[i]` on a **`multi`** is `get(c, i)` — an out-of-range index traps + `BOUNDS`, because a bad index is a fault, not an absence. +- `m[k]` on a **`map`** is the **optional** read (`map_get_opt`): a missing key + yields nil, which is what makes `let v = m[k]; if v != nil { … }` the + ordinary lookup idiom the driving workload uses for HTTP headers. + `get(m, k)` remains the **asserting** read and still traps `KEY`. **Shadowing.** A user-declared free `fn` of the same name always wins. A declared name is never silently replaced by a builtin. diff --git a/runtime/src/builtin.c b/runtime/src/builtin.c index d0eb36b..480264e 100644 --- a/runtime/src/builtin.c +++ b/runtime/src/builtin.c @@ -63,9 +63,11 @@ static int elem_cmp(uint8_t kind, uint64_t a, uint64_t b) { int wo_builtin(wo_vm *vm, uint64_t *R, uint32_t ins, const char **msg) { wo_rt *rt = &vm->rt; uint8_t A = wo_ins_a(ins), B = wo_ins_b(ins), C = wo_ins_c(ins); - /* the OS half and json live in their own translation units */ - if (C >= WO_B_JSON_ENCODE) return wo_builtin_json(vm, R, ins, msg); - if (C >= WO_B_SYS_FIRST) return wo_builtin_sys(vm, R, ins, msg); + /* the OS half and json live in their own translation units; ids outside + both ranges (WO_B_MAP_GET_OPT and anything added after it) stay here */ + if (C == WO_B_JSON_ENCODE || C == WO_B_JSON_DECODE) + return wo_builtin_json(vm, R, ins, msg); + if (C >= WO_B_SYS_FIRST && C <= WO_B_PROC_RUN) return wo_builtin_sys(vm, R, ins, msg); switch (C) { case WO_B_NOW: { /* wall-clock milliseconds */ struct timespec ts; @@ -185,6 +187,12 @@ int wo_builtin(wo_vm *vm, uint64_t *R, uint32_t ins, const char **msg) { } return 0; } + case WO_B_MAP_GET_OPT: { /* `m[k]`: a missing key is nil, not a trap */ + wo_map *m = native_check(R[B], WO_CLS_MAP, msg); + if (!m) return WO_T_BOUNDS; + if (wo_map_get(m, R[B + 1], &R[A]) != 0) R[A] = 0; + return 0; + } case WO_B_MAP_HAS: { wo_map *m = native_check(R[B], WO_CLS_MAP, msg); if (!m) return WO_T_BOUNDS; diff --git a/runtime/src/json.c b/runtime/src/json.c index 8af705e..cc50211 100644 --- a/runtime/src/json.c +++ b/runtime/src/json.c @@ -548,7 +548,11 @@ int wo_builtin_json(wo_vm *vm, uint64_t *R, uint32_t ins, const char **msg) { case WO_B_JSON_ENCODE: { jbuf b = {NULL, 0, 0, 0}; uint8_t kind = (uint8_t)R[B + 1]; - enc_value(&b, vm->mod, R[B], kind, WOB_NONE); + /* a `json.Value` argument is raw JSON already (an id echoed back into + a response, say) — quoting it as a Text would change `1` into `"1"` */ + uint32_t fclass = kind == WO_JSON_KIND_RAW ? WOB_FIELD_JSON_RAW : WOB_NONE; + if (kind == WO_JSON_KIND_RAW) kind = WO_K_TEXT; + enc_value(&b, vm->mod, R[B], kind, fclass); if (b.oom) { free(b.p); *msg = "out of memory"; diff --git a/runtime/src/loader.c b/runtime/src/loader.c index 6b13611..15d46e1 100644 --- a/runtime/src/loader.c +++ b/runtime/src/loader.c @@ -61,7 +61,7 @@ static const uint8_t b_arity[WO_B_MAX + 1] = { [WO_B_NET_CLOSE] = 1, [WO_B_PROC_RUN] = 3, /* json (json.c): encode takes the value's static kind, decode the class id to build */ - [WO_B_JSON_ENCODE] = 2, [WO_B_JSON_DECODE] = 2, + [WO_B_JSON_ENCODE] = 2, [WO_B_JSON_DECODE] = 2, [WO_B_MAP_GET_OPT] = 2, }; static int vtab_cmp(const void *a, const void *b) { diff --git a/runtime/src/wob.h b/runtime/src/wob.h index f077efb..4d9f351 100644 --- a/runtime/src/wob.h +++ b/runtime/src/wob.h @@ -273,13 +273,22 @@ enum { * top level comes from object headers and the class table. decode takes * the class id to build, and yields nil (0) on malformed input — never a * trap, which is what makes `json.decode(t) as T` a checked decode. ---- */ - WO_B_JSON_ENCODE = 57, /* (value, kind) -> Text */ + WO_B_JSON_ENCODE = 57, /* (value, kind) -> Text; kind 255 = the value is a + * `json.Value`, i.e. raw JSON to emit verbatim */ WO_B_JSON_DECODE = 58, /* (text, cls) -> ?instance of cls */ + /* `m[k]` on a map is the OPTIONAL read: a missing key is nil, not a trap. + * `get(m, k)` (WO_B_MAP_GET) stays the asserting form. Indexing a `multi` + * out of range still traps — a bad index is a fault, not an absence. */ + WO_B_MAP_GET_OPT = 59, /* (map, key) -> value or nil */ }; -#define WO_B_MAX 58u +#define WO_B_MAX 59u /* ids at or above this one live in sysio.c, not builtin.c */ #define WO_B_SYS_FIRST WO_B_FS_EXISTS +/* json.encode's "this is already JSON" kind (a `json.Value`): not a field + * kind, only an argument marker. */ +#define WO_JSON_KIND_RAW 255u + /* ---- instruction encode/decode: op:8 A:8 then B:8 C:8 or Bx:16 ---- */ static inline uint32_t wo_ins_abc(uint8_t op, uint8_t a, uint8_t b, uint8_t c) { return (uint32_t)op | ((uint32_t)a << 8) | ((uint32_t)b << 16) | ((uint32_t)c << 24); diff --git a/tests/corpus/trap/missing-map-key/fixture.wo b/tests/corpus/trap/missing-map-key/fixture.wo index 8450259..461f8eb 100644 --- a/tests/corpus/trap/missing-map-key/fixture.wo +++ b/tests/corpus/trap/missing-map-key/fixture.wo @@ -3,6 +3,13 @@ -- missing key traps KEY"). Unprovable at compile time -- the key is a -- runtime `Text` value -- so it traps rather than failing to compile: -- the hybrid boundary's runtime half. +-- +-- Written as `get(...)` on purpose: the two map reads differ. `get(m, k)` +-- ASSERTS the key is there and traps KEY when it isn't (this fixture); +-- `m[k]` is the OPTIONAL read and yields nil for a missing key, which is +-- what makes `let v = m[k]; if v != nil { ... }` the ordinary lookup idiom +-- (the driving workload's own header lookup). Before 2026-08-14 both +-- spellings trapped, and this fixture used the sugar. class Catalog { prices: map } @@ -10,5 +17,5 @@ class Catalog { fn main() { let c = Catalog { prices: map_new() } set(c.prices, "mug", 499) - print_int(c.prices["shirt"]) + print_int(get(c.prices, "shirt")) }