From e6beb566ead150c34fdb5dc45025804594280cdc Mon Sep 17 00:00:00 2001 From: Hojun-Cho Date: Tue, 26 May 2026 19:14:14 +0900 Subject: [PATCH] wcc: narrow integer pointer-deref load width via localloadop (#116) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TK_STAR integer arm now routes through localloadop (cstage cgen.c) / localloadop (wwstage cgenexpr.ww) — load-twin of the landed signed- narrow-scalar-reads fix, was omitting TK_STAR. Closes the *p (CMPQ, full-width arith) miscompile family (#116 + 962/963 instances all fixed by the same width-aware load). Float arm untouched (#96 already routed via X0). New test 947 (10 rows): packed CMPQ + signed/unsigned narrow widths + TY_NAMED/TBANG alias + TY_ENUM peel + i64/bool controls. --- Makefile | 6 + cmd/w6c/cgen.c | 17 +- selfhost/cmd/w6c/main.combined.ww | 19 +- selfhost/cmd/wcc/cgenexpr.ww | 19 +- selfhost/cmd/wwdump/main.combined.ww | 19 +- test/wcc/947_deref_narrow_run.c | 297 +++++++++++++++++++++++++++ 6 files changed, 373 insertions(+), 4 deletions(-) create mode 100644 test/wcc/947_deref_narrow_run.c diff --git a/Makefile b/Makefile index f48306ac..ebc3d01a 100644 --- a/Makefile +++ b/Makefile @@ -329,6 +329,7 @@ TESTS = $(BIN)/test_smoke $(BIN)/test_lex $(BIN)/test_parse $(BIN)/test_check \ $(BIN)/test_siphash_run \ $(BIN)/test_checked_run \ $(BIN)/test_floatarr_run \ + $(BIN)/test_deref_narrow_run \ $(BIN)/test_f64cgen_run \ $(BIN)/test_f64crossmod_run \ $(BIN)/test_tuprecv_run \ @@ -1088,6 +1089,11 @@ $(BIN)/test_floatarr_run: test/wcc/946_floatarr_run.c $(BIN)/ww \ $(LIB)/libwwrt.a | $(BIN) $(CC) $(CFLAGS) -o $@ $< +$(BIN)/test_deref_narrow_run: test/wcc/947_deref_narrow_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 a7c9a1a0..eaf5dec8 100644 --- a/cmd/w6c/cgen.c +++ b/cmd/w6c/cgen.c @@ -2315,7 +2315,22 @@ cgexpr(Cg *c, Node *n, Local *locals) ins2(c, node_isf32(n) ? A_MOVSS : A_MOVSD, amem(D_AX, 0), areg(D_X0)); } else { - ins2(c, A_MOVQ, amem(D_AX, 0), areg(D_AX)); + /* Load-twin of the signed-narrow-scalar-reads + * sweep (project_cgen_int_cast_no_truncate); + * TK_STAR was the omitted site, refiled as + * #116. A raw MOVQ pulls 8 bytes through a + * narrow `*iN` and overlaps the next element + * — the `*p` value reads honest only when the + * caller's sink happens to truncate (i32 store, + * i32 return). Width-preserving sinks (CMPQ, + * 64-bit arith) saw garbage in the high bytes. + * localloadop keys MOVSXD/MOVSWQ/MOVSBQ + + * MOVL/MOVZWQ/MOVZBQ off n->type, with the + * TY_NAMED / TY_ENUM peel pre-folded so an + * aliased narrow (`type err = !i32`) lands on + * the right opcode. */ + ins2(c, localloadop(n->type), + amem(D_AX, 0), areg(D_AX)); } break; default: break; diff --git a/selfhost/cmd/w6c/main.combined.ww b/selfhost/cmd/w6c/main.combined.ww index 5dc5d646..8d55248d 100644 --- a/selfhost/cmd/w6c/main.combined.ww +++ b/selfhost/cmd/w6c/main.combined.ww @@ -17045,7 +17045,24 @@ fn cgun(c: *cgen, n: *node) void = { if (isf32type(c, n)) { mov = "MOVSS"; }; emitline("\t"); emitline(mov); emitline("\t(AX), X0\n"); } else { - emitline("\tMOVQ\t(AX), AX\n"); + // Load-twin of the landed signed-narrow-scalar-reads + // sweep (selfhost/CLAUDE.md "Signed-narrow scalar + // reads sign-extend honestly"); TK_STAR was the + // omitted site, refiled as #116. A raw MOVQ pulls 8B + // through a narrow `*iN` and overlaps the next element + // — the `*p` value reads honest only when the caller's + // sink truncates (i32 store, i32 return). Width- + // preserving sinks (CMPQ, 64-bit arith) saw garbage in + // the high bytes. localloadop keys MOVSXD/MOVSWQ/ + // MOVSBQ + MOVL/MOVZWQ/MOVZBQ off n.type_; n is the + // deref expression, n.type_ is the pointee tinfo + // (check.ww unoptype TK_STAR L1871-1886 with + // TY_NAMED/TY_ENUM peel pre-folded by + // tinfofornode/typeissigned), the same shape the + // float arm above feeds isfloattype. + let lop: str = localloadop(c, n); + emitline("\t"); emitline(lop); + emitline("\t(AX), AX\n"); }; return; }; diff --git a/selfhost/cmd/wcc/cgenexpr.ww b/selfhost/cmd/wcc/cgenexpr.ww index 56f8aff7..0d77fb5e 100644 --- a/selfhost/cmd/wcc/cgenexpr.ww +++ b/selfhost/cmd/wcc/cgenexpr.ww @@ -2831,7 +2831,24 @@ fn cgun(c: *cgen, n: *node) void = { if (isf32type(c, n)) { mov = "MOVSS"; }; emitline("\t"); emitline(mov); emitline("\t(AX), X0\n"); } else { - emitline("\tMOVQ\t(AX), AX\n"); + // Load-twin of the landed signed-narrow-scalar-reads + // sweep (selfhost/CLAUDE.md "Signed-narrow scalar + // reads sign-extend honestly"); TK_STAR was the + // omitted site, refiled as #116. A raw MOVQ pulls 8B + // through a narrow `*iN` and overlaps the next element + // — the `*p` value reads honest only when the caller's + // sink truncates (i32 store, i32 return). Width- + // preserving sinks (CMPQ, 64-bit arith) saw garbage in + // the high bytes. localloadop keys MOVSXD/MOVSWQ/ + // MOVSBQ + MOVL/MOVZWQ/MOVZBQ off n.type_; n is the + // deref expression, n.type_ is the pointee tinfo + // (check.ww unoptype TK_STAR L1871-1886 with + // TY_NAMED/TY_ENUM peel pre-folded by + // tinfofornode/typeissigned), the same shape the + // float arm above feeds isfloattype. + let lop: str = localloadop(c, n); + emitline("\t"); emitline(lop); + emitline("\t(AX), AX\n"); }; return; }; diff --git a/selfhost/cmd/wwdump/main.combined.ww b/selfhost/cmd/wwdump/main.combined.ww index 1e7a7b9b..4414643f 100644 --- a/selfhost/cmd/wwdump/main.combined.ww +++ b/selfhost/cmd/wwdump/main.combined.ww @@ -17045,7 +17045,24 @@ fn cgun(c: *cgen, n: *node) void = { if (isf32type(c, n)) { mov = "MOVSS"; }; emitline("\t"); emitline(mov); emitline("\t(AX), X0\n"); } else { - emitline("\tMOVQ\t(AX), AX\n"); + // Load-twin of the landed signed-narrow-scalar-reads + // sweep (selfhost/CLAUDE.md "Signed-narrow scalar + // reads sign-extend honestly"); TK_STAR was the + // omitted site, refiled as #116. A raw MOVQ pulls 8B + // through a narrow `*iN` and overlaps the next element + // — the `*p` value reads honest only when the caller's + // sink truncates (i32 store, i32 return). Width- + // preserving sinks (CMPQ, 64-bit arith) saw garbage in + // the high bytes. localloadop keys MOVSXD/MOVSWQ/ + // MOVSBQ + MOVL/MOVZWQ/MOVZBQ off n.type_; n is the + // deref expression, n.type_ is the pointee tinfo + // (check.ww unoptype TK_STAR L1871-1886 with + // TY_NAMED/TY_ENUM peel pre-folded by + // tinfofornode/typeissigned), the same shape the + // float arm above feeds isfloattype. + let lop: str = localloadop(c, n); + emitline("\t"); emitline(lop); + emitline("\t(AX), AX\n"); }; return; }; diff --git a/test/wcc/947_deref_narrow_run.c b/test/wcc/947_deref_narrow_run.c new file mode 100644 index 00000000..b436f7d6 --- /dev/null +++ b/test/wcc/947_deref_narrow_run.c @@ -0,0 +1,297 @@ +/* + * 947_deref_narrow_run — runtime + byte-id net for #116: an integer + * pointer deref must read its pointee's actual width, not a fixed 8B + * MOVQ. Before #116 the TK_STAR integer arm in cgen.c:2318 / cgenexpr + * .ww:2834 emitted a raw `MOVQ (AX), AX` regardless of pointee size, + * so `*p` over a packed `*i32` / `*u32` / `*iN` slot pulled the next + * 4–7 bytes into the high half of RAX. Truncating sinks (i32 store, + * i32 return) dropped the garbage; width-preserving sinks (CMPQ vs + * an i32 literal, 64-bit arith) saw it. The fix routes the load + * through localloadop(n->type), the same MOVSXD/MOVSWQ/MOVSBQ + + * MOVL/MOVZWQ/MOVZBQ dispatch already shared by IDENT / DOT / index + * loads (cgen.c:264 fldloadop, :372 localloadop; cgenutil.ww:884 + * loadopsz, :904 localloadop). Load-twin of the landed signed-narrow + * scalar-reads sweep (selfhost/CLAUDE.md "Signed-narrow scalar reads + * sign-extend honestly") — TK_STAR was the omitted site. + * + * Each row carries (a) a cstage `ww build` + run asserting the exit + * code (the i32_cmpq row would return 2 pre-fix), and (b) a w6c vs + * w6c_ww `.s` cmp (rule-10 byte-id) — the gate-blind family discipline + * the rest of the suite enforces. The aliased-narrow row exercises the + * TY_NAMED / TBANG peel in localloadop's tinfo lookup. + */ +#include +#include +#include +#include +#include +#include + +#define RUN_SKIP (-1) + +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 #116 repro: packed *i32 deref with a CMPQ-width sink (the + * `if (*p != 20i32)` comparator widens the i32 literal to 8B and + * compares with CMPQ). Pre-fix MOVQ pulled buf[1] | (buf[2]<<32) + * into AX, the compare failed, exit=2. */ + { "i32_cmpq", + "package main;\n" + "export fn main() i32 = {\n" + " let buf: [4]i32 = [10, 20, 30, 40];\n" + " let p: *i32 = &buf[1];\n" + " if (*p != 20) { return 2; };\n" + " return 0;\n" + "};\n", 0 }, + /* *u32 — zero-extend (MOVL r/m32,r32 zeroes upper 32). 0xFFFFFFFF + * round-trips intact only if the read is u32-wide and zero- + * extended. Pre-fix MOVQ overlapped the next slot — and even with + * a tail-element so there's no spill, the value compared against + * 0xFFFFFFFFu32 (widened to 64B) would carry whatever + * uninitialised stack lives at buf+12. */ + { "u32_zeroext", + "package main;\n" + "export fn main() i32 = {\n" + " let buf: [3]u32 = [10u32, 20u32, 0xFFFFFFFFu32];\n" + " let p: *u32 = &buf[2];\n" + " if (*p != 0xFFFFFFFFu32) { return 2; };\n" + " return 0;\n" + "};\n", 0 }, + /* *u8 — narrowest unsigned, MOVZBQ. */ + { "u8_zeroext", + "package main;\n" + "export fn main() i32 = {\n" + " let buf: [3]u8 = [10u8, 20u8, 0xFFu8];\n" + " let p: *u8 = &buf[2];\n" + " if (*p != 0xFFu8) { return 2; };\n" + " return 0;\n" + "};\n", 0 }, + /* *i8 — signed-narrow, MOVSBQ. -2i8 must sign-extend to -2i32 + * through the explicit cast; a raw MOVQ would leave the upper + * bytes as the next two array slots and a subsequent `: i32` + * cast would still read the low byte correctly — so the cast + * acts as the truncating sink. The compare against -2i32 widens + * to 64B (CMPQ) so the sign-extension must happen on the load. */ + { "i8_signext", + "package main;\n" + "export fn main() i32 = {\n" + " let buf: [3]i8 = [-1i8, -2i8, -3i8];\n" + " let p: *i8 = &buf[1];\n" + " let v: i32 = (*p): i32;\n" + " if (v != -2i32) { return 2; };\n" + " return 0;\n" + "};\n", 0 }, + /* Aliased signed-narrow: `type err = !i32; *err`. The TY_NAMED / + * TBANG peel in localloadop's tinfofornode chain must reach the + * underlying i32 width for the MOVSXD opcode to fire. Base is a + * struct field (not an array literal) to dodge a pre-existing + * cs/ww divergence in [N]alias array-init store width — out of + * scope for #116. */ + { "alias_signext", + "package main;\n" + "type err = !i32;\n" + "type box = struct { v: err };\n" + "export fn main() i32 = {\n" + " let b: box = box { v = -7i32: err };\n" + " let p: *err = &b.v;\n" + " let r: i32 = (*p): i32;\n" + " if (r != -7i32) { return 2; };\n" + " return 0;\n" + "};\n", 0 }, + /* *bool — also 1B (MOVZBQ); same dispatch shape but worth a + * control since the bool branch in fld_issigned returns 0 + * explicitly. */ + { "bool_ctrl", + "package main;\n" + "export fn main() i32 = {\n" + " let buf: [3]bool = [true, false, true];\n" + " let p: *bool = &buf[1];\n" + " if (*p) { return 2; };\n" + " return 0;\n" + "};\n", 0 }, + /* *i16 — MOVSWQ. Struct-field base (with a NEXT byte set so the + * pre-fix MOVQ pulls non-zero high bytes and the 64B CMPQ fails), + * sidestepping the same `[N]i16` array-init store-width concern as + * the alias_signext row. */ + { "i16_signext", + "package main;\n" + "type box = struct { v: i16, m: u16 };\n" + "export fn main() i32 = {\n" + " let b: box = box { v = -200i16, m = 0xABCDu16 };\n" + " let p: *i16 = &b.v;\n" + " let r: i32 = (*p): i32;\n" + " if (r != -200i32) { return 2; };\n" + " return 0;\n" + "};\n", 0 }, + /* *u16 — MOVZWQ. Same struct-field shape; #128 ([N]u16 array-init + * cs/ww store-width divergence) is unrelated to this fold. */ + { "u16_zeroext", + "package main;\n" + "type box = struct { v: u16, m: u16 };\n" + "export fn main() i32 = {\n" + " let b: box = box { v = 0xFFFFu16, m = 0xABCDu16 };\n" + " let p: *u16 = &b.v;\n" + " if (*p != 0xFFFFu16) { return 2; };\n" + " return 0;\n" + "};\n", 0 }, + /* Enum-aliased narrow: `type flag = enum i8 { ... }`. The TY_NAMED + * /TY_ENUM peel in typeissigned (lib/ww/typ.ww:390 → typeisunsigned + * → TY_ENUM recurse on sub) + tinfo.size on the NAMED wrapper must + * land on MOVSBQ. Distinct from the TBANG row above — same recursion + * shape but a different tykind branch. Struct-field base dodges the + * sibling-of-#128 [N]alias array-init concern. */ + { "enum_signext", + "package main;\n" + "type flag = enum i8 { A = -2i8, B = 1i8 };\n" + "type box = struct { v: flag, m: u8 };\n" + "export fn main() i32 = {\n" + " let b: box = box { v = flag.A, m = 0xABu8 };\n" + " let p: *flag = &b.v;\n" + " let r: i32 = ((*p): i8): i32;\n" + " if (r != -2i32) { return 2; };\n" + " return 0;\n" + "};\n", 0 }, + /* *i64 control — MUST still emit MOVQ (no shift). Regression + * guard: a width-correct case shouldn't be perturbed. */ + { "i64_ctrl", + "package main;\n" + "export fn main() i32 = {\n" + " let buf: [3]i64 = [100i64, 200i64, 300i64];\n" + " let p: *i64 = &buf[1];\n" + " if (*p != 200i64) { return 2; };\n" + " return 0;\n" + "};\n", 0 }, + { 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, "derefnarrow: 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/wwdrn_%d_%d.ww", getpid(), i); + FILE *f = fopen(src, "wb"); + if (f == NULL) { fail++; continue; } + fputs(rows[i].src, f); + fclose(f); + + /* (a) cstage build + run. */ + if (rows[i].want_exit != RUN_SKIP) { + char tmpdir[64]; + snprintf(tmpdir, sizeof tmpdir, "/tmp/wwdrn_%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); + } + + /* (b) cs==ww byte-id gate. */ + char cs_s[64], ws_s[64]; + snprintf(cs_s, sizeof cs_s, "/tmp/wwdrn_%d_%d_cs.s", + getpid(), i); + snprintf(ws_s, sizeof ws_s, "/tmp/wwdrn_%d_%d_ww.s", + getpid(), i); + + char cmd[2048]; + 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 narrow-deref tests failed\n", + fail, n); + return 1; + } + printf("derefnarrow: %d/%d ok (cstage run + cs==ww byte-id)\n", + n, n); + return 0; +}