diff --git a/NEXT.md b/NEXT.md index b73917f..5d47830 100644 --- a/NEXT.md +++ b/NEXT.md @@ -49,15 +49,37 @@ Four agents are working these in parallel worktrees. Listed so a session reading 3. **The first ten raylib core examples**, plus the window and input bindings they need. The deliverable is the *language gap list* as much as the ported files — sand.flan is the only real raylib program today, so this is the first time the language is pushed by code it was not designed around. -4. **Names in DWARF.** The two items under "One line away" below, promoted here because the inspector and the - conditions buffer are only as good as what lldb can say: let-bound locals print as `s0`, `s2` because `Tast` refers - to them by slot index and `Check` drops the names, and `Session.eval` never passes the `~debug` that - `Emit.redefinition` already takes and is already tested for. The second also unblocks source interleaving in the - disassembly buffer. +4. **Names in DWARF.** *Done.* `Tast.fn` carries `snames` beside `slots`, so a let-bound local is its own name under + lldb instead of `s0`; a slot the compiler invented keeps `s`, because a synthesized slot has no source name + and inventing one puts a variable in the debugger that is not in the file. Shadowing had to be decided rather than + assumed: every `!DILocalVariable` is scoped to the subprogram — the typed IR has no block structure to build a + `!DILexicalBlock` from — so two slots called `v` left lldb answering `p v` with the outer one while the body computed + with the inner, and not listing the inner at all. A repeat now gets a `~2` suffix, which is unambiguous because `~` + cannot occur in a source symbol. That is a way of not lying, not a way of being right; the real fix is a lexical + block per `Let` and the `llvm.dbg.declare`s moved out of the entry block, and it is the one thing left here. -**DWARF is not missing, and `flan-cnr.el`'s stack pane reads as though it were.** `flan build --debug` emits it and -`flan-dape.el` drives lldb with it. What the break loop lacks is DWARF *in a dev build* — a `flan dev` process is not a -`--debug` one. Worth rewording that refusal so it names the real gap. + And `flan dev --debug` now builds the host *and* every redefinition module with DWARF. One flag, because it is one + decision — measured, not reasoned: a line breakpoint needs a line table on the host to fire before the first + `C-c C-c` and one in each module to still be firing after. Off by default, because a debug build is an `-O0` build + and silently making every reloaded body `-O0` changes the frame time of the function being iterated on. + +**What a dlopen'd redefinition module does to a breakpoint, measured.** lldb picks the new module's DWARF up on the +`dlopen` and says so — "1 location added to breakpoint 3". A breakpoint set by *name* gains a second location either +way, so dlopen was never the difficulty; what the module's line table buys is that it stops with **source** rather than +disassembly, and that a *file and line* breakpoint on the new body resolves at all — it sits at `locations = 0 +(pending)` forever without one. A file-and-line breakpoint on the **host's** copy stays pinned at `locations = 1`, which +is correct rather than stale: the old body is still mapped and every call site that has not gone through its cell again +still reaches it. The stack crosses the boundary intact — a frame in the reloaded `.so` and the frame below it in the +host each name their own `.flan` file. The transcripts are in `emacs/flan-dape.el`, under "Reloading and breakpoints". + +**Source interleaving in the disassembly buffer is unblocked, and not done.** `Dev.asm_of` runs `objdump -d`; with a +`--debug` module `objdump -dS` interleaves the Flan source correctly (verified). What it needs is the `-S` and a +`parse_listing` that tolerates source lines among the instructions. + +**`flan-cnr.el`'s stack pane was refusing for the wrong reason** and now names the real one. DWARF was never the gap: +what is missing is anything *attached* to the stopped program. That buffer reaches it over the daemon's socket, and a +socket cannot read another process's frames — the break loop stopped itself, it is not being debugged. It wants either +an unwinder in the agent or lldb on the same pid. ### Managed classes are planned. Do not start them. @@ -565,10 +587,6 @@ Sixty mutations, nineteen left the whole suite green. The severe cluster is clos - **`match` over enums.** Fully desugarable, wanted, and blocked only by `Ast.pattern` needing a keyword case, which `load.ml` matches exhaustively. -- **[in flight]** **DWARF for a redefinition module.** `Emit.redefinition` takes `~debug` and is tested; `Session.eval` does not pass - it. That also unblocks source interleaving in the disassembly buffer. -- **[in flight]** **Let-bound locals print as `s0`, `s2`** under lldb. Parameters get their real names; `Tast` refers to the rest by - slot index and `Check` drops the names. - **`Build.executable` returns only `out`**, so the daemon recovers the host `.ll` by recomputing `Build.workdir ()`. ### Deferred with a reason diff --git a/bin/main.ml b/bin/main.ml index a77137e..22d1794 100644 --- a/bin/main.ml +++ b/bin/main.ml @@ -202,13 +202,22 @@ let () = against — and it owns the build, which is what makes its layout rules describe the process that is actually running. *) | _ :: "dev" :: path :: rest -> + (* --debug builds the host *and* every module this daemon sends with DWARF, + which is one flag because it is one decision: a line breakpoint in a + .flan buffer needs a line table on the host to fire at all, and one in + each redefinition module to still be firing after C-c C-c. It implies + -O0 on both, so it is asked for rather than assumed. *) + let debug = List.mem debug_flag rest in + let rest = List.filter (fun a -> not (is_flag a)) rest in let sock = match rest with | [ "-s"; s ] -> s | [] -> Filename.concat (Filename.dirname path) ".flan-dev.sock" - | _ -> prerr_endline "usage: flan dev [-s socket]"; exit 2 + | _ -> + prerr_endline "usage: flan dev [-s socket] [--debug]"; + exit 2 in - with_errors path (fun () -> Flan.Dev.start ~file:path ~sock) + with_errors path (fun () -> Flan.Dev.start ~debug ~file:path ~sock ()) (* One redefinition, built the way an editor will ask for it: a session over the program the process was built from, and a file of the forms that @@ -216,19 +225,22 @@ let () = is one a running process can be told at all — neither of which a command given only a list of function names could. *) | _ :: "reload" :: prog :: forms :: rest -> + let debug = List.mem debug_flag rest in + let rest = List.filter (fun a -> not (is_flag a)) rest in let out = match rest with | [ "-o"; o ] -> o | [] -> Filename.remove_extension (Filename.basename forms) ^ ".so" | _ -> - prerr_endline "usage: flan reload [-o out.so]"; + prerr_endline + "usage: flan reload [-o out.so] [--debug]"; exit 2 in with_errors forms (fun () -> - let t, _ = Flan.Session.create ~file:prog in + let t, _ = Flan.Session.create ~debug ~file:prog () in let src = In_channel.with_open_bin forms In_channel.input_all in let c = Flan.Session.eval ~origin:forms t src in - let opts = { Flan.Build.default with dev = true } in + let opts = { Flan.Build.default with dev = true; debug } in let timing = Flan.Build.shared ~opts ~ir:c.Flan.Session.ir ~out () in Printf.eprintf "%s %s llc %.1fms ld %.1fms\n" out (String.concat " " c.Flan.Session.fns) timing.Flan.Build.llc_ms diff --git a/emacs/flan-cnr.el b/emacs/flan-cnr.el index 525aaa8..9e0d8c6 100644 --- a/emacs/flan-cnr.el +++ b/emacs/flan-cnr.el @@ -78,9 +78,9 @@ from fixtures, and so `flan-dev.el' is named in one place.") (params . "restart arguments are checked at run time against a frame that does not record its arity [needs a field in the restart frame]") (stack - . "a Flan build carries no frame metadata, so there is nothing to walk the stopped stack with [blocked: needs DWARF]") + . "nothing here is attached to the stopped program. DWARF is not the gap and has not been for a while: `flan build --debug' emits it, `flan dev --debug' now builds the host and every redefinition module with it, and lldb walks a stack that crosses from a reloaded .so back into the host naming both sides' .flan files. But this buffer reaches the program over the daemon's socket, and a socket cannot read another process's frames — the break loop stopped itself, it is not being debugged. So this wants either an unwinder in the agent, beside `flan_rt.c', or lldb attached to the same pid and this buffer reading it [needs one of those two, not DWARF]") (locals - . "reading a stopped frame's locals needs both the frame layout and a renderer aimed at an address rather than at an expression [blocked: needs DWARF, and the same thunk the condition's fields need]")) + . "same reason as the stack above it, plus a renderer aimed at an address rather than at an expression. The names and types are in the DWARF now — a let-bound local is its own name there, not `s0' — so whatever walks the frames can read them; nothing is walking the frames [needs the same attachment, and the same thunk the condition's fields need]")) "Why a section of this buffer is empty, by name.") (defun flan-cnr--why (key) diff --git a/emacs/flan-dape.el b/emacs/flan-dape.el index 84def30..9ab2b31 100644 --- a/emacs/flan-dape.el +++ b/emacs/flan-dape.el @@ -27,8 +27,8 @@ ;; - a --dev build and a --debug build are different builds, and M-x ;; flan-debug does not attach to the program `flan dev' is running; ;; - across a redefinition a breakpoint set by NAME gains a second location -;; and both stay live, while one set by FILE AND LINE stops firing — -;; because the redefinition module carries no DWARF yet. +;; and both stay live; one set by FILE AND LINE follows the reload if the +;; module was built with `flan dev --debug', and does not otherwise. ;;; Code: @@ -207,23 +207,53 @@ common case is one command rather than a config prompt." ;; Both fire, and both are correct — 1.1 is not stale, it is the body the old ;; call sites still run. That is `dape-breakpoint-global', which sets by name. ;; -;; A breakpoint set by FILE AND LINE does not follow, and the reason is not -;; that dape pinned it to an address. It stays at locations = 1 because the -;; redefinition module carries no line table for it to resolve against. Given -;; one it does follow: a pending breakpoint on a file the executable had never -;; heard of went from "no locations (pending)" to "1 location added" the moment -;; a .so with DWARF for that file was dlopened, and stopped with full source. -;; So the gap is exactly one missing thing, and nothing about dlopen. +;; A breakpoint set by FILE AND LINE depends on how the module was built, and +;; the difference is a line table and nothing about dlopen. Both halves were +;; measured against the same host, one redefinition module built each way. ;; -;; That missing thing, by name: `Emit.redefinition' takes a ~debug argument and -;; `Session.eval' does not pass it, so `flan reload' and the `flan dev' daemon -;; build modules without DWARF. Until they do, a reloaded body breaks by name -;; and shows disassembly instead of source, and a line breakpoint in the .flan -;; buffer silently stops firing after the first C-c C-c. lib/session.ml is the -;; dev loop's file and is not this one's to change. +;; Without DWARF in the module the line breakpoint stays where it was: ;; -;; So, for now: debug with `dape-breakpoint-global' if you are also reloading, -;; and use line breakpoints for a program you are only running. +;; 2: file = 'v2local.flan', line = 25, locations = 0 (pending) +;; +;; and the name breakpoint still gains its second location — so dlopen was +;; never the problem — but stops into disassembly, because there is no source +;; to show: +;; +;; frame #0: 0x7ffff7fba190 nodbg-v2.so`flan.bump +;; -> 0x7ffff7fba190 <+0>: pushq %rbx +;; +;; With DWARF in the module, the same breakpoint resolves on the dlopen — lldb +;; prints "1 location added to breakpoint 3" as the module loads — and stops +;; with source and named locals: +;; +;; 3: file = 'v2local.flan', line = 25, locations = 1, resolved = 1 +;; 3.1: where = v2.so`flan.bump + 78 at v2local.flan:25:21, resolved +;; +;; (lldb) frame variable +;; (long) step = 10 +;; (long) prior = 11 +;; +;; The stack crosses the boundary intact, which is the part worth knowing: a +;; frame in the reloaded .so and the frame below it in the host each name their +;; own .flan file, and the C host below both. +;; +;; frame #0: v2.so`flan.bump at v2local.flan:25:21 +;; frame #2: host`flan.outer at reload.flan:38:20 +;; frame #3: host`main at reload_host.c:94:44 +;; +;; A breakpoint set by FILE AND LINE on the *host's* copy stays at locations = 1 +;; and does not move. That is correct rather than stale: the old body is still +;; mapped and every call site that has not gone through the cell again still +;; reaches it, so pinning there is the only honest thing to do. +;; +;; How to get it: `flan dev --debug'. It is one flag on purpose — the host +;; needs a line table for a breakpoint to fire before the first C-c C-c, and +;; each module needs one for it to still be firing after — and it is off by +;; default because a debug build is an -O0 build, which is not what you want +;; under a frame budget unless you asked for it. +;; +;; So: with `flan dev --debug', line breakpoints work across a reload. Without +;; it, debug with `dape-breakpoint-global', which sets by name. (provide 'flan-dape) ;;; flan-dape.el ends here diff --git a/lib/dev.ml b/lib/dev.ml index 0cb1dfc..1de7937 100644 --- a/lib/dev.ml +++ b/lib/dev.ml @@ -321,7 +321,9 @@ let eval t ~code ~origin = nothing else on this machine still has that text. *) let ll = Filename.concat t.dir (Printf.sprintf "m%d.ll" t.n) in write_file ll c.Session.ir; - (match Build.shared ~opts:{ Build.default with Build.dev = true } + (match Build.shared + ~opts:{ Build.default with Build.dev = true; + Build.debug = t.session.Session.debug } ~ir:c.Session.ir ~out () with | timing -> (match deliver t out with @@ -357,7 +359,9 @@ let eval_expr t ~code ~origin = let before = match result t with Some (g, _) -> g | None -> 0L in t.n <- t.n + 1; let out = Filename.concat t.dir (Printf.sprintf "e%d.so" t.n) in - (match Build.shared ~opts:{ Build.default with Build.dev = true } + (match Build.shared + ~opts:{ Build.default with Build.dev = true; + Build.debug = t.session.Session.debug } ~ir:c.Session.ir ~out () with | _ -> (match deliver t out with @@ -874,7 +878,13 @@ let serve t fd = in go () -let start ~file ~sock = +(* [debug] is off by default, which keeps [flan dev] exactly what it was: a + -O2 host and -O2 modules. It is opt-in rather than always-on because a debug + build is an -O0 build — [llvm.dbg.declare] describes an alloca and mem2reg + deletes it — and silently making every reloaded body -O0 would change the + frame time of the one function you are iterating on, in the loop whose whole + point is watching that number. *) +let start ?(debug = false) ~file ~sock () = let t0 = Unix.gettimeofday () in (* Absolute, because every location this daemon ever reports is derived from it and an editor is not in this process's working directory. [flan dev @@ -882,7 +892,7 @@ let start ~file ~sock = "src/game.flan:12:7", which the editor can only resolve by guessing which directory it was relative to. *) let file = try Unix.realpath file with Unix.Unix_error _ -> file in - let session, l = Session.create ~file in + let session, l = Session.create ~debug ~file () in let dir = Filename.concat (Filename.get_temp_dir_name ()) (Printf.sprintf "flan-dev-%d" (Unix.getpid ())) @@ -895,9 +905,15 @@ let start ~file ~sock = probably was. [Build.executable] leaves it in its own working directory under the module's basename; it is moved here so that nothing else in this process can reuse the name. *) + (* The host and the modules are one decision. DWARF in a redefinition is + only half a debuggable dev loop: lldb re-resolves a *name* breakpoint + against each module as it loads either way, but a breakpoint set on a line + in the .flan buffer needs a line table on both sides — the host's to fire + before the first C-c C-c, the module's to follow the reload. *) ignore (Build.executable - ~opts:{ Build.default with Build.dev = true; Build.keep = true } + ~opts:{ Build.default with Build.dev = true; Build.keep = true; + Build.debug } ~csrcs:l.Load.csrcs ~lflags:l.Load.lflags session.Session.host ~out:exe); let host_ll = Filename.concat dir "host.ll" in (try diff --git a/lib/session.ml b/lib/session.ml index 92851a7..2fa492d 100644 --- a/lib/session.ml +++ b/lib/session.ml @@ -36,6 +36,13 @@ type t = { host : Tast.program; (* what the process was built from *) pkgs : Load.pkg list; (* alias, directory, names owned *) mutable thunks : int; (* expression evaluations so far *) + (* Whether the modules this session emits carry DWARF. It belongs to the + session rather than to each call because it has to match the process the + modules are loaded into: a redefinition with debug info, dlopened into a + host built without it, gives a debugger a second module to resolve names + against and nothing to line up the host's own frames with. Both ends are + set from one flag — see [Dev.start]. *) + debug : bool; } let fail = Loc.fail @@ -59,11 +66,11 @@ let rec same_const (a : Tast.expr) (b : Tast.expr) = && List.for_all2 same_const xs ys | _ -> false -let create ~file = +let create ?(debug = false) ~file () = let l = Load.program ~file (Parse.program (Reader.read_file file)) in let p, env = Check.program_with_env l.Load.decls in ({ file; decls = l.Load.decls; program = p; env; host = p; pkgs = l.Load.pkgs; - thunks = 0 }, l) + thunks = 0; debug }, l) (* Which package a file being edited belongs to, if any. @@ -346,7 +353,10 @@ let eval ?(origin = "") t src : change = program.Tast.globals) names in - let ir = Emit.redefinition ~dev:true ~known:(known t) ~consts program ~fns in + let ir = + Emit.redefinition ~dev:true ~debug:t.debug ~known:(known t) ~consts program + ~fns + in let allocates = List.exists (fun (g : Tast.global) -> not (known t g.Tast.gname)) @@ -603,6 +613,11 @@ let eval_expr ?(origin = "") t src : change = externs = t.program.Tast.externs @ externs } in let ir = - Emit.redefinition ~dev:true ~known:(known t) ~call:name program ~fns:[ name ] + (* The thunk gets debug info on the same flag as everything else. It is a + function nobody sets a breakpoint on by name, but it is a frame on the + stack when the expression signals, and a frame the debugger cannot name + is the thing the conditions buffer is trying to stop showing. *) + Emit.redefinition ~dev:true ~debug:t.debug ~known:(known t) ~call:name + program ~fns:[ name ] in { ir; names = []; fns = []; installs = true } diff --git a/test/test_agent.ml b/test/test_agent.ml index 4f16c62..2143080 100644 --- a/test/test_agent.ml +++ b/test/test_agent.ml @@ -67,7 +67,7 @@ let () = through it rather than calling Emit directly is the point: it is what knows [tick] is a name the host has, so the module binds to its cell as a symbol instead of inventing a registry entry nobody publishes. *) - let t, l = Session.create ~file:"programs/agent.flan" in + let t, l = Session.create ~file:"programs/agent.flan" () in (* A dev build, because that is what has cells to install into and exports them. The agent's own C and its -lpthread come from the package. *) @@ -173,7 +173,7 @@ let () = and they differ, so a loop that always took the same restart fails. *) let bsock = tmp "break.sock" and bout = tmp "break.out" in (try Sys.remove bsock with Sys_error _ -> ()); - let bt, bl = Session.create ~file:"programs/break.flan" in + let bt, bl = Session.create ~file:"programs/break.flan" () in let bexe = tmp "break" in ignore (Build.executable ~opts:dev ~csrcs:bl.Load.csrcs ~lflags:bl.Load.lflags diff --git a/test/test_session.ml b/test/test_session.ml index b2b8281..c5da50b 100644 --- a/test/test_session.ml +++ b/test/test_session.ml @@ -24,7 +24,7 @@ let checked_program file = (Load.program ~file (Parse.program (Reader.read_file file))).Load.decls let refuses ?(file = "programs/reload.flan") name src reason = - let t, _ = Session.create ~file in + let t, _ = Session.create ~file () in match Session.eval t src with | _ -> fail "%s was accepted" name | exception Loc.Error (_, msg) -> @@ -70,7 +70,7 @@ let () = "changes layout"; (* An ordinary redefinition, and what the session works out about it. *) - let t, _ = Session.create ~file:"programs/reload.flan" in + let t, _ = Session.create ~file:"programs/reload.flan" () in let c = Session.eval t "(defn bump [] i64 (set counter (+ counter 5)) counter)" in if not c.Session.installs then fail "a redefined function had nothing to install"; if c.Session.fns <> [ "bump" ] then @@ -83,6 +83,40 @@ let () = if has c.Session.ir "flan_dev_cell" then fail "a name the host has went through the registry"; + (* DWARF in a redefinition module, which is a property of the session and + not of the call. [Emit.redefinition] has taken a ~debug argument all + along and was tested with it; what was missing was anyone passing it, so + every body installed by C-c C-c lost its debug info in a running process. + The defect was one unpassed argument, so the test is that the argument + arrives — asserted on the emitted text, which is the only place it shows. + + Both directions matter. A session that always emitted debug info would + force -O0 on every reloaded body ([Build.shared] does that, and must), + which would change the frame time of the one function being iterated on. + Off unless asked for is the behaviour, so off is asserted too. *) + let dt, _ = Session.create ~debug:true ~file:"programs/reload.flan" () in + let dc = + Session.eval dt + "(defn bump [] i64 (let [step (i64 5)] (set counter (+ counter step)) counter))" + in + if not (has dc.Session.ir "!DILocalVariable(name: \"step\"") then + fail "a debug session's redefinition carries no name for its local"; + if not (has dc.Session.ir "!DISubprogram(name: \"bump\"") then + fail "a debug session's redefinition carries no subprogram"; + let pt, _ = Session.create ~file:"programs/reload.flan" () in + let pc = + Session.eval pt + "(defn bump [] i64 (let [step (i64 5)] (set counter (+ counter step)) counter))" + in + if has pc.Session.ir "!DILocalVariable" then + fail "a plain session's redefinition carries debug info it was not asked for"; + + (* The same for an expression evaluation, which takes the other path out of + the session and so can lose the flag on its own. *) + let ec = Session.eval_expr dt "(+ counter 1)" in + if not (has ec.Session.ir "!DISubprogram") then + fail "a debug session's eval thunk carries no debug info"; + (* A form that does not check must leave the session exactly as it was. This is the one that decides whether a REPL survives a typo. *) (match Session.eval t "(defn bump [] i64 nonsense)" with @@ -141,7 +175,7 @@ let () = (* A file with imports, re-evaluated whole — the C-c C-k case. The session keeps the *expanded* declarations, so the package's names are replaced in place rather than appended a second time and rejected as duplicates. *) - let t, _ = Session.create ~file:"../sand.flan" in + let t, _ = Session.create ~file:"../sand.flan" () in let src = In_channel.with_open_bin "../sand.flan" In_channel.input_all in (match Session.eval t src with | c -> @@ -157,7 +191,7 @@ let () = importer and written nowhere in the file, so the path is the only thing that can decide it — which is why it is derived here and not sent by the editor. *) - let t, _ = Session.create ~file:"../sand.flan" in + let t, _ = Session.create ~file:"../sand.flan" () in (match Session.eval ~origin:"../vendor/agent/agent.flan" t "(defn poll [] i32 (poll-raw))" @@ -172,7 +206,7 @@ let () = member of a directory, so matching on the directory alone would answer "not a package" — and the failure is the silent one above: the form splices as a bare [step] and the running program keeps the one it had. *) - let t2, _ = Session.create ~file:"programs/sand-headless.flan" in + let t2, _ = Session.create ~file:"programs/sand-headless.flan" () in (match Session.eval ~origin:"../sand.flan" t2 "(defn step [] Unit (do))" with | c -> if c.Session.fns <> [ "sand/step" ] then @@ -194,7 +228,7 @@ let () = must not take a registry slot either — there are 4096 of those and an expression evaluated in a loop would exhaust them. A module that publishes a body can never say this; its whole purpose is to leave a pointer. *) - let t, _ = Session.create ~file:"programs/reload.flan" in + let t, _ = Session.create ~file:"programs/reload.flan" () in let e = Session.eval_expr t "(+ 1 2)" in if not (has e.Session.ir "@flan_reload_transient") then fail "an expression's module did not declare itself unloadable";