From 10b736f23e0788c33d8a4cbec632146c4f1b6451 Mon Sep 17 00:00:00 2001 From: Joseph Ferano Date: Sat, 12 Sep 2026 14:47:26 +0700 Subject: [PATCH 1/3] The slot fingerprint was emitted and never read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The refusal for a frame whose body has been redefined underneath it did not fire because four of its five hand-offs were never written. `Emit.fninfo` has been storing `slot_fingerprint` in the last `i32` of every `%fninfo` all along; `flan_dev.c` called that field `spare`, there was no accessor for it, the agent never snapshotted it, the backtrace line never carried it, and `Dev.locals` compared slot counts and nothing else. The handoff note's "every piece is written and the refusal does not happen" was a guess, and the first step it suggested — printing both sides of the comparison — could not have found it, because there was no comparison. So: `spare` becomes `slotsig` and gets `flan_dev_frame_slotsig`; the agent snapshots it beside the slot count and puts it on the backtrace line *before* the location, since the name is the one field that can contain a space and has to stay last; `Dev.backtrace` parses it; `Dev.locals` compares it against `Emit.slot_fingerprint` of the body this session holds and refuses by name when they differ. No change to `emit.ml` — the value was already there. The mechanism itself is right and stays. `slot_fingerprint` hashes every slot's name together with the spelling of its type, so a rename that keeps the count and the types — exactly the case this exists for — changes it. The count check stays in front of it because its message is the more specific one. The fingerprint stays off the wire. A hash is not something an editor can act on, and the refusal says the fact in words: this frame's body was redefined since it was entered, so its names no longer describe its values. `test_dev.ml` gains the inverse and the control. A body that drops a `let` is refused on the count, and `main` — untouched by the redefinition of `look` — must still answer, which is the assertion that would catch a fingerprint that never matched anything and made the verb useless while turning the suite green. --- lib/dev.ml | 52 +++++++++++++++++++++++++++++++-------- runtime/flan_dev.c | 16 +++++++++++- test/test_dev.ml | 29 +++++++++++++++++++++- vendor/agent/flan_agent.c | 14 +++++++++-- 4 files changed, 97 insertions(+), 14 deletions(-) diff --git a/lib/dev.ml b/lib/dev.ml index bdf2443..0206371 100644 --- a/lib/dev.ml +++ b/lib/dev.ml @@ -235,7 +235,15 @@ let restarts t = framing [restarts] uses, terminated by a lone dot, because it comes back over the same one-line-out socket. - Each line is [I ± NSLOTS LOC NAME]. The flag says whether the frame belongs + Each line is [I ± NSLOTS SIG LOC NAME]. [SIG] is the slot fingerprint of + the body this frame was compiled from — [Emit.slot_fingerprint] over the + name and the type of every slot — and it is how [locals] tells a frame whose + body has been redefined underneath it from one that still matches. It stays + off the wire: a hash is not something a client can act on, and the refusal + it produces says the fact in words instead. It sits before [LOC] because + [NAME] is the only field that can contain a space and so has to be last. + + The flag says whether the frame belongs to the program or to the C-x C-e thunk the break happens to be inside: a break inside an evaluation has that evaluation's frames on top, and answering "where is my program" with [eval/7] would be true and useless. @@ -264,11 +272,14 @@ let backtrace t = end else match String.split_on_char ' ' line with - | idx :: flag :: nslots :: loc :: rest when rest <> [] -> - (match int_of_string_opt idx, int_of_string_opt nslots with - | Some _, Some k -> + | idx :: flag :: nslots :: sig_ :: loc :: rest when rest <> [] -> + (match + int_of_string_opt idx, int_of_string_opt nslots, + int_of_string_opt sig_ + with + | Some _, Some k, Some g -> Some (String.concat " " rest, (if loc = "?" then "" else loc), - flag = "+", k) + flag = "+", k, g) | _ -> None) | _ -> None in @@ -700,7 +711,7 @@ let backtrace_op t = [ ":frames " ^ Wire.list (List.map - (fun (name, loc, mine, nslots) -> + (fun (name, loc, mine, nslots, _sig) -> Wire.list [ Wire.quote name; Wire.quote loc; Wire.quote (if mine then "program" else "eval"); @@ -731,9 +742,17 @@ let backtrace_op t = showed neither would be the same lie twice. And two whole frames it refuses: one belonging to a [C-x C-e] thunk, which - this session does not keep the [Tast] of, and one whose slot count does not - match the body this session holds — which is a frame running a body that - has since been redefined, where every slot index would be a guess. *) + this session does not keep the [Tast] of, and one whose *body* is not the + body this session holds. The second is the one that needed a fingerprint + rather than a count: installing while stopped is deliberately allowed — it + is the fix-it-and-retry loop — so the frame on the stack and the body here + can be two bodies of one function, and a redefinition that renames a local + changes neither the count nor the types. [Emit.slot_fingerprint] hashes + every slot's name together with the spelling of its type, the frame carries + the value for the body it was compiled from, and this end recomputes it + from the body it holds. A collision is possible in principle — it is a + 30-bit hash — but only between two differing bodies of the function whose + qualified name already matched, since [find_fn] gates the comparison. *) let locals t ~frame = if not (alive t) then error "the program exited; restart flan dev" else @@ -752,7 +771,7 @@ let locals t ~frame = error (Printf.sprintf "there is no frame %d; the backtrace has %d" frame (List.length frames)) - | Some (name, _, mine, nslots) -> + | Some (name, _, mine, nslots, sig_) -> if not mine then error (name @@ -775,6 +794,19 @@ let locals t ~frame = (Printf.sprintf "%s on the stack has %d slots and the %s this session holds has %d: the frame is running a body that has been redefined since, so every slot index here would be a guess" name nslots name (Array.length fn.Tast.slots)) + else if sig_ <> Emit.slot_fingerprint fn then + (* The count matching is not the same as the body matching. + A redefinition that renames a local, or changes its type + to one of the same shape, keeps the count — and then + every name here would be the new body's read against the + old body's storage, which is the "visible rather than + correct" answer this project refuses to give. Said by + name, because a frame that is missing and a frame that + cannot be trusted are different facts. *) + error + (Printf.sprintf + "%s on the stack was compiled from a different body than the %s this session holds: this frame's body was redefined since it was entered, so its names no longer describe its values" + name name) else match bound_slots t ~frame with | Error m -> error ("the program refused to say which slots are bound: " ^ m) diff --git a/runtime/flan_dev.c b/runtime/flan_dev.c index 0be0ba9..0293214 100644 --- a/runtime/flan_dev.c +++ b/runtime/flan_dev.c @@ -335,7 +335,13 @@ typedef struct { const char *loc; /* file:line:col, as Loc spells it */ int64_t loclen; int32_t nslots; - int32_t spare; + /* What the two ends compare about a frame's slots, from + [Emit.slot_fingerprint]: a hash over every slot's name and the spelling of + its type. The frame carries the one belonging to the body it was compiled + from; the daemon recomputes it from the body it now holds, and a + difference means the frame is running a superseded body. A count alone + cannot see a rename, which is the case this exists for. */ + int32_t slotsig; } flan_fninfo; typedef struct flan_frame { @@ -390,6 +396,14 @@ int32_t flan_dev_frame_nslots(const void *frame) { return (f == NULL || f->info == NULL) ? 0 : f->info->nslots; } +/* The fingerprint of the body this frame was compiled from. Zero for a frame + * with no description, which is the same "nothing to compare" a zero slot + * count already means. */ +int32_t flan_dev_frame_slotsig(const void *frame) { + const flan_frame *f = frame; + return (f == NULL || f->info == NULL) ? 0 : f->info->slotsig; +} + /* Where slot [i] of this frame lives, or NULL — which means one of three * things, all of which are "there is nothing to read here": this build records * no slots, the index is not one of them, or the binding that fills it had not diff --git a/test/test_dev.ml b/test/test_dev.ml index 84a8662..264ba63 100644 --- a/test/test_dev.ml +++ b/test/test_dev.ml @@ -806,6 +806,24 @@ let () = let r = ask "(:op \"locals\" :frame 9)" in if status r <> "error" then fail "a frame index past the end answered"; + (* The inverse, first, because it is the cheap half of the same + claim: a redefinition that really does change the slots must be + refused too, and that one a count comparison can see. This body + drops [flag] and keeps the signature, so the frame on the stack has + one slot more than the body this session now holds. *) + let r = + ask + "(:op \"eval\" :code \"(defn look [n i64 label string] i64 (let [p (Point {:x 1.5 :y 2.5}) xs [10 20 30]] (restart-case (do (error (Boom {:why 7})) (let [after (i64 99)] after)) (carry-on [] 5))))\" :file \"/tmp/buf.flan\")" + in + if status r <> "ok" then + fail "installing a body with fewer slots while stopped: %s" + (Option.value ~default:"" (Wire.string_field r "message")) + else begin + let r = ask "(:op \"locals\" :frame 0)" in + if status r <> "error" then + fail "the frame of a body whose slots changed answered anyway" + end; + (* And the case that makes this a fingerprint rather than a slot count. Installing while stopped is deliberately allowed — it is the fix-it-and-retry loop — so the body on the stack and the body the @@ -823,7 +841,16 @@ let () = else begin let r = ask "(:op \"locals\" :frame 0)" in if status r <> "error" then - fail "the frame of a superseded body answered with the new body's names" + fail "the frame of a superseded body answered with the new body's names"; + (* And the other half of that claim, which is the one a fingerprint + that never matched would fail: redefining [look] says nothing + about [main], and its frame must still answer. A refusal that + fires for every frame would pass the test above and make the + whole verb useless. *) + let r = ask "(:op \"locals\" :frame 1)" in + if status r <> "ok" then + fail "redefining one function refused an untouched frame: %s" + (Option.value ~default:(status r) (Wire.string_field r "message")) end end; (* Running again, and then the locals verb is refused: a frame that is diff --git a/vendor/agent/flan_agent.c b/vendor/agent/flan_agent.c index 24f3e54..1479ab8 100644 --- a/vendor/agent/flan_agent.c +++ b/vendor/agent/flan_agent.c @@ -155,6 +155,7 @@ extern void *flan_dev_frame_at(int32_t i); extern const char *flan_dev_frame_name(const void *frame, int64_t *len); extern const char *flan_dev_frame_loc(const void *frame, int64_t *len); extern int32_t flan_dev_frame_nslots(const void *frame); +extern int32_t flan_dev_frame_slotsig(const void *frame); extern void *flan_dev_frame_slot(const void *frame, int32_t i); extern const uint8_t *flan_restart_name(int32_t i, int64_t *len); extern void *flan_restart_frame(int32_t i); @@ -259,6 +260,8 @@ typedef struct { int32_t fnoff[FRAME_MAX], fnlen[FRAME_MAX]; int32_t floff[FRAME_MAX], fllen[FRAME_MAX]; int32_t fslots[FRAME_MAX]; + int32_t fsig[FRAME_MAX]; /* the slot fingerprint of the body + * this frame was compiled from */ int32_t fmine[FRAME_MAX]; /* 0 = the evaluation's, not the * program's */ char ftext[FRAME_TEXT]; @@ -363,6 +366,10 @@ static int snap_push(void) { s->fused += (int32_t)ll; s->ftext[s->fused++] = 0; s->fslots[s->fn] = flan_dev_frame_nslots(fr); + /* Snapshotted with the rest of the frame rather than read later: the + * module this description lives in can be unloaded once the daemon + * installs a replacement, and the comparison happens after that. */ + s->fsig[s->fn] = flan_dev_frame_slotsig(fr); /* The outermost [frame_floor] frames are the program's; anything above * them belongs to the evaluation this break is inside. */ s->fmine[s->fn] = (frame_floor < 0) || (i >= fn - frame_floor); @@ -667,8 +674,11 @@ static void serve(int fd) { } for (int32_t i = 0; i < s->fn; i++) { char hdr[64]; - int k = snprintf(hdr, sizeof hdr, "%d %c %d ", i, - s->fmine[i] ? '+' : '-', s->fslots[i]); + /* The fingerprint goes before the location and the location before + * the name, because the name is the one field that can contain a + * space and so has to be last. */ + int k = snprintf(hdr, sizeof hdr, "%d %c %d %d ", i, + s->fmine[i] ? '+' : '-', s->fslots[i], s->fsig[i]); if (k > 0) send(fd, hdr, (size_t)k, MSG_NOSIGNAL); if (s->fllen[i] > 0) send(fd, s->ftext + s->floff[i], (size_t)s->fllen[i], MSG_NOSIGNAL); From b5d7a6e45f24e411f620e80c2ab183a82de0e7a0 Mon Sep 17 00:00:00 2001 From: Joseph Ferano Date: Sat, 12 Sep 2026 14:48:45 +0700 Subject: [PATCH 2/3] Say what the fingerprint is for, and correct the note that guessed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BUILT.md's locals section said the second whole-frame refusal was a slot count mismatch. It is a fingerprint, and the paragraph now says why a count could not have done the job: the case it exists for is a rename, which changes neither the count nor the types. It also states the bound honestly — a 30-bit hash can collide, and a collision would reproduce exactly the wrong answer this catches, but only between two differing bodies of a function whose name already matched. NEXT.md's item 1 is struck, and the handoff paragraph that diagnosed this is marked wrong rather than deleted. It claimed every piece was written and one of five hand-offs was dropping the number; four were never written. The step it recommended first could not have found that, and a lane stopping mid-repair should say which pieces it ran rather than which it believes it wrote. --- BUILT.md | 28 ++++++++++++++++++++++++++-- NEXT.md | 41 +++++++++++++++++------------------------ 2 files changed, 43 insertions(+), 26 deletions(-) diff --git a/BUILT.md b/BUILT.md index 5f882f8..eb18060 100644 --- a/BUILT.md +++ b/BUILT.md @@ -1214,8 +1214,32 @@ on what it shows rather than on what it saves. **Four refusals, each by name and with its reason.** Three per slot — invented, not yet bound, and no printer for the type (a map, a function value, a type variable; the arm exists, no program the checker accepts has reached it yet, so it is written and untested) — and two whole frames: one belonging to a `C-x C-e` thunk, whose `Tast` the session does -not keep, and one whose slot count does not match the body this session holds, which is a frame running a body that has -been redefined since and where every slot index would be a guess. +not keep, and one whose *body* is not the body this session holds. + +**The superseded frame, and why a count could not find it.** Installing while stopped is deliberately allowed — it is +the fix-it-and-retry loop — so the frame on the stack and the body the session holds can be two bodies of one function. +A slot count catches a body that gained or lost a binding and nothing else; the case that matters is a **rename**, which +changes neither the count nor the types, and which would otherwise show every *new* name against the *old* body's +storage with nothing saying so. That is the confident wrong answer, in the one place someone is working out what went +wrong. + +So each `%fninfo` carries a **slot fingerprint**: `Emit.slot_fingerprint` over every slot's name together with the +spelling of its type, since either can change on its own. The frame carries the value for the body it was compiled from, +`flan_dev_frame_slotsig` reads it, the agent snapshots it with the rest of the frame and puts it on the backtrace line, +and `Dev.locals` recomputes it from the body it holds and compares. Different means refused by name — *this frame's body +was redefined since it was entered, so its names no longer describe its values* — rather than answered. The count check +stays in front of it because its message is the more specific one. + +Computed in `emit.ml` and read from there by `dev.ml`, so there is one definition of it and the two ends cannot drift. +It stays **off the wire**: `(:op "backtrace")` still answers four fields per frame, because a hash is not something an +editor can act on and the refusal says the fact in words instead. The bound is worth stating: it is a 30-bit hash, so a +collision is possible in principle, and it would reproduce exactly the silent wrong answer this catches — but only +between two *differing* bodies of a function whose qualified name has already matched, since `find_fn` is what gates the +comparison at all. + +The first build of this shipped the fingerprint into `%fninfo` and stopped there — no accessor, no wire field, no +comparison — and was left in the tree with its own test red. Four of five hand-offs missing looks exactly like one +hand-off dropping a number, which is what the note left behind said it was. **A `(Vec T)` shows as `` and a `(Ptr T)` as ``**, because that is what `render.ml` already does for them everywhere else: following a pointer a REPL was handed is not a safe thing to do on someone's behalf, and walking a diff --git a/NEXT.md b/NEXT.md index 5b0d339..13fb82d 100644 --- a/NEXT.md +++ b/NEXT.md @@ -410,11 +410,13 @@ forgotten. Agreed at the end of 2026-09-12. Ordered by priority, not by size. Items 1-3 and 5-6 want the compiler core and should run one lane at a time; item 4 is disjoint and runs alongside any of them. -1. **Fix the one failing test** — `the frame of a superseded body answered with the new body's names`. The slot - fingerprint that should refuse a frame whose body was redefined since it was entered does not fire, so a rename - keeping the same slot count and types shows new names against old values. The suite has been green all session apart - from this, and a red suite stops being a signal quickly. The handoff says to start by printing both sides of the - comparison in `Dev.locals`. +~~1. **Fix the one failing test** — `the frame of a superseded body answered with the new body's names`.~~ **Done**, and + the handoff's diagnosis was wrong. Nothing was dropping the number: the fingerprint was emitted into `%fninfo` and + never read back. `flan_dev.c` called the field `spare`, there was no accessor, the agent never snapshotted it, the + backtrace line never carried it, and `Dev.locals` compared slot counts and nothing else — four of the five + hand-offs were never written, and printing both sides of the comparison could not have found it because there was + no comparison. The mechanism was sound and stayed: it hashes slot *names* as well as types, so it does see a + rename. See BUILT.md, "Locals of a stopped frame". 2. **The colon-to-dot change.** Cheap, mechanical, ~284 sites across 45 files — and it must land **before** `Map`, or map literals and struct literals collide and both have to change instead of one. It gets more expensive every day @@ -1096,26 +1098,17 @@ tests assert on the reason, not just on the failure. ## Handoff: the shadow stack lane, stopped mid-repair Two commits landed and are green: the shadow stack with `(:op "backtrace")`, and `(:op "locals" :frame N)`. See -BUILT.md's two new sections for the design and the measurements. A third commit is **half-built and its own test is -red**, deliberately left that way rather than deleted. +BUILT.md's two new sections for the design and the measurements. A third commit was **half-built and its own test +left red** on purpose; **it is finished now** — see the struck item 1 above — and the rest of this section is kept +because the parts of it that were true are still worth having. -**What is broken, exactly.** `locals` compares the frame on the stack against the body this session holds, and the -comparison is not firing. Installing while stopped is deliberately allowed, so the two can be different bodies of one -function — and a redefinition that renames the locals while keeping their count and types shows every *new* name -against the *old* body's values, with nothing refused. `test_dev.ml`'s "the frame of a superseded body answered with -the new body's names" fails on exactly that, and reproducing it takes one run of that test. - -The fix that is in the tree and does not work yet: `Emit.slot_fingerprint` hashes each slot's name and the spelling of -its type; `emit_fn` stores it in the `flan_fninfo` the frame points at; `flan_dev_frame_slotsig` reads it; the agent -puts it on the `backtrace` line as a fifth field; `Dev.locals` compares it with `Emit.slot_fingerprint fn`. Every piece -is written and the refusal does not happen, so **one of those five hand-offs is dropping the number** — the next person -should print both sides of that comparison first, which is a two-line change in `Dev.locals`, rather than re-deriving -the design. The likeliest suspects in order: `Dev.backtrace`'s line parse silently falling through to `None` for the -new five-field line (it would drop the frame entirely, so probably not); `find_fn` handing back a stale `Tast.fn`; the -hash being computed over a `snames` array that the redefinition path fills in differently. - -Until it is fixed, `locals` is trustworthy for a frame whose body has not been redefined since it was entered — which -is every frame in a program that has not been edited while stopped — and silently wrong for one that has. +**What is broken, exactly — and this paragraph was wrong; kept for what it cost.** It said `locals` compares the +frame on the stack against the body this session holds and the comparison is not firing, that every piece of the +fingerprint was written, and that one of five hand-offs was dropping the number. Four of the five were never written at +all: `Emit.fninfo` stored the fingerprint and nothing else touched it. The first step it recommended — printing both +sides of the comparison in `Dev.locals` — could not have worked, because `Dev.locals` had no comparison to print. The +lesson is the ordinary one: a lane that stops mid-repair should say which pieces it *ran*, not which it believes it +wrote. **Not obvious from the diff.** Two things cost a day between them. The linked-list frame beat an array-with-a-stack- pointer on both benchmarks, which is the opposite of what the escaping-alloca argument predicts, and the measurement From 4e6b3f618369456100c4d16424b221c7c262172b Mon Sep 17 00:00:00 2001 From: Joseph Ferano Date: Sat, 12 Sep 2026 14:51:24 +0700 Subject: [PATCH 3/3] A frame with no slots still has a body, and the note spoke for it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "every slot in it is one the compiler made up" is a claim about the body this session holds, not about the frame, and it was answered before either body check ran — so a zero-slot frame whose body had since been replaced by one with slots got that note instead of the refusal. No values were misattributed, which is why it is not the defect just fixed, but the reason given was untrue. The count and fingerprint checks now run first and the note is the last arm. --- lib/dev.ml | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/lib/dev.ml b/lib/dev.ml index 0206371..24b2c38 100644 --- a/lib/dev.ml +++ b/lib/dev.ml @@ -783,13 +783,12 @@ let locals t ~frame = (name ^ " is not a function this session holds; a lifted handler clause has no declaration of its own to read slot names from") | Some fn -> - if nslots = 0 then - ok - [ ":frame " ^ Wire.quote name; ":locals ()"; ":refused ()"; - ":note " - ^ Wire.quote - "that frame records no slots; every slot in it is one the compiler made up" ] - else if nslots <> Array.length fn.Tast.slots then + (* The two body checks come first, including for a frame + with no slots. "every slot in it is one the compiler made + up" is a claim about the body this session holds, and a + zero-slot frame whose body has since been replaced by one + with slots is a frame that claim is false about. *) + if nslots <> Array.length fn.Tast.slots then error (Printf.sprintf "%s on the stack has %d slots and the %s this session holds has %d: the frame is running a body that has been redefined since, so every slot index here would be a guess" @@ -807,6 +806,12 @@ let locals t ~frame = (Printf.sprintf "%s on the stack was compiled from a different body than the %s this session holds: this frame's body was redefined since it was entered, so its names no longer describe its values" name name) + else if nslots = 0 then + ok + [ ":frame " ^ Wire.quote name; ":locals ()"; ":refused ()"; + ":note " + ^ Wire.quote + "that frame records no slots; every slot in it is one the compiler made up" ] else match bound_slots t ~frame with | Error m -> error ("the program refused to say which slots are bound: " ^ m)