Fix call stack limits.

RELEASES.md

@@ -16,6 +16,7 @@ Bug fixes
 * Fix bug when accessing properties in closures.
 * Fix bug when accessing a deep index with a function call.
 * Fix bug that sometimes allow assigning to an invalid l-value.
+* Fix off-by-one error with `Engine::set_max_call_levels`.
 
 Breaking changes
 ----------------

src/engine.rs

@@ -170,6 +170,7 @@ impl FromIterator<(ImmutableString, Shared<Module>)> for Imports {
 
 #[cfg(not(feature = "unchecked"))]
 #[cfg(debug_assertions)]
+#[cfg(not(feature = "no_function"))]
 pub const MAX_CALL_STACK_DEPTH: usize = 8;
 #[cfg(not(feature = "unchecked"))]
 #[cfg(debug_assertions)]
@@ -181,6 +182,7 @@ pub const MAX_FUNCTION_EXPR_DEPTH: usize = 16;
 
 #[cfg(not(feature = "unchecked"))]
 #[cfg(not(debug_assertions))]
+#[cfg(not(feature = "no_function"))]
 pub const MAX_CALL_STACK_DEPTH: usize = 128;
 #[cfg(not(feature = "unchecked"))]
 #[cfg(not(debug_assertions))]
@@ -527,8 +529,8 @@ impl State {
 #[derive(Debug, Clone, Eq, PartialEq, Hash)]
 pub struct Limits {
     /// Maximum levels of call-stack to prevent infinite recursion.
-    ///
+    /// Not available under `no_function`.
-    /// Defaults to 16 for debug builds and 128 for non-debug builds.
+    #[cfg(not(feature = "no_function"))]
     pub max_call_stack_depth: usize,
     /// Maximum depth of statements/expressions at global level (0 = unlimited).
     pub max_expr_depth: usize,
@@ -809,6 +811,7 @@ impl Engine {
 
             #[cfg(not(feature = "unchecked"))]
             limits: Limits {
+                #[cfg(not(feature = "no_function"))]
                 max_call_stack_depth: MAX_CALL_STACK_DEPTH,
                 max_expr_depth: MAX_EXPR_DEPTH,
                 #[cfg(not(feature = "no_function"))]
@@ -864,6 +867,7 @@ impl Engine {
 
             #[cfg(not(feature = "unchecked"))]
             limits: Limits {
+                #[cfg(not(feature = "no_function"))]
                 max_call_stack_depth: MAX_CALL_STACK_DEPTH,
                 max_expr_depth: MAX_EXPR_DEPTH,
                 #[cfg(not(feature = "no_function"))]

src/engine_api.rs

@@ -1453,7 +1453,7 @@ impl Engine {
     ) -> Result<T, Box<EvalAltResult>> {
         let mods = &mut (&self.global_sub_modules).into();
 
-        let result = self.eval_ast_with_scope_raw(scope, mods, ast)?;
+        let result = self.eval_ast_with_scope_raw(scope, mods, ast, 0)?;
 
         let typ = self.map_type_name(result.type_name());
 
@@ -1473,12 +1473,13 @@ impl Engine {
         scope: &mut Scope,
         mods: &mut Imports,
         ast: &'a AST,
+        level: usize,
     ) -> Result<Dynamic, Box<EvalAltResult>> {
         let state = &mut State {
             source: ast.clone_source(),
             ..Default::default()
         };
-        self.eval_statements_raw(scope, mods, state, ast.statements(), &[ast.lib()])
+        self.eval_statements_raw(scope, mods, state, ast.statements(), &[ast.lib()], level)
     }
     /// Evaluate a file, but throw away the result and only return error (if any).
     /// Useful for when you don't need the result, but still need to keep track of possible errors.
@@ -1542,7 +1543,7 @@ impl Engine {
             source: ast.clone_source(),
             ..Default::default()
         };
-        self.eval_statements_raw(scope, mods, state, ast.statements(), &[ast.lib()])?;
+        self.eval_statements_raw(scope, mods, state, ast.statements(), &[ast.lib()], 0)?;
         Ok(())
     }
     /// Call a script function defined in an [`AST`] with multiple arguments.

src/engine_settings.rs

@@ -38,6 +38,7 @@ impl Engine {
     /// Set the maximum levels of function calls allowed for a script in order to avoid
     /// infinite recursion and stack overflows.
     #[cfg(not(feature = "unchecked"))]
+    #[cfg(not(feature = "no_function"))]
     #[inline(always)]
     pub fn set_max_call_levels(&mut self, levels: usize) -> &mut Self {
         self.limits.max_call_stack_depth = levels;
@@ -45,6 +46,7 @@ impl Engine {
     }
     /// The maximum levels of function calls allowed for a script.
     #[cfg(not(feature = "unchecked"))]
+    #[cfg(not(feature = "no_function"))]
     #[inline(always)]
     pub fn max_call_levels(&self) -> usize {
         self.limits.max_call_stack_depth

src/fn_call.rs

@@ -405,11 +405,11 @@ impl Engine {
             mods.extend(fn_def.mods.iter_raw().map(|(n, m)| (n.clone(), m.clone())));
         }
 
-        // Evaluate the function at one higher level of call depth
+        // Evaluate the function
         let stmt = &fn_def.body;
 
         let result = self
-            .eval_stmt(scope, mods, state, unified_lib, this_ptr, stmt, level + 1)
+            .eval_stmt(scope, mods, state, unified_lib, this_ptr, stmt, level)
             .or_else(|err| match *err {
                 // Convert return statement to return value
                 EvalAltResult::Return(x, _) => Ok(x),
@@ -586,6 +586,8 @@ impl Engine {
 
                         mem::swap(&mut state.source, &mut source);
 
+                        let level = _level + 1;
+
                         let result = self.call_script_fn(
                             scope,
                             mods,
@@ -595,7 +597,7 @@ impl Engine {
                             func,
                             rest,
                             pos,
-                            _level,
+                            level,
                         );
 
                         // Restore the original source
@@ -610,8 +612,10 @@ impl Engine {
 
                         mem::swap(&mut state.source, &mut source);
 
+                        let level = _level + 1;
+
                         let result = self.call_script_fn(
-                            scope, mods, state, lib, &mut None, func, args, pos, _level,
+                            scope, mods, state, lib, &mut None, func, args, pos, level,
                         );
 
                         // Restore the original source
@@ -667,11 +671,12 @@ impl Engine {
         state: &mut State,
         statements: impl IntoIterator<Item = &'a Stmt>,
         lib: &[&Module],
+        level: usize,
     ) -> Result<Dynamic, Box<EvalAltResult>> {
         statements
             .into_iter()
             .try_fold(().into(), |_, stmt| {
-                self.eval_stmt(scope, mods, state, lib, &mut None, stmt, 0)
+                self.eval_stmt(scope, mods, state, lib, &mut None, stmt, level)
             })
             .or_else(|err| match *err {
                 EvalAltResult::Return(out, _) => Ok(out),
@@ -691,7 +696,7 @@ impl Engine {
         lib: &[&Module],
         script: &str,
         pos: Position,
-        _level: usize,
+        level: usize,
     ) -> Result<Dynamic, Box<EvalAltResult>> {
         self.inc_operations(state, pos)?;
 
@@ -700,13 +705,6 @@ impl Engine {
             return Ok(Dynamic::UNIT);
         }
 
-        // Check for stack overflow
-        #[cfg(not(feature = "no_function"))]
-        #[cfg(not(feature = "unchecked"))]
-        if _level > self.max_call_levels() {
-            return Err(Box::new(EvalAltResult::ErrorStackOverflow(pos)));
-        }
-
         // Compile the script text
         // No optimizations because we only run it once
         let ast = self.compile_with_scope_and_optimization_level(
@@ -727,7 +725,8 @@ impl Engine {
             ..Default::default()
         };
 
-        let result = self.eval_statements_raw(scope, mods, &mut new_state, ast.statements(), lib);
+        let result =
+            self.eval_statements_raw(scope, mods, &mut new_state, ast.statements(), lib, level);
 
         state.operations = new_state.operations;
         result
@@ -1208,6 +1207,8 @@ impl Engine {
                 let mut source = module.id_raw().clone();
                 mem::swap(&mut state.source, &mut source);
 
+                let level = level + 1;
+
                 let result = self.call_script_fn(
                     new_scope, mods, state, lib, &mut None, &fn_def, args, pos, level,
                 );

src/module/mod.rs

@@ -1720,7 +1720,7 @@ impl Module {
         let orig_mods_len = mods.len();
 
         // Run the script
-        engine.eval_ast_with_scope_raw(&mut scope, &mut mods, &ast)?;
+        engine.eval_ast_with_scope_raw(&mut scope, &mut mods, &ast, 0)?;
 
         // Create new module
         let mut module = Module::new();

src/parser.rs

@@ -19,7 +19,7 @@ use crate::stdlib::{
     vec::Vec,
 };
 use crate::syntax::{CustomSyntax, MARKER_BLOCK, MARKER_EXPR, MARKER_IDENT};
-use crate::token::{is_doc_comment, is_keyword_function, is_valid_identifier, Token, TokenStream};
+use crate::token::{is_keyword_function, is_valid_identifier, Token, TokenStream};
 use crate::utils::{get_hasher, StraightHasherBuilder};
 use crate::{
     calc_script_fn_hash, Dynamic, Engine, ImmutableString, LexError, ParseError, ParseErrorType,
@@ -2534,17 +2534,19 @@ fn parse_stmt(
 ) -> Result<Stmt, ParseError> {
     use AccessMode::{ReadOnly, ReadWrite};
 
-    let mut comments: Vec<String> = Default::default();
+    let mut _comments: Vec<String> = Default::default();
+
+    #[cfg(not(feature = "no_function"))]
+    {
         let mut comments_pos = Position::NONE;
 
         // Handle doc-comments.
-    #[cfg(not(feature = "no_function"))]
+        while let (Token::Comment(ref comment), pos) = input.peek().unwrap() {
-    while let (Token::Comment(ref comment), comment_pos) = input.peek().unwrap() {
             if comments_pos.is_none() {
-            comments_pos = *comment_pos;
+                comments_pos = *pos;
             }
 
-        if !is_doc_comment(comment) {
+            if !crate::token::is_doc_comment(comment) {
                 unreachable!("expecting doc-comment, but gets {:?}", comment);
             }
 
@@ -2554,7 +2556,7 @@ fn parse_stmt(
 
             match input.next().unwrap().0 {
                 Token::Comment(comment) => {
-                comments.push(comment);
+                    _comments.push(comment);
 
                     match input.peek().unwrap() {
                         (Token::Fn, _) | (Token::Private, _) => break,
@@ -2565,6 +2567,7 @@ fn parse_stmt(
                 t => unreachable!("expecting Token::Comment, but gets {:?}", t),
             }
         }
+    }
 
     let (token, token_pos) = match input.peek().unwrap() {
         (Token::EOF, pos) => return Ok(Stmt::Noop(*pos)),
@@ -2618,7 +2621,7 @@ fn parse_stmt(
                         pos: pos,
                     };
 
-                    let func = parse_fn(input, &mut new_state, lib, access, settings, comments)?;
+                    let func = parse_fn(input, &mut new_state, lib, access, settings, _comments)?;
 
                     lib.insert(
                         // Qualifiers (none) + function name + number of arguments.

tests/stack.rs

@@ -10,21 +10,24 @@ fn test_stack_overflow_fn_calls() -> Result<(), Box<EvalAltResult>> {
         engine.eval::<INT>(
             r"
                 fn foo(n) { if n <= 1 { 0 } else { n + foo(n-1) } }
-                foo(7)
+                foo(6)
             ",
         )?,
-        27
+        20
     );
 
+    let max = engine.max_call_levels();
+
     #[cfg(not(feature = "unchecked"))]
     assert!(matches!(
         *engine
-            .eval::<()>(
+            .eval::<()>(&format!(
                 r"
-            fn foo(n) { if n == 0 { 0 } else { n + foo(n-1) } }
+                    fn foo(n) {{ if n == 0 {{ 0 }} else {{ n + foo(n-1) }} }}
-            foo(1000)
+                    foo({})
-    "
+                ",
-            )
+                max + 1
+            ))
             .expect_err("should error"),
         EvalAltResult::ErrorStackOverflow(_)
     ));