diff --git a/core/engine/src/bytecompiler/statement/loop.rs b/core/engine/src/bytecompiler/statement/loop.rs index 0c046ae3f37..12e663a10e0 100644 --- a/core/engine/src/bytecompiler/statement/loop.rs +++ b/core/engine/src/bytecompiler/statement/loop.rs @@ -122,8 +122,6 @@ impl ByteCompiler<'_> { } } - self.bytecode.emit_increment_loop_iteration(); - if let Some(final_expr) = for_loop.final_expr() { self.compile_expr_for_side_effects(final_expr); } @@ -134,6 +132,11 @@ impl ByteCompiler<'_> { .condition() .map(|condition| self.compile_condition_and_branch(condition, hoisted.as_ref())); + // Count the iteration after the condition check but before the body, so the + // limit counts body executions (including the first, which skips the loop + // preamble via `initial_jump`) and loops that finish within the limit pass. + self.bytecode.emit_increment_loop_iteration(); + self.compile_stmt(for_loop.body(), use_expr, true); self.bytecode.emit_jump(start_address); @@ -183,7 +186,6 @@ impl ByteCompiler<'_> { let start_address = self.next_opcode_location(); self.push_loop_control_info_for_of_in_loop(label, start_address, use_expr); - self.bytecode.emit_increment_loop_iteration(); self.bytecode.emit_iterator_next(); @@ -192,6 +194,10 @@ impl ByteCompiler<'_> { let exit = self.jump_if_true(&done_reg); self.register_allocator.dealloc(done_reg); + // Count the iteration only after the done check, so exhausting the iterator + // within the limit does not raise a limit error. + self.bytecode.emit_increment_loop_iteration(); + let outer_scope = self.push_declarative_scope(for_in_loop.scope()); // For let/const with a local identifier binding, emit iterator_value @@ -285,7 +291,6 @@ impl ByteCompiler<'_> { } else { self.push_loop_control_info_for_of_in_loop(label, start_address, use_expr); } - self.bytecode.emit_increment_loop_iteration(); self.bytecode.emit_iterator_next(); if for_of_loop.r#await() { @@ -308,6 +313,10 @@ impl ByteCompiler<'_> { let exit = self.jump_if_true(&done_reg); self.register_allocator.dealloc(done_reg); + // Count the iteration only after the done check, so exhausting the iterator + // within the limit does not raise a limit error. + self.bytecode.emit_increment_loop_iteration(); + let outer_scope = self.push_declarative_scope(for_of_loop.scope()); // For let/const with a local identifier binding, emit iterator_value @@ -428,11 +437,14 @@ impl ByteCompiler<'_> { let hoisted = self.try_hoist_loop_condition(Some(while_loop.condition())); let start_address = self.next_opcode_location(); - self.bytecode.emit_increment_loop_iteration(); self.push_loop_control_info(label, start_address, use_expr); let exit = self.compile_condition_and_branch(while_loop.condition(), hoisted.as_ref()); + // Count the iteration after the condition check but before the body, so the + // limit counts body executions and loops that finish within the limit pass. + self.bytecode.emit_increment_loop_iteration(); + self.compile_stmt(while_loop.body(), use_expr, true); self.bytecode.emit_jump(start_address); @@ -461,12 +473,16 @@ impl ByteCompiler<'_> { self.push_loop_control_info(label, start_address, use_expr); let condition_label_address = self.next_opcode_location(); - self.bytecode.emit_increment_loop_iteration(); let exit = self.compile_condition_and_branch(do_while_loop.cond(), hoisted.as_ref()); self.patch_jump(initial_label); + // Count the iteration after the condition check but before the body, so the + // limit counts body executions (including the first, which skips the condition + // via `initial_label`) and loops that finish within the limit pass. + self.bytecode.emit_increment_loop_iteration(); + self.compile_stmt(do_while_loop.body(), use_expr, true); self.bytecode.emit_jump(condition_label_address); diff --git a/core/engine/src/vm/opcode/iteration/loop_ops.rs b/core/engine/src/vm/opcode/iteration/loop_ops.rs index 21cd262fd6c..9824fc9dcca 100644 --- a/core/engine/src/vm/opcode/iteration/loop_ops.rs +++ b/core/engine/src/vm/opcode/iteration/loop_ops.rs @@ -15,7 +15,7 @@ impl IncrementLoopIteration { let frame = context.vm.frame_mut(); let previous_iteration_count = frame.loop_iteration_count; - if previous_iteration_count > max { + if previous_iteration_count >= max { return Err(RuntimeLimitError::LoopIteration.into()); } diff --git a/core/engine/src/vm/tests.rs b/core/engine/src/vm/tests.rs index 0b25366ac41..9df53703732 100644 --- a/core/engine/src/vm/tests.rs +++ b/core/engine/src/vm/tests.rs @@ -343,6 +343,66 @@ fn loop_runtime_limit() { ]); } +/// Test that the loop iteration limit allows exactly `limit` body executions +/// and that all loop kinds agree on the count. +/// +/// See: +#[test] +fn loop_iteration_limit_counts_body_executions() { + run_test_actions([ + TestAction::inspect_context(|context| { + context.runtime_limits_mut().set_loop_iteration_limit(10); + }), + // Loops that run exactly `limit` iterations must complete without error. + TestAction::assert_eq("var a = 0; for (let i = 0; i < 10; ++i) { a++; } a", 10), + TestAction::assert_eq("var b = 0; while (b < 10) { b++; } b", 10), + TestAction::assert_eq("var c = 0; do { c++; } while (c < 10); c", 10), + TestAction::assert_eq( + "var d = 0; for (const x of [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]) { d++; } d", + 10, + ), + TestAction::assert_eq( + indoc! {r#" + var e = 0; + var o = { k0: 0, k1: 0, k2: 0, k3: 0, k4: 0, k5: 0, k6: 0, k7: 0, k8: 0, k9: 0 }; + for (const k in o) { e++; } + e + "#}, + 10, + ), + // Loops that would exceed the limit must error after exactly `limit` + // body executions, and `for` and `while` loops must agree. + TestAction::assert_runtime_limit_error( + "var forCount = 0; for (let i = 0; i < 1000; ++i) { forCount++; }", + RuntimeLimitError::LoopIteration, + ), + TestAction::assert_eq("forCount", 10), + TestAction::assert_runtime_limit_error( + "var whileCount = 0; while (true) { whileCount++; }", + RuntimeLimitError::LoopIteration, + ), + TestAction::assert_eq("whileCount", 10), + TestAction::assert_runtime_limit_error( + "var doWhileCount = 0; do { doWhileCount++; } while (true);", + RuntimeLimitError::LoopIteration, + ), + TestAction::assert_eq("doWhileCount", 10), + TestAction::assert_runtime_limit_error( + indoc! {r#" + var forOfCount = 0; + var infinite = { + [Symbol.iterator]() { + return { next() { return { done: false, value: 1 }; } }; + } + }; + for (const x of infinite) { forOfCount++; } + "#}, + RuntimeLimitError::LoopIteration, + ), + TestAction::assert_eq("forOfCount", 10), + ]); +} + #[test] fn recursion_runtime_limit() { run_test_actions([ diff --git a/tests/insta-bytecode/src/snapshots/insta_bytecode__compile_bytecode@basic-loop.js.snap b/tests/insta-bytecode/src/snapshots/insta_bytecode__compile_bytecode@basic-loop.js.snap index be560d0264e..3968ac2f83d 100644 --- a/tests/insta-bytecode/src/snapshots/insta_bytecode__compile_bytecode@basic-loop.js.snap +++ b/tests/insta-bytecode/src/snapshots/insta_bytecode__compile_bytecode@basic-loop.js.snap @@ -7,10 +7,10 @@ input_file: tests/insta-bytecode/scripts/basic-loop.js Location Handler Opcode Operands 000000 StoreZero dst:r02 000005 StoreInt8 value:100, dst:r03 - 00000b Jump address:00001a - 000010 IncrementLoopIteration - 000011 Inc src:r02, dst:r02 - 00001a JumpIfNotLessThan lhs:r02, rhs:r03, address:00002c + 00000b Jump address:000019 + 000010 Inc src:r02, dst:r02 + 000019 JumpIfNotLessThan lhs:r02, rhs:r03, address:00002c + 000026 IncrementLoopIteration 000027 Jump address:000010 00002c CheckReturn 00002d Return @@ -20,4 +20,4 @@ Constants: Bindings: Handlers: Source Map: - 0000: 17..26: (1, 26) + 0000: 16..25: (1, 26) diff --git a/tests/insta-bytecode/src/snapshots/insta_bytecode__compile_bytecode@double-loop-function.js.snap b/tests/insta-bytecode/src/snapshots/insta_bytecode__compile_bytecode@double-loop-function.js.snap index cf5237ef717..614fef957c3 100644 --- a/tests/insta-bytecode/src/snapshots/insta_bytecode__compile_bytecode@double-loop-function.js.snap +++ b/tests/insta-bytecode/src/snapshots/insta_bytecode__compile_bytecode@double-loop-function.js.snap @@ -7,16 +7,16 @@ input_file: tests/insta-bytecode/scripts/double-loop-function.js Location Handler Opcode Operands 000000 StoreZero dst:r02 000005 StoreInt8 value:20, dst:r03 - 00000b Jump address:00001a - 000010 IncrementLoopIteration - 000011 Inc src:r02, dst:r02 - 00001a JumpIfNotLessThan lhs:r02, rhs:r03, address:00007a + 00000b Jump address:000019 + 000010 Inc src:r02, dst:r02 + 000019 JumpIfNotLessThan lhs:r02, rhs:r03, address:00007a + 000026 IncrementLoopIteration 000027 StoreZero dst:r05 00002c StoreInt8 value:50, dst:r06 - 000032 Jump address:000041 - 000037 IncrementLoopIteration - 000038 Inc src:r05, dst:r05 - 000041 JumpIfNotLessThan lhs:r05, rhs:r06, address:000075 + 000032 Jump address:000040 + 000037 Inc src:r05, dst:r05 + 000040 JumpIfNotLessThan lhs:r05, rhs:r06, address:000075 + 00004d IncrementLoopIteration 00004e PushFromRegister src:r00 000053 GetNameGlobal dst:r07, binding_index:0, ic_index:0 000060 PushFromRegister src:r07 @@ -39,6 +39,6 @@ Bindings: 0001: undefined, scope: GlobalObject Handlers: Source Map: - 0000: 17..56: (5, 25) - 0001: 56..101: (6, 27) + 0000: 16..55: (5, 25) + 0001: 55..101: (6, 27) 0002: 101..112: (7, 6) diff --git a/tests/insta-bytecode/src/snapshots/insta_bytecode__compile_bytecode@loop-hoisting.js.snap b/tests/insta-bytecode/src/snapshots/insta_bytecode__compile_bytecode@loop-hoisting.js.snap index c4f44884d67..15ecdb79d0c 100644 --- a/tests/insta-bytecode/src/snapshots/insta_bytecode__compile_bytecode@loop-hoisting.js.snap +++ b/tests/insta-bytecode/src/snapshots/insta_bytecode__compile_bytecode@loop-hoisting.js.snap @@ -7,19 +7,19 @@ input_file: tests/insta-bytecode/scripts/loop-hoisting.js Location Handler Opcode Operands 000000 StoreZero dst:r02 000005 StoreInt8 value:100, dst:r03 - 00000b Jump address:00001a - 000010 IncrementLoopIteration - 000011 Inc src:r02, dst:r02 - 00001a JumpIfNotLessThan lhs:r02, rhs:r03, address:00002c + 00000b Jump address:000019 + 000010 Inc src:r02, dst:r02 + 000019 JumpIfNotLessThan lhs:r02, rhs:r03, address:00002c + 000026 IncrementLoopIteration 000027 Jump address:000010 00002c StoreInt8 value:100, dst:r03 000032 PutLexicalValue src:r03, binding_index:0 00003b Move src:r03, dst:r04 000044 StoreZero dst:r05 - 000049 Jump address:000058 - 00004e IncrementLoopIteration - 00004f Inc src:r05, dst:r05 - 000058 JumpIfNotLessThan lhs:r05, rhs:r04, address:00006a + 000049 Jump address:000057 + 00004e Inc src:r05, dst:r05 + 000057 JumpIfNotLessThan lhs:r05, rhs:r04, address:00006a + 000064 IncrementLoopIteration 000065 Jump address:00004e 00006a StoreInt8 value:100, dst:r06 000070 PutLexicalValue src:r06, binding_index:1 @@ -57,7 +57,7 @@ Bindings: 0004: foo, scope: GlobalObject Handlers: Source Map: - 0000: 17..79: (2, 26) - 0001: 79..153: (5, 24) + 0000: 16..78: (2, 26) + 0001: 78..153: (5, 24) 0002: 153..197: (14, 4) 0003: 197..207: (23, 4) diff --git a/tests/insta-bytecode/src/snapshots/insta_bytecode__compile_bytecode@try-finally.js.snap b/tests/insta-bytecode/src/snapshots/insta_bytecode__compile_bytecode@try-finally.js.snap index 2972605ee3b..15c83270be2 100644 --- a/tests/insta-bytecode/src/snapshots/insta_bytecode__compile_bytecode@try-finally.js.snap +++ b/tests/insta-bytecode/src/snapshots/insta_bytecode__compile_bytecode@try-finally.js.snap @@ -9,10 +9,10 @@ Location Handler Opcode Operands 000005 PutLexicalValue src:r01, binding_index:0 00000e StoreZero dst:r02 000013 StoreInt8 value:5, dst:r03 - 000019 Jump address:000028 - 00001e IncrementLoopIteration - 00001f Inc src:r02, dst:r02 - 000028 JumpIfNotLessThan lhs:r02, rhs:r03, address:000147 + 000019 Jump address:000027 + 00001e Inc src:r02, dst:r02 + 000027 JumpIfNotLessThan lhs:r02, rhs:r03, address:000147 + 000034 IncrementLoopIteration 000035 StoreTrue dst:r04 00003a StoreZero dst:r05 00003f > 0: 0000aa StoreInt8 value:2, dst:r07 @@ -65,6 +65,6 @@ Handlers: 0000: Range: [00003f, 0000aa): Handler: 0000aa, Environment: 00 0001: Range: [0000aa, 0000b9): Handler: 0000b9, Environment: 00 Source Map: - 0000: 31..190: (7, 24) + 0000: 30..190: (7, 24) 0001: 190..285: (12, 5) 0002: 285..322: (14, 3)