Skip to content

Commit

Permalink
[MERGE #4900 @sigatrev] ensure JIT try blocks have enough stack to ba…
Browse files Browse the repository at this point in the history
…ilout to the interpeter

Merge pull request #4900 from sigatrev:finallyStack

exceptions in JIT result in bailouts, and BailOutHelper does a stack probe before going to the interpreter. If that stack probe fails, a stack overflow would be thrown before the original exception was handled, and the top level catch/finally was not executed.

This commit ensures there is enough stack space to bail out to the interpreter to handle and exception if one is thrown.
  • Loading branch information
sigatrev committed Apr 11, 2018
2 parents 578ba29 + ed0c484 commit 7de7b31
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 10 deletions.
2 changes: 2 additions & 0 deletions lib/Runtime/Base/Constants.h
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ namespace Js
static const unsigned MaxProcessJITCodeHeapSize = 1024 * 1024 * 1024;
#endif

static const unsigned MinStackJitEHBailout = MinStackInterpreter + MinStackDefault;

static const size_t StackLimitForScriptInterrupt;


Expand Down
18 changes: 9 additions & 9 deletions lib/Runtime/Language/JavascriptExceptionOperators.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ namespace Js
void *tryCatchFrameAddr = nullptr;
scriptContext->GetThreadContext()->SetHasBailedOutBitPtr((bool*)((char*)frame + hasBailedOutOffset));

PROBE_STACK(scriptContext, Constants::MinStackDefault + spillSize + argsSize);
PROBE_STACK(scriptContext, Constants::MinStackJitEHBailout + spillSize + argsSize);
{
Js::JavascriptExceptionOperators::TryCatchFrameAddrStack tryCatchFrameAddrStack(scriptContext, frame);
try
Expand Down Expand Up @@ -161,7 +161,7 @@ namespace Js
JavascriptExceptionObject *exception = nullptr;
scriptContext->GetThreadContext()->SetHasBailedOutBitPtr((bool*)((char*)frame + hasBailedOutOffset));

PROBE_STACK(scriptContext, Constants::MinStackDefault + spillSize + argsSize);
PROBE_STACK(scriptContext, Constants::MinStackJitEHBailout + spillSize + argsSize);

try
{
Expand Down Expand Up @@ -214,7 +214,7 @@ namespace Js
void *finallyContinuation = nullptr;
JavascriptExceptionObject *exception = nullptr;

PROBE_STACK(scriptContext, Constants::MinStackDefault + spillSize + argsSize);
PROBE_STACK(scriptContext, Constants::MinStackJitEHBailout + spillSize + argsSize);
try
{
tryContinuation = amd64_CallWithFakeFrame(tryAddr, frame, spillSize, argsSize);
Expand Down Expand Up @@ -260,7 +260,7 @@ namespace Js
void * tryCatchFrameAddr = nullptr;
scriptContext->GetThreadContext()->SetHasBailedOutBitPtr((bool*)((char*)localsPtr + hasBailedOutOffset));

PROBE_STACK(scriptContext, Constants::MinStackDefault + argsSize);
PROBE_STACK(scriptContext, Constants::MinStackJitEHBailout + argsSize);
{
Js::JavascriptExceptionOperators::TryCatchFrameAddrStack tryCatchFrameAddrStack(scriptContext, framePtr);

Expand Down Expand Up @@ -334,7 +334,7 @@ namespace Js
JavascriptExceptionObject *exception = nullptr;
scriptContext->GetThreadContext()->SetHasBailedOutBitPtr((bool*)((char*)localsPtr + hasBailedOutOffset));

PROBE_STACK(scriptContext, Constants::MinStackDefault + argsSize);
PROBE_STACK(scriptContext, Constants::MinStackJitEHBailout + argsSize);
try
{
#if defined(_M_ARM)
Expand Down Expand Up @@ -396,7 +396,7 @@ namespace Js
void *finallyContinuation = nullptr;
JavascriptExceptionObject *exception = nullptr;

PROBE_STACK(scriptContext, Constants::MinStackDefault + argsSize);
PROBE_STACK(scriptContext, Constants::MinStackJitEHBailout + argsSize);

try
{
Expand Down Expand Up @@ -446,7 +446,7 @@ namespace Js
void *tryCatchFrameAddr = nullptr;
scriptContext->GetThreadContext()->SetHasBailedOutBitPtr((bool*)((char*)framePtr + hasBailedOutOffset));

PROBE_STACK(scriptContext, Constants::MinStackDefault);
PROBE_STACK(scriptContext, Constants::MinStackJitEHBailout);
{
Js::JavascriptExceptionOperators::TryCatchFrameAddrStack tryCatchFrameAddrStack(scriptContext, framePtr);

Expand Down Expand Up @@ -608,7 +608,7 @@ namespace Js
Js::JavascriptExceptionObject* pExceptionObject = NULL;
void* continuationAddr = NULL;
scriptContext->GetThreadContext()->SetHasBailedOutBitPtr((bool*)((char*)framePtr + hasBailedOutOffset));
PROBE_STACK(scriptContext, Constants::MinStackDefault);
PROBE_STACK(scriptContext, Constants::MinStackJitEHBailout);

try
{
Expand Down Expand Up @@ -764,7 +764,7 @@ namespace Js
Js::JavascriptExceptionObject* pExceptionObject = NULL;
void* continuationAddr = NULL;

PROBE_STACK(scriptContext, Constants::MinStackDefault);
PROBE_STACK(scriptContext, Constants::MinStackJitEHBailout);

try
{
Expand Down
15 changes: 15 additions & 0 deletions test/EH/StackOverflow.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// //-------------------------------------------------------------------------------------------------------
// // Copyright (C) Microsoft. All rights reserved.
// // Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
// //-------------------------------------------------------------------------------------------------------

function foo(a)
{
try { a[0] } finally {}
try { foo(0) } catch(e) {}
try { foo() } catch(e) {}
}

foo(0)

WScript.Echo("PASS");
7 changes: 6 additions & 1 deletion test/EH/rlexe.xml
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,12 @@
<default>
<files>hasBailedOutBug.js</files>
<baseline>hasBailedOutBug.baseline</baseline>
</default>
</default>
</test>
<test>
<default>
<files>StackOverflow.js</files>
</default>
</test>
<test>
<default>
Expand Down

0 comments on commit 7de7b31

Please sign in to comment.