From 3df49184a7630eb6057e21644707c0fac1920fb5 Mon Sep 17 00:00:00 2001 From: Joseph Ferano Date: Mon, 21 Sep 2026 10:10:32 +0700 Subject: [PATCH] The combination nothing built is built now, eight cases of it --dev --sanitize had been unbuildable for as long as a dev build has armed the registry from a constructor, and nobody knew because no alias built it: test_sanitize built the corpus twice and neither time --dev, test_dev builds --dev and never with a sanitizer. A configuration nothing builds can be broken for a month, and that one was. af71459 fixed it; this is what keeps it fixed. dev_sweep, in the alias that already exists rather than a new one -- five names to remember was already four too many, and this is the same question @sanitize is for. Seven programs built --dev twice, plain and sanitized, run standalone with no daemon: a sanitizer report fails, and so does any divergence from the unsanitized dev run. Standalone is what makes it nearly free, and it works because a dev build is not a dev session. The cells, the marker and the constructor are in the executable either way, and a program that never calls agent/start runs to its own end. That is why the list is mostly ordinary programs built the other way: dev-noagent is the only one of sixteen dev-*.flan that does not import the agent, and the rest stop in the break loop waiting for an editor, which is test_dev.ml's business against a real daemon. The three dyn programs are not interchangeable and the difference is the collector. dyn-vec allocates and never collects -- flan_dyn.c has a one-megabyte floor and dyn-vec does not reach it -- so what it says is that the registry and the allocator agree. dyn-map and p13-dyn-collect are the only two programs anywhere past that floor, so they are the only two under which a mark and a sweep run; without them a collection had still never happened in a dev build under ASan. dev_segv is beside the sweep rather than in it, because the program that faults cannot be compared against an unsanitized run: that build's handler prints its line and parks in the break loop, so the plain half would hang, and the two are supposed to differ. ASan is meant to own the fault -- flan_dev_crash_enable checks a weak __asan_init and stands down -- so the case asserts ASan's report and the absence of the handler's line. At -O0, because at -O2 the write through a bytes-view of a literal does not fault at all and both builds print the string unchanged. That yield had never run in any build anywhere; it was behind a link that did not happen. Checked both ways rather than asserted: eight failures with the constructor naming declarations again, clean with it fixed. Twenty-six seconds of the alias's 2m30 warm, and nothing added to dune test. Two aliases were also green only because dune test runs first. @sanitize never listed the package directories pkg-diamond.flan imports, and @page never listed sand.flan, which quotes.sh reaches through sand-headless.flan; from a cold tree the first died before its first sanitized build, and @page sits inside @checks where CI hides the same thing. Both listed now. @valgrind, @x86, @js and @cells were checked and are complete. What it still does not reach is in FIX.org: a program driven by a real flan dev daemon under ASan, which wants a --sanitize the CLI does not have and a way through Dev.serve. --x86 --sanitize is refused by Build by name, so there is no second backend to track here. --- FIX.org | 83 +++++++++++++++++++++++++ docs/BUILT.md | 28 +++++++++ test/dune | 16 ++++- test/test_sanitize.ml | 140 ++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 262 insertions(+), 5 deletions(-) diff --git a/FIX.org b/FIX.org index 4649ca5..aa874ed 100644 --- a/FIX.org +++ b/FIX.org @@ -5542,3 +5542,86 @@ condition's payload" reached the struct-field refusal that fired first, never the condition arm it was named for. The struct refusal is gone since the descriptors landed, so the row is an [accepts] now and a new [rejects_check] signals a dyn directly to reach the arm that is still there. +* 2026-09-21 — [--dev --sanitize] was unbuildable, and nothing built it +[--dev --sanitize] could not compile a single program. Any program, any +optimisation level: clang 20's AddressSanitizer module pass segfaults on a +module whose [llvm.global_ctors] names a function the module only declares, +and both of ours were declarations — they are C in the runtime. It had been +that way for as long as the allocation registry has been armed from a +constructor. The fix is f75b0ca on the crash-handler lane, merged into +dev-loop as af71459: the table names a local [Mangle.dev_ctor] definition +that calls the two, which is the shape clang emits for its own constructors. +This lane hit the same wall independently on a base that still had the +one-entry table, wrote the same fix restricted to it, and dropped that commit +on the rebase in favour of dev-loop's. + +Nobody knew because no alias built the combination. [test_sanitize.ml] built +the corpus twice and neither time with [--dev]; [test_dev.ml] builds [--dev] +and never with a sanitizer. A configuration nothing builds can be broken for +a month, and this one was — and the fix could have rotted the same way. + +** What the new check is +[dev_sweep] in test/test_sanitize.ml, under the existing @sanitize alias — +no new alias to remember, and nothing added to [dune test]. Seven programs +built [--dev] twice, plain and sanitized, and run standalone with no daemon: +dev-noagent.flan (the only one of the sixteen dev-*.flan that does not import +the agent, and so the only one that does not stop waiting for an editor), +println, values, text, and the three dyn programs. A sanitizer report fails, +and so does any divergence from the unsanitized dev run. + +Standalone is what makes it nearly free, and it works because a dev build is +not a dev *session*: the cells, the marker and the constructor are in the +executable either way, and a program that never calls (agent/start ...) runs +to its own end. + +The three dyn programs are not interchangeable and the difference is the +collector. [dyn-vec] allocates objects of three kinds and never collects — +flan_dyn.c has a one-megabyte floor and dyn-vec does not reach it — so what +it says is that the dev registry and the allocator agree. [dyn-map] and +spike/x86/p13-dyn-collect.flan are the two programs anywhere that allocate +past that floor, so they are the only ones under which a mark and a sweep +actually run; until they were in this list, a collection had never happened +in a dev build under ASan at all. An earlier draft of this entry claimed +dyn-vec covered that, and it did not. + +[dev_segv] is beside the sweep rather than in it, because the one dev program +that faults cannot be compared against an unsanitized run: the dev build's +SIGSEGV handler prints its line and parks in the break loop, so the plain +half of the pair would hang, and the two are supposed to differ. ASan is +meant to own the fault — [flan_dev_crash_enable] checks a weak [__asan_init] +and declines to install the handler when ASan is in the process — so the case +asserts ASan's report and the absence of the handler's line. Built at -O0, +because at the sweep's -O2 the write through a bytes-view of a literal does +not fault at all; both builds print the unmodified string. This is the line +the emitter fix unblocked: until the constructor stopped naming a +declaration, no [--dev --sanitize] build linked, so the yield had never run +anywhere. + +Verified both directions rather than asserted: reverting [Emit] to a table +naming the two declarations gives eight failures — the seven dev programs +and dev-segv, each of them the clang crash — and a nonzero alias exit, and +the fixed emitter is clean. Twenty-six seconds of the whole alias's 2m30 on +this machine, measured warm; BUILT.md's nine minutes is the cold figure. The +static link is the expensive half and the object cache already holds it by +the time this part runs. + +** Two aliases were green only because [dune test] runs first +@sanitize's deps never listed the package directories pkg-diamond.flan +imports, and @page's never listed sand.flan, which quotes.sh reaches through +test/programs/sand-headless.flan. Both only ever passed because [dune test] +had already materialised those files in _build; from a genuinely cold tree +@sanitize died on pkg-diamond before its first sanitized build. @page sits +inside @checks, where CI hides the same thing. Both fixed here. @valgrind, +@x86, @js and @cells were checked and are complete. + +** What it still does not check +- A program driven by a real [flan dev] daemon under ASan. The daemon builds + its host through [Dev]'s own path and the CLI has no [--sanitize] to pass + it ([flan dev [-s socket] [--x86]]), so this wants a flag and a + way through [Dev.serve]. Not built. It is the check worth adding next, and + it is a day rather than an hour. +- The x86 backend. [--x86 --sanitize] is refused by [Build] by name — there + is no sanitizer pass in the hand-written backend — so the configuration is + LLVM-only and there is nothing to track. +- The reload path, which was already outside @sanitize: a redefinition module + is built by llc and ld, so nothing instruments it. diff --git a/docs/BUILT.md b/docs/BUILT.md index b4112fa..2e2f5c9 100644 --- a/docs/BUILT.md +++ b/docs/BUILT.md @@ -911,6 +911,34 @@ second net, not a replacement. The sweep lives on its own dune alias rather than on `dune test`: a sanitized program links to a statically linked 1.8MB binary, and twenty-eight of them twice over is minutes against the suite's seconds. +**`--dev --sanitize` is in the sweep now, and the reason it is worth a paragraph is that it did not compile.** Not one +program, at any optimisation level, for as long as a dev build has armed the allocation registry from a constructor: +clang 20's AddressSanitizer module pass segfaults on a module whose `llvm.global_ctors` names a function the module only +*declares*, and both of the names in that table were runtime C. The table now names a local `Mangle.dev_ctor` definition +that calls them — the shape clang emits for its own constructors — and the crash goes away. Nobody knew because nothing +built the combination: `test_sanitize.ml` built the corpus twice and neither time `--dev`, `test_dev.ml` builds `--dev` +and never with a sanitizer, and a configuration nothing builds can be broken for a month. + +`dev_sweep` closes that: seven programs built `--dev` twice, plain and sanitized, and run standalone, which works +because a dev build is not a dev *session* — the cells and the constructor are in the executable either way, and a +program that never calls `agent/start` runs to its own end. Three of the seven are the dyn programs, and they are not +interchangeable: `dyn-vec` allocates and never collects, so it says the dev registry and the allocator agree, while +`dyn-map` and `p13-dyn-collect` are the only two programs anywhere that pass `flan_dyn.c`'s one-megabyte floor and so +the only two under which a mark and a sweep run at all. `dev_segv` sits beside the sweep rather than in it, because the +program that faults cannot be compared against an unsanitized run — that build's handler parks in the break loop, and +the two builds are *supposed* to differ, since `flan_dev_crash_enable` checks a weak `__asan_init` and declines to +install the handler when ASan is in the process. So the case asserts ASan's report and the absence of the handler's +line, built at `-O0` because at `-O2` the write through a bytes-view of a literal does not fault at all. That yield had +never run in any build anywhere: it was behind a link that did not happen. Twenty-six seconds of the alias's 2m30 warm. +What it still does not reach is a program driven by a real daemon under ASan: `flan dev` builds its host through its own +path and has no `--sanitize` to pass it. + +**Two aliases were green only because `dune test` runs first, and that is the same disease in a different place.** +`@sanitize` never listed the package directories `pkg-diamond.flan` imports and `@page` never listed `sand.flan`, which +`quotes.sh` reaches through `sand-headless.flan`. Both passed because the suite had already materialised those files in +`_build`; from a genuinely cold tree `@sanitize` died on `pkg-diamond` before its first sanitized build. `@page` sits +inside `@checks`, so CI hid it too. `@valgrind`, `@x86`, `@js` and `@cells` were checked and are complete. + ## Valgrind, and the two questions a sanitizer cannot be asked `dune build --root . @valgrind` runs the headless corpus under memcheck: forty-nine programs checked, twelve of them a diff --git a/test/dune b/test/dune index 62338d6..dc7ff09 100644 --- a/test/dune +++ b/test/dune @@ -173,7 +173,15 @@ ; translation unit here that frees the most, which is what makes it worth a ; sanitized run at all. See [dyn_sweep]. (file dyn_ops.c) - (glob_files programs/assets/json/*)) + (glob_files programs/assets/json/*) + ; The package tree pkg-diamond.flan imports — it reaches shape through area + ; and draw — a glob per directory because dune's glob does not descend. + ; Missing until now, and the alias only looked green because `dune test` had + ; run first and left the directories in _build: from a clean tree the sweep + ; died on pkg-diamond before it reached a single sanitized run. + (glob_files programs/pkgs/shape/*) + (glob_files programs/pkgs/area/*) + (glob_files programs/pkgs/draw/*)) (action (run ./test_sanitize.exe))) ; The corpus a third time, under Valgrind's memcheck. Its own alias for the @@ -336,6 +344,12 @@ (file %{workspace_root}/conditions.org) (glob_files %{workspace_root}/emacs/*.el) (glob_files programs/*.flan) + ; And sand.flan itself, which the hash comes from: quotes.sh runs + ; test/programs/sand-headless.flan and that imports ../../sand.flan as a + ; single-file package. Missing until now, the same hole @sanitize had — the + ; alias was green only because `dune test` had already put the file in + ; _build, and in CI it sits inside @checks where the same thing hides it. + (file %{workspace_root}/sand.flan) ; breakdemo.flan is built --dev, so it imports the agent; raylib is here for ; the binding line quotes.sh greps out of it. (glob_files %{workspace_root}/vendor/agent/*) diff --git a/test/test_sanitize.ml b/test/test_sanitize.ml index 2a4e481..d9f18ca 100644 --- a/test/test_sanitize.ml +++ b/test/test_sanitize.ml @@ -61,17 +61,19 @@ let run exe args = (try Sys.remove out with Sys_error _ -> ()); (code, text) -let compile ~sanitize ~checks path = +let compile ?(dev = false) ~sanitize ~checks path = let exe = Filename.concat scratch - (Printf.sprintf "flan-san-%s-%s" + (Printf.sprintf "flan-san-%s%s-%s" (if sanitize then "s" else "p") + (if dev then "d" else "") (Filename.remove_extension (Filename.basename path))) in - let p, csrcs, lflags = Test_support.linked path in + let p, csrcs, lflags = Test_support.linked ~dev path in ignore (Build.executable - ~opts:{ Build.default with checks; sanitize } ~csrcs ~lflags p ~out:exe); + ~opts:{ Build.default with checks; sanitize; dev } ~csrcs ~lflags p + ~out:exe); exe let contains = Test_support.contains @@ -325,6 +327,134 @@ let dyn_sweep () = "classes" ]; (try Sys.remove exe with Sys_error _ -> ()) +(* A third sweep, over a handful of the same programs built [--dev]. + + Why this exists at all. [--dev --sanitize] could not compile a single + program, at any optimisation level, and had not been able to for as long + as a dev build has armed the allocation registry from a constructor: + clang 20's AddressSanitizer module pass segfaults on a module whose + [llvm.global_ctors] names a function the module only declares, and ours + was a runtime C function. The fix is in [Emit] and its comment says what + the shape has to be. What this is, is the reason nobody knew — no alias + built the combination. [corpus] above is built twice, and both times + without [--dev]; [test_dev.ml] builds [--dev] and never with a sanitizer. + A configuration nothing builds is a configuration that can be broken for a + month, and this one was. + + Standalone and with no daemon, which is what makes it nearly free. A dev + build is not a dev *session*: the cells, the ABI marker and the + constructor are in the executable either way, and a program that does not + call [agent/start] runs to its own end and exits. That is why the list is + mostly ordinary programs built the other way rather than the [dev-*] + ones — every [dev-*] program but one imports the agent and stops in the + break loop waiting for an editor, and those are test_dev.ml's against a + real daemon. + + So what it checks is narrower than the corpus sweep and is deliberately + the half that was broken: that the combination *builds*, and that the + program it builds still prints what the unsanitized dev build printed and + exits the same way. The crash was at compile time, so the build is the + part that carries the weight; the run is what says the constructor the fix + introduced actually calls both of the things it replaced. + + Not covered, and worth naming rather than leaving to be discovered the way + this bug was: a program driven by [flan dev] under ASan. The daemon builds + its host through its own path and the CLI has no [--sanitize] to pass it, + so that one wants a flag and a way through [Dev.serve]. See FIX.org. The + faulting dev build, which was on that list too, is covered now — see + [dev_segv] below. *) +let dev_corpus = + [ (* The only [dev-*] program with no agent import: it prints and returns. + Here because it is the one program in the tree written for a dev + build that a sweep can run on its own. *) + "programs/dev-noagent.flan"; + (* Ordinary programs, built the way the dev loop builds them. A dev build + puts every function behind a cell and routes every call through it, so + these say the indirection still computes what the direct call did — + under ASan, which is the part the corpus sweep cannot say. *) + "programs/println.flan"; + "programs/values.flan"; + "programs/text.flan"; + (* And the ones that allocate, because a dev build's registry notes every + block and a dyn program is the one that frees. [dyn-vec] builds objects + of three kinds and never collects — flan_dyn.c has a one-megabyte floor + and nothing here reaches it — so on its own it says the registry and + the allocator agree, which is worth having and is not the collector. + [p13] and [dyn-map] are the two programs anywhere that allocate past + that floor, so they are the only ones under which a mark and a sweep + actually run, and until they were in this list a collection had never + happened in a dev build under ASan at all. *) + "programs/dyn-vec.flan"; + "programs/dyn-map.flan"; + "../spike/x86/p13-dyn-collect.flan" ] + +let dev_sweep () = + List.iter + (fun path -> + match compile ~dev:true ~sanitize:false ~checks:true path with + | exception Failure m -> fail "dev %s: unsanitized dev build: %s" path m + | plain -> + (match compile ~dev:true ~sanitize:true ~checks:true path with + | exception Failure m -> + fail "dev %s: --dev --sanitize did not build: %s" path m + | san -> + let c1, t1 = run plain [] in + let c2, t2 = run san [] in + if reported t2 then fail "dev %s: sanitizer report\n%s" path t2 + else if c1 <> c2 || t1 <> t2 then + fail "dev %s: diverged from the unsanitized dev run\n \ + plain (exit %d): %S\n sanitized (exit %d): %S" + path c1 t1 c2 t2; + (try Sys.remove san with Sys_error _ -> ())); + (try Sys.remove plain with Sys_error _ -> ())) + dev_corpus + +(* And the one dev program that faults, which is its own case rather than a + line in [dev_corpus] for two reasons. + + It cannot be compared against an unsanitized run. A dev build's SIGSEGV + handler prints its line and then parks in the break loop waiting for an + editor, so the plain half of the pair would hang rather than answer, and + the two are *supposed* to differ here: [flan_dev_crash_enable] checks a + weak [__asan_init] and declines to install the handler when ASan is in the + process, on the grounds that two owners of SIGSEGV is one too many. So the + sanitized run must produce ASan's report and must NOT produce the + handler's line, and that is the assertion. + + And it has to be built at -O0. At the sweep's -O2 the write through a + bytes-view of a string literal does not fault at all — measured, both + builds print the unmodified string — so a case that is about what happens + on a fault has to be compiled where the fault happens. Same family as the + -O0/-O2 split [unchecked_controls] records for bounds.flan. + + This is the line the emitter fix unblocked: until [llvm.global_ctors] + stopped naming a declaration, no [--dev --sanitize] build linked, so the + yield had never run in any build anywhere. *) +let dev_segv () = + let path = "programs/dev-segv.flan" in + match + (let p, csrcs, lflags = Test_support.linked ~dev:true path in + let exe = Filename.concat scratch "flan-san-sd-dev-segv" in + ignore + (Build.executable + ~opts:{ Build.default with Build.dev = true; sanitize = true; + opt = "-O0" } + ~csrcs ~lflags p ~out:exe); + exe) + with + | exception Failure m -> fail "dev-segv: --dev --sanitize -O0 build: %s" m + | exe -> + let _, text = run exe [] in + if not (reported text) then + fail "dev-segv: no sanitizer report. ASan is meant to own the fault \ + here — check the weak [__asan_init] in flan_dev_crash_enable\n%s" + text + else if contains text "flan: SIGSEGV" then + fail "dev-segv: the Flan handler reported as well as ASan. Both of them \ + own SIGSEGV now, which is what the [__asan_init] check exists to \ + prevent\n%s" text; + (try Sys.remove exe with Sys_error _ -> ()) + (* The positive controls, which are the only evidence that a clean sweep means anything. Both are written here rather than kept in test/programs because neither is a program anybody should build: one reads off the end of an @@ -449,6 +579,8 @@ let () = \ 0)\n"; sweep ~checks:true "checked"; dyn_sweep (); + dev_sweep (); + dev_segv (); unchecked_controls (); if !failures = 0 then print_endline "sanitizer sweep: clean" else Printf.printf "%d sanitizer failure(s)\n" !failures;