From 3eaa3e23bd4a67bce1022274a5da9311531ef841 Mon Sep 17 00:00:00 2001 From: Joseph Ferano Date: Thu, 17 Sep 2026 19:50:29 +0700 Subject: [PATCH] A macro module keeps its own prelude, and --x86 gets the merged daemon 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 compiler expands a macro by dlopening a module into that same process, and the module is built by Emit.program whatever backend the session uses -- so under --x86 the caller was LLVM's and the body it landed in was the dev backend's, which is a crossed pair. It died with SIGSEGV inside flan.[clamp] during the first expansion, before the program had run a line, and Dev.start refused the combination rather than do that. Build.macro_module now asks Emit.program for hidden visibility on the module's own Flan definitions. There is nothing left for the host to interpose, and the flan.macro.* thunks stay exported because dlsym is how the compiler reaches them -- nm -D on the built module lists those three and nothing else of Flan's. The -Wl,-Bsymbolic that had been binding everything locally since 65d14f4 goes with it: the module links its own flan_rt.c, and binding that locally aimed its calls at a runtime flan_rt_init never ran on, with a null flan_exit_hook, so a trap raised inside an expansion would have exited the process instead of parking it. Nothing about the host moved, which is what keeps redefinition modules reaching its cells, its globals and flan_dev_cell. hidden defaults to false, and the 540 IR files this compiler emits for the test corpus are byte-identical to the ones before it. test_dev.ml's assertion that the merged daemon refuses --x86 becomes the session it was standing in for: dev-macro.flan calls a prelude macro at the top level, so the daemon coming up at all is the old crash not happening, and one build then carries C-x C-e, a C-c C-c whose body calls a macro again, the park and the rerun. --- NEXT.md | 17 +-- docs/handoffs/HANDOFF-x86-devloop.md | 13 +- docs/handoffs/HANDOFF-x86-macro-visibility.md | 134 ++++++++++++++++++ lib/build.ml | 30 ++-- lib/dev.ml | 52 +++---- lib/emit.ml | 40 +++++- test/programs/dev-macro.flan | 43 ++++++ test/test_dev.ml | 130 ++++++++++++++--- 8 files changed, 380 insertions(+), 79 deletions(-) create mode 100644 docs/handoffs/HANDOFF-x86-macro-visibility.md create mode 100644 test/programs/dev-macro.flan 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..635df6d --- /dev/null +++ b/docs/handoffs/HANDOFF-x86-macro-visibility.md @@ -0,0 +1,134 @@ +# 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. + +## 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 this is the argument made +mechanical rather than a sample. + +`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`.** Its package is `sand.flan`, which calls `rl/with-drawing`. If hidden + visibility did not cover 65d14f4's 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..dba850d 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,36 @@ 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 = 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 +3001,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 ];