From 1752305be7803ae6e5544880c922f4a3b736e04f Mon Sep 17 00:00:00 2001 From: Hojun-Cho Date: Sun, 14 Jun 2026 11:46:10 +0900 Subject: [PATCH] fmt: scandigits rejects the i32-overflowing last digit (F-A) The one-sided guard `v > 214748364` never fired for the last digit: at v==214748364 a next digit of '8'/'9' made `v*10+digit` overflow i32 and wrap negative, slipping past the signed args-index bound check at fmt.ww:703 -> OOB arg read -> SIGSEGV on any format directive carrying an over-i32 digit run (index, width or precision). Complete it to the canonical two-part pre-multiply Horner guard (MAX/10, MAX%10). Hand-rolled in signed i32, not Hare scan_sz's unsigned post-multiply wrap-check (ref/hare/strconv/stou.ha:60), which would be signed-overflow UB-class here; noted at the site. Table-driven subprocess test over all three scandigits call sites, 5 rows x both stages; reverting the guard reproduces exit=139. --- Makefile | 8 + lib/fmt/fmt.ww | 5 +- test/wcc/989_fmt_scanoverflow_run.c | 235 ++++++++++++++++++++++++++++ 3 files changed, 247 insertions(+), 1 deletion(-) create mode 100644 test/wcc/989_fmt_scanoverflow_run.c diff --git a/Makefile b/Makefile index d031737e..ef3962ec 100644 --- a/Makefile +++ b/Makefile @@ -478,6 +478,7 @@ TESTS = $(BIN)/test_smoke $(BIN)/test_lex $(BIN)/test_parse $(BIN)/test_check \ $(BIN)/test_memio_vstream_run \ $(BIN)/test_fmt_handle_run \ $(BIN)/test_fmt_mods_run \ + $(BIN)/test_fmt_scanoverflow_run \ $(BIN)/test_fmt_compositions_run \ $(BIN)/test_fmt_int_run \ $(BIN)/test_fieldfn_leaf_collide_run \ @@ -2034,6 +2035,13 @@ $(BIN)/test_fmt_mods_run: test/wcc/780_fmt_mods_run.c \ $(LIB)/libwwrt.a | $(BIN) $(CC) $(CFLAGS) -o $@ $< +$(BIN)/test_fmt_scanoverflow_run: test/wcc/989_fmt_scanoverflow_run.c \ + $(BIN)/ww $(BIN)/w6c $(BIN)/w6a $(BIN)/w6l \ + $(BIN)/ww_ww $(BIN)/w6c_ww $(BIN)/w6a_ww $(BIN)/w6l_ww \ + lib/fmt/fmt.ww \ + $(LIB)/libwwrt.a | $(BIN) + $(CC) $(CFLAGS) -o $@ $< + $(BIN)/test_fmt_compositions_run: test/wcc/781_fmt_compositions_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/lib/fmt/fmt.ww b/lib/fmt/fmt.ww index 9a0d33b8..347ce2e2 100644 --- a/lib/fmt/fmt.ww +++ b/lib/fmt/fmt.ww @@ -221,7 +221,10 @@ fn scandigits(s: str, pos: *i32) i32 = { return v; }; any = true; - if (v > 214748364) { fmtabort(); }; + // PRE-multiply guard (i32 MAX 2147483647, /10, %10): Hare's + // scan_sz uses stoz's UNSIGNED post-multiply wrap-check + // (stou.ha:60 `n < old`), UB-class here in signed i32. + if (v > 214748364 || (v == 214748364 && (c - 48u8): i32 > 7)) { fmtabort(); }; v = v * 10 + (c - 48u8): i32; *pos += 1; }; diff --git a/test/wcc/989_fmt_scanoverflow_run.c b/test/wcc/989_fmt_scanoverflow_run.c new file mode 100644 index 00000000..c0764f57 --- /dev/null +++ b/test/wcc/989_fmt_scanoverflow_run.c @@ -0,0 +1,235 @@ +/* + * 989_fmt_scanoverflow_run — drain item F-A: lib/fmt scandigits dropped + * the last-digit overflow case. + * + * scandigits (lib/fmt/fmt.ww) accumulates a format-directive digit run + * into an i32 with a Horner loop. The guard was a one-sided + * `if (v > 214748364)`, which never fires when v == 214748364 and the + * next digit is '8' or '9': v * 10 + 8/9 = 2147483648/2147483649 + * overflows i32 and wraps NEGATIVE. A negative explicit arg index then + * slips past the signed `idx >= args.len` guard in fprintf (fmt.ww:703) + * and `args[idx]` reads far out of bounds -> SIGSEGV. + * + * The fix completes the canonical Horner pre-multiply guard to the + * two-part form `v > MAX/10 || (v == MAX/10 && digit > MAX%10)` + * (MAX=2147483647, MAX/10=214748364, MAX%10=7), so the last digit is + * rejected and scandigits aborts cleanly via fmtabort (os.exit(255)). + * + * row | directive | arg | want_exit + * -----------------+--------------------+---------+---------- + * idx_2147483648 | "{2147483648}" | 42i64 | 255 (clean abort, was SIGSEGV) + * idx_2147483649 | "{2147483649}" | 42i64 | 255 (clean abort, was SIGSEGV) + * idx_9999999999 | "{9999999999}" | 42i64 | 255 (clear overflow, aborts pre+post) + * width_2147483648 | "{:2147483648}" | 42i64 | 255 (width call site, last-digit abort) + * prec_2147483647 | "{:.2147483647}" | "ok" | 7 (i32 MAX still PARSES, prints "ok") + * + * scandigits feeds three directive call sites — index (fmt.ww:690), + * width (:259) and precision (:255). idx_*, width_* and prec_* exercise + * all three; the last-digit guard must fire identically at each. + * + * The first two rows are the regression proper: on the pre-fix guard the + * program is killed by signal 11 (runwait -> -1), not 255. The valid + * boundary row proves the fix does not over-reject i32 MAX itself. + * + * Rule 10: every row runs on cstage `ww` and (when present) wwstage + * `ww_ww`; both drivers compile the same fixed lib/fmt so both must agree. + */ +#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; /* killed by a signal (e.g. SIGSEGV) */ +} + +struct row { + const char *label; + const char *src; + int want_exit; +}; + +static const struct row rows[] = { + { "idx_2147483648", + "package main;\n" + "import fmt;\n" + "import io;\n" + "export fn main() i32 = {\n" + " let buf: [64]u8;\n" + " let r: (str | io.error) = fmt.bsprintf(buf[0:64], \"{2147483648}\", 42i64);\n" + " match (r) {\n" + " case let s: str => { return 0; };\n" + " case let e: io.error => { return 1; };\n" + " };\n" + "};\n", + 255 }, + { "idx_2147483649", + "package main;\n" + "import fmt;\n" + "import io;\n" + "export fn main() i32 = {\n" + " let buf: [64]u8;\n" + " let r: (str | io.error) = fmt.bsprintf(buf[0:64], \"{2147483649}\", 42i64);\n" + " match (r) {\n" + " case let s: str => { return 0; };\n" + " case let e: io.error => { return 1; };\n" + " };\n" + "};\n", + 255 }, + { "idx_9999999999", + "package main;\n" + "import fmt;\n" + "import io;\n" + "export fn main() i32 = {\n" + " let buf: [64]u8;\n" + " let r: (str | io.error) = fmt.bsprintf(buf[0:64], \"{9999999999}\", 42i64);\n" + " match (r) {\n" + " case let s: str => { return 0; };\n" + " case let e: io.error => { return 1; };\n" + " };\n" + "};\n", + 255 }, + { "width_2147483648", + "package main;\n" + "import fmt;\n" + "import io;\n" + "export fn main() i32 = {\n" + " let buf: [64]u8;\n" + " let r: (str | io.error) = fmt.bsprintf(buf[0:64], \"{:2147483648}\", 42i64);\n" + " match (r) {\n" + " case let s: str => { return 0; };\n" + " case let e: io.error => { return 1; };\n" + " };\n" + "};\n", + 255 }, + { "prec_2147483647", + "package main;\n" + "import fmt;\n" + "import io;\n" + "export fn main() i32 = {\n" + " let buf: [64]u8;\n" + " let r: (str | io.error) = fmt.bsprintf(buf[0:64], \"{:.2147483647}\", \"ok\");\n" + " match (r) {\n" + " case let s: str => { if (s.len == 2) { return 7; }; return 8; };\n" + " case let e: io.error => { return 1; };\n" + " };\n" + "};\n", + 7 }, +}; + +static int +write_source(const char *path, const char *src) +{ + FILE *f = fopen(path, "wb"); + if (!f) return -1; + fputs(src, f); + fclose(f); + return 0; +} + +/* ww_ww writes intermediates next to the source (filed task #15); clear + * each row's .{combined.ww,s,o} + bare exe. Mirror of 780's. */ +static void +cleanup_tmp(const char *tmpdir, const char *base) +{ + char p[640]; + snprintf(p, sizeof p, "%s/%s.ww", tmpdir, base); unlink(p); + snprintf(p, sizeof p, "%s/%s.s", tmpdir, base); unlink(p); + snprintf(p, sizeof p, "%s/%s.o", tmpdir, base); unlink(p); + snprintf(p, sizeof p, "%s/%s.combined.ww", tmpdir, base); unlink(p); + snprintf(p, sizeof p, "%s/%s", tmpdir, base); unlink(p); + rmdir(tmpdir); +} + +static int +build_via_driver(const char *driver, const char *tmpdir, const char *cwd, + const char *src) +{ + char cmd[2048]; + snprintf(cmd, sizeof cmd, + "cd %s && timeout 180 %s build -I %s/lib %s 2>/dev/null", + tmpdir, driver, cwd, src); + return runwait(cmd); +} + +static int +run_row(const char *driver, const char *cwd, const struct row *r, int seq) +{ + char tmpdir[256], src[512], base[64], outbin[768]; + snprintf(tmpdir, sizeof tmpdir, "/tmp/fso_%d_d_%d", getpid(), seq); + snprintf(base, sizeof base, "main989so"); + snprintf(src, sizeof src, "%s/%s.ww", tmpdir, base); + mkdir(tmpdir, 0755); + if (write_source(src, r->src) != 0) { cleanup_tmp(tmpdir, base); return -2; } + int rc; + int br = build_via_driver(driver, tmpdir, cwd, src); + if (br == 0) { + snprintf(outbin, sizeof outbin, "%s/%s", tmpdir, base); + rc = runwait(outbin); + } else { + rc = -3; /* build failure: distinct from a run signal */ + } + cleanup_tmp(tmpdir, base); + return rc; +} + +int +main(void) +{ + const char *bin = getenv("BIN"); + if (!bin) bin = "out/bin"; + char cwd[256]; + if (getcwd(cwd, sizeof cwd) == NULL) return 1; + char absbin[512]; + if (bin[0] != '/') { + 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 n = (int)(sizeof rows / sizeof rows[0]); + int total = 0, fail = 0; + int wwpresent = (access(wdrv, X_OK) == 0); + int seq = 0; + + for (int i = 0; i < n; i++) { + total++; + int got = run_row(cdrv, cwd, &rows[i], seq++); + if (got != rows[i].want_exit) { + fprintf(stderr, + "fmt_scanoverflow[cs][%s]: exit=%d want=%d\n", + rows[i].label, got, rows[i].want_exit); + fail++; + } + if (wwpresent) { + total++; + got = run_row(wdrv, cwd, &rows[i], seq++); + if (got != rows[i].want_exit) { + fprintf(stderr, + "fmt_scanoverflow[ww][%s]: exit=%d want=%d\n", + rows[i].label, got, rows[i].want_exit); + fail++; + } + } + } + + if (!wwpresent) + fprintf(stderr, "fmt_scanoverflow: skip wwstage (no %s)\n", wdrv); + + if (fail) { + fprintf(stderr, "fmt_scanoverflow: %d/%d fixtures failed\n", + fail, total); + return 1; + } + printf("fmt_scanoverflow: %d/%d ok\n", total, total); + return 0; +}