From df74d9c429f157465499fdc63e0595466e322abb Mon Sep 17 00:00:00 2001 From: Hojun-Cho Date: Fri, 15 May 2026 02:07:34 +0900 Subject: [PATCH] selfhost: cgmatch bsz for TY_STRUCT variant bind (closes #31) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Match-arm bind size in wwstage hardcoded str=16, []T=24, else=8 in both cgmatch (emit) and scanlocals (frame pre-scan). A TY_STRUCT variant fell into the 8B fallback: only the first quadword reached the bind, and the prologue SUBQ underbooked the frame so the emit-time localalloc(bsz=24+) wrote past SP. Replace the hand-rolled table with slotsize(c, pat) at both sites. slotsize already covers N_TNAME named structs (returns si.totsize), str (16), []T (24), tuples, aliases, and primitives (8). Mirrors cstage cgen.c cgmatch which falls through to bu->size for TY_STRUCT. Cstage is correct; no mirror needed. Test 695 covers seven shapes: the 3xi64 headline repro, a 4xi64 struct via let-init scrut (exercises >24B bind), a mixed-quadword struct (i32+i32+i64+i64), str/slice/i32 negative controls, and a direct let-init scrutinee variant. The wider 4xi64 row uses the let-init shape because the N_DOT spill path in cgmatch tops out at AX/DX/CX/R8 — a separate, unrelated gap from the bind size. --- Makefile | 7 + selfhost/cmd/w6c/main.combined.ww | 42 +++- selfhost/cmd/wcc/cgendecl.ww | 14 +- selfhost/cmd/wcc/cgenexpr.ww | 28 ++- selfhost/cmd/wwdump/main.combined.ww | 42 +++- test/wcc/695_match_bind_struct.c | 302 +++++++++++++++++++++++++++ 6 files changed, 405 insertions(+), 30 deletions(-) create mode 100644 test/wcc/695_match_bind_struct.c diff --git a/Makefile b/Makefile index 6d0fabc6..627aba2d 100644 --- a/Makefile +++ b/Makefile @@ -223,6 +223,7 @@ TESTS = $(BIN)/test_smoke $(BIN)/test_lex $(BIN)/test_parse $(BIN)/test_check \ $(BIN)/test_dot_slice_arg \ $(BIN)/test_dot_tagged_source \ $(BIN)/test_tagged_store_intlit \ + $(BIN)/test_match_bind_struct \ $(BIN)/test_field_signed $(BIN)/test_frame_argcount \ $(BIN)/test_selfhost $(BIN)/test_w6a_ww $(BIN)/test_w6l_ww \ $(BIN)/test_w6c_ww $(BIN)/test_ww_ww $(BIN)/test_self_rebuild \ @@ -340,6 +341,12 @@ $(BIN)/test_tagged_store_intlit: test/wcc/694_tagged_store_intlit.c $(BIN)/ww \ $(LIB)/libwwrt.a | $(BIN) $(CC) $(CFLAGS) -o $@ $< +$(BIN)/test_match_bind_struct: test/wcc/695_match_bind_struct.c $(BIN)/ww \ + $(BIN)/w6c $(BIN)/w6a $(BIN)/w6l \ + $(BIN)/ww_ww $(BIN)/w6c_ww $(BIN)/w6a_ww $(BIN)/w6l_ww \ + $(LIB)/libwwrt.a | $(BIN) + $(CC) $(CFLAGS) -o $@ $< + $(BIN)/test_field_signed: test/wcc/660_field_signed.c $(BIN)/ww \ $(BIN)/w6c $(BIN)/w6a $(BIN)/w6l \ $(BIN)/ww_ww $(BIN)/w6c_ww $(BIN)/w6a_ww $(BIN)/w6l_ww \ diff --git a/selfhost/cmd/w6c/main.combined.ww b/selfhost/cmd/w6c/main.combined.ww index e9e24faa..adf800bf 100644 --- a/selfhost/cmd/w6c/main.combined.ww +++ b/selfhost/cmd/w6c/main.combined.ww @@ -9429,23 +9429,37 @@ fn cgmatch(c: *cgen, n: *node) void = { emitline("(BP)\n"); }; } else { - let bsz: i32 = 8; - if (isstrtype(c, pat)) { bsz = 16; } - else { if (isslicetype(c, pat)) { bsz = 24; }; }; + // Size the bind from the variant's declared + // layout. slotsize covers str (16), []T (24), + // N_TNAME named struct (si.totsize), aliases, + // tuples, primitives (8). Hardcoding str/slice + // + fall-through-8 dropped the high words of a + // TY_STRUCT variant (e.g. only v.x reached the + // bind for `case let v: pair`, project #31); + // mirrors cstage's `bu->size` fallback in + // cgen.c cgmatch. + let bsz: i32 = slotsize(c, pat); + if (bsz <= 0) { bsz = 8; }; // localalloc (not localadd): match-arm // binds don't dedup with same-named binds // in *other* matches, since C's cgexpr // allocates a fresh slot per match expr. let voff: i32 = localalloc(c, bn, bsz, pat); + // Word-by-word copy. Round bsz up to 8 in case + // a non-multiple-of-8 struct size leaked through + // (registerstruct already pads totsize, but be + // defensive — same shape as cstage's nwords = + // (bsz + 7) / 8). + let nwords: i32 = (bsz + 7) / 8; let bw: i32 = 0; - for (bw < bsz) { + for (bw < nwords) { emitline("\tMOVQ\t"); - emitoff((scrutoff + 8 + bw): i64); + emitoff((scrutoff + 8 + 8 * bw): i64); emitline("(BP), AX\n"); emitline("\tMOVQ\tAX, "); - emitoff((voff + bw): i64); + emitoff((voff + 8 * bw): i64); emitline("(BP)\n"); - bw += 8; + bw += 1; }; }; }; @@ -14505,9 +14519,17 @@ fn scanlocals(c: *cgen, n: *node) i32 = { if (bn.len > 0) { let pat: *node = n.lhs; if (pat != nil) { - if (isstrtype(c, pat)) { total += 16; } - else { if (isslicetype(c, pat)) { total += 24; } - else { total += 8; }; }; + // Must mirror cgmatch's bind-slot sizing in + // cgenexpr.ww (`bsz = slotsize(c, pat)`): + // hardcoding str/slice/8 here underbooked the + // frame for TY_STRUCT variants — the emit-time + // localalloc(bsz=24) then wrote past the SUBQ'd + // SP, smashing whatever the OS put under it + // (project #31). + let psz: i32 = slotsize(c, pat); + if (psz <= 0) { psz = 8; }; + if ((psz & 7) != 0) { psz = (psz + 7) & ~7; }; + total += psz; }; }; // Match arms get a fresh local scope at emission time diff --git a/selfhost/cmd/wcc/cgendecl.ww b/selfhost/cmd/wcc/cgendecl.ww index 7af3b97f..38f996f0 100644 --- a/selfhost/cmd/wcc/cgendecl.ww +++ b/selfhost/cmd/wcc/cgendecl.ww @@ -150,9 +150,17 @@ fn scanlocals(c: *cgen, n: *node) i32 = { if (bn.len > 0) { let pat: *node = n.lhs; if (pat != nil) { - if (isstrtype(c, pat)) { total += 16; } - else { if (isslicetype(c, pat)) { total += 24; } - else { total += 8; }; }; + // Must mirror cgmatch's bind-slot sizing in + // cgenexpr.ww (`bsz = slotsize(c, pat)`): + // hardcoding str/slice/8 here underbooked the + // frame for TY_STRUCT variants — the emit-time + // localalloc(bsz=24) then wrote past the SUBQ'd + // SP, smashing whatever the OS put under it + // (project #31). + let psz: i32 = slotsize(c, pat); + if (psz <= 0) { psz = 8; }; + if ((psz & 7) != 0) { psz = (psz + 7) & ~7; }; + total += psz; }; }; // Match arms get a fresh local scope at emission time diff --git a/selfhost/cmd/wcc/cgenexpr.ww b/selfhost/cmd/wcc/cgenexpr.ww index 0e997785..1dd4a709 100644 --- a/selfhost/cmd/wcc/cgenexpr.ww +++ b/selfhost/cmd/wcc/cgenexpr.ww @@ -1111,23 +1111,37 @@ fn cgmatch(c: *cgen, n: *node) void = { emitline("(BP)\n"); }; } else { - let bsz: i32 = 8; - if (isstrtype(c, pat)) { bsz = 16; } - else { if (isslicetype(c, pat)) { bsz = 24; }; }; + // Size the bind from the variant's declared + // layout. slotsize covers str (16), []T (24), + // N_TNAME named struct (si.totsize), aliases, + // tuples, primitives (8). Hardcoding str/slice + // + fall-through-8 dropped the high words of a + // TY_STRUCT variant (e.g. only v.x reached the + // bind for `case let v: pair`, project #31); + // mirrors cstage's `bu->size` fallback in + // cgen.c cgmatch. + let bsz: i32 = slotsize(c, pat); + if (bsz <= 0) { bsz = 8; }; // localalloc (not localadd): match-arm // binds don't dedup with same-named binds // in *other* matches, since C's cgexpr // allocates a fresh slot per match expr. let voff: i32 = localalloc(c, bn, bsz, pat); + // Word-by-word copy. Round bsz up to 8 in case + // a non-multiple-of-8 struct size leaked through + // (registerstruct already pads totsize, but be + // defensive — same shape as cstage's nwords = + // (bsz + 7) / 8). + let nwords: i32 = (bsz + 7) / 8; let bw: i32 = 0; - for (bw < bsz) { + for (bw < nwords) { emitline("\tMOVQ\t"); - emitoff((scrutoff + 8 + bw): i64); + emitoff((scrutoff + 8 + 8 * bw): i64); emitline("(BP), AX\n"); emitline("\tMOVQ\tAX, "); - emitoff((voff + bw): i64); + emitoff((voff + 8 * bw): i64); emitline("(BP)\n"); - bw += 8; + bw += 1; }; }; }; diff --git a/selfhost/cmd/wwdump/main.combined.ww b/selfhost/cmd/wwdump/main.combined.ww index 90ccb303..8186c4e5 100644 --- a/selfhost/cmd/wwdump/main.combined.ww +++ b/selfhost/cmd/wwdump/main.combined.ww @@ -9429,23 +9429,37 @@ fn cgmatch(c: *cgen, n: *node) void = { emitline("(BP)\n"); }; } else { - let bsz: i32 = 8; - if (isstrtype(c, pat)) { bsz = 16; } - else { if (isslicetype(c, pat)) { bsz = 24; }; }; + // Size the bind from the variant's declared + // layout. slotsize covers str (16), []T (24), + // N_TNAME named struct (si.totsize), aliases, + // tuples, primitives (8). Hardcoding str/slice + // + fall-through-8 dropped the high words of a + // TY_STRUCT variant (e.g. only v.x reached the + // bind for `case let v: pair`, project #31); + // mirrors cstage's `bu->size` fallback in + // cgen.c cgmatch. + let bsz: i32 = slotsize(c, pat); + if (bsz <= 0) { bsz = 8; }; // localalloc (not localadd): match-arm // binds don't dedup with same-named binds // in *other* matches, since C's cgexpr // allocates a fresh slot per match expr. let voff: i32 = localalloc(c, bn, bsz, pat); + // Word-by-word copy. Round bsz up to 8 in case + // a non-multiple-of-8 struct size leaked through + // (registerstruct already pads totsize, but be + // defensive — same shape as cstage's nwords = + // (bsz + 7) / 8). + let nwords: i32 = (bsz + 7) / 8; let bw: i32 = 0; - for (bw < bsz) { + for (bw < nwords) { emitline("\tMOVQ\t"); - emitoff((scrutoff + 8 + bw): i64); + emitoff((scrutoff + 8 + 8 * bw): i64); emitline("(BP), AX\n"); emitline("\tMOVQ\tAX, "); - emitoff((voff + bw): i64); + emitoff((voff + 8 * bw): i64); emitline("(BP)\n"); - bw += 8; + bw += 1; }; }; }; @@ -14505,9 +14519,17 @@ fn scanlocals(c: *cgen, n: *node) i32 = { if (bn.len > 0) { let pat: *node = n.lhs; if (pat != nil) { - if (isstrtype(c, pat)) { total += 16; } - else { if (isslicetype(c, pat)) { total += 24; } - else { total += 8; }; }; + // Must mirror cgmatch's bind-slot sizing in + // cgenexpr.ww (`bsz = slotsize(c, pat)`): + // hardcoding str/slice/8 here underbooked the + // frame for TY_STRUCT variants — the emit-time + // localalloc(bsz=24) then wrote past the SUBQ'd + // SP, smashing whatever the OS put under it + // (project #31). + let psz: i32 = slotsize(c, pat); + if (psz <= 0) { psz = 8; }; + if ((psz & 7) != 0) { psz = (psz + 7) & ~7; }; + total += psz; }; }; // Match arms get a fresh local scope at emission time diff --git a/test/wcc/695_match_bind_struct.c b/test/wcc/695_match_bind_struct.c new file mode 100644 index 00000000..fa30b34d --- /dev/null +++ b/test/wcc/695_match_bind_struct.c @@ -0,0 +1,302 @@ +/* + * 695_match_bind_struct — `case let v: T => ...` where T is a TY_STRUCT + * variant of a tagged-union scrutinee. wwstage's cgmatch (and the + * matching scanlocals frame pre-scan) hardcoded the bind-slot size to + * + * str -> 16 + * []T -> 24 + * else -> 8 + * + * so a struct variant collapsed to a single 8B word: only the first + * quadword reached the per-arm bind slot, and (worse) the prologue's + * SUBQ underbooked the frame, so localalloc at emit time then ran the + * SP cursor off the bottom of the function's frame and the bind's + * subsequent stores smashed whatever was past it. + * + * cstage cgen.c is fine: the else-branch falls through to + * `bsz = (int)bu->size` and the SP cursor / cg_frame counter both + * ride the same `bu->size`. Worker-28 noted this while landing #28, + * and a reread of cstage's cgmatch (cgen.c ~3998-4017) and stack- + * frame setup confirms — no cstage mirror needed. + * + * Closed in task #31 by: + * - selfhost/cmd/wcc/cgenexpr.ww: cgmatch bsz table replaced with + * `slotsize(c, pat)`. slotsize already covers N_TNAME named struct + * (returns si.totsize), str (16), []T (24), aliases, tuples, and + * primitives (8). Defensive nwords = (bsz + 7) / 8 mirrors cstage. + * - selfhost/cmd/wcc/cgendecl.ww: scanlocals N_MCASE pre-scan + * mirrored — must agree with cgmatch's bind-slot size so the + * prologue SUBQ reserves enough frame. + * - selfhost/cmd/{w6c,wwdump}/main.combined.ww: combined-file mirrors + * of both sites kept in lockstep. + * + * Coverage — seven rows: four structurally distinct TY_STRUCT shapes + * plus three negative controls. The 3xi64 headline repro, a 4-i64 + * struct via let-init scrutinee (exercise > 24B payload, so the + * historical 24-cap doesn't quietly pass), a mixed-quadword struct + * with i32 alignment + i64 tail (size still pads to a multiple of 8), + * str/slice/i32 negative controls (the existing sized paths must not + * regress), and a direct `let x: tag = (pair{...}: tag);` scrutinee + * (different from `match (h.e)` — exercises cglet's tagged-init + * route to the same bind site). + */ +#include +#include +#include +#include +#include +#include + +static int +runwait(const char *cmd) +{ + int rc = system(cmd); + if (rc == -1) return -1; + if (WIFEXITED(rc)) return WEXITSTATUS(rc); + return -1; +} + +struct row { const char *label; const char *src; int want; }; + +static const struct row rows[] = { + /* Headline repro. 3xi64 = 24B struct as a tagged-union variant. + * Bind copies all three quadwords; sum to 31. Pre-fix only v.x + * reached the bind and v.y/v.z were uninitialized stack — exit + * was nondeterministic but never 0. */ + { "pair_3xi64", + "type pair = struct { x: i64, y: i64, z: i64 };\n" + "type tag = (pair | i32);\n" + "type holder = struct { e: tag, mark: i32 };\n" + "fn main() i32 = {\n" + " let h: holder;\n" + " h.mark = 99;\n" + " let p: pair = pair { x = 7i64, y = 11i64, z = 13i64 };\n" + " h.e = (p: tag);\n" + " match (h.e) {\n" + " case let v: pair => {\n" + " let s: i64 = v.x + v.y + v.z;\n" + " if (s != 31i64) { return 1; };\n" + " return 0;\n" + " };\n" + " case let z: i32 => { return 2; };\n" + " };\n" + "};\n", + 0 }, + /* 4xi64 = 32B struct payload, exercising a >24B bind copy via the + * direct-let-init scrutinee shape (`match (x)` over an ident, + * bypassing the N_DOT spill path). Hardcoding a 24B cap (an + * obvious fix-sketch hack — "if struct then 24") would drop + * v.d and the sum would miss 24, returning 1 not 0. + * + * The N_DOT route through `match (h.t)` for a 32B-payload variant + * is a separate, unrelated gap: cgmatch's scrut spill only writes + * AX/DX/CX/R8 (4 registers, one of which is the tag), so the 4th + * payload quadword is dropped at spill time — not at bind time. + * Cstage sidesteps this with a direct-field addressing special- + * case in cgmatch (cgen.c ~3858-3876) that skips the spill + * entirely; wwstage lacks that path. Filing that as a follow-up; + * the let-init scrutinee already exercises the bind size for + * payloads > 24B without entangling the two bugs. */ + { "wide_4xi64_letinit", + "type wide = struct { a: i64, b: i64, c: i64, d: i64 };\n" + "type tag = (wide | i32);\n" + "fn main() i32 = {\n" + " let x: tag = (wide { a = 1i64, b = 2i64, c = 4i64,\n" + " d = 24i64 }: tag);\n" + " match (x) {\n" + " case let v: wide => {\n" + " let s: i64 = v.a + v.b + v.c + v.d;\n" + " if (s != 31i64) { return 1; };\n" + " return 0;\n" + " };\n" + " case let z: i32 => { return 2; };\n" + " };\n" + "};\n", + 0 }, + /* Mixed-quadword: i32+i32+i64+i64. registerstruct packs the two + * i32s into one quadword (x@0, y@4, z@8, w@16), totsize = 24 + * (after the trailing pad-to-8). Bind must copy all three + * quadwords; reading w must see the right value. */ + { "mixed_i32_i32_i64_i64", + "type mix = struct { x: i32, y: i32, z: i64, w: i64 };\n" + "type tag = (mix | i32);\n" + "type holder = struct { t: tag, mark: i32 };\n" + "fn main() i32 = {\n" + " let h: holder;\n" + " h.mark = 99;\n" + " let m: mix = mix { x = 3, y = 5, z = 11i64, w = 12i64 };\n" + " h.t = (m: tag);\n" + " match (h.t) {\n" + " case let v: mix => {\n" + " if (v.x != 3) { return 10; };\n" + " if (v.y != 5) { return 11; };\n" + " if (v.z != 11i64) { return 12; };\n" + " if (v.w != 12i64) { return 13; };\n" + " return 0;\n" + " };\n" + " case let z: i32 => { return 2; };\n" + " };\n" + "};\n", + 0 }, + /* Negative control: str variant in (str | pair). Pins that the + * fix didn't shadow the existing 16B str bind. */ + { "str_neg_control", + "type pair = struct { x: i64, y: i64, z: i64 };\n" + "type tag = (str | pair);\n" + "type holder = struct { e: tag, mark: i32 };\n" + "fn main() i32 = {\n" + " let h: holder;\n" + " h.mark = 99;\n" + " h.e = (\"hello\": tag);\n" + " match (h.e) {\n" + " case let s: str => { return s.len: i32; };\n" + " case let v: pair => { return -1; };\n" + " };\n" + "};\n", + 5 }, + /* Negative control: []u8 variant in ([]u8 | pair). Pins 24B slice + * bind. */ + { "slice_neg_control", + "type pair = struct { x: i64, y: i64, z: i64 };\n" + "type tag = ([]u8 | pair);\n" + "type holder = struct { e: tag, mark: i32 };\n" + "fn main() i32 = {\n" + " let raw: [8]u8;\n" + " raw[0] = 1u8; raw[1] = 2u8; raw[2] = 3u8;\n" + " let h: holder;\n" + " h.mark = 99;\n" + " h.e = (raw[0:3]: tag);\n" + " match (h.e) {\n" + " case let v: []u8 => {\n" + " if (v.cap != 3) { return 20; };\n" + " return v.len: i32;\n" + " };\n" + " case let p: pair => { return -1; };\n" + " };\n" + "};\n", + 3 }, + /* Negative control: primitive i32 variant — pins the 8B fall- + * through. */ + { "i32_neg_control", + "type pair = struct { x: i64, y: i64, z: i64 };\n" + "type tag = (pair | i32);\n" + "type holder = struct { e: tag, mark: i32 };\n" + "fn main() i32 = {\n" + " let h: holder;\n" + " h.mark = 99;\n" + " h.e = (42i32: tag);\n" + " match (h.e) {\n" + " case let v: pair => { return -1; };\n" + " case let z: i32 => { return z; };\n" + " };\n" + "};\n", + 42 }, + /* Direct `let x: tag = (struct: tag);` scrutinee — different + * write site than the holder-field assign route, but the bind + * read-side hits the same cgmatch table. Pins the let-init + * shape. */ + { "letinit_direct_scrutinee", + "type pair = struct { x: i64, y: i64, z: i64 };\n" + "type tag = (pair | i32);\n" + "fn main() i32 = {\n" + " let x: tag = (pair { x = 7i64, y = 11i64, z = 13i64 }: tag);\n" + " match (x) {\n" + " case let v: pair => {\n" + " let s: i64 = v.x + v.y + v.z;\n" + " if (s != 31i64) { return 1; };\n" + " return 0;\n" + " };\n" + " case let z: i32 => { return 2; };\n" + " };\n" + "};\n", + 0 }, +}; + +static int +run_driver(const char *driver, const struct row *r, int i) +{ + char src[64], tmpdir[64], cmd[1024]; + snprintf(src, sizeof src, "/tmp/wmbs_%d_%d.ww", getpid(), i); + snprintf(tmpdir, sizeof tmpdir, "/tmp/wmbs_%d_d_%d", getpid(), i); + + FILE *f = fopen(src, "wb"); + if (!f) return -1; + fputs(r->src, f); + fclose(f); + + mkdir(tmpdir, 0755); + snprintf(cmd, sizeof cmd, "cd %s && %s build %s", + tmpdir, driver, src); + if (runwait(cmd) != 0) { + fprintf(stderr, "row[%s]: build via %s failed\n", + r->label, driver); + unlink(src); rmdir(tmpdir); + return -1; + } + + const char *base = strrchr(src, '/'); + base = base ? base + 1 : src; + char outbin[128]; + snprintf(outbin, sizeof outbin, "%s/%s", tmpdir, base); + char *dot = strrchr(outbin, '.'); + if (dot && strcmp(dot, ".ww") == 0) *dot = '\0'; + int got = runwait(outbin); + + unlink(src); unlink(outbin); rmdir(tmpdir); + return got; +} + +int +main(void) +{ + const char *bin = getenv("BIN"); + if (!bin) bin = "out/bin"; + char absbin[1024]; + if (bin[0] != '/') { + char cwd[1024]; + if (getcwd(cwd, sizeof cwd) == NULL) return 1; + snprintf(absbin, sizeof absbin, "%s/%s", cwd, bin); + bin = absbin; + } + + char cdrv[1024]; + snprintf(cdrv, sizeof cdrv, "%s/ww", bin); + char wdrv[1024]; + snprintf(wdrv, sizeof wdrv, "%s/ww_ww", bin); + + struct { const char *name; const char *path; int gated_on_existence; } + drivers[] = { + { "cstage", cdrv, 0 }, + { "wwstage", wdrv, 1 }, + { NULL, NULL, 0 }, + }; + + int n = (int)(sizeof rows / sizeof rows[0]); + int total = 0, fail = 0; + for (int d = 0; drivers[d].name; d++) { + if (drivers[d].gated_on_existence + && access(drivers[d].path, X_OK) != 0) { + fprintf(stderr, "match_bind_struct: skip %s (no %s)\n", + drivers[d].name, drivers[d].path); + continue; + } + for (int i = 0; i < n; i++) { + int got = run_driver(drivers[d].path, &rows[i], i); + total++; + if (got != rows[i].want) { + fprintf(stderr, + "match_bind_struct[%s][%s]: exit=%d want=%d\n", + drivers[d].name, rows[i].label, + got, rows[i].want); + fail++; + } + } + } + if (fail) { + fprintf(stderr, + "match_bind_struct: %d/%d fixtures failed\n", fail, total); + return 1; + } + printf("match_bind_struct: %d/%d ok\n", total, total); + return 0; +}