diff --git a/BUILT.md b/BUILT.md index a66685b..c30c1eb 100644 --- a/BUILT.md +++ b/BUILT.md @@ -18,7 +18,9 @@ assignable, which makes the generated step its only writer. level of a function body. Each one is checked in place, then registered on the context; it emits nothing where it stands. Function exit runs them innermost-first, and an explicit `return` runs the ones registered *above* it — a defer written below a return has not executed yet and must not fire. A trap runs none of them, which follows from the -bounds-check shape (`noreturn` then `unreachable`) rather than being a separate decision. +bounds-check shape (`noreturn` then `unreachable`) rather than being a separate decision. **Amended** once a bounds +failure became a signal: an *answered* one leaves through the unwind path and runs them like any other transfer, an +unanswered one still runs none. See "An index out of range is a condition" at the foot of this file. `defer` inside a `let`, a loop or a branch is **rejected**, not accepted with function scope. It would run once at function exit rather than once per iteration, and that is the silent-wrongness class the rule below is about. @@ -4195,3 +4197,131 @@ bad.flan:8:3: unknown function mystery **No editor work was needed and none was done.** Flycheck and a structured JSON report were both considered and are not wanted: the workflow is compile-at-the-end, not live linting, and the GNU first line already buys the clickable list. +## The four raylib lines siam-farmer needed + +`PORTING.md` measured the author's game against the binding and found the renderer unwritable in a default build. +Four lines closed it, and the interesting part is not the lines. + +`draw-texture-pro` is the one that mattered. It is the only call in the package that takes both a source rectangle and +a destination rectangle, which is what a tilemap is: `source` picks a cell out of an atlas, `dest` says where it lands +and how big, and a 16px tile drawn at 4x is a dest four times the source. `draw-texture-rec` has the source and no +scale; `draw-texture-ex` has the scale and no source. Neither half draws a tile. + +It *was* reachable — with `FLAN_RAYLIB_H` exported the importer brings it in with 250-odd others — and that is the +finding worth keeping. `vendor/raylib/headers` keeps the import opt-in on purpose, so a build needs libraylib linkable +and not raylib-devel installed. That property is worth keeping and it means **the default build had no draw call for a +grid-based game**. The rule that follows: *a raylib function on a game's per-frame path is hand-written in +`raylib.flan` and checked against the header; it is not left to the import.* The import widens the surface; it must +not be load-bearing. + +The other three: `image-from-image` (the non-mutating `image-crop` — carving a sheet into twenty tiles with +`image-crop` destroys the sheet on the first one, and 5.5 has no `ImageCopy`), `window-ready?` (an engine's "am I +already running" guard), and `left-shift 340` in the `Key` `defenum`. Nothing else went into the enum: it carries the +keys that have a customer, and `PORTING.md` §5 checked every other value the game touches and found them all present. + +**What could be tested, and what could not.** `PORTING.md` asked for an acceptance case making raylib compute with the +source rect so a permuted `Rectangle` goes red. That cannot exist for `DrawTexturePro` — it needs a GL context, and +`raylib.flan`'s Shapes comment already says none of the drawing calls can be in the table. So `raylib-ffi.flan` links +it instead: the call sits behind `(when (rl/window-ready?) …)`, false headless, so the shim is generated and the +symbol resolves at link time and the body never runs. That catches a name or an arity libraylib does not have. It does +**not** catch the argument order, and three structs in a row is where an argument order goes wrong. Only looking at +the screen catches that, and saying so is better than a test that implies otherwise. + +The computed case moved to `image-from-image`, which is CPU-side and is exactly what the Images section says is +assertable. `raylib-image.flan` carves one 6×3 sheet twice at two different `y`s and then re-reads the sheet: `x`, +`y`, `width` and `height` are each pinned by an answer that moves if they do, and the sheet surviving both carves is +what distinguishes this from `image-crop`. Bind it to `ImageCrop` by mistake and the second carve reads out of a 2×1 +image and the case goes red. + +## An index out of range is a condition + +`flan_bounds_fail` printed the source location, the index and the length, and called `exit(134)`. That was defensible +while `flan dev` was two processes. It is not now: the compiler runs **inside the program**, so the trap took the +whole session with it — and the session not having to restart is the project's thesis. `PORTING.md` found the customer +and found it on the most ordinary path there is: a grid indexed straight from a mouse position is out of bounds the +first time the pointer leaves the window, and the author's Common Lisp port had to add an `in-bounds-p` to survive it. + +A failed bounds check now signals **`BoundsError`** with `error`, the same way a failed allocation signals +`StorageExhausted`. `runtime/flan_rt.c` has `flan_bounds_error` and `flan_slice_error`; each walks the handlers, then +offers the break loop, and only if neither transferred does it tail into the `flan_bounds_fail`/`flan_slice_fail` that +were there before — same message, same status 134. Nothing was removed; a die was demoted to a last resort. + +`(defstruct BoundsError [low i64 high i64 length i64])` is in the prelude. Fixed numeric fields and no rendered +message, for `StorageExhausted`'s reason: formatting allocates, and a condition raised on a path that may be out of +storage must not. `low` and `high` are the same index for an `(at xs i)` and the two ends of the range for a `(slice +xs lo hi)`, so **one** condition type covers both and a handler that wants to survive a bad index writes one clause +rather than two. The three `int64_t`s in `flan_rt.c` have to agree with that `defstruct` field for field — the same +hand-kept agreement `flan_name_id` already keeps with `Check.type_id`, and for the same reason: a struct is a layout, +a type is a number, and neither side can see the other. + +### Why no restart is established at the failing index + +This is the decision, and it is a decision rather than an omission. + +`alloc_guard` offers `retry` and `file_guard` offers `retry`/`use-value` **because their attempt is repeatable**. A +handler frees something and the allocation succeeds; a handler supplies another path and the open succeeds. That is +what makes them `plan.org`'s named exceptions to "restarts go at the resync point, once" — a restart at an outer loop +cannot re-attempt an allocation, and only the allocation site can. + +Nothing a handler can do makes index 51 valid for a length-50 array. There is no attempt to re-run, so there is +nothing for a site restart to resume into, and bounds falls on the **default** side of that rule. + +- **`continue` is wrong.** `(at xs i)` has to produce a value of the element type and there is none to produce. It + would mean something in the `(set (at xs i) v)` position and nothing in the other, and `at` is one form. +- **`use-value` for the index is the near miss and is still wrong.** It costs the hot path, not just the cold block: + `idx` is an SSA value feeding the gep, and retrying needs it in an alloca reloaded per attempt, plus a restart frame + pushed and popped on **every** indexing operation. What it buys is a *different element*, silently — the class of + answer this codebase refuses everywhere else. +- **What actually answers a bad index is already on the stack.** A frame loop's `continue` — `sand.flan`'s shape — is + an ordinary `restart-case`, `flan_find_restart` walks to it, and the break loop lists it. Signalling is the whole + fix. A site restart would add nothing the frame loop does not already offer, at a cost on every index in the + program. + +### Release, and what a build without the agent does + +`flan_break_hook` is NULL unless the agent package was imported, so a release build — or any program that did not +import it — signals, finds no handler, finds no hook, and dies with the message and the status it always had. That is +still right: there is nowhere to stand. The change is not "bounds failures stopped being fatal"; it is "a bounds +failure is now answerable, and is fatal when unanswered". + +### Defer, which had to be answered rather than inherited + +The note at the top of this file said *a trap runs no defers, which follows from the bounds-check shape (`noreturn` +then `unreachable`) rather than being a separate decision*. The shape changed, so the consequence could not be +inherited. It now splits: + +- **An answered bounds failure runs the defers.** `guard` routes through `current_pad`, which with no enclosing + `restart-case` sets `f.unwound` and branches to the function's unwind block — the same path `return` uses, which is + where §5's defers already live. So a transfer out of a bad index is an ordinary transfer and runs cleanup + innermost-first, like every other one. +- **An unanswered one still runs none**, because it is still a `rt_die()` inside C with no Flan frame involved. + +`bounds-condition.flan` asserts the first of those directly: five `defer`s across five abandoned and finished frames, +counted. + +### Both halves are tested, and they are different tests + +`bounds-condition.flan` (acceptance table, at `-O2`, `-O0` and as a dev build) is the **answered** half: a +`handler-bind` over five routes to a bad index, taking the frame loop's `continue` each time. It does not import the +agent, so `flan_break_hook` is NULL in all three rows and nothing there says anything about the break loop. + +`dev-break-bounds.flan` (`test_dev.ml`) is the half the change is actually for: **nothing handles it**, so the signal +walks the handlers, finds none, and reaches the hook. The daemon sees a program that stopped without being told to; +`BoundsError` is what the break reports; its own name resolves to a layout whose fields are `low`, `high`, `length`, +so the conditions buffer shows the numbers with nothing special-cased for it; the restart list is exactly +`continue` — the *program's* own, which is the visible consequence of establishing none at the site — and taking it +resumes, with an ordinary evaluation working on the far side. That last step is the whole claim: the session outlived +the index. + +### Vec and Map + +`(at v i)` and `(at arr i)` are the same form in the source, so shipping one signalling and the other exiting would +have read as a bug. A `Vec`'s bounds check lives *inside* `flan_vec_at` and `flan_vec_as_slice` rather than in emitted +IR (which is also why `--no-bounds-checks` never reached it), so both grew a trailing transfer-channel parameter and +`Emit`'s `Rt` arm guards those two symbols and no others — they are the only ones in that family that can transfer; +everything else there is arithmetic over a container header. + +**A `Map`'s bounds and a `Vec`'s stale-allocator check still die.** `flan_vec_stale_fail` is a different kind of +failure — the region a container lived in was released, and there is no frame to go back to that would not read freed +memory — and the map path was left alone rather than converted half-way. Written down here so it is a known edge +rather than a discovery. diff --git a/NEXT.md b/NEXT.md index 5f22367..166f5e8 100644 --- a/NEXT.md +++ b/NEXT.md @@ -42,19 +42,48 @@ skip a frame and carry on rather than die — exactly the case where a non-idemp Three things, in order. The first two are one line each and unblock a real game. -**1. `DrawTexturePro` is not bound, and it is the one true blocker for `siam-farmer`.** See `PORTING.md`, written -against both the Clojure implementation and the WIP Common Lisp port. Every tile in both goes through it; -`DrawTextureRec` does not scale and `DrawTextureEx` takes no source rect, so the renderer **cannot be written at all** -in a default build. It is reachable only through the opt-in `FLAN_RAYLIB_H` import, which `vendor/raylib/headers` -deliberately keeps optional. One `declare-c` line. +**1. `DrawTexturePro` is not bound — DONE.** It is `draw-texture-pro` in `vendor/raylib/raylib.flan` now, hand-written +beside `draw-texture-rec` and read off a raylib header rather than remembered. `image-from-image` and `window-ready?` +went in with it. The rule the gap exposed is written down in `BUILT.md` and `PORTING.md` §1: *a raylib function on a +game's per-frame path is hand-written and header-checked, not left to the opt-in import* — the import widens the +surface and must not be load-bearing, because the default build has no `FLAN_RAYLIB_H` and still has to draw. -**2. `Key` has no `left-shift`.** Both implementations use shift+1..5 to pick the tilemap. One enum member. +**2. `Key` has no `left-shift` — DONE.** `left-shift 340`, and nothing else: `PORTING.md` §5 checked every other enum +value the game touches and they were all already right. -**3. An out-of-bounds index should signal a condition, not `exit(134)`.** `PORTING.md`'s own first recommendation -after the binding lines. The game indexes grids straight from mouse coordinates — `game.lisp` had to add an -`in-bounds-p` to survive it — and today a trap ends the process *and* the `flan dev` session with it. `flan_rt.c`'s -`flan_bounds_fail` should go through the condition machinery, which would make it a break loop with a restart instead -of a dead session. Note this interacts with the merged one-process build: killing the program now kills the compiler. +**3. An out-of-bounds index should signal a condition, not `exit(134)` — DONE.** A failed bounds check signals +`BoundsError` with `error`; `runtime/flan_rt.c`'s `flan_bounds_error`/`flan_slice_error` walk the handlers, then offer +the break loop, and tail into the old `flan_bounds_fail` message and status only if nothing answered. `Vec`'s two +checks are plumbed the same way, since `(at v i)` and `(at arr i)` are one form in the source. + +**No restart is established at the failing index**, and `BUILT.md` has the argument: `retry` exists for allocation and +for files because those attempts are *repeatable*, and nothing a handler can do makes index 51 valid for a length-50 +array. `use-value` for the index would cost every indexing operation a restart frame and buy a silently different +element. What answers a bad index is the restart the program already had — a frame loop's `continue`, `sand.flan`'s +shape — which is on the restart stack and on the break loop's list without anything being pushed at the site. + +**Defer** had to be answered rather than inherited, since the `noreturn`-then-`unreachable` shape is what the old note +followed from: an answered bounds failure leaves through the function's unwind block, which is `return`'s path, so it +runs the defers; an unanswered one still runs none. `test/programs/bounds-condition.flan` counts them. + +Two tests, because there are two paths. `bounds-condition.flan` is the *answered* half — a `handler-bind` taking +`continue` over five routes to a bad index, at `-O2`, `-O0` and as a dev build. `dev-break-bounds.flan` in +`test_dev.ml` is the half this was built for: nothing handles it, the break loop reports `BoundsError`, the layout for +that name resolves to `low`/`high`/`length`, the only restart on offer is the program's own `continue`, and taking it +resumes with the session intact. + +**Still dying, deliberately:** a `Map`'s bounds check and `flan_vec_stale_fail`. The stale-allocator case is a +different kind of failure — the region the container lived in was released — and there is no frame to go back to that +would not read freed memory. The map path was left alone rather than converted half-way. + +### What is left on `PORTING.md`'s list + +Tier 0 is finished. Of Tier 1, items 5 and 6 are the next two and they are both dev-loop work rather than language +work: **a watch for a *running* program** (the stopped-stack inspector is a different tool for a different moment — +`watch.clj` + `spy-num` is the shape, and the numeric accumulator for hot loops is the part that is least obvious), +and **frame rollback as a worked example** — `snapshot`/`restore` callbacks beside the `continue` restart, which is +now genuinely reachable from a bad index and so is worth more than it was yesterday. `bounds-condition.flan` shows an +abandoned frame leaving half-written state behind; rollback is what finishes that thought. **What `PORTING.md` says NOT to build, with evidence:** escaping closures (one capture site, fixed by one parameter), `Handle`/pools, `Result`/`try`, `handler-case`, `loop`/`recur` and tail calls, user allocators, structural typing — @@ -66,6 +95,13 @@ state holds fixed arrays or `Vec`s. **raylib 6.0 is not urgent:** all seven struct layouts on the game's path and every enum value it touches are byte-identical between 5.5 and the vendored 6.0. +### One flaky test, measured rather than suspected + +`test_dev.ml`'s first block fails about one run in four with *"the merged program never bound +…/agent.sock"*. **It is not new** — reproduced at `54027ca`, before any of today's work, at the same rate. It is a +startup race in the one-process daemon's socket bind, not a real failure, and it makes "the suite is green" a +statement that needs a second run to make. Worth fixing before it trains someone to re-run on red. + ### Left mid-flight when the session ended Both lanes committed their main work and died on trailing polish; both are merged and the suite is green. diff --git a/PORTING.md b/PORTING.md index d88c68e..9289b55 100644 --- a/PORTING.md +++ b/PORTING.md @@ -27,6 +27,11 @@ hypothetical. ## 1. The one thing that cannot be written at all +> **Bound, 2026-09-13.** `draw-texture-pro`, `image-from-image`, `window-ready?` and +> `Key/left-shift` are all hand-written in `vendor/raylib/raylib.flan` now. The section +> is kept as it was written, because the reasoning is the part worth having; what the +> fix cost, and what could and could not be tested, is in the box at the end of it. + ### `DrawTexturePro` is not bound Every tile in this game is drawn by one call: @@ -77,6 +82,40 @@ And one enum member, not a function: **`raylib.flan`'s `Key` has no `left-shift` `KEY_LEFT_SHIFT` (340). The `defenum` carries a deliberate subset — the keys `sand.flan` uses — and this is one member short of what this game needs. One entry. +### What the four lines actually cost, and what a test could say about them + +All four landed together: `draw-texture-pro` beside `draw-texture-rec`, +`image-from-image` beside `image-crop`, `window-ready?` beside `window-should-close?`, +and `left-shift 340` at the end of the `Key` `defenum`. The three signatures were read +off a raylib header rather than remembered, and all three are unchanged across 5.1, 5.5 +and the 6.0 this game vendors. + +**The test suggestion further down this file was wrong and is corrected here.** Tier 0 +asked for "an acceptance case that makes raylib compute with the source rect so a +permuted `Rectangle` goes red". `DrawTexturePro` cannot have one: it needs a GL context, +and `raylib.flan`'s own Shapes comment already says none of the drawing calls can be in +the acceptance table. What `raylib-ffi.flan` does instead is link it — the call sits +behind `(when (rl/window-ready?) …)`, which is false headless, so the shim is generated +and the symbol is resolved at link time and the body never runs. That catches a name or +an arity that does not exist in libraylib. It does **not** catch the argument order, and +three structs in a row is exactly where an argument order goes wrong. Only looking at +the screen catches that. + +The computed test the suggestion wanted does exist — on `image-from-image`, which is +CPU-side and is what that Images section comment says is assertable. +`raylib-image.flan` carves the same 6×3 sheet twice, at two different `y`s, and then +re-reads the sheet: the rectangle's `x`, `y`, `width` and `height` are each pinned by an +answer that changes if they move, and the source surviving both carves is what +distinguishes this from `image-crop`. Bind it to `ImageCrop` by mistake and the second +carve reads out of a 2×1 image and the case goes red. + +**And the rule Tier 0 item 4 asked for, written down:** *a raylib function on a game's +per-frame path is hand-written in `raylib.flan` and checked against the header; it is +not left to the opt-in import.* The import widens the surface and is worth having, but a +build that does not have `FLAN_RAYLIB_H` set is the default build, and the default build +has to be able to draw. The test that goes with the rule is a link check, which is cheap +and is all a GL-context call can have. + --- ## 2. What looks like a gap and is not @@ -273,6 +312,17 @@ another name. Two things are missing behind it: `lib/dev.ml` answers every subsequent request with "the program exited; restart flan dev". + > **Fixed, 2026-09-13.** It signals `BoundsError` with `error` now, and dies with that + > same message only if nothing answered. The site establishes **no restart** — nothing + > a handler can do makes a bad index good, so there is no attempt to re-run — and what + > answers it is the restart the program already had, which is exactly the frame loop's + > `continue` this section is about. `BUILT.md` has the reasoning, + > `test/programs/bounds-condition.flan` has the handled case, and + > `test/programs/dev-break-bounds.flan` drives the break loop over an + > *un*handled one from the editor's side — stopped, restarts listed, + > `continue` taken, session intact. Item 1 below, the rollback, is now the + > whole of what is left here. + That is not theoretical for this game. `game.clj`'s `update-game` computes `row` and `col` straight from the mouse position and indexes the grid with them, with no bounds check anywhere. **`game.lisp` added `in-bounds-p` and calls it in `update-drag`** — the @@ -405,15 +455,19 @@ path; the remainder is unchecked and should not be read as verified. through `KEY_NINE`=57, `KEY_SPACE`=32, `KEY_E`=69, `KEY_LEFT_SHIFT`=340, `MOUSE_BUTTON_LEFT`/`RIGHT`/`MIDDLE`=0/1/2, and `LOG_WARNING` as the fifth `TraceLogLevel` member (4) are all unchanged in 6.0 and all match `raylib.flan` — except -that `left-shift` is absent from the `Key` `defenum` entirely, which is the §1 note, not a -skew. +that `left-shift` was absent from the `Key` `defenum` entirely, which is the §1 note and +not a skew. **It is `left-shift 340` there now**, and nothing else went in beside it: the +`defenum`'s own comment says it carries the keys that have a customer, and of the +modifier siblings only this one does. This paragraph is the record that every other enum +value the game touches was already present and already right, so "check the surrounding +enum for other omissions" has an answer and the answer is none. So the version skew is not a correctness problem for this game today. It is still worth closing, because the whole point of `headers` is that a silent disagreement is the failure mode, and "I checked by hand once" is exactly the state `headers` was built to replace. -The binding's real gap against `rl.clj` is §1: `DrawTexturePro`, then `ImageFromImage` and -`IsWindowReady`. +The binding's real gap against `rl.clj` was §1: `DrawTexturePro`, then `ImageFromImage` +and `IsWindowReady`. All three are bound. --- @@ -422,32 +476,42 @@ The binding's real gap against `rl.clj` is §1: `DrawTexturePro`, then `ImageFro Split into two tiers, because a one-line binding fix and a month of language work should not compete for the same slot. -### Tier 0 — bindings. Hours, not weeks. Do these first. +### Tier 0 — bindings. Hours, not weeks. **Done, 2026-09-13.** -1. **`declare-c draw-texture-pro`.** Unblocks the entire renderer. Without it this game - has no draw call. One line, plus an acceptance case that makes raylib compute with the - source rect so a permuted `Rectangle` goes red. +1. ~~**`declare-c draw-texture-pro`.**~~ Bound. The renderer is writable. The acceptance + case this asked for could not be written as described — see §1's closing box — so it + is a link check behind a `window-ready?` guard, and the computed test moved to + `image-from-image` where raylib does the arithmetic on the CPU. -2. **`left-shift` in the `Key` `defenum`.** Shift+1..5 picks the tilemap in both - implementations, and the member is not there. One entry, and without it that input is - unwritable. +2. ~~**`left-shift` in the `Key` `defenum`.**~~ One entry, `340`. §5 records that nothing + else in the enum was missing. -3. **`declare-c image-from-image` and `window-ready?`.** The atlas tool and the engine's - reentrancy guard. Two lines. +3. ~~**`declare-c image-from-image` and `window-ready?`.**~~ Both bound. + `image-from-image` carries the strongest new assertion in the table: two carves at + different `y`s out of one sheet, and the sheet re-read afterwards to show it was not + destroyed. -4. **Decide the rule the first item exposes.** A function the game calls every frame - should be hand-written and header-checked, not left to the opt-in import. Worth writing - down, because the next game-shaped program will find the next `DrawTexturePro`. +4. ~~**Decide the rule the first item exposes.**~~ Decided and written into §1: *a raylib + function on a game's per-frame path is hand-written in `raylib.flan` and checked + against the header, not left to the opt-in import.* The default build has no + `FLAN_RAYLIB_H` and the default build has to be able to draw. ### Tier 1 — language and tooling, in the order that unblocks the most of this game -4. **A bounds failure should stop the program, not end it.** Route `flan_bounds_fail` - through the condition machinery so it reaches the break loop with the restarts that are - on the stack, instead of `exit(134)`. This is the single largest gap between what Flan - promises and what this game would experience, and the game reaches it through the most - ordinary path in it — a mouse coordinate outside the window, which `game.lisp` had to - add `in-bounds-p` to survive. Everything else here is a workaround with a known cost; - this one ends the session. +4. ~~**A bounds failure should stop the program, not end it.**~~ **Done, 2026-09-13.** It + signals `BoundsError`, reaches handlers and the break loop, and is fatal only when + nothing answers. `Vec`'s checks went with it, since `(at v i)` and `(at arr i)` are one + form in the source. No restart is established at the site: `retry` exists for + allocation and for files because those attempts are repeatable, and this one is not — + so what answers a bad index is the `continue` the frame loop already offered, which is + the restart this document was pointing at all along. + + Two consequences worth carrying forward. **Defer** had to be decided rather than + inherited — an answered bounds failure runs the function's defers, because it leaves + through the same unwind path a `return` does; an unanswered one still runs none. + And **item 6 below got more valuable**, not less: now that a bad index lands in + `continue` instead of ending the process, the missing half is the rollback, because an + abandoned frame leaves the grid half-written. That is the next thing on this list. 5. **A watch for a running program.** `watch.clj` + `watch.el` + `spy` + `spy-num` is a tool the author built deliberately and uses constantly, and the stopped-stack inspector diff --git a/lib/emit.ml b/lib/emit.ml index d27055d..4d5c285 100644 --- a/lib/emit.ml +++ b/lib/emit.ml @@ -731,7 +731,12 @@ let fninfo m (fn : Tast.fn) ~nslots = Indices are i32 in Flan and sign-extended to i64 for the gep, so a negative one arrives here as a huge unsigned value: an unsigned comparison catches - the negative and the too-large case in a single test. *) + the negative and the too-large case in a single test. + + This is the *trapping* shape and it still has three users: the restart + lookups and the unarmed-clause check, none of which is recoverable — there + is nowhere to resume a transfer whose target does not exist. The two bounds + checks moved off it; see [signal_block]. *) let fail_block f (loc : Loc.t) ok emit_call = let good = fresh_label f "inb" and bad = fresh_label f "oob" in term f "br i1 %s, label %%%s, label %%%s" ok good bad; @@ -741,21 +746,44 @@ let fail_block f (loc : Loc.t) ok emit_call = term f "unreachable"; label f good +(* The same branch, for a failure that *signals* rather than dying. The call is + an ordinary one — it returns when a handler or the break loop transferred — + so it is followed by a guard, and the fall-through past the guard is what is + unreachable: nothing answered, so the runtime already died inside the call. + + [guard] is passed in rather than called directly because [guard] is part of + the expression emitter's recursive group and this is defined above it. It is + the same [guard f] every call site emits, so a bounds failure that is + answered leaves the function through the innermost pad — a restart-case's, + or the function's own unwind block, which runs its defers and returns. + **That is the answer to "does a trap run defers": an answered one does, an + unanswered one still does not, because the unanswered one is still a die + inside C.** *) +let signal_block f (loc : Loc.t) ~guard ok emit_call = + let good = fresh_label f "inb" and bad = fresh_label f "oob" in + term f "br i1 %s, label %%%s, label %%%s" ok good bad; + label f bad; + let id, n = string_bytes f.md (Loc.to_string loc) in + emit_call id n; + guard (); + term f "unreachable"; + label f good + (* [at] is strict: the last valid index is len - 1. *) -let check_at f loc idx len = +let check_at f ~guard loc idx len = if f.md.checks then begin let ok = fresh f in ins f "%s = icmp ult i64 %s, %s" ok idx len; - fail_block f loc ok (fun id n -> - ins f "call void @flan_bounds_fail(ptr %s, i64 %d, i64 %s, i64 %s)" - id n idx len) + signal_block f loc ~guard ok (fun id n -> + ins f "call void @flan_bounds_error(ptr %s, i64 %d, i64 %s, i64 %s, ptr %s)" + id n idx len xfer_param) end (* [slice] is not: a slice ending at len — or an empty one at lo = len — is legal, and its one-past-the-end gep is defined. [lo <= hi] is not redundant with it, because a reversed range would otherwise yield hi - lo as a huge unsigned length, which is a worse hole than the missing check. *) -let check_slice f loc lo hi len = +let check_slice f ~guard loc lo hi len = if f.md.checks then begin let a = fresh f in ins f "%s = icmp ule i64 %s, %s" a lo hi; @@ -763,9 +791,11 @@ let check_slice f loc lo hi len = ins f "%s = icmp ule i64 %s, %s" b hi len; let ok = fresh f in ins f "%s = and i1 %s, %s" ok a b; - fail_block f loc ok (fun id n -> - ins f "call void @flan_slice_fail(ptr %s, i64 %d, i64 %s, i64 %s, i64 %s)" - id n lo hi len) + signal_block f loc ~guard ok (fun id n -> + ins f + "call void @flan_slice_error(ptr %s, i64 %d, i64 %s, i64 %s, i64 %s, \ + ptr %s)" + id n lo hi len xfer_param) end (* ── Expressions ───────────────────────────────────────────────────── *) @@ -1050,7 +1080,7 @@ and element_addr f (target : Tast.expr) idx = (match ty with | Types.Array (n, elem) -> (* The bound is static; LLVM folds the check away for a literal index. *) - check_at f i.Tast.loc i64 (Int64.to_string n); + check_at f ~guard:(fun () -> guard f) i.Tast.loc i64 (Int64.to_string n); let p = fresh f in ins f "%s = getelementptr inbounds %s, ptr %s, i64 0, i64 %s" p (ll ty) ptr i64; @@ -1062,7 +1092,7 @@ and element_addr f (target : Tast.expr) idx = ins f "%s = extractvalue %%slice %s, 0" base s; let len = fresh f in ins f "%s = extractvalue %%slice %s, 1" len s; - check_at f i.Tast.loc i64 len; + check_at f ~guard:(fun () -> guard f) i.Tast.loc i64 len; let p = fresh f in ins f "%s = getelementptr inbounds %s, ptr %s, i64 %s" p (ll elem) base i64; go p elem rest @@ -1774,7 +1804,7 @@ and prim f (e : Tast.expr) (p : Tast.prim) (args : Tast.expr list) = match target.Tast.ty with | Types.Array (n, _) -> let a = addr f target in - check_slice f e.Tast.loc lo64 hi64 (Int64.to_string n); + check_slice f ~guard:(fun () -> guard f) e.Tast.loc lo64 hi64 (Int64.to_string n); let p = fresh f in ins f "%s = getelementptr inbounds %s, ptr %s, i64 0, i64 %s" p (ll target.Tast.ty) a lo64; @@ -1785,7 +1815,7 @@ and prim f (e : Tast.expr) (p : Tast.prim) (args : Tast.expr list) = ins f "%s = extractvalue %%slice %s, 0" q v; let n = fresh f in ins f "%s = extractvalue %%slice %s, 1" n v; - check_slice f e.Tast.loc lo64 hi64 n; + check_slice f ~guard:(fun () -> guard f) e.Tast.loc lo64 hi64 n; let p = fresh f in ins f "%s = getelementptr inbounds %s, ptr %s, i64 %s" p (ll elem) q lo64; p @@ -1795,7 +1825,7 @@ and prim f (e : Tast.expr) (p : Tast.prim) (args : Tast.expr list) = ins f "%s = extractvalue %%slice %s, 0" q v; let n = fresh f in ins f "%s = extractvalue %%slice %s, 1" n v; - check_slice f e.Tast.loc lo64 hi64 n; + check_slice f ~guard:(fun () -> guard f) e.Tast.loc lo64 hi64 n; let p = fresh f in ins f "%s = getelementptr inbounds i8, ptr %s, i64 %s" p q lo64; p @@ -1855,13 +1885,25 @@ and prim f (e : Tast.expr) (p : Tast.prim) (args : Tast.expr list) = | t -> [ ll t ^ " " ^ value f a ]) args) in + (* The two runtime entry points whose bounds check signals. They are the + only [Rt] symbols that can transfer, so they are the only ones that take + the channel and the only ones guarded — everything else in this family + is arithmetic over a container header and cannot reach a handler. A Vec + is checked inside the runtime rather than in emitted IR (BUILT.md), so + this is where (at v i) gets what (at arr i) gets from [check_at]. *) + let signals = + String.equal sym "flan_vec_at" || String.equal sym "flan_vec_as_slice" + in + let vs = if signals then vs @ [ "ptr " ^ xfer_param ] else vs in let args' = String.concat ", " vs in if is_void e.Tast.ty then begin ins f "call void @%s(%s)" sym args'; + if signals then guard f; "zeroinitializer" end else begin let t = fresh f in ins f "%s = call %s @%s(%s)" t (ll e.Tast.ty) sym args'; + if signals then guard f; t end | Tast.SizeOf t, [] -> Printf.sprintf "%d" (fst (lay f.md t)) @@ -2330,8 +2372,10 @@ declare void @flan_restart_fail(ptr, i64, ptr, i64) noreturn cold declare void @flan_restart_args_fail(ptr, i64, ptr, i64, ptr, i64, ptr, i64) noreturn cold declare void @flan_restart_unarmed(ptr, i64, ptr, i64, ptr, i64) noreturn cold declare void @flan_transfer_fail(ptr, i64) noreturn cold -declare void @flan_bounds_fail(ptr, i64, i64, i64) noreturn cold -declare void @flan_slice_fail(ptr, i64, i64, i64, i64) noreturn cold +; Not noreturn: each signals BoundsError and returns when something answered +; it, which is the one path out. The trailing ptr is the transfer channel. +declare void @flan_bounds_error(ptr, i64, i64, i64, ptr) cold +declare void @flan_slice_error(ptr, i64, i64, i64, i64, ptr) cold declare ptr @flan_context_allocator() declare ptr @flan_context_temp() declare ptr @flan_heap_allocator() @@ -2355,8 +2399,11 @@ declare i8 @flan_vec_reserve(ptr, i64, i64, i64, ptr, i64) declare i8 @flan_vec_push(ptr, ptr, i64, i64, ptr, i64) declare i8 @flan_vec_clone(ptr, ptr, ptr, i64, i64, ptr, i64) declare i64 @flan_vec_len(ptr, ptr, i64) -declare ptr @flan_vec_at(ptr, i32, i64, ptr, i64) -declare void @flan_vec_as_slice(ptr, ptr, i32, i32, i64, ptr, i64) +; These two take the transfer channel as well, because a Vec's bounds check is +; inside the runtime rather than emitted here and (at v i) has to signal the +; same condition (at arr i) does. +declare ptr @flan_vec_at(ptr, i32, i64, ptr, i64, ptr) +declare void @flan_vec_as_slice(ptr, ptr, i32, i32, i64, ptr, i64, ptr) declare void @flan_vec_free(ptr, i64, i64, ptr, i64) ; (Pool T) and (Handle T). A handle crosses as the i64 it is; the pool, like ; every other owning container, crosses as its address. [resolve] answers a diff --git a/lib/prelude.ml b/lib/prelude.ml index 11eb872..fa56723 100644 --- a/lib/prelude.ml +++ b/lib/prelude.ml @@ -44,6 +44,40 @@ let source = {flan| ;; handler or the break loop, where a working allocator is known. (defstruct StorageExhausted [bytes i64 align i64 allocator i64]) +;; What an out-of-range index signals. Same shape as StorageExhausted and for +;; the same reasons: fixed numeric fields, no rendered message, nothing that +;; allocates — the condition is built on the failing frame's stack and the +;; formatting is the handler's or the break loop's job, where a working +;; allocator is known. +;; +;; It is signalled with `error`, from the runtime rather than from Flan: +;; flan_bounds_error and flan_slice_error in runtime/flan_rt.c, which every +;; bounds check now branches to. **The three fields there are a C struct that +;; has to agree with this one field for field**, the same hand-kept agreement +;; flan_name_id keeps with Check.type_id. +;; +;; `low` and `high` are the same index for an (at xs i), and the two ends of +;; the range for a (slice xs lo hi). One condition type rather than two, +;; because a handler that wants to survive a bad index should not have to +;; write two clauses to cover the two ways of writing one. +;; +;; **Nothing establishes a restart at the failing site**, which is the +;; difference from StorageExhausted and from FileError. Those offer `retry` +;; because their attempt is repeatable: free something, or supply another path, +;; and the same operation succeeds the second time. Nothing a handler can do +;; makes index 51 valid for a length-50 array, so there is no attempt to +;; re-run. `use-value` for the index is the near miss and is not built: it +;; would put an alloca and a restart frame on every indexing operation, and +;; what it buys is a *different element*, silently. +;; +;; The restarts that matter are the ones the program already established — a +;; frame loop's `continue`, sand.flan's shape — and they are on the restart +;; stack and reachable from a handler or from the break loop without anything +;; being pushed here. That is plan.org's "restarts go at the resync point, +;; once", with allocation and file failure as the named exceptions and this on +;; the default side of the rule. +(defstruct BoundsError [low i64 high i64 length i64]) + ;; A breakpoint. (pause) stops the program where it stands and hands it to the ;; break loop, with the whole stack under it readable — C-c C-b lists the ;; frames, TAB opens one, and taking `continue` resumes at the call. diff --git a/runtime/flan_rt.c b/runtime/flan_rt.c index 066b329..9f32348 100644 --- a/runtime/flan_rt.c +++ b/runtime/flan_rt.c @@ -466,6 +466,80 @@ _Noreturn void flan_slice_fail(const uint8_t *loc, int64_t loclen, rt_die(); } +/* ── An index out of range is a condition ────────────────────────────── + * + * The two functions above are still here and still die; what changed is that + * they are no longer the *first* thing a bad index reaches. A bounds failure + * now signals BoundsError with `error`, exactly as a failed allocation signals + * StorageExhausted, and only reaches the message above if nothing answered. + * + * Why it had to change. `flan dev` runs the compiler inside the program, in + * one process. exit(134) therefore took the session with it, and the session + * is the thing the project is built around never having to restart. The + * ordinary route into it is not exotic: a grid indexed from a mouse position + * is out of bounds the first time the pointer leaves the window. + * + * **No restart is established here**, and that is a decision rather than an + * omission. flan_alloc_* and the file guards offer `retry` because their + * attempt is repeatable — a handler frees something, or supplies another + * path, and the same operation then succeeds. Nothing a handler can do makes + * index 51 valid for a length-50 array, so there is no attempt to re-run and + * nothing for a site restart to resume into. `use-value` for the index is the + * near miss: it would cost every indexing operation an alloca and a restart + * frame, and what it buys is a *different element*, silently, which is the + * class of answer this codebase refuses everywhere else. The restarts that + * matter are the ones the program already established — a frame loop's + * `continue` — and those are on the restart stack and reachable from the break + * loop without anything being pushed here. + * + * With no break hook — a release build, or any program that did not import the + * agent — flan_break_hook is NULL, nothing transfers, and this falls through + * to the same message and the same status it always had. That is still the + * right answer: there is nowhere to stand. + * + * The condition is three int64s on this frame and it must agree field for + * field with the prelude's (defstruct BoundsError [low i64 high i64 length + * i64]) — the same hand-kept agreement flan_name_id has with Check.type_id, + * and for the same reason: a struct is a layout and a type is a number, and + * neither side can see the other. `low` and `high` are the same index for an + * `at`, and the two ends of the range for a `slice`, so one condition type + * covers both and a handler writes one clause rather than two. */ + +typedef struct { int64_t low, high, length; } flan_bounds_cond; + +static const uint8_t flan_bounds_name[] = "BoundsError"; +#define FLAN_BOUNDS_NAMELEN 11 + +/* Returns nonzero if something transferred, in which case the caller returns + * and its caller's guard carries the transfer out. */ +static int flan_bounds_signal(void *xfer, int64_t low, int64_t high, + int64_t len) { + flan_bounds_cond c; + uint32_t id = flan_name_id(flan_bounds_name, FLAN_BOUNDS_NAMELEN); + c.low = low; + c.high = high; + c.length = len; + flan_signal(id, &c, xfer); + if (*(void **)xfer != NULL) return 1; + if (flan_break_hook != NULL) { + flan_break_hook(flan_bounds_name, FLAN_BOUNDS_NAMELEN, &c, xfer); + if (*(void **)xfer != NULL) return 1; + } + return 0; +} + +void flan_bounds_error(const uint8_t *loc, int64_t loclen, int64_t idx, + int64_t len, void *xfer) { + if (flan_bounds_signal(xfer, idx, idx, len)) return; + flan_bounds_fail(loc, loclen, idx, len); +} + +void flan_slice_error(const uint8_t *loc, int64_t loclen, int64_t lo, + int64_t hi, int64_t len, void *xfer) { + if (flan_bounds_signal(xfer, lo, hi, len)) return; + flan_slice_fail(loc, loclen, lo, hi, len); +} + /* ── Allocators, spec-memory.md ──────────────────────────────────────── * * One type-erased procedure plus an opaque data pointer, which is Odin's @@ -955,23 +1029,40 @@ int64_t flan_vec_len(flan_vec *v, const uint8_t *loc, int64_t loclen) { return v->len; } +/* [xfer] is this operation's end of the transfer channel, so that an index out + * of range signals BoundsError instead of ending the process — see the note + * above flan_bounds_error. (at v i) and (at arr i) are the same form in the + * source and shipping one of them signalling and the other exiting would read + * as a bug, so the two are plumbed together. The channel is the last + * parameter, as it is on every Flan signature; Emit appends it and guards the + * call, and a transfer therefore leaves through the caller's pad with NULL + * here never dereferenced. */ void *flan_vec_at(flan_vec *v, int32_t i, int64_t size, const uint8_t *loc, - int64_t loclen) { + int64_t loclen, void *xfer) { flan_vec_check(v, loc, loclen); /* The same unsigned comparison the fixed-array bounds check uses: a negative * index sign-extends to a huge unsigned and is caught by the one test. */ - if ((uint64_t)(int64_t)i >= (uint64_t)v->len) + if ((uint64_t)(int64_t)i >= (uint64_t)v->len) { + if (flan_bounds_signal(xfer, (int64_t)i, (int64_t)i, v->len)) return NULL; flan_vec_bounds_fail(loc, loclen, (int64_t)i, v->len); + } return (uint8_t *)v->ptr + (int64_t)i * size; } /* [hi] of -1 means "to the end": (as-slice v) has no static length to write. */ void flan_vec_as_slice(flan_vec *v, void *out, int32_t lo, int32_t hi, - int64_t size, const uint8_t *loc, int64_t loclen) { + int64_t size, const uint8_t *loc, int64_t loclen, + void *xfer) { struct { void *p; int64_t n; } s; int64_t l = lo, h = (hi < 0) ? v->len : hi; flan_vec_check(v, loc, loclen); - if (l < 0 || h > v->len || l > h) flan_vec_bounds_fail(loc, loclen, l, v->len); + if (l < 0 || h > v->len || l > h) { + /* Both ends, because both are what went wrong — the fixed-array slice + * check reports the same pair. [out] is left untouched on the transfer + * path; the caller's guard branches before it reads the slice. */ + if (flan_bounds_signal(xfer, l, h, v->len)) return; + flan_vec_bounds_fail(loc, loclen, l, v->len); + } s.p = (uint8_t *)v->ptr + l * size; s.n = h - l; memcpy(out, &s, sizeof s); diff --git a/test/programs/bounds-condition.flan b/test/programs/bounds-condition.flan new file mode 100644 index 0000000..e289c8c --- /dev/null +++ b/test/programs/bounds-condition.flan @@ -0,0 +1,190 @@ +;;;; An index out of range is a condition, not the end of the process. +;;;; +;;;; Until now a bad index printed its source location and called exit(134). +;;;; That was defensible when `flan dev` was two processes; it is not now that +;;;; the compiler runs inside the program, because the trap takes the session +;;;; with it and the session is the thing the project is built around never +;;;; having to restart. And the route in is the most ordinary one there is: a +;;;; grid indexed from a mouse position is out of bounds the first time the +;;;; pointer leaves the window (PORTING.md, §3). +;;;; +;;;; So a failed bounds check signals BoundsError with `error`, the same way a +;;;; failed allocation signals StorageExhausted, and dies with the old message +;;;; only if nothing answered. This program is the "something answered" half — +;;;; the unhandled half is bounds.flan, which still exits 134 with the same +;;;; text it always did. +;;;; +;;;; **No restart is established at the failing index**, and that is the +;;;; decision worth reading this file for. StorageExhausted offers `retry` +;;;; because its attempt is repeatable: free something and the allocation +;;;; succeeds. Nothing a handler can do makes index 7 valid for a length-4 +;;;; array. `use-value` for the index would cost every indexing operation a +;;;; restart frame and buy a *different element*, silently. What answers a bad +;;;; index is the restart the program already had — the frame loop's +;;;; `continue`, which is sand.flan's shape and is what a game wants: abandon +;;;; this frame, keep the window open. +;;;; +;;;; Four things are asserted, and the first two are the ones that matter: +;;;; +;;;; 1. The frame is abandoned and the program carries on. `frames` counts +;;;; the ones that finished and `skipped` the ones that did not. +;;;; 2. **Defers run.** A trap ran none, which BUILT.md recorded as following +;;;; from the noreturn-then-unreachable shape rather than as a decision. +;;;; The shape changed, so the question had to be answered rather than +;;;; inherited: an *answered* bounds failure leaves through the same +;;;; unwind path a `return` uses, and therefore runs the function's +;;;; defers, innermost first. An unanswered one still runs none, because +;;;; it is still a die inside C. +;;;; 3. The condition carries the numbers. `low` and `high` are the same +;;;; index for an `at` and the two ends of the range for a `slice`, which +;;;; is why there is one condition type and not two. +;;;; 4. Every route to a bad index signals: reading a fixed array, writing +;;;; one (a different lowering — place/Pindex, not At), a slice, a Vec +;;;; element, and a Vec's as-slice. A Vec's check lives inside the +;;;; runtime rather than in emitted IR, so those two are plumbed +;;;; separately and are the ones most likely to be left behind. + +(defvar grid [4 i32]) + +;;; Handlers cannot see the locals of the function that established them — +;;; check.ml refuses a capture by name and says to use a global — so +;;; everything this program counts lives up here. +(defvar frames i64) +(defvar skipped i64) +(defvar cleaned i64) +(defvar low i64) +(defvar high i64) +(defvar length i64) + +;;; Two frames deep, with a defer on the way, so the transfer has something to +;;; cross and something to run on its way out. +(defn show [name string n i64] () + (print name) (print " ") (print n) (println "")) + +(defn read-cell [i i32] i32 + (defer (set cleaned (+ cleaned 1))) + (at grid i)) + +(defn write-cell [i i32] () + (defer (set cleaned (+ cleaned 1))) + (set (at grid i) 99)) + +;;; The frame loop's shape: one restart-case around the work, offering +;;; `continue`, which abandons this frame and nothing else. sand.flan's main +;;; loop is this. +(defn read-frame [i i32] () + (restart-case + (do (show "read" (i64 (read-cell i))) + (set frames (+ frames 1))) + (continue [] (set skipped (+ skipped 1))))) + +(defn write-frame [i i32] () + (restart-case + (do (write-cell i) + (show "wrote" (i64 (at grid i))) + (set frames (+ frames 1))) + (continue [] (set skipped (+ skipped 1))))) + +(defn slice-frame [s [u8] lo i32 hi i32] () + (restart-case + (do (show "slice" (i64 (len (slice s lo hi)))) + (set frames (+ frames 1))) + (continue [] (set skipped (+ skipped 1))))) + +;;; The two Vec frames are written inline in main rather than as functions, +;;; because a (Vec T) is move-only: passing one to a helper would hand +;;; ownership over and the caller's binding would be dead afterwards. A +;;; restart-case does not have to be in a different function from the +;;; handler-bind that answers into it — the transfer is by frame address and +;;; the frames here are simply both this one. + +(defn main [] i32 + (set (at grid 0) 10) + (set (at grid 1) 11) + (set (at grid 2) 12) + (set (at grid 3) 13) + + (let [s (bytes "hello") ; len 5 + v (vec-new i32)] + (push v 100) + (push v 200) + + (handler-bind + [(BoundsError [c] + ;; The numbers are here rather than in a message, for the same reason + ;; StorageExhausted has none: formatting allocates, and this is a path + ;; that must be able to run when allocation is what failed. + (set low (.low c)) + (set high (.high c)) + (set length (.length c)) + ;; Abandon the frame. The transfer crosses read-cell (running its + ;; defer) and lands in the clause of the restart-case two frames out. + (invoke-restart 'continue))] + + ;; In bounds: the frame finishes, the handler never runs, and the defer + ;; runs on the ordinary return path. + (read-frame 2) + ;; Past the end, then negative. A negative index sign-extends to a huge + ;; unsigned and is caught by the same single comparison, but the number + ;; the condition carries is the signed one. + (read-frame 7) + (show "low" low) + (show "length" length) + (read-frame -1) + (show "low" low) + + ;; The write path lowers through place/Pindex rather than through At, so + ;; it would be perfectly possible to convert one and not the other. + (write-frame 1) + (write-frame 4) + (show "low" low) + + ;; A slice reports both ends, which is the whole reason low and high are + ;; two fields: [2 9) against a length of 5. + (slice-frame s 1 4) + (slice-frame s 2 9) + (show "low" low) + (show "high" high) + (show "length" length) + ;; A reversed range, which the lo <= hi test is what catches: without it + ;; this builds a slice of length hi - lo as a huge unsigned. + (slice-frame s 3 1) + (show "low" low) + (show "high" high) + + ;; And the Vec pair, whose checks are inside the runtime rather than in + ;; emitted IR — a different code path entirely, and the one most likely + ;; to be left behind by a change made in emit.ml. + (restart-case + (do (show "vec" (i64 (at v 1))) + (set frames (+ frames 1))) + (continue [] (set skipped (+ skipped 1)))) + (restart-case + (do (show "vec" (i64 (at v 5))) + (set frames (+ frames 1))) + (continue [] (set skipped (+ skipped 1)))) + (show "low" low) + (show "length" length) + (restart-case + (do (show "vec-slice" (i64 (len (as-slice v 0 2)))) + (set frames (+ frames 1))) + (continue [] (set skipped (+ skipped 1)))) + (restart-case + (do (show "vec-slice" (i64 (len (as-slice v 0 9)))) + (set frames (+ frames 1))) + (continue [] (set skipped (+ skipped 1)))) + (show "high" high)) + + (free v)) + + ;; Six frames finished, six were abandoned, and every one of the twelve ran + ;; its defer — which is the claim about defer that the old shape could not + ;; make. + (show "frames" frames) + (show "skipped" skipped) + (show "cleaned" cleaned) + ;; The write that did land, and the one that did not: grid[1] is 99 and + ;; nothing else moved. + (print (at grid 0)) (print " ") (print (at grid 1)) (print " ") + (print (at grid 2)) (print " ") (print (at grid 3)) (println "") + 0) diff --git a/test/programs/dev-break-bounds.flan b/test/programs/dev-break-bounds.flan new file mode 100644 index 0000000..6d57da5 --- /dev/null +++ b/test/programs/dev-break-bounds.flan @@ -0,0 +1,52 @@ +;;;; A program that stops on a bad index, for driving the break loop over one. +;;;; +;;;; bounds-condition.flan is the other half of the same change and covers the +;;;; path where a `handler-bind` answers. This one covers the path the change +;;;; is actually *for*: **nothing handles it**, so the signal walks the +;;;; handlers, finds none, and reaches `flan_break_hook` — which is where an +;;;; editor picks up a stopped program with the stack, the locals and the +;;;; globals readable and the restarts on offer. +;;;; +;;;; That is the claim `PORTING.md` said was missing, and it is worth testing +;;;; end to end rather than reasoning about: before this, a bad index called +;;;; exit(134), and with `flan dev` running as one process that took the +;;;; compiler and the session with it. +;;;; +;;;; The shape is a frame loop's: one `restart-case` offering `continue` around +;;;; the work, which is sand.flan's shape and the game's. No restart is +;;;; established at the failing index — nothing a handler could do would make +;;;; index 9 valid for a length-4 array — so `continue` is the *program's* own +;;;; restart, found by the ordinary walk, and taking it is the proof that a +;;;; bounds failure now lands somewhere a session can be recovered from. +(import agent "vendor:agent") + +(defvar grid [4 i32]) +(defvar skipped i64) + +;;; One frame deep under the restart-case, so the transfer has something to +;;; cross and the backtrace has something to show. +(defn touch [i i32] () + (set (at grid i) 1)) + +(defn frame [i i32] () + (restart-case + (do (touch i) (println "frame done")) + (continue [] (set skipped (+ skipped 1))))) + +(defvar ticks i64) + +(defn main [] i32 + (agent/start "/tmp/flan-dev-break-bounds-fallback.sock") + ;; Out of bounds on the first frame, with nothing handling it: the daemon + ;; meets a program that has already stopped, which is the state an editor + ;; has to cope with and the hardest one to arrange later. + (frame 9) + ;; 1 once `continue` was taken. Printing it is how the transcript says which + ;; way the program left the break, rather than that it left. + (print skipped) (println "") + ;; And it keeps polling on the far side, because the claim worth testing is + ;; that everything still works after a break over a bad index. + (dotimes [i 4000] + (agent/wait 5) + (set ticks (+ ticks 1))) + 0) diff --git a/test/programs/raylib-ffi.flan b/test/programs/raylib-ffi.flan index a5a8400..a8de1c9 100644 --- a/test/programs/raylib-ffi.flan +++ b/test/programs/raylib-ffi.flan @@ -288,4 +288,33 @@ (print (rl/get-gamepad-axis-movement 0 (rl/GamepadAxis 9))) (println "") + ;; ── window-ready?, and the one draw call that has to link ─────────── + ;; + ;; No window was opened, so this is false — which is the answer an engine's + ;; "am I already running" guard needs, and the only thing about it that can + ;; be asserted without a display. + (show-bool "window ready" (rl/window-ready?)) + + ;; draw-texture-pro needs a GL context, so it cannot be *called* here and + ;; cannot be in the assertion table at all — the whole Shapes and Textures + ;; section is in that position. What it can be is *linked*, which is what + ;; this branch is for: the shim is generated, the symbol is resolved at link + ;; time, and a signature that does not exist in libraylib fails the build + ;; rather than the frame a game first draws in. The guard is + ;; window-ready?, which has just printed no, so the body never runs. + ;; + ;; This is the honest limit of what a headless table says about a draw call, + ;; and it is worth saying out loud: nothing here checks the argument order. + ;; The source rect, the dest rect and the origin are three structs in a row + ;; and permuting them links perfectly. Only looking at the screen catches + ;; that, which is why the file's header says so about the Shapes family. + (when (rl/window-ready?) + (rl/draw-texture-pro (rl/Texture2D {.id 0 .width 0 .height 0 + .mipmaps 0 .format 0}) + (rl/Rectangle {.x 0.0 .y 0.0 .width 16.0 .height 16.0}) + (rl/Rectangle {.x 0.0 .y 0.0 .width 64.0 .height 64.0}) + (rl/Vector2 {.x 0.0 .y 0.0}) + 0.0 + rl/white)) + 0) diff --git a/test/programs/raylib-image.flan b/test/programs/raylib-image.flan index 8ac680a..82811a5 100644 --- a/test/programs/raylib-image.flan +++ b/test/programs/raylib-image.flan @@ -157,4 +157,44 @@ (show-pixel "cropped at 0,0" img 0 0) (rl/unload-image img)) + ;; ── image-from-image, which is crop without the destruction ───────── + ;; + ;; The same 6 x 3 image and the same two marks, carved twice. The point that + ;; matters is the one the assertions below make in three parts: + ;; + ;; 1. It reads the rectangle the same way image-crop does — (4,0,2,1) + ;; picks two pixels of the first row and the mark at (5,0) lands at + ;; (1,0) of a 2 x 1 result. Exchange width and height and the result is + ;; 1 x 2 with nothing in it. + ;; + ;; 2. It reads `y`. The second carve is (4,2,2,1), one row lower than + ;; anything image-crop's case reaches, and the mark it finds is the + ;; *other* colour. A binding that ignored y would answer mark-a twice. + ;; + ;; 3. **The source survives.** That is the whole reason this exists beside + ;; image-crop: crop mutates in place, so carving a sheet into twenty + ;; tiles with it destroys the sheet on the first one. The original is + ;; re-read after both carves and still reports 6 x 3 with both marks + ;; where they were put. Bind this to ImageCrop by mistake and the second + ;; carve reads out of a 2 x 1 image and the source check goes red. + (let [sheet (rl/gen-image-color 6 3 bg)] + (rl/image-draw-pixel (addr sheet) 5 0 mark-a) + (rl/image-draw-pixel (addr sheet) 4 2 mark-b) + (let [top (rl/image-from-image + sheet (rl/Rectangle {.x 4.0 .y 0.0 .width 2.0 .height 1.0}))] + (show-image "piece-top" top) + (show-pixel "piece-top at 1,0" top 1 0) + (show-pixel "piece-top at 0,0" top 0 0) + (rl/unload-image top)) + (let [bottom (rl/image-from-image + sheet (rl/Rectangle {.x 4.0 .y 2.0 .width 2.0 .height 1.0}))] + (show-image "piece-bottom" bottom) + (show-pixel "piece-bottom at 0,0" bottom 0 0) + (show-pixel "piece-bottom at 1,0" bottom 1 0) + (rl/unload-image bottom)) + (show-image "sheet after" sheet) + (show-pixel "sheet at 5,0" sheet 5 0) + (show-pixel "sheet at 4,2" sheet 4 2) + (rl/unload-image sheet)) + 0) diff --git a/test/test_acceptance.ml b/test/test_acceptance.ml index 1a4406f..b310a06 100644 --- a/test/test_acceptance.ml +++ b/test/test_acceptance.ml @@ -809,7 +809,8 @@ let () = point in poly yes\npoint outside poly no\n\ in square, four corners yes\nout of triangle, three no\n\ no crossing\n\ - axes 0 0 0 0 -1 -1 past-end 0\n" + axes 0 0 0 0 -1 -1 past-end 0\n\ + window ready no\n" in if Sys.command "ldconfig -p 2>/dev/null | grep -q libraylib" = 0 then begin outputs "raylib ffi, headless" "programs/raylib-ffi.flan" raylib_out; @@ -911,7 +912,16 @@ let () = resized 2 6 1 7\n\ cropped 2 1 1 7\n\ cropped at 1,0 200 0 0 255\n\ - cropped at 0,0 10 20 30 255\n" + cropped at 0,0 10 20 30 255\n\ + piece-top 2 1 1 7\n\ + piece-top at 1,0 200 0 0 255\n\ + piece-top at 0,0 10 20 30 255\n\ + piece-bottom 2 1 1 7\n\ + piece-bottom at 0,0 0 200 0 255\n\ + piece-bottom at 1,0 10 20 30 255\n\ + sheet after 6 3 1 7\n\ + sheet at 5,0 200 0 0 255\n\ + sheet at 4,2 0 200 0 255\n" in if Sys.command "ldconfig -p 2>/dev/null | grep -q libraylib" = 0 then begin outputs "raylib images, headless" "programs/raylib-image.flan" @@ -1149,16 +1159,56 @@ let () = let p = Reader.read_file "programs/bounds.flan" |> Parse.program |> Check.program in - if not (contains (Emit.program p) "call void @flan_bounds_fail(") then begin + if not (contains (Emit.program p) "call void @flan_bounds_error(") then begin incr failures; print_endline "FAIL checks on: no bounds call emitted" end; let off = Emit.program ~checks:false p in - if contains off "call void @flan_bounds_fail(" || contains off "call void @flan_slice_fail(" then begin + if contains off "call void @flan_bounds_error(" + || contains off "call void @flan_slice_error(" then begin incr failures; print_endline "FAIL --no-bounds-checks: a check survived" end; + (* The other half of the same change: a bad index that something *answers*. + bounds.flan above is still the unhandled case and still exits 134 with + the same text; this one establishes a frame loop's `continue` and a + handler that takes it, and the program runs to the end. + + Three claims, and the second is the one that had to be decided rather + than inherited. (1) Five frames finish and seven are abandoned, out of + five routes to a bad index — fixed-array read, fixed-array write (a + different lowering), slice, Vec element and Vec as-slice, the last two + checked inside the runtime rather than in emitted IR. (2) `cleaned` is + 5, which is every defer on every one of those paths: an answered bounds + failure leaves through the same unwind path a `return` uses and runs + them, where a trap ran none. (3) The condition's numbers are the real + ones — 7 and -1 for the two bad `at`s, [2 9) and [3 1) for the two bad + slices, with `low` and `high` equal for an index and the two ends for a + range, which is why there is one condition type and not two. + + Also at -O0 and as a dev build. The dev build is here for the reason + the rest of the dev rows are — every call goes through a cell and a + shadow-stack frame is pushed per call — so this pins that the + indirection does not change where a transfer lands. It says nothing + about the break loop: this program does not import the agent, so + flan_break_hook is NULL in all three rows and the handler is what + answers. The break loop over a bad index is its own case, in + test_dev.ml, over programs/dev-break-bounds.flan, with nothing + handling it at all. *) + let bounds_cond_out = + "read 12\nlow 7\nlength 4\nlow -1\nwrote 99\nlow 4\n\ + slice 3\nlow 2\nhigh 9\nlength 5\nlow 3\nhigh 1\n\ + vec 200\nlow 5\nlength 2\nvec-slice 2\nhigh 9\n\ + frames 5\nskipped 7\ncleaned 5\n10 99 12 13\n" + in + outputs "a bad index is a condition" "programs/bounds-condition.flan" + bounds_cond_out; + outputs ~opt:"-O0" "a bad index is a condition, -O0" + "programs/bounds-condition.flan" bounds_cond_out; + outputs ~dev:true "a bad index is a condition, dev" + "programs/bounds-condition.flan" bounds_cond_out; + (* ── Packages: the link follows the program ──────────────────────── A package's C and linker arguments used to come with the import, whatever [main] did — which is what made sand's two halves two files diff --git a/test/test_dev.ml b/test/test_dev.ml index 38a8921..a4d71d9 100644 --- a/test/test_dev.ml +++ b/test/test_dev.ml @@ -764,6 +764,142 @@ let () = (try ignore (Unix.waitpid [] bpid) with Unix.Unix_error _ -> ()) end end; + (* ── A break over a bad index ──────────────────────────────────── *) + + (* The block above stops on an [error] the program wrote. This one stops on + one nobody wrote: an out-of-bounds index, which until now printed its + location and called exit(134) — taking the compiler and the session with + it, since [flan dev] is one process. + + Three claims, and the third is the point. The condition arrives named + [BoundsError] and its name resolves to a layout, so the conditions + buffer can show the numbers without anything special-casing it. The + restart on offer is the *program's* own [continue] — nothing is + established at the failing index, deliberately, because nothing a + handler could do would make index 9 valid for a length-4 array. And + taking it resumes: the transcript says 1, which is what [continue]'s + clause set, and the program goes on polling on the far side. + + Its own daemon and its own program, for the reason every block here has + one: these claims are about one frame of one program. *) + let xsock = tmp "break-bounds.sock" and xout = tmp "break-bounds.out" in + (try Sys.remove xsock with Sys_error _ -> ()); + let xfd = + Unix.openfile xout [ Unix.O_WRONLY; Unix.O_CREAT; Unix.O_TRUNC ] 0o600 + in + let xpid = + Unix.create_process flan + [| flan; "dev"; "programs/dev-break-bounds.flan"; "-s"; xsock |] + Unix.stdin xfd Unix.stderr + in + Unix.close xfd; + if not (await (fun () -> Sys.file_exists xsock)) then begin + fail "the bad-index daemon never listened"; + (try Unix.kill xpid Sys.sigkill with Unix.Unix_error _ -> ()) + end + else begin + let xoutput = Buffer.create 256 in + let c = connect xsock in + let ask sexp = + let r = Wire.parse (Wire.send c sexp; Wire.recv c) in + (match Wire.string_field r "output" with + | Some t -> Buffer.add_string xoutput t + | None -> ()); + r + in + let stopped r = + match Wire.field r "stopped" with + | Some { Form.v = Form.Sym "t"; _ } -> true + | _ -> false + in + let last = ref (ask "(:op \"describe\")") in + if not + (await (fun () -> last := ask "(:op \"describe\")"; stopped !last)) + then fail "a bad index never stopped the program" + else begin + let cname = + match Wire.string_field !last "condition" with Some c -> c | None -> "" + in + if cname <> "BoundsError" then + fail "a bad index is reported as %S, wanted %S" cname "BoundsError"; + (* The same round trip the block above makes: the name the break + reports is handed straight back as [:type], because that is the + conditions buffer's whole path. Three i64s — low, high and length — + with low and high the same index for an [at] and the two ends of a + range for a [slice], which is why there is one condition type and + not two. *) + let r = + ask (Printf.sprintf "(:op \"layout\" :type %s)" (Wire.quote cname)) + in + if status r <> "ok" then + fail "BoundsError did not resolve to a layout: %s" + (Option.value ~default:"" (Wire.string_field r "message")) + else + (match Wire.field r "fields" with + | Some { Form.v = Form.List fs; _ } -> + let names = + List.filter_map + (fun (e : Form.t) -> + match e.Form.v with + | Form.List ({ Form.v = Form.Str n; _ } :: _) -> Some n + | _ -> None) + fs + in + if names <> [ "low"; "high"; "length" ] then + fail "BoundsError's fields: %s" (String.concat ", " names) + | _ -> fail "BoundsError's layout has no fields"); + (* Only the program's own restart is on offer. Nothing is pushed at the + failing index, so a list with anything else on it would mean a site + restart had been established after all. *) + let r = ask "(:op \"break\")" in + if status r <> "ok" then fail "break over a bad index: %s" (status r); + (match Wire.field r "restarts" with + | Some { Form.v = Form.List l; _ } -> + let names = + List.filter_map + (fun (n : Form.t) -> + match n.Form.v with Form.Str x -> Some x | _ -> None) + l + in + if names <> [ "continue" ] then + fail "restarts at a bad index: %s" (String.concat ", " names) + | _ -> fail "break over a bad index listed no restarts"); + (* And the payoff: taking it resumes, which is the difference between a + stop you can recover from and a dead session. *) + let r = ask "(:op \"restart\" :name \"continue\")" in + if status r <> "ok" then + fail "continuing past a bad index: %s" + (Option.value ~default:"" (Wire.string_field r "message")); + let printed () = + ignore (ask "(:op \"describe\")"); + List.exists (String.equal "1") + (String.split_on_char '\n' (Buffer.contents xoutput)) + in + if not (await printed) then + fail "the program never resumed past a bad index"; + (* Ordinary work on the far side of it, which is the whole claim: the + session outlived the index. *) + let r = + ask "(:op \"eval-expr\" :code \"(+ 2 2)\" :file \"/tmp/buf.flan\")" + in + if Wire.string_field r "value" <> Some "4" then + fail "an expression after a bad index: %s" + (Option.value ~default:(status r) (Wire.string_field r "message")) + end; + ignore (ask "(:op \"close\")"); + Unix.close c; + if not + (await ~ms:5000 (fun () -> + match Unix.waitpid [ Unix.WNOHANG ] xpid with + | 0, _ -> false + | _ -> true + | exception Unix.Unix_error _ -> true)) + then begin + (try Unix.kill xpid Sys.sigkill with Unix.Unix_error _ -> ()); + (try ignore (Unix.waitpid [] xpid) with Unix.Unix_error _ -> ()) + end + end; + (* ── The locals of a stopped frame ─────────────────────────────── *) (* A third daemon, over a program that stops with something worth looking diff --git a/vendor/raylib/raylib.flan b/vendor/raylib/raylib.flan index f8ed5ad..c7df385 100644 --- a/vendor/raylib/raylib.flan +++ b/vendor/raylib/raylib.flan @@ -52,7 +52,8 @@ j 74 k 75 l 76 m 77 n 78 o 79 p 80 q 81 r 82 s 83 t 84 u 85 v 86 w 87 x 88 y 89 z 90 escape 256 enter 257 tab 258 backspace 259 - right 262 left 263 down 264 up 265]) + right 262 left 263 down 264 up 265 + left-shift 340]) (defenum MouseButton [left 0 right 1 middle 2 side 3 extra 4 forward 5 back 6]) @@ -65,6 +66,12 @@ (declare-c init-window [width i32 height i32 title string] "InitWindow") (declare-c close-window [] "CloseWindow") (declare-c window-should-close? [] bool "WindowShouldClose") + +;; False before init-window and after close-window, true between. A game loop +;; started twice is the thing this answers — an engine that can be re-entered +;; from a REPL or a dev session asks it before opening a second window onto +;; the same context. +(declare-c window-ready? [] bool "IsWindowReady") (declare-c set-target-fps [fps i32] "SetTargetFPS") (declare-c set-trace-log-level [level TraceLogLevel] "SetTraceLogLevel") @@ -355,6 +362,23 @@ (declare-c draw-texture-rec [texture Texture2D source Rectangle position Vector2 tint Color] "DrawTextureRec") +;; The two above, in one call, and the only one of the four that both takes a +;; source rectangle and scales: `source` picks a cell out of an atlas, `dest` +;; says where on the screen it lands and how big, so a 16px tile drawn at 4x is +;; a dest four times the source. draw-texture-rec has the source and no scale; +;; draw-texture-ex has the scale and no source. Neither half is usable alone +;; for a tilemap, which is why this is the draw call a grid-based game makes +;; every frame and for every tile. +;; +;; `origin` is the point within *dest* that lands on dest's x,y and that +;; `rotation` (degrees, clockwise) turns about — {0 0} draws from the corner, +;; and half the dest size spins a tile about its middle. A negative source +;; width or height flips, the same as in draw-texture-rec. +(declare-c draw-texture-pro + [texture Texture2D source Rectangle dest Rectangle origin Vector2 + rotation f32 tint Color] + "DrawTexturePro") + ;; ── Images ────────────────────────────────────────────────────────── ;; ;; An Image is pixels in RAM. Nothing here touches the GPU, which makes it the @@ -435,6 +459,18 @@ (declare-c image-crop [image (Ptr Image) crop Rectangle] "ImageCrop") +;; The non-mutating form of the line above, and the reason it is worth having +;; both: image-crop changes the image it is given, so carving a sheet into +;; twenty tiles with it destroys the sheet on the first one. This returns a +;; fresh Image and leaves the original alone. There is no ImageCopy in 5.5, so +;; this is also how a whole image is duplicated — a rec covering all of it. +;; +;; The result owns its own buffer: unload-image it, like anything else that +;; allocated. +(declare-c image-from-image + [image Image rec Rectangle] Image + "ImageFromImage") + (declare-c image-flip-horizontal [image (Ptr Image)] "ImageFlipHorizontal") (declare-c image-flip-vertical [image (Ptr Image)] "ImageFlipVertical")