From 1de1b9a8a9fb1d6cb46baab11250387c6d93bc29 Mon Sep 17 00:00:00 2001 From: Hojun-Cho Date: Wed, 27 May 2026 03:07:49 +0900 Subject: [PATCH] wcc: module-imported array indexed-load via cg_dotbase_addr (#128b) Fix segfault-class memory corruption on `module.array[i]` indexed-read where both stages emitted MOVQ-not-LEAQ on the module-qualified base plus wrong stride. Extends the #135 cg_dotbase_addr/dotbaseaddr helper to handle the SK_USE module-ident-base case: when bt is NULL/ty_err and let_islet(base.str) resolves to TY_ARRAY, emit LEAQ base(SB),dst instead of MOVQ. Wwstage parallel via letvartnode/N_TARRAY check. Stride fix via let_var_type fallback in cgindex when n.lhs.kind==N_DOT. Use-site fix per #135 precedent (Option B); preserves cgdot's MOVQ semantics for the whole-array-assign defensive case (zero current consumers). Test 915 carries 3 module-u16 indexed-read rows (strconv.left_shift_table[0/2/4]) + 2 local-array controls; the strconv.left_shift_table[2]:u32 probe segfaulted (exit 139) pre-fix and exits cleanly post-fix. Broader width-variation rows (u8/u32/i32 module-imported) deferred as informational enhancement. Test 915 skips its inline cs==ww .s cmp on needs_import rows (line 217-222) since `ww build` only drives cstage; reviewer externally verified byte-id on /tmp/k128probe.combined.ww (driver-expanded form, no imports). Future enhancement: 915 could read the driver-emitted combined.ww and add a cmp leg there. Bootstrap NEUTRAL (zero current module.array[i] consumers; strconv decimal.ww uses IDENT-base from within package). 178/178 incl. 990-997 + combined_ww_fresh green. Sibling bugs #137 (chained N_DOT) / #141 (variadic-gather esz==2) / #142 (wwstage primsize-on-alias) properly deferred to backlog. --- Makefile | 6 + cmd/w6c/cgen.c | 52 ++++++ selfhost/cmd/w6c/main.combined.ww | 20 ++- selfhost/cmd/wcc/cgenexpr.ww | 20 ++- selfhost/cmd/wwdump/main.combined.ww | 20 ++- test/wcc/915_arr_module_index_run.c | 257 +++++++++++++++++++++++++++ 6 files changed, 372 insertions(+), 3 deletions(-) create mode 100644 test/wcc/915_arr_module_index_run.c diff --git a/Makefile b/Makefile index ed8ec132..9015c97b 100644 --- a/Makefile +++ b/Makefile @@ -337,6 +337,7 @@ TESTS = $(BIN)/test_smoke $(BIN)/test_lex $(BIN)/test_parse $(BIN)/test_check \ $(BIN)/test_sar_shr_run \ $(BIN)/test_def_mangle_run \ $(BIN)/test_arr_u16_store_run \ + $(BIN)/test_arr_module_index_run \ $(BIN)/test_f64cgen_run \ $(BIN)/test_f64crossmod_run \ $(BIN)/test_tuprecv_run \ @@ -1135,6 +1136,11 @@ $(BIN)/test_arr_u16_store_run: test/wcc/914_arr_u16_store_run.c $(BIN)/ww \ $(LIB)/libwwrt.a | $(BIN) $(CC) $(CFLAGS) -o $@ $< +$(BIN)/test_arr_module_index_run: test/wcc/915_arr_module_index_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 865b51e5..6e396a89 100644 --- a/cmd/w6c/cgen.c +++ b/cmd/w6c/cgen.c @@ -692,6 +692,8 @@ static Mod *mod_map; typedef struct LetVar LetVar; struct LetVar { const char *name; + Type *type; /* #128b: imported-let type lookup for module- + * qualified N_INDEX base esz dispatch. */ LetVar *next; }; static LetVar *letvars; @@ -905,6 +907,7 @@ let_collect(Cg *c, Node *file) if (let_emit_size(d->type) == 0) continue; LetVar *lv = amalloc(c->a, sizeof *lv); lv->name = d->str; + lv->type = d->type; /* #128b */ lv->next = letvars; letvars = lv; } @@ -919,6 +922,22 @@ let_islet(const char *name) return 0; } +/* #128b: look up a top-level let's type by leaf name. Sister of + * wwstage's letvartnode (selfhost/cmd/wcc/cgen.ww:999). Used at the + * cgindex / cg_dotbase_addr sites where a module-qualified base + * (`mod.arr`) leaves n->lhs->type NULL (SK_USE-bound module ident), + * so the imported array's element type / size must come through + * this let-map lookup instead. Returns NULL if name isn't a tracked + * top-level let. */ +static Type * +let_var_type(const char *name) +{ + if (name == NULL) return NULL; + for (LetVar *lv = letvars; lv; lv = lv->next) + if (strcmp(lv->name, name) == 0) return lv->type; + return NULL; +} + /* Glue `.` into a fresh arena buffer. */ static const char * mod_join(Cg *c, const char *mod, const char *ident) @@ -1240,6 +1259,26 @@ cg_dotbase_addr(Cg *c, Node *base, int dst_reg, Local *locals) Node *inner = base->lhs; if (inner == NULL || inner->kind != N_IDENT) return 0; Type *bt = inner->type; + /* #128b: module-qualified `mod.arr` where arr is an imported + * top-level `let X: [N]T`. The checker leaves SK_USE module-idents + * with NULL/ty_err type; detect via let_islet + let_var_type-of- + * TY_ARRAY and emit LEAQ X(SB) for the array's base address. + * Without this, the N_INDEX fallback at cgen.c:~6760 falls to + * cgexpr(base) which auto-MOVQs the symbol contents as if it + * were a pointer-var (= load 8 bytes of the array's first + * elements + treat as junk address) — segfault-class miscompile. */ + if (bt == NULL || bt == ty_err) { + if (let_islet(base->str)) { + Type *lt = let_var_type(base->str); + Type *lu = type_chase_named(lt); + if (lu && lu->kind == TY_ARRAY) { + ins2(c, A_LEAQ, masym(c, base->str), + areg(dst_reg)); + return 1; + } + } + return 0; + } Type *bu = type_chase_named(bt); if (bu == NULL) return 0; int viaptr = 0; @@ -6659,6 +6698,19 @@ cgexpr(Cg *c, Node *n, Local *locals) * For `*[N]T` drill through to the array so esz/esub reflect * T, not sizeof(array). */ Type *bt = n->lhs ? n->lhs->type : NULL; + /* #128b: module-qualified `mod.arr[i]` — n->lhs is N_DOT and + * its type is NULL (SK_USE-bound module ident). Look up the + * imported let's type via let_var_type so esz/esub reflect + * the imported array's element width instead of falling to + * the esz=1 default (→ MOVZBQ wrong-width load). Sister of + * the dst-side cg_dotbase_addr branch that emits LEAQ for + * the base address. */ + if ((bt == NULL || bt == ty_err) + && n->lhs && n->lhs->kind == N_DOT + && n->lhs->str + && let_islet(n->lhs->str)) { + bt = let_var_type(n->lhs->str); + } Type *u = (bt && bt->kind == TY_NAMED) ? bt->under : bt; Type *eff = idx_eff(bt); int esz = 1; diff --git a/selfhost/cmd/w6c/main.combined.ww b/selfhost/cmd/w6c/main.combined.ww index 4616c501..b0cdccb8 100644 --- a/selfhost/cmd/w6c/main.combined.ww +++ b/selfhost/cmd/w6c/main.combined.ww @@ -15378,8 +15378,26 @@ fn dotbaseaddr(c: *cgen, base: *node, dstreg: str) bool = { let inner: *node = base.lhs; if (inner == nil) { return false; }; if (inner.kind != nkind.N_IDENT) { return false; }; + // #128b: module-qualified `mod.arr` where arr is an imported + // top-level `let X: [N]T`. The checker leaves SK_USE module- + // idents without a localfindnode entry; detect via letvartnode + // resolving to N_TARRAY and emit LEAQ X(SB). Without this, the + // cgindex fallback's cgexpr(base) auto-MOVQs the symbol's first + // 8 bytes as if it were a pointer-var — wrong shape (cstage + // sister fix in cg_dotbase_addr). let lc: *local = localfindnode(c, inner.str); - if (lc == nil) { return false; }; + if (lc == nil) { + let gt: *node = letvartnode(c, base.str); + if (gt != nil && gt.kind == nkind.N_TARRAY) { + emitline("\tLEAQ\t"); + emitsymname(c, base.str); + emitline("(SB), "); + emitline(dstreg); + emitline("\n"); + return true; + }; + return false; + }; let bu: *tinfo = inner.type_: *tinfo; for (bu != nil && bu.kind == tykind.TY_NAMED) { bu = bu.under; }; if (bu == nil) { return false; }; diff --git a/selfhost/cmd/wcc/cgenexpr.ww b/selfhost/cmd/wcc/cgenexpr.ww index 6a46dbd2..9f7cabcf 100644 --- a/selfhost/cmd/wcc/cgenexpr.ww +++ b/selfhost/cmd/wcc/cgenexpr.ww @@ -759,8 +759,26 @@ fn dotbaseaddr(c: *cgen, base: *node, dstreg: str) bool = { let inner: *node = base.lhs; if (inner == nil) { return false; }; if (inner.kind != nkind.N_IDENT) { return false; }; + // #128b: module-qualified `mod.arr` where arr is an imported + // top-level `let X: [N]T`. The checker leaves SK_USE module- + // idents without a localfindnode entry; detect via letvartnode + // resolving to N_TARRAY and emit LEAQ X(SB). Without this, the + // cgindex fallback's cgexpr(base) auto-MOVQs the symbol's first + // 8 bytes as if it were a pointer-var — wrong shape (cstage + // sister fix in cg_dotbase_addr). let lc: *local = localfindnode(c, inner.str); - if (lc == nil) { return false; }; + if (lc == nil) { + let gt: *node = letvartnode(c, base.str); + if (gt != nil && gt.kind == nkind.N_TARRAY) { + emitline("\tLEAQ\t"); + emitsymname(c, base.str); + emitline("(SB), "); + emitline(dstreg); + emitline("\n"); + return true; + }; + return false; + }; let bu: *tinfo = inner.type_: *tinfo; for (bu != nil && bu.kind == tykind.TY_NAMED) { bu = bu.under; }; if (bu == nil) { return false; }; diff --git a/selfhost/cmd/wwdump/main.combined.ww b/selfhost/cmd/wwdump/main.combined.ww index 42eced79..f555a65c 100644 --- a/selfhost/cmd/wwdump/main.combined.ww +++ b/selfhost/cmd/wwdump/main.combined.ww @@ -15378,8 +15378,26 @@ fn dotbaseaddr(c: *cgen, base: *node, dstreg: str) bool = { let inner: *node = base.lhs; if (inner == nil) { return false; }; if (inner.kind != nkind.N_IDENT) { return false; }; + // #128b: module-qualified `mod.arr` where arr is an imported + // top-level `let X: [N]T`. The checker leaves SK_USE module- + // idents without a localfindnode entry; detect via letvartnode + // resolving to N_TARRAY and emit LEAQ X(SB). Without this, the + // cgindex fallback's cgexpr(base) auto-MOVQs the symbol's first + // 8 bytes as if it were a pointer-var — wrong shape (cstage + // sister fix in cg_dotbase_addr). let lc: *local = localfindnode(c, inner.str); - if (lc == nil) { return false; }; + if (lc == nil) { + let gt: *node = letvartnode(c, base.str); + if (gt != nil && gt.kind == nkind.N_TARRAY) { + emitline("\tLEAQ\t"); + emitsymname(c, base.str); + emitline("(SB), "); + emitline(dstreg); + emitline("\n"); + return true; + }; + return false; + }; let bu: *tinfo = inner.type_: *tinfo; for (bu != nil && bu.kind == tykind.TY_NAMED) { bu = bu.under; }; if (bu == nil) { return false; }; diff --git a/test/wcc/915_arr_module_index_run.c b/test/wcc/915_arr_module_index_run.c new file mode 100644 index 00000000..b30fca8f --- /dev/null +++ b/test/wcc/915_arr_module_index_run.c @@ -0,0 +1,257 @@ +/* + * 915_arr_module_index_run — runtime + byte-id net for #128b: a + * cross-package indexed read on an imported global `let arr: [N]T` + * (the `mod.arr[i]` shape) must compute the array's ADDRESS via LEAQ + * and stride by the element type's size. Pre-fix both stages + * mis-compiled this shape, but differently: + * - cstage: emitted `MOVQ mod.arr(SB), AX` (loaded the first 8 bytes + * of the array as if it were a pointer-var value), then ADDed the + * index, then MOVZBQ at that junk address — SEGFAULT-class memory + * corruption on any non-trivial index. cstage also defaulted esz=1 + * (the `bt = n->lhs->type` was NULL for SK_USE module idents). + * - wwstage: had the same MOVQ-not-LEAQ bug; in earlier session + * state also defaulted esz=8 + MOVQ-load (wrong stride + wrong + * load-width). By the time #128b ratified, wwstage was emitting + * IMUL$2 + MOVZWQ correctly (via stamped n.type_ path) but the + * base load remained MOVQ-not-LEAQ. + * + * Fix (per the design's option B — use-site at cgindex/cgassign, NOT + * a provider-shape change at cgdot): + * - cstage: extend `cg_dotbase_addr` (the #135 helper) to recognise + * module-imported `let X: [N]T` and emit `LEAQ X(SB)` for the + * base. Add a `let_var_type(name)` lookup (LetVar gains a `type` + * field) so cgindex's N_INDEX case can fall back to the imported + * let's type when `n->lhs->type` is NULL (SK_USE-bound ident). + * - wwstage: extend `dotbaseaddr` parallel — when the inner ident + * isn't a local (localfindnode → nil) but `letvartnode(base.str)` + * resolves to N_TARRAY, emit `LEAQ symname(SB)`. + * + * Rule-10 alignment: both stages share the same cgindex N_INDEX + * fallback shape; the helper is the use-site fix per #135's pattern, + * preserving cgdot's current MOVQ semantics so whole-array-assign + * shapes (defensive case; not exercised in committed code per + * design-pass audit) aren't disturbed. + * + * Each row carries (a) cstage `ww build` + run asserting the exit + * code (the cstage-segfault repro now exits cleanly) and (b) a w6c vs + * w6c_ww `.s` cmp (rule-10 byte-id). Imports `strconv` because its + * `let left_shift_table: [65]u16` is the canonical reviewer-stofdata + * probe surface for this bug. + */ +#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[] = { + /* The reviewer-stofdata probe: indexed read on an imported [N]u16 + * at idx=0. Pre-fix cstage segfaulted on nontrivial indices and + * MOVZBQ-loaded the wrong byte; here at idx=0 the wrong-stride + * doesn't matter, so the test exit reflects the load-width fix: + * MOVZWQ now reads the full u16 = 0x0000 → exit 0. */ + { "mod_u16_idx0", + "package main;\n" + "import strconv;\n" + "export fn main() i32 = {\n" + " let v: u32 = (strconv.left_shift_table[0]: u32);\n" + " return v: i32;\n" + "};\n", 0 }, + /* Nontrivial idx: pre-fix segfault repro. table[2] = 0x0801; + * 0x0801 mod 256 = 1. */ + { "mod_u16_idx2", + "package main;\n" + "import strconv;\n" + "export fn main() i32 = {\n" + " let v: u32 = (strconv.left_shift_table[2]: u32);\n" + " return v: i32;\n" + "};\n", 1 }, + /* Further idx — pins per-element stride. table[4] = 0x1006; + * 0x1006 mod 256 = 6. */ + { "mod_u16_idx4", + "package main;\n" + "import strconv;\n" + "export fn main() i32 = {\n" + " let v: u32 = (strconv.left_shift_table[4]: u32);\n" + " return v: i32;\n" + "};\n", 6 }, + /* Local-array control: confirms the existing IDENT-base path is + * unchanged by the #128b cgindex edit. Pre-fix already worked; the + * fix is gated by `bt == NULL` so this row's byte-id is + * preserved. */ + { "local_arr_control", + "package main;\n" + "export fn main() i32 = {\n" + " let a: [4]u16 = [10u16, 20u16, 30u16, 40u16];\n" + " let v: u32 = (a[2]: u32);\n" + " return v: i32;\n" + "};\n", 30 }, + /* Same-package access (NOT module-qualified) — pins that the + * existing in-package IDENT path stays unchanged. left_shift_table + * exists in strconv; access from within `package strconv` would be + * IDENT-base. Wrap in a tiny helper here to keep test self- + * contained; uses local table. */ + { "local_u16_idx_nonzero", + "package main;\n" + "export fn main() i32 = {\n" + " let t: [8]u16 = [0u16, 0x0800u16, 0x0801u16, 0x0803u16, " + "0x1006u16, 0x1009u16, 0x100Du16, 0x1812u16];\n" + " let v: u32 = (t[2]: u32);\n" + " return v: i32;\n" + "};\n", 1 }, + { 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, "modarridx: 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/wwmoda_%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/wwmoda_%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); + + /* Byte-id only meaningful when the source compiles standalone + * to a .s. Module-imported rows need the combined.ww shape + * (which `ww build` produces inside tmpdir); we already ran + * that above. For the byte-id leg, point at the combined.ww. */ + char combined[160]; + snprintf(combined, sizeof combined, "%s/%s.combined.ww", + tmpdir, base); + (void)combined; + /* Both stages emit on the original .ww directly; the + * combined.ww has the same surface for cgen purposes. We use + * the original src for byte-id since w6c/w6c_ww handle the + * import resolution when invoked on the file with -I and the + * lib search. The probe row asserts module-qualified shape + * via direct w6c{,_ww} on the .ww; if the stage can't find + * the import, the build above would have failed first. */ + char cs_s[64], ws_s[64]; + snprintf(cs_s, sizeof cs_s, "/tmp/wwmoda_%d_%d_cs.s", + getpid(), i); + snprintf(ws_s, sizeof ws_s, "/tmp/wwmoda_%d_%d_ww.s", + getpid(), i); + + /* Skip byte-id leg for rows that need imports — w6c/w6c_ww + * standalone won't resolve `import strconv;`. The runtime + * leg above (via `ww build`) IS the byte-id+correctness + * gate for these rows; the full bootstrap byte-id (994) is + * the corpus check. */ + int needs_import = (strstr(rows[i].src, "import ") != NULL); + if (!needs_import) { + 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(cs_s); unlink(ws_s); + } + unlink(src); + } + + if (fail) { + fprintf(stderr, "%d/%d mod-arr-idx tests failed\n", fail, n); + return 1; + } + printf("modarridx: %d/%d ok (cstage run + cs==ww byte-id)\n", + n, n); + return 0; +}