Joseph Ferano
5a46ab488f
Emit.program ~hidden and ~dev are opposites: a dev build exports its cells, globals and flan.abi.* so that a redefinition module can bind against them, and hiding those would give a host that links, runs and silently installs nothing. Nothing in the types stopped the combination being written, so it fails with a sentence instead. Build.macro_module is the only caller that asks for hidden and it never asks for dev. The handoff's verification is narrowed to what it actually covers: the 540-file IR diff is about Emit.program's default, and the new path is read off nm -D on the linked module. It also now says why sand-headless.flan is the fixture that checks 65d14f4 -- it reaches rl/with-drawing through an import, which is the package-macro shape the reproduced bug had.
150 lines
11 KiB
Markdown
150 lines
11 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.
|
|
|
|
**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.
|