From a8d1df6090ffd041ffdae97fc29f6dcbc5a7e3f5 Mon Sep 17 00:00:00 2001 From: Hojun-Cho Date: Mon, 18 May 2026 15:24:59 +0900 Subject: [PATCH] selfhost+test: graduate bare-leaf fnretlookup same-module-first (#4e) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Class A silent miscompile, latent until two modules export the same fn leaf name with diverging return-type categories (str vs scalar, tagged vs not, tuple vs not, float vs int, struct-payload-size). Wwstage's fnretlookup (selfhost/cmd/wcc/cgen.ww) walked c.fnrets head-first by fname and returned the FIRST match's rtype. cgcall's str-shuffle decision (cgenexpr.ww:3249) handed it calleename (the bare leaf from an N_IDENT callee); a same-leaf foo registered later (at head) returning str then mis-fired isstrtype(c, rt) for an i64-returning callee, emitting a spurious MOVQ DX, BX after the CALL — the SysV (AX, DX) → ww str (AX, BX) shuffle — corrupting BX even though the callee never returned an str pair. Every other bare-leaf consumer (taggedcallslot, callsretsize, exprfloatkind, rhstaggedabicall, tuple destructure in cglet/cgmlet, fn-rvalue LEAQ in cgident, cgtry{prop,unw} success-shuffle) keys on the same fnretlookup return and was silently miscompiling under the same collision shape. Cstage carries no sister bug: cmd/wcc/check.c N_CALL routes cexpr(c, n->lhs) through scope_lookup_prefer for an N_IDENT callee, then cmd/w6c/cgen.c reads the return type from the typed n->lhs->type's TY_FN sig — module-aware via the typed AST, sidestepping any bare-leaf table. cs vs ws diverged on every same- leaf fn return-category collision but no in-tree corpus declares two same-leaf fns with diverging return categories today: 995 stays green (same surfacing pattern as #4a enumlookup post-strings, #4b structlookup, #4c def, #4d fnparams). Eighth and FINAL leaf of the trio graduation (after #27 aliaslookup, #28 fnparams *mod*-variant, #31 fnret *mod*-variant, #4a enum, #4b struct, #4c def, #4d fnparams bare-leaf). fnretlookupmod (the N_DOT consumer at cgen.ww:1585) already exists post-#31; this commit graduates only the BARE-LEAF entry point with a same-module-first walk mirroring fnparamslookup's two-pass shape (#4d). 12+ bare-leaf callsites consume the graduated lookup uniformly — none separately re-routed to fnretlookupmod since the in-tree N_DOT collisions (strings.next vs utf8.next; bytes.hasprefix vs strings.hasprefix and equivalents) all have invariant return shape across the colliding overloads. A future stdlib port introducing a return- category-divergent same-leaf N_DOT collision will need the *mod re-routing — file at that surfacing. Pre-flight on 995_self_rebuild green: rob's brief warned 1-2 byte- id surfaces possible because bare-leaf graduation could flip MOVQ↔MOVSXD or push-count on selfhost compile paths not routed through *lookupmod. Audit confirms the corpus has bare-leaf same- name fn pairs (compare in lib/strings vs lib/time; next in utf8 vs strings) but downstream consumer behavior is invariant under both shapes — cross-module calls all go through N_DOT → fnretlookupmod, not the bare-leaf path. Zero actual surfaces. 731_fnret_bare_leaf_shadow pins the fix with 1 row: alpha defines fn foo() i64 + fn alphacaller() i64 = { return foo(); }, beta defines fn foo() str declared LAST in source so beta.foo prepends to the head of c.fnrets. alphacaller's bare foo() must compile against alpha.foo's i64 return (no str-shuffle) even with beta.foo at the head of c.fnrets. Asserts CALL alpha.foo inside the right TEXT sym + bad_imm MOVQ DX, BX anti-check on each stage plus cs-vs-ws byte-id per row. --- Makefile | 5 + selfhost/cmd/w6c/main.combined.ww | 21 ++- selfhost/cmd/wcc/cgen.ww | 21 ++- selfhost/cmd/wwdump/main.combined.ww | 21 ++- test/wcc/731_fnret_bare_leaf_shadow.c | 251 ++++++++++++++++++++++++++ 5 files changed, 313 insertions(+), 6 deletions(-) create mode 100644 test/wcc/731_fnret_bare_leaf_shadow.c diff --git a/Makefile b/Makefile index 89c89a5d..1a37908a 100644 --- a/Makefile +++ b/Makefile @@ -266,6 +266,7 @@ TESTS = $(BIN)/test_smoke $(BIN)/test_lex $(BIN)/test_parse $(BIN)/test_check \ $(BIN)/test_struct_modshadow \ $(BIN)/test_def_modshadow \ $(BIN)/test_fnparams_bare_leaf_shadow \ + $(BIN)/test_fnret_bare_leaf_shadow \ $(BIN)/test_param_shadow_mod \ $(BIN)/test_localoff_scope \ $(BIN)/test_cast_enum_movl \ @@ -628,6 +629,10 @@ $(BIN)/test_fnparams_bare_leaf_shadow: test/wcc/732_fnparams_bare_leaf_shadow.c $(BIN)/w6c $(BIN)/w6c_ww | $(BIN) $(CC) $(CFLAGS) -o $@ $< +$(BIN)/test_fnret_bare_leaf_shadow: test/wcc/731_fnret_bare_leaf_shadow.c \ + $(BIN)/w6c $(BIN)/w6c_ww | $(BIN) + $(CC) $(CFLAGS) -o $@ $< + $(BIN)/test_match_4arm_cross_module_run: test/wcc/929_match_4arm_cross_module_run.c \ $(BIN)/ww $(BIN)/w6c $(BIN)/w6a $(BIN)/w6l \ $(BIN)/ww_ww $(BIN)/w6c_ww $(BIN)/w6a_ww $(BIN)/w6l_ww \ diff --git a/selfhost/cmd/w6c/main.combined.ww b/selfhost/cmd/w6c/main.combined.ww index b7ad73c5..93867119 100644 --- a/selfhost/cmd/w6c/main.combined.ww +++ b/selfhost/cmd/w6c/main.combined.ww @@ -20293,11 +20293,28 @@ fn collectfnrets(c: *cgen, file: *node) void = { }; }; +// fnretlookup — declared return-type node for a fn by leaf name, or nil +// if the name isn't a registered fn. Same-module-first walk before the +// head-walk fallback. Eighth and final leaf of the trio graduation (#4e) +// mirroring aliaslookup (#27), fnret/fnparamslookupmod (#28/#31), +// enum/struct/deflookup (#4a/#4b/#4c), fnparamslookup (#4d): without +// the prefer pass a bare-leaf `foo()` call site in module M (N_IDENT +// callee) silently picks another module's same-leaf `foo` from the +// head of c.fnrets, then every downstream consumer keying on the +// return type (str-pair shuffle, tagged-union ABI, tuple destructure, +// float ABI, sret slot sizing, fn-rvalue LEAQ, slice flow) fires +// against the wrong-module shape. fn fnretlookup(c: *cgen, name: str) *node = { let f: *fnret = c.fnrets; for (f != nil) { - let fn_: str = f.fname; - if (streq(fn_, name)) { return f.rtype; }; + if (streq(f.fname, name)) { + if (streq(f.fmod, c.curmod)) { return f.rtype; }; + }; + f = f.frnext; + }; + f = c.fnrets; + for (f != nil) { + if (streq(f.fname, name)) { return f.rtype; }; f = f.frnext; }; return nil; diff --git a/selfhost/cmd/wcc/cgen.ww b/selfhost/cmd/wcc/cgen.ww index 47c06ef1..1831a2bf 100644 --- a/selfhost/cmd/wcc/cgen.ww +++ b/selfhost/cmd/wcc/cgen.ww @@ -1563,11 +1563,28 @@ fn collectfnrets(c: *cgen, file: *node) void = { }; }; +// fnretlookup — declared return-type node for a fn by leaf name, or nil +// if the name isn't a registered fn. Same-module-first walk before the +// head-walk fallback. Eighth and final leaf of the trio graduation (#4e) +// mirroring aliaslookup (#27), fnret/fnparamslookupmod (#28/#31), +// enum/struct/deflookup (#4a/#4b/#4c), fnparamslookup (#4d): without +// the prefer pass a bare-leaf `foo()` call site in module M (N_IDENT +// callee) silently picks another module's same-leaf `foo` from the +// head of c.fnrets, then every downstream consumer keying on the +// return type (str-pair shuffle, tagged-union ABI, tuple destructure, +// float ABI, sret slot sizing, fn-rvalue LEAQ, slice flow) fires +// against the wrong-module shape. fn fnretlookup(c: *cgen, name: str) *node = { let f: *fnret = c.fnrets; for (f != nil) { - let fn_: str = f.fname; - if (streq(fn_, name)) { return f.rtype; }; + if (streq(f.fname, name)) { + if (streq(f.fmod, c.curmod)) { return f.rtype; }; + }; + f = f.frnext; + }; + f = c.fnrets; + for (f != nil) { + if (streq(f.fname, name)) { return f.rtype; }; f = f.frnext; }; return nil; diff --git a/selfhost/cmd/wwdump/main.combined.ww b/selfhost/cmd/wwdump/main.combined.ww index 4111a71d..e43059eb 100644 --- a/selfhost/cmd/wwdump/main.combined.ww +++ b/selfhost/cmd/wwdump/main.combined.ww @@ -20293,11 +20293,28 @@ fn collectfnrets(c: *cgen, file: *node) void = { }; }; +// fnretlookup — declared return-type node for a fn by leaf name, or nil +// if the name isn't a registered fn. Same-module-first walk before the +// head-walk fallback. Eighth and final leaf of the trio graduation (#4e) +// mirroring aliaslookup (#27), fnret/fnparamslookupmod (#28/#31), +// enum/struct/deflookup (#4a/#4b/#4c), fnparamslookup (#4d): without +// the prefer pass a bare-leaf `foo()` call site in module M (N_IDENT +// callee) silently picks another module's same-leaf `foo` from the +// head of c.fnrets, then every downstream consumer keying on the +// return type (str-pair shuffle, tagged-union ABI, tuple destructure, +// float ABI, sret slot sizing, fn-rvalue LEAQ, slice flow) fires +// against the wrong-module shape. fn fnretlookup(c: *cgen, name: str) *node = { let f: *fnret = c.fnrets; for (f != nil) { - let fn_: str = f.fname; - if (streq(fn_, name)) { return f.rtype; }; + if (streq(f.fname, name)) { + if (streq(f.fmod, c.curmod)) { return f.rtype; }; + }; + f = f.frnext; + }; + f = c.fnrets; + for (f != nil) { + if (streq(f.fname, name)) { return f.rtype; }; f = f.frnext; }; return nil; diff --git a/test/wcc/731_fnret_bare_leaf_shadow.c b/test/wcc/731_fnret_bare_leaf_shadow.c new file mode 100644 index 00000000..59950df0 --- /dev/null +++ b/test/wcc/731_fnret_bare_leaf_shadow.c @@ -0,0 +1,251 @@ +/* + * 731_fnret_bare_leaf_shadow — sentinel for the trio leaf-name + * pattern's 8th and FINAL leaf: wwstage's bare-leaf fnretlookup + * (selfhost/cmd/wcc/cgen.ww). Pins bare-leaf `foo()` calls (N_IDENT + * callee) to a same-module-first walk so cgcall's str-pair shuffle + * decision fires against the caller's own foo's return type, not + * another module's same-leaf-name foo sitting at the head of + * c.fnrets. + * + * Pre-fix wwstage's `fnretlookup` walked c.fnrets head-first by + * fname and returned the FIRST match's rtype. cgcall (cgenexpr.ww + * cgcall:3249) handed it `calleename` (the bare leaf from an + * N_IDENT callee) and the head-pick silently picked a sibling + * module's same-leaf foo. When that sibling foo returned `str` + * and the caller's own foo returned a scalar (i64 here), the + * `isstrtype(c, rt)` guard fired against the wrong type and + * emitted a spurious `MOVQ DX, BX` shuffle after the CALL — the + * SysV (AX, DX) → ww str (AX, BX) shape — corrupting BX even + * though the callee never returned an str pair. Every other bare- + * leaf consumer (taggedcallslot, callsretsize, exprfloatkind, + * rhstaggedabicall, tuple destructure in cglet / cgmlet, fn-rvalue + * LEAQ in cgident, cgtryprop/cgtryunw success-shuffle) keys on + * the same fnretlookup return and was silently miscompiling under + * the same collision shape. + * + * Cstage carries no sister bug: cmd/wcc/check.c N_CALL routes + * `cexpr(c, n->lhs)` through scope_lookup_prefer for an N_IDENT + * callee, then cmd/w6c/cgen.c reads the return type from the + * typed `n->lhs->type`'s TY_FN sig — module-aware via the typed + * AST, sidestepping any bare-leaf table. Same shape as #4a/#4b/ + * #4c/#4d: cs vs ws diverge on every same-leaf fn return-type + * collision but no in-tree corpus declares two same-leaf fns + * with diverging return-type *categories* (str vs scalar, tagged + * vs not, tuple vs not, float vs int) today, so 995_self_rebuild + * stays green — the in-corpus leaf collisions (utf8.next vs + * strings.next, strconv.invalid vs utf8.invalid, etc.) are all + * routed through the existing fnretlookupmod N_DOT variant and + * the bare-leaf path never sees them on the present corpus. + * + * Eighth and FINAL leaf of the trio graduation (after #27 + * aliaslookup, #28 fnparams *mod*-variant, #31 fnret *mod*-variant, + * #4a enum, #4b struct, #4c def, #4d fnparams bare-leaf). + * fnretlookupmod (the N_DOT consumer at cgen.ww:1585) already + * exists post-#31; this commit graduates only the BARE-LEAF entry + * point with a same-module-first walk mirroring fnparamslookup's + * two-pass shape (#4d). Twelve+ bare-leaf callsites consume the + * graduated lookup uniformly (cgenutil.ww:468/494/611/1428/1674/ + * 2286/2795, cgenexpr.ww:160/204/565/1709/3249, cgenstmt.ww:173/ + * 1017, cgendecl.ww:104) — none are independently re-routed to + * fnretlookupmod in this commit. The in-tree N_DOT cross-module + * fn collisions (utf8.next vs strings.next; bytes.hasprefix vs + * strings.hasprefix and equivalents) all have invariant return + * shape across the colliding overloads (rune-or-done tagged on + * both next, bool on both hasprefix), so the consumer behaviour + * is invariant either way for the N_DOT-feeding sites (e.g. the + * cgcall:3249 callsite reads callee.kind == N_DOT and feeds the + * bare leaf — invariant on the present corpus). A future stdlib + * port introducing a return-category-divergent same-leaf N_DOT + * collision will need the *mod re-routing — file at that + * surfacing. + * + * Pin: row 1 sentinel-flips the bare-leaf graduation on wwstage — + * revert the prefer pass and row 1 fails on wwstage (a spurious + * `MOVQ DX, BX` for the wrong-module str shuffle sneaks in after + * the CALL). Cstage sees no shuffle either way (typed AST is + * module-aware). Asserts CALL alpha.foo present inside the right + * TEXT sym + bad_imm (`MOVQ\tDX, BX`) anti-check on each stage + * plus cs-vs-ws byte-id per row. + */ +#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; + const char *textsym; /* TEXT sym containing the call */ + const char *want_imm; /* substring that MUST appear */ + const char *bad_imm; /* substring that MUST NOT appear */ +}; + +/* Source ordering picks which module's `fn foo` sits at the head of + * c.fnrets after collectfnrets' head-prepend walk. The LAST `fn foo` + * in source order ends at the head — that's the leaf-collision the + * same-module-first walk must beat. Caller is inside alpha; alpha's + * foo returns i64 (no str shuffle), beta's foo returns str (head- + * pick would falsely emit MOVQ DX, BX). */ +static const struct row rows[] = { + { "bare_leaf_same_module", + "// MODULE: gamma\n" + "use alpha;\n" + "use beta;\n" + "export fn main() i32 = { return 0; };\n" + "// MODULE: alpha\n" + "export fn foo() i64 = { return 0; };\n" + "export fn alphacaller() i64 = { return foo(); };\n" + "// MODULE: beta\n" + "export fn foo() str = { return \"x\"; };\n", + "TEXT alpha.alphacaller", "CALL\talpha.foo", "MOVQ\tDX, BX" }, +}; + +static int +slurp(const char *path, char *buf, size_t cap) +{ + FILE *f = fopen(path, "rb"); + if (!f) return -1; + size_t n = fread(buf, 1, cap - 1, f); + fclose(f); + buf[n] = '\0'; + return (int)n; +} + +static int +emit_s(const char *w6c, const struct row *r, int i, char *out_s, size_t cap) +{ + char src[64], cmd[1024]; + snprintf(src, sizeof src, "/tmp/frb_%d_%d.ww", getpid(), i); + snprintf(out_s, cap, "/tmp/frb_%d_%d_%s.s", + getpid(), i, w6c[strlen(w6c) - 1] == 'w' ? "ww" : "c"); + + FILE *f = fopen(src, "wb"); + if (!f) return -1; + fputs(r->src, f); + fclose(f); + + snprintf(cmd, sizeof cmd, "%s -o %s %s 2>/dev/null", w6c, out_s, src); + int rc = runwait(cmd); + unlink(src); + return rc; +} + +/* Inside the named TEXT sym, before its first RET, the want_imm MUST + * appear and the bad_imm MUST NOT. bad_imm flags pre-fix bare-leaf + * head-walk picking beta's str-returning foo and emitting a spurious + * MOVQ DX, BX after the CALL to a scalar-returning fn. */ +static int +check_imm(const char *spath, const struct row *r, const char *stage) +{ + char buf[1 << 14]; + if (slurp(spath, buf, sizeof buf) < 0) { + fprintf(stderr, "row[%s][%s]: cannot read %s\n", + r->label, stage, spath); + return -1; + } + const char *fn = strstr(buf, r->textsym); + if (!fn) { + fprintf(stderr, "row[%s][%s]: no %s in %s\n", + r->label, stage, r->textsym, spath); + return -1; + } + const char *ret = strstr(fn, "\tRET"); + if (!ret) { + fprintf(stderr, "row[%s][%s]: no RET inside %s\n", + r->label, stage, r->textsym); + return -1; + } + const char *good = strstr(fn, r->want_imm); + if (!good || good >= ret) { + fprintf(stderr, + "row[%s][%s]: want_imm %s missing inside %s\n", + r->label, stage, r->want_imm, r->textsym); + return -1; + } + const char *bad = strstr(fn, r->bad_imm); + if (bad && bad < ret) { + fprintf(stderr, + "row[%s][%s]: bad_imm %s present inside %s — wrong-module foo str-shuffled\n", + r->label, stage, r->bad_imm, r->textsym); + return -1; + } + return 0; +} + +int +main(void) +{ + const char *bin = getenv("BIN"); + if (!bin) bin = "out/bin"; + char absbin[512]; + if (bin[0] != '/') { + char cwd[256]; + if (getcwd(cwd, sizeof cwd) == NULL) return 1; + snprintf(absbin, sizeof absbin, "%s/%s", cwd, bin); + bin = absbin; + } + + char w6c[640], w6c_ww[640]; + snprintf(w6c, sizeof w6c, "%s/w6c", bin); + snprintf(w6c_ww, sizeof w6c_ww, "%s/w6c_ww", bin); + + int have_ww = (access(w6c_ww, X_OK) == 0); + int n = (int)(sizeof rows / sizeof rows[0]); + int total = 0, fail = 0; + + for (int i = 0; i < n; i++) { + char cs_path[128], ws_path[128]; + + if (emit_s(w6c, &rows[i], i, cs_path, sizeof cs_path) != 0) { + fprintf(stderr, + "fnret_bare_leaf_shadow[cstage][%s]: w6c failed\n", + rows[i].label); + fail++; total++; continue; + } + total++; + if (check_imm(cs_path, &rows[i], "cstage") != 0) fail++; + + if (!have_ww) { unlink(cs_path); continue; } + + if (emit_s(w6c_ww, &rows[i], i, ws_path, sizeof ws_path) != 0) { + fprintf(stderr, + "fnret_bare_leaf_shadow[wwstage][%s]: w6c_ww failed\n", + rows[i].label); + fail++; total++; + unlink(cs_path); continue; + } + total++; + if (check_imm(ws_path, &rows[i], "wwstage") != 0) fail++; + + total++; + char cmd[512]; + snprintf(cmd, sizeof cmd, "cmp -s %s %s", cs_path, ws_path); + if (runwait(cmd) != 0) { + fprintf(stderr, + "fnret_bare_leaf_shadow[%s]: cstage vs wwstage asm differs\n", + rows[i].label); + fail++; + } + + unlink(cs_path); unlink(ws_path); + } + + if (fail) { + fprintf(stderr, + "fnret_bare_leaf_shadow: %d/%d fixtures failed\n", + fail, total); + return 1; + } + printf("fnret_bare_leaf_shadow: %d/%d ok\n", total, total); + return 0; +}