diff --git a/Makefile b/Makefile index ef0d5821..aa2b8d0d 100644 --- a/Makefile +++ b/Makefile @@ -233,6 +233,7 @@ TESTS = $(BIN)/test_smoke $(BIN)/test_lex $(BIN)/test_parse $(BIN)/test_check \ $(BIN)/test_dot_structlit \ $(BIN)/test_nested_call_rhs \ $(BIN)/test_fnlabel_mangle \ + $(BIN)/test_cgreturn_variant_zero \ $(BIN)/test_use_promote_alias \ $(BIN)/test_field_signed $(BIN)/test_frame_argcount \ $(BIN)/test_selfhost $(BIN)/test_w6a_ww $(BIN)/test_w6l_ww \ @@ -416,6 +417,12 @@ $(BIN)/test_fnlabel_mangle: test/wcc/706_fnlabel_mangle.c \ $(LIB)/libwwrt.a | $(BIN) $(CC) $(CFLAGS) -o $@ $< +$(BIN)/test_cgreturn_variant_zero: test/wcc/707_cgreturn_variant_zero.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_use_promote_alias: test/wcc/699_use_promote_alias.c \ $(BIN)/ww $(BIN)/w6c $(BIN)/w6a $(BIN)/w6l \ $(LIB)/libwwrt.a | $(BIN) diff --git a/cmd/w6c/cgen.c b/cmd/w6c/cgen.c index 4a9cea53..31a5bee9 100644 --- a/cmd/w6c/cgen.c +++ b/cmd/w6c/cgen.c @@ -1082,6 +1082,12 @@ cg_widen_tagged_store(Cg *c, Local **locals_p, Type *dst, Node *src, amem(D_BP, write_off + k)); } } else { + /* Tagged source returned via the tagged-return ABI + * (AX=tag, DX=word0, CX=word1, R8=word2). The unused + * ABI words are zeroed by the producer (#18 cgreturn + * variant-widen) so the unconditional store here is + * safe even when the source variant has fewer payload + * words than the dst slot. */ cgexpr(c, src, *locals_p); ins2(c, A_MOVQ, areg(D_AX), amem(D_BP, write_off + 0)); @@ -6065,8 +6071,15 @@ cgstmt(Cg *c, Node *n, Local **locals, int *frame) * Tagged-return ABI: AX=tag, DX=word0, * CX=word1, R8=word2. Slice payload uses * all four; str uses three; scalar uses - * two. */ + * two. Unused ABI words must still be + * zeroed because the receiver + * (cg_widen_tagged_store call-source arm) + * writes AX/DX/CX/R8 unconditionally sized + * by the dst slot; stale CX/R8 from the + * caller (e.g. a slice-stride IMULQ) would + * land in slot+16 / slot+24. (Task #18.) */ int tag = cg_tag_for_variant(rt, vt); + int rsz = (int)rt->size; cgexpr(c, n->lhs, *locals); if (type_isslice(vt)) { /* cgexpr leaves (AX=ptr, BX=len, @@ -6083,9 +6096,23 @@ cgstmt(Cg *c, Node *n, Local **locals, int *frame) areg(D_CX)); ins2(c, A_MOVQ, areg(D_AX), areg(D_DX)); + /* str fills DX,CX. Zero R8 if dst + * slot covers slot+24. */ + if (rsz > 24) + ins2(c, A_MOVQ, aimm(0), + areg(D_R8)); } else { ins2(c, A_MOVQ, areg(D_AX), areg(D_DX)); + /* scalar fills DX only. Zero + * CX / R8 if dst slot covers + * slot+16 / slot+24. */ + if (rsz > 16) + ins2(c, A_MOVQ, aimm(0), + areg(D_CX)); + if (rsz > 24) + ins2(c, A_MOVQ, aimm(0), + areg(D_R8)); } ins2(c, A_MOVQ, aimm(tag < 0 ? 0 : tag), areg(D_AX)); diff --git a/selfhost/cmd/w6c/main.combined.ww b/selfhost/cmd/w6c/main.combined.ww index 8e47805a..7ef2a17b 100644 --- a/selfhost/cmd/w6c/main.combined.ww +++ b/selfhost/cmd/w6c/main.combined.ww @@ -14663,6 +14663,14 @@ fn cgreturn(c: *cgen, n: *node) void = { return; }; let idx: i32 = taggedvariantindex(c, c.fnret, rhs); + // Tagged-return ABI: AX=tag, DX=word0, CX=word1, + // R8=word2. Receiver (cgwidentaggedstore call-source + // arm) writes AX/DX/CX/R8 unconditionally sized by the + // dst slot; unused ABI words must be zeroed here so a + // stale CX/R8 from the caller (e.g. a slice-stride + // IMULQ before the call) does not land in slot+16 / + // slot+24. (Task #18.) + let rsz: i32 = slotsize(c, c.fnret); if (nodeisslice(c, rhs)) { // cgexpr leaves (AX=ptr, BX=len, CX=cap). // Shuffle into return ABI: DX=ptr, CX=len, @@ -14673,8 +14681,20 @@ fn cgreturn(c: *cgen, n: *node) void = { } else { if (nodeisstr(c, rhs)) { emitline("\tMOVQ\tBX, CX\n"); emitline("\tMOVQ\tAX, DX\n"); + // str fills DX,CX. Zero R8 if dst covers slot+24. + if (rsz > 24) { + emitline("\tMOVQ\t$0, R8\n"); + }; } else { emitline("\tMOVQ\tAX, DX\n"); + // scalar fills DX only. Zero CX / R8 if dst + // covers slot+16 / slot+24. + if (rsz > 16) { + emitline("\tMOVQ\t$0, CX\n"); + }; + if (rsz > 24) { + emitline("\tMOVQ\t$0, R8\n"); + }; };}; emitline("\tMOVQ\t$"); if (idx < 0) { idx = 0; }; diff --git a/selfhost/cmd/wcc/cgenstmt.ww b/selfhost/cmd/wcc/cgenstmt.ww index dd98aee1..51a9e889 100644 --- a/selfhost/cmd/wcc/cgenstmt.ww +++ b/selfhost/cmd/wcc/cgenstmt.ww @@ -230,6 +230,14 @@ fn cgreturn(c: *cgen, n: *node) void = { return; }; let idx: i32 = taggedvariantindex(c, c.fnret, rhs); + // Tagged-return ABI: AX=tag, DX=word0, CX=word1, + // R8=word2. Receiver (cgwidentaggedstore call-source + // arm) writes AX/DX/CX/R8 unconditionally sized by the + // dst slot; unused ABI words must be zeroed here so a + // stale CX/R8 from the caller (e.g. a slice-stride + // IMULQ before the call) does not land in slot+16 / + // slot+24. (Task #18.) + let rsz: i32 = slotsize(c, c.fnret); if (nodeisslice(c, rhs)) { // cgexpr leaves (AX=ptr, BX=len, CX=cap). // Shuffle into return ABI: DX=ptr, CX=len, @@ -240,8 +248,20 @@ fn cgreturn(c: *cgen, n: *node) void = { } else { if (nodeisstr(c, rhs)) { emitline("\tMOVQ\tBX, CX\n"); emitline("\tMOVQ\tAX, DX\n"); + // str fills DX,CX. Zero R8 if dst covers slot+24. + if (rsz > 24) { + emitline("\tMOVQ\t$0, R8\n"); + }; } else { emitline("\tMOVQ\tAX, DX\n"); + // scalar fills DX only. Zero CX / R8 if dst + // covers slot+16 / slot+24. + if (rsz > 16) { + emitline("\tMOVQ\t$0, CX\n"); + }; + if (rsz > 24) { + emitline("\tMOVQ\t$0, R8\n"); + }; };}; emitline("\tMOVQ\t$"); if (idx < 0) { idx = 0; }; diff --git a/selfhost/cmd/wwdump/main.combined.ww b/selfhost/cmd/wwdump/main.combined.ww index 0f207d15..784b19a0 100644 --- a/selfhost/cmd/wwdump/main.combined.ww +++ b/selfhost/cmd/wwdump/main.combined.ww @@ -14663,6 +14663,14 @@ fn cgreturn(c: *cgen, n: *node) void = { return; }; let idx: i32 = taggedvariantindex(c, c.fnret, rhs); + // Tagged-return ABI: AX=tag, DX=word0, CX=word1, + // R8=word2. Receiver (cgwidentaggedstore call-source + // arm) writes AX/DX/CX/R8 unconditionally sized by the + // dst slot; unused ABI words must be zeroed here so a + // stale CX/R8 from the caller (e.g. a slice-stride + // IMULQ before the call) does not land in slot+16 / + // slot+24. (Task #18.) + let rsz: i32 = slotsize(c, c.fnret); if (nodeisslice(c, rhs)) { // cgexpr leaves (AX=ptr, BX=len, CX=cap). // Shuffle into return ABI: DX=ptr, CX=len, @@ -14673,8 +14681,20 @@ fn cgreturn(c: *cgen, n: *node) void = { } else { if (nodeisstr(c, rhs)) { emitline("\tMOVQ\tBX, CX\n"); emitline("\tMOVQ\tAX, DX\n"); + // str fills DX,CX. Zero R8 if dst covers slot+24. + if (rsz > 24) { + emitline("\tMOVQ\t$0, R8\n"); + }; } else { emitline("\tMOVQ\tAX, DX\n"); + // scalar fills DX only. Zero CX / R8 if dst + // covers slot+16 / slot+24. + if (rsz > 16) { + emitline("\tMOVQ\t$0, CX\n"); + }; + if (rsz > 24) { + emitline("\tMOVQ\t$0, R8\n"); + }; };}; emitline("\tMOVQ\t$"); if (idx < 0) { idx = 0; }; diff --git a/test/wcc/707_cgreturn_variant_zero.c b/test/wcc/707_cgreturn_variant_zero.c new file mode 100644 index 00000000..0cc92555 --- /dev/null +++ b/test/wcc/707_cgreturn_variant_zero.c @@ -0,0 +1,288 @@ +/* + * 707_cgreturn_variant_zero — tagged-return ABI variant-widen zero-pad + * for unused AX/DX/CX/R8 words (task #18). + * + * Pre-fix: cgreturn's `!istagged && !isstruct` variant-widen arm only + * filled the registers a given variant actually uses (scalar → DX; + * str → DX, CX; slice → DX, CX, R8). The unused AX/CX/R8 words were + * left holding whatever the caller's last write parked there. The + * receive side (cg_widen_tagged_store call-source arm) writes + * AX/DX/CX/R8 into the dst slot unconditionally, sized by the slot + * total — so stale registers landed at slot+16 / slot+24 and silently + * corrupted the tagged-union slot. + * + * The corruption only surfaced when the caller had primed CX/R8 + * shortly before the call. Array / slice indexing emits + * `MOVQ $stride, CX; IMULQ CX, AX` — the canonical primer. Happens + * once per index, including in for-loop bodies. Hence the original + * "for-loop miscompile" framing; the bug is in fact context-free, + * but an index expression in any straight-line code triggers it + * just as well. + * + * Fix: in cgreturn's variant-widen arm, after the variant-specific + * register shuffle and before the tag-into-AX, emit `MOVQ $0, CX` / + * `MOVQ $0, R8` for variants that don't naturally fill those words, + * conditional on the dst tagged-union slot size. Symmetric across + * cstage cgen.c and wwstage cgenstmt.ww. + * + * What this test pins (runtime only): + * - Every scalar-variant arm of a 24B tagged-union return zeroes + * slot+16 (CX) regardless of caller priming. + * - The str variant of a 24B slot still propagates .len correctly + * into slot+16 (the fix's `if (rsz > 24)` guard skips the R8 + * zero for the 24B case, leaving CX/.len intact). + * - Cross-shape probes: straight-line array-index call, in-loop + * body call, nested-loop inner-body call. + * + * Asm byte-identity is NOT diffed here: wwstage has a pre-existing + * gap in taggedvariantindex's str/bool-N_IDENT lookup (returns 0 + * regardless of declared variant), so non-i64 variant rows produce + * a divergent `MOVQ $tag, AX`. The fix in this commit IS symmetric + * (the new `MOVQ $0, CX` / `MOVQ $0, R8` lines match byte-for-byte + * across stages); global bootstrap byte-identity at test 995 + * covers it. The wwstage variant-index gap is tracked separately. + * + * Test rows use `[N]i64` array indexing as the CX primer (well- + * supported on both stages) to dodge a pre-existing cstage/wwstage + * divergence in slice-creation reg scheduling. + */ +#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[] = { + /* 1. Straight-line array-index then scalar-variant 24B-tagged + * call. Array indexing emits MOVQ $8, CX; IMULQ CX, AX — so CX + * carries 8 into the call. Pre-fix: slot+16 = 8. Post-fix: 0. */ + { "scalar_24B_after_arridx", + "type ft = (i64 | str | bool);\n" + "fn aski64(v: i64) ft = { return v; };\n" + "fn main() i32 = {\n" + " let arr: [3]i64 = [10i64, 20i64, 30i64];\n" + " let i: i32 = 1;\n" + " let v: i64 = arr[i];\n" + " let a: ft = aski64(v);\n" + " let p: u64 = (&a): u64;\n" + " let p2: *i64 = (p + 16u64): *i64;\n" + " let w: i64 = *p2;\n" + " return w: i32;\n" + "};\n", + 0 }, + /* 2. Bool variant of the same 24B slot. i64-array-index primer + * (well-supported shape) -> derive bool -> call -> probe slot+16. + * Pre-fix: stride leak. Post-fix: 0. */ + { "bool_24B_after_arridx", + "type ft = (i64 | str | bool);\n" + "fn askbool(b: bool) ft = { return b; };\n" + "fn main() i32 = {\n" + " let arr: [3]i64 = [1i64, 0i64, 1i64];\n" + " let i: i32 = 0;\n" + " let v: i64 = arr[i];\n" + " let b: bool = v != 0i64;\n" + " let a: ft = askbool(b);\n" + " let p: u64 = (&a): u64;\n" + " let p2: *i64 = (p + 16u64): *i64;\n" + " let w: i64 = *p2;\n" + " return w: i32;\n" + "};\n", + 0 }, + /* 3. Str variant of a 24B slot — sender already fills CX with + * the len, so slot+16 was correct pre-fix too. Sanity row: must + * keep working; the fix's `if (rsz > 24)` guard skips the R8 + * zero for the 24B case, leaving CX (and slot+16) at len. The + * i64-array index primes CX; the str literal feeds askstr. */ + { "str_24B_after_arridx", + "type ft = (i64 | str | bool);\n" + "fn askstr(s: str) ft = { return s; };\n" + "fn main() i32 = {\n" + " let arr: [3]i64 = [10i64, 20i64, 30i64];\n" + " let i: i32 = 1;\n" + " let _v: i64 = arr[i];\n" + " let a: ft = askstr(\"world\");\n" + " let p: u64 = (&a): u64;\n" + " let p2: *i64 = (p + 16u64): *i64;\n" + " let w: i64 = *p2;\n" + " return w: i32;\n" + "};\n", + 5 }, + /* 4. In-loop body: scalar variant call. Same shape as row 1, + * but inside a `for (i < 3)` body where IMULQ primes CX + * on every iteration. Pre-fix: slot+16 leaks 8 every iter; + * sum-of-leaks > 0. Post-fix: 0. */ + { "scalar_24B_in_loop", + "type ft = (i64 | str | bool);\n" + "fn aski64(v: i64) ft = { return v; };\n" + "fn main() i32 = {\n" + " let arr: [3]i64 = [10i64, 20i64, 30i64];\n" + " let i: i32 = 0;\n" + " let bad: i32 = 0;\n" + " for (i < 3) {\n" + " let v: i64 = arr[i];\n" + " let a: ft = aski64(v);\n" + " let p: u64 = (&a): u64;\n" + " let p2: *i64 = (p + 16u64): *i64;\n" + " let w: i64 = *p2;\n" + " if (w != 0i64) { bad += 1; };\n" + " i += 1;\n" + " };\n" + " return bad;\n" + "};\n", + 0 }, + /* 5. Nested loop, inner-body scalar-variant call. Pins that the + * fix doesn't depend on loop depth. Each inner iter primes CX. */ + { "scalar_24B_in_nested_loop", + "type ft = (i64 | str | bool);\n" + "fn aski64(v: i64) ft = { return v; };\n" + "fn main() i32 = {\n" + " let arr: [2]i64 = [10i64, 20i64];\n" + " let i: i32 = 0;\n" + " let bad: i32 = 0;\n" + " for (i < 2) {\n" + " let j: i32 = 0;\n" + " for (j < 2) {\n" + " let v: i64 = arr[j];\n" + " let a: ft = aski64(v);\n" + " let p: u64 = (&a): u64;\n" + " let p2: *i64 = (p + 16u64): *i64;\n" + " let w: i64 = *p2;\n" + " if (w != 0i64) { bad += 1; };\n" + " j += 1;\n" + " };\n" + " i += 1;\n" + " };\n" + " return bad;\n" + "};\n", + 0 }, + /* 6. Mixed loop: alternating str / i64 variants — pins that the + * scalar-variant zero doesn't regress the str-variant CX fill + * across iterations. iter-0 stores str (slot+16 = 5), iter-1 + * stores i64 (slot+16 = 0). bad counts slot+16 != expected. */ + { "mixed_variants_in_loop", + "type ft = (i64 | str | bool);\n" + "fn askstr(s: str) ft = { return s; };\n" + "fn aski64(v: i64) ft = { return v; };\n" + "fn main() i32 = {\n" + " let arr: [2]i64 = [10i64, 20i64];\n" + " let i: i32 = 0;\n" + " let bad: i32 = 0;\n" + " for (i < 2) {\n" + " let _v: i64 = arr[i];\n" + " let a: ft;\n" + " let expect: i64 = 0i64;\n" + " if (i == 0) { a = askstr(\"world\"); expect = 5i64; };\n" + " if (i == 1) { a = aski64(99i64); expect = 0i64; };\n" + " let p: u64 = (&a): u64;\n" + " let p2: *i64 = (p + 16u64): *i64;\n" + " let w: i64 = *p2;\n" + " if (w != expect) { bad += 1; };\n" + " i += 1;\n" + " };\n" + " return bad;\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/wcrv_%d_%d.ww", getpid(), i); + snprintf(tmpdir, sizeof tmpdir, "/tmp/wcrv_%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[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]; + snprintf(cdrv, sizeof cdrv, "%s/ww", bin); + char wdrv[640]; + 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, "cgreturn_variant_zero: 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, + "cgreturn_variant_zero[%s][%s]: exit=%d want=%d\n", + drivers[d].name, rows[i].label, + got, rows[i].want); + fail++; + } + } + } + + if (fail) { + fprintf(stderr, + "cgreturn_variant_zero: %d/%d fixtures failed\n", fail, total); + return 1; + } + printf("cgreturn_variant_zero: %d/%d ok\n", total, total); + return 0; +}