Merge branch 'worktree-agent-aba36ddb6a383320f' into dev-loop
# Conflicts: # FIX.org
This commit is contained in:
commit
13145cc52a
84
FIX.org
84
FIX.org
@ -5629,3 +5629,87 @@ hidden-failure shape a previous lane found. sand.flan also feeds ~@x86~,
|
||||
~@js~, ~@sanitize~ and ~@valgrind~ through their workspace-file deps, so those
|
||||
go red too. Everything underneath was verified green against a copy of the
|
||||
file with those two lines changed.
|
||||
|
||||
* 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 <program> [-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.
|
||||
|
||||
@ -912,6 +912,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
|
||||
|
||||
16
test/dune
16
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/*)
|
||||
|
||||
@ -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;
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user