Joseph Ferano
3eaa3e23bd
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.
135 lines
9.3 KiB
Markdown
135 lines
9.3 KiB
Markdown
# 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-<key>.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.
|