diff --git a/contributors/people.toml b/contributors/people.toml index 43252507d..5ce80fe59 100644 --- a/contributors/people.toml +++ b/contributors/people.toml @@ -99,3 +99,7 @@ real_name = "Tyrel Haveman" [people.helenkwok] real_name = "Helen Kwok" + +[people.dotCooCoo] +real_name = "Robert Lee" +sync_github_name = true diff --git a/crates/calc/src/eval.rs b/crates/calc/src/eval.rs index 931854732..4bdb3a30d 100644 --- a/crates/calc/src/eval.rs +++ b/crates/calc/src/eval.rs @@ -58,6 +58,24 @@ fn err(e: CellError) -> Ev { Ev::V(Value::Error(e)) } +/// LAMBDA calls that one formula may have in progress at once, counted the same way on every build. +/// The next call answers `#NUM!`. A countdown from 131, the deepest that `MAX_DEPTH` allows, has 132 +/// calls in progress, so it still runs. +const MAX_LAMBDA_CALLS: usize = 140; + +/// Native stack, in bytes, that one formula's evaluation may use below its first `eval`. Past it, +/// `eval` answers `#NUM!`. It guards the stack and leaves 128 KiB of a 1 MiB thread for the frames +/// above the formula. A cell that another cell's formula computes on demand shares the budget of the +/// outermost formula ([`Host::stack_top`]). +const STACK_BUDGET: usize = 896 * 1024; + +/// The address of a local variable, which shows how far down the native stack this call is. +#[inline(always)] +pub(crate) fn stack_position() -> usize { + let marker = 0u8; + std::hint::black_box(&marker) as *const u8 as usize +} + /// What the evaluator needs from its surroundings. pub trait Host { fn workbook(&self) -> &Workbook; @@ -85,6 +103,13 @@ pub trait Host { fn visit_range(&mut self, _sheet: usize, _range: RangeRef, _f: &mut dyn FnMut(&Value) -> bool) -> bool { false } + /// While a formula is calculated on demand inside another one: `stack_position()` where the + /// outermost one started, which the stack budget is counted from. + fn stack_top(&self) -> Option { + None + } + /// Called when a formula's evaluation runs past the stack budget. + fn stack_exhausted(&mut self) {} } pub struct Evaluator<'h> { @@ -93,6 +118,10 @@ pub struct Evaluator<'h> { pub at: CellRef, env: Vec<(String, Ev)>, depth: usize, + /// LAMBDA calls in progress. + calls: usize, + /// `stack_position()` at the outermost `eval`. + stack_top: usize, /// Lambda invocations this cell still has left. Every call spends one charge, so a /// compact hostile formula (a LAMBDA that calls itself, nested `MAP`s over big arrays) /// runs out of work after the same number of calls that a legitimately wide one could @@ -137,7 +166,16 @@ impl Ctx for FnCtx<'_, '_> { _ => None, }) .collect(); - let mut ev = Evaluator { host: &mut *self.ev.host, sheet: area.sheet, at, env, depth: self.ev.depth, fuel: self.ev.fuel }; + let mut ev = Evaluator { + host: &mut *self.ev.host, + sheet: area.sheet, + at, + env, + depth: self.ev.depth, + calls: self.ev.calls, + stack_top: self.ev.stack_top, + fuel: self.ev.fuel, + }; let v = matches!(ev.value(&shifted).scalar().to_bool(), Ok(true)); // The child's spend counts against this cell's budget. self.ev.fuel = ev.fuel; @@ -160,7 +198,7 @@ impl Ctx for FnCtx<'_, '_> { impl<'h> Evaluator<'h> { pub fn new(host: &'h mut dyn Host, sheet: usize, at: CellRef) -> Self { - Evaluator { host, sheet, at, env: Vec::new(), depth: 0, fuel: LAMBDA_FUEL } + Evaluator { host, sheet, at, env: Vec::new(), depth: 0, calls: 0, stack_top: 0, fuel: LAMBDA_FUEL } } /// Evaluates to a value (references are dereferenced; multi-cell → array). @@ -246,10 +284,18 @@ impl<'h> Evaluator<'h> { } pub fn eval(&mut self, e: &Expr) -> Ev { + let here = stack_position(); + if self.depth == 0 { + self.stack_top = self.host.stack_top().unwrap_or(here); + } + if self.stack_top.abs_diff(here) > STACK_BUDGET { + self.host.stack_exhausted(); + return err(CellError::Num); + } self.depth += 1; if self.depth > MAX_DEPTH { self.depth -= 1; - return err(CellError::Calc); + return err(CellError::Num); } let r = self.eval_inner(e); self.depth -= 1; @@ -533,7 +579,7 @@ impl<'h> Evaluator<'h> { } _ => { // A long chain such as A1+A2+…+A500 nests to the left: fold it in a loop instead - // of one level of recursion per operator (past the depth limit it was #CALC!). + // of one level of recursion per operator (past the depth limit it was an error). let mut rest = vec![(op, b)]; let mut left = a; while let Expr::Binary(o, l, r) = left @@ -842,10 +888,11 @@ impl<'h> Evaluator<'h> { fn builtin(&mut self, name: &str, args: &[Expr]) -> Ev { let Some(spec) = gridcraft_functions::lookup(name) else { // A defined name holding a LAMBDA, or a LET-bound lambda. - if let Ev::L(l) = self.name(name) { - return self.apply_lambda(&l, args); - } - return err(CellError::Name); + return match self.name(name) { + Ev::L(l) => self.apply_lambda(&l, args), + Ev::V(Value::Error(x)) => err(x), + _ => err(CellError::Name), + }; }; if args.len() < spec.min_args || args.len() > spec.max_args { return err(CellError::Value); @@ -996,6 +1043,9 @@ impl<'h> Evaluator<'h> { } fn call_lambda_values(&mut self, l: &Lambda, bound: Vec<(String, Ev)>) -> Ev { + if self.calls >= MAX_LAMBDA_CALLS { + return err(CellError::Num); + } // #2382: one charge per invocation, so a cell's total work stays bounded however the // lambdas nest. Foregrounding the charge here means every path into a body (direct // `(2,3)` application, `MAP`/`REDUCE`/`BYROW` helpers, D-function criteria) meters. @@ -1003,10 +1053,12 @@ impl<'h> Evaluator<'h> { Some(f) => f, None => return err(CellError::Calc), }; + self.calls += 1; let saved = std::mem::replace(&mut self.env, l.env.clone()); self.env.extend(bound); let r = self.eval(&l.body); self.env = saved; + self.calls -= 1; r } diff --git a/crates/calc/src/recalc.rs b/crates/calc/src/recalc.rs index 66374bed2..eabc29314 100644 --- a/crates/calc/src/recalc.rs +++ b/crates/calc/src/recalc.rs @@ -44,7 +44,7 @@ use gridcraft_core::{Array, CellError, CellRef, RangeRef, Value}; use gridcraft_formula::Expr; use gridcraft_model::{CalcMode, Workbook}; -use crate::eval::{Area, Evaluator, Host, precedents}; +use crate::eval::{Area, Evaluator, Host, precedents, stack_position}; pub type Key = (usize, CellRef); @@ -432,6 +432,10 @@ struct PassHost<'a> { tainted: HashSet, /// Whether the formula being evaluated has read a previous value. taint: bool, + /// `stack_position()` where the outermost formula being calculated started. + stack_top: usize, + /// Whether an evaluation ran past the stack budget since the innermost formula started. + out_of_stack: bool, } impl<'a> PassHost<'a> { @@ -462,6 +466,8 @@ impl<'a> PassHost<'a> { members: HashSet::default(), tainted: HashSet::default(), taint: false, + stack_top: 0, + out_of_stack: false, } } } @@ -580,6 +586,9 @@ impl PassHost<'_> { self.suspend = Some(k); return Value::Error(CellError::Calc); } + if self.depth == 0 { + self.stack_top = stack_position(); + } self.in_progress.insert(k); self.depth += 1; let outer_taint = std::mem::replace(&mut self.taint, false); @@ -587,10 +596,24 @@ impl PassHost<'_> { self.stack.push(k); } let expr = std::sync::Arc::clone(expr); + let outer_out_of_stack = std::mem::replace(&mut self.out_of_stack, false); let v = { let mut ev = Evaluator::new(self, k.0, k.1); ev.value(&expr) }; + let out_of_stack = std::mem::replace(&mut self.out_of_stack, outer_out_of_stack); + if out_of_stack && self.depth > 1 && self.suspend.is_none() { + // It ran out of the stack left below the formulas that read it: it is calculated + // from the top instead, as a formula needed at `MAX_NESTED` levels deep is. + self.suspend = Some(k); + self.depth -= 1; + self.in_progress.remove(&k); + if self.iterate { + self.stack.pop(); + } + self.taint = outer_taint; + return Value::Error(CellError::Calc); + } if self.suspend.is_some() { // Abandoned: `calculate` evaluates this formula again once the one it needs is done. self.depth -= 1; @@ -745,6 +768,12 @@ impl Host for PassHost<'_> { fn workbook(&self) -> &Workbook { self.wb } + fn stack_top(&self) -> Option { + (self.depth > 1).then_some(self.stack_top) + } + fn stack_exhausted(&mut self) { + self.out_of_stack = true; + } fn spill_range(&mut self, sheet: usize, anchor: CellRef) -> Option { let k = (sheet, anchor); if !(self.pending.contains(&k) || self.results.contains_key(&k)) { diff --git a/crates/calc/src/tests.rs b/crates/calc/src/tests.rs index ef21c49e3..1720bcb57 100644 --- a/crates/calc/src/tests.rs +++ b/crates/calc/src/tests.rs @@ -1025,6 +1025,100 @@ fn lambda_argument_count() { assert_eq!(t.get("A9"), Value::Bool(true)); } +#[test] +fn a_lambda_that_calls_itself_without_end_is_num_on_a_small_stack() { + // 1 MiB is the main thread's stack on Windows, where commands, recalculation and file open run. + let handle = std::thread::Builder::new() + .stack_size(1 << 20) + .spawn(|| { + let mut t = T::new(); + t.wb.names.push(gridcraft_model::DefinedName { + name: "LOOP".into(), + scope: None, + formula: "LAMBDA(n,LOOP(n))".into(), + comment: String::new(), + hidden: false, + }); + t.calc.rebuild(&t.wb); + let endless = "=LET(f,LAMBDA(g,x,g(g,x)),f(f,1))"; + t.set("A1", endless); + t.set("A2", "=LOOP(1)"); + t.set("A3", "=LET(F,LAMBDA(me,n,IF(n<=0,0,1+SUM(MAP({1},LAMBDA(v,me(me,n-1)))))),F(F,100000))"); + t.set("A4", "=LET(F,LAMBDA(me,n,IF(n<=0,0,1+me(me,n-1))),F(F,131))"); + let evaluated = crate::evaluate(&t.wb, 0, c("B1"), endless); + [t.get("A1"), t.get("A2"), t.get("A3"), evaluated, t.get("A4")] + }) + .expect("spawn"); + let [endless, endless_name, through_map, evaluated, countdown] = handle.join().expect("recursion must not overflow the stack"); + assert_eq!(endless, Value::Error(CellError::Num)); + assert_eq!(endless_name, Value::Error(CellError::Num)); + assert_eq!(through_map, Value::Error(CellError::Num)); + assert_eq!(evaluated, Value::Error(CellError::Num)); + assert_eq!(countdown, Value::Number(131.0)); +} + +#[test] +fn a_recursive_lambda_stops_at_the_same_call_on_any_stack_size() { + let run = |stack: usize| { + std::thread::Builder::new() + .stack_size(stack) + .spawn(|| { + let mut wb = Workbook::new(); + wb.names.push(gridcraft_model::DefinedName { + name: "CNT".into(), + scope: None, + formula: "LAMBDA(n,IF(n=0,0,1+CNT(n-1)))".into(), + comment: String::new(), + hidden: false, + }); + // F(F,n) has n + 1 calls in progress at its deepest; W adds one more call per step. + let tail = |n: u32| format!("=LET(F,LAMBDA(me,n,IF(n=0,0,me(me,n-1))),F(F,{n}))"); + let wrapped = |n: u32| format!("=LET(W,LAMBDA(f,k,f(f,k)),F,LAMBDA(me,n,IF(n<=0,0,1+W(me,n-1))),F(F,{n}))"); + [tail(139), tail(140), wrapped(69), wrapped(70), "=CNT(131)".to_string()].map(|f| crate::evaluate(&wb, 0, c("B1"), &f)) + }) + .expect("spawn") + .join() + .expect("recursion must not overflow the stack") + }; + let small = run(1 << 20); + assert_eq!(small, [Value::Number(0.0), Value::Error(CellError::Num), Value::Number(69.0), Value::Error(CellError::Num), Value::Number(131.0)]); + assert_eq!(run(64 << 20), small); +} + +#[test] +fn a_recursive_lambda_that_reads_another_cells_recursive_lambda_shares_the_stack() { + let run = |stack: usize| { + std::thread::Builder::new() + .stack_size(stack) + .spawn(|| { + // Row-major order calculates A1 first, so each cell below is calculated on demand + // at the deepest call of the one above it. + let countdown = |then: &str| format!("=LET(F,LAMBDA(me,n,IF(n<=0,{then},1+me(me,n-1))),F(F,131))"); + let mut wb = Workbook::new(); + let sheet = wb.sheet_mut(0).expect("sheet"); + for (at, f) in [ + ("A1", countdown("A2")), + ("A2", countdown("A3")), + ("A3", countdown("0")), + ("B1", countdown("B2")), + ("B2", "=LET(f,LAMBDA(g,x,g(g,x)),f(f,1))".to_string()), + ] { + sheet.cells.set(c(at), Cell::formula(Formula::new(&f))); + } + let mut calc = Calc::new(); + calc.recalc_all(&mut wb); + ["A1", "A2", "A3", "B1", "B2"].map(|at| wb.sheet(0).map(|s| s.value(c(at))).unwrap_or_default()) + }) + .expect("spawn") + .join() + .expect("recursion must not overflow the stack") + }; + let small = run(1 << 20); + let num = Value::Error(CellError::Num); + assert_eq!(small, [Value::Number(393.0), Value::Number(262.0), Value::Number(131.0), num.clone(), num]); + assert_eq!(run(64 << 20), small); +} + #[test] fn names_and_sheets() { let mut t = T::new();