From d9b0c90fbc0755793007aa0c6920b426d8d2e52f Mon Sep 17 00:00:00 2001 From: Hojun-Cho Date: Tue, 19 May 2026 04:56:56 +0900 Subject: [PATCH] selfhost+test: route callee_variadic_param N_DOT via fnparamslookupmod (#16) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Latent silent miscompile surfaced by worker-strcontains3 attempting strings.contains tagged-variadic graduation: wwstage cgcall's callee_variadic_param helper (cgenutil.ww:60-70) consumed the N_DOT callee's leaf via callee.str but routed bare fnparamslookup — bypassed the module hint at callee.lhs.str. When two modules export same-leaf fns with differing variadic shapes (e.g. strings.contains(str|rune)... + bytes.contains scalar (u8|[]u8)), the bare walk returned the wrong fn's params for arg-prep while the CALL targeted the correct module-qualified symbol — ABI mismatch. Direct sister of #34 (049ebc1) which graduated fnret's N_DOT arm through fnretlookupmod. #4d's commit body (862715d) explicitly deferred callee_variadic_param's *mod re-routing pending "future stdlib port introducing a tagged-vs-scalar or variadic-vs-non-variadic same-leaf N_DOT collision shape." This is that surfacing. cgenutil.ww: split callee_variadic_param on callee.kind. N_IDENT stays on bare fnparamslookup (same-module-first post-#4d). N_DOT routes through fnparamslookupmod(c, callee.str, callee.lhs.str), pattern- identical to cgcall's N_DOT branch at cgenexpr.ww:2922-2935. Cstage cmd/w6c/cgen.c:4279-4302 reads callee params via typed AST (n->lhs->type + cu->params) — module-aware natively, no sister change needed (mirrors #4d/#28/#31/#34 cstage no-sister notes). 752_modparam_callee: table-driven 3 rows x 2 stages = 6 fixtures. cross_module_same_leaf_variadic_vs_scalar (the wedge), same_module_same_leaf (no-regress), bare_leaf_no_collision (control). #17 filed for the wider convenience-wrapper audit (enumerate all wwstage cgen* helpers that take *node and do bare-leaf lookups; sweep for N_DOT-arm omissions). This commit is narrow to callee_variadic_param. make test 126/126; ww2==ww3==ww4 byte-id holds via 995_self_rebuild. --- Makefile | 7 + selfhost/cmd/w6c/main.combined.ww | 30 ++- selfhost/cmd/wcc/cgenutil.ww | 30 ++- selfhost/cmd/wwdump/main.combined.ww | 30 ++- test/wcc/752_modparam_callee.c | 285 +++++++++++++++++++++++++++ 5 files changed, 364 insertions(+), 18 deletions(-) create mode 100644 test/wcc/752_modparam_callee.c diff --git a/Makefile b/Makefile index 609cc477..46904b97 100644 --- a/Makefile +++ b/Makefile @@ -286,6 +286,7 @@ TESTS = $(BIN)/test_smoke $(BIN)/test_lex $(BIN)/test_parse $(BIN)/test_check \ $(BIN)/test_sumtype_forward \ $(BIN)/test_mklabel_modscoped \ $(BIN)/test_vararg_seq_percall \ + $(BIN)/test_modparam_callee \ $(BIN)/test_param_shadow_mod \ $(BIN)/test_localoff_scope \ $(BIN)/test_cast_enum_movl \ @@ -727,6 +728,12 @@ $(BIN)/test_vararg_seq_percall: test/wcc/751_vararg_seq_percall.c \ $(LIB)/libwwrt.a | $(BIN) $(CC) $(CFLAGS) -o $@ $< +$(BIN)/test_modparam_callee: test/wcc/752_modparam_callee.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_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 b2d2c275..0a859be8 100644 --- a/selfhost/cmd/w6c/main.combined.ww +++ b/selfhost/cmd/w6c/main.combined.ww @@ -7282,15 +7282,33 @@ fn findvariadicparam(ps: *node, nfixed_out: *i32) *node = { // callee_variadic_param — convenience wrapper: looks up the callee // by name and finds its variadic param + nfixed. Returns nil if the // callee isn't registered or has no variadic param. +// +// N_DOT routes through fnparamslookupmod with the module hint +// (callee.lhs.str) — bare fnparamslookup walks same-module-first +// (#4d) which is wrong for a cross-module N_DOT call into a module +// whose same-leaf fn has divergent variadic-vs-non-variadic shape. +// #4d explicitly deferred this re-routing; surfaced by #16 when +// strings.contains gained a variadic shape and a caller's +// bytes.contains call site picked strings.contains' variadic +// params for arg-prep while emitting CALL bytes.contains. fn callee_variadic_param(c: *cgen, callee: *node, nfixed_out: *i32) *node = { *nfixed_out = 0; if (callee == nil) { return nil; }; - let cnm: str; - cnm.ptr = nil; cnm.len = 0; - if (callee.kind == nkind.N_IDENT) { cnm = callee.str; }; - if (callee.kind == nkind.N_DOT) { cnm = callee.str; }; - if (cnm.len == 0) { return nil; }; - let ps: *node = fnparamslookup(c, cnm); + let ps: *node = nil; + if (callee.kind == nkind.N_IDENT) { + if (callee.str.len == 0) { return nil; }; + ps = fnparamslookup(c, callee.str); + } else { if (callee.kind == nkind.N_DOT) { + if (callee.str.len == 0) { return nil; }; + let cmod: str; + cmod.ptr = nil; cmod.len = 0; + if (callee.lhs != nil) { + if (callee.lhs.kind == nkind.N_IDENT) { + cmod = callee.lhs.str; + }; + }; + ps = fnparamslookupmod(c, callee.str, cmod); + }; }; return findvariadicparam(ps, nfixed_out); }; diff --git a/selfhost/cmd/wcc/cgenutil.ww b/selfhost/cmd/wcc/cgenutil.ww index b545e4bb..6d0be67e 100644 --- a/selfhost/cmd/wcc/cgenutil.ww +++ b/selfhost/cmd/wcc/cgenutil.ww @@ -57,15 +57,33 @@ fn findvariadicparam(ps: *node, nfixed_out: *i32) *node = { // callee_variadic_param — convenience wrapper: looks up the callee // by name and finds its variadic param + nfixed. Returns nil if the // callee isn't registered or has no variadic param. +// +// N_DOT routes through fnparamslookupmod with the module hint +// (callee.lhs.str) — bare fnparamslookup walks same-module-first +// (#4d) which is wrong for a cross-module N_DOT call into a module +// whose same-leaf fn has divergent variadic-vs-non-variadic shape. +// #4d explicitly deferred this re-routing; surfaced by #16 when +// strings.contains gained a variadic shape and a caller's +// bytes.contains call site picked strings.contains' variadic +// params for arg-prep while emitting CALL bytes.contains. fn callee_variadic_param(c: *cgen, callee: *node, nfixed_out: *i32) *node = { *nfixed_out = 0; if (callee == nil) { return nil; }; - let cnm: str; - cnm.ptr = nil; cnm.len = 0; - if (callee.kind == nkind.N_IDENT) { cnm = callee.str; }; - if (callee.kind == nkind.N_DOT) { cnm = callee.str; }; - if (cnm.len == 0) { return nil; }; - let ps: *node = fnparamslookup(c, cnm); + let ps: *node = nil; + if (callee.kind == nkind.N_IDENT) { + if (callee.str.len == 0) { return nil; }; + ps = fnparamslookup(c, callee.str); + } else { if (callee.kind == nkind.N_DOT) { + if (callee.str.len == 0) { return nil; }; + let cmod: str; + cmod.ptr = nil; cmod.len = 0; + if (callee.lhs != nil) { + if (callee.lhs.kind == nkind.N_IDENT) { + cmod = callee.lhs.str; + }; + }; + ps = fnparamslookupmod(c, callee.str, cmod); + }; }; return findvariadicparam(ps, nfixed_out); }; diff --git a/selfhost/cmd/wwdump/main.combined.ww b/selfhost/cmd/wwdump/main.combined.ww index dece3f48..018cd310 100644 --- a/selfhost/cmd/wwdump/main.combined.ww +++ b/selfhost/cmd/wwdump/main.combined.ww @@ -7282,15 +7282,33 @@ fn findvariadicparam(ps: *node, nfixed_out: *i32) *node = { // callee_variadic_param — convenience wrapper: looks up the callee // by name and finds its variadic param + nfixed. Returns nil if the // callee isn't registered or has no variadic param. +// +// N_DOT routes through fnparamslookupmod with the module hint +// (callee.lhs.str) — bare fnparamslookup walks same-module-first +// (#4d) which is wrong for a cross-module N_DOT call into a module +// whose same-leaf fn has divergent variadic-vs-non-variadic shape. +// #4d explicitly deferred this re-routing; surfaced by #16 when +// strings.contains gained a variadic shape and a caller's +// bytes.contains call site picked strings.contains' variadic +// params for arg-prep while emitting CALL bytes.contains. fn callee_variadic_param(c: *cgen, callee: *node, nfixed_out: *i32) *node = { *nfixed_out = 0; if (callee == nil) { return nil; }; - let cnm: str; - cnm.ptr = nil; cnm.len = 0; - if (callee.kind == nkind.N_IDENT) { cnm = callee.str; }; - if (callee.kind == nkind.N_DOT) { cnm = callee.str; }; - if (cnm.len == 0) { return nil; }; - let ps: *node = fnparamslookup(c, cnm); + let ps: *node = nil; + if (callee.kind == nkind.N_IDENT) { + if (callee.str.len == 0) { return nil; }; + ps = fnparamslookup(c, callee.str); + } else { if (callee.kind == nkind.N_DOT) { + if (callee.str.len == 0) { return nil; }; + let cmod: str; + cmod.ptr = nil; cmod.len = 0; + if (callee.lhs != nil) { + if (callee.lhs.kind == nkind.N_IDENT) { + cmod = callee.lhs.str; + }; + }; + ps = fnparamslookupmod(c, callee.str, cmod); + }; }; return findvariadicparam(ps, nfixed_out); }; diff --git a/test/wcc/752_modparam_callee.c b/test/wcc/752_modparam_callee.c new file mode 100644 index 00000000..6ca91391 --- /dev/null +++ b/test/wcc/752_modparam_callee.c @@ -0,0 +1,285 @@ +/* + * 752_modparam_callee — sentinel for task #16 (sub-bug of #4d): + * wwstage's callee_variadic_param helper (selfhost/cmd/wcc/ + * cgenutil.ww) took an N_DOT callee but routed both arms through + * bare-leaf fnparamslookup. Post-#4d that walk is same-module-first + * then head-walk fallback — for a cross-module N_DOT call from a + * caller whose c.curmod doesn't match either side, the head-walk + * returns whichever module's same-leaf fn sits at the head of + * c.fnrets. When that head-side fn has a divergent variadic-vs- + * non-variadic shape the callee_variadic_param result fires the + * gather machinery against the wrong-module shape: arg-prep + * builds a slice for `xs: T...` while the CALL still targets the + * non-variadic module-qualified label. Label correct, ABI wrong. + * + * Surfaced by attempting strings.contains' Hare-shaped + * `(needles: (str | rune)...)` graduation while bytes.contains + * stays at `(needle: (u8 | []u8))` — caller's + * `bytes.contains(b, n)` site picked strings.contains' variadic + * params for arg-prep. + * + * Cstage carries no sister bug: cmd/w6c/cgen.c reads the callee + * fn-type from the typed `n->lhs->type` (TY_FN sig) and walks + * `callee_params->variadic` directly, module-aware via the typed + * AST. Mirror of #4d / #28 / #31 / #34: cstage sidesteps every + * bare-leaf table. + * + * Fix: route the N_DOT arm through fnparamslookupmod(c, leaf, + * callee.lhs.str). The N_IDENT arm stays on bare fnparamslookup + * (already same-module-first post-#4d). + * + * row | what it pins + * ---------------------------------------|------------------------- + * cross_module_same_leaf_variadic_vs_scalar + * | primary wedge. A.foo + * | variadic, B.foo scalar + * | same leaf. Caller in + * | a third package calls + * | B.foo with a tagged + * | arg; pre-fix wwstage + * | preps the call site as + * | variadic gather and + * | mis-marshals — rc=11. + * | Post-fix both stages + * | rc=0. + * same_module_same_leaf | non-regression: two + * | fns in the same module + * | sharing a leaf (one + * | variadic, one not). + * | mklabel seq stays + * | distinct + each call + * | dispatches to its own + * | shape. + * bare_leaf_no_collision | control: variadic fn + * | declared in only one + * | module, called via + * | bare leaf (N_IDENT) + * | from the same module. + * | Confirms the N_IDENT + * | arm still graduates + * | through bare + * | fnparamslookup + * | (post-#4d). + */ +#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; + int want_exit; + int n_files; + const char *files[8]; /* {path, src, path, src, ...} */ + const char *entry; /* file to `ww build` (relative) */ +}; + +static const struct row rows[] = { + /* Primary wedge. alpha.foo is variadic (i32...), beta.foo is + * non-variadic with a tagged-union param. Caller in `main` + * imports both and calls beta.foo with a tagged (u8|[]u8). The + * combined-source layout puts `beta` after `alpha` so beta.foo + * sits at the head of c.fnrets among non-main fns; with + * c.curmod="main" the bare-leaf walk falls through same-module + * to head-walk and picks beta.foo here — but reverse the import + * order or any other ordering perturbation flips it to alpha.foo. + * Either fnparamslookup result is wrong half the time; the + * N_DOT arm must route through fnparamslookupmod to be stable. + * Pre-#16 the head-pick happens to be the variadic side and + * arg-prep builds an `i32...` gather slice while CALL targets + * beta.foo's scalar ABI. */ + { "cross_module_same_leaf_variadic_vs_scalar", 0, 3, + { "alpha/alpha.ww", + "package alpha;\n" + "export fn foo(a: i32, xs: i32...) i32 = {\n" + "\treturn a + xs.len;\n" + "};\n", + "beta/beta.ww", + "package beta;\n" + "export fn foo(a: i32, n: (u8 | []u8)) i32 = {\n" + "\tmatch (n) {\n" + "\tcase let c: u8 => return a + (c: i32);\n" + "\tcase let s: []u8 => return a + s.len;\n" + "\t};\n" + "\treturn 0;\n" + "};\n", + "main.ww", + "package main;\n" + "import alpha;\n" + "import beta;\n" + "export fn main() i32 = {\n" + "\tlet n: (u8 | []u8) = 7u8;\n" + "\tlet r: i32 = beta.foo(10, n);\n" + "\tif (r != 17) { return 11; };\n" + "\treturn 0;\n" + "};\n" }, + "main.ww" }, + + /* Non-regression. Same module declares two fns sharing a leaf + * (one variadic, one not). The N_IDENT bare-leaf path still + * graduates via fnparamslookup's same-module-first walk (#4d); + * the N_DOT arm doesn't fire because there's no `mod.fn` use + * site. */ + { "same_module_same_leaf", 0, 1, + { "main.ww", + "package main;\n" + "fn variadic_foo(a: i32, xs: i32...) i32 = {\n" + "\treturn a + xs.len;\n" + "};\n" + "fn scalar_foo(a: i32, n: (u8 | []u8)) i32 = {\n" + "\tmatch (n) {\n" + "\tcase let c: u8 => return a + (c: i32);\n" + "\tcase let s: []u8 => return a + s.len;\n" + "\t};\n" + "\treturn 0;\n" + "};\n" + "export fn main() i32 = {\n" + "\tlet n: (u8 | []u8) = 5u8;\n" + "\tlet a: i32 = variadic_foo(1, 2, 3, 4);\n" + "\tlet b: i32 = scalar_foo(10, n);\n" + "\tif (a != 4) { return 30; };\n" + "\tif (b != 15) { return 31; };\n" + "\treturn 0;\n" + "};\n" }, + "main.ww" }, + + /* Control. A variadic fn exists in only one imported module; + * the caller calls it via N_DOT. No same-leaf collision, so + * fnparamslookupmod's same-module hint and the head-walk + * fallback agree. Pre+post fix both stages rc=0. */ + { "bare_leaf_no_collision", 0, 2, + { "alpha/alpha.ww", + "package alpha;\n" + "export fn sum(a: i32, xs: i32...) i32 = {\n" + "\tlet t: i32 = a;\n" + "\tlet i: i32 = 0;\n" + "\tfor (i < xs.len) { t += xs[i]; i += 1; };\n" + "\treturn t;\n" + "};\n", + "main.ww", + "package main;\n" + "import alpha;\n" + "export fn main() i32 = {\n" + "\tlet r: i32 = alpha.sum(1, 2, 3, 4);\n" + "\tif (r != 10) { return 40; };\n" + "\treturn 0;\n" + "};\n" }, + "main.ww" }, +}; + +static int +write_one(const char *dir, const char *rel, const char *src) +{ + char path[512]; + snprintf(path, sizeof path, "%s/%s", dir, rel); + char *slash = strchr(path + strlen(dir) + 1, '/'); + if (slash) { + *slash = '\0'; + mkdir(path, 0755); + *slash = '/'; + } + FILE *f = fopen(path, "wb"); + if (!f) return -1; + fputs(src, f); + fclose(f); + return 0; +} + +static int +run_row(const char *driver, const struct row *r, int idx, const char *tag) +{ + char tmpdir[64]; + snprintf(tmpdir, sizeof tmpdir, "/tmp/modparam_%s_%d_%d", + tag, getpid(), idx); + mkdir(tmpdir, 0755); + int fail = 0; + + for (int k = 0; k < r->n_files; k++) { + if (write_one(tmpdir, r->files[2 * k], + r->files[2 * k + 1]) != 0) { + fprintf(stderr, + "modparam_callee[%s][%s]: write %s failed\n", + tag, r->label, r->files[2 * k]); + return 1; + } + } + + char cmd[2048]; + snprintf(cmd, sizeof cmd, + "cd %s && %s build %s >/dev/null 2>&1", tmpdir, driver, r->entry); + if (runwait(cmd) != 0) { + fprintf(stderr, + "modparam_callee[%s][%s]: build failed\n", + tag, r->label); + fail++; + goto cleanup; + } + + char entry_bin[512]; + snprintf(entry_bin, sizeof entry_bin, "%s/%s", tmpdir, r->entry); + char *dot = strrchr(entry_bin, '.'); + if (dot && strcmp(dot, ".ww") == 0) *dot = '\0'; + int got = runwait(entry_bin); + if (got != r->want_exit) { + fprintf(stderr, + "modparam_callee[%s][%s]: rc=%d want=%d\n", + tag, r->label, got, r->want_exit); + fail++; + } + +cleanup: + snprintf(cmd, sizeof cmd, "rm -rf %s", tmpdir); + (void)runwait(cmd); + return fail; +} + +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 cdrv[640], wdrv[640]; + snprintf(cdrv, sizeof cdrv, "%s/ww", bin); + snprintf(wdrv, sizeof wdrv, "%s/ww_ww", bin); + int have_ww = (access(wdrv, X_OK) == 0); + + int n = (int)(sizeof rows / sizeof rows[0]); + int total = 0, fail = 0; + + for (int i = 0; i < n; i++) { + total++; + fail += run_row(cdrv, &rows[i], i, "cs"); + if (have_ww) { + total++; + fail += run_row(wdrv, &rows[i], i, "ws"); + } + } + + if (fail) { + fprintf(stderr, + "modparam_callee: %d/%d fixtures failed\n", + fail, total); + return 1; + } + printf("modparam_callee: %d/%d ok\n", total, total); + return 0; +}