Skip to content

Commit

Permalink
Fix handleRecursiveTailCall for osr exit at op_tail_call
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=254574
rdar://107598022

Reviewed by Yusuke Suzuki.

Previously, we introduced a patch https://commits.webkit.org/260787@main
which merges op_enter, op_get_scope, and op_check_traps into op_enter
for less prologue overhead. However, the patch crashes in a tail recursion
when OSR exit from FTL to Baseline at op_tail_call. This is becuase we
exit to the offset(op_enter) + 1 which would miss the execution of op_get_scope
that merged into op_enter in the previous path. In that case, program would
crash when trying to dereference an undefined scope after OSR exit. To fix this
issue we should just update the exit to  offset(op_enter) instead of
offset(op_enter) + 1.

JavaScript tail recursion foo:
function foo(n) {
    ...
    return foo(n);
}

Bytecode for foo with scope at loc4:
[  0] enter
[  1] ...
...
[ 11] resolve_scope  dst:loc10, scope:loc4
...
[ 38] tail_call ...
[...] ret       ...

DFG for foo:
...
--> foo // inlined recursive tail call
    ...
    @node(..., bc#38, exit: bc#38 --> bc#1, ...)
    ...
<-- foo

DFG for foo:
...
--> foo // inlined recursive tail call
    ...
    @node(..., bc#38, exit: bc#38 --> bc#0, ...)
    ...
<-- foo

* JSTests/stress/osr-exit-at-tail-call-in-tail-recursion.js: Added.
(foo):
(bar):
* Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::handleRecursiveTailCall):

Canonical link: https://commits.webkit.org/263183@main
  • Loading branch information
Yijia Huang committed Apr 20, 2023
1 parent ee7418d commit a75b74f
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 1 deletion.
30 changes: 30 additions & 0 deletions JSTests/stress/osr-exit-at-tail-call-in-tail-recursion.js
@@ -0,0 +1,30 @@
//@ runDefault

function shouldThrow(func, errorMessage) {
var errorThrown = false;
var error = null;
try {
func();
} catch (e) {
errorThrown = true;
error = e;
}
if (!errorThrown)
throw new Error('not thrown');
if (String(error) !== errorMessage)
throw new Error(`bad error: ${String(error)}`);
}

"use strict";

function bar(x, y) {
function foo(a, b) {
if (a == 0) b += ',';
return foo(b - 1, a, 43);
}
return foo(x, y);
}

shouldThrow(() => {
bar(1, 1);
}, "RangeError: Maximum call stack size exceeded.");
2 changes: 1 addition & 1 deletion Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
Expand Up @@ -1598,7 +1598,7 @@ bool ByteCodeParser::handleRecursiveTailCall(Node* callTargetNode, CallVariant c
auto oldStackTop = m_inlineStackTop;
m_inlineStackTop = stackEntry;
static_assert(OpcodeIDWidthBySize<JSOpcodeTraits, OpcodeSize::Wide32>::opcodeIDSize == 1);
m_currentIndex = BytecodeIndex(opcodeLengths[op_enter] + 1);
m_currentIndex = BytecodeIndex(opcodeLengths[op_enter]);
m_exitOK = true;
processSetLocalQueue();
m_currentIndex = oldIndex;
Expand Down

0 comments on commit a75b74f

Please sign in to comment.