-
-
Notifications
You must be signed in to change notification settings - Fork 478
Description
Describe the bug
Our current implementation of getter and setter properties makes it possible to overflow the stack with bigger recursion limits.
To Reproduce
async function* f() {}
f().return({
get then() {
this.then;
},
});Then run in debug mode (you might need to bump the default value of recursion in RuntimeLimits to something like 2048)
boa/core/engine/src/vm/runtime_limits.rs
Line 22 in 6200e4b
| recursion: 512, |
cargo run --bin boa -- script.jsExpected behavior
The engine throws a recursion limit error.
Build environment (please complete the following information):
- OS: Arch Linux
- Version: 6.17.3-arch2-1
- Target triple: x86_64-unknown-linux-gnu
- Rustc version: rustc 1.91.1
Additional context
When executing a property get or set, we directly run any Javascript code if the property is a getter or setter property:
boa/core/engine/src/object/internal_methods/mod.rs
Lines 738 to 741 in 6200e4b
| DescriptorKind::Accessor { get: Some(get), .. } if !get.is_undefined() => { | |
| // 8. Return ? Call(getter, Receiver). | |
| get.call(&receiver, &[], context) | |
| } |
boa/core/engine/src/object/internal_methods/mod.rs
Lines 918 to 920 in 6200e4b
| Some(set) if !set.is_undefined() => { | |
| // 7. Perform ? Call(setter, Receiver, « V »). | |
| set.call(&receiver, &[value], context)?; |
Which runs the bytecode directly:
boa/core/engine/src/object/operations.rs
Lines 394 to 427 in 6200e4b
| pub fn call( | |
| &self, | |
| this: &JsValue, | |
| args: &[JsValue], | |
| context: &mut Context, | |
| ) -> JsResult<JsValue> { | |
| // SKIP: 1. If argumentsList is not present, set argumentsList to a new empty List. | |
| // SKIP: 2. If IsCallable(F) is false, throw a TypeError exception. | |
| // NOTE(HalidOdat): For object's that are not callable we implement a special __call__ internal method | |
| // that throws on call. | |
| context.vm.stack.push(this.clone()); // this | |
| context.vm.stack.push(self.clone()); // func | |
| let argument_count = args.len(); | |
| context.vm.stack.calling_convention_push_arguments(args); | |
| // 3. Return ? F.[[Call]](V, argumentsList). | |
| let frame_index = context.vm.frames.len(); | |
| if self.__call__(argument_count).resolve(context)? { | |
| return Ok(context.vm.stack.pop()); | |
| } | |
| if frame_index + 1 == context.vm.frames.len() { | |
| context.vm.frame.set_exit_early(true); | |
| } else { | |
| context.vm.frames[frame_index + 1].set_exit_early(true); | |
| } | |
| let result = context.run().consume(); | |
| context.vm.pop_frame().expect("frame must exist"); | |
| result | |
| } |
This needs to be adjusted such that it bubbles up to the parent if the property is a getter, which gives a chance to the vm to prepare execution and increase its recursion count.