Fixing stack balance under generator/yield #1502
Conversation
We were caching and restoring the sp in the processtryfinally in order to balance the stack when we are in the middle of call. However in the case of generator we can yield out of the call. In that case we actually haven't finished the try part. So once we comeback after yield we don't need to cache the stack again. Introduced the flag to capture such state and fix the problem.
|
@aneeshdk @tcare @ThomsonTan please have a look. |
|
Looks good |
| this->m_flags |= InterpreterStackFrameFlags_WithinTryBlock; | ||
|
|
||
| CacheSp(); | ||
| if (shouldCacheSP) |
There was a problem hiding this comment.
shouldCacheSP [](start = 16, length = 13)
Do we also need flip shouldCacheSP right after this if in case we re-enter ProcessTryFinally?
There was a problem hiding this comment.
We shouldn't do that, look at the second example in the test file we have two trys so we should not cache-sp in both cases. however say if you have two destructuring pattern in the call-expression we would have to cache sp on both occasion.
There was a problem hiding this comment.
I see. Yes, we need ignore cache SP for both ProcessTryFinally for your second test, but how about if there are some more ProcessTryFinally following but which are not for the previous yield? Seems this fix will also ignore caching SP for them.
In reply to: 76500192 [](ancestors = 76500192)
|
Can you also add one test case for async functions too? |
Merge pull request #1502 from akroshg:fix1 We were caching and restoring the sp in the processtryfinally in order to balance the stack when we are in the middle of call. However in the case of generator we can yield out of the call. In that case we actually haven't finished the try part. So once we comeback after yield we don't need to cache the stack again. Introduced the flag to capture such state and fix the problem.
6 participants
We were caching and restoring the sp in the processtryfinally in order to balance the stack when we are in the middle of call. However in the case of generator we can yield out of the call. In that case we actually haven't finished the try part.
So once we comeback after yield we don't need to cache the stack again. Introduced the flag to capture such state and fix the problem.