Skip to content

fix(vm): make loop iteration limit consistent across loop kinds #5464

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mansiverma897993 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(vm): make loop iteration limit consistent across loop kinds #5464

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 22 additions & 6 deletions core/engine/src/bytecompiler/statement/loop.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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);
Expand Down Expand Up @@ -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();

Expand All @@ -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
Expand Down Expand Up @@ -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() {
Expand All @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion core/engine/src/vm/opcode/iteration/loop_ops.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}

Expand Down
60 changes: 60 additions & 0 deletions core/engine/src/vm/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: <https://github.com/boa-dev/boa/issues/5461>
#[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([
Expand Down
10 changes: 5 additions & 5 deletions tests/insta-bytecode/src/snapshots/insta_bytecode__compile_bytecode@basic-loop.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -20,4 +20,4 @@ Constants: <empty>
Bindings: <empty>
Handlers: <empty>
Source Map:
0000: 17..26: (1, 26)
0000: 16..25: (1, 26)
20 changes: 10 additions & 10 deletions ...nsta-bytecode/src/snapshots/insta_bytecode__compile_bytecode@double-loop-function.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -39,6 +39,6 @@ Bindings:
0001: undefined, scope: GlobalObject
Handlers: <empty>
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)
20 changes: 10 additions & 10 deletions tests/insta-bytecode/src/snapshots/insta_bytecode__compile_bytecode@loop-hoisting.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -57,7 +57,7 @@ Bindings:
0004: foo, scope: GlobalObject
Handlers: <empty>
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)
10 changes: 5 additions & 5 deletions tests/insta-bytecode/src/snapshots/insta_bytecode__compile_bytecode@try-finally.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Loading