diff --git a/.github/workflows/ci-dev.yml b/.github/workflows/ci-dev.yml index 505b90d4..4ed0d01d 100644 --- a/.github/workflows/ci-dev.yml +++ b/.github/workflows/ci-dev.yml @@ -5,6 +5,10 @@ on: pull_request: branches: - main + # Lets the -memcheck step (see the build job below) run on demand, + # from the Actions tab or `gh workflow run`, without adding its own + # several-minutes-plus cost to every ordinary push/PR. + workflow_dispatch: {} jobs: build: @@ -142,6 +146,30 @@ jobs: export FUN_STDLIB_DIR="$PWD/stdlib" mkdir -p coverage ./fun-out/bin/fun test . -cover -cover-min 80 -cover-exclude examples,scripts,editors,src/cli/main.fn,src/fls/main.fn,src/fls/server.fn,src/cli/coverage_samples,stdlib/std/sync_backend_windows.fn -cover-report lcov=coverage/lcov.info,cobertura=coverage/cobertura.xml,json=coverage/coverage.json,html=coverage/index.html,text=coverage/summary.txt | tee coverage/test-output.txt + # Every allocation across the whole suite redirected through the + # built-in memory checker (see stdlib/std/memcheck.fn): a + # double-free, use-after-free, or buffer overflow anywhere in the + # compiler, stdlib, or their own tests fails this step (leak + # reporting stays off, see Codegen.memcheck_leaks's own note - a + # test's own process is short-lived by design, so "still + # allocated when it exits a moment later anyway" isn't a bug). + # One platform only, same reasoning as coverage just above: a + # double-free/UAF/overflow is a real bug regardless of which + # platform's build happens to hit it, so the other two lanes + # would only pay the extra time for no extra coverage. Manual- + # trigger only (`workflow_dispatch`, the GitHub Actions + # equivalent of GitLab's manual job): the guard-page allocator + # is markedly slower than a real malloc (every allocation costs + # a real mmap/mprotect syscall pair), enough that this step alone + # can run well past 30 minutes under load - too costly to add to + # every ordinary push/PR, but still there on demand from the + # Actions tab or `gh workflow run ci-dev.yml`. + - name: Run the test suite on ${{ matrix.lane }} under -memcheck + if: matrix.os == 'ubuntu-24.04' && github.event_name == 'workflow_dispatch' + timeout-minutes: 60 + run: | + export FUN_STDLIB_DIR="$PWD/stdlib" + ./fun-out/bin/fun test . -memcheck - name: Summarize coverage if: always() && matrix.os == 'ubuntu-24.04' run: | diff --git a/docs/language.md b/docs/language.md index c40bf597..1ffa2006 100644 --- a/docs/language.md +++ b/docs/language.md @@ -1076,6 +1076,22 @@ fun main() { } ``` +### Ternary Conditional + +`cond ? then : else` is a conditional expression, not a statement - it +produces a value, for use anywhere an expression can go: + +```fun +fun main() { + num x = 5; + str size = x > 10 ? "big" : x > 0 ? "small" : "non-positive"; +} +``` + +Both branches must share a type (the same rule an array literal's +elements already follow); the condition must be a `flag`. `?`/`:` chain +right-associatively, so `a ? b : c ? d : e` reads as `a ? b : (c ? d : e)`. + ### For - Array: `for item : arr { ... }` diff --git a/docs/tooling.md b/docs/tooling.md index de74bcb0..2661558e 100644 --- a/docs/tooling.md +++ b/docs/tooling.md @@ -182,6 +182,53 @@ test: path: cobertura.xml ``` +## Memory Checker + +`-memcheck` redirects every `malloc`/`calloc`/`realloc`/`free` in the +compiled program through a built-in, native-Fun checker, catching leaks, +double-frees, use-after-free, and buffer overflows, with no external +sanitizer and no change to your own source: + +```text +fun -in file.fn -memcheck +fun test [] -memcheck +``` + +- Each allocation gets its own guarded page(s): freeing one revokes + access to the whole span forever rather than returning it to the + system, so a later read or write through it faults immediately and + deterministically, naming the real `.fn` file and line (both where it + was allocated and, for a double-free or use-after-free, where it was + first freed): + + ```text + [memcheck] use-after-free: allocation #0 (16 bytes), allocated at main.fn:5, freed at main.fn:8 + [memcheck] double-free: allocation #0, 16 bytes, allocated at main.fn:5, already freed at main.fn:6, double-freed at main.fn:7 + [memcheck] buffer overflow: past the end of allocation #0 (16 bytes), allocated at main.fn:5 + ``` + + A fault or a double-free aborts the process immediately (a nonzero, + CI-visible exit). +- Leak reporting (every allocation still live when the process exits) is + **off by default**, including under `-test`: a short-lived process - + every CLI invocation this compiler itself makes, and every `-test` + binary - never has to free a transient allocation before it exits a + moment later anyway, so treating that as a leak would flag ordinary, + harmless code. Pass `-memcheck-leaks` alongside `-memcheck` to turn + it back on for a program that's meant to run long enough that "still + allocated when it finally exits" is a real bug (a long-running server + or daemon): + + ```text + fun -in server.fn -memcheck -memcheck-leaks + ``` + + ```text + [memcheck] leak: allocation #0, 16 bytes, allocated at main.fn:5, never freed + ``` +- Works with any C compiler this project already supports (`clang`, + `gcc`, `cl.exe`), on Windows, macOS and Linux. + ## Fuzzing ```fun @@ -393,11 +440,12 @@ plain text in the GitHub UI. ## CLI ```text -fun -in [-out ] [-no-exec] [-outf] [-ast] [-g] [-warn-unused] [-warn-unused-lenient] [-D name=value] [-test] [-fuzz] [-fuzz-target ] [-help] [-version] [-- ] +fun -in [-out ] [-no-exec] [-outf] [-ast] [-g] [-warn-unused] [-warn-unused-lenient] [-D name=value] [-test] [-fuzz] [-fuzz-target ] [-memcheck] [-memcheck-leaks] [-help] [-version] [-- ] fun test (shorthand for `fun -in -test`) fun test [] (runs every `test` block under , default '.'; aggregate summary) fun test [] -cover [-cover-report ] [-cover-min ] [-cover-exclude ] (measures code coverage, see Code Coverage) +fun test [] -memcheck (runs under the memory checker, see Memory Checker) fun fuzz [] (shorthand for `fun -in -fuzz [-fuzz-target ]`) fun fuzz [] (runs every `fuzz` target under for FUN_FUZZ_DEFAULT_SECONDS each, default '.'/30s) fun lint [] [-summary] (reports every warning under , a file or directory, default '.') diff --git a/src/cli/driver.fn b/src/cli/driver.fn index f15cdd8d..c371f15a 100644 --- a/src/cli/driver.fn +++ b/src/cli/driver.fn @@ -75,6 +75,11 @@ pub compound Options { // Redirect every allocation through `std.memcheck`'s own page-guarded // allocator, catching leaks, double-frees, and use-after-free. flag memcheck; + // Keeps the leak sweep's own forced-nonzero-exit live under + // `memcheck`, for a program meant to run long enough that "still + // allocated when it exits" is a real bug - see + // `Codegen.memcheck_leaks`'s own note on why that's not the default. + flag memcheck_leaks; // Print the parsed AST before compiling. flag show_ast; // Rewrite the input in place, and with `fmt_all` its local imports too. @@ -154,6 +159,7 @@ fun _options_new() Options { o.warn_unused = false; o.warn_unused_lenient = false; o.memcheck = false; + o.memcheck_leaks = false; o.show_ast = false; o.fmt = false; o.fmt_all = false; @@ -319,6 +325,7 @@ pub fun parse_args(str[] argv) Options { o.warn_unused = true; } o.memcheck = parsed.has("memcheck"); + o.memcheck_leaks = parsed.has("memcheck-leaks"); o.show_ast = parsed.has("ast"); o.fmt = parsed.has("fmt"); o.fmt_all = parsed.has("fmt-all"); @@ -529,9 +536,10 @@ pub fun compile_to_c_mode( flag debug_info, flag warn_unused_lenient, flag prune_unreachable = true, - flag memcheck = false + flag memcheck = false, + flag memcheck_leaks = false ) Result { - ret compile_to_c_cover(input, out_path, defines, test_mode, warn_unused, fuzz_mode, fuzz_target, debug_info, warn_unused_lenient, nil, prune_unreachable, memcheck); + ret compile_to_c_cover(input, out_path, defines, test_mode, warn_unused, fuzz_mode, fuzz_target, debug_info, warn_unused_lenient, nil, prune_unreachable, memcheck, memcheck_leaks); } // `compile_to_c_mode`, optionally instrumented for coverage @@ -553,7 +561,8 @@ pub fun compile_to_c_cover( flag warn_unused_lenient, CoverPlan* cover, flag prune_unreachable = true, - flag memcheck = false + flag memcheck = false, + flag memcheck_leaks = false ) Result { let cc = _resolve_c_compiler_cached(); ret lower_to_c( @@ -573,7 +582,8 @@ pub fun compile_to_c_cover( host_arch_name(), true, false, - memcheck + memcheck, + memcheck_leaks ); } @@ -1327,6 +1337,7 @@ pub fun help_text() str { ` fun test [] runs every test block found under (default '.') ` fun test [] -cover [-cover-report lcov|cobertura|json|html|text[=file],...] [-cover-min ] [-cover-exclude ] ` measures code coverage while running the tests + ` fun test [] -memcheck runs the tests under the built-in memory checker (leaks, double-frees, use-after-free) ` fun fuzz [] builds and runs one fuzz block, open-ended ` fun fuzz [] runs every fuzz target under , briefly bounded ` fun lint [] [-summary] reports every warning under (a file or directory, default '.'), then totals; exits 1 if any @@ -1350,6 +1361,8 @@ pub fun help_text() str { ` -warn-unused-lenient Like -warn-unused, but keeps reporting them past an unrelated type error. ` -D name=value Inject a constant the program can read. ` -test Run -in's own test blocks, in place of a plain compile. + ` -memcheck Redirect every allocation through the built-in memory checker, catching leaks, double-frees, and use-after-free. + ` -memcheck-leaks With -memcheck, also fail on an allocation still live when the program exits (for a long-running program; off by default since a short-lived one never frees everything before exiting anyway). ` -fuzz Build and run -in's own fuzz target, in place of a plain compile. ` -fuzz-target Which target, when -in declares more than one. ` -fmt Format the input file in place. @@ -1792,6 +1805,46 @@ test "a plain run has coverage off, and bad coverage flags are rejected" { assert bad_min.has_error, "expected a minimum over 100 to be rejected"; } +test "'test -memcheck' is read, off by default" { + Vec a; + a.init(0); + defer a.free(); + a.push("test"); + let plain = parse_args(_argv(&a)); + defer plain.free(); + assert !plain.memcheck, "expected memcheck off by default"; + + Vec b; + b.init(0); + defer b.free(); + b.push("test"); + b.push("-memcheck"); + let on = parse_args(_argv(&b)); + defer on.free(); + assert on.memcheck, "expected -memcheck to be read on 'test' the same as on a plain compile"; +} + +test "'-memcheck-leaks' is read, off by default" { + Vec a; + a.init(0); + defer a.free(); + a.push("test"); + a.push("-memcheck"); + let plain = parse_args(_argv(&a)); + defer plain.free(); + assert !plain.memcheck_leaks, "expected memcheck_leaks off by default"; + + Vec b; + b.init(0); + defer b.free(); + b.push("test"); + b.push("-memcheck"); + b.push("-memcheck-leaks"); + let on = parse_args(_argv(&b)); + defer on.free(); + assert on.memcheck_leaks, "expected -memcheck-leaks to be read"; +} + test "percentages read as tenths of a percent" { assert parse_percent_tenths("80") == 800, "expected a whole number"; assert parse_percent_tenths("87.5") == 875, "expected one decimal"; @@ -2484,7 +2537,9 @@ pub fun run_tests( flag warn_unused_lenient, flag keep_c, Vec* extra_defines, - CoverOptions* cover + CoverOptions* cover, + flag memcheck = false, + flag memcheck_leaks = false ) num { Vec defines = resolved_defines(extra_defines); defer defines.free(); @@ -2506,6 +2561,18 @@ pub fun run_tests( if cover.enabled { plan_ptr = &plan; } + // Same reasoning as `-memcheck`'s own single-file dispatch in main.fn: + // `-g` for real `#line` directives (meaningful names in a leak/ + // double-free/use-after-free report), reachability pruning off since + // `std.memcheck`'s own allocator/handler/leak-sweep are only ever + // "called" through codegen's own macro redirect, invisible to a pass + // that runs before that macro exists. + flag mc_debug_info = debug_info; + flag prune_unreachable = true; + if memcheck { + mc_debug_info = true; + prune_unreachable = false; + } let lowered = compile_to_c_cover( input, c_path, @@ -2514,9 +2581,12 @@ pub fun run_tests( warn_unused, false, "", - debug_info, + mc_debug_info, warn_unused_lenient, - plan_ptr + plan_ptr, + prune_unreachable, + memcheck, + memcheck_leaks ); if lowered.is_err() { writeln_to(Sink.Stderr, format("[Error]\n{str}", lowered.unwrap_err())); @@ -2528,7 +2598,7 @@ pub fun run_tests( } str exe = default_exe_path(input, ".test"); - let cc_code = compile_c_to_exe(c_path, exe, debug_info); + let cc_code = compile_c_to_exe(c_path, exe, mc_debug_info); if !keep_c { remove_file(c_path); } @@ -2663,7 +2733,9 @@ fun _run_tests_captured( flag warn_unused, flag warn_unused_lenient, Vec* extra_defines, - CoverOptions* cover + CoverOptions* cover, + flag memcheck = false, + flag memcheck_leaks = false ) SuiteTestResult { SuiteTestResult bad; bad.exit_code = 1; @@ -2684,6 +2756,13 @@ fun _run_tests_captured( if cover.enabled { plan_ptr = &plan; } + // See `run_tests`'s own identical note. + flag mc_debug_info = debug_info; + flag prune_unreachable = true; + if memcheck { + mc_debug_info = true; + prune_unreachable = false; + } let t_lower = _wall_ms(); let lowered = compile_to_c_cover( input, @@ -2693,9 +2772,12 @@ fun _run_tests_captured( warn_unused, false, "", - debug_info, + mc_debug_info, warn_unused_lenient, - plan_ptr + plan_ptr, + prune_unreachable, + memcheck, + memcheck_leaks ); let lower_ms = _wall_ms() - t_lower; bad.lower_ms = lower_ms; @@ -2706,7 +2788,7 @@ fun _run_tests_captured( str exe = default_exe_path(input, format(".w{num}.test", compile_tag)); let t_cc = _wall_ms(); - let cc_code = compile_c_to_exe(c_path, exe, debug_info); + let cc_code = compile_c_to_exe(c_path, exe, mc_debug_info); let cc_ms = _wall_ms() - t_cc; bad.cc_ms = cc_ms; remove_file(c_path); @@ -2775,6 +2857,8 @@ compound _TestSuiteWorkerCtx { num total_lower_ms; num total_cc_ms; num total_run_ms; + flag memcheck; + flag memcheck_leaks; } // A run_test_suite worker: claims files one at a time from the shared @@ -2814,7 +2898,9 @@ fun _test_suite_worker(raw* arg) raw* { ctx.warn_unused, ctx.warn_unused_lenient, ctx.extra_defines, - ctx.cover + ctx.cover, + ctx.memcheck, + ctx.memcheck_leaks ); ctx.lock.lock(); @@ -2912,7 +2998,9 @@ pub fun run_test_suite( flag warn_unused, flag warn_unused_lenient, Vec* extra_defines, - CoverOptions* cover + CoverOptions* cover, + flag memcheck = false, + flag memcheck_leaks = false ) num { str dir = root; if equals(dir, "") { @@ -2972,6 +3060,8 @@ pub fun run_test_suite( ctx.warn_unused = warn_unused; ctx.warn_unused_lenient = warn_unused_lenient; ctx.extra_defines = extra_defines; + ctx.memcheck = memcheck; + ctx.memcheck_leaks = memcheck_leaks; ctx.lock = mutex_new(); ctx.passed = 0; ctx.failed = 0; @@ -4079,6 +4169,97 @@ sequential test "running one file's tests returns their result, and can stop at assert run_tests(path_join(dir, "bad.fn"), &args, false, false, "", false, false, false, &defines, &cover) == 1, "expected a file that does not compile to fail"; } +test "run_tests under -memcheck ignores an ordinary leak by default, but catches a real double-free" { + Vec defines; + defines.init(0); + defer defines.free(); + Vec args; + args.init(0); + defer args.free(); + CoverOptions cover = _cover_options_new(); + defer cover.formats.free(); + defer cover.paths.free(); + defer cover.exclude.free(); + + str dir = _scratch_project("memcheck_test"); + str leaky = path_join(dir, "leaky.fn"); + write_string( + leaky, + `use std.c.mem; + `test "leaks" { + ` raw* p = malloc(16); + `} +); + // Off by default: a `-test` binary's own process is short-lived, so + // "still allocated when it exits a moment later anyway" isn't a bug + // (see `Codegen.memcheck_leaks`'s own note) - this is exactly the + // false-positive flood that made the leak sweep unusable across a + // real test corpus. + assert run_tests(leaky, &args, false, false, "", false, false, false, &defines, &cover, true) == 0, "expected an ordinary leak not to fail the test under plain -memcheck"; + // Opting into `-memcheck-leaks` turns it back on, for a caller that + // genuinely wants it (testing a long-running program's own body). + assert run_tests(leaky, &args, false, false, "", false, false, false, &defines, &cover, true, true) != 0, "expected -memcheck-leaks to still catch a real leak"; + + str clean = path_join(dir, "clean.fn"); + write_string( + clean, + `use std.c.mem; + `test "frees what it allocates" { + ` raw* p = malloc(16); + ` free(p); + `} +); + assert run_tests(clean, &args, false, false, "", false, false, false, &defines, &cover, true, true) == 0, "expected a clean allocation/free pair to pass even with -memcheck-leaks on"; + + str doubled = path_join(dir, "doubled.fn"); + write_string( + doubled, + `use std.c.mem; + `test "double-frees" { + ` raw* p = malloc(16); + ` free(p); + ` free(p); + `} +); + // A double-free is never a matter of process lifetime, so it's + // caught unconditionally, with no opt-in needed. + assert run_tests(doubled, &args, false, false, "", false, false, false, &defines, &cover, true) != 0, "expected a real double-free to fail the test under plain -memcheck"; +} + +// `-memcheck` auto-injects `use std.memcheck;` into whatever is being +// compiled, which depends on std.c.mem/std.c.string/std.sync alone +// (see memcheck.fn's own note on staying off std.io/std.string on +// purpose) - but before that dependency was trimmed down, and before +// `lower_to_c`'s own `_stdlib_canonical_path` routed a stdlib entry +// file through the identical resolution a real `use std.x` import of +// it already goes through, compiling one of `std.memcheck`'s own +// (then-wider) dependencies directly under `-memcheck` reached that +// exact file a second time, transitively, through the import just +// injected, and double-merged its declarations. +test "compiling a std.memcheck dependency directly under -memcheck does not double-merge it" { + Vec defines; + defines.init(0); + defer defines.free(); + str out = default_c_path("stdlib/std/io.fn", ".memcheck_selfimport_test.c"); + defer remove_file(out); + let lowered = compile_to_c_mode( + "stdlib/std/io.fn", + out, + &defines, + false, + false, + false, + "", + false, + false, + true, + true + ); + if lowered.is_err() { + panic(format("expected std.io to compile cleanly under -memcheck, found: {str}", lowered.unwrap_err())); + } +} + test "lint reports findings and gives a nonzero exit for them" { str dir = _scratch_project("lint"); write_string(path_join(dir, "a.fn"), "fun main() {\n num unused = 1;\n}\n"); diff --git a/src/cli/lower.fn b/src/cli/lower.fn index 8ce5ded2..c1a42040 100644 --- a/src/cli/lower.fn +++ b/src/cli/lower.fn @@ -28,6 +28,7 @@ use std.option; use std.result; use std.io; use std.fs; +use std.path; // Where a coverage-instrumented test binary writes its counts. `nil` // means no instrumentation. @@ -118,6 +119,45 @@ pub fun drop_foreign_tests(Vec* nodes, str entry) { // into an immediate, real compile error instead - every other caller // (the native/MSVC/emcc CLI paths, which never hit this problem) // stays `false`. +// `path`, rewritten the same way a real `use std.x` import of the +// identical file would resolve it - `stdlib_root(path)` joined onto +// whatever comes after its own "std/" segment - or "" when `path` +// isn't under a "std/" segment at all. See `lower_to_c`'s own +// `memcheck` branch for why this matters: compiling a standard +// library file directly under `-memcheck` can reach that exact file a +// second time, transitively, through the import `-memcheck` itself +// injects (`use std.memcheck;` pulls in `std.io`, which pulls in +// `std.string`, and so on) - unless both routes to it resolve to the +// identical path string, the merge's own dedup (keyed on that string) +// never recognizes them as one file and double-merges its +// declarations. Scoped to a whole path segment ("std/" at the very +// start, or right after a '/'), not just the substring, so a project's +// own file that merely contains "std" somewhere in its name is left +// alone. +fun _stdlib_canonical_path(str path) str { + str slashed = replace_char(path, '\\', '/'); + num seg_start = -1; + if starts_with(slashed, "std/") { + seg_start = 0; + } else { + num idx = index_of_str(slashed, "/std/"); + if idx >= 0 { + seg_start = idx + 1; + } + } + if seg_start < 0 { + free(slashed); + ret ""; + } + str suffix = substr(slashed, seg_start, len(slashed) - seg_start); + free(slashed); + str root = stdlib_root(path); + str canonical = path_join(root, suffix); + free(root); + free(suffix); + ret canonical; +} + pub fun lower_to_c( str input, str out_path, @@ -135,9 +175,12 @@ pub fun lower_to_c( str host_arch, flag parallel_preload, flag reject_concurrency, - flag memcheck = false + flag memcheck = false, + flag memcheck_leaks = false ) Result { ProgramMergeResult merged; + str entry_path = input; + flag owns_entry_path = false; if memcheck { // `-memcheck` redirects every allocation through `std.memcheck`'s // own tracking allocator (see `Codegen.memcheck`'s own note) - the @@ -146,16 +189,42 @@ pub fun lower_to_c( // file to add that import itself. Prepending it to the live source // text (rather than splicing a node in after parsing) lets the // normal import-resolution pass pick it up like any other import. + // + // `entry_path` below (not the raw `input` the caller spelled) is + // what actually goes into `parse_program_with_source`: compiling + // one of `std.memcheck`'s own dependencies directly reaches that + // exact same file a second time through the import just injected + // (`use std.memcheck;` -> `use std.io;`, say), and unless BOTH + // routes to it resolve to the identical path string, the merge's + // own dedup (keyed on that string) never recognizes them as one + // file, double-merging its declarations. See + // `_stdlib_canonical_path`'s own doc comment for the resolution + // this applies (never a general "make any path absolute" + // primitive - the bootstrap compiler used for stage 1 has no way + // to emit a brand-new one of those yet). + str canonical = _stdlib_canonical_path(input); + if !equals(canonical, "") { + entry_path = canonical; + owns_entry_path = true; + } str original = read_to_string(input); str prepended = format("use std.memcheck;\n{str}", original); free(original); - merged = parse_program_with_source(input, prepended, parallel_preload)!; + merged = parse_program_with_source(entry_path, prepended, parallel_preload)!; free(prepended); } else { merged = parse_program_with_imports(input, parallel_preload)!; } if test_mode { - drop_foreign_tests(&merged.nodes, input); + // `entry_path` (whatever the real parse above actually used, not + // always the caller's own `input`) - see its own note just above: + // every node `drop_foreign_tests` looks at is stamped with that + // same path, so comparing against anything else would read every + // one of this file's own tests as "foreign" and drop them all. + drop_foreign_tests(&merged.nodes, entry_path); + } + if owns_entry_path { + free(entry_path); } if reject_concurrency && uses_concurrency_in(merged.nodes, test_mode, fuzz_mode) { ret .Err( @@ -270,6 +339,7 @@ pub fun lower_to_c( cg.prune_unreachable = prune_unreachable; cg.debug_info = debug_info; cg.memcheck = memcheck; + cg.memcheck_leaks = memcheck_leaks; if cover != nil { cg.coverage = true; cg.cover_out = cover.out_path; diff --git a/src/cli/main.fn b/src/cli/main.fn index ddacac4b..5b010b73 100644 --- a/src/cli/main.fn +++ b/src/cli/main.fn @@ -94,9 +94,9 @@ fun main(str[] args) num { if equals(root, "") { root = "."; } - ret run_test_suite(root, opts.debug_info, opts.warn_unused, opts.warn_unused_lenient, &opts.defines, &opts.cover); + ret run_test_suite(root, opts.debug_info, opts.warn_unused, opts.warn_unused_lenient, &opts.defines, &opts.cover, opts.memcheck, opts.memcheck_leaks); } - ret run_tests(opts.input, &opts.program_args, opts.debug_info, opts.no_exec, opts.output, opts.warn_unused, opts.warn_unused_lenient, opts.keep_c, &opts.defines, &opts.cover); + ret run_tests(opts.input, &opts.program_args, opts.debug_info, opts.no_exec, opts.output, opts.warn_unused, opts.warn_unused_lenient, opts.keep_c, &opts.defines, &opts.cover, opts.memcheck, opts.memcheck_leaks); } if equals(opts.subcommand, "fuzz") { // No path, or a path that names a directory rather than a `.fn` @@ -119,7 +119,7 @@ fun main(str[] args) num { // the `test`/`fuzz` subcommand spellings above. if opts.test_flag { apply_manifest_coverage(&opts.cover); - ret run_tests(opts.input, &opts.program_args, opts.debug_info, opts.no_exec, opts.output, opts.warn_unused, opts.warn_unused_lenient, opts.keep_c, &opts.defines, &opts.cover); + ret run_tests(opts.input, &opts.program_args, opts.debug_info, opts.no_exec, opts.output, opts.warn_unused, opts.warn_unused_lenient, opts.keep_c, &opts.defines, &opts.cover, opts.memcheck, opts.memcheck_leaks); } if opts.fuzz_flag { ret run_fuzz(opts.input, opts.fuzz_target, &opts.program_args, opts.debug_info, opts.no_exec, opts.output, opts.warn_unused, opts.warn_unused_lenient, opts.keep_c, &opts.defines); @@ -213,7 +213,8 @@ fun main(str[] args) num { mc_debug_info, opts.warn_unused_lenient, !opts.memcheck, - opts.memcheck + opts.memcheck, + opts.memcheck_leaks ); if lowered.is_err() { ret _fail(lowered.unwrap_err()); diff --git a/src/codegen/codegen.fn b/src/codegen/codegen.fn index 570e8f85..9d3c1f09 100644 --- a/src/codegen/codegen.fn +++ b/src/codegen/codegen.fn @@ -601,6 +601,16 @@ pub compound Codegen { // auto-injects `use std.memcheck;` so no user `.fn` source needs to // change at all. flag memcheck; + // `-memcheck-leaks`: lets the leak sweep's own forced-nonzero-exit + // stay live for a build under `self.memcheck`. Off by default: a + // leak is only real when the process keeps running after it happens + // (a long-lived server, a daemon) - for an ordinary short-lived + // program (every CLI invocation this compiler itself makes, and + // every `-test` binary), "still allocated when it exits a moment + // later anyway" describes virtually every transient allocation ever + // made, not a bug. See `emit_main_thread_wrapper`'s and + // `emit_test_runner`'s own `__fun_memcheck_disable_leak_sweep` calls. + flag memcheck_leaks; // How many enclosing expressions may run a sub-expression zero times or // more than once (the right side of `&&`/`||`, a loop condition, an // `elif` condition). A call-result receiver evaluated there cannot be @@ -715,6 +725,7 @@ pub fun codegen_new(CodegenState* state) Codegen { g.cover_id_files.init(64); g.cover_id_lines.init(64); g.memcheck = false; + g.memcheck_leaks = false; g.cond_eval_depth = 0; g.cover_skip_files.init(8); g.cover_skip_from.init(8); @@ -5511,7 +5522,7 @@ impl Codegen { // without this, a debugger stepping into `expr?`/`expr!` under // MSVC showed the wrong location for the hoisted temp-and-guard, // inheriting whatever directive was last active before it. - if self.debug_info { + if self.debug_info && self.state.pending_line_directive_is_valid { try_decl_sb.append(line_directive_text(self.state.pending_line_directive)); } num try_indent_ii = 0; @@ -6793,7 +6804,7 @@ impl Codegen { // hoisted vararg (MSVC's own dialect, no GNU statement // expressions) lands on the right line instead of whatever // was last active before it. - if self.debug_info { + if self.debug_info && self.state.pending_line_directive_is_valid { va_decl_sb.append(line_directive_text(self.state.pending_line_directive)); } num va_indent_ii = 0; @@ -7616,11 +7627,21 @@ impl Codegen { // receiver - which can trigger an entirely separate generic // instantiation, rendered into its OWN scratch state and restored // before control returns here - never contaminates it, unlike a - // shared field would. It always holds a real value by this point: + // shared field would. Usually already real by this point - // `emit_statement` sets it before dispatching into any statement's - // own expression emission, and nothing between there and here - // calls `set_pending_line_directive` again on `self.state` itself. - if self.debug_info { + // own expression emission, and nothing between there and here calls + // `set_pending_line_directive` again on `self.state` itself - except + // when the statement node reaching `emit_statement` has no position + // of its own (`node.has_pos` false, left unset entirely rather than + // zeroed to a real one), which `emit_statement` already treats as + // "nothing to flush" by skipping the call outright. + // `pending_line_directive_is_valid` (not `has_pending_line_directive`, + // which `write_indent`'s own flush already cleared for this exact + // statement by the time a hoisted temp like this one needs to + // write a second, mid-statement `#line` of its own) is what tells + // "never set at all" apart from "already flushed once, but still + // the correct value." + if self.debug_info && self.state.pending_line_directive_is_valid { recv_tmp_stmt_sb.append(line_directive_text(self.state.pending_line_directive)); } num recv_tmp_ii = 0; @@ -16397,6 +16418,12 @@ impl Codegen { // preamble - `std.memcheck` itself never branches on platform. if self.memcheck { self.state.write(`#ifdef _WIN32 + `#ifndef WIN32_LEAN_AND_MEAN + `#define WIN32_LEAN_AND_MEAN + `#endif + `#ifndef NOGDI + `#define NOGDI + `#endif `#include `#else `#include @@ -16404,18 +16431,18 @@ impl Codegen { `#include `#include `#endif - `static void* __fun_real_malloc(long long size) { return malloc((size_t)size); } + `static void* __fun_real_malloc(int64_t size) { return malloc((size_t)size); } `static void __fun_real_free(void* p) { free(p); } - `static void* __fun_real_realloc(void* p, long long size) { return realloc(p, (size_t)size); } - `typedef void (*__fun_mc_handler_t)(long long, void*, void*); + `static void* __fun_real_realloc(void* p, int64_t size) { return realloc(p, (size_t)size); } + `typedef void (*__fun_mc_handler_t)(int64_t, void*, void*); `static __fun_mc_handler_t __fun_mc_user_handler = 0; `#ifdef _WIN32 - `static long long __fun_mc_page_size(void) { SYSTEM_INFO si; GetSystemInfo(&si); return (long long)si.dwPageSize; } - `static void* __fun_mc_guard_alloc(long long data_size, long long* out_total_size) { - ` long long ps = __fun_mc_page_size(); - ` long long data_pages = (data_size + ps - 1) / ps; + `static int64_t __fun_mc_page_size(void) { SYSTEM_INFO si; GetSystemInfo(&si); return (int64_t)si.dwPageSize; } + `static void* __fun_mc_guard_alloc(int64_t data_size, int64_t* out_total_size) { + ` int64_t ps = __fun_mc_page_size(); + ` int64_t data_pages = (data_size + ps - 1) / ps; ` if (data_pages < 1) data_pages = 1; - ` long long total = (data_pages + 1) * ps; + ` int64_t total = (data_pages + 1) * ps; ` void* base = VirtualAlloc(NULL, (SIZE_T)total, MEM_RESERVE | MEM_COMMIT, PAGE_READWRITE); ` if (!base) return 0; ` void* guard = (char*)base + data_pages * ps; @@ -16424,7 +16451,7 @@ impl Codegen { ` *out_total_size = total; ` return base; `} - `static long long __fun_mc_guard_protect_all(void* base, long long total_size) { + `static int64_t __fun_mc_guard_protect_all(void* base, int64_t total_size) { ` DWORD old; ` return VirtualProtect(base, (SIZE_T)total_size, PAGE_NOACCESS, &old) ? 0 : 1; `} @@ -16435,18 +16462,18 @@ impl Codegen { ` } ` return EXCEPTION_CONTINUE_SEARCH; `} - `static long long __fun_mc_register_handler(void* handler) { + `static int64_t __fun_mc_register_handler(void* handler) { ` __fun_mc_user_handler = (__fun_mc_handler_t)handler; ` AddVectoredExceptionHandler(1, __fun_mc_veh); ` return 0; `} `#else - `static long long __fun_mc_page_size(void) { return (long long)sysconf(_SC_PAGESIZE); } - `static void* __fun_mc_guard_alloc(long long data_size, long long* out_total_size) { - ` long long ps = __fun_mc_page_size(); - ` long long data_pages = (data_size + ps - 1) / ps; + `static int64_t __fun_mc_page_size(void) { return (int64_t)sysconf(_SC_PAGESIZE); } + `static void* __fun_mc_guard_alloc(int64_t data_size, int64_t* out_total_size) { + ` int64_t ps = __fun_mc_page_size(); + ` int64_t data_pages = (data_size + ps - 1) / ps; ` if (data_pages < 1) data_pages = 1; - ` long long total = (data_pages + 1) * ps; + ` int64_t total = (data_pages + 1) * ps; ` void* base = mmap(0, (size_t)total, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); ` if (base == MAP_FAILED) return 0; ` void* guard = (char*)base + data_pages * ps; @@ -16454,38 +16481,38 @@ impl Codegen { ` *out_total_size = total; ` return base; `} - `static long long __fun_mc_guard_protect_all(void* base, long long total_size) { + `static int64_t __fun_mc_guard_protect_all(void* base, int64_t total_size) { ` return mprotect(base, (size_t)total_size, PROT_NONE) == 0 ? 0 : 1; `} `static void __fun_mc_trampoline(int sig, siginfo_t* info, void* ctx) { ` if (__fun_mc_user_handler) { - ` __fun_mc_user_handler((long long)sig, info ? info->si_addr : 0, ctx); + ` __fun_mc_user_handler((int64_t)sig, info ? info->si_addr : 0, ctx); ` } `} - `static long long __fun_mc_register_handler(void* handler) { + `static int64_t __fun_mc_register_handler(void* handler) { ` __fun_mc_user_handler = (__fun_mc_handler_t)handler; ` struct sigaction sa; ` memset(&sa, 0, sizeof(sa)); ` sa.sa_sigaction = __fun_mc_trampoline; ` sa.sa_flags = SA_SIGINFO; ` sigemptyset(&sa.sa_mask); - ` long long rc1 = (long long)sigaction(SIGSEGV, &sa, 0); + ` int64_t rc1 = (int64_t)sigaction(SIGSEGV, &sa, 0); ` /* Accessing an mprotect'd PROT_NONE page raises SIGBUS rather ` than SIGSEGV on Darwin/BSD-derived kernels for some fault ` paths (Linux is consistently SIGSEGV) - both are registered ` with the same handler so a guarded-page fault is always ` caught regardless of which one the kernel actually raises. */ - ` long long rc2 = (long long)sigaction(SIGBUS, &sa, 0); + ` int64_t rc2 = (int64_t)sigaction(SIGBUS, &sa, 0); ` return rc1 != 0 ? rc1 : rc2; `} `#endif - `static long long __fun_mc_ptr_in_range(void* addr, void* base, long long size) { + `static int64_t __fun_mc_ptr_in_range(void* addr, void* base, int64_t size) { ` return ((char*)addr >= (char*)base && (char*)addr < (char*)base + size) ? 1 : 0; `} - `extern void* __fun_memcheck_malloc(long long size, char* file, long long line); - `extern void* __fun_memcheck_calloc(long long count, long long size, char* file, long long line); - `extern void* __fun_memcheck_realloc(void* ptr, long long size, char* file, long long line); - `extern void __fun_memcheck_free(void* ptr, char* file, long long line); + `extern void* __fun_memcheck_malloc(int64_t size, char* file, int64_t line); + `extern void* __fun_memcheck_calloc(int64_t count, int64_t size, char* file, int64_t line); + `extern void* __fun_memcheck_realloc(void* ptr, int64_t size, char* file, int64_t line); + `extern void __fun_memcheck_free(void* ptr, char* file, int64_t line); `#define malloc(sz) __fun_memcheck_malloc((sz), __FILE__, __LINE__) `#define calloc(n, sz) __fun_memcheck_calloc((n), (sz), __FILE__, __LINE__) `#define realloc(p, sz) __fun_memcheck_realloc((p), (sz), __FILE__, __LINE__) @@ -18863,6 +18890,10 @@ impl Codegen { // no header of its own. emit_main_thread_wrapper() Option { str real_name = self.effective_function_name("main"); + str mc_leak_suppress = ""; + if self.memcheck && !self.memcheck_leaks { + mc_leak_suppress = " __fun_memcheck_disable_leak_sweep();\n"; + } if self.emscripten_mode { // A browser's own main (document) thread must never block // synchronously waiting on another thread - `pthread_join` from @@ -18874,7 +18905,13 @@ impl Codegen { // time instead (`-sSTACK_SIZE`, see `_append_compile_args` in // driver.fn) - `main` just calls straight through, no worker // thread, no wrapper needed at all for this target. - self.state.write(format("int main(int argc, char** argv) {\n return {str}(argc, argv);\n}\n", real_name)); + self.state.write( + format( + "int main(int argc, char** argv) {\n{str} return {str}(argc, argv);\n}\n", + mc_leak_suppress, + real_name + ) + ); ret .None; } self.state.write(format( @@ -18891,7 +18928,7 @@ impl Codegen { `} `#endif `int main(int argc, char** argv) { - ` __fun_main_run_args __fun_main_a; + `{str} __fun_main_run_args __fun_main_a; ` __fun_main_a.argc = argc; ` __fun_main_a.argv = argv; ` __fun_main_a.result = 0; @@ -18911,7 +18948,8 @@ impl Codegen { ` return __fun_main_a.result; `} ``, - real_name + real_name, + mc_leak_suppress )); ret .None; } @@ -19231,6 +19269,14 @@ impl Codegen { self.state.write(" return 0;\n"); self.state.write("}\n\n"); self.state.write("int main(int argc, char** argv) {\n"); + // Same reasoning as `emit_main_thread_wrapper`'s own + // `Codegen.memcheck_leaks`-gated call: a `-test` binary's own + // process is short-lived by design (run the tests, report, exit), + // so it's "unopted" out of leak reporting the same as every other + // build, unless `-memcheck-leaks` asked for it. + if self.memcheck && !self.memcheck_leaks { + self.state.write(" __fun_memcheck_disable_leak_sweep();\n"); + } self.state.write(" __fun_test_main_args __fun_test_a;\n"); self.state.write(" __fun_test_a.argc = argc;\n"); self.state.write(" __fun_test_a.argv = argv;\n"); diff --git a/src/codegen/codegen_state.fn b/src/codegen/codegen_state.fn index a24cd2c1..41b5c58e 100644 --- a/src/codegen/codegen_state.fn +++ b/src/codegen/codegen_state.fn @@ -18,6 +18,20 @@ pub compound CodegenState { // directive is valid C - it must start its own line. flag debug_info; flag has_pending_line_directive; + // Unlike `has_pending_line_directive` (cleared the moment + // `write_indent` flushes it, so the *next* one knows whether it + // still needs writing), this one is never cleared once set: it + // tracks whether `pending_line_directive` itself has ever been + // assigned a real value at all, for a caller that wants to re-read + // it directly (a hoisted temp's own extra `#line`, written mid- + // statement, well after the statement's own leading `write_indent` + // already flushed it once) rather than go through the normal + // flush-once-per-statement path. Without this, such a caller can't + // tell "already flushed, but still the correct value" apart from + // "never set at all" (left as this struct's own zero-initialized + // default, including the empty `Pos.filename` dereferencing that + // produces when written as a C string). + flag pending_line_directive_is_valid; Pos pending_line_directive; } @@ -27,6 +41,7 @@ pub fun codegen_state_new() CodegenState { st.indent_level = 0; st.debug_info = false; st.has_pending_line_directive = false; + st.pending_line_directive_is_valid = false; ret st; } @@ -44,6 +59,7 @@ impl CodegenState { } self.pending_line_directive = pos; self.has_pending_line_directive = true; + self.pending_line_directive_is_valid = true; } // Like `write_indent`, but re-pins the `#line` directive to `pos` diff --git a/src/fls/analysis.fn b/src/fls/analysis.fn index 3fe75347..369fdcad 100644 --- a/src/fls/analysis.fn +++ b/src/fls/analysis.fn @@ -308,12 +308,17 @@ impl AnalysisCache { // genuinely failed to resolve, not just that they did. str err_msg = merged.unwrap_err(); if _debug_imports() { + // Both strings are already NUL-terminated, so a plain `%s` + // prints all of either exactly like a `%.*s` precision of + // its own `len()` would - without that precision argument's + // own real hazard: C's varargs promote based on the actual + // type passed, not the format string, so `len()`'s `int64_t` + // read back as the `int` width `%.*s` expects is genuinely + // undefined on a platform where the two differ in size. fprintf( stderr_stream(), - "[fls debug] import resolution failed for %.*s: %.*s\n", - len(doc.path), + "[fls debug] import resolution failed for %s: %s\n", doc.path, - len(err_msg), err_msg ); } diff --git a/stdlib/std/memcheck.fn b/stdlib/std/memcheck.fn index f1ad3ed0..abc721a9 100644 --- a/stdlib/std/memcheck.fn +++ b/stdlib/std/memcheck.fn @@ -24,15 +24,44 @@ // never dereferences a user-facing guarded pointer), so the faulting // thread can never already hold the lock it's about to wait on. +// Deliberately nothing from std.io/std.string here, even though a +// Fun-side format()-based message would read better: compiling one of +// THIS module's own transitive dependencies directly under -memcheck +// (every stdlib file does, in this project's own test suite) makes the +// "use std.memcheck" injected above its text pull this module back in +// as a dependency of itself, and anything this module then called back +// into - std.io's own format/writeln_to among them - would be a real +// forward reference into a declaration the entry file is the one +// supplying, landing before it in the generated C. Building reports +// with std.c.io's own snprintf/fprintf instead avoids the whole class +// of problem outright: everything this module touches is a bodiless +// C extern with no body of its own to be out of order with. use std.c.mem; use std.c.string; -use std.io; +use std.c.io; use std.sync; num __mc_count = 0; num __mc_capacity = 0; num __mc_next_seq = 0; flag __mc_started = false; +// Set by codegen's own `emit_main_thread_wrapper`/`emit_test_runner`, +// as the very first statement of a program's real entry point, unless +// `-memcheck-leaks` asked to keep leak reporting - see +// `Codegen.memcheck_leaks`'s own note on why it's off by default. +flag __mc_suppress_leak_sweep = false; + +// How much room a report line gets (plenty for every field this module +// ever prints, file paths included). Fixed rather than measured, so +// building one never allocates more than once. +const num __mc_msg_cap = 4096; + +// Opts a running program out of the leak sweep below: real for every +// `-memcheck` build unless `-memcheck-leaks` turns it back on, never +// meant to be called directly from `.fn` source. +pub fun __fun_memcheck_disable_leak_sweep() { + __mc_suppress_leak_sweep = true; +} // Runtime hooks codegen's own preamble provides when `-memcheck` is on // (see `write_prelude`'s own `self.memcheck` block) - real @@ -134,11 +163,13 @@ fun __mc_fault_handler(num sig, raw* fault_addr, raw* ctx) { info_freed_line = __mc_table[idx].freed_line; } let _ulk = __mc_mu.unlock(); - str msg = ""; + str msg = malloc(__mc_msg_cap); if idx >= 0 { if is_freed { - msg = format( - "[memcheck] use-after-free: allocation #{num} ({num} bytes), allocated at {str}:{num}, freed at {str}:{num}", + let _n = snprintf( + msg, + __mc_msg_cap, + "[memcheck] use-after-free: allocation #%lld (%lld bytes), allocated at %s:%lld, freed at %s:%lld", info_seq, info_size, info_file, @@ -147,8 +178,10 @@ fun __mc_fault_handler(num sig, raw* fault_addr, raw* ctx) { info_freed_line ); } else { - msg = format( - "[memcheck] buffer overflow: past the end of allocation #{num} ({num} bytes), allocated at {str}:{num}", + let _n = snprintf( + msg, + __mc_msg_cap, + "[memcheck] buffer overflow: past the end of allocation #%lld (%lld bytes), allocated at %s:%lld", info_seq, info_size, info_file, @@ -157,11 +190,11 @@ fun __mc_fault_handler(num sig, raw* fault_addr, raw* ctx) { } } else { str lit = "[memcheck] memory access violation (not within any tracked allocation)"; - msg = substr(lit, 0, len(lit)); + let _n = snprintf(msg, __mc_msg_cap, "%s", lit); } - writeln_to(.Stderr, msg); + let _fp = fprintf(stderr_stream(), "%s\n", msg); free(msg); - let _fl = Sink.Stderr.flush(); + let _ff = fflush(stderr_stream()); abort(); } @@ -170,6 +203,9 @@ fun __mc_fault_handler(num sig, raw* fault_addr, raw* ctx) { // freeing it yet. Forces a nonzero exit on any leak found, even though // `main` itself already returned cleanly - what makes this usable in CI. fun __mc_leak_sweep() { + if __mc_suppress_leak_sweep { + ret; + } num i = 0; num leaks = 0; for i < __mc_count { @@ -185,14 +221,23 @@ fun __mc_leak_sweep() { num line = __mc_table[i].line; let _ulk = __mc_mu.unlock(); if !is_freed { - str msg = format("[memcheck] leak: allocation #{num}, {num} bytes, allocated at {str}:{num}, never freed", seq, size, file, line); - writeln_to(.Stderr, msg); + str msg = malloc(__mc_msg_cap); + let _n = snprintf( + msg, + __mc_msg_cap, + "[memcheck] leak: allocation #%lld, %lld bytes, allocated at %s:%lld, never freed", + seq, + size, + file, + line + ); + let _fp = fprintf(stderr_stream(), "%s\n", msg); free(msg); leaks = leaks + 1; } i = i + 1; } - let _fl = Sink.Stderr.flush(); + let _ff = fflush(stderr_stream()); if leaks > 0 { exit(1); } @@ -299,8 +344,11 @@ pub fun __fun_memcheck_free(raw* ptr, str file, num line) { str dbl_freed_file = entry.freed_file; num dbl_freed_line = entry.freed_line; let _ulk1 = __mc_mu.unlock(); - str msg = format( - "[memcheck] double-free: allocation #{num}, {num} bytes, allocated at {str}:{num}, already freed at {str}:{num}, double-freed at {str}:{num}", + str msg = malloc(__mc_msg_cap); + let _n = snprintf( + msg, + __mc_msg_cap, + "[memcheck] double-free: allocation #%lld, %lld bytes, allocated at %s:%lld, already freed at %s:%lld, double-freed at %s:%lld", dbl_seq, dbl_size, dbl_alloc_file, @@ -310,9 +358,9 @@ pub fun __fun_memcheck_free(raw* ptr, str file, num line) { file, line ); - writeln_to(.Stderr, msg); + let _fp = fprintf(stderr_stream(), "%s\n", msg); free(msg); - let _fl = Sink.Stderr.flush(); + let _ff = fflush(stderr_stream()); abort(); } let _rc = __fun_mc_guard_protect_all(entry.base, entry.total_size);