diff --git a/NEXT.md b/NEXT.md index e151fd9..2fcaeb2 100644 --- a/NEXT.md +++ b/NEXT.md @@ -21,15 +21,16 @@ What the headline hides is where the remaining time now sits: `ld` did not move about 9ms of the round trip is the frontend re-checking the program — 14% of the LLVM loop but a third of this one. The next millisecond is in the linker, not the code generator. -## The one thing that does not work, and it is not small +## The thing that did not work, and now does -**`flan dev --x86` requires `--two-process` and refuses the merged daemon.** A merged host is `-rdynamic`, so -it exports every `flan.*` body, and those interpose the prelude bodies of the LLVM-built macro module the -compiler loads into itself. An LLVM caller lands in an x86 body and takes SIGSEGV inside `flan.[clamp]` during -the first macro expansion, before the program has started. `flan.abi.x86` does not guard this and was never -meant to — a macro module deliberately carries no marker. `-Bsymbolic` is the wrong fix. The honest one is -hidden visibility on a macro module's Flan bodies, and it wants its own lane. `start_merged`'s `--x86` -plumbing is written and correct, just unreachable. See `docs/handoffs/HANDOFF-x86-devloop.md`. +**`flan dev --x86` runs the merged daemon.** It used to refuse it and demand `--two-process`: a merged host is +`-rdynamic`, so it exports every `flan.*` body, and those interposed the prelude bodies of the LLVM-built macro +module the compiler loads into itself — an LLVM caller landing in an x86 body, SIGSEGV inside `flan.[clamp]` +during the first macro expansion, before the program had started. `Build.macro_module` now asks `Emit.program` +to give that module's own Flan definitions hidden visibility, so there is nothing left for the host to +interpose and only the `flan.macro.*` thunks `dlsym` has to find stay exported. The host did not move, which is +what keeps redefinition modules reaching its cells. `-Bsymbolic`, which had been doing the same job bluntly +since the package-macro fix, is gone with it. See `docs/handoffs/HANDOFF-x86-macro-visibility.md`. ## Still open, unchanged diff --git a/docs/handoffs/HANDOFF-x86-devloop.md b/docs/handoffs/HANDOFF-x86-devloop.md index 73036b9..3cb0554 100644 --- a/docs/handoffs/HANDOFF-x86-devloop.md +++ b/docs/handoffs/HANDOFF-x86-devloop.md @@ -12,6 +12,15 @@ There is one finding that is not good news and it is in §The merged daemon: `fl *merged* daemon and needs `--two-process`, because the compiler's macro module is a third way the two backends can meet in one process and `flan.abi.x86` does not guard it. +> **Closed.** `docs/handoffs/HANDOFF-x86-macro-visibility.md` took the recommendation in §The merged daemon — +> hidden visibility on a macro module's Flan bodies — and `flan dev --x86` starts a merged daemon now. The +> refusal in `Dev.start` is gone, `test_dev.ml` drives a merged `--x86` session through C-x C-e, C-c C-c, a +> park and a `rerun`, and the analysis below stands as the account of what the failure *was*. One correction +> to it: by the time that lane ran, `-Wl,-Bsymbolic` had already landed for an unrelated reason (a package's +> `defmacro` resolving to the merged host's copy, commit 65d14f4) and was incidentally suppressing this crash +> too. It has been removed in favour of the narrow fix, for the reason §The merged daemon gives for not +> wanting it. + ## What was built | file | what | @@ -239,9 +248,7 @@ comes back as 143, which is not a result. And do not run the survey while `dune ## What remains -- **The macro module, so the merged daemon can be `--x86`.** §The merged daemon above has the mechanism, the - measurement and the fix that should be tried. This is the largest remaining piece and it is what stands - between `flan dev --x86` and being the default shape of the loop. +- ~~**The macro module, so the merged daemon can be `--x86`.**~~ Done — `HANDOFF-x86-macro-visibility.md`. - **The linker.** 9ms, unchanged between backends, now the bulk of an `--x86` build. - **DWARF from `X86.redefinition`**, which is what `flan dev --x86 --debug` is refused for. - **`X86.program ~macros`**, if the macro module is ever to be built by this backend rather than merely made diff --git a/docs/handoffs/HANDOFF-x86-macro-visibility.md b/docs/handoffs/HANDOFF-x86-macro-visibility.md new file mode 100644 index 0000000..d8cc99f --- /dev/null +++ b/docs/handoffs/HANDOFF-x86-macro-visibility.md @@ -0,0 +1,149 @@ +# Handoff — a macro module keeps its own prelude, so the merged daemon can be `--x86` + +Branch `dev-loop`, worktree `agent-a8f1c40df2a41e094`, from `9e80147`. This closes the finding of +`HANDOFF-x86-devloop.md` §The merged daemon: `flan dev --x86` refused the merged daemon and demanded +`--two-process`, because the compiler's macro module is a third way the two backends can meet in one process +and `flan.abi.x86` does not guard it. + +**`flan dev --x86` starts a merged daemon now.** One process, a program that uses macros, C-x C-e, C-c C-c, a +park and a `rerun`, all in `dune test`. The LLVM path is byte-identical — see §Verification, which is the one +section of this file that had to be written before anything else was believed. + +## What was built + +| file | what | +|---|---| +| `lib/emit.ml` — `program ?hidden`, `emit_global ?hidden` | every Flan definition in the module emitted `hidden`, except the `flan.macro.*` thunks. Defaults to `false`, so no other caller moves | +| `lib/build.ml` — `macro_module` | passes `~hidden:true`, and drops the `-Wl,-Bsymbolic` that was doing the same job bluntly | +| `lib/dev.ml` — `Dev.start` | the `x86 && merged` refusal is gone; the comment in its place records what made the combination safe | +| `test/programs/dev-macro.flan` | new: `dev-loop.flan`'s shape with a prelude macro in it, and a bounded run so it parks on the clock | +| `test/test_dev.ml` | the assertion that the merged daemon refuses `--x86` is now a merged `--x86` session driven through both editor verbs, a park and a `rerun` | +| `NEXT.md`, `HANDOFF-x86-devloop.md` | the open finding, pointed at this file | + +## The correction that came first + +`HANDOFF-x86-devloop.md` says `-Bsymbolic` is the wrong fix and should not be reached for. By the time this +lane opened it was **already in the tree**: commit 65d14f4, for a different bug — a package's `defmacro` +compiled into the merged host, whose body was qualified after the quasiquote had become a string literal, so +expanding through the host's copy produced an unqualified `begin-drawing` the checker refused. That flag binds +*every* reference locally, so it had been incidentally suppressing the SIGSEGV as well. + +So the first thing done here was to delete the refusal and change nothing else. `flan dev --x86` on +`test/programs/dev-repl.flan` and on `test/programs/sand-headless.flan` — which imports `sand.flan` and so +expands `rl/with-drawing` — both came up merged, served a C-c C-c and a C-x C-e, parked and reran. **The crash +was already gone.** That is worth stating plainly, because it changes what the rest of this lane is: not +"reproduce and fix a segfault" but "replace a blunt fix that works with a narrow one that works for a reason, +and prove both properties the blunt one was covering". + +`-Bsymbolic` is worth replacing rather than leaving alone, and the reason is not tidiness. The macro module +links its own `flan_rt.c`. Binding symbolically aims its calls at that copy rather than the host's — a copy +`flan_rt_init` never ran on, and one whose `flan_exit_hook` is null. That hook is how a merged build parks +instead of exiting: a runtime trap raised inside a macro expansion would have called `exit(3)` and taken the +session down with it. Nothing in the suite goes there, which is exactly the shape of a bug that hides behind a +green run. + +## The fix + +`Emit.program ~hidden:true` writes `define hidden` on every Flan function and `hidden global` on every Flan +global. The thunks stay at default visibility, and that is the whole of the boundary: `flan_macro_call` +(`lib/dynload_stubs.c`) reaches a macro by `dlsym`, a hidden symbol is not in the table `dlsym` searches, and +nothing else in the module is anybody's to call. Measured on the module the suite builds: + +``` +$ nm -D --defined-only ~/.cache/flan/objcache/flan-macros-.so | grep 'flan\.' +0000000000006490 T flan.macro.clamp +00000000000064b0 T flan.macro.unless +00000000000064d0 T flan.macro.into +``` + +Three symbols, and they are the three the compiler asks for by name. The 155 `flan_*` C symbols are still +exported and still bind to the host's when there is a host, which is the property `-Bsymbolic` was taking +away. + +The mechanism was already in the file: `emit_fn ?hidden` exists for `Emit.redefinition`, whose comment records +the mirror-image failure — a redefinition's own body interposed by the host's, so the installer publishes the +function it was replacing and the reload appears to do nothing. `emit_global` needed the parameter added. + +## What still depends on `-rdynamic`, audited + +The export exists so a redefinition module can reach a running program's cells, and none of that moved: the fix +is on the module that is *loaded*, not on the process loading it. `Build.executable` still passes `-rdynamic` +for a dev build and `Emit.program` still defaults `hidden` to `false`, so a merged host's dynamic symbol table +is what it was. Item by item, with what says so: + +- **A redefinition module reaching the host's cells and globals.** `Emit.redefinition` emits an `external` for + a global the host has and reaches a function through the host's cell symbol. Both are host exports. Covered + by the merged `--x86` C-c C-c in `test_dev.ml` and by every LLVM case above it. +- **`flan_dev_cell` / `flan_dev_global`.** C symbols in `runtime/flan_dev.c`, linked into the host, reached + from an installer by the same export. They are the new-name path. `test_dev.ml`'s `--two-process` `--x86` + case still does a new `defvar`, a new `defn` and a round trip through both. +- **`Emit.cellptr` / `Emit.globalptr`.** Module-local slots in the *redefinition* module that the installer + fills by string. Nothing in a macro module emits either, and `Emit.redefinition` is untouched. +- **`flan_reload_transient`.** Defined by a redefinition module and `dlsym`'d by `vendor/agent/flan_agent.c` + to decide whether the module can be unloaded. Also `Emit.redefinition`'s, also untouched — and note it is a + symbol looked up in a *dlopened object*, which is the one place hidden visibility would have broken + something had it been applied there. +- **The weak-symbol pattern in `lib/dynload_stubs.c`.** `flan_agent_request`, `flan_merged_rerun` and + `flan_merged_program_state` are `__attribute__((weak))` declarations resolved when the *compiler* is linked + into the merged executable — a static link, decided before any `dlopen` happens. A macro module has no + bearing on them. `rerun` working in the new test is the end-to-end proof: it is that exact pattern. + +The one thing that did change is stated in §The correction above, and it changed *back*: the macro module's +calls into the C runtime bind to the host's copy again, as they did before 65d14f4. + +**The invariant that keeps it that way: `~hidden:true` is passed by exactly one caller, `Build.macro_module`, +and it never passes `~dev`.** `Emit.program` refuses the pair outright now, because nothing in the types stops +someone writing it and the consequence would be a merged host that links, runs, and silently installs nothing: +a dev build's cells, globals and `flan.abi.*` marker are in the dynamic symbol table *on purpose*, for a +redefinition module to bind against. The other four call sites (`Build.executable`, `Dev.merged_executable`, +`bin/main.ml`'s `emit`, and the tests) take the default. + +## Verification + +| | before (`9e80147`) | after | +|---|---|---| +| `dune test --root . --force` | exit 0, 232 checks | **exit 0, 232 checks, `dev: all tests passed`** | +| emitted IR, 540 files | — | **byte-identical** | +| `flan dev --x86` merged, `sand-headless.flan` | SIGSEGV / refused | **up, parked, reran, same grid hash as LLVM** | +| `flan dev` merged, `sand-headless.flan` | up | **up, `rl/with-drawing` still expands** | + +**The IR comparison is how "the LLVM path did not move" was verified, and it is the check worth repeating.** +Every `test/programs/*.flan` plus `sand.flan` and `calc-me.flan`, emitted twice — once release, once `--dev` — +by the compiler at `9e80147` and by this one: 540 files, `diff -rq` reports nothing. `hidden` defaults to +`false` and the only other edit to a format string adds an empty interpolation, so for those two paths this is +the argument made mechanical rather than a sample. + +Be clear about what that diff does *not* say: it covers `Emit.program`'s default, which is every path but the +new one. `macro_module` deletes its `.ll` unless `opts.keep`, so the `hidden:true` output is not in it. What +stands behind the new path is `nm -D` on the linked object above — three thunks and nothing else of Flan's, +which is the property being claimed, read off the artefact rather than off the text that made it. + +`spike/x86/survey.sh` was **not** run here — it is forty minutes and this lane changes no lowering. It will +report one more `runs-forever` when it next runs: `dev-macro.flan` waits on an agent that is not there, which +is what `dev-loop.flan` and `dev-repl.flan` already do. + +The two properties that had to hold together, in one run each: + +- **LLVM merged, `sand-headless.flan`.** This is the one that checks 65d14f4, and the fixture is chosen for + it: `sand-headless.flan` imports `sand.flan` as a package, `sand.flan` imports `vendor:raylib` and calls + `rl/with-drawing` — a *package's* `defmacro`, reached through an import, which is exactly the shape the + reproduced bug had. A prelude macro would not have covered it, because the prelude's macros are not the ones + whose bodies were qualified after the quasiquote became a literal. If hidden visibility did not cover that + bug the daemon would have died with the checker refusing an unqualified `begin-drawing`. It came up and ran. +- **`--x86` merged, same program.** Both runs print the same grid hash, `15595743031174623232`, which is the + simulation having produced identical output through two backends in a process that also expanded macros. + +## Notes for whoever is next + +- **`dev-macro.flan` parks on a clock, not on a delivery**, and that is deliberate. `dev-loop.flan` waits for + a delivery per `while`, which means a test has to pace its ops to match the fixture's count; `dev-repl.flan` + runs for two minutes. This one runs 400 frames of 5ms and then returns, so the park is going to happen + whether or not `agent/wait` reported the thing the test sent. Two seconds of the default suite's 44. +- **The C-x C-e in that case is retried, not asked once.** `merged_setup` binds the socket and the program's + thread starts after it returns, so the first ask can land before there is an agent to reach. That is a race + with the startup and not a result. +- **`X86.program ~macros` is still unbuilt**, and is still not needed. A macro module is LLVM's and immune to + the host rather than matched to it, which is the cheaper of the two answers and the one that works when the + host is a backend the macro path has never heard of. +- `--x86 --debug` is still refused for the daemon, for the unrelated reason in `Dev.start`: + `X86.redefinition` emits no DWARF. diff --git a/lib/build.ml b/lib/build.ml index 6d8eb50..319a396 100644 --- a/lib/build.ml +++ b/lib/build.ml @@ -1033,7 +1033,21 @@ let macro_module ?(opts = default) ?(csrcs = []) ?(lflags = []) ~macros compiler, and wasm has no dlopen"; let dir = workdir () in let ll = Filename.concat dir (Filename.basename out ^ ".ll") in - write ll (Emit.program ~checks:opts.checks ~macros p); + (* [~hidden] is what keeps this module's Flan bodies its own. [flan dev]'s + merged build is the program and the compiler in one [-rdynamic] + executable, so it exports every [flan.*] body it has and ELF gives it + precedence over anything dlopened afterwards; the macro module's copy of a + prelude function would be interposed by the host's, which is a different + function compiled by a possibly different backend. [Emit.program]'s + [hidden] comment has the two failures that were measured. This is the + narrow half of what [-Wl,-Bsymbolic] would do and is the half that is + wanted: the module links its own [flan_rt.c], and binding *that* locally + would aim its calls at a copy of the runtime [flan_rt_init] never ran on + and at a [flan_exit_hook] the merged build never installed, so a trap + raised inside an expansion would take the process down instead of parking + it. Only the Flan symbols are pinned here; the C goes on resolving the way + it always did. *) + write ll (Emit.program ~checks:opts.checks ~macros ~hidden:true p); (* -fPIC on every object, the .ll included. Without it the link fails with a relocation against a symbol that cannot be used in a shared object — at link time, not at codegen, which is the same trap [shared] meets and @@ -1053,19 +1067,7 @@ let macro_module ?(opts = default) ?(csrcs = []) ?(lflags = []) ~macros let cmd = String.concat " " ([ Filename.quote (compiler opts); opts.opt; "-Wno-override-module"; - "-shared"; "-fPIC"; - (* Bind this module's own references to its own definitions. ELF - gives the executable's symbols precedence over a shared object's, - and [flan dev]'s merged build is an executable that carries the - whole program -- including the package's [defmacro]s, compiled - from a body whose names were qualified after the quasiquote became - string literals and so are not the ones this module holds. Without - this, dlopening the macro module into that host makes - [flan.rl/with-drawing] resolve to the host's copy, and - [(rl/with-drawing ...)] expands to an unqualified [begin-drawing] - that the checker then refuses. The module is self-contained by - design, so there is nothing it wants from the host. *) - "-Wl,-Bsymbolic" ] + "-shared"; "-fPIC" ] @ tflags @ [ Filename.quote ll ] @ List.map Filename.quote objs diff --git a/lib/dev.ml b/lib/dev.ml index 4c0bd95..0ffc092 100644 --- a/lib/dev.ml +++ b/lib/dev.ml @@ -3556,36 +3556,30 @@ let start ?(debug = false) ?(merged = true) ?(x86 = false) ~file ~sock () = "flan dev --x86 --debug: the dev backend emits DWARF for a whole program \ but not yet for a redefinition module, so a breakpoint set on a line \ would stop firing at the first C-c C-c. Use one or the other."; - (* And the merged daemon is refused outright, which is a finding and not a - convenience. A merged build is the program and the compiler in one - process, and the compiler expands macros by [dlopen]ing a module - [Build.macro_module] made -- through [Emit.program], always, and cached on - disk by the macro source rather than by the backend. A merged host is - linked [-rdynamic] so a redefinition module can reach its cells, which - also exports every [flan.*] body it has; the macro module's own copy of a - prelude function is then interposed by the host's. In an LLVM session both - are LLVM and nobody notices. In an [--x86] one the caller is LLVM and the - body it lands in is this backend's, which is the crossed pair -- measured - here as a SIGSEGV inside [flan.\[clamp\]] during the *first* macro - expansion, before the program had started. + (* The merged daemon used to be refused here for [--x86] and no longer is, + and what made the combination safe is worth stating where the refusal + stood. A merged build is the program and the compiler in one process, and + the compiler expands macros by [dlopen]ing a module [Build.macro_module] + made -- through [Emit.program], always, so always LLVM whatever backend + this session uses. A merged host is linked [-rdynamic] so a redefinition + module can reach its cells, and that also exports every [flan.*] body the + host has; ELF gives the executable precedence, so the macro module's own + copy of a prelude function used to be interposed by the host's. In an LLVM + session both copies are LLVM and nobody notices. In an [--x86] one the + caller is LLVM and the body it landed in was this backend's, which is the + crossed pair -- measured as a SIGSEGV inside [flan.\[clamp\]] during the + *first* macro expansion, before the program had started. - [flan.abi.x86] does not catch it and was never meant to: it guards a - redefinition module, and a macro module deliberately neither defines nor - requires a marker (docs/handoffs/HANDOFF-x86-abi-marker.md says so, and - the reasoning was right for what it covered). This is a third path. The - honest fix is hidden visibility on a macro module's Flan bodies, which - changes the cached object for both backends and wants a lane of its own. + [Build.macro_module] now asks [Emit.program] for [~hidden:true], which + takes every Flan definition in that module out of the dynamic symbol table + and leaves only the [flan.macro.*] thunks [dlsym] has to find. There is + nothing left for the host to interpose, so the two backends never meet + inside an expansion however the host was built. Nothing about the host + moved: it still exports its cells, its globals and [flan_dev_cell] to + redefinition modules exactly as before, because the fix is on the module + that is loaded and not on the process that is loading it. - [--two-process] has no such meeting: the compiler is a separate binary - that LLVM built, the macro module is loaded into it and never into the - program, and the only thing crossing between them is a redefinition module - -- which this session now builds with the same backend as the host. *) - if x86 && merged then - failwith - "flan dev --x86 needs --two-process. A merged daemon expands macros by \ - loading a module LLVM built into the program itself, and a --rdynamic \ - x86 host interposes that module's own prelude bodies -- an LLVM caller \ - lands in an x86 body and the process dies during the first macro \ - expansion. See docs/handoffs/HANDOFF-x86-devloop.md."; + [--x86 --debug] above is still refused, and for a reason that has nothing + to do with this one. *) if merged then start_merged ~debug ~x86 ~file ~sock () else two_process ~debug ~x86 ~file ~sock () diff --git a/lib/emit.ml b/lib/emit.ml index 337ea9e..fdae2c3 100644 --- a/lib/emit.ml +++ b/lib/emit.ml @@ -2578,9 +2578,10 @@ let rec const m (e : Tast.expr) = module can store a new value into it — so tuning a constant live works, which it cannot when its only copy is immutable in .rodata. A release build emits [constant] and gets all the folding back. *) -let emit_global m (g : Tast.global) = +let emit_global m ?(hidden = false) (g : Tast.global) = Buffer.add_string m.out - (Printf.sprintf "%s = %s %s %s\n" (gname g.Tast.gname) + (Printf.sprintf "%s = %s%s %s %s\n" (gname g.Tast.gname) + (if hidden then "hidden " else "") (if g.Tast.gconst && not m.dev then "constant" else "global") (ll g.Tast.gty) (const m g.Tast.ginit)) @@ -2940,9 +2941,48 @@ let macro_thunk m (fn : Tast.fn) = [macros] names the functions that also get a thunk. It is a list of names and not a flag because a macro module carries the whole prelude with it — only the handful of functions that were written [defmacro] are reachable - from outside. *) + from outside. + + [hidden] takes every Flan definition in the module out of the dynamic symbol + table, and it exists for the one build that is dlopened into a process that + already has Flan in it: the macro module, loaded into [flan dev]'s merged + binary. That binary is the program *and* the compiler, linked [-rdynamic] so + a redefinition module can reach its cells, and [-rdynamic] exports every + [flan.*] body it has. ELF gives an executable precedence over a shared + object, so without this the macro module's own copy of a prelude function is + interposed by the host's — and the two copies are not interchangeable. Twice + measured, in two different ways: + + - The host's [flan.rl/with-drawing] is the package's [defmacro] compiled as + an ordinary function, whose body was qualified at the [Ast] level after + the quasiquote had already become a string literal. Expanding through the + host's copy produced an unqualified [begin-drawing] the checker refused. + - Under [flan dev --x86] the host's bodies are the dev backend's and the + macro module's caller is LLVM's, which is the crossed pair: a SIGSEGV + inside [flan.\[clamp\]] during the first expansion, before the program had + started. + + The thunks stay at default visibility, because [dlsym] is how the compiler + reaches them and a hidden symbol is not in the table it searches. Nothing + else in the module is anybody's to call. The C the module links — the + runtime, the shims — is untouched by this and goes on binding to the host's + copy where there is one, which is what keeps [flan_exit_hook] the merged + build installed in reach of a trap raised inside an expansion. *) let program ?(checks = true) ?(dev = false) ?(debug = false) ?(pnames = []) - ?(sanitize = false) ?(macros = []) (p : Tast.program) : string = + ?(sanitize = false) ?(macros = []) ?(hidden = false) + (p : Tast.program) : string = + (* [hidden] and [dev] are opposites and the refusal is here so that they + cannot be written together by accident. A dev build's whole point is that + its cells, its globals and [flan.abi.*] are in the dynamic symbol table + for a redefinition module to bind against; hiding them would leave a host + that links, runs, and silently installs nothing. There is no such thing as + a reloadable macro module, so nothing is lost by saying so out loud. *) + if hidden && dev then + failwith + "Emit.program ~hidden ~dev: a dev build exports its cells so that a \ + redefinition module can reach them, and hiding them would break every \ + reload. [hidden] is the macro module's flag and a macro module is not a \ + dev build."; let m = new_module ~checks ~dev ~known:(fun _ -> true) ~debug ~sanitize p in (* One cell per function, initialised to the function this build compiled. Nothing has been redefined yet, so a dev build starts out behaving exactly @@ -2973,10 +3013,10 @@ let program ?(checks = true) ?(dev = false) ?(debug = false) ?(pnames = []) [{ i32, ptr, ptr } { i32 65535, ptr @flan_dev_reg_enable, ptr null }]\n"; Buffer.add_char m.out '\n' end; - List.iter (emit_global m) p.Tast.globals; + List.iter (emit_global m ~hidden) p.Tast.globals; List.iter (fun (fn : Tast.fn) -> - emit_fn m + emit_fn m ~hidden ~pnames:(match List.assoc_opt fn.Tast.name pnames with | Some ns -> ns | None -> []) fn) diff --git a/test/programs/dev-macro.flan b/test/programs/dev-macro.flan new file mode 100644 index 0000000..20fb6db --- /dev/null +++ b/test/programs/dev-macro.flan @@ -0,0 +1,43 @@ +;;;; dev-loop.flan's shape, with a macro in it, and the macro is the whole +;;;; point of the file. +;;;; +;;;; [flan dev]'s merged daemon is the program and the compiler in one process, +;;;; and the compiler expands a macro by dlopening a module into *itself* -- +;;;; which is now also the program's address space. The module is always built +;;;; by LLVM whatever backend the session uses, and the host is linked +;;;; -rdynamic so a redefinition module can reach its cells, so until +;;;; Build.macro_module hid the module's own Flan definitions the host's bodies +;;;; interposed them. Under --x86 that is a crossed pair and it was a SIGSEGV +;;;; during the first expansion, before the program had run a line. +;;;; +;;;; So [unless] is called twice over: once here at the top level, which is the +;;;; expansion that used to kill the daemon on its way up, and once from a body +;;;; typed in later, which is the same expansion with the program already +;;;; running beside it. +(import agent "vendor:agent") + +(defvar ticks i64) + +(defn parity [n i64] string + (let [out "even"] + (unless (= 0 (% n 2)) + (set out "odd")) + out)) + +(defn step [] i64 + (set ticks (+ ticks 1)) + ticks) + +;;; A *bounded* run of frame boundaries, where dev-loop.flan waits for a +;;; delivery and dev-repl.flan runs long enough to outlast a whole test file. +;;; This program is here to park, and it has to park whether or not the thing +;;; that was delivered to it is one [agent/wait] reports — so the clock ends +;;; it rather than the editor does. Two seconds is long enough for a client to +;;; be served twice and short enough to be well inside a watchdog. +(defn main [] i32 + (agent/start "/tmp/flan-dev-macro-fallback.sock") + (println (parity (step))) + (dotimes [i 400] + (agent/wait 5)) + (println (parity (step))) + 0) diff --git a/test/test_dev.ml b/test/test_dev.ml index 162e992..a968c39 100644 --- a/test/test_dev.ml +++ b/test/test_dev.ml @@ -3275,10 +3275,12 @@ let () = they are loaded into. A C-c C-c that works in [test_reload.ml] and not here is not a dev loop. - [--two-process], because [--x86] refuses the merged daemon and says why: - a merged host exports every [flan.*] body for -rdynamic and so interposes - the prelude bodies of the LLVM-built macro module the compiler loads into - itself. The refusal is asserted below. *) + [--two-process] here and merged below, and both are kept: the merged + daemon is the shape a user gets by default and the one that was refused + on this backend until the macro module stopped being interposable, but + [--two-process] is the arm where the compiler is a separate LLVM binary + and the only thing crossing is a redefinition module. Those are two + different claims about the same flag and neither covers the other. *) let xsock2 = tmp "x86.sock" and xout2 = tmp "x86.out" in (try Sys.remove xsock2 with Sys_error _ -> ()); let xfd2 = @@ -3349,23 +3351,113 @@ let () = (try Unix.close c with Unix.Unix_error _ -> ()); (try ignore (Unix.waitpid [] xpid2) with Unix.Unix_error _ -> ()) end; - (* And the merged daemon refuses --x86 by name. A refusal is the whole - deliverable here: the alternative was a SIGSEGV inside a prelude - function during the first macro expansion, before the program started, - with nothing having said a word. *) - let mout = tmp "x86merged.err" in - let mcode = - Sys.command - (Printf.sprintf "%s dev programs/dev-repl.flan --x86 > /dev/null 2> %s" - (Filename.quote flan) (Filename.quote mout)) + (* And the *merged* daemon on this backend, which used to be refused by + name and is the case the refusal was standing in for. + + [programs/dev-macro.flan] and not the dev-repl above, and the choice is + the whole test: it calls a prelude [defmacro] at the top level, so + starting it means the compiler built a macro module and dlopened it into + *this* process -- which is an [--x86] [-rdynamic] host, and until + [Build.macro_module] hid that module's own Flan definitions the host's + bodies interposed them. The failure was a SIGSEGV inside + [flan.\[clamp\]] during the first expansion, before the program had run + a line, so a daemon that reaches a bound socket at all is most of the + deliverable. + + The rest of it is one daemon carrying everything this combination has to + be able to do: C-x C-e, a C-c C-c whose body calls a macro *again* with + the program now running beside the compiler, and — because the fixture's + [main] returns after one delivery — the park and the [rerun] that the + merged build exists for. *) + let msock = tmp "x86merged.sock" and mout = tmp "x86merged.out" in + (try Sys.remove msock with Sys_error _ -> ()); + let mfd = + Unix.openfile mout [ Unix.O_WRONLY; Unix.O_CREAT; Unix.O_TRUNC ] 0o600 in - let msaid = In_channel.with_open_bin mout In_channel.input_all in - if mcode = 0 then fail "the merged daemon accepted --x86 (exit 0)"; - if not (contains_sub msaid "--two-process") then - fail "the merged daemon refused --x86 without naming the remedy: %S" - msaid; + let mpid = + Unix.create_process flan + [| flan; "dev"; "programs/dev-macro.flan"; "-s"; msock; "--x86" |] + Unix.stdin mfd Unix.stderr + in + Unix.close mfd; + if not (listening ~pid:mpid msock) then begin + fail "the merged --x86 daemon %s (%S)" !listen_why + (In_channel.with_open_bin mout In_channel.input_all); + (try Unix.kill mpid Sys.sigkill with Unix.Unix_error _ -> ()) + end + else begin + let c = connect msock in + let said r = Option.value ~default:"" (Wire.string_field r "message") in + let parked () = + match Wire.field (request c "(:op \"describe\")") "parked" with + | Some { Form.v = Form.Sym "t"; _ } -> true + | _ -> false + in + (* C-x C-e first, while the program is certainly still in its loop: an + expression is answered at a frame boundary and a parked program has + none. Retried rather than asked once, because the daemon binds its + socket in [merged_setup] and the program's own thread only starts + after that returns — so the first ask can arrive before there is an + agent to reach, which is a race with the startup and not a result. *) + let answered = ref "" in + let asked () = + let r = + request c + "(:op \"eval-expr\" :code \"(+ 2 3)\" :file \"programs/dev-macro.flan\")" + in + status r = "ok" + && (answered := Option.value ~default:"" (Wire.string_field r "value"); + true) + in + if not (await asked) then + fail "merged --x86 C-x C-e never reached a frame boundary" + else if !answered <> "5" then + fail "merged --x86 C-x C-e answered %S" !answered; + (* And a C-c C-c whose body is itself a macro call. This is the expansion + that matters most here: the macro module is already resident, the + program is running in the same process, and the caller landing in the + host's body instead of the module's is exactly the crash. *) + let r = + request c + "(:op \"eval\" :code \"(defn step [] i64 (unless false (set ticks (+ \ + ticks 10))) ticks)\" :file \"programs/dev-macro.flan\")" + in + if status r <> "ok" then fail "merged --x86 C-c C-c: %s" (said r); + if not (await parked) then + fail "the merged --x86 program never parked after main returned"; + (* And round [main] again in the same process, which is what the merged + build exists for. "ok" only says the op was accepted, so the check is + what the program prints: every reply drains its stdout into [output], + and [parity] -- the macro-using function -- is what it prints. So a + line appearing there after this mark is a second run having really + happened, with the expansion still good in it. + + The second run parks like the first, which is what there is to wait + for; asking for the text immediately would be asking before the + thread had been let go. *) + let mark = Buffer.length output in + let r = request c "(:op \"rerun\")" in + if status r <> "ok" then fail "merged --x86 rerun: %s" (said r) + else begin + let printed () = + let s = Buffer.contents output in + let s = String.sub s mark (String.length s - mark) in + contains_sub s "even" || contains_sub s "odd" + in + (* [parked] first, and the order is not style: it is the call that asks + the daemon anything, and so the only thing that drains the program's + stdout into [output] at all. *) + if not (await ~ms:10000 (fun () -> parked () && printed ())) then + fail "merged --x86 rerun printed %S, so main did not run again" + (let s = Buffer.contents output in + String.sub s mark (String.length s - mark)) + end; + ignore (request c "(:op \"close\")"); + (try Unix.close c with Unix.Unix_error _ -> ()); + (try ignore (Unix.waitpid [] mpid) with Unix.Unix_error _ -> ()) + end; List.iter (fun f -> try Sys.remove f with Sys_error _ -> ()) - [ xsock2; xout2; mout ]; + [ xsock2; xout2; msock; mout ]; List.iter (fun f -> try Sys.remove f with Sys_error _ -> ()) [ sock; out; bsock; bout ];