From d77d54136097d2d61b4a61a7b3c7f2c70e28fd05 Mon Sep 17 00:00:00 2001 From: Joseph Ferano Date: Fri, 25 Sep 2026 21:27:24 +0700 Subject: [PATCH] A value that traps or faults while the inspector reads it shows as # with no break pushed, a restart already accepted is taken before queued reads run, and a fault in the inspector's read names the read. --- docs/BUILT.md | 6 +- lib/dev.ml | 6 +- runtime/flan_dev.c | 16 +++- test/agent_hooks.c | 56 +++++++++++++ test/programs/agent-hooks.flan | 4 + test/programs/dev-dyn-trap.flan | 4 +- test/test_agent.ml | 12 +++ test/test_dev.ml | 140 ++++++++++++++++++++------------ vendor/agent/flan_agent.c | 59 +++++++++++++- 9 files changed, 239 insertions(+), 64 deletions(-) diff --git a/docs/BUILT.md b/docs/BUILT.md index bb4ee176..085fd508 100644 --- a/docs/BUILT.md +++ b/docs/BUILT.md @@ -2628,7 +2628,11 @@ they say a thunk renders it, `Inspect` now does. **What the program is asked** is six agent verbs, all refused unless stopped: `slot F I`, `cond-at` and `global SYM` for where a root is; `peek ADDR LEN` for bytes; `ptr ADDR` for the registry's live/dead/unknown and the epitaph (`flan_dev_reg_epitaph`, which `flan_dev_reg_emit` now writes through, so the sentence has one author); and `dyn WORD`, -because a dyn value's tag is the runtime's to read and `flan_dyn_emit_to` renders it. They go through `Dev.request`, +because a dyn value's tag is the runtime's to read. `dyn` queues a job the break loop runs on the program's thread, since the +dyn printer follows pointers: a trap or fault there is the reader's, so `break_loop_at` writes the value as +`#` and unwinds through the evaluation escape without pushing a break, and the fault report says the +inspector's read touched the address. A restart already accepted is taken before the ring is drained, and a job +that needs the stop is refused after one is. They go through `Dev.request`, so the two-process daemon answers them over the socket and the merged one by a call. `peek` reads through `process_vm_readv` on its own process, so an unmapped address is a refusal and not a fault that takes the program — and in one process, the daemon — down with it. diff --git a/lib/dev.ml b/lib/dev.ml index e90f67f6..0ee5f343 100644 --- a/lib/dev.ml +++ b/lib/dev.ml @@ -2475,9 +2475,9 @@ let agent_mem t : Inspect.mem = | Ok l when starts l "dead" -> Inspect.Dead (after l "dead") | _ -> Inspect.Unknown); (* Rendered by the program, on its own thread, as an evaluation is: the - dyn printer follows pointers and can trap, and a trap there is a nested - break rather than a daemon that never answers again. See the agent's - [dyn] verb. *) + dyn printer follows pointers and can trap or fault, and there that is + caught and the value reads [#], rather than a daemon + that never answers again. See the agent's [dyn] verb. *) dyn = (fun w -> let mark = job_mark ~at_stop:1 t in diff --git a/runtime/flan_dev.c b/runtime/flan_dev.c index 54f73c45..45512a4e 100644 --- a/runtime/flan_dev.c +++ b/runtime/flan_dev.c @@ -2545,6 +2545,11 @@ static void crash_hex(uintptr_t x) { crash_puts(b + i, sizeof b - i); } +/* Non-NULL while the agent renders a value for the inspector, naming it. A + * fault then is the reader's, and the report says so instead of blaming the + * frame that happens to be on top — the program was stopped, not running. */ +const char *volatile flan_dev_crash_reading; + static void crash_handler(int sig, siginfo_t *si, void *uc) { /* Not the program's thread: this is the daemon's own fault to deal with, * and OCaml's handler is the one that knows how. See the header. */ @@ -2555,12 +2560,19 @@ static void crash_handler(int sig, siginfo_t *si, void *uc) { if (flan_crash_entered++) goto die; crash_puts("\nflan: ", 7); if (sig == SIGBUS) crash_puts("SIGBUS", 6); else crash_puts("SIGSEGV", 7); - { + if (flan_dev_crash_reading != NULL) { + static const char reading[] = " \xe2\x80\x94 reading "; + static const char tail[] = " for the inspector touched "; + crash_puts(reading, sizeof reading - 1); + crash_puts(flan_dev_crash_reading, strlen(flan_dev_crash_reading)); + crash_puts(tail, sizeof tail - 1); + } else { static const char touched[] = " \xe2\x80\x94 the program touched "; crash_puts(touched, sizeof touched - 1); } crash_hex((uintptr_t)si->si_addr); - if (flan_frame_head != NULL && flan_frame_head->info != NULL) { + if (flan_dev_crash_reading == NULL + && flan_frame_head != NULL && flan_frame_head->info != NULL) { const flan_fninfo *fi = flan_frame_head->info; crash_puts(" in ", 4); crash_puts(fi->name, (size_t)fi->namelen); diff --git a/test/agent_hooks.c b/test/agent_hooks.c index 455213e6..2d1315fb 100644 --- a/test/agent_hooks.c +++ b/test/agent_hooks.c @@ -23,6 +23,8 @@ #include #include #include +#include +#include void flan_rt_init(int32_t argc, char **argv); int32_t flan_agent_start(const uint8_t *path, int64_t len); @@ -190,6 +192,59 @@ static int stale(void) { return 0; } +/* ── A restart accepted with a read queued behind it ──────────────── + * + * The inspector's [dyn] read is a job for the stopped thread, named for the + * stop. One queued before a restart is accepted, and one asked for after, + * must neither run in the break being left: the first stays in the ring and + * is dropped at the next poll, the second is refused at the door. The break + * takes the restart without draining the ring first. */ +extern uint64_t flan_dynword(void *xfer) __asm__("flan.dynword"); +int32_t flan_agent_poll(void); +static int resuming_done; +static char r_queued[128], r_take[128], r_late[128]; +static pthread_t resumer; + +/* The listener's side of it, from another thread: the break loop is asleep + * between turns when these land, as a request from the editor would be. */ +static void *resume_from_outside(void *arg) { + const char *line = arg; + struct timespec pause = { 0, 500000 }; + nanosleep(&pause, NULL); + snprintf(r_queued, sizeof r_queued, "%s", ask(line)); + snprintf(r_take, sizeof r_take, "%s", ask("restart-at 0 retry")); + snprintf(r_late, sizeof r_late, "%s", ask(line)); + return NULL; +} + +static void resuming_hook(void) { + static char line[64]; + void *x = NULL; + if (resuming_done) return; + resuming_done = 1; + snprintf(line, sizeof line, "dyn %llu", + (unsigned long long)flan_dynword(&x)); + pthread_create(&resumer, NULL, resume_from_outside, line); +} + +static long refusal_count(void) { return strtol(ask("refusals"), NULL, 10); } + +static int resuming(void) { + void *xfer = NULL; + int32_t v; + long before; + flan_agent_break_poll_hook = resuming_hook; + v = flan_deep(0, &xfer); + flan_agent_break_poll_hook = NULL; + pthread_join(resumer, NULL); + printf("queued %stake %slate %s", r_queued, r_take, r_late); + printf("returned %d\n", v); + before = refusal_count(); + flan_agent_poll(); + printf("dropped %ld\n", refusal_count() - before); + return 0; +} + int main(int argc, char **argv) { flan_rt_init(argc, argv); if (argc < 3) { @@ -204,6 +259,7 @@ int main(int argc, char **argv) { if (strcmp(argv[1], "snapmax") == 0) return snapmax(); if (strcmp(argv[1], "snapnames") == 0) return snapnames(); if (strcmp(argv[1], "stale") == 0) return stale(); + if (strcmp(argv[1], "resuming") == 0) return resuming(); fprintf(stderr, "unknown mode %s\n", argv[1]); return 2; } diff --git a/test/programs/agent-hooks.flan b/test/programs/agent-hooks.flan index 827b025e..fb8e4cf9 100644 --- a/test/programs/agent-hooks.flan +++ b/test/programs/agent-hooks.flan @@ -21,3 +21,7 @@ (restart-case (if (= n 0) (do (error (Deep {.n n})) 0) (wide (- n 1))) (retry-with-a-name-long-enough-that-twenty-of-them-fill-the-four-kilobytes-a-break-loop-keeps-for-the-names-of-its-restarts-and-the-twenty-first-does-not-fit-anywhere-in-the-buffer-at-all-xxx-and-so-on [] n))) + +;;; A dyn keyword, so the dyn runtime is linked and the agent's [dyn] verb has +;;; something to render. agent_hooks.c's "resuming" mode reads one. +(defn dynword [] dyn :k) diff --git a/test/programs/dev-dyn-trap.flan b/test/programs/dev-dyn-trap.flan index 383b78b6..3738ff21 100644 --- a/test/programs/dev-dyn-trap.flan +++ b/test/programs/dev-dyn-trap.flan @@ -2,8 +2,8 @@ ;;;; whose arena was freed, which traps DynRange when printed; the first ;;;; element of [s] is a word that reads as a boxed pointer to address 0x10, ;;;; which faults when printed as a dyn. The inspector hands each to the -;;;; program's own thread to render, so both become a nested break and the -;;;; daemon keeps answering. +;;;; program's own thread to render, which catches the trap: each reads as +;;;; #, no break is pushed, and the daemon keeps answering. (import agent "vendor:agent") (defstruct Boom [why i32]) diff --git a/test/test_agent.ml b/test/test_agent.ml index 93ba88e0..0c2e1a41 100644 --- a/test/test_agent.ml +++ b/test/test_agent.ml @@ -956,6 +956,18 @@ let () = if code <> 0 || out <> want then fail "a choice addressed to an outer break, met by a nested one\n got: %S (exit %d, err %S)\n wanted: %S" out code err want; + (* A restart accepted with an inspector read queued behind it, and a + second asked for after. Neither runs in the break being left: the + break takes the restart without draining the ring, the queued job is + dropped at the next poll, and the late one is refused at the door. *) + let code, out, err = hook_mode "resuming" in + let want = + "queued ok\ntake ok\nlate err the program is resuming: a restart was taken\n\ + returned 0\ndropped 1\n" + in + if code <> 0 || out <> want then + fail "a restart accepted with a read queued behind it\n got: %S (exit %d, err %S)\n wanted: %S" + out code err want; (try Sys.remove hexe with Sys_error _ -> ()); Test_support.report ~label:"agent" () diff --git a/test/test_dev.ml b/test/test_dev.ml index 32d91b05..4d1e3a81 100644 --- a/test/test_dev.ml +++ b/test/test_dev.ml @@ -6759,59 +6759,95 @@ let () = (* The reader hands a dyn word to the program's own thread to render, because the dyn printer follows pointers: a view of a freed arena - traps, and a scribbled word faults. Rendered on the agent's thread - either one hung the daemon for good or killed it. Here each must come - back as a refusal, and the daemon must go on answering. *) - let dsock = tmp "dyntrap.sock" and dout = tmp "dyntrap.out" in - (try Sys.remove dsock with Sys_error _ -> ()); - let dfd = Unix.openfile dout [ Unix.O_WRONLY; Unix.O_CREAT; Unix.O_TRUNC ] 0o600 in - let dpid = - Unix.create_process flan - [| flan; "dev"; "programs/dev-dyn-trap.flan"; "-s"; dsock; "--llvm" |] - Unix.stdin dfd Unix.stderr - in - Unix.close dfd; - if not (listening ~pid:dpid dsock) then begin - fail "the dyn-trap daemon %s" !listen_why; - (try Unix.kill dpid Sys.sigkill with Unix.Unix_error _ -> ()) - end - else begin - let c = connect dsock in - let stopped r = - match Wire.field r "stopped" with - | Some { Form.v = Form.Sym "t"; _ } -> true - | _ -> false - in - let answers what = - let r = request c "(:op \"eval-expr\" :code \"n\")" in - if Wire.string_field r "value" <> Some "5" then - fail "after %s the daemon no longer answers: %s" what - (Option.value ~default:(status r) (Wire.string_field r "message")) - in - if not (await (fun () -> stopped (request c "(:op \"describe\")"))) then - fail "the dyn-trap program never stopped" - else begin - let r = request c "(:op \"inspect\" :frame 0 :slot 0 :path (0))" in - (match Wire.field r "addr" with - | Some { Form.v = Form.Int a; _ } -> - let r = - request c (Printf.sprintf "(:op \"at\" :addr %Ld :type \"dyn\")" a) + traps, and a scribbled word faults. Either is the reader's trouble and + not the program's, so it pushes no break: the value shows as the trap's + name, the rest of the section renders, and the stop on top is still + the program's own after any number of refreshes. A fault says it was + the inspector's read that touched the address, not the stopped frame. + Both backends. *) + List.iter + (fun backend -> + let dsock = tmp ("dyntrap" ^ backend ^ ".sock") + and dout = tmp ("dyntrap" ^ backend ^ ".out") in + (try Sys.remove dsock with Sys_error _ -> ()); + let dfd = + Unix.openfile dout [ Unix.O_WRONLY; Unix.O_CREAT; Unix.O_TRUNC ] 0o600 + in + let dpid = + Unix.create_process flan + [| flan; "dev"; "programs/dev-dyn-trap.flan"; "-s"; dsock; backend |] + Unix.stdin dfd dfd + in + Unix.close dfd; + if not (listening ~pid:dpid dsock) then begin + fail "the %s dyn-trap daemon %s" backend !listen_why; + (try Unix.kill dpid Sys.sigkill with Unix.Unix_error _ -> ()) + end + else begin + let c = connect dsock in + let stopped r = + match Wire.field r "stopped" with + | Some { Form.v = Form.Sym "t"; _ } -> true + | _ -> false in - if status r <> "error" then - fail "a dyn word pointing at 0x10 rendered: %s" - (Option.value ~default:"" (Wire.string_field r "value")); - answers "a dyn word that faults" - | _ -> fail "inspecting the slice element gave no :addr"); - let r = request c "(:op \"globals\")" in - if status r <> "error" then - fail "a dyn view of a freed arena rendered in the globals section"; - answers "a dyn value that traps" - end; - ignore (request c "(:op \"close\")"); - (try Unix.close c with Unix.Unix_error _ -> ()); - (try ignore (Unix.waitpid [] dpid) with Unix.Unix_error _ -> ()) - end; - List.iter (fun f -> try Sys.remove f with Sys_error _ -> ()) [ dsock; dout ]; + let still_boom what = + let r = request c "(:op \"describe\")" in + if Wire.string_field r "condition" <> Some "Boom" then + fail "%s: after %s the program is stopped on %s, not its own Boom" + backend what + (Option.value ~default:"nothing" (Wire.string_field r "condition")) + in + if not (await (fun () -> stopped (request c "(:op \"describe\")"))) then + fail "the %s dyn-trap program never stopped" backend + else begin + let r = request c "(:op \"inspect\" :frame 0 :slot 0 :path (0))" in + (match Wire.field r "addr" with + | Some { Form.v = Form.Int a; _ } -> + let r = + request c + (Printf.sprintf "(:op \"at\" :addr %Ld :type \"dyn\")" a) + in + if Wire.string_field r "value" <> Some ">" + then + fail "%s: a dyn word pointing at 0x10 read as %s" backend + (Option.value ~default:(status r) (Wire.string_field r "value")); + still_boom "a dyn word that faults" + | _ -> fail "%s: inspecting the slice element gave no :addr" backend); + for i = 1 to 10 do + let r = request c "(:op \"globals\")" in + let rows = + match Wire.field r "globals" with + | Some { Form.v = Form.List l; _ } -> + List.filter_map + (fun (e : Form.t) -> + match e.Form.v with + | Form.List ({ Form.v = Form.Str n; _ } :: _ + :: { Form.v = Form.Str v; _ } :: _) -> + Some (n, v) + | _ -> None) + l + | _ -> [] + in + if List.assoc_opt "dv" rows <> Some "#" + || List.assoc_opt "n" rows <> Some "5" + then + fail "%s: globals refresh %d with a trapping dyn: %s" backend i + (String.concat ", " (List.map (fun (n, v) -> n ^ "=" ^ v) rows)) + done; + still_boom "ten refreshes of a section holding a trapping dyn" + end; + ignore (request c "(:op \"close\")"); + (try Unix.close c with Unix.Unix_error _ -> ()); + (try ignore (Unix.waitpid [] dpid) with Unix.Unix_error _ -> ()); + let said = In_channel.with_open_bin dout In_channel.input_all in + if not (contains_sub said "reading a dyn value for the inspector touched") + || contains_sub said "the program touched" + then + fail "%s: the fault in the inspector's read was reported as: %s" + backend said + end; + List.iter (fun f -> try Sys.remove f with Sys_error _ -> ()) [ dsock; dout ]) + [ "--llvm"; "--x86" ]; (* ── What a half-finished assignment looks like from the break ────── *) diff --git a/vendor/agent/flan_agent.c b/vendor/agent/flan_agent.c index 7fbc1d5f..eb492230 100644 --- a/vendor/agent/flan_agent.c +++ b/vendor/agent/flan_agent.c @@ -465,6 +465,13 @@ static const uint8_t abandon_report[] = * frames jumped over do not run. NULL when no evaluation is in progress; * saved and restored around the call like [eval_boundary]. */ static sigjmp_buf *eval_escape; +/* Set while [dyn_job] renders a value for the inspector: a trap in there is + * the reader's, not the program's. See [break_loop_at]. Game thread only. */ +static int inspector_reading; +/* flan_dev.c's: what the fault report names when the fault is a read the + * inspector asked for, rather than the program's own. */ +extern const char *volatile flan_dev_crash_reading; +void flan_dev_emit(const uint8_t *bytes, int64_t len); /* Whether the game thread is inside an evaluated thunk's call, at any depth, * rather than in the program's own code. A break records it, and it is what @@ -1078,6 +1085,27 @@ void (*flan_agent_break_poll_hook)(void); static void break_loop_at(const uint8_t *name, int64_t namelen, void *condition, void *xfer, int resumable) { struct timespec step = { 0, 2000000 }; /* 2ms */ + /* A trap or fault inside the inspector's own read of a value — the dyn + * printer following a pointer that is no longer good. It is not the + * program's error and no break is pushed for it: the value is written as + * the trap's name, the evaluation escape unwinds the job exactly as an + * abandon would, and the stop the reader was looking at stays the one on + * top. A break here would leave the program one level deeper on every + * refresh of a section that holds the value. */ + if (inspector_reading && eval_escape != NULL) { + inspector_reading = 0; + flan_dev_crash_reading = NULL; + fprintf(stderr, "flan: reading a value for the inspector stopped on " + "%.*s; it is shown as #\n", + (int)namelen, (const char *)name, (int)namelen, (const char *)name); + fflush(stderr); + flan_dev_result_begin(); + flan_dev_emit((const uint8_t *)"#", 1); + flan_dev_result_end(); + siglongjmp(*eval_escape, 1); + } fflush(stdout); fprintf(stderr, "\nflan: unhandled %.*s — stopped, not dead.\n", (int)namelen, (const char *)name); @@ -1164,9 +1192,16 @@ static void break_loop_at(const uint8_t *name, int64_t namelen, void *condition, fflush(stderr); die_now(); } + /* The ring is not drained while a choice for this break is waiting. A job + * queued after a restart was accepted belongs to the stop being left; run + * first, it would run in that stop and could push a break of its own that + * takes the restart meant for this one. Left in the ring, it meets the gate + * in [flan_agent_poll] at the next boundary and is dropped. */ for (;;) { - flan_agent_poll(); - if (flan_agent_break_poll_hook != NULL) flan_agent_break_poll_hook(); + if (!(atomic_load(&chosen_ready) && atomic_load(&chosen_gen) == my_gen)) { + flan_agent_poll(); + if (flan_agent_break_poll_hook != NULL) flan_agent_break_poll_hook(); + } if (atomic_load(&aborting)) { fflush(stdout); fprintf(stderr, "flan: aborted at the break loop\n"); @@ -1529,7 +1564,11 @@ static pthread_mutex_t request_lock = PTHREAD_MUTEX_INITIALIZER; static uint64_t dyn_word; static void dyn_job(void) { flan_dev_result_begin(); + inspector_reading = 1; + flan_dev_crash_reading = "a dyn value"; flan_dyn_emit_dev(dyn_word); + inspector_reading = 0; + flan_dev_crash_reading = NULL; flan_dev_result_end(); } @@ -2111,8 +2150,9 @@ static void handle_line(char *line, sink *o) { * trap — a view of a Vec whose arena was freed — or fault on a scribbled * word, and on this thread either one is a break loop that holds * [request_lock] for ever, or the daemon's own death. On the game thread it - * runs as an evaluation does, inside [flan_agent_poll]'s escape, so it - * becomes a nested break the break buffer shows. */ + * runs as an evaluation does, inside [flan_agent_poll]'s escape, and a trap + * there is caught by [break_loop_at] and written as the value's text + * without pushing a break. */ if (strncmp(line, "dyn ", 4) == 0) { char *end = NULL; unsigned long long w = strtoull(line + 4, &end, 0); @@ -2124,6 +2164,10 @@ static void handle_line(char *line, sink *o) { return; } if (s == NULL) { reply(o, "err no snapshot\n"); return; } + if (atomic_load(&chosen_ready)) { + reply(o, "err the program is resuming: a restart was taken\n"); + return; + } if (!queue_room()) { reply(o, "err the install queue is full\n"); return; } dyn_word = (uint64_t)w; j.call = dyn_job; @@ -2355,6 +2399,13 @@ static void handle_line(char *line, sink *o) { * one there is no point relocating, and refusing here means no handle is * taken for it at all. Only one producer runs at a time, so room seen now is * room still there at [publish] below. */ + /* A job that needs the stop, arriving after a restart was accepted and + * before the break loop has taken it: the stop it was built for is being + * left, so it is refused here rather than run in a break that is ending. */ + if ((stopped_only || at_stop != 0) && atomic_load(&chosen_ready)) { + reply(o, "err the program is resuming: a restart was taken\n"); + return; + } if (!queue_room()) { reply(o, "err reload queue full; the program is not calling " "agent/poll\n");