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;