diff --git a/crates/benilla-ui/src/script/verb_prof.rs b/crates/benilla-ui/src/script/verb_prof.rs index 8a61bee8..afb5d77b 100644 --- a/crates/benilla-ui/src/script/verb_prof.rs +++ b/crates/benilla-ui/src/script/verb_prof.rs @@ -8,8 +8,9 @@ //! (`CreateFrame`, `SetPoint`, `SetTexture`, `Show`), and nothing charged it to them. //! //! **How it attaches.** Armed, [`install`] walks `_G` and every widget method table at the end of -//! [`UiScript::new`](super::UiScript::new) — before any FrameXML or addon runs, so everything it -//! finds is ours — and replaces each function with a closure that times the call. A verb that +//! [`UiScript::new`](super::UiScript::new) — before any FrameXML or addon runs, so every function +//! it finds is one of ours or one of Lua's own, never an addon's — and replaces each with a closure +//! that times the call. Lua's own are in there too, which is why [`STACK_RELATIVE`] exists. A verb that //! calls back into Lua (a handler fired from inside `Show`) has that nested time inside its own, //! as the handler profiler's *total* column does; the two instruments read against each other. //! @@ -234,6 +235,23 @@ pub fn reset() { *ROWS.lock().unwrap_or_else(|p| p.into_inner()) = None; } +/// Functions [`wrap_table`] must leave alone: each reads the **call stack by level**, and a +/// wrapper puts its own frame between the caller and the callee, so the level means something +/// else. +/// +/// `setfenv` is the one that bit, in grom's 2026-10-19 session. pfUI's `api/api.lua:4` is +/// `setfenv(1, pfUI:GetEnvironment())`; level 1 from inside a wrapper is the wrapper, which is a C +/// function, so Lua raises ``setfenv' cannot change environment of given object`. That one armed +/// run logged **91** of those and took pfUI, AI_VoiceOver, WhatsTraining_Turtle, MasterTradeSkills +/// and others down with it — the instrument unloaded a third of the interface it was there to +/// measure, and the session's numbers were of a client without them. +/// +/// `error(msg, level)` positions the message by level, and `debugstack([start])` is the reference's +/// traceback by level — `FuBarPlugin-2.0.lua:752` asks for `debugstack(6, 1, 0)` and +/// `AceDB-2.0.lua:742` reads a specific line of it. `pcall`/`xpcall` stay wrapped: they take no +/// level, and one armed session shows them working and worth seeing (pfUI's feature probes). +const STACK_RELATIVE: &[&str] = &["setfenv", "getfenv", "error", "debugstack"]; + /// Wrap one table's function values in place, naming each ``. fn wrap_table(lua: &Lua, table: &Table, prefix: &str) -> mlua::Result { let mut wrapped = 0; @@ -247,6 +265,9 @@ fn wrap_table(lua: &Lua, table: &Table, prefix: &str) -> mlua::Result { }) .collect(); for key in keys { + if STACK_RELATIVE.contains(&key.as_str()) { + continue; + } let Ok(Value::Function(orig)) = table.raw_get::(key.as_str()) else { continue; }; @@ -318,4 +339,28 @@ mod tests { rows[0].micros ); } + + /// The 2026-10-19 regression, as a falsifier. Wrapping `_G` wholesale took `setfenv` with it, + /// and `setfenv(1, env)` from a chunk then raised ``setfenv' cannot change environment of + /// given object` — level 1 was the wrapper, a C function. pfUI's `api/api.lua:4` is exactly + /// this line, which is why one armed session cost 91 load errors. + #[test] + fn wrapping_the_globals_spares_the_stack_relative_ones() { + let lua = mlua::Lua::new(); + super::wrap_table(&lua, &lua.globals(), "").expect("wrap _G"); + // The globals are bound to locals first: after `setfenv` the chunk's own lookups go + // through the new environment, where they are not. + let answered: bool = lua + .load( + r#" + local setfenv, getfenv = setfenv, getfenv + local env = { ok = true } + setfenv(1, env) + return ok and getfenv(1) == env + "#, + ) + .eval() + .expect("setfenv(1, …) must still reach the calling chunk"); + assert!(answered, "the chunk got someone else's environment"); + } }