-
-
Couldn't load subscription status.
- Fork 465
Description
Reproducer
function f() {}
function g(inner) {
console.log(inner); // should be undefined, but it prints `5`
}
5;
let outer = g(f());In this case, f does NOT return undefined as it should. Instead, it returns the last evaluated expression: ({}).
Analysis
Any function that does NOT use return will pick up the most recent value set by SetAccumulator, instead of overwriting the return register with undefined. Thus, after returning to the callee, executing the Return instruction will get the wrong return value.
This also affects new f() calls, but in a different way. There are certain checks on CheckReturn to ensure the returned value is at least an object, so the equivalent to the above snippet won't trigger the bug as it is.
function f() {}
function g(inner) {
console.log(inner); // correctly prints an object of type `f` (`f {}`)
}
(5);
let outer = g(new f());However, a simple adjustment will trigger the issue again
function f() {}
function g(inner) {
console.log(inner); // incorrectly prints `{}`, a simple empty object
}
({}); // <---- changed `5` to `{}`
let outer = g(new f());This is because a check on CheckReturn ensures the return value is an object:
boa/core/engine/src/vm/opcode/control_flow/return.rs
Lines 48 to 49 in 80ef082
| let result = if result.is_object() { | |
| result |
However, if SetAccumulator sets the return value with an object instead of a primitive value, the bug triggers again.
Solution
I guess we could push the accumulator to the stack before calling a function, then pop it after calling the function. An alternative approach would be to do this only if SetAccumulator was already called before (at compile time), but I guess the naive approach should work fine for now and we could optimize this later.