From 5c3764828f9f27159a01f3f6f6a6cfe70ca94d8c Mon Sep 17 00:00:00 2001 From: Hojun-Cho Date: Mon, 8 Jun 2026 20:51:01 +0900 Subject: [PATCH] =?UTF-8?q?wcc/cgen:=20#150=20by-value=20module-global=20s?= =?UTF-8?q?truct-arg=20base=20=E2=80=94=20load=20main.g(SB)=20all=20words?= =?UTF-8?q?=20(both=20stages)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Passing a module-global struct by value -- let g: pt = pt{...}; take(g) -- was silently miscompiled, mirror-opposite on the two stages. cstage's by-value struct-arg arm hit localfind(g)->0 and read 2 words from the frame (MOVQ (BP)), never main.g(SB) -> returned garbage. wwstage used the correct main.g(SB) base but fell through to the scalar single-PUSHQ default, pushing one word for a 2-word struct -> dropped a field. Both stages now take the off==0 global branch: LEAQ main.NAME(SB) and copy all struct-size/8 eightbytes (reusing the GAP-A.ptr/#231 global-base predicate), converging to one byte-identical sequence. The local path (off!=0) is unchanged; >16B aggregates (#271) already resolved globals. Commit A of the cluster; the cstage-only inferred-global-type Sym-repoint (every let g = ... module-global yields downstream) is Commit B (#18). Slice/str global-by-value args have the same wwstage field-drop -- filed (#10 G-valglobal-arg; struct closed here). byte-id 990-997 8/8. test/wcc/822 table-driven, byte-id per stage. --- Makefile | 7 + cmd/w6c/cgen.c | 19 ++ selfhost/cmd/w6c/main.combined.ww | 33 ++++ selfhost/cmd/wcc/cgenutil.ww | 33 ++++ selfhost/cmd/wwdump/main.combined.ww | 33 ++++ test/wcc/822_struct_global_byval_arg.c | 247 +++++++++++++++++++++++++ 6 files changed, 372 insertions(+) create mode 100644 test/wcc/822_struct_global_byval_arg.c diff --git a/Makefile b/Makefile index 9323252e..8f6cdc03 100644 --- a/Makefile +++ b/Makefile @@ -250,6 +250,7 @@ TESTS = $(BIN)/test_smoke $(BIN)/test_lex $(BIN)/test_parse $(BIN)/test_check \ $(BIN)/test_def_str_table \ $(BIN)/test_arr_zero_vs_infer \ $(BIN)/test_def_str_index_reject \ + $(BIN)/test_struct_global_byval_arg \ $(BIN)/test_slice_str_global_zero \ $(BIN)/test_slice_literal_global \ $(BIN)/test_global_arr_elem_field \ @@ -658,6 +659,12 @@ $(BIN)/test_def_str_index_reject: test/wcc/821_def_str_index_reject.c $(BIN)/ww $(LIB)/libwwrt.a | $(BIN) $(CC) $(CFLAGS) -o $@ $< +$(BIN)/test_struct_global_byval_arg: test/wcc/822_struct_global_byval_arg.c $(BIN)/ww \ + $(BIN)/w6c $(BIN)/w6a $(BIN)/w6l \ + $(BIN)/ww_ww $(BIN)/w6c_ww $(BIN)/w6a_ww $(BIN)/w6l_ww \ + $(LIB)/libwwrt.a | $(BIN) + $(CC) $(CFLAGS) -o $@ $< + $(BIN)/test_def_arr_infer_len: test/wcc/814_def_arr_infer_len.c $(BIN)/ww \ $(BIN)/w6c $(BIN)/w6a $(BIN)/w6l \ $(BIN)/ww_ww $(BIN)/w6c_ww $(BIN)/w6a_ww $(BIN)/w6l_ww \ diff --git a/cmd/w6c/cgen.c b/cmd/w6c/cgen.c index 86065cda..2b690c89 100644 --- a/cmd/w6c/cgen.c +++ b/cmd/w6c/cgen.c @@ -9243,6 +9243,25 @@ cgexpr(Cg *c, Node *n, Local *locals) /* load qword(s) directly from the struct's slot */ int off = localfind(locals, args[i]->str); int sz = struct_arg_size(args[i]->type); + /* #150: a module-global struct source — off==0 + * is the localfind footgun (GAP-A.ptr/#231 + * family); the BP loads below read the stack + * frame, not main.g(SB). Resolve the global base + * into BX (the #129-A.2 struct-global LEAQ shape) + * and copy ALL eightbytes from it. */ + if (off == 0 && (let_islet(args[i]->str) + || def_isstructdef(args[i]->str))) { + ins2(c, A_LEAQ, masym(c, args[i]->str), + areg(D_BX)); + if (sz > 8) { + ins2(c, A_MOVQ, amem(D_BX, 8), + areg(D_AX)); + ins1(c, A_PUSHQ, areg(D_AX)); + } + ins2(c, A_MOVQ, amem(D_BX, 0), areg(D_AX)); + ins1(c, A_PUSHQ, areg(D_AX)); + continue; + } if (sz > 8) { ins2(c, A_MOVQ, amem(D_BP, off + 8), areg(D_AX)); ins1(c, A_PUSHQ, areg(D_AX)); diff --git a/selfhost/cmd/w6c/main.combined.ww b/selfhost/cmd/w6c/main.combined.ww index d669b8b9..39d10ab3 100644 --- a/selfhost/cmd/w6c/main.combined.ww +++ b/selfhost/cmd/w6c/main.combined.ww @@ -17043,6 +17043,39 @@ fn pushargsrev(c: *cgen, arg: *node, param: *node, memphase: bool) i32 = { return rest + nw; }; }; + // #150: a module-global by-value struct arg. localfindnode + // above misses it (a global is not a frame slot), and the #271 + // arm below excludes a ≤16B struct ident (structident=true), so + // pre-fix it fell through to the scalar single-PUSHQ default and + // silently dropped word1. cstage's mirror-opposite bug read the + // FRAME (localfind→0 off==0 footgun); both stages converge here + // on the global-base load: LEAQ name(SB) into BX, then copy ALL + // eightbytes high→low (the #256/#129-A.2 struct-global shape; the + // isletvar||deflookup gate is the let_islet||def_isstructdef twin + // from dotchainaddr). The slot-word count is the stamped tinfo + // size (the type table, byte-id with cstage struct_arg_size). + if (lc == nil) { + let gst: *tinfo = arg.type_: *tinfo; + gst = tichase(gst); + if (gst != nil) { if (gst.kind == tykind.TY_STRUCT) { + let gsz: i32 = gst.size: i32; + if (gsz > 0 && gsz <= 16 + && (isletvar(c, nm) || deflookup(c, nm))) { + emitline("\tLEAQ\t"); + emitsymname(c, nm); + emitline("(SB), BX\n"); + if (gsz > 8) { + emitline("\tMOVQ\t8(BX), AX\n"); + emitline("\tPUSHQ\tAX\n"); + }; + emitline("\tMOVQ\t(BX), AX\n"); + emitline("\tPUSHQ\tAX\n"); + let gnw: i32 = 1; + if (gsz > 8) { gnw = 2; }; + return rest + gnw; + }; + }; }; + }; }; // #271: aggregate (struct/array) arg from any source the ≤16B // struct-IDENT fast path above doesn't cover — a 16B struct from a diff --git a/selfhost/cmd/wcc/cgenutil.ww b/selfhost/cmd/wcc/cgenutil.ww index c31e448a..f471ae89 100644 --- a/selfhost/cmd/wcc/cgenutil.ww +++ b/selfhost/cmd/wcc/cgenutil.ww @@ -723,6 +723,39 @@ fn pushargsrev(c: *cgen, arg: *node, param: *node, memphase: bool) i32 = { return rest + nw; }; }; + // #150: a module-global by-value struct arg. localfindnode + // above misses it (a global is not a frame slot), and the #271 + // arm below excludes a ≤16B struct ident (structident=true), so + // pre-fix it fell through to the scalar single-PUSHQ default and + // silently dropped word1. cstage's mirror-opposite bug read the + // FRAME (localfind→0 off==0 footgun); both stages converge here + // on the global-base load: LEAQ name(SB) into BX, then copy ALL + // eightbytes high→low (the #256/#129-A.2 struct-global shape; the + // isletvar||deflookup gate is the let_islet||def_isstructdef twin + // from dotchainaddr). The slot-word count is the stamped tinfo + // size (the type table, byte-id with cstage struct_arg_size). + if (lc == nil) { + let gst: *tinfo = arg.type_: *tinfo; + gst = tichase(gst); + if (gst != nil) { if (gst.kind == tykind.TY_STRUCT) { + let gsz: i32 = gst.size: i32; + if (gsz > 0 && gsz <= 16 + && (isletvar(c, nm) || deflookup(c, nm))) { + emitline("\tLEAQ\t"); + emitsymname(c, nm); + emitline("(SB), BX\n"); + if (gsz > 8) { + emitline("\tMOVQ\t8(BX), AX\n"); + emitline("\tPUSHQ\tAX\n"); + }; + emitline("\tMOVQ\t(BX), AX\n"); + emitline("\tPUSHQ\tAX\n"); + let gnw: i32 = 1; + if (gsz > 8) { gnw = 2; }; + return rest + gnw; + }; + }; }; + }; }; // #271: aggregate (struct/array) arg from any source the ≤16B // struct-IDENT fast path above doesn't cover — a 16B struct from a diff --git a/selfhost/cmd/wwdump/main.combined.ww b/selfhost/cmd/wwdump/main.combined.ww index a760b746..87ef45a2 100644 --- a/selfhost/cmd/wwdump/main.combined.ww +++ b/selfhost/cmd/wwdump/main.combined.ww @@ -17043,6 +17043,39 @@ fn pushargsrev(c: *cgen, arg: *node, param: *node, memphase: bool) i32 = { return rest + nw; }; }; + // #150: a module-global by-value struct arg. localfindnode + // above misses it (a global is not a frame slot), and the #271 + // arm below excludes a ≤16B struct ident (structident=true), so + // pre-fix it fell through to the scalar single-PUSHQ default and + // silently dropped word1. cstage's mirror-opposite bug read the + // FRAME (localfind→0 off==0 footgun); both stages converge here + // on the global-base load: LEAQ name(SB) into BX, then copy ALL + // eightbytes high→low (the #256/#129-A.2 struct-global shape; the + // isletvar||deflookup gate is the let_islet||def_isstructdef twin + // from dotchainaddr). The slot-word count is the stamped tinfo + // size (the type table, byte-id with cstage struct_arg_size). + if (lc == nil) { + let gst: *tinfo = arg.type_: *tinfo; + gst = tichase(gst); + if (gst != nil) { if (gst.kind == tykind.TY_STRUCT) { + let gsz: i32 = gst.size: i32; + if (gsz > 0 && gsz <= 16 + && (isletvar(c, nm) || deflookup(c, nm))) { + emitline("\tLEAQ\t"); + emitsymname(c, nm); + emitline("(SB), BX\n"); + if (gsz > 8) { + emitline("\tMOVQ\t8(BX), AX\n"); + emitline("\tPUSHQ\tAX\n"); + }; + emitline("\tMOVQ\t(BX), AX\n"); + emitline("\tPUSHQ\tAX\n"); + let gnw: i32 = 1; + if (gsz > 8) { gnw = 2; }; + return rest + gnw; + }; + }; }; + }; }; // #271: aggregate (struct/array) arg from any source the ≤16B // struct-IDENT fast path above doesn't cover — a 16B struct from a diff --git a/test/wcc/822_struct_global_byval_arg.c b/test/wcc/822_struct_global_byval_arg.c new file mode 100644 index 00000000..95419def --- /dev/null +++ b/test/wcc/822_struct_global_byval_arg.c @@ -0,0 +1,247 @@ +/* + * 822_struct_global_byval_arg — a module-global struct passed BY VALUE to + * a fn (`let g: pt = pt{...}; take(g)`) was silently miscompiled, mirror- + * opposite on the two stages (task #150, the off==0 localfind footgun / + * GAP-A.ptr / #231 family): + * + * - cstage (cmd/w6c/cgen.c by-value struct-IDENT call-arg arm): localfind + * of a global returns 0, so the BP loads read the stack FRAME, never + * `LEAQ main.g(SB)` → garbage (ken saw 104, not the field sum). + * - wwstage (selfhost cgenexpr/cgenutil pushargsrev struct arm): the + * ≤16B-struct ident with no local slot fell past the local fast path + * AND was excluded from the #271 aggregate arm (structident), landing + * on the scalar single-PUSHQ default — ONE word for a 2-word struct, + * so the callee's word1 read stack garbage (silent field-drop). + * + * The fix (ONE both-stage cgen commit, converged byte-IDENTICAL): when the + * struct-arg operand is a module-global (off==0 && let_islet||def_isstructdef), + * resolve the global base into BX via `LEAQ name(SB)` and copy ALL + * eightbytes (`MOVQ off(BX),AX; PUSHQ`) high→low — the #129-A.2 struct- + * global access shape. The LOCAL path (off!=0) is untouched. + * + * row | shape | want + * ------------+------------------------------------------------+------ + * two_field | let g:pt{a:int,b:int}=pt{a=5,b=9}; take(g) | 14 + * narrow_mix | let g:struct{a:i32,b:i32,c:int}; sum | 6 + * three_word | let g:struct{a,b,c:int} (24B, #271 arm); sum | 7 + * ctrl_local | LOCAL struct arg (off!=0, unchanged, byte-id) | 14 + * + * two_field / narrow_mix are the mutation-sane rows: pre-fix cstage reads + * the frame (garbage) and wwstage drops word1. three_word confirms the >16B + * #271 aggregate arm already handles a struct global (let_islet base). + * ctrl_local guards the unchanged local-struct path. A byte-id row pins + * cstage==wwstage `.s` for each source — the convergence the fix delivers. + * + * Explicit-typed globals ONLY (`let g: pt = ...`): an inferred `let g = + * pt{}` would loud via the separate checker Bug 1 (#18) and mask #150. + */ +#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; }; + +static const struct row rows[] = { + /* two_field — THE regression pin: 2-field 16B struct global by value. + * Pre-fix cstage reads the frame (garbage), wwstage drops field b. */ + { "two_field", + "package main;\n" + "type pt = struct { a: int, b: int };\n" + "let g: pt = pt { a = 5, b = 9 };\n" + "fn take(x: pt) int = { return x.a + x.b; };\n" + "export fn main() i32 = { return take(g): i32; };\n", + 14 }, + + /* narrow_mix — i32/i32/int packs two narrow fields into eightbyte 0, + * int into eightbyte 1; 16B → the struct-ident fast arm (the fix). */ + { "narrow_mix", + "package main;\n" + "type pt = struct { a: i32, b: i32, c: int };\n" + "let g: pt = pt { a = 1, b = 2, c = 3 };\n" + "fn take(x: pt) int = { return (x.a: int) + (x.b: int) + x.c; };\n" + "export fn main() i32 = { return take(g): i32; };\n", + 6 }, + + /* three_word — 24B struct global (>16B) routes through the #271 + * aggregate arm; confirms the global base (let_islet) is honoured. */ + { "three_word", + "package main;\n" + "type pt = struct { a: int, b: int, c: int };\n" + "let g: pt = pt { a = 1, b = 2, c = 4 };\n" + "fn take(x: pt) int = { return x.a + x.b + x.c; };\n" + "export fn main() i32 = { return take(g): i32; };\n", + 7 }, + + /* ctrl_local — a LOCAL struct arg (off!=0). cgen unchanged; pins the + * local-struct push path stays byte-id (no regress). */ + { "ctrl_local", + "package main;\n" + "type pt = struct { a: int, b: int };\n" + "fn take(x: pt) int = { return x.a + x.b; };\n" + "export fn main() i32 = {\n" + "\tlet g: pt = pt { a = 5, b = 9 };\n" + "\treturn take(g): i32;\n" + "};\n", + 14 }, +}; + +static int +run_driver(const char *driver, const struct row *r, int i) +{ + char src[64], tmpdir[64], cmd[1024]; + snprintf(src, sizeof src, "/tmp/sga_%d_%d.ww", getpid(), i); + snprintf(tmpdir, sizeof tmpdir, "/tmp/sga_%d_d_%d", getpid(), i); + + FILE *f = fopen(src, "wb"); + if (!f) return -1; + fputs(r->src, f); + fclose(f); + + mkdir(tmpdir, 0755); + snprintf(cmd, sizeof cmd, "cd %s && %s build %s 2>/dev/null", + tmpdir, driver, src); + if (runwait(cmd) != 0) { + fprintf(stderr, "row[%s]: build via %s failed\n", + r->label, driver); + unlink(src); rmdir(tmpdir); + return -1; + } + + const char *base = strrchr(src, '/'); + base = base ? base + 1 : src; + char outbin[128]; + snprintf(outbin, sizeof outbin, "%s/%s", tmpdir, base); + char *dot = strrchr(outbin, '.'); + if (dot && strcmp(dot, ".ww") == 0) *dot = '\0'; + int got = runwait(outbin); + + unlink(src); unlink(outbin); rmdir(tmpdir); + return got; +} + +/* asm_byte_identical — w6c vs w6c_ww .s for the same source must match. */ +static int +asm_byte_identical(const char *bin, const struct row *r, int i) +{ + char src[64], cs[64], ws[64], cmd[1024]; + snprintf(src, sizeof src, "/tmp/sga_asm_%d_%d.ww", getpid(), i); + snprintf(cs, sizeof cs, "/tmp/sga_asm_%d_%d_c.s", getpid(), i); + snprintf(ws, sizeof ws, "/tmp/sga_asm_%d_%d_w.s", getpid(), i); + + FILE *f = fopen(src, "wb"); + if (!f) return -1; + fputs(r->src, f); + fclose(f); + + snprintf(cmd, sizeof cmd, "%s/w6c -o %s %s 2>/dev/null", bin, cs, src); + if (runwait(cmd) != 0) { + fprintf(stderr, "row[%s]: w6c errored\n", r->label); + unlink(src); + return -1; + } + snprintf(cmd, sizeof cmd, "%s/w6c_ww -o %s %s 2>/dev/null", + bin, ws, src); + if (runwait(cmd) != 0) { + fprintf(stderr, "row[%s]: w6c_ww errored\n", r->label); + unlink(src); unlink(cs); + return -1; + } + + FILE *fc = fopen(cs, "rb"); + FILE *fw = fopen(ws, "rb"); + int rc = 0; + if (!fc || !fw) { + rc = -1; + } else { + for (;;) { + int a = fgetc(fc); + int b = fgetc(fw); + if (a != b) { rc = -1; break; } + if (a == EOF) break; + } + } + if (fc) fclose(fc); + if (fw) fclose(fw); + if (rc != 0) + fprintf(stderr, "row[%s]: cstage vs wwstage asm differs\n", + r->label); + unlink(src); unlink(cs); unlink(ws); + 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 cdrv[1024]; + snprintf(cdrv, sizeof cdrv, "%s/ww", bin); + char wdrv[1024]; + snprintf(wdrv, sizeof wdrv, "%s/ww_ww", bin); + + struct { const char *name; const char *path; int gated_on_existence; } + drivers[] = { + { "cstage", cdrv, 0 }, + { "wwstage", wdrv, 1 }, + { NULL, NULL, 0 }, + }; + + int n = (int)(sizeof rows / sizeof rows[0]); + int total = 0, fail = 0; + + for (int d = 0; drivers[d].name; d++) { + if (drivers[d].gated_on_existence + && access(drivers[d].path, X_OK) != 0) { + fprintf(stderr, "struct_global_byval_arg: skip %s (no %s)\n", + drivers[d].name, drivers[d].path); + continue; + } + for (int i = 0; i < n; i++) { + int got = run_driver(drivers[d].path, &rows[i], i); + total++; + if (got != rows[i].want) { + fprintf(stderr, + "struct_global_byval_arg[%s][%s]: exit=%d want=%d\n", + drivers[d].name, rows[i].label, + got, rows[i].want); + fail++; + } + } + } + + if (access(wdrv, X_OK) == 0) { + for (int i = 0; i < n; i++) { + total++; + if (asm_byte_identical(bin, &rows[i], i) != 0) + fail++; + } + } + + if (fail) { + fprintf(stderr, + "struct_global_byval_arg: %d/%d fixtures failed\n", fail, total); + return 1; + } + printf("struct_global_byval_arg: %d/%d ok\n", total, total); + return 0; +}