From a75b74fc83c06296ff667aaaabf593697eb2fa1b Mon Sep 17 00:00:00 2001 From: Yijia Huang Date: Thu, 20 Apr 2023 11:32:44 -0700 Subject: [PATCH] Fix handleRecursiveTailCall for osr exit at op_tail_call 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 --- ...osr-exit-at-tail-call-in-tail-recursion.js | 30 +++++++++++++++++++ .../JavaScriptCore/dfg/DFGByteCodeParser.cpp | 2 +- 2 files changed, 31 insertions(+), 1 deletion(-) create mode 100644 JSTests/stress/osr-exit-at-tail-call-in-tail-recursion.js diff --git a/JSTests/stress/osr-exit-at-tail-call-in-tail-recursion.js b/JSTests/stress/osr-exit-at-tail-call-in-tail-recursion.js new file mode 100644 index 000000000000..3764fe66991b --- /dev/null +++ b/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."); diff --git a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp index 212b90e6cc51..704c998bc637 100644 --- a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp +++ b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp @@ -1598,7 +1598,7 @@ bool ByteCodeParser::handleRecursiveTailCall(Node* callTargetNode, CallVariant c auto oldStackTop = m_inlineStackTop; m_inlineStackTop = stackEntry; static_assert(OpcodeIDWidthBySize::opcodeIDSize == 1); - m_currentIndex = BytecodeIndex(opcodeLengths[op_enter] + 1); + m_currentIndex = BytecodeIndex(opcodeLengths[op_enter]); m_exitOK = true; processSetLocalQueue(); m_currentIndex = oldIndex;