Implement runtime limits for loops

[HalidOdat]

Depends on #2866

Related to #2350

This PR adds runtime limits on all loops (while, do..while, for, for-in, and for-of) that can be controlled at runtime, this is feature gated under the "runtime-limits" feature flag because introduces some minor overhead due to needed iteration tracking.

By default when the feature is enabled it is set to u64::MAX, which means no runtime limit

It changes the following:

• Add RuntimeLimits that contains the runtime limits.
• Add $boa.limits.loop getter and setter for easier testing.
• Make the error non-catchable.

Setting the limit from rust:

context.runtime_limits_mut().set_loop_iteration_limit(1024); // Set to an arbitrary number

Or using the $boa object for debugging:

$boa.limits.loop = 4 // Set max loop iteration through the debug object $boa

// All the following statements throw a range error with the message:
//
// RangeError: max loop iteration limit 4 exceeded

while (1) {}
do { } while(1)
for (let i = 0; 1; ++i) {}
for (let e of [1, 2, 3, 4, 5]) {} // One above max
for (let e in [1, 2, 3, 4, 5]) {} // One above max

[github-actions]

Test262 conformance changes

Test result	main count	PR count	difference
Total	94,601	94,601	0
Passed	73,293	73,293	0
Ignored	17,540	17,540	0
Failed	3,768	3,768	0
Panics	0	0	0
Conformance	77.48%	77.48%	0.00%

[codecov]

Codecov Report

Merging #2857 (4ccc5e6) into main (f4ebb2b) will decrease coverage by 0.53%.
The diff coverage is 39.86%.

❗ Current head 4ccc5e6 differs from pull request most recent head 4f15682. Consider uploading reports for the commit 4f15682 to get more accurate results

@@            Coverage Diff             @@
##             main    #2857      +/-   ##
==========================================
- Coverage   51.77%   51.24%   -0.53%     
==========================================
  Files         427      430       +3     
  Lines       42610    42714     +104     
==========================================
- Hits        22061    21890     -171     
- Misses      20549    20824     +275     

Impacted Files	Coverage Δ
boa_cli/src/debug/limits.rs	0.00% <0.00%> (ø)
boa_cli/src/debug/mod.rs	0.00% <0.00%> (ø)
boa_engine/src/bytecompiler/statement/loop.rs	81.32% <ø> (ø)
boa_engine/src/lib.rs	78.20% <ø> (ø)
boa_engine/src/vm/flowgraph/mod.rs	0.00% <ø> (ø)
boa_examples/src/bin/runtime_limits.rs	0.00% <0.00%> (ø)
boa_engine/src/error.rs	33.48% <16.00%> (-1.44%)	⬇️
boa_engine/src/context/mod.rs	47.48% <33.33%> (-0.40%)	⬇️
boa_engine/src/vm/opcode/call/mod.rs	52.12% <37.50%> (ø)
boa_engine/src/vm/opcode/new/mod.rs	75.67% <50.00%> (ø)
... and 5 more

... and 15 files with indirect coverage changes

[raskad]

I have not done a complete review, but I saw the new opcodes and though; can we not keep the limit counts in the EnvStackEntrys and manage them in the LoopStart, LoopContinue and LoopEnd opcodes?

[HalidOdat]

I have not done a complete review, but I saw the new opcodes and though; can we not keep the limit counts in the EnvStackEntrys and manage them in the LoopStart, LoopContinue and LoopEnd opcodes?

Thanks for the suggestion! It's probably the best place to put it! I realized this when I was half way through implementing the initial POC 😆

[raskad]

Looks really good.

Can we set the stack_size_limit to 1024 (or another value that currently works) in our tests and the boa_tester? That way we use the feature ourselves and we may also catch bugs that spill the stack.

[HalidOdat]

Can we set the stack_size_limit to 1024 (or another value that currently works) in our tests and the boa_tester? That way we use the feature ourselves and we may also catch bugs that spill the stack.

It set to 1024 but It's hidden under the feature flag, Maybe this should be a opt-out feature? So we also prevent infinite loops too (especially in the CI and boa_tester)

[Razican]

Can we set the stack_size_limit to 1024 (or another value that currently works) in our tests and the boa_tester? That way we use the feature ourselves and we may also catch bugs that spill the stack.

It set to 1024 but It's hidden under the feature flag, Maybe this should be a opt-out feature? So we also prevent infinite loops too (especially in the CI and boa_tester)

Does it affect performance? If it's negligible, I would say we should use an Option<NonZeroU64>> to handle it, and make it None by default (or even Some(NonZeroU64::MAX)). Then, we could have an unset() function, to disable runtime limits. I think it makes sense to limit it by default, and not have a feature on it.

[Razican]

I'm missing a test, where we run a loop without limits for 10 iterations (as an example) and it works, then we set the limit to 5 iterations, try to run it again, and check that we get the error.

[nekevss]

Just curious on what your thoughts might be about setting this as a u16 vs. u64.

[HalidOdat]

I set it because I felt like that was too restrictive [0, 65535]for u16, u32 with [0, 4294967295] is better but it is possible to iterate over it (takes like a second on node with jitting), but u64 should be more than enough for everyone [0, 18446744073709551615] :D

[nekevss]

😆 I'd agree that u64 is more than enough.

I mostly brought it up because of how large u64 is. It may be worth keeping track or maybe getting a poll/input to see if people are setting runtime limits past a certain point and see if we should lower to a u32 or something. But that could just be me being overly concerned and thinking about it completely wrong.

[HalidOdat]

Does it affect performance? If it's negligible, I would say we should use an Option> to handle it, and make it None by default (or even Some(NonZeroU64::MAX)). Then, we could have an unset() function, to disable runtime limits. I think it makes sense to limit it by default, and not have a feature on it.

Yes, ran the benchmarks locally and it wasn't showing that it had any effect on performance, and it makes sense to allow limiting by default.

[HalidOdat]

Added an example in boa_examples for loop limits also removed the feature flag made it default,

I stuck with using u64 instead of Option<NonZeroU64> for loop iteration count because it would introduce conditional jumps. The way we know if there is a limit set, is by checking if it's set to u64::MAX. Changed the condition to check for > instead of >=, this allows us to exploit a property of comparisons and unsigned integers that u64::MAX is not greater than u64::MAX , adding 1 to u64::MAX overflows to 0. Now when we increment the counter we do a wrapping_add.

With these changes the limit u64::MAX will never satisfy the condition, as if it was not set.

[Razican]

Looks good to me :) check my suggestions and see if they make sense

[nekevss]

Everything looks just about good to me 😄

[raskad]

Nice addition!