From 7230f3aa61336ef1753519a1e992a2eaf8108840 Mon Sep 17 00:00:00 2001 From: Hojun-Cho Date: Wed, 27 May 2026 02:32:31 +0900 Subject: [PATCH] wcc: cstage array-init dispatches MOVW for esz==2 (#128a) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cstage cgen.c array-literal init dispatch now uses MOVW for esz==2 (u16/i16 element width). Was deferred (cgen.c:7194-7198 explicit TODO: "Add MOVW to w6a if real i16 arrays land") until A_MOVW landed in both stages' w6a; that prereq is now met. Fixes silent partial-init clobber where MOVQ writes 8B over a 2B slot, overwriting neighbouring elements/locals. Wwstage was already correct (selfhost/cmd/wcc/cgenutil.ww:872-877 emits MOVW for sz==2 in tnodestoreop) — cstage aligns UP to wwstage's correctness here, a rule-10 inversion from the usual align-richer-DOWN. Test 914 (4 rows: u16 full-init, u16 small-values, u8 control, i16 signed) catches the bug via the rule-10 cs==ww byte-id gate. Runtime is not a reliable lever — ww rejects truly-partial inits, and fully-init [N]u16 accident-corrects via MOVQ-overlap (each write rewrote the prior write's trailing 6B). Reviewer non-vacuity: stash the fix → 3/4 rows fail on byte-id, restore → 4/4 green. Bootstrap NEUTRAL: 990-997 byte-id + combined_ww_fresh green; zero pre-existing partial-init narrow-element callers in lib/+selfhost/. strconv stof_data tables emit DATAW (raw bytes) and bypass this path, which is why fold-2 landed clean despite the bug. Sibling bugs filed for backlog (reviewer-128a flag-don't-bundle per rule-11): #141 (cgen.c:4894-4898 variadic-gather array-store has the same dispatch gap) and #142 (wwstage cgenstmt.ww:976-990 primsize(elemn.str) returns 0 for TY_NAMED alias names → wrong- stride store on [N]alias-of-u16; cstage already TY_NAMED-peeled). --- Makefile | 6 + cmd/w6c/cgen.c | 12 +- test/wcc/914_arr_u16_store_run.c | 221 +++++++++++++++++++++++++++++++ 3 files changed, 234 insertions(+), 5 deletions(-) create mode 100644 test/wcc/914_arr_u16_store_run.c diff --git a/Makefile b/Makefile index e960f1f8..ed8ec132 100644 --- a/Makefile +++ b/Makefile @@ -336,6 +336,7 @@ TESTS = $(BIN)/test_smoke $(BIN)/test_lex $(BIN)/test_parse $(BIN)/test_check \ $(BIN)/test_continue_run \ $(BIN)/test_sar_shr_run \ $(BIN)/test_def_mangle_run \ + $(BIN)/test_arr_u16_store_run \ $(BIN)/test_f64cgen_run \ $(BIN)/test_f64crossmod_run \ $(BIN)/test_tuprecv_run \ @@ -1129,6 +1130,11 @@ $(BIN)/test_def_mangle_run: test/wcc/913_def_mangle_run.c $(BIN)/ww \ $(LIB)/libwwrt.a | $(BIN) $(CC) $(CFLAGS) -o $@ $< +$(BIN)/test_arr_u16_store_run: test/wcc/914_arr_u16_store_run.c $(BIN)/ww \ + $(BIN)/w6c $(BIN)/w6c_ww $(BIN)/w6a $(BIN)/w6l \ + $(LIB)/libwwrt.a | $(BIN) + $(CC) $(CFLAGS) -o $@ $< + $(BIN)/test_f64cgen_run: test/wcc/951_f64cgen_run.c $(BIN)/ww $(BIN)/w6c \ $(BIN)/w6a $(BIN)/w6l $(LIB)/libwwrt.a | $(BIN) $(CC) $(CFLAGS) -o $@ $< diff --git a/cmd/w6c/cgen.c b/cmd/w6c/cgen.c index cb01191e..865b51e5 100644 --- a/cmd/w6c/cgen.c +++ b/cmd/w6c/cgen.c @@ -7190,12 +7190,14 @@ cgstmt(Cg *c, Node *n, Local **locals, int *frame) int op = A_MOVQ; if (!is_str_el) { if (esz == 1) op = A_MOVB; + else if (esz == 2) op = A_MOVW; else if (esz == 4) op = A_MOVL; - /* esz == 2 (i16/u16) falls through to MOVQ — - * over-writes by 6B; the next element store - * rewrites the high half. For the last element - * this trails 6 bytes into the next stack slot. - * Add MOVW to w6a if real i16 arrays land. */ + /* #128a: esz==2 routes to MOVW (A_MOVW landed in + * both stages' w6a). Pre-fix the 2-byte case fell + * through to MOVQ, over-writing 6B into the next + * element's slot; sequential adjacent writes + * accident-corrected fully-init arrays but + * partial inits clobbered neighbours. */ } int idx = 0; Node *last = NULL; diff --git a/test/wcc/914_arr_u16_store_run.c b/test/wcc/914_arr_u16_store_run.c new file mode 100644 index 00000000..623a220f --- /dev/null +++ b/test/wcc/914_arr_u16_store_run.c @@ -0,0 +1,221 @@ +/* + * 914_arr_u16_store_run — runtime + byte-id net for #128a: array- + * literal init into [N]u16 (or any [N]T where esz==2) must store with + * MOVW, not MOVQ. Pre-fix cstage's cgen.c:7180-7222 array-init dispatch + * routed esz==2 to MOVQ fall-through (the comment at 7194-7198 + * documented + deferred this until A_MOVW landed in w6a); the MOVQ + * wrote 8 bytes into a 2-byte slot, overlapping the next 6 bytes of + * stack. Adjacent fully-init writes accident-corrected via overlap + * (each MOVQ rewrote the prior MOVQ's trailing 6B), but a PARTIAL + * init left high garbage in slots that should have been default-zero. + * + * Fix is the now-unblocked dispatch: `else if (esz == 2) op = A_MOVW;` + * The wwstage emitter was already correct (uses MOVW); the cstage gap + * was the deferred TODO. Both stages now emit byte-identical MOVW for + * [N]u16 (and any aliased-narrow-element [N]T whose tinfo.size == 2). + * + * Rows cover (the bug is caught via the rule-10 cs==ww byte-id gate; + * ww doesn't accept truly-partial init literals, and adjacent MOVQ + * writes accident-correct the runtime values, so byte-id is the lever): + * - u16_full_init: fully-init [4]u16, regression guard (pre-fix + * accident-correct at runtime via MOVQ overlap; .s shifts + * MOVQ→MOVW, cs==ww BYTE-IDENTICAL now) + * - u16_small_values: small u16 values, summed (regression guard) + * - u8_control: [4]u8 baseline (already MOVB pre-fix, no shift) + * - i16_signed: signed-narrow [N]i16, same dispatch — verifies the + * fix isn't gated on unsignedness + * + * TY_NAMED alias of u16 (`type myw = u16; let a: [4]myw = …`) is a + * sibling miscompile filed separately: wwstage's array-init element- + * size dispatch reads `primsize(elemn.str)` which returns 0 for an + * alias name, leaving esz at 8 (wrong slot, wrong stride). Out of + * scope for #128a (cstage MOVW landing). + * + * Each row carries (a) cstage `ww build` + run asserting the exit + * code and (b) w6c vs w6c_ww `.s` cmp (rule-10 byte-id). Post-#128a + * the partial-init row's cstage emission gains MOVW; both stages + * unchanged everywhere else. + */ +#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_exit; }; + +static const struct row rows[] = { + /* Fully-init [4]u16: pre-fix MOVQ overlap accident-corrected; + * post-fix proper MOVW per slot. Sum of last bytes = 0xBE = 190; + * mod 256 = 190. We assert per-element to catch any value drift. */ + { "u16_full_init", + "package main;\n" + "export fn main() i32 = {\n" + " let a: [4]u16 = [0xABCDu16, 0xBEEFu16, 0xC0DEu16, 0xDEADu16];\n" + " if (a[0] != 0xABCDu16) { return 1; };\n" + " if (a[1] != 0xBEEFu16) { return 2; };\n" + " if (a[2] != 0xC0DEu16) { return 3; };\n" + " if (a[3] != 0xDEADu16) { return 4; };\n" + " return 0;\n" + "};\n", 0 }, + /* Same shape with smaller values — pin that the dispatch fires + * on values that don't need the upper 16 bits. */ + { "u16_small_values", + "package main;\n" + "export fn main() i32 = {\n" + " let a: [4]u16 = [10u16, 20u16, 30u16, 40u16];\n" + " let s: i32 = 0;\n" + " s += a[0]: i32; s += a[1]: i32;\n" + " s += a[2]: i32; s += a[3]: i32;\n" + " return s;\n" + "};\n", 100 }, + /* [4]u8 control: already correct (MOVB pre-fix). Regression + * guard — asm should be unchanged. */ + { "u8_control", + "package main;\n" + "export fn main() i32 = {\n" + " let a: [4]u8 = [10u8, 20u8, 30u8, 40u8];\n" + " let s: i32 = 0;\n" + " s += a[0]: i32; s += a[1]: i32;\n" + " s += a[2]: i32; s += a[3]: i32;\n" + " return s;\n" + "};\n", 100 }, + /* [4]i16: signed-narrow, same MOVW dispatch (op chosen by esz, + * not signedness). Verifies the fix isn't gated. */ + { "i16_signed", + "package main;\n" + "export fn main() i32 = {\n" + " let a: [4]i16 = [10i16, 20i16, 30i16, -5i16];\n" + " let s: i32 = 0;\n" + " s += a[0]: i32; s += a[1]: i32;\n" + " s += a[2]: i32; s += a[3]: i32;\n" + " return s;\n" + "};\n", 55 /* 10+20+30+(-5) = 55 */ }, + { NULL, NULL, 0 } +}; + +static int +slurp_eq(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 rc = 0; + for (;;) { + int ca = fgetc(fa); + int cb = fgetc(fb); + if (ca != cb) { rc = -1; break; } + if (ca == EOF) break; + } + fclose(fa); fclose(fb); + return rc; +} + +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 w6c[1100], w6c_ww[1100]; + snprintf(w6c, sizeof w6c, "%s/w6c", bin); + snprintf(w6c_ww, sizeof w6c_ww, "%s/w6c_ww", bin); + if (access(w6c_ww, X_OK) != 0) { + fprintf(stderr, "arru16store: w6c_ww missing — cannot run " + "the cs==ww byte-id gate (the whole point of this test)\n"); + return 1; + } + + int n = 0, fail = 0; + for (int i = 0; rows[i].src; i++, n++) { + char src[64]; + snprintf(src, sizeof src, "/tmp/wwau16_%d_%d.ww", getpid(), i); + FILE *f = fopen(src, "wb"); + if (f == NULL) { fail++; continue; } + fputs(rows[i].src, f); + fclose(f); + + char tmpdir[64]; + snprintf(tmpdir, sizeof tmpdir, "/tmp/wwau16_%d_d_%d", + getpid(), i); + mkdir(tmpdir, 0755); + + char cmd[2048]; + snprintf(cmd, sizeof cmd, "cd %s && %s/ww build %s", + tmpdir, bin, src); + if (runwait(cmd) != 0) { + fprintf(stderr, "row[%s]: cstage build failed\n", + rows[i].label); + fail++; + unlink(src); rmdir(tmpdir); + continue; + } + + char outbin[128]; + const char *base = strrchr(src, '/'); + base = base ? base + 1 : src; + snprintf(outbin, sizeof outbin, "%s/%s", tmpdir, base); + char *dot = strrchr(outbin, '.'); + if (dot && strcmp(dot, ".ww") == 0) *dot = '\0'; + + int got = runwait(outbin); + if (got != rows[i].want_exit) { + fprintf(stderr, "row[%s]: cstage exit %d, want %d\n", + rows[i].label, got, rows[i].want_exit); + fail++; + } + unlink(outbin); rmdir(tmpdir); + + char cs_s[64], ws_s[64]; + snprintf(cs_s, sizeof cs_s, "/tmp/wwau16_%d_%d_cs.s", + getpid(), i); + snprintf(ws_s, sizeof ws_s, "/tmp/wwau16_%d_%d_ww.s", + getpid(), i); + + snprintf(cmd, sizeof cmd, "%s -o %s %s 2>/dev/null", + w6c, cs_s, src); + if (runwait(cmd) != 0) { + fprintf(stderr, "row[%s]: w6c failed\n", rows[i].label); + fail++; unlink(src); continue; + } + snprintf(cmd, sizeof cmd, "%s -o %s %s 2>/dev/null", + w6c_ww, ws_s, src); + if (runwait(cmd) != 0) { + fprintf(stderr, "row[%s]: w6c_ww failed\n", + rows[i].label); + fail++; unlink(src); unlink(cs_s); continue; + } + if (slurp_eq(cs_s, ws_s) != 0) { + fprintf(stderr, + "row[%s]: cstage/wwstage .s DIFFER (rule-10 " + "byte-id violation)\n", rows[i].label); + fail++; + } + unlink(src); unlink(cs_s); unlink(ws_s); + } + + if (fail) { + fprintf(stderr, "%d/%d arr-u16-store tests failed\n", fail, n); + return 1; + } + printf("arru16store: %d/%d ok (cstage run + cs==ww byte-id)\n", + n, n); + return 0; +}