From 34c437fd633024a3fab38385bca5aa9d7ef7fe56 Mon Sep 17 00:00:00 2001 From: Hojun-Cho Date: Sat, 30 May 2026 04:54:12 +0900 Subject: [PATCH] wwstage: register error-structs in collectstructs so large-union struct-variant returns widen (#9) collectstructs registered a struct only when the typedecl body is N_TSTRUCT, so an error-struct (type X = !struct{...}, whose body is N_TBANG{N_TSTRUCT}) never entered wwstage's c.structs table. The name-keyed structlookup then missed at the return-widen sites, and wwstage dropped the struct construction when returning a struct variant of a large (>4-eightbyte) union -- wrong runtime value and cs!=ww. cstage has no struct name-table (pure tinfo) and was correct. Peel the N_TBANG body in collectstructs so error-structs register; both existing cstage-mirrored widen arms then fire. Provably byte-id-inert: no committed source defines a !struct today. Adds test/wcc/785 (struct-variant return + named-void control, both-stage byte-id + runtime). The >4-eightbyte 5th-word truncation on return remains, symmetric (cs==ww) and unread by the tag/early-word path; #222's sret hidden-pointer cutover is the committed fix (table-retirement tracked as the wwstage->tinfo SSoT arc). Aligns wwstage up to cstage (rule-10). --- Makefile | 11 + selfhost/cmd/w6c/main.combined.ww | 25 ++ selfhost/cmd/wcc/cgenstmt.ww | 11 + selfhost/cmd/wcc/cgenutil.ww | 14 + selfhost/cmd/wwdump/main.combined.ww | 25 ++ .../wcc/785_structvariant_largeunion_return.c | 269 ++++++++++++++++++ 6 files changed, 355 insertions(+) create mode 100644 test/wcc/785_structvariant_largeunion_return.c diff --git a/Makefile b/Makefile index a0243fe3..f3db0929 100644 --- a/Makefile +++ b/Makefile @@ -336,6 +336,7 @@ TESTS = $(BIN)/test_smoke $(BIN)/test_lex $(BIN)/test_parse $(BIN)/test_check \ $(BIN)/test_fieldfn_leaf_collide_run \ $(BIN)/test_amp_fn_assign_run \ $(BIN)/test_xmod_alias_struct_collide_run \ + $(BIN)/test_structvariant_largeunion_return \ $(BIN)/test_bufio_vstream_run \ $(BIN)/test_log_vstream_run \ $(BIN)/test_use_promote_alias \ @@ -740,6 +741,16 @@ $(BIN)/test_xmod_alias_struct_collide_run: test/wcc/784_xmod_alias_struct_collid $(LIB)/libwwrt.a | $(BIN) $(CC) $(CFLAGS) -o $@ $< +# #9: returning a STRUCT variant of a LARGE (>4-eightbyte) tagged union. +# Both-stage byte-id + runtime (reads tag AND the widened &fn field, so a +# dropped-store regression can't hide behind self-consistent byte-id). +# Self-contained single-file probes via the driver, no lib imports. +$(BIN)/test_structvariant_largeunion_return: test/wcc/785_structvariant_largeunion_return.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_fmt_vstream_mods_run: test/wcc/780_fmt_vstream_mods_run.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 f2e3e0ef..fcc315e8 100644 --- a/selfhost/cmd/w6c/main.combined.ww +++ b/selfhost/cmd/w6c/main.combined.ww @@ -16349,6 +16349,20 @@ fn collectstructs(c: *cgen, file: *node) void = { for (d != nil) { if (d.kind == nkind.N_TYPEDECL) { let body: *node = d.lhs; + // #9: an error-struct (`type X = !struct{...}`) carries an + // N_TBANG-wrapped body; peel it so X registers like any + // struct. cstage is tinfo-based and needs no table, but + // wwstage's name-keyed widen dispatch (cgreturn needswiden + // → cgwidentaggedstore) resolves struct layout through + // c.structs; an unregistered error-struct made the + // struct-variant-of-large-union return silently drop its + // construction (cs!=ww). The strategic fix is #222's sret + // cutover, which deletes this name-keyed dispatch outright; + // until then the table must be complete (errors.opaque_ is + // the first such type). #10 tracks retiring the table. + if (body != nil && body.kind == nkind.N_TBANG) { + body = body.lhs; + }; if (body != nil) { if (body.kind == nkind.N_TSTRUCT) { registerstruct(c, d.str, d.nmod, body); @@ -25753,6 +25767,17 @@ fn cgreturn(c: *cgen, n: *node) void = { }; cgwidentaggedstore(c, c.fnret.type_: *tinfo, rhs, "BP", scroff, rsz); + // Tagged-return ABI loads at most 4 eightbytes + // (AX/DX/CX/R8). A union whose slot exceeds 32B + // (tag + >3 payload words, e.g. a 32B struct + // variant = 40B slot) drops its 5th+ word here — + // SYMMETRICALLY with cstage, so byte-id holds and + // the tag/early-word read paths are correct. The + // dropped tail is #222 (the >4-eightbyte sret ABI + // asymmetry); its real fix routes large unions + // through a hidden-pointer sret on both paths. + // Sound only while consumers never read the tail + // (errno's tag/strerror path does not). emitline("\tMOVQ\t"); emitoff(scroff: i64); emitline("(BP), AX\n"); diff --git a/selfhost/cmd/wcc/cgenstmt.ww b/selfhost/cmd/wcc/cgenstmt.ww index d613f67f..d55e7125 100644 --- a/selfhost/cmd/wcc/cgenstmt.ww +++ b/selfhost/cmd/wcc/cgenstmt.ww @@ -435,6 +435,17 @@ fn cgreturn(c: *cgen, n: *node) void = { }; cgwidentaggedstore(c, c.fnret.type_: *tinfo, rhs, "BP", scroff, rsz); + // Tagged-return ABI loads at most 4 eightbytes + // (AX/DX/CX/R8). A union whose slot exceeds 32B + // (tag + >3 payload words, e.g. a 32B struct + // variant = 40B slot) drops its 5th+ word here — + // SYMMETRICALLY with cstage, so byte-id holds and + // the tag/early-word read paths are correct. The + // dropped tail is #222 (the >4-eightbyte sret ABI + // asymmetry); its real fix routes large unions + // through a hidden-pointer sret on both paths. + // Sound only while consumers never read the tail + // (errno's tag/strerror path does not). emitline("\tMOVQ\t"); emitoff(scroff: i64); emitline("(BP), AX\n"); diff --git a/selfhost/cmd/wcc/cgenutil.ww b/selfhost/cmd/wcc/cgenutil.ww index 882933cd..23260df2 100644 --- a/selfhost/cmd/wcc/cgenutil.ww +++ b/selfhost/cmd/wcc/cgenutil.ww @@ -1838,6 +1838,20 @@ fn collectstructs(c: *cgen, file: *node) void = { for (d != nil) { if (d.kind == nkind.N_TYPEDECL) { let body: *node = d.lhs; + // #9: an error-struct (`type X = !struct{...}`) carries an + // N_TBANG-wrapped body; peel it so X registers like any + // struct. cstage is tinfo-based and needs no table, but + // wwstage's name-keyed widen dispatch (cgreturn needswiden + // → cgwidentaggedstore) resolves struct layout through + // c.structs; an unregistered error-struct made the + // struct-variant-of-large-union return silently drop its + // construction (cs!=ww). The strategic fix is #222's sret + // cutover, which deletes this name-keyed dispatch outright; + // until then the table must be complete (errors.opaque_ is + // the first such type). #10 tracks retiring the table. + if (body != nil && body.kind == nkind.N_TBANG) { + body = body.lhs; + }; if (body != nil) { if (body.kind == nkind.N_TSTRUCT) { registerstruct(c, d.str, d.nmod, body); diff --git a/selfhost/cmd/wwdump/main.combined.ww b/selfhost/cmd/wwdump/main.combined.ww index 9de4130c..02ac53d5 100644 --- a/selfhost/cmd/wwdump/main.combined.ww +++ b/selfhost/cmd/wwdump/main.combined.ww @@ -16349,6 +16349,20 @@ fn collectstructs(c: *cgen, file: *node) void = { for (d != nil) { if (d.kind == nkind.N_TYPEDECL) { let body: *node = d.lhs; + // #9: an error-struct (`type X = !struct{...}`) carries an + // N_TBANG-wrapped body; peel it so X registers like any + // struct. cstage is tinfo-based and needs no table, but + // wwstage's name-keyed widen dispatch (cgreturn needswiden + // → cgwidentaggedstore) resolves struct layout through + // c.structs; an unregistered error-struct made the + // struct-variant-of-large-union return silently drop its + // construction (cs!=ww). The strategic fix is #222's sret + // cutover, which deletes this name-keyed dispatch outright; + // until then the table must be complete (errors.opaque_ is + // the first such type). #10 tracks retiring the table. + if (body != nil && body.kind == nkind.N_TBANG) { + body = body.lhs; + }; if (body != nil) { if (body.kind == nkind.N_TSTRUCT) { registerstruct(c, d.str, d.nmod, body); @@ -25753,6 +25767,17 @@ fn cgreturn(c: *cgen, n: *node) void = { }; cgwidentaggedstore(c, c.fnret.type_: *tinfo, rhs, "BP", scroff, rsz); + // Tagged-return ABI loads at most 4 eightbytes + // (AX/DX/CX/R8). A union whose slot exceeds 32B + // (tag + >3 payload words, e.g. a 32B struct + // variant = 40B slot) drops its 5th+ word here — + // SYMMETRICALLY with cstage, so byte-id holds and + // the tag/early-word read paths are correct. The + // dropped tail is #222 (the >4-eightbyte sret ABI + // asymmetry); its real fix routes large unions + // through a hidden-pointer sret on both paths. + // Sound only while consumers never read the tail + // (errno's tag/strerror path does not). emitline("\tMOVQ\t"); emitoff(scroff: i64); emitline("(BP), AX\n"); diff --git a/test/wcc/785_structvariant_largeunion_return.c b/test/wcc/785_structvariant_largeunion_return.c new file mode 100644 index 00000000..cece472f --- /dev/null +++ b/test/wcc/785_structvariant_largeunion_return.c @@ -0,0 +1,269 @@ +/* + * 785_structvariant_largeunion_return — project #9 close. Pins that + * RETURNING a STRUCT variant of a LARGE (>4-eightbyte) tagged union + * constructs the struct and lays it into the union payload slot, on + * BOTH stages byte-identically (rule-10). + * + * THE BUG (wwstage-only, cs!=ww): wwstage's return-widen DISPATCH + * (cgreturn needswiden -> cgwidentaggedstore, selfhost/cmd/wcc/ + * cgenstmt.ww + cgenutil.ww) is name-keyed via structlookup. An + * error-struct (`type big = !struct{...}`, the errors.opaque_ shape) + * was NEVER registered in c.structs — collectstructs only registered + * a bare N_TSTRUCT body, not the N_TBANG-wrapped one — so structlookup + * missed it, needswiden stayed false, and the return collapsed to a + * register move (frame $32) that DROPPED both the `x.f = &impl` field + * store and the 32B struct -> 40B union-payload copy (wrong runtime + * value AND 34-line asm divergence vs cstage's correct $80 frame). + * cstage is tinfo-based (peel src->type -> TY_STRUCT, cgen.c:2058) and + * needs no table, so it built correctly. + * + * THE FIX (A1, collectstructs N_TBANG peel): register error-structs + * like any struct so the existing already-cstage-mirrored widen arms + * fire. Zero byte-id delta on the prior corpus (no committed source + * defined a `!struct` before errors.opaque_). The strategic fix is + * #222's sret cutover (deletes this name-keyed dispatch); #10 tracks + * retiring the c.structs table. + * + * GATE-BLIND class: byte-id alone is necessary-not-sufficient — a + * dropped store keeps cs==ww on self-consistent garbage. So the + * struct_variant row READS BACK BOTH the tag (match arm) AND the + * widened field (calls through z.f), and asserts the runtime exit. + * + * row | shape | exit | byte-id + * -----------------+----------------------------------------+------+-------- + * struct_variant | mkbig: x.f=&impl; return x (struct | 42 | cs==ww + * | variant of 40B union); main matches | | + * | big, calls (*z.f)(&1)=1+41 — proves | | + * | the field survived the widen | | + * namedvoid_ctrl | mksmall: return y (named-void variant | 22 | cs==ww + * | of the same 40B union) — the path | | + * | that was correct pre-fix; guards we | | + * | did not regress it | | + * + * GATE POLARITY: must stay GREEN. Red means the struct-variant widen + * dropped its construction again, or the named-void variant regressed. + * GRADUATES nothing — #222's residual 5th-eightbyte truncation on the + * 40B return is symmetric (cs==ww) and unread by this match path. + */ +#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; +} + +#define STAGE_CS 1 +#define STAGE_WW 2 + +struct row { + const char *label; + const char *src; + int want_exit; + int stage_mask; + int byte_id; +}; + +static const struct row rows[] = { + /* POSITIVE byte-id + runtime: struct variant of a 40B union is + * constructed, the &fn field stored, returned, then matched and + * called through — exit proves the widened field survived. */ + { "struct_variant", + "package main;\n" + "type a = !void; type b = !void;\n" + "type big = !struct { f: *fn(p: *i32) i32, data: [3]u64 };\n" + "type u = !(a | b | big);\n" + "fn impl(p: *i32) i32 = { return *p + 41i32; };\n" + "fn mkbig() u = { let x: big; x.f = (&impl): *fn(p: *i32) i32; return x; };\n" + "export fn main() i32 = {\n" + " let r = mkbig();\n" + " match (r) {\n" + " case a => return 90;\n" + " case b => return 91;\n" + " case let z: big => { let n: i32 = 1i32; let fp = z.f; return (*fp)(&n): i32; };\n" + " };\n" + "};\n", + 42, STAGE_CS | STAGE_WW, 1 }, + + /* CONTROL byte-id + runtime: named-void variant of the SAME 40B + * union — the path that was already correct pre-fix. */ + { "namedvoid_ctrl", + "package main;\n" + "type a = !void; type b = !void;\n" + "type big = !struct { f: *fn(p: *i32) i32, data: [3]u64 };\n" + "type u = !(a | b | big);\n" + "fn mksmall() u = { let y: b; return y; };\n" + "export fn main() i32 = {\n" + " let r = mksmall();\n" + " match (r) {\n" + " case a => return 90;\n" + " case b => return 22;\n" + " case big => return 9;\n" + " };\n" + "};\n", + 22, STAGE_CS | STAGE_WW, 1 }, +}; + +static void +cleanup_tmp(const char *tmpdir, const char *base) +{ + char p[1024]; + snprintf(p, sizeof p, "%s/%s", tmpdir, base); unlink(p); + snprintf(p, sizeof p, "%s/%s.ww", tmpdir, base); unlink(p); + snprintf(p, sizeof p, "%s/%s.s", tmpdir, base); unlink(p); + snprintf(p, sizeof p, "%s/%s.o", tmpdir, base); unlink(p); + snprintf(p, sizeof p, "%s/%s.combined.ww", tmpdir, base); unlink(p); + rmdir(tmpdir); +} + +static int +write_source(const char *path, const char *src) +{ + FILE *f = fopen(path, "wb"); + if (!f) return -1; + fputs(src, f); + fclose(f); + return 0; +} + +static int +build_via_driver(const char *driver, const char *tmpdir, const char *src) +{ + char cmd[2048]; + snprintf(cmd, sizeof cmd, "cd %s && timeout 180 %s build %s 2>/dev/null", + tmpdir, driver, src); + return runwait(cmd); +} + +/* run_row — build via driver, run the binary, return exit (or -1 on + * build failure). */ +static int +run_row(const char *driver, const struct row *r, int seq) +{ + char tmpdir[256], src[512], base[64], outbin[768]; + snprintf(tmpdir, sizeof tmpdir, "/tmp/svlu_%d_d_%d", getpid(), seq); + snprintf(base, sizeof base, "main785"); + snprintf(src, sizeof src, "%s/%s.ww", tmpdir, base); + mkdir(tmpdir, 0755); + if (write_source(src, r->src) != 0) { cleanup_tmp(tmpdir, base); return -1; } + int rc; + if (build_via_driver(driver, tmpdir, src) == 0) { + snprintf(outbin, sizeof outbin, "%s/%s", tmpdir, base); + rc = runwait(outbin); + } else { + rc = -1; + } + cleanup_tmp(tmpdir, base); + return rc; +} + +/* asm_byte_identical — diff cstage vs wwstage .s. Parallel trees so + * ww_ww writing intermediates next to the source doesn't clobber the + * cstage .s (CLAUDE.md rule 14 phase split). */ +static int +asm_byte_identical(const char *cdrv, const char *wdrv, const struct row *r, + int seq) +{ + char src[512], tdc[256], tdw[256], base[64], cs[512], ws[512]; + snprintf(tdc, sizeof tdc, "/tmp/svlu_%d_c_%d", getpid(), seq); + snprintf(tdw, sizeof tdw, "/tmp/svlu_%d_w_%d", getpid(), seq); + snprintf(base, sizeof base, "main785"); + mkdir(tdc, 0755); + mkdir(tdw, 0755); + snprintf(src, sizeof src, "%s/%s.ww", tdc, base); + if (write_source(src, r->src) != 0) { cleanup_tmp(tdc, base); cleanup_tmp(tdw, base); return -1; } + int rc = -1; + if (build_via_driver(cdrv, tdc, src) != 0) goto out; + snprintf(cs, sizeof cs, "%s/%s.s", tdc, base); + + snprintf(src, sizeof src, "%s/%s.ww", tdw, base); + if (write_source(src, r->src) != 0) goto out; + if (build_via_driver(wdrv, tdw, src) != 0) goto out; + snprintf(ws, sizeof ws, "%s/%s.s", tdw, base); + + FILE *fc = fopen(cs, "rb"); + FILE *fw = fopen(ws, "rb"); + if (fc && fw) { + rc = 0; + for (;;) { + int a = fgetc(fc); + int b = fgetc(fw); + if (a != b) { rc = -1; break; } + if (a == EOF) break; + } + } + if (fc) fclose(fc); + if (fw) fclose(fw); +out: + cleanup_tmp(tdc, base); + cleanup_tmp(tdw, base); + return rc; +} + +int +main(void) +{ + const char *bin = getenv("BIN"); + if (!bin) bin = "out/bin"; + char cwd[256]; + if (getcwd(cwd, sizeof cwd) == NULL) return 1; + char absbin[512]; + if (bin[0] != '/') { + snprintf(absbin, sizeof absbin, "%s/%s", cwd, bin); + bin = absbin; + } + + char cdrv[640], wdrv[640]; + snprintf(cdrv, sizeof cdrv, "%s/ww", bin); + snprintf(wdrv, sizeof wdrv, "%s/ww_ww", bin); + + int n = (int)(sizeof rows / sizeof rows[0]); + int total = 0, fail = 0, seq = 0; + int wwpresent = (access(wdrv, X_OK) == 0); + + for (int i = 0; i < n; i++) { + const struct row *r = &rows[i]; + + if (r->stage_mask & STAGE_CS) { + total++; + int got = run_row(cdrv, r, seq++); + if (got != r->want_exit) { + fprintf(stderr, "structvariant[cs][%s]: exit=%d want=%d\n", + r->label, got, r->want_exit); + fail++; + } + } + + if (wwpresent && (r->stage_mask & STAGE_WW)) { + total++; + int got = run_row(wdrv, r, seq++); + if (got != r->want_exit) { + fprintf(stderr, "structvariant[ww][%s]: exit=%d want=%d\n", + r->label, got, r->want_exit); + fail++; + } + if (r->byte_id) { + total++; + if (asm_byte_identical(cdrv, wdrv, r, seq++) != 0) { + fprintf(stderr, "structvariant[byte-id][%s]: cstage vs wwstage asm differs\n", + r->label); + fail++; + } + } + } + } + + if (fail) { + fprintf(stderr, "structvariant: %d/%d checks failed\n", fail, total); + return 1; + } + printf("structvariant: %d/%d ok\n", total, total); + return 0; +}