From 2a2ac49c64df933acbfba2faf67151eb469f533f Mon Sep 17 00:00:00 2001 From: Hojun-Cho Date: Fri, 12 Jun 2026 22:52:20 +0900 Subject: [PATCH] wcc: for-range destructure copies the full str/slice binding, both stages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The per-binding copy loop moved ONE word of a 24B str/slice binding — .len and .cap read zero/garbage in BOTH stages (byte-identical, the deepest both-wrong-identical of the drain: the F7-era stride fix asserted convergence without re-measuring the absolute). Copy the full extent for an sz>8 str/slice binding; the rewritten 989_tupfieldsize pins all three header words with sliced caps so cap!=len has teeth. The tagged-binding arm remains open as task #53 (wwstage paramfieldsize). Review-era task #40, recategorized #263 fused. Both stages move in one commit: one emission contract; splitting would leave the byte-id gates red between the halves. --- cmd/w6c/cgen.c | 44 ++++++++++++++ selfhost/cmd/w6c/main.combined.ww | 48 +++++++++++++++ selfhost/cmd/wcc/cgenstmt.ww | 48 +++++++++++++++ selfhost/cmd/wwdump/main.combined.ww | 48 +++++++++++++++ test/wcc/989_tupfieldsize_run.c | 89 ++++++++++++++++++++-------- 5 files changed, 252 insertions(+), 25 deletions(-) diff --git a/cmd/w6c/cgen.c b/cmd/w6c/cgen.c index c44d7cc2..457ac0a6 100644 --- a/cmd/w6c/cgen.c +++ b/cmd/w6c/cgen.c @@ -14344,6 +14344,50 @@ cgstmt(Cg *c, Node *n, Local **locals, int *frame) } } else { for (int b = 0; b < nbinds; b++) { + /* #40 (#263): a str/slice/struct destructure + * binding (24B header / aggregate, sz>8) copies + * its FULL extent — the single fldloadop word + * truncated a slice binding to its .ptr, dropping + * .len/.cap (both stages identically, byte-id- + * WRONG; F7-c4 fixed only the STRIDE). Same + * word-run + sized-tail idiom as the non- + * destructure aggregate copy above. */ + int bsz = binds[b].sz; + if (bsz > 8) { + int k = 0; + for (; k + 8 <= bsz; k += 8) { + ins2(c, A_MOVQ, + amem(D_BX, binds[b].foff + k), + areg(D_AX)); + ins2(c, A_MOVQ, areg(D_AX), + amem(D_BP, binds[b].off + k)); + } + if (k + 4 <= bsz) { + ins2(c, A_MOVL, + amem(D_BX, binds[b].foff + k), + areg(D_AX)); + ins2(c, A_MOVL, areg(D_AX), + amem(D_BP, binds[b].off + k)); + k += 4; + } + if (k + 2 <= bsz) { + ins2(c, A_MOVW, + amem(D_BX, binds[b].foff + k), + areg(D_AX)); + ins2(c, A_MOVW, areg(D_AX), + amem(D_BP, binds[b].off + k)); + k += 2; + } + if (k + 1 <= bsz) { + ins2(c, A_MOVB, + amem(D_BX, binds[b].foff + k), + areg(D_AX)); + ins2(c, A_MOVB, areg(D_AX), + amem(D_BP, binds[b].off + k)); + k += 1; + } + continue; + } int op = fldloadop(binds[b].ftype, binds[b].sz); ins2(c, op, amem(D_BX, binds[b].foff), areg(D_AX)); diff --git a/selfhost/cmd/w6c/main.combined.ww b/selfhost/cmd/w6c/main.combined.ww index dd8658fd..16386d44 100644 --- a/selfhost/cmd/w6c/main.combined.ww +++ b/selfhost/cmd/w6c/main.combined.ww @@ -39696,6 +39696,54 @@ fn cgforrange(c: *cgen, n: *node) void = { } else { let b: i32 = 0; for (b < nbinds) { + // #40 (#263): a str/slice/struct destructure binding + // (24B header / aggregate, sz>8) copies its FULL extent + // — the single load word truncated a slice binding to + // its .ptr, dropping .len/.cap (both stages identically, + // byte-id-WRONG; F7-c4 fixed only the STRIDE). Same + // word-run + sized-tail idiom as the non-destructure + // aggregate copy above. + if (bind_sz[b] > 8) { + let k: i32 = 0; + for (k + 8 <= bind_sz[b]) { + emitline("\tMOVQ\t"); + emitoff((bind_foff[b] + k): i64); + emitline("(BX), AX\n"); + emitline("\tMOVQ\tAX, "); + emitoff((bind_off[b] + k): i64); + emitline("(BP)\n"); + k += 8; + }; + if (k + 4 <= bind_sz[b]) { + emitline("\tMOVL\t"); + emitoff((bind_foff[b] + k): i64); + emitline("(BX), AX\n"); + emitline("\tMOVL\tAX, "); + emitoff((bind_off[b] + k): i64); + emitline("(BP)\n"); + k += 4; + }; + if (k + 2 <= bind_sz[b]) { + emitline("\tMOVW\t"); + emitoff((bind_foff[b] + k): i64); + emitline("(BX), AX\n"); + emitline("\tMOVW\tAX, "); + emitoff((bind_off[b] + k): i64); + emitline("(BP)\n"); + k += 2; + }; + if (k + 1 <= bind_sz[b]) { + emitline("\tMOVB\t"); + emitoff((bind_foff[b] + k): i64); + emitline("(BX), AX\n"); + emitline("\tMOVB\tAX, "); + emitoff((bind_off[b] + k): i64); + emitline("(BP)\n"); + k += 1; + }; + b += 1; + continue; + }; let op: str = loadopsz(bind_signed[b], bind_sz[b]); emitline("\t"); emitline(op); diff --git a/selfhost/cmd/wcc/cgenstmt.ww b/selfhost/cmd/wcc/cgenstmt.ww index 3bb50c28..f2d17917 100644 --- a/selfhost/cmd/wcc/cgenstmt.ww +++ b/selfhost/cmd/wcc/cgenstmt.ww @@ -4144,6 +4144,54 @@ fn cgforrange(c: *cgen, n: *node) void = { } else { let b: i32 = 0; for (b < nbinds) { + // #40 (#263): a str/slice/struct destructure binding + // (24B header / aggregate, sz>8) copies its FULL extent + // — the single load word truncated a slice binding to + // its .ptr, dropping .len/.cap (both stages identically, + // byte-id-WRONG; F7-c4 fixed only the STRIDE). Same + // word-run + sized-tail idiom as the non-destructure + // aggregate copy above. + if (bind_sz[b] > 8) { + let k: i32 = 0; + for (k + 8 <= bind_sz[b]) { + emitline("\tMOVQ\t"); + emitoff((bind_foff[b] + k): i64); + emitline("(BX), AX\n"); + emitline("\tMOVQ\tAX, "); + emitoff((bind_off[b] + k): i64); + emitline("(BP)\n"); + k += 8; + }; + if (k + 4 <= bind_sz[b]) { + emitline("\tMOVL\t"); + emitoff((bind_foff[b] + k): i64); + emitline("(BX), AX\n"); + emitline("\tMOVL\tAX, "); + emitoff((bind_off[b] + k): i64); + emitline("(BP)\n"); + k += 4; + }; + if (k + 2 <= bind_sz[b]) { + emitline("\tMOVW\t"); + emitoff((bind_foff[b] + k): i64); + emitline("(BX), AX\n"); + emitline("\tMOVW\tAX, "); + emitoff((bind_off[b] + k): i64); + emitline("(BP)\n"); + k += 2; + }; + if (k + 1 <= bind_sz[b]) { + emitline("\tMOVB\t"); + emitoff((bind_foff[b] + k): i64); + emitline("(BX), AX\n"); + emitline("\tMOVB\tAX, "); + emitoff((bind_off[b] + k): i64); + emitline("(BP)\n"); + k += 1; + }; + b += 1; + continue; + }; let op: str = loadopsz(bind_signed[b], bind_sz[b]); emitline("\t"); emitline(op); diff --git a/selfhost/cmd/wwdump/main.combined.ww b/selfhost/cmd/wwdump/main.combined.ww index 08785573..aec5a1c4 100644 --- a/selfhost/cmd/wwdump/main.combined.ww +++ b/selfhost/cmd/wwdump/main.combined.ww @@ -39696,6 +39696,54 @@ fn cgforrange(c: *cgen, n: *node) void = { } else { let b: i32 = 0; for (b < nbinds) { + // #40 (#263): a str/slice/struct destructure binding + // (24B header / aggregate, sz>8) copies its FULL extent + // — the single load word truncated a slice binding to + // its .ptr, dropping .len/.cap (both stages identically, + // byte-id-WRONG; F7-c4 fixed only the STRIDE). Same + // word-run + sized-tail idiom as the non-destructure + // aggregate copy above. + if (bind_sz[b] > 8) { + let k: i32 = 0; + for (k + 8 <= bind_sz[b]) { + emitline("\tMOVQ\t"); + emitoff((bind_foff[b] + k): i64); + emitline("(BX), AX\n"); + emitline("\tMOVQ\tAX, "); + emitoff((bind_off[b] + k): i64); + emitline("(BP)\n"); + k += 8; + }; + if (k + 4 <= bind_sz[b]) { + emitline("\tMOVL\t"); + emitoff((bind_foff[b] + k): i64); + emitline("(BX), AX\n"); + emitline("\tMOVL\tAX, "); + emitoff((bind_off[b] + k): i64); + emitline("(BP)\n"); + k += 4; + }; + if (k + 2 <= bind_sz[b]) { + emitline("\tMOVW\t"); + emitoff((bind_foff[b] + k): i64); + emitline("(BX), AX\n"); + emitline("\tMOVW\tAX, "); + emitoff((bind_off[b] + k): i64); + emitline("(BP)\n"); + k += 2; + }; + if (k + 1 <= bind_sz[b]) { + emitline("\tMOVB\t"); + emitoff((bind_foff[b] + k): i64); + emitline("(BX), AX\n"); + emitline("\tMOVB\tAX, "); + emitoff((bind_off[b] + k): i64); + emitline("(BP)\n"); + k += 1; + }; + b += 1; + continue; + }; let op: str = loadopsz(bind_signed[b], bind_sz[b]); emitline("\t"); emitline(op); diff --git a/test/wcc/989_tupfieldsize_run.c b/test/wcc/989_tupfieldsize_run.c index e053feba..ea7ce091 100644 --- a/test/wcc/989_tupfieldsize_run.c +++ b/test/wcc/989_tupfieldsize_run.c @@ -17,26 +17,32 @@ * slice-header SSoT, rule-13) and the N_TTUPLE arm (sum of 8B-floored * element slots, recursive), aligning wwstage UP. * - * NB on the assertion: the slice-field row is pinned cs==ww (MATCH-ONLY), - * NOT to an absolute value. cstage independently mis-loads a single-word - * tuple-destructure element (a SEPARATE bug ken flagged out of F7 scope, - * filed as task #40 — cstage's absolute value is wrong: 40 != 45), so - * both stages currently land 40, not the arithmetic-expected 45. The - * #43 fix's job is to remove the cs≠ww STRIDE/OFFSET divergence (40 vs 16 - * → 40 vs 40); pinning cs==ww tracks exactly that and won't false-fail - * when the separate destructure-load bug is later fixed (both move - * together). The scalar-tuple control IS correct on both stages, so it - * also pins the absolute value (no-regression teeth). + * #40 (#263, now CLOSED both stages — fused): F7-c4 fixed only the STRIDE, + * so the slice-field row was pinned cs==ww (MATCH-ONLY) at 40, NOT the + * arithmetic-expected 45 — because the destructure LOAD-into-binding copied + * only the PTR word of the 24B slice binding, dropping .len/.cap (both stages + * identically; nobody had re-measured the ww absolute after F7-c4). The fused + * #40 fix (cgen.c N_FORRANGE destructure + cgenstmt.ww cgforrange twin: copy + * the binding's FULL extent for an aggregate sz>8 binding, same word-run + + * sized-tail idiom as the non-destructure copy) lands all three header words. + * The rows now pin the ABSOLUTE value cs==ww across all three words — the + * recurring dropped word in this 24B-aggregate-destructure family is the CAP + * (cf #29 chained-dot, #43 sizer), so the cap row reads b.cap with cap!=len + * (sliced) to teeth it; a ptr+len-only set would pass green while cap stayed + * stale (exactly how #40 hid). * - * Tuples are built by whole-tuple element store (`xs[i] = (..)`); the - * `xs[i].0 = ..` field-store target and the `[[..]]` nested literal are - * both independently unsupported / #270-1c-blocked (orthogonal). + * Tuples are built by whole-tuple element store (`xs[i] = (..)`). * - * Rows (cstage `ww` always; wwstage `ww_ww` when present; rule-10): - * row | shape | assert - * -------------------+--------------------------------+---------------- - * slice_field_tuple | for(.. [2]([]u8,i64)) | cs==ww [#43 bug] - * scalar_tuple_ctl | for(.. [2](i64,i64)) = 48 | cs==ww AND == 48 + * Rows (cstage `ww` + wwstage `ww_ww`; rule-10; all pin the absolute value): + * row | reads | exit (cs==ww) + * ------------------+------------------------------------+-------------- + * len_read | len(b)+n over [2]([]u8,i64) | 45 (2+10+3+30) + * cap_read | b.cap+n, sliced caps 4/5 (cap!=len) | 49 (4+10+5+30) + * ptr_read | b[0]+ (byte through ptr) | 217 (97+120) + * scalar_tuple_ctl | for(.. [2](i64,i64)) | 48 (control) + * Pre-fix (single-word copy): len_read=40, cap_read=40 (len/cap words both 0), + * ptr_read=217 (word0 always copied — coverage, no teeth); cs==ww==wrong (the + * #263 both-wrong-identical residual the fused fix retires). */ #include #include @@ -61,9 +67,9 @@ struct row { }; static const struct row rows[] = { - /* (1) #43 — a []u8 tuple-field sized 8 not 24 → wrong stride/offset. - * cs==ww only (cstage's 40≠45 is the separate destructure-load bug). */ - { "slice_field_tuple", + /* (1) len_read (#40 teeth, word 1): the slice binding's .len must be + * copied. Pre-fix the single-word copy dropped it → len(b)=0 → 40. */ + { "len_read", "package main;\n" "export fn main() int = {\n" " let b0: []u8 = ['a', 'b'];\n" @@ -77,11 +83,44 @@ static const struct row rows[] = { " };\n" " return sum: int;\n" "};\n", - -1 }, + 45 }, - /* (2) control — scalar-only tuple (no slice/str field): paramfieldsize - * already handled it, so c4 must not regress it. Correct on both stages: - * 3+10+5+30 == 48. Pins cs==ww AND the absolute value. */ + /* (2) cap_read (#40 teeth, word 2 — THE recurring dropped word): caps + * 4/5 are SLICED so cap != len (2/3); pre-fix b.cap=0 → 40. */ + { "cap_read", + "package main;\n" + "export fn main() int = {\n" + " let base0: []u8 = ['a', 'b', 'c', 'd'];\n" + " let base1: []u8 = ['p', 'q', 'r', 's', 't'];\n" + " let xs: [2]([]u8, i64);\n" + " xs[0] = (base0[0:2], 10);\n" + " xs[1] = (base1[0:3], 30);\n" + " let sum: i64 = 0;\n" + " for (let (b, n) .. xs) {\n" + " sum = sum + b.cap: i64 + n;\n" + " };\n" + " return sum: int;\n" + "};\n", + 49 }, + + /* (3) ptr_read (word 0 — coverage; word0 was always copied, no teeth): + * a byte read through the binding's .ptr. b0[0]='a'=97, b1[0]='x'=120. */ + { "ptr_read", + "package main;\n" + "export fn main() int = {\n" + " let b0: []u8 = ['a', 'b'];\n" + " let b1: []u8 = ['x', 'y', 'z'];\n" + " let xs: [2]([]u8, i64);\n" + " xs[0] = (b0, 10);\n" + " xs[1] = (b1, 30);\n" + " let sum: i64 = 0;\n" + " for (let (b, n) .. xs) { sum = sum + b[0]: i64; };\n" + " return sum: int;\n" + "};\n", + 217 }, + + /* (4) control — scalar-only tuple (no aggregate field): paramfieldsize + * already handled it; the sz>8 arm must not touch it. 3+10+5+30 == 48. */ { "scalar_tuple_ctl", "package main;\n" "export fn main() int = {\n"