From b4b77e77b486f02b28929a0b1899ac4c81b28e72 Mon Sep 17 00:00:00 2001 From: Haled Odat <8566042+HalidOdat@users.noreply.github.com> Date: Fri, 5 May 2023 22:49:13 +0200 Subject: [PATCH] Implement runtime limits for recursion (#2904) * Implement runtime limits for recursion * Remove "tail-call-optimization" from ignore list * Run prettier * Add example and tests --- boa_cli/src/debug/limits.rs | 35 +++++++++++++- boa_engine/src/vm/opcode/call/mod.rs | 32 +++++++++++++ .../src/vm/opcode/iteration/loop_ops.rs | 2 +- boa_engine/src/vm/opcode/new/mod.rs | 16 +++++++ boa_engine/src/vm/runtime_limits.rs | 17 +++++++ boa_engine/src/vm/tests.rs | 41 +++++++++++++++- boa_examples/src/bin/runtime_limits.rs | 47 +++++++++++++++++-- docs/boa_object.md | 16 ++++++- test_ignore.toml | 1 - 9 files changed, 196 insertions(+), 11 deletions(-) diff --git a/boa_cli/src/debug/limits.rs b/boa_cli/src/debug/limits.rs index 49fff9970d..5a3b59a700 100644 --- a/boa_cli/src/debug/limits.rs +++ b/boa_cli/src/debug/limits.rs @@ -1,7 +1,7 @@ use boa_engine::{ object::{FunctionObjectBuilder, ObjectInitializer}, property::Attribute, - Context, JsArgs, JsObject, JsResult, JsValue, NativeFunction, + Context, JsArgs, JsNativeError, JsObject, JsResult, JsValue, NativeFunction, }; fn get_loop(_: &JsValue, _: &[JsValue], context: &mut Context<'_>) -> JsResult { @@ -15,6 +15,22 @@ fn set_loop(_: &JsValue, args: &[JsValue], context: &mut Context<'_>) -> JsResul Ok(JsValue::undefined()) } +fn get_recursion(_: &JsValue, _: &[JsValue], context: &mut Context<'_>) -> JsResult { + let max = context.runtime_limits().recursion_limit(); + Ok(JsValue::from(max)) +} + +fn set_recursion(_: &JsValue, args: &[JsValue], context: &mut Context<'_>) -> JsResult { + let value = args.get_or_undefined(0).to_length(context)?; + let Ok(value) = value.try_into() else { + return Err( + JsNativeError::range().with_message(format!("Argument {value} greater than usize::MAX")).into() + ); + }; + context.runtime_limits_mut().set_recursion_limit(value); + Ok(JsValue::undefined()) +} + pub(super) fn create_object(context: &mut Context<'_>) -> JsObject { let get_loop = FunctionObjectBuilder::new(context, NativeFunction::from_fn_ptr(get_loop)) .name("get loop") @@ -24,6 +40,17 @@ pub(super) fn create_object(context: &mut Context<'_>) -> JsObject { .name("set loop") .length(1) .build(); + + let get_recursion = + FunctionObjectBuilder::new(context, NativeFunction::from_fn_ptr(get_recursion)) + .name("get recursion") + .length(0) + .build(); + let set_recursion = + FunctionObjectBuilder::new(context, NativeFunction::from_fn_ptr(set_recursion)) + .name("set recursion") + .length(1) + .build(); ObjectInitializer::new(context) .accessor( "loop", @@ -31,5 +58,11 @@ pub(super) fn create_object(context: &mut Context<'_>) -> JsObject { Some(set_loop), Attribute::WRITABLE | Attribute::CONFIGURABLE | Attribute::NON_ENUMERABLE, ) + .accessor( + "recursion", + Some(get_recursion), + Some(set_recursion), + Attribute::WRITABLE | Attribute::CONFIGURABLE | Attribute::NON_ENUMERABLE, + ) .build() } diff --git a/boa_engine/src/vm/opcode/call/mod.rs b/boa_engine/src/vm/opcode/call/mod.rs index 6f59fcb301..e25bf3d149 100644 --- a/boa_engine/src/vm/opcode/call/mod.rs +++ b/boa_engine/src/vm/opcode/call/mod.rs @@ -17,6 +17,14 @@ impl Operation for CallEval { const INSTRUCTION: &'static str = "INST - CallEval"; fn execute(context: &mut Context<'_>) -> JsResult { + if context.vm.runtime_limits.recursion_limit() <= context.vm.frames.len() { + return Err(JsNativeError::runtime_limit() + .with_message(format!( + "Maximum recursion limit {} exceeded", + context.vm.runtime_limits.recursion_limit() + )) + .into()); + } if context.vm.runtime_limits.stack_size_limit() <= context.vm.stack.len() { return Err(JsNativeError::runtime_limit() .with_message("Maximum call stack size exceeded") @@ -77,6 +85,14 @@ impl Operation for CallEvalSpread { const INSTRUCTION: &'static str = "INST - CallEvalSpread"; fn execute(context: &mut Context<'_>) -> JsResult { + if context.vm.runtime_limits.recursion_limit() <= context.vm.frames.len() { + return Err(JsNativeError::runtime_limit() + .with_message(format!( + "Maximum recursion limit {} exceeded", + context.vm.runtime_limits.recursion_limit() + )) + .into()); + } if context.vm.runtime_limits.stack_size_limit() <= context.vm.stack.len() { return Err(JsNativeError::runtime_limit() .with_message("Maximum call stack size exceeded") @@ -143,6 +159,14 @@ impl Operation for Call { const INSTRUCTION: &'static str = "INST - Call"; fn execute(context: &mut Context<'_>) -> JsResult { + if context.vm.runtime_limits.recursion_limit() <= context.vm.frames.len() { + return Err(JsNativeError::runtime_limit() + .with_message(format!( + "Maximum recursion limit {} exceeded", + context.vm.runtime_limits.recursion_limit() + )) + .into()); + } if context.vm.runtime_limits.stack_size_limit() <= context.vm.stack.len() { return Err(JsNativeError::runtime_limit() .with_message("Maximum call stack size exceeded") @@ -182,6 +206,14 @@ impl Operation for CallSpread { const INSTRUCTION: &'static str = "INST - CallSpread"; fn execute(context: &mut Context<'_>) -> JsResult { + if context.vm.runtime_limits.recursion_limit() <= context.vm.frames.len() { + return Err(JsNativeError::runtime_limit() + .with_message(format!( + "Maximum recursion limit {} exceeded", + context.vm.runtime_limits.recursion_limit() + )) + .into()); + } if context.vm.runtime_limits.stack_size_limit() <= context.vm.stack.len() { return Err(JsNativeError::runtime_limit() .with_message("Maximum call stack size exceeded") diff --git a/boa_engine/src/vm/opcode/iteration/loop_ops.rs b/boa_engine/src/vm/opcode/iteration/loop_ops.rs index 3b586aeb24..6352da6582 100644 --- a/boa_engine/src/vm/opcode/iteration/loop_ops.rs +++ b/boa_engine/src/vm/opcode/iteration/loop_ops.rs @@ -86,7 +86,7 @@ impl Operation for LoopContinue { cleanup_loop_environment(context); return Err(JsNativeError::runtime_limit() - .with_message(format!("max loop iteration limit {max} exceeded")) + .with_message(format!("Maximum loop iteration limit {max} exceeded")) .into()); } } diff --git a/boa_engine/src/vm/opcode/new/mod.rs b/boa_engine/src/vm/opcode/new/mod.rs index 80c58ea00a..8e6c78a69a 100644 --- a/boa_engine/src/vm/opcode/new/mod.rs +++ b/boa_engine/src/vm/opcode/new/mod.rs @@ -16,6 +16,14 @@ impl Operation for New { const INSTRUCTION: &'static str = "INST - New"; fn execute(context: &mut Context<'_>) -> JsResult { + if context.vm.runtime_limits.recursion_limit() <= context.vm.frames.len() { + return Err(JsNativeError::runtime_limit() + .with_message(format!( + "Maximum recursion limit {} exceeded", + context.vm.runtime_limits.recursion_limit() + )) + .into()); + } if context.vm.runtime_limits.stack_size_limit() <= context.vm.stack.len() { return Err(JsNativeError::runtime_limit() .with_message("Maximum call stack size exceeded") @@ -55,6 +63,14 @@ impl Operation for NewSpread { const INSTRUCTION: &'static str = "INST - NewSpread"; fn execute(context: &mut Context<'_>) -> JsResult { + if context.vm.runtime_limits.recursion_limit() <= context.vm.frames.len() { + return Err(JsNativeError::runtime_limit() + .with_message(format!( + "Maximum recursion limit {} exceeded", + context.vm.runtime_limits.recursion_limit() + )) + .into()); + } if context.vm.runtime_limits.stack_size_limit() <= context.vm.stack.len() { return Err(JsNativeError::runtime_limit() .with_message("Maximum call stack size exceeded") diff --git a/boa_engine/src/vm/runtime_limits.rs b/boa_engine/src/vm/runtime_limits.rs index 00083b697a..5d41451e30 100644 --- a/boa_engine/src/vm/runtime_limits.rs +++ b/boa_engine/src/vm/runtime_limits.rs @@ -6,6 +6,9 @@ pub struct RuntimeLimits { /// Max loop iterations before an error is thrown. loop_iteration_limit: u64, + + /// Max function recursion limit + resursion_limit: usize, } impl Default for RuntimeLimits { @@ -13,6 +16,7 @@ impl Default for RuntimeLimits { fn default() -> Self { Self { loop_iteration_limit: u64::MAX, + resursion_limit: 400, stack_size_limit: 1024, } } @@ -58,4 +62,17 @@ impl RuntimeLimits { pub fn set_stack_size_limit(&mut self, value: usize) { self.stack_size_limit = value; } + + /// Get recursion limit. + #[inline] + #[must_use] + pub const fn recursion_limit(&self) -> usize { + self.resursion_limit + } + + /// Set recursion limit before an error is thrown. + #[inline] + pub fn set_recursion_limit(&mut self, value: usize) { + self.resursion_limit = value; + } } diff --git a/boa_engine/src/vm/tests.rs b/boa_engine/src/vm/tests.rs index 9460ae2dce..edbf25aece 100644 --- a/boa_engine/src/vm/tests.rs +++ b/boa_engine/src/vm/tests.rs @@ -255,7 +255,7 @@ fn loop_runtime_limit() { for (let i = 0; i < 20; ++i) { } "#}, JsNativeErrorKind::RuntimeLimit, - "max loop iteration limit 10 exceeded", + "Maximum loop iteration limit 10 exceeded", ), TestAction::assert_eq( indoc! {r#" @@ -268,7 +268,44 @@ fn loop_runtime_limit() { while (1) { } "#}, JsNativeErrorKind::RuntimeLimit, - "max loop iteration limit 10 exceeded", + "Maximum loop iteration limit 10 exceeded", + ), + ]); +} + +#[test] +fn recursion_runtime_limit() { + run_test_actions([ + TestAction::run(indoc! {r#" + function factorial(n) { + if (n == 0) { + return 1; + } + + return n * factorial(n - 1); + } + "#}), + TestAction::assert_eq("factorial(8)", JsValue::new(40_320)), + TestAction::assert_eq("factorial(11)", JsValue::new(39_916_800)), + TestAction::inspect_context(|context| { + context.runtime_limits_mut().set_recursion_limit(10); + }), + TestAction::assert_native_error( + "factorial(11)", + JsNativeErrorKind::RuntimeLimit, + "Maximum recursion limit 10 exceeded", + ), + TestAction::assert_eq("factorial(8)", JsValue::new(40_320)), + TestAction::assert_native_error( + indoc! {r#" + function x() { + x() + } + + x() + "#}, + JsNativeErrorKind::RuntimeLimit, + "Maximum recursion limit 10 exceeded", ), ]); } diff --git a/boa_examples/src/bin/runtime_limits.rs b/boa_examples/src/bin/runtime_limits.rs index c1e518063c..593ec80bf4 100644 --- a/boa_examples/src/bin/runtime_limits.rs +++ b/boa_examples/src/bin/runtime_limits.rs @@ -1,9 +1,13 @@ -use boa_engine::{Context, Source}; +use boa_engine::{Context, JsValue, Source}; fn main() { // Create the JavaScript context. let mut context = Context::default(); + // ----------------------------------------- + // Loop Iteration Limit + // ----------------------------------------- + // Set the context's runtime limit on loops to 10 iterations. context.runtime_limits_mut().set_loop_iteration_limit(10); @@ -13,7 +17,7 @@ fn main() { for (let i = 0; i < 5; ++i) { } ", )); - result.expect("no error should be thrown"); + assert!(result.is_ok()); // Here we exceed the limit by 1 iteration and a `RuntimeLimit` error is thrown. // @@ -27,7 +31,7 @@ fn main() { } ", )); - result.expect_err("should have throw an error"); + assert!(result.is_err()); // Preventing an infinity loops let result = context.eval_script(Source::from_bytes( @@ -35,7 +39,7 @@ fn main() { while (true) { } ", )); - result.expect_err("should have throw an error"); + assert!(result.is_err()); // The limit applies to all types of loops. let result = context.eval_script(Source::from_bytes( @@ -43,5 +47,38 @@ fn main() { for (let e of [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]) { } ", )); - result.expect_err("should have throw an error"); + assert!(result.is_err()); + + // ----------------------------------------- + // Recursion Limit + // ----------------------------------------- + + // Create and register `factorial` function. + let result = context.eval_script(Source::from_bytes( + r" + function factorial(n) { + if (n == 0) { + return 1; + } + + return n * factorial(n - 1); + } + ", + )); + assert!(result.is_ok()); + + // Run function before setting the limit and assert that it works. + let result = context.eval_script(Source::from_bytes("factorial(11)")); + assert_eq!(result, Ok(JsValue::new(39_916_800))); + + // Setting runtime limit for recustion to 10. + context.runtime_limits_mut().set_recursion_limit(10); + + // Run without exceeding recursion limit and assert that it works. + let result = context.eval_script(Source::from_bytes("factorial(8)")); + assert_eq!(result, Ok(JsValue::new(40_320))); + + // Run exceeding limit by 1 and assert that it fails. + let result = context.eval_script(Source::from_bytes("factorial(11)")); + assert!(result.is_err()); } diff --git a/docs/boa_object.md b/docs/boa_object.md index 184e1d54d7..bb685c00d1 100644 --- a/docs/boa_object.md +++ b/docs/boa_object.md @@ -253,5 +253,19 @@ Its setter can be used to set the loop iteration limit. ```javascript $boa.limits.loop = 10; -while (true) {} // RuntimeLimit: max loop iteration limit 10 exceeded +while (true) {} // RuntimeLimit: Maximum loop iteration limit 10 exceeded +``` + +### Getter & Setter `$boa.limits.recursion` + +This is an accessor property on the module, its getter returns the recursion limit before an error is thrown. +Its setter can be used to set the recursion limit. + +```javascript +$boa.limits.recursion = 100; + +function x() { + return x(); +} +x(); // RuntimeLimit: Maximum recursion limit 100 exceeded ``` diff --git a/test_ignore.toml b/test_ignore.toml index 56cf37445f..ceed610a55 100644 --- a/test_ignore.toml +++ b/test_ignore.toml @@ -7,7 +7,6 @@ features = [ "SharedArrayBuffer", "resizable-arraybuffer", "Temporal", - "tail-call-optimization", "ShadowRealm", "FinalizationRegistry", "Atomics",