Hojun-Cho/ww
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

cgen: store a 2-byte struct-literal field with MOVW, not an over-wide MOVQ (#15)

Browse Source 
cg_structlit_fill/cgstructlitfill dispatched scalar field stores as {1->MOVB, 4->MOVL, else->MOVQ} with no fsz==2 case, so a 2-byte field was stored with an 8-byte MOVQ. Interior over-stores were harmlessly overwritten by the next field, but the LAST field at the frame edge corrupted the saved base pointer: an (S|e) union success variant places the struct payload after the 8B tag, landing the last field at -4(BP), so MOVQ AX,-4(BP) wrote into saved [BP] and POPQ BP restored garbage — a silent both-stage caller-frame clobber. Route the scalar store through the existing fldstoreop/fieldstoreop helper ({1->MOVB,2->MOVW,4->MOVL,else->MOVQ}), both stages; the #13 graduation comments already pre-documented this resolution. Pure width fix, no loud-stop (scalar widths are always {1,2,4,8} and narrowing is always correct). Value-asserting pin: an i64 sentinel live across the union-maker call (detects the clobber directly) + all members, with a non-union control.
This commit is contained in:
Hojun-Cho
2026-06-28 01:59:21 +09:00
parent b691a6d8c3
commit 8fa59f9d45
3 changed files with 72 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
cmd/w6c/cgen.c
View File

@@ -3362,9 +3362,12 @@ cg_widen_tagged_push(Cg *c, Local **locals_p, Type *dst, Node *src, int sz)
* because the existing inline code's reload-before-each-store pattern
* matches the helper's per-store reload exactly.
*
* The scalar store dispatch stays at the explicit {1->MOVB, 4->MOVL,
* else MOVQ} shape (not fieldstoreop, which emits MOVW for fsz==2) to
* stay byte-identical with cstage pending task #13. */
* The scalar store routes through fldstoreop {1->MOVB, 2->MOVW,
* 4->MOVL, else MOVQ}: the missing MOVW for fsz==2 over-stored a
* 2-byte tail field past its slot into saved BP on a union-return
* success variant (#15). Natural field offsets aren't 8-aligned, so
* MOVQ-rely-on-ceil-8-pad (sound for #10's 8-aligned scratch) is
* wrong here; the store must be width-sized. */
static void
cg_structlit_fill(Cg *c, Local **locals_p, Type *lu, Node *lit,
int mode, int srcoff, const char *name, int disp)
@@ -3666,9 +3669,7 @@ cg_structlit_fill(Cg *c, Local **locals_p, Type *lu, Node *lit,
amem(base_reg, disp + (int)foff));
continue;
}
int op = A_MOVQ;
if (fsz == 1) op = A_MOVB;
else if (fsz == 4) op = A_MOVL;
int op = fldstoreop(ft, fsz);
ins2(c, op, areg(D_AX),
amem(base_reg, disp + (int)foff));
}

26
selfhost/cmd/wcc/cgenutil.ww
View File

@@ -5189,11 +5189,10 @@ export fn dotchainresolve(c: *cgen, n: *syntax.node,
// preserved because the existing inline code's reload-before-each-
// store pattern matches the helper's per-store reload exactly.
//
// Graduation note (task #13): the scalar store currently uses the
// explicit {1→MOVB, 4→MOVL, else MOVQ} dispatch to match cstage
// byte-identically — cstage hasn't yet learned MOVW for fsz==2. Once
// #13 aligns both stages, the dispatch can switch to fieldstoreop
// which already returns MOVW where appropriate.
// The scalar store routes through fieldstoreop (full field-width
// {1→MOVB, 2→MOVW, 4→MOVL, else MOVQ}); the missing MOVW for fsz==2
// over-stored a 2-byte tail field past its slot into saved BP on a
// union-return success variant (#15). Symmetric with cstage fldstoreop.
fn cgstructlitfill(c: *cgen, si: *structinfo, lit: *syntax.node,
mode: i32, srcoff: i32, srcname: str,
disp: i32) void = {
@@ -5704,16 +5703,13 @@ fn cgstructlitfill(c: *cgen, si: *structinfo, lit: *syntax.node,
};
fi = nil;
} else {
// Explicit {1→MOVB, 4→MOVL, else MOVQ}
// dispatch (not fieldstoreop) to match
// cstage byte-identically. wwstage's
// fieldstoreop would return MOVW for
// fsz==2 which cstage doesn't emit —
// tracked as task #13.
let fsz: i32 = fi.fsz;
let op: str = "MOVQ";
if (fsz == 1) { op = "MOVB"; };
if (fsz == 4) { op = "MOVL"; };
// Full field-width store via fieldstoreop
// {1→MOVB, 2→MOVW, 4→MOVL, else MOVQ}: the
// missing MOVW for fsz==2 over-stored a
// 2-byte tail field past its slot into
// saved BP on a union-return success
// variant (#15). Symmetric with cstage.
let op: str = fieldstoreop(c, fi);
emitline("\t");
emitline(op);
emitline("\tAX, ");

54
test/lang/union_subtail_bp_test.ww Normal file
View File

@@ -0,0 +1,54 @@
// union_subtail_bp_test — #15: a union-return struct-lit fill stored every
// scalar field with MOVQ except the explicit {1→MOVB, 4→MOVL} arms, so a
// 2-byte (i16/u16) field fell through to an 8-byte MOVQ. For the LAST field at
// the frame edge this over-store ran past the slot into saved [BP]: a
// `(s14|e)` success variant places the s14 payload at -16(BP) after the 8B
// tag, so the tail field g (off 12) lands at -4(BP) and `MOVQ AX,-4(BP)` writes
// bytes -4..+3 — clobbering the low 4 bytes of saved BP. POPQ BP then restores
// a corrupted BP and the CALLER runs on a garbage frame. SILENT both-stage and
// byte-id-BLIND (both stages emit the same wrong MOVQ), so these VALUE asserts
// are the sole tooth. Fix: route the scalar store through fldstoreop /
// fieldstoreop (adds the missing MOVW for fsz==2). Reverting either stage to
// the MOVQ-default reverts the clobber and reddens.
//
// Two teeth: (1) a sentinel i64 live ACROSS the maker call detects the BP
// clobber directly — on the buggy build the corrupted POPQ BP moves the
// caller frame so the sentinel read mismatches. (2) assert all s14 members.
// union_maker (the bug) + plain_maker control (same struct, NON-union maker:
// g lands at -12(BP), in-frame dead space, no live neighbour — passes even on
// master). The control isolates the union-return path as the buggy one.
package union_subtail_bp_test;
type e = !i32;
type s14 = struct { a: i16, b: i16, c: i16, d: i16, e: i16, f: i16, g: i16 };
fn mk14() (s14 | e) = {
return s14 { a = 1i16, b = 2i16, c = 3i16, d = 4i16, e = 5i16, f = 6i16, g = 7i16 };
};
fn mk14p() s14 = {
return s14 { a = 1i16, b = 2i16, c = 3i16, d = 4i16, e = 5i16, f = 6i16, g = 7i16 };
};
@test fn union_maker() void = {
let sentinel: i64 = 0x5151515151515151i64; // lives across the mk14 call
let arr: [2]s14;
arr[1] = mk14()!; // maker over-stores g past its slot into saved BP
assert(arr[1].a == 1i16);
assert(arr[1].b == 2i16);
assert(arr[1].c == 3i16);
assert(arr[1].d == 4i16);
assert(arr[1].e == 5i16);
assert(arr[1].f == 6i16);
assert(arr[1].g == 7i16); // the over-stored tail field
assert(sentinel == 0x5151515151515151i64); // BP-clobber tooth
};
@test fn plain_maker() void = { // CONTROL: same struct, NON-union maker
let sentinel: i64 = 0x5252525252525252i64;
let arr: [2]s14;
arr[1] = mk14p();
assert(arr[1].a == 1i16);
assert(arr[1].g == 7i16);
assert(sentinel == 0x5252525252525252i64);
};