diff --git a/DISCUSS.md b/DISCUSS.md index cb37870..92753e5 100644 --- a/DISCUSS.md +++ b/DISCUSS.md @@ -1096,3 +1096,156 @@ Two things are worth doing before that, and both are cheap. Decide what a bounds handler — because "no check" is what it means today and nothing says so. And take item 15's question 4 seriously now that there are two backends to disagree: `(uninit)` and `unreachable` already differ, deliberately, and the difference is currently documented only in a comment in `x86.ml`. + +## 17. Conditions on the x86 backend, and with them bounds checks: the corpus, not 41 programs + +Item 16's verdict was that conditions were the only obstacle left and that a bounds check was made of the same parts. +Both halves held. The transfer channel's guard, the landing pads, the per-function transfer exit, `fdefers` on it, +`emit_restart_case`, `emit_with_alloc`, `signal`, `error`, `handler-bind`, `invoke-restart`, `check_at` and +`check_slice` are all in `lib/x86.ml` now, written from `spec-conditions.md` and `emit.ml`'s semantics rather than +ported. **`dune test --root .` is green, LLVM is untouched and still the default and the release backend, and `--x86` +is still off by default and still refused with `--dev`, `--debug`, `--sanitize` and every wasm target.** + +### Question 1 — the counts, and what was compared + +`spike/x86/survey.sh`. Item 16 describes a script that builds every program both ways and diffs the output; it was +never committed, so this one is. It builds each program in `test/programs` and each probe in `spike/x86` twice — once +default, once `--x86`, **with the same bounds-check setting on both sides**, because a checked build compared against +an unchecked one says nothing about `bounds.flan` — runs both, and compares **stdout, stderr and the exit status**. + +stderr is not a detail. Every message the new machinery produces goes there — the bounds and slice errors, the three +restart refusals, the transfer failure — and each carries a `Loc.to_string` string this backend emits by hand as a +`.rodata` label and a length in a register. An exit status of 134 with the wrong text beside it is exactly the failure +that reads as a match. + +| | before | after | +|---|---|---| +| **MATCH** — same stdout, same stderr, same exit status | **41** | **89** | +| **DIFFER** | 1 (`bounds.flan`) | **0** | +| **refused by name** — a node this backend does not lower | 41 | **0** | +| skipped: does not compile (checker-error fixtures) | 25 | 25 | +| skipped: no `main` (package and library fixtures) | 6 | 6 | +| skipped: never terminates (`dev-loop`, `dev-watch`) | 2 | 2 | + +The "before" row is measured, not quoted from item 16 — it is the same corpus seven files larger, and the 41 refusals +split as 26 `restart-case`, 7 `signal`, 4 `handler-bind`, 1 `with-allocator`, 1 `fdefers`, and one each for +`flan_vec_at` and `flan_vec_as_slice`. + +**Every program in `test/programs` that compiles, has a `main` and terminates now goes through the hand-written backend +and agrees with the LLVM build.** That is the whole corpus: `restarts.flan`, `conditions.flan`, `bounds.flan`, +`bounds-condition.flan`, `allocators.flan`, `defers.flan`, the `Vec` and `Map` programs, `edn.flan`, `format.flan`. +The programs that only trap when given an argument — `restarts.flan`'s four signature-mismatch cases, `bounds.flan`'s +three — were run by hand with their arguments and agree on stderr and on exit 134 as well. + +### Question 2 — what a bounds violation means in a build with no handler + +**The same thing it means on the LLVM path, and that is the answer rather than a decision.** `check_at` and +`check_slice` here are `emit.ml`'s: a compare, a branch, a call to `flan_bounds_error` or `flan_slice_error` with the +transfer channel, and then **the guard** — which is why they could not exist before. The call is an ordinary one that +returns only when a handler or the break loop transferred, so the guard is the way out and the fall-through past it is +`ud2` where `emit.ml` writes `unreachable`. + +So: a bounds violation signals `BoundsError`; a `handler-bind` can answer it; a `restart-case` catches the transfer +and its clause's value stands; an unanswered one dies inside the runtime and the program exits 134 with the location +and the index. `--no-bounds-checks` omits the check on both backends and both then exit 139. **`bounds.flan` has +stopped being a DIFFER**, and the row of item 16's question-4 table that said "no check at all" is gone rather than +documented. + +The transitional refusal item 16 asked for — a build that says out loud that its behaviour differs — was not written, +because `check_at` landed in the same pass and the refusal would have been created and retired inside one commit. +There is nothing left to be loud about. + +Item 16's other two divergences are unchanged and still deliberate: `(uninit)` reads whatever the slot held rather +than LLVM's `poison`, and an exhausted `match` is `ud2` rather than `unreachable`. Both are still documented only in +`x86.ml`. + +### Question 3 — `check_no_transfer` narrowed, not removed + +It was a whole-program argument: this backend emitted no guard, which is sound exactly when nothing reachable can +write the channel, so the build refused by name the moment it found something that could. Every call site is guarded +now and the argument has retired — **in every function.** It still stands in one place, so the walk is still there and +now walks only global initialisers: + +A global's initialiser runs from `flan..init-globals`, before `main` and before anything has established a handler or +a restart. It owns its own channel cell because no caller hands it one, so a transfer out of it has nowhere to go — +its exit would return into the loader. `signal`, `error`, `restart-case` and `handler-bind` in a `defvar` initialiser +are refused by name. A bounds check there is *not* refused: it signals into a cell nothing is listening on, finds no +handler, and dies, which is the right answer. + +### Question 4 — five bugs, and four are item 16's shape exactly + +Found by what the programs printed, never by reading bytes. Two of them existed before this work and only became +reachable once the guard let the programs that expose them compile. + +**The body fell through into the transfer exit.** Every `fdefer` ran twice on a normal return, so `restarts.flan`'s +`log` was one too high at every checkpoint and nothing else was wrong. `emit.ml` cannot have this bug: its `ret` +terminates the block, and there is no fall-through to forget. This is the same class as item 16's "a discarded value +was stored over the return address" — a construct this backend has that LLVM does not. + +**A `Vec` crossed to the runtime as the address of a copy.** `eval` copies an aggregate into a temporary, so +`flan_vec_push` grew the temporary and the caller's header stayed at length zero — and an *in-bounds* `(at v 1)` then +signalled against a length of 0. `emit.ml` says so in a comment beside its own `addr`; this had no such case. Pre- +existing, and invisible until a program using a `Vec` could build. + +**`ucomis` sets CF, ZF and PF together for a NaN**, so `sete` answered *true* for `(= x x)`. Flan's comparisons are +LLVM's *ordered* ones (`oeq`, `olt`, ...), which are false for a NaN; the unsigned table is not those. `<` and `<=` +now swap their operands and ask for a/ae, and `=` and `!=` take a `setnp` beside them. The symptom was +`(/ 0.0 0.0)` formatting as `-9223372036854775808`, because the prelude's NaN test is `(not (= x x))` and nothing +else. Pre-existing; `format.flan` could not build before. + +**A union read field 0 through the struct table** and was refused by name. A union is a tag and a payload blob, which +is a two-field struct at that level, and the structural printer reads the tag without unwrapping the value. + +**And one that could not have been found later:** `emit_globals_init` stored a null *into* the channel slot rather +than a cell's address into it, so every callee of a global initialiser was handed a null pointer to write a transfer +through. Harmless while nothing could transfer; a fault the first time a guard loaded through it. `emit_main` had it +right and was the model. + +Against those: nothing went wrong with the frame, the stack alignment, or the pads' nesting. The one place worth +naming is the one item 16 could not have: **the channel is one indirection deeper here than in `emit.ml`.** There +`%xfer` is an alloca and the target is one `load` away; here `xfer_off` is a frame slot *holding the caller's +pointer*, so reading the target is two loads and clearing the channel is a store *through* the pointer and never a +store to the slot. `chan_into`, `xfer_load`, `xfer_store` and `xfer_clear` exist so that no call site has to remember +which. + +### Question 5 — what the corpus does not walk, and the probe that does + +Every pad has two halves: the one a body reaches by finishing, and the one a transfer reaches by passing through. The +corpus walks the first everywhere and the second in one place only — `allocators.flan`'s last case aims an +`invoke-restart` out of a `with-allocator` body at a `restart-case` outside it, which is the `wxfer` re-propagation. + +The other two it never reaches. In `restarts.flan` every transfer stops at a `restart-case` *inside* the +`handler-bind`'s extent, so the handler frames never come off on the transfer path; and in `nested` and `shadowed` the +*inner* restart frame offers the name, so a restart-case that the transfer is not aimed at never has to put the target +back. `spike/x86/p6-transfer.flan` is those two, beside a defer and a clause parameter, and it agrees with the LLVM +build. It is in the survey, and it is why the count is 89 rather than 83. + +The branch nothing exercises is `flan_transfer_fail` — a defer that starts a *second* transfer while the first is +unwinding. It is emitted and refused loudly, and it is untested. + +### The honest no-plan bucket + +- **`Rt` with an aggregate return.** Still refused by name. `flan_vec_as_slice` returns a slice by value, and + `bounds-condition.flan` exercises it both in and out of bounds through a `restart-case` and matches — so it does not + reach the refusal, and **nobody traced why.** That is a loose end, not a result. +- **`Fnval`'s indirection cell.** `FnAddr (Fnval n)` still emits the symbol. Correct for a whole-program build, wrong + the instant anything is redefined into it; there are no cells here and no `--dev`, deliberately. +- **`f64` → `i64` out of range**, and **`INT64_MIN / -1`**. Unchanged from items 15 and 16: `idiv` raises `SIGFPE` + where LLVM says undefined. A language decision, not a backend one. +- **`(uninit)` and `unreachable`** still differ from LLVM on purpose and are still written down only in a comment. +- **The `flan_transfer_fail` branch**, above. +- **`"defers on a transfer path nothing reaches"`** — the refusal that replaced the old `fdefers` one. No program in + the corpus hits it; it exists so that if the reasoning behind it is ever wrong, it says so. +- **Debug information.** None. `--x86` and `--debug` together are still refused. +- **Code size and speed.** Still not measured, and now there is more to measure: a guard after every call, two loads + and a branch each, and a bounds check that spends three frame temporaries. Nobody has put a number on any of it. + +### The verdict + +**The row with no plan is gone.** What item 15 called the last obstacle and item 16 called the only one is written, +and the measurement that made it the only one is the measurement that says it is finished: every program in the corpus +that can run, runs, and prints what LLVM's build prints — down to stderr. + +What is left in `x86.ml` is not conditions. It is a container return convention, a redefinition cell, two arithmetic +edge cases the language has not decided, and no debug info. None of those is the shape conditions were: each is a +known thing in a known place, and the guard is not underneath any of them. diff --git a/lib/x86.ml b/lib/x86.ml index f484184..3ca87bf 100644 --- a/lib/x86.ml +++ b/lib/x86.ml @@ -2086,6 +2086,9 @@ let emit_fn (md : Emit.m) ~externs ~fns (fn : Tast.fn) : string * string = Emitted here, *before* the prologue buffer is made, because [frame_bytes] is read when the prologue is built and everything below allocates temporaries and makes calls that move the high-water mark. *) + let zero_return () = + if not (is_void fn.Tast.ret) then zero_value f (ret_loc f) fn.Tast.ret + in if f.unwound then begin (* The body falls through to the epilogue, so it has to be sent there explicitly before this: otherwise the last statement runs straight into @@ -2093,6 +2096,11 @@ let emit_fn (md : Emit.m) ~externs ~fns (fn : Tast.fn) : string * string = have this bug — its [ret] terminates the block. *) jmp_lbl f.b f.retlbl; lbl f.b f.xfer_lbl; + (* [emit.ml] leaves here with [ret zeroinitializer]. The value is + meaningless to a caller — its guard sees the channel set and never looks + at it — but [main] is a caller with no guard, and what it finds in [rax] + is the process exit status. Zero rather than whatever the return + temporary held. *) if fn.Tast.fdefers <> [] then begin (* The channel is cleared while the defers run and put back after. A defer makes ordinary calls and each one is guarded; with the channel @@ -2107,6 +2115,7 @@ let emit_fn (md : Emit.m) ~externs ~fns (fn : Tast.fn) : string * string = f.pads <- []; load_int f.b ~dst:rax ~mm:(Frame saved) ~size:8 ~signed:false; xfer_store f ~reg:rax ~scratch:r11; + zero_return (); jmp_lbl f.b f.retlbl; (* A defer that starts a *second* transfer while the first is unwinding. §6's per-frame slot nests, but nothing here does: the first @@ -2118,7 +2127,7 @@ let emit_fn (md : Emit.m) ~externs ~fns (fn : Tast.fn) : string * string = die f "flan_transfer_fail" end end - else jmp_lbl f.b f.retlbl + else begin zero_return (); jmp_lbl f.b f.retlbl end end else if fn.Tast.fdefers <> [] then (* Nothing in this function can transfer, so the second exit has no path diff --git a/spike/x86/p6-transfer.flan b/spike/x86/p6-transfer.flan new file mode 100644 index 0000000..b1951ab --- /dev/null +++ b/spike/x86/p6-transfer.flan @@ -0,0 +1,72 @@ +;;;; The re-propagation branches, which the corpus walks past. +;;;; +;;;; Every landing pad this backend emits has two halves: the one a body +;;;; reaches by finishing, and the one a transfer reaches by passing through. +;;;; `test/programs` exercises the first everywhere and the second only for +;;;; with-allocator (allocators.flan's last case aims an invoke-restart out of +;;;; a with-allocator body at a restart-case outside it). The two below it does +;;;; not reach at all, so they are here: +;;;; +;;;; hxfer a transfer crossing a handler-bind, which has to take the handler +;;;; frames off before it goes any further -- in restarts.flan every +;;;; transfer stops at a restart-case *inside* the handler-bind's +;;;; extent, so that pop never runs on the transfer path +;;;; +;;;; rxfer's last branch +;;;; a restart-case the transfer is not aimed at: it puts the target +;;;; back in the channel and re-propagates. restarts.flan's `nested` +;;;; and `shadowed` both have the *inner* frame offering the name, so +;;;; the inner one always wins and this branch is never taken. +;;;; +;;;; Proved the way everything else here is: built both ways, and compared by +;;;; what it prints. + +(defstruct Blip [n i32]) + +(defvar log i64) + +;;; Two frames down, with a defer between, so the transfer crosses a function +;;; boundary and a transfer exit that has work to do. +(defn deep [n i32] i32 + (signal (Blip {.n n})) + 0) + +(defn middle [n i32] i32 + (defer (set log (+ log 1))) + (deep n)) + +;;; The handler frames come off on the transfer path. The restart-case is +;;; outside the handler-bind, so the pad pops and re-propagates rather than +;;; the body's own pop running. +(defn crosses-handler [n i32] i32 + (restart-case + (handler-bind [(Blip [c] (invoke-restart 'outer-one))] + (middle n)) + (outer-one [] 11))) + +;;; An inner restart-case that does not offer the name. Its pad clears the +;;; channel, pops its frames, matches nothing, and puts the target back. +(defn crosses-restart [n i32] i32 + (restart-case + (handler-bind [(Blip [c] (invoke-restart 'outer-two))] + (restart-case (middle n) + (inner-only [] 22))) + (outer-two [] 33))) + +;;; Both at once, and a parameter as well, so the buffer the outer frame owns +;;; is written by an invoke two pads and one function away from it. +(defn crosses-both [n i32] i32 + (restart-case + (handler-bind [(Blip [c] (invoke-restart 'outer-three 7)) ] + (restart-case (middle n) + (inner-only [] 44))) + (outer-three [v i32] (* v 100)))) + +(defn main [] i32 + (print (crosses-handler 1)) (println "") ; 11 + (print log) (println "") ; 1 — the defer ran + (print (crosses-restart 2)) (println "") ; 33 + (print log) (println "") ; 2 + (print (crosses-both 3)) (println "") ; 700 + (print log) (println "") ; 3 + 0) diff --git a/spike/x86/survey.sh b/spike/x86/survey.sh index 4e89ec5..785b1c0 100755 --- a/spike/x86/survey.sh +++ b/spike/x86/survey.sh @@ -4,8 +4,16 @@ # The only honest test of a hand-encoded backend is what the program prints and # what it exits with -- DISCUSS.md item 15 and item 16 both say so, and both # say it after a disassembly that read perfectly beside a wrong answer. So this -# builds every program in test/programs twice, runs both, and diffs stdout and -# the exit status. objdump is for after a program already has the wrong answer. +# builds every program in test/programs twice, runs both, and diffs stdout, +# stderr and the exit status. objdump is for after a program already has the +# wrong answer. +# +# stderr is not an afterthought: every message the condition machinery produces +# goes there -- the bounds and slice errors, the three restart refusals, the +# transfer failure -- and each carries a location string this backend emits by +# hand as a .rodata label and a length in a register. An exit status of 134 +# with the wrong text beside it is exactly the failure that looks like a +# match. # # Both sides get the same bounds-check setting (the default: on). A sweep that # compared a checked build against an unchecked one would say nothing about @@ -13,12 +21,15 @@ # # Five outcomes, and the third is the progress meter: # -# MATCH built both ways, same stdout, same exit status +# MATCH built both ways, same stdout, same stderr, same exit status # DIFFER built both ways, and disagreed # REFUSED X86.Unsupported -- a node this backend does not lower (exit 3) # NOX86 failed to build through --x86 for some other reason # SKIP no main, does not compile at all, or does not terminate # +# Over test/programs, and over spike/x86's own probes, which are here for the +# paths the corpus does not walk. +# # Usage: spike/x86/survey.sh [name-substring ...] set -u here=$(cd "$(dirname "$0")" && pwd) @@ -40,7 +51,7 @@ TIMEOUT=${TIMEOUT:-20} declare -a match=() differ=() refused=() nox86=() skip=() -for src in "$root"/test/programs/*.flan; do +for src in "$root"/test/programs/*.flan "$root"/spike/x86/*.flan; do name=$(basename "$src" .flan) if [ $# -gt 0 ]; then want=0 @@ -72,17 +83,21 @@ for src in "$root"/test/programs/*.flan; do continue fi - ( cd "$out" && timeout "$TIMEOUT" "$out/$name.llvm" >"$out/$name.llvm.out" 2>/dev/null ) + ( cd "$out" && timeout "$TIMEOUT" "$out/$name.llvm" \ + >"$out/$name.llvm.out" 2>"$out/$name.llvm.diag" ) a=$? - ( cd "$out" && timeout "$TIMEOUT" "$out/$name.x86" >"$out/$name.x86.out" 2>/dev/null ) + ( cd "$out" && timeout "$TIMEOUT" "$out/$name.x86" \ + >"$out/$name.x86.out" 2>"$out/$name.x86.diag" ) b=$? - if [ "$a" = "$b" ] && cmp -s "$out/$name.llvm.out" "$out/$name.x86.out"; then + if [ "$a" = "$b" ] && cmp -s "$out/$name.llvm.out" "$out/$name.x86.out" \ + && cmp -s "$out/$name.llvm.diag" "$out/$name.x86.diag"; then match+=("$name") else differ+=("$name:llvm=$a/x86=$b") if [ "${SURVEY_SHOW:-}" = 1 ]; then echo "--- $name: llvm exit $a, x86 exit $b" diff "$out/$name.llvm.out" "$out/$name.x86.out" | head -20 + diff "$out/$name.llvm.diag" "$out/$name.x86.diag" | head -20 fi fi done