From f9f83faca73732fb9d5685c51b0f7df80d695bcf Mon Sep 17 00:00:00 2001 From: Hojun-Cho Date: Fri, 29 May 2026 19:43:48 +0900 Subject: [PATCH] wwstage: remap error tag on try-propagate across reordered unions (#173) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit wwstage cgtryprop returned the operand union's RAW tag when propagating a `c(s)?` error, while cstage (cmd/w6c/cgen.c:6161-6184) remaps it to the enclosing return union's variant ordering. When the operand and return unions differ in variant order, wwstage propagated the WRONG error variant at runtime — gate-blind: byte-id (990-997) and cstage==wwstage asm both pass because the bootstrap only ever tries same-order unions, while the differing-order case is silently wrong. Port cstage's remap loop into cgtryprop (iserror-only): for each error variant whose return-union index differs, emit the CMPQ/JNE/MOVQ/JMP that rewrites the tag in AX; the error payload words (DX/CX/R8) are untouched and ride the RET. Mirror cstage's emission exactly — lazy tryprop_ret allocation on the first remap, j==i skip, j<0 fallback, no dead label when empty, identical label strings and operand order — so same-order emits zero extra instructions (byte-id preserved) and differing-order is now byte-identical cstage==wwstage. Scope: error-variant remap only. wwstage's hardcoded success-tag=0 and iserror-only error detection (vs cstage's cg_tagged_success_tag + cg_variant_is_error legacy fallback) diverge for non-idx-0-success or unmarked unions — also gate-blind, also latent — filed separately as #216. test/wcc/925_tryprop_tag_remap_run: 5 rows (differing-order for both error variants, success unwrap, same-order byte-id witness, and a multi-word !str payload row asserting the payload bytes survive the remap), each with a cstage==wwstage .s byte-id check. --- Makefile | 7 + selfhost/cmd/w6c/main.combined.ww | 55 ++++- selfhost/cmd/wcc/cgenexpr.ww | 55 ++++- selfhost/cmd/wwdump/main.combined.ww | 55 ++++- test/wcc/925_tryprop_tag_remap_run.c | 330 +++++++++++++++++++++++++++ 5 files changed, 493 insertions(+), 9 deletions(-) create mode 100644 test/wcc/925_tryprop_tag_remap_run.c diff --git a/Makefile b/Makefile index 097e8ca0..703c5143 100644 --- a/Makefile +++ b/Makefile @@ -248,6 +248,7 @@ TESTS = $(BIN)/test_smoke $(BIN)/test_lex $(BIN)/test_parse $(BIN)/test_check \ $(BIN)/test_signed_data_emit_run \ $(BIN)/test_tagged_call_arg \ $(BIN)/test_tagged_call_arg_run \ + $(BIN)/test_tryprop_tag_remap_run \ $(BIN)/test_sret_struct_return \ $(BIN)/test_sret_struct_return_run \ $(BIN)/test_sret_narrow_field \ @@ -822,6 +823,12 @@ $(BIN)/test_tagged_call_arg_run: test/wcc/924_tagged_call_arg_run.c \ $(LIB)/libwwrt.a | $(BIN) $(CC) $(CFLAGS) -o $@ $< +$(BIN)/test_tryprop_tag_remap_run: test/wcc/925_tryprop_tag_remap_run.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_sret_struct_return: test/wcc/721_sret_struct_return.c \ $(BIN)/w6c $(BIN)/w6c_ww | $(BIN) $(CC) $(CFLAGS) -o $@ $< diff --git a/selfhost/cmd/w6c/main.combined.ww b/selfhost/cmd/w6c/main.combined.ww index 60eec532..147314d1 100644 --- a/selfhost/cmd/w6c/main.combined.ww +++ b/selfhost/cmd/w6c/main.combined.ww @@ -18377,9 +18377,8 @@ fn cgtagvariantidx(c: *cgen, tagged: *node, vt: *node) i32 = { }; // cgtryprop — `e?` propagates the error variant up the stack. -// Legacy semantics only (success tag = 0). No tag remap; the -// selfhost code that uses ? today has the same variant order in -// operand and enclosing fn. +// Success tag = 0 (#216 tracks the legacy/flag-aware success-tag +// divergence — out of scope here, success check stays `CMPQ $0`). fn cgtryprop(c: *cgen, n: *node) void = { cgexpr(c, n.lhs); // AX = tag. If non-zero, this is an error; pop frame and RET. @@ -18388,6 +18387,56 @@ fn cgtryprop(c: *cgen, n: *node) void = { emitline("\tJE\t"); emitline(cl); emitline("\n"); + // #173: remap the operand union's error-variant tag to the + // enclosing fn return union's variant order before propagating. + // When the `?` operand and the enclosing fn return differ in + // variant order, the raw operand tag names the WRONG variant in + // the return union. Mirrors cstage cmd/w6c/cgen.c:6161-6184. + // Payload words DX/CX/R8 ride the RET untouched; only AX (the + // tag) is rewritten. Same-order unions map every error variant to + // itself → zero instructions, byte-id with the pre-#173 emit. + let u: *tinfo = nil; + if (n.lhs != nil) { u = n.lhs.type_: *tinfo; }; + for (u != nil && u.kind == tykind.TY_NAMED) { u = u.under; }; + let r: *tinfo = nil; + if (c.fnret != nil) { r = c.fnret.type_: *tinfo; }; + for (r != nil && r.kind == tykind.TY_NAMED) { r = r.under; }; + if (u != nil && r != nil && r.kind == tykind.TY_TAGGED && u.params != nil) { + let propret: str; + propret.ptr = nil; propret.len = 0; + let haveret: bool = false; + let p: *tparam = u.params; + let i: i32 = 0; + for (p != nil) { + if (p.iserror) { + let j: i32 = flatvariantidxt(r, p.type_); + if (j < 0) { j = 0; }; + if (j != i) { + let skip: str = mklabel(c, "tryprop_skip"); + emitline("\tCMPQ\t$"); + emitint(i: i64); + emitline(", AX\n"); + emitline("\tJNE\t"); + emitline(skip); + emitline("\n"); + emitline("\tMOVQ\t$"); + emitint(j: i64); + emitline(", AX\n"); + if (!haveret) { + propret = mklabel(c, "tryprop_ret"); + haveret = true; + }; + emitline("\tJMP\t"); + emitline(propret); + emitline("\n"); + emitlabel(skip); + }; + }; + p = p.tnext; + i += 1; + }; + if (haveret) { emitlabel(propret); }; + }; emitline("\tMOVQ\tBP, SP\n\tPOPQ\tBP\n\tRET\n"); emitlabel(cl); // Success: unwrap value. Tag-only result was AX; the rest of diff --git a/selfhost/cmd/wcc/cgenexpr.ww b/selfhost/cmd/wcc/cgenexpr.ww index d9ac1f9f..fc8180b2 100644 --- a/selfhost/cmd/wcc/cgenexpr.ww +++ b/selfhost/cmd/wcc/cgenexpr.ww @@ -156,9 +156,8 @@ fn cgtagvariantidx(c: *cgen, tagged: *node, vt: *node) i32 = { }; // cgtryprop — `e?` propagates the error variant up the stack. -// Legacy semantics only (success tag = 0). No tag remap; the -// selfhost code that uses ? today has the same variant order in -// operand and enclosing fn. +// Success tag = 0 (#216 tracks the legacy/flag-aware success-tag +// divergence — out of scope here, success check stays `CMPQ $0`). fn cgtryprop(c: *cgen, n: *node) void = { cgexpr(c, n.lhs); // AX = tag. If non-zero, this is an error; pop frame and RET. @@ -167,6 +166,56 @@ fn cgtryprop(c: *cgen, n: *node) void = { emitline("\tJE\t"); emitline(cl); emitline("\n"); + // #173: remap the operand union's error-variant tag to the + // enclosing fn return union's variant order before propagating. + // When the `?` operand and the enclosing fn return differ in + // variant order, the raw operand tag names the WRONG variant in + // the return union. Mirrors cstage cmd/w6c/cgen.c:6161-6184. + // Payload words DX/CX/R8 ride the RET untouched; only AX (the + // tag) is rewritten. Same-order unions map every error variant to + // itself → zero instructions, byte-id with the pre-#173 emit. + let u: *tinfo = nil; + if (n.lhs != nil) { u = n.lhs.type_: *tinfo; }; + for (u != nil && u.kind == tykind.TY_NAMED) { u = u.under; }; + let r: *tinfo = nil; + if (c.fnret != nil) { r = c.fnret.type_: *tinfo; }; + for (r != nil && r.kind == tykind.TY_NAMED) { r = r.under; }; + if (u != nil && r != nil && r.kind == tykind.TY_TAGGED && u.params != nil) { + let propret: str; + propret.ptr = nil; propret.len = 0; + let haveret: bool = false; + let p: *tparam = u.params; + let i: i32 = 0; + for (p != nil) { + if (p.iserror) { + let j: i32 = flatvariantidxt(r, p.type_); + if (j < 0) { j = 0; }; + if (j != i) { + let skip: str = mklabel(c, "tryprop_skip"); + emitline("\tCMPQ\t$"); + emitint(i: i64); + emitline(", AX\n"); + emitline("\tJNE\t"); + emitline(skip); + emitline("\n"); + emitline("\tMOVQ\t$"); + emitint(j: i64); + emitline(", AX\n"); + if (!haveret) { + propret = mklabel(c, "tryprop_ret"); + haveret = true; + }; + emitline("\tJMP\t"); + emitline(propret); + emitline("\n"); + emitlabel(skip); + }; + }; + p = p.tnext; + i += 1; + }; + if (haveret) { emitlabel(propret); }; + }; emitline("\tMOVQ\tBP, SP\n\tPOPQ\tBP\n\tRET\n"); emitlabel(cl); // Success: unwrap value. Tag-only result was AX; the rest of diff --git a/selfhost/cmd/wwdump/main.combined.ww b/selfhost/cmd/wwdump/main.combined.ww index bb1207d2..0cadc597 100644 --- a/selfhost/cmd/wwdump/main.combined.ww +++ b/selfhost/cmd/wwdump/main.combined.ww @@ -18377,9 +18377,8 @@ fn cgtagvariantidx(c: *cgen, tagged: *node, vt: *node) i32 = { }; // cgtryprop — `e?` propagates the error variant up the stack. -// Legacy semantics only (success tag = 0). No tag remap; the -// selfhost code that uses ? today has the same variant order in -// operand and enclosing fn. +// Success tag = 0 (#216 tracks the legacy/flag-aware success-tag +// divergence — out of scope here, success check stays `CMPQ $0`). fn cgtryprop(c: *cgen, n: *node) void = { cgexpr(c, n.lhs); // AX = tag. If non-zero, this is an error; pop frame and RET. @@ -18388,6 +18387,56 @@ fn cgtryprop(c: *cgen, n: *node) void = { emitline("\tJE\t"); emitline(cl); emitline("\n"); + // #173: remap the operand union's error-variant tag to the + // enclosing fn return union's variant order before propagating. + // When the `?` operand and the enclosing fn return differ in + // variant order, the raw operand tag names the WRONG variant in + // the return union. Mirrors cstage cmd/w6c/cgen.c:6161-6184. + // Payload words DX/CX/R8 ride the RET untouched; only AX (the + // tag) is rewritten. Same-order unions map every error variant to + // itself → zero instructions, byte-id with the pre-#173 emit. + let u: *tinfo = nil; + if (n.lhs != nil) { u = n.lhs.type_: *tinfo; }; + for (u != nil && u.kind == tykind.TY_NAMED) { u = u.under; }; + let r: *tinfo = nil; + if (c.fnret != nil) { r = c.fnret.type_: *tinfo; }; + for (r != nil && r.kind == tykind.TY_NAMED) { r = r.under; }; + if (u != nil && r != nil && r.kind == tykind.TY_TAGGED && u.params != nil) { + let propret: str; + propret.ptr = nil; propret.len = 0; + let haveret: bool = false; + let p: *tparam = u.params; + let i: i32 = 0; + for (p != nil) { + if (p.iserror) { + let j: i32 = flatvariantidxt(r, p.type_); + if (j < 0) { j = 0; }; + if (j != i) { + let skip: str = mklabel(c, "tryprop_skip"); + emitline("\tCMPQ\t$"); + emitint(i: i64); + emitline(", AX\n"); + emitline("\tJNE\t"); + emitline(skip); + emitline("\n"); + emitline("\tMOVQ\t$"); + emitint(j: i64); + emitline(", AX\n"); + if (!haveret) { + propret = mklabel(c, "tryprop_ret"); + haveret = true; + }; + emitline("\tJMP\t"); + emitline(propret); + emitline("\n"); + emitlabel(skip); + }; + }; + p = p.tnext; + i += 1; + }; + if (haveret) { emitlabel(propret); }; + }; emitline("\tMOVQ\tBP, SP\n\tPOPQ\tBP\n\tRET\n"); emitlabel(cl); // Success: unwrap value. Tag-only result was AX; the rest of diff --git a/test/wcc/925_tryprop_tag_remap_run.c b/test/wcc/925_tryprop_tag_remap_run.c new file mode 100644 index 00000000..5b54c5cc --- /dev/null +++ b/test/wcc/925_tryprop_tag_remap_run.c @@ -0,0 +1,330 @@ +/* + * 925_tryprop_tag_remap_run — the `?` try operator must remap the + * operand union's error-variant tag into the ENCLOSING fn return + * union's variant order before propagating (project #173). + * + * Gate-blind hazard: when the `?` operand union and the enclosing fn + * return union differ in variant ORDER, the raw operand tag names the + * WRONG variant in the return union. Pre-fix wwstage cgtryprop did NO + * remap (selfhost/cmd/wcc/cgenexpr.ww) and propagated the raw tag, so + * the caller saw the wrong error variant at runtime — while the + * byte-id (990-997) and cs==ww asm gates both passed, because the + * bootstrap only ever tries SAME-order unions. cstage already remapped + * (cmd/w6c/cgen.c:6161-6184); the fix ports that loop into wwstage. + * + * Two assertions per row: + * - RUNTIME: build with both `ww` (cstage) and `ww_ww` (wwstage), + * run, compare the exit code (= which variant the caller matched). + * This is what was wrong pre-fix. + * - ASM BYTE-ID: compile the same source through `w6c` and `w6c_ww` + * and require byte-identical .s. Proves rule-10 (symmetric stages) + * holds AFTER the fix on differing-order too, and that same-order + * emits no remap (zero delta). + * + * Rows: + * 1. diff_order_e1 — g→e1 on a branched callee; f's union swaps the + * error order; caller asserts the e1 arm. Pre-fix wwstage = 2. + * 2. diff_order_e2 — g→e2; caller asserts the e2 arm. Proves BOTH + * error variants remap (not just the first). + * 3. diff_order_success — g→value; caller asserts the i32 success + * unwrap survives unperturbed by the remap. + * 4. same_order — f's union same order as g; the only shape the + * bootstrap exercises; byte-id witness that the remap is inert. + * 5. multiword — differing-order propagation of a MULTI-WORD `!str` + * error (payload in DX/CX/R8). Caller asserts the PAYLOAD BYTES + * ("AB") survive, not just the tag — proves the remap touches + * only AX and the payload regs ride the RET. + */ +#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[] = { + { "diff_order_e1", + "type e1 = !void;\n" + "type e2 = !void;\n" + "fn g(which: i32) (i32 | e1 | e2) = {\n" + " if (which == 1) { return e1{}; };\n" + " if (which == 2) { return e2{}; };\n" + " return 100;\n" + "};\n" + "fn f(which: i32) (i32 | e2 | e1) = {\n" + " let v: i32 = g(which)?;\n" + " return v + 1;\n" + "};\n" + "export fn main() i32 = {\n" + " match (f(1)) {\n" + " case let v: i32 => return 50;\n" + " case e2 => return 2;\n" + " case e1 => return 1;\n" + " };\n" + " return 99;\n" + "};\n", + 1 }, + { "diff_order_e2", + "type e1 = !void;\n" + "type e2 = !void;\n" + "fn g(which: i32) (i32 | e1 | e2) = {\n" + " if (which == 1) { return e1{}; };\n" + " if (which == 2) { return e2{}; };\n" + " return 100;\n" + "};\n" + "fn f(which: i32) (i32 | e2 | e1) = {\n" + " let v: i32 = g(which)?;\n" + " return v + 1;\n" + "};\n" + "export fn main() i32 = {\n" + " match (f(2)) {\n" + " case let v: i32 => return 50;\n" + " case e2 => return 2;\n" + " case e1 => return 1;\n" + " };\n" + " return 99;\n" + "};\n", + 2 }, + { "diff_order_success", + "type e1 = !void;\n" + "type e2 = !void;\n" + "fn g(which: i32) (i32 | e1 | e2) = {\n" + " if (which == 1) { return e1{}; };\n" + " if (which == 2) { return e2{}; };\n" + " return 100;\n" + "};\n" + "fn f(which: i32) (i32 | e2 | e1) = {\n" + " let v: i32 = g(which)?;\n" + " return v + 1;\n" + "};\n" + "export fn main() i32 = {\n" + " match (f(0)) {\n" + " case let v: i32 => { if (v != 101) { return 40; }; return 7; };\n" + " case e2 => return 2;\n" + " case e1 => return 1;\n" + " };\n" + " return 99;\n" + "};\n", + 7 }, + { "same_order", + "type e1 = !void;\n" + "type e2 = !void;\n" + "fn g(which: i32) (i32 | e1 | e2) = {\n" + " if (which == 1) { return e1{}; };\n" + " if (which == 2) { return e2{}; };\n" + " return 100;\n" + "};\n" + "fn f(which: i32) (i32 | e1 | e2) = {\n" + " let v: i32 = g(which)?;\n" + " return v + 1;\n" + "};\n" + "export fn main() i32 = {\n" + " match (f(1)) {\n" + " case let v: i32 => return 50;\n" + " case e1 => return 1;\n" + " case e2 => return 2;\n" + " };\n" + " return 99;\n" + "};\n", + 1 }, + { "multiword", + "type e1 = !void;\n" + "type emsg = !str;\n" + "fn g(which: i32) (i32 | e1 | emsg) = {\n" + " if (which == 1) { return e1{}; };\n" + " if (which == 2) { let m: emsg = \"AB\": emsg; return m; };\n" + " return 100;\n" + "};\n" + "fn f(which: i32) (i32 | emsg | e1) = {\n" + " let v: i32 = g(which)?;\n" + " return v + 1;\n" + "};\n" + "export fn main() i32 = {\n" + " match (f(2)) {\n" + " case let v: i32 => return 50;\n" + " case let s: emsg => {\n" + " if (s.len != 2) { return 30; };\n" + " if (s[0] != 65u8) { return 31; };\n" + " if (s[1] != 66u8) { return 32; };\n" + " return 7;\n" + " };\n" + " case e1 => return 1;\n" + " };\n" + " return 99;\n" + "};\n", + 7 }, +}; + +/* Write r->src to /.ww; returns 0 on success. */ +static int +write_src(const char *dir, const char *base, const struct row *r, char *out, + size_t outsz) +{ + snprintf(out, outsz, "%s/%s.ww", dir, base); + FILE *f = fopen(out, "wb"); + if (!f) return -1; + fputs(r->src, f); + fclose(f); + return 0; +} + +/* Build src with `driver build` inside its own subdir, run the binary, + * return its exit code (or -1 on a build/exec failure). */ +static int +build_run(const char *driver, const char *src, const char *workdir) +{ + char cmd[8192]; + snprintf(cmd, sizeof cmd, "cd %s && %s build %s > /dev/null 2>&1", + workdir, driver, src); + if (runwait(cmd) != 0) return -1; + + const char *base = strrchr(src, '/'); + base = base ? base + 1 : src; + char outbin[1024]; + snprintf(outbin, sizeof outbin, "%s/%s", workdir, base); + char *dot = strrchr(outbin, '.'); + if (dot && strcmp(dot, ".ww") == 0) *dot = '\0'; + return runwait(outbin); +} + +static int +files_equal(const char *a, const char *b) +{ + FILE *fa = fopen(a, "rb"); + FILE *fb = fopen(b, "rb"); + if (!fa || !fb) { if (fa) fclose(fa); if (fb) fclose(fb); return -1; } + int ca, cb, eq = 1; + do { + ca = fgetc(fa); + cb = fgetc(fb); + if (ca != cb) { eq = 0; break; } + } while (ca != EOF); + fclose(fa); + fclose(fb); + return eq; +} + +int +main(void) +{ + const char *bin = getenv("BIN"); + if (!bin) bin = "out/bin"; + char absbin[512]; + if (bin[0] != '/') { + char cwd[256]; + if (getcwd(cwd, sizeof cwd) == NULL) return 1; + snprintf(absbin, sizeof absbin, "%s/%s", cwd, bin); + bin = absbin; + } + + char cdrv[640], wdrv[640], cw6[640], ww6[640]; + snprintf(cdrv, sizeof cdrv, "%s/ww", bin); + snprintf(wdrv, sizeof wdrv, "%s/ww_ww", bin); + snprintf(cw6, sizeof cw6, "%s/w6c", bin); + snprintf(ww6, sizeof ww6, "%s/w6c_ww", bin); + + int have_ww = (access(wdrv, X_OK) == 0); + int have_w6cww = (access(ww6, X_OK) == 0); + + int n = (int)(sizeof rows / sizeof rows[0]); + int total = 0, fail = 0; + + for (int i = 0; i < n; i++) { + const struct row *r = &rows[i]; + + char dir[] = "/tmp/tprm.XXXXXX"; + if (mkdtemp(dir) == NULL) { + fprintf(stderr, "row[%s]: mkdtemp failed\n", r->label); + fail++; total++; + continue; + } + + char src[1024]; + if (write_src(dir, "p", r, src, sizeof src) != 0) { + fprintf(stderr, "row[%s]: write src failed\n", r->label); + fail++; total++; + goto cleanup; + } + + /* ASM byte-id: w6c vs w6c_ww .s must be identical. */ + if (have_w6cww) { + char css[1024], wss[1024], cmd[8192]; + snprintf(css, sizeof css, "%s/cs.s", dir); + snprintf(wss, sizeof wss, "%s/ww.s", dir); + snprintf(cmd, sizeof cmd, "%s -o %s %s > /dev/null 2>&1", + cw6, css, src); + int rc1 = runwait(cmd); + snprintf(cmd, sizeof cmd, "%s -o %s %s > /dev/null 2>&1", + ww6, wss, src); + int rc2 = runwait(cmd); + total++; + if (rc1 != 0 || rc2 != 0) { + fprintf(stderr, + "row[%s]: w6c/w6c_ww emit failed (%d/%d)\n", + r->label, rc1, rc2); + fail++; + } else if (files_equal(css, wss) != 1) { + fprintf(stderr, + "row[%s]: cs.s != ww.s (rule-10 break)\n", + r->label); + fail++; + } + } + + /* RUNTIME: cstage. */ + { + char wk[1024]; + snprintf(wk, sizeof wk, "%s/cs", dir); + mkdir(wk, 0755); + int got = build_run(cdrv, src, wk); + total++; + if (got != r->want) { + fprintf(stderr, + "row[%s][cstage]: exit=%d want=%d\n", + r->label, got, r->want); + fail++; + } + } + + /* RUNTIME: wwstage (the side #173 fixes). */ + if (have_ww) { + char wk[1024]; + snprintf(wk, sizeof wk, "%s/ww", dir); + mkdir(wk, 0755); + int got = build_run(wdrv, src, wk); + total++; + if (got != r->want) { + fprintf(stderr, + "row[%s][wwstage]: exit=%d want=%d\n", + r->label, got, r->want); + fail++; + } + } + +cleanup: + { + char rm[1100]; + snprintf(rm, sizeof rm, "rm -rf %s", dir); + runwait(rm); + } + } + + if (fail) { + fprintf(stderr, + "tryprop_tag_remap_run: %d/%d checks failed\n", fail, total); + return 1; + } + printf("tryprop_tag_remap_run: %d/%d ok\n", total, total); + return 0; +}