flan/docs/BUGS-2026-09-18.md

168 lines
12 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# Bug hunt, 2026-09-18
Five search agents swept the runtime, checker, backends, dev loop, and Emacs client.
Everything below was either executed to failure or traced to the exact line. The five
marked **DISPATCHED** have fix lanes; the rest are recorded here and wait.
## Dispatched
### 1. `borrowed` grants the borrow flag to whole subtrees — moves inside container targets vanish
**Repealed, not fixed (2026-09-18):** the flow analysis this hole lived in was removed wholesale
(spec-memory.md, "The repeal"). The trigger programs now compile by design and misbehave at run time.
`lib/check.ml:2062-2076`. The flag gates both `moved` (2003) and `global_borrow` (2033),
and is set across the entire checking of the target: the index argument of `at`, the base
of any `Field`. A move nested there is never recorded.
Confirmed by execution, two programs:
- `(println (at rows (eat w)))` then `(free w)` — double free, exit 134.
- Same shape with a move-only global `g`: accepted, `g` freed, `(len g)` reads a freed
header, exit 0 silent. Defeats the rule test/test_flan.ml:1067-1068 pins.
Fix direction: narrow the flag to the target's own read — restore `ctx.borrow` for the
index of `at`; treat `Field` as simple only when its base chain bottoms out at a `Var`.
### 2. A `while` condition is move-checked outside `in_loop` — double free on iteration two
**Fixed, then repealed (2026-09-18):** the fix merged (47cb46a) and was removed the same day with the
whole flow analysis. The trigger compiles and aborts in the allocator at run time, by design.
`lib/check.ml:1688-1691`. The condition is checked before `in_loop` is entered, but
emit re-runs it every trip (`emit.ml:1838`). A condition that moves a local frees it
once per iteration. Confirmed: glibc double-free abort, exit 134.
`dotimes`' count (2504) and `loop`'s inits (2561) are also outside but evaluate once — correct.
Fix: check the condition inside `in_loop`.
### 3. A checked declaration that fails to build or deliver leaves a NULL cell the session will call
`lib/session.ml:601` commits `t.program` before `Dev.eval` builds (`dev.ml:603` can raise)
or delivers (`dev.ml:596` can refuse — e.g. queue full at `vendor/agent/flan_agent.c:1186`,
which a parked program guarantees after 64 installs since nothing drains the ring while
parked). The session keeps the declaration; the process never got the body; the next
thunk's install prologue interns the name with `cell = NULL` (`runtime/flan_dev.c:65`)
and `body_of` calls through it with no null test (`emit.ml:1472`) — jump to address 0
on the game thread. Nearest reachable mechanism to FIX.org's transient SIGSEGV; the
stranded-Vec hypothesis there was read out and refuted (reloads never redefine a host
global; storage addresses are stable; `global_borrow` holds).
Related, same lane: the queue-full refusal says "the program is not calling agent/poll",
which is the wrong cause for a parked program.
### 4. The merged build never drains the program's stdout pipe while serving a request
fd 1 is a 64K pipe whose only reader is `accept_loop`'s select (`lib/dev.ml:2739`), not
running while `serve` handles a request. `eval_expr`'s wait (664-672) and
`run_render_thunk`'s wait (1026-1036) poll for five seconds and never drain, so a
program that prints (sand.flan does) blocks in `flan_write_stdout`, stalls the frame
thread for the whole timeout, and gets the false diagnostic "is it calling (agent/poll)?".
Same root on the exit path: `flan_merged_exit`/`park` `fflush(NULL)` before
`program_state = PROGRAM_PARKED` (dev.ml:3000/3017/3024), so a full pipe delays the park
and `rerun` refuses a finished program as "already running".
Also same family: `rt_die` (`runtime/flan_rt.c:384-388`) starts with `fflush(stdout)`
a bounds trap can hang on the full pipe — and calls `exit(134)`, the route `die_now`
documents as unsafe (atexit/ELF destructors want the loader lock a dlopening thread may
hold); it should `_exit` like the break loop does.
### 5. x86 shifts are always 64-bit, so the count is masked to 63 instead of width1
`lib/x86.ml:2530-2534` / `shift_cl` at 369 (`rex ~w:true` unconditionally). The comment
claims the hardware masks to operand width; it masks to 63. `emit.ml:2041` masks to
`bits-1` explicitly (NEXT.md "Sharp edges" records this as the language's rule). Six
confirmed divergences, e.g. `(<< x 32)` on i32: LLVM 1, x86 0; `(>> i8min 8)`: LLVM
-128, x86 -1. Invisible because `spike/x86/survey.sh:80` never globs `spike/js/*.flan`,
where `p1-int-semantics.flan` already catches it — widen the glob in the same lane.
Same wide-compute root, second divergence: float→int overflow under `--no-bounds-checks`
gives 0 on x86 (64-bit `cvttsd2si` then truncate) vs INT_MIN on LLVM. Acknowledged-UB
territory; fix or record, the lane's call.
## Recorded, not scheduled
- **Region guard never asks about elements** — **repealed, not fixed (2026-09-18)**: with the flow
analysis gone, `free` of an `at` result is no longer a checker question; the mixed-allocator
construction is legal and its misuse is a run-time matter. (`lib/check.ml:1272`, refusals 4092/4165):
a heap-backed inner Vec pushed into an arena-backed outer passes the guard; `at` then
hands out an owning header, `free` accepts it, and the later read is a confirmed UAF
(printed garbage). Breaks the premise stated in the clone note at check.ml:4152.
Candidate fixes: refuse `free` of a non-binding target, or region-check the element at
`push`/`put`. Sixth on the list; needs a deliberate mixed-allocator construction.
- **Reversed slice under `--no-bounds-checks`** — **fixed**. The `lo <= hi` test is a
representation invariant, not a bounds check, but sat behind `if f.md.checks` in both
backends (`emit.ml:1037`, `x86.ml:2164`; same for SliceFromPtr's `n >= 0`), so a
negative-length slice reached user code. Split in both backends: `hi <= len` stays
behind the flag, `lo <= hi` and `n >= 0` are now emitted in every build, through the
same slice-failure path. `flan_vec_as_slice` was the model. See "A slice's length word
is a count" in docs/BUILT.md.
- ~~**`reg leaks` lies at >3072 live blocks**~~ — **fixed**. Compaction now also asks
whether there is an eighth of a table's worth of dead to reclaim, so a table full of
live blocks stops thrashing the epoch; `flan_dev_reg_by_type` returns 1 with an unread
count rather than zero rows, and the agent refuses in a sentence; a note dropped by a
genuinely full table says so on stderr once. `test/dev_limits.c regfull` pins it under
a writer thread: 3100 live blocks, 200 asks, 199 wrong before and 200 right after.
- **Emacs framing**: a truncated frame raises wrong-type instead of the timeout message
and leaves the partial frame in the buffer, desyncing every later request by one frame
(`flan-dev.el:220`); `extract-reply` reads before it deletes, so an unreadable payload
wedges the connection permanently (:187). One ordering fix covers both.
- **Emacs poll vs watch**: `flan-dev--poll` bypasses `flan-dev-settle-hook` and consumes
the watch's reply; the two consumers stay swapped for the session (`flan-dev.el:406`).
Also `request` reconnects before running the settle hook — 30s freeze after a daemon
restart with the watch armed.
- **C-x C-e at point-min** installs an empty declaration (`flan-dev.el:1745`
`backward-sexp` no-op unchecked); same predicate fires inside strings and misjudges
narrowed buffers.
- **`flan-connect` + `flan-dev-quit` kill two sessions** (`flan-dev.el:721`): quit sends
`close` down the current connection and kills the daemon it started for another program.
- ~~**`reg at` TOCTOU**~~ — FIXED. The gate was checked three round trips before the
render thunk ran, and nothing held the break across the ~300ms build, so a `restart`
in between let the thunk chase freed memory with the gate's blessing. The sound fix is
agent-side: the render job now carries the condition it was built under. `inspect` by
address delivers its module as `stopped-only <path>`, the job header keeps the flag,
and `flan_agent_poll` drops such a job — counted, handle closed, nothing installed and
nothing called — when `depth` is 0 at the moment it is claimed. That read is sound
rather than narrower: only the game thread polls and only the game thread raises
`depth`, so `depth > 0` seen inside a poll means *this* thread is parked in the break
loop and cannot be running a frame. The daemon reads the `refusals` count either side
of the delivery and surfaces the agent's own sentence — "the program resumed while this
inspection was being built — stop it again and re-ask" — instead of the timeout's
wrong-cause "is it calling (agent/poll)?". `reg at`'s depth gate stays as the front
door. `locals`, `globals` and `inspect` by slot are deliberately *not* stopped-only:
they hold no address, re-deriving the frame through `snap_top` or binding a global by
name at thunk-run time, so they carry no blessing that can expire. Pinned in
`test_agent.ml`, where a stopped-only job delivered to a program that is definitively
running is dropped while the eval module beside it installs — one install, not two.
- ~~**defenum values never range-checked to i32**~~ — FIXED. Every resolved member value,
explicit or autoincremented, is range-checked against i32 in the parser before the collision scan,
so the scan compares the numbers the program will actually have. Out of range is refused
by name (`parse/enum-value-out-of-range`); explicit-duplicate aliasing stays legal.
- **NaN sign** — **fixed**. LLVM constant-folded `0.0/0.0` to `nan`, x86 computed `-nan`
— a stdout DIFFER on a two-line program. Resolved by canonicalizing the *printed* form
rather than the arithmetic: `flan_f64_to_bytes` and the two dev emitters render any NaN
as unsigned `nan`, which is what `format-f64` in the prelude always did. Pinned in
`test/programs/format.flan`. See docs/BUILT.md.
- **x86's slice-from-ptr refusal is the wrong sentence**: `x86.ml` still reports a
negative promise through `flan_slice_error` — "slice [0 -2) is out of bounds for
length 0", naming a range and a length the caller never wrote — where `emit.ml` has
its own `flan_slice_promise_error`. Same condition and same exit on both sides, only
the text differs. The survey cannot see it: `bounds.flan` picks its case out of
`(at args 1)` and `survey.sh` runs every program with no arguments, so nothing in the
corpus reaches the `n = -2` case on the x86 path. Noticed while making the check
unconditional (which did not change what it prints); the fix is one `bounds_call` with
one extra instead of three.
- **`emit.ml:3369` transient test ignores `new_globals`**: on the `retains=false` path a
module first to intern a global gets dlclosed; zero-init makes it moot today, a literal
init would dangle.
- **x86 to-bytes helper-return clobber**: `(defn numstr [n] (string (i64->bytes n)))`
works on LLVM, garbage on x86 — documented caller's-problem UB, but the divergence
makes the documented edge invisible.
- **map-grow at log2cap>=40 quotes a stale failure** (`flan_rt.c:2438`, pre-dates
yesterday).
## JS backend (deprioritised 2026-09-18, do not schedule)
- `lib/js.ml:889` still matches 1-arg `*ToBytes`; commit 81b807f made them 2-arg, so
every number-printing program is refused — JS corpus MATCH went 24 → 3 and `@js`
stays green because a refusal counts as success. Needs a MATCH floor in strict mode.
- f32 literals keep double precision (`js.ml:671` discards the fkind; `Math.fround` it).
- The cast-range trap quotes the cast's column, not the operand's (`js.ml:1077`).
## Verified clean, don't re-hunt
`flan_map_remove` (two randomized ASan harnesses incl. non-power-of-two cells, zero
mismatches), the grow-path overflow guards, the seqlock protocol itself, the park/rerun
condvar, `to_bytes`'s 64-byte slot arithmetic, elisp multibyte framing (unibyte
throughout, tested), JS struct value semantics and 64-bit integer semantics (broad
probes), division sign/INT_MIN traps at all widths, `if`/`match` dead-set joins,
`defer`'s kill-at-registration, index widening rules.