diff --git a/changelog.d/8238-param-guard-visit-tracking.md b/changelog.d/8238-param-guard-visit-tracking.md new file mode 100644 index 0000000000..d7e050c71d --- /dev/null +++ b/changelog.d/8238-param-guard-visit-tracking.md @@ -0,0 +1,79 @@ +### perf(codegen, runtime): the parameter validator stops recording visits it can never consult + +`js_param_type_guard` runs on every unproven call into a guarded +ordinary-parameter clone. #8201 routed the scalar descriptors to the typed-abi +leaf guards; what stayed on the interpretive validator is the structural half, +and #8202 priced its **fixed** per-call component. Two of those three costs are +removable without changing what the guard decides. + +**The visited set was unconditional.** Every container the walk touched paid a +linear scan of up to 64 inline entries and, past that, a `HashSet` insert — per +ARRAY ELEMENT. Validating `p: { toks: Token[], pos: number }` on every `peek(p)` +therefore recorded one entry per token, and not one of them could ever be +consulted: `Token` lies on no descriptor cycle and is reachable by exactly one +path, so a second arrival at the same `(address, node)` pair is impossible. + +The set is load-bearing for exactly two facts, and both are properties of the +immutable compiler-emitted graph rather than of the value being validated: + +* **termination** — a value cycle (`env.parent === env`) can only walk forever + through a node that reaches itself; +* **no re-walk blowup** — a node the traversal can enter twice with the same + address must memoize, or a shared graph re-walks exponentially. + +So the compiler decides it. `visit_tracking_bits` runs Tarjan over the graph it +just built and propagates a saturating "ways in" count from the root, then sets +the high bit of the op byte on exactly the container nodes that need recording; +the runtime masks the op byte and reads the bit instead of recomputing the +answer per call. On `interp.ts`: `peek(p: Parser)` — 6 nodes, **0 tracked**, +where every token used to be recorded; `asNum(v: Value)` — 123 nodes, **15 +tracked**, exactly the recursive `Node`/`Env` cluster. Descriptor length is +unchanged; the bit rides in a byte that only ever held ops 0–16. The magic goes +`PGT1` → `PGT2` so a mismatched compiler/runtime pair fails closed on the magic +(guard returns 0, caller takes the generic function) rather than reading a v1 +blob as one that opts out of tracking everywhere. + +**`GuardState` zeroed 1 KB of stack per call.** `inline_visited` is now +`MaybeUninit`; only `[..inline_visited_len]` is ever read, and after the change +above most guarded calls never write a slot at all. + +Measured on the 19-program corpus (instructions retired, best-of-3, stdout +byte-exact, `iso_miss` still `misses 0`, both arms built from their own tree +with the same `-p` set and `PERRY_RUNTIME_DIR` pinned per arm). **Exactly 2 of +the 19 rows emit a `js_param_type_guard` call site** — `interp` and `iso_miss`, +two each (`asNum`, `peek`) — and both improve: `interp` **−1.63%**, `iso_miss` +**−1.36%**, peak RSS unchanged on both. Differencing against the same runtime +archive so binary-layout effects cancel (a `PGT1` blob under a `PGT2` runtime +fails every guard), the validator's own cost falls `interp` 1.671 B → 1.422 B +(**−14.9%**, 12.05% → 10.45% of the program) and `iso_miss` 1.611 B → 1.400 B +(**−13.1%**, 9.76% → 8.60%). + +★ The other 17 rows are **not attributable in either direction**. The two arms' +`libperry_runtime.a` differ in exactly two functions out of 11,185 in the +crate's codegen unit — `js_param_type_guard` (808 → 316 bytes) and +`GuardState::matches` (+28) — with every other function byte-identical, and +those 17 programs execute neither. Their movement (`pipeline` −3.9%, +`retain_wide1` +0.6%, `deeplist` −0.5%, the rest within ±0.1%) is address-layout +noise: two `main` builds from identical source came out byte-identical (archive +and `perry` binary alike) and repeat runs of one binary spread ~0.1%, so the +build is deterministic and `pipeline`'s ±4% is what an address-hash-sensitive +program does when the heap moves. None of it is claimed here. + +★★ #8202's premise — that the fixed per-call overhead dominates — does not hold. +It is ~15% of the validator's cost; the structural walk is the other ~10.5pp of +`interp`. Measured separately (see the issue), a diagnostic runtime whose guard +always accepts and one whose guard always rejects land within 0.3% of each +other, 12% below `main`: on these two rows the specialization the validator +gates is worth ~0.2% while running the validator costs ~12%. That is a policy +question for #8094/#8079, not a per-call-overhead one. + +Review follow-up: the analysis decides tracking from the DESCRIPTOR graph, but +"entered twice with the same address" is a property of the VALUE. One object +held at several fields re-enters an untracked node at `entries == 1`, and +nesting that duplication multiplies — `d` levels of a two-way share re-walk +`k^d` times where the unconditional memo ran once. The realistic sharing shapes +are safe (a recursive type is on a cycle, a diamond has two ways in), but +`MAX_DEPTH` bounds depth, not total work. `MAX_VISITS` now caps cumulative +visits and fails the guard to the generic function — the same safe direction as +the depth cap, and the better choice on its own terms past a million checks. +Covered by `nested_value_duplication_through_untracked_nodes_is_bounded`. diff --git a/crates/perry-codegen/src/codegen/param_guard.rs b/crates/perry-codegen/src/codegen/param_guard.rs index 683d220526..06035ee0e0 100644 --- a/crates/perry-codegen/src/codegen/param_guard.rs +++ b/crates/perry-codegen/src/codegen/param_guard.rs @@ -325,7 +325,186 @@ impl<'a> GuardGraphBuilder<'a> { } } -const MAGIC: u32 = 0x3154_4750; // `PGT1`, little-endian. +const MAGIC: u32 = 0x3254_4750; // `PGT2`, little-endian. + +/// Set on a container node's op byte to tell `js_param_type_guard` that a +/// visit to that node must be recorded in the traversal's visited set +/// (`OP_TRACK_VISIT` in `perry-runtime`'s `param_type_guard`). +/// +/// The validator kept the set unconditionally, which cost a linear scan of up +/// to 64 entries — and past that a `HashSet` insert — for every container it +/// touched. On the shapes that actually pay for guards that is per ARRAY +/// ELEMENT: validating `p: { toks: Token[], pos: number }` on every `peek(p)` +/// recorded one entry per token, none of which can ever be consulted. The set +/// only earns its keep on a node that can be entered twice, and the compiler +/// owns the graph, so it decides that here instead (#8202). +const OP_TRACK_VISIT: u8 = 0x80; + +fn node_children(node: &GuardNode) -> Vec { + match node { + GuardNode::Array(elem) | GuardNode::Set(elem) | GuardNode::RecursiveRef(elem) => { + vec![*elem] + } + GuardNode::Tuple(elems) | GuardNode::Union(elems) => elems.clone(), + GuardNode::Object { fields, .. } => fields.iter().map(|field| field.ty).collect(), + GuardNode::Map { key, value } => vec![*key, *value], + _ => Vec::new(), + } +} + +/// Only these ops consult the visited set at all; tagging anything else would +/// change bytes the validator never reads. +fn is_container(node: &GuardNode) -> bool { + matches!( + node, + GuardNode::Array(_) + | GuardNode::Tuple(_) + | GuardNode::Object { .. } + | GuardNode::Map { .. } + | GuardNode::Set(_) + ) +} + +/// Every node that lies on a directed cycle: its strongly-connected component +/// has more than one member, or it points at itself. Iterative Tarjan, so a +/// 4096-node descriptor cannot blow the compiler's stack. +fn cycle_members(children: &[Vec]) -> Vec { + let count = children.len(); + let mut index = vec![u32::MAX; count]; + let mut low = vec![0u32; count]; + let mut on_stack = vec![false; count]; + let mut component: Vec = Vec::new(); + let mut cycle = vec![false; count]; + let mut next_index = 0u32; + // (node, next unvisited child slot) + let mut work: Vec<(u32, usize)> = Vec::new(); + + for root in 0..count { + if index[root] != u32::MAX { + continue; + } + index[root] = next_index; + low[root] = next_index; + next_index += 1; + component.push(root as u32); + on_stack[root] = true; + work.push((root as u32, 0)); + + while let Some((node, cursor)) = work.pop() { + let node_index = node as usize; + if let Some(child) = children[node_index].get(cursor).copied() { + work.push((node, cursor + 1)); + let child_index = child as usize; + if child_index >= count { + continue; + } + if child == node { + cycle[node_index] = true; + } else if index[child_index] == u32::MAX { + index[child_index] = next_index; + low[child_index] = next_index; + next_index += 1; + component.push(child); + on_stack[child_index] = true; + work.push((child, 0)); + } else if on_stack[child_index] { + low[node_index] = low[node_index].min(index[child_index]); + } + continue; + } + if low[node_index] == index[node_index] { + let mut members: Vec = Vec::new(); + while let Some(top) = component.pop() { + on_stack[top as usize] = false; + members.push(top); + if top == node { + break; + } + } + if members.len() > 1 { + for member in members { + cycle[member as usize] = true; + } + } + } + if let Some((parent, _)) = work.last().copied() { + let parent_index = parent as usize; + low[parent_index] = low[parent_index].min(low[node_index]); + } + } + } + cycle +} + +/// One bit per node: does a visit to it have to go in the visited set? +/// +/// Two facts make the set load-bearing, and both are properties of this +/// immutable graph rather than of the value being validated: +/// +/// * **termination** — a value cycle (`env.parent === env`) can only walk +/// forever through a node that reaches itself, so every node on a descriptor +/// cycle is recorded; +/// * **no re-walk blowup** — a node the traversal can enter twice with the same +/// address memoizes, which keeps total work linear in (address, node) pairs. +/// `entries` answers that by propagating a saturating "how many ways in" +/// count from the root, resetting at each node already known to memoize. +/// +/// Everything else — the tree-shaped descriptors that dominate real guarded +/// parameters — records nothing, because a second visit could never be +/// consulted anyway. +fn visit_tracking_bits(nodes: &[GuardNode], root: u32) -> Vec { + let count = nodes.len(); + let children: Vec> = nodes.iter().map(node_children).collect(); + let mut parents: Vec> = vec![Vec::new(); count]; + for (id, edges) in children.iter().enumerate() { + for child in edges { + if let Some(slot) = parents.get_mut(*child as usize) { + slot.push(id as u32); + } + } + } + + // Only a CONTAINER on a cycle actually memoizes — the validator consults + // the set nowhere else — so only those cut the propagation below. A union + // or recursive-reference cycle carrying no container is bounded by the + // validator's depth cap instead, exactly as it is today. + let cycles = cycle_members(&children); + let mut track: Vec = nodes + .iter() + .enumerate() + .map(|(id, node)| is_container(node) && cycles[id]) + .collect(); + // Saturating at 2: "can be entered more than once" is the whole question. + let mut entries = vec![0u8; count]; + if let Some(slot) = entries.get_mut(root as usize) { + *slot = 1; + } + let mut work: Vec = (0..count as u32).collect(); + while let Some(node) = work.pop() { + let node_index = node as usize; + let mut value = u8::from(node == root); + for parent in &parents[node_index] { + let parent_index = *parent as usize; + // A node that already memoizes hands its subtree exactly one entry, + // however many ways the walk reached the node itself. + let out = if track[parent_index] { + 1 + } else { + entries[parent_index] + }; + value = value.saturating_add(out).min(2); + } + if value > entries[node_index] { + entries[node_index] = value; + work.extend_from_slice(&children[node_index]); + } + } + + for (id, node) in nodes.iter().enumerate() { + track[id] = is_container(node) && (track[id] || entries[id] >= 2); + } + track +} fn put_u16(out: &mut Vec, value: u16) { out.extend_from_slice(&value.to_le_bytes()); @@ -415,11 +594,21 @@ fn descriptor_for_type( class_ids, }; let root = builder.build_type(ty, false)?; - let bodies = builder + let mut bodies = builder .nodes .iter() .map(encode_node) .collect::>>()?; + for (body, tracked) in bodies + .iter_mut() + .zip(visit_tracking_bits(&builder.nodes, root)) + { + if tracked { + if let Some(op) = body.first_mut() { + *op |= OP_TRACK_VISIT; + } + } + } let node_count: u32 = bodies.len().try_into().ok()?; let header_len = 12usize.checked_add((bodies.len() + 1).checked_mul(4)?)?; let mut offset: u32 = header_len.try_into().ok()?; @@ -589,7 +778,7 @@ pub(crate) fn scalar_descriptor_rep(descriptor: &[u8]) -> Option Some(TypedParamRep::F64), 2 => Some(TypedParamRep::I32), 3 => Some(TypedParamRep::I1), @@ -659,6 +848,118 @@ mod tests { assert_eq!(scalar_descriptor_rep(b"PGT1"), None); } + fn object_alias(name: &str, fields: &[(&str, Type)]) -> (String, Type) { + let mut properties = HashMap::new(); + for (field, ty) in fields { + properties.insert( + (*field).to_string(), + perry_hir::types::PropertyInfo { + ty: ty.clone(), + optional: false, + readonly: false, + }, + ); + } + ( + name.to_string(), + Type::Object(perry_hir::types::ObjectType { + name: Some(name.to_string()), + properties, + property_order: Some(fields.iter().map(|(f, _)| (*f).to_string()).collect()), + index_signature: None, + }), + ) + } + + fn tracked_ops(descriptor: &[u8]) -> Vec { + let word = |at: usize| u32::from_le_bytes(descriptor[at..at + 4].try_into().unwrap()); + let node_count = word(8) as usize; + (0..node_count) + .map(|id| descriptor[word(12 + id * 4) as usize]) + .filter(|op| op & OP_TRACK_VISIT != 0) + .map(|op| op & !OP_TRACK_VISIT) + .collect() + } + + /// (#8202) The shape that pays for guards in practice — `peek(p: Parser)`, + /// whose `toks: Token[]` walk touches every element on every call — is a + /// tree, so no visit is worth recording. Nothing may carry the bit. + #[test] + fn a_tree_shaped_descriptor_records_no_visits() { + let aliases = HashMap::from([ + object_alias("Token", &[("kind", Type::String), ("text", Type::String)]), + object_alias( + "Parser", + &[ + ( + "toks", + Type::Array(Box::new(Type::Named("Token".to_string()))), + ), + ("pos", Type::Number), + ], + ), + ]); + let descriptor = descriptor_for_type( + &Type::Named("Parser".to_string()), + &aliases, + &HashMap::new(), + &HashMap::new(), + &HashMap::new(), + ) + .unwrap(); + assert_eq!(tracked_ops(&descriptor), Vec::::new()); + } + + /// A value cycle can only walk forever through a node that reaches itself, + /// so the container on the cycle MUST carry the bit — the validator's only + /// termination argument for `env.parent === env` rests on it. + #[test] + fn a_container_on_a_cycle_records_its_visits() { + let aliases = HashMap::from([object_alias( + "Env", + &[( + "parent", + Type::Union(vec![Type::Named("Env".to_string()), Type::Null]), + )], + )]); + let descriptor = descriptor_for_type( + &Type::Named("Env".to_string()), + &aliases, + &HashMap::new(), + &HashMap::new(), + &HashMap::new(), + ) + .unwrap(); + assert_eq!(tracked_ops(&descriptor), vec![11]); + } + + /// A node two fields share can be entered twice with the SAME address, so + /// it memoizes; dropping that would make a deep shared graph re-walk + /// exponentially. Its own children stay untracked — the memo at the + /// convergence point already holds their entry count at one. + #[test] + fn a_shared_container_records_its_visits() { + let aliases = HashMap::from([ + object_alias("Leaf", &[("v", Type::Number)]), + object_alias( + "Pair", + &[ + ("a", Type::Named("Leaf".to_string())), + ("b", Type::Named("Leaf".to_string())), + ], + ), + ]); + let descriptor = descriptor_for_type( + &Type::Named("Pair".to_string()), + &aliases, + &HashMap::new(), + &HashMap::new(), + &HashMap::new(), + ) + .unwrap(); + assert_eq!(tracked_ops(&descriptor), vec![11]); + } + #[test] fn recursive_alias_serializes_as_a_finite_graph() { let mut props = HashMap::new(); diff --git a/crates/perry-runtime/src/param_type_guard.rs b/crates/perry-runtime/src/param_type_guard.rs index de68f921bd..5fd0a49979 100644 --- a/crates/perry-runtime/src/param_type_guard.rs +++ b/crates/perry-runtime/src/param_type_guard.rs @@ -10,11 +10,48 @@ use crate::array::ArrayHeader; use crate::object::ObjectHeader; use crate::value::{JSValue, POINTER_MASK, TAG_FALSE, TAG_HOLE, TAG_TRUE}; use std::collections::HashSet; +use std::mem::MaybeUninit; -const MAGIC: u32 = 0x3154_4750; // `PGT1`, little-endian. +const MAGIC: u32 = 0x3254_4750; // `PGT2`, little-endian. + +/// Set by the compiler on a container node's op byte when a visit to that +/// node MUST be recorded in the traversal's visited set (#8202). +/// +/// The set exists for two reasons, and only nodes that can hit either one +/// need to pay for it: +/// +/// * **termination** — a value cycle (`env.parent === env`) can only walk +/// forever through a descriptor node that reaches itself, so a node off +/// every descriptor cycle cannot loop; +/// * **no re-walk blowup** — a node the traversal can enter twice with the +/// same address memoizes, which keeps total work linear in (address, node) +/// pairs. Below that, every node is entered once per address and the memo +/// could never hit. +/// +/// `visit_tracking_bits` in `perry-codegen`'s `codegen::param_guard` decides +/// both from the immutable graph it just built, so the runtime reads the +/// answer instead of recomputing it per call. A tree-shaped +/// descriptor — `{ toks: Token[], pos: number }`, a union of object literals — +/// now records nothing at all, where before every container visit paid a +/// linear scan of up to 64 entries and then a `HashSet` insert per element. +const OP_TRACK_VISIT: u8 = 0x80; +const OP_MASK: u8 = 0x7F; const MAX_DESCRIPTOR_LEN: usize = 1 << 20; const MAX_NODES: usize = 4096; const MAX_DEPTH: usize = 256; +/// Cumulative node visits allowed in one validation. `MAX_DEPTH` bounds how +/// DEEP the walk goes, not how much of it runs. #8238 stopped recording a +/// visit for descriptor nodes that are neither on a cycle nor reachable two +/// ways, which is exactly right for the descriptor graph — but a *value* can +/// still re-enter such a node with the same address, by holding one object at +/// many indices of an array. Nesting that duplication multiplies, so an +/// untracked subtree that used to be memoized into one walk can run k^d times. +/// Exhausting the budget fails the guard, which is the same safe direction as +/// the depth cap: the caller falls back to the generic function. A million +/// checks is also the point past which the guard has lost on its own terms — +/// no specialized call recoups that — so the fallback is the better choice +/// here even when the walk would have terminated. +const MAX_VISITS: u32 = 1 << 20; const MAX_CONTAINER_LEN: usize = 16_000_000; const INLINE_VISITED: usize = 64; @@ -103,10 +140,16 @@ struct GuardState<'a> { /// Container/node pairs already proved during this traversal. The log /// makes speculative union arms reversible: a failed arm must not leave /// behind a fact that could make a later recursive visit succeed. - inline_visited: [(usize, u32); INLINE_VISITED], + /// + /// Deliberately uninitialised (#8202): only `[..inline_visited_len]` is + /// ever read, and most guarded calls insert nothing at all, so zeroing + /// 1 KB of stack on entry was pure fixed cost on every guarded call. + inline_visited: [MaybeUninit<(usize, u32)>; INLINE_VISITED], inline_visited_len: usize, spill_visited: Option>, spill_log: Vec<(usize, u32)>, + /// Cumulative `matches` entries, capped by `MAX_VISITS`. + visits: u32, /// The last object `plain_object` validated, keyed on the NaN-box bits it /// came from (#8202). A union tries its arms against the SAME value, so /// every arm past the first re-ran the whole validation — including the @@ -150,7 +193,13 @@ impl GuardState<'_> { fn seen_or_insert(&mut self, address: usize, node_id: u32) -> bool { let key = (address, node_id); - if self.inline_visited[..self.inline_visited_len].contains(&key) + // SAFETY: every slot below `inline_visited_len` was written by an + // earlier insert. `rollback` only lowers the length, so a slot can go + // back out of range but never becomes readable while uninitialised. + let seen_inline = self.inline_visited[..self.inline_visited_len] + .iter() + .any(|slot| unsafe { slot.assume_init() } == key); + if seen_inline || self .spill_visited .as_ref() @@ -159,7 +208,7 @@ impl GuardState<'_> { return true; } if self.inline_visited_len < INLINE_VISITED { - self.inline_visited[self.inline_visited_len] = key; + self.inline_visited[self.inline_visited_len] = MaybeUninit::new(key); self.inline_visited_len += 1; } else { self.spill_visited @@ -410,12 +459,20 @@ impl GuardState<'_> { if depth > MAX_DEPTH { return false; } + self.visits += 1; + if self.visits > MAX_VISITS { + return false; + } let Some(node) = self.descriptor.node(node_id) else { return false; }; - let Some(op) = node.first().copied() else { + let Some(tagged_op) = node.first().copied() else { return false; }; + // The compiler folds the visit-tracking decision into the op byte's + // high bit; the low seven bits are the op itself (#8202). + let track = tagged_op & OP_TRACK_VISIT != 0; + let op = tagged_op & OP_MASK; match op { OP_ANY => node.len() == 1, OP_NUMBER => node.len() == 1 && (value.is_number() || value.is_int32()), @@ -450,7 +507,7 @@ impl GuardState<'_> { let Some((array, length)) = self.plain_array(value) else { return false; }; - if self.seen_or_insert(array as usize, node_id) { + if track && self.seen_or_insert(array as usize, node_id) { return true; } let elements = @@ -476,7 +533,7 @@ impl GuardState<'_> { if length != count { return false; } - if self.seen_or_insert(array as usize, node_id) { + if track && self.seen_or_insert(array as usize, node_id) { return true; } let elements = @@ -507,7 +564,7 @@ impl GuardState<'_> { { return false; } - if self.seen_or_insert(address, node_id) { + if track && self.seen_or_insert(address, node_id) { return true; } let (keys, may_have_accessors) = if field_count == 0 { @@ -595,7 +652,7 @@ impl GuardState<'_> { let Some((map, size)) = self.plain_map(value) else { return false; }; - if self.seen_or_insert(map as usize, node_id) { + if track && self.seen_or_insert(map as usize, node_id) { return true; } let entries = (*map).entries as *const f64; @@ -621,7 +678,7 @@ impl GuardState<'_> { let Some((set, size)) = self.plain_set(value) else { return false; }; - if self.seen_or_insert(set as usize, node_id) { + if track && self.seen_or_insert(set as usize, node_id) { return true; } let elements = (*set).elements as *const f64; @@ -656,10 +713,11 @@ pub extern "C" fn js_param_type_guard(value: f64, descriptor: *const u8, length: let root = descriptor.root; let mut state = GuardState { descriptor, - inline_visited: [(0, 0); INLINE_VISITED], + inline_visited: [const { MaybeUninit::uninit() }; INLINE_VISITED], inline_visited_len: 0, spill_visited: None, spill_log: Vec::new(), + visits: 0, validated_object: None, }; unsafe { state.matches(JSValue::from_bits(value.to_bits()), root, 0) as i32 } @@ -824,6 +882,135 @@ mod tests { assert_eq!(guard(JSValue::number(1.0), &descriptor), 0); } + /// `{ : }`, optionally carrying the compiler's + /// visit-tracking bit, as one object node. + fn tracked_single_field_node(track: bool, name: &[u8], child: u32) -> Vec { + let mut body = vec![if track { + OP_OBJECT | OP_TRACK_VISIT + } else { + OP_OBJECT + }]; + body.extend_from_slice(&0u32.to_le_bytes()); // class_id: structural + body.extend_from_slice(&1u32.to_le_bytes()); // one field + body.push(0); // required + body.extend_from_slice(&(name.len() as u16).to_le_bytes()); + body.extend_from_slice(name); + body.extend_from_slice(&child.to_le_bytes()); + body + } + + fn plain_object(fields: &[(&[u8], JSValue)]) -> (*mut ObjectHeader, JSValue) { + let object = crate::object::js_object_alloc(0, fields.len() as u32); + for (name, value) in fields { + let key = crate::string::js_string_from_bytes(name.as_ptr(), name.len() as u32); + crate::object::js_object_set_field_by_name(object, key, f64::from_bits(value.bits())); + } + let value = JSValue::from_bits(crate::value::js_nanbox_pointer(object as i64).to_bits()); + (object, value) + } + + /// (#8202) The bit is the validator's ONLY termination argument for a + /// value cycle: `node.next === node` against a self-referential + /// descriptor node must memoize on the second arrival and accept. + #[test] + fn a_tracked_node_terminates_on_a_cyclic_value() { + let (_, node) = plain_object(&[(b"next", JSValue::undefined())]); + let key = crate::string::js_string_from_bytes(b"next".as_ptr(), 4); + crate::object::js_object_set_field_by_name( + crate::value::js_nanbox_get_pointer(f64::from_bits(node.bits())) as *mut ObjectHeader, + key, + f64::from_bits(node.bits()), + ); + + assert_eq!( + guard(node, &descriptor(0, &[&tracked_single_field_node(true, b"next", 0)])), + 1 + ); + // Without it the walk runs out of depth and the caller conservatively + // takes the generic function — never a hang, never a false accept. + assert_eq!( + guard(node, &descriptor(0, &[&tracked_single_field_node(false, b"next", 0)])), + 0 + ); + } + + /// An untracked node is a pure cost saving, not a semantic change: a + /// shared address reached twice re-walks and still decides the same way. + #[test] + fn an_untracked_shared_node_decides_the_same_way() { + let (_, leaf) = plain_object(&[(b"v", JSValue::number(1.0))]); + let (_, pair) = plain_object(&[(b"a", leaf), (b"b", leaf)]); + + let mut pair_body = vec![OP_OBJECT]; + pair_body.extend_from_slice(&0u32.to_le_bytes()); + pair_body.extend_from_slice(&2u32.to_le_bytes()); + for name in [b"a", b"b"] { + pair_body.push(0); + pair_body.extend_from_slice(&1u16.to_le_bytes()); + pair_body.extend_from_slice(name); + pair_body.extend_from_slice(&1u32.to_le_bytes()); + } + let leaf_body = tracked_single_field_node(false, b"v", 2); + let blob = descriptor(0, &[&pair_body, &leaf_body, &[OP_NUMBER]]); + assert_eq!(guard(pair, &blob), 1); + + let mut mismatched = blob.clone(); + *mismatched.last_mut().unwrap() = OP_STRING; + assert_eq!(guard(pair, &mismatched), 0); + } + + /// #8238 drops the visit record for nodes that are neither on a cycle nor + /// reachable two ways in the DESCRIPTOR graph. A *value* can still re-enter + /// such a node with the same address, by holding one object at several + /// fields, and nesting that duplication multiplies: `d` levels of a + /// two-way share re-walk the leaf 2^d times where the memoized walk ran it + /// once. `MAX_DEPTH` does not bound that — it bounds depth, not total work. + /// `MAX_VISITS` does, in the same safe direction as the depth cap. + #[test] + fn nested_value_duplication_through_untracked_nodes_is_bounded() { + const LEVELS: u32 = 40; + + // Descriptor: LEVELS untracked `{a: next, b: next}` nodes over a number. + // Every node is single-entry and acyclic, so #8238 leaves them all + // untracked — this is precisely the shape the analysis declines to mark. + let mut nodes: Vec> = Vec::new(); + for level in 0..LEVELS { + let mut body = vec![OP_OBJECT]; + body.extend_from_slice(&0u32.to_le_bytes()); + body.extend_from_slice(&2u32.to_le_bytes()); + for name in [b"a", b"b"] { + body.push(0); + body.extend_from_slice(&1u16.to_le_bytes()); + body.extend_from_slice(name); + body.extend_from_slice(&(level + 1).to_le_bytes()); + } + nodes.push(body); + } + nodes.push(vec![OP_NUMBER]); + let refs: Vec<&[u8]> = nodes.iter().map(|n| n.as_slice()).collect(); + let blob = descriptor(0, &refs); + + // Value: the same child at BOTH fields, all the way down. + let mut value = JSValue::number(1.0); + for _ in 0..LEVELS { + value = plain_object(&[(b"a", value), (b"b", value)]).1; + } + + // Unbounded this is 2^40 visits. The budget stops it and fails the + // guard, so the caller takes the generic function. + assert_eq!(guard(value, &blob), 0); + } + + /// The tracking bits changed what the op byte means, so a descriptor from + /// a compiler that predates them must not be read as one that opts out of + /// tracking everywhere — it fails closed on the magic instead. + #[test] + fn the_previous_descriptor_format_is_refused() { + let mut previous = one_node(&[OP_NUMBER]); + previous[0..4].copy_from_slice(&0x3154_4750u32.to_le_bytes()); + assert_eq!(guard(JSValue::number(1.0), &previous), 0); + } + #[test] fn collection_descriptors_validate_every_entry() { let map_descriptor = descriptor(