Skip to content
This repository has been archived by the owner on Oct 15, 2020. It is now read-only.

Commit

Permalink
deps: update ChakraCore to chakra-core/ChakraCore@d7d99e1dbe
Browse files Browse the repository at this point in the history
[1.8>1.9] [MERGE #4535 @meg-gupta] Fix setting hasBailout when there are inlined functions in try/catch

Merge pull request #4535 from meg-gupta:fixCatchEatingUpEx

Fixes OS#15078638

When we bailout executing trycode from within OP_TryCatch, we complete the execution of the current function enclosing the try/catch in the interpreter.
If there was an exception within the try region, it is caught and handled accordingly in ProcessTryHandlerBailOut which reconstructs try/catch/finally frames
when we bailout midway executing code within OP_TryCatch. If there was an exception outside the try region, the catch of the OP_TryCatch ends up catching it,
because it happens to be on the callstack. For this we use the hasBailOut bit which is per function, so we know that this exception has to be passed above.

When we have inlined functions inside the try, and for bailouts inside the inlined code, we do not set the hasBailedOut bit, so that the enclosing functions catch in OP_TryCatch catches it.

But when we bailout from inlined code inside try, we execute inlined code, as well as the enclosing function's code in the interpreter.
We will be execution code past the try/catch of the current function in the interpreter. Now if any code outside the eh region throws,
we will catch that in OP_TryCatch which happens to be on the callstack. And we will end up handling it instead of passing above because we have not set the hasBailedOutBit from the bailout point.

This change fixes this issue. We pass a pointer to the hasBailedOutBit and set it once we have finished executing the inlined frames and are ready to process the interpreter frame of the current function.

Reviewed-By: chakrabot <chakrabot@users.noreply.github.com>
  • Loading branch information
meg-gupta authored and chakrabot committed Jan 17, 2018
1 parent bf5c71f commit 246f843
Show file tree
Hide file tree
Showing 7 changed files with 106 additions and 5 deletions.
11 changes: 11 additions & 0 deletions deps/chakrashim/core/lib/Backend/BailOut.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1217,6 +1217,12 @@ BailOutRecord::BailOutInlinedCommon(Js::JavascriptCallStackLayout * layout, Bail
BailOutReturnValue bailOutReturnValue;
Js::ScriptFunction * innerMostInlinee = nullptr;
BailOutInlinedHelper(layout, currentBailOutRecord, bailOutOffset, returnAddress, bailOutKind, registerSaves, &bailOutReturnValue, &innerMostInlinee, false, branchValue);

bool * hasBailOutBit = layout->functionObject->GetScriptContext()->GetThreadContext()->GetHasBailedOutBitPtr();
if (hasBailOutBit != nullptr && bailOutRecord->ehBailoutData)
{
*hasBailOutBit = true;
}
Js::Var result = BailOutCommonNoCodeGen(layout, currentBailOutRecord, currentBailOutRecord->bailOutOffset, returnAddress, bailOutKind, branchValue,
registerSaves, &bailOutReturnValue);
ScheduleFunctionCodeGen(Js::ScriptFunction::FromVar(layout->functionObject), innerMostInlinee, currentBailOutRecord, bailOutKind, bailOutOffset, savedImplicitCallFlags, returnAddress);
Expand Down Expand Up @@ -1255,6 +1261,11 @@ BailOutRecord::BailOutFromLoopBodyInlinedCommon(Js::JavascriptCallStackLayout *
BailOutReturnValue bailOutReturnValue;
Js::ScriptFunction * innerMostInlinee = nullptr;
BailOutInlinedHelper(layout, currentBailOutRecord, bailOutOffset, returnAddress, bailOutKind, registerSaves, &bailOutReturnValue, &innerMostInlinee, true, branchValue);
bool * hasBailOutBit = layout->functionObject->GetScriptContext()->GetThreadContext()->GetHasBailedOutBitPtr();
if (hasBailOutBit != nullptr && bailOutRecord->ehBailoutData)
{
*hasBailOutBit = true;
}
uint32 result = BailOutFromLoopBodyHelper(layout, currentBailOutRecord, currentBailOutRecord->bailOutOffset,
bailOutKind, nullptr, registerSaves, &bailOutReturnValue);
ScheduleLoopBodyCodeGen(Js::ScriptFunction::FromVar(layout->functionObject), innerMostInlinee, currentBailOutRecord, bailOutKind);
Expand Down
1 change: 1 addition & 0 deletions deps/chakrashim/core/lib/Runtime/Base/ThreadContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ ThreadContext::ThreadContext(AllocationPolicyManager * allocationPolicyManager,
stackProber(nullptr),
isThreadBound(false),
hasThrownPendingException(false),
hasBailedOutBitPtr(nullptr),
pendingFinallyException(nullptr),
noScriptScope(false),
heapEnum(nullptr),
Expand Down
11 changes: 11 additions & 0 deletions deps/chakrashim/core/lib/Runtime/Base/ThreadContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -638,6 +638,7 @@ class ThreadContext sealed :
StackProber * stackProber;
bool isThreadBound;
bool hasThrownPendingException;
bool * hasBailedOutBitPtr;
bool callDispose;
#if ENABLE_JS_REENTRANCY_CHECK
bool noJsReentrancy;
Expand Down Expand Up @@ -1531,6 +1532,16 @@ class ThreadContext sealed :
this->hasThrownPendingException = true;
}

bool * GetHasBailedOutBitPtr()
{
return this->hasBailedOutBitPtr;
}

void SetHasBailedOutBitPtr(bool *setValue)
{
this->hasBailedOutBitPtr = setValue;
}

void SetRecordedException(Js::JavascriptExceptionObject* exceptionObject, bool propagateToDebugger = false)
{
this->recyclableData->exceptionObject = exceptionObject;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,11 @@ namespace Js
void *continuation = nullptr;
JavascriptExceptionObject *exception = nullptr;
void *tryCatchFrameAddr = nullptr;
scriptContext->GetThreadContext()->SetHasBailedOutBitPtr((bool*)((char*)frame + hasBailedOutOffset));

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

try
{
Js::JavascriptExceptionOperators::AutoCatchHandlerExists autoCatchHandlerExists(scriptContext);
Expand Down Expand Up @@ -129,18 +130,22 @@ namespace Js

exception = exception->CloneIfStaticExceptionObject(scriptContext);
bool hasBailedOut = *(bool*)((char*)frame + hasBailedOutOffset); // stack offsets are negative
// If an inlinee bailed out due to some reason, the execution of the current function enclosing the try catch will also continue in the interpreter
// During execution in the interpreter, if we throw outside the region enclosed in try/catch, this catch ends up catching that exception because its present on the call stack
if (hasBailedOut)
{
// If we have bailed out, this exception is coming from the interpreter. It should not have been caught;
// it so happens that this catch was on the stack and caught the exception.
// Re-throw!
scriptContext->GetThreadContext()->SetHasBailedOutBitPtr(nullptr);
JavascriptExceptionOperators::DoThrow(exception, scriptContext);
}
}

Var exceptionObject = exception->GetThrownObject(scriptContext);
AssertMsg(exceptionObject, "Caught object is null.");
continuation = amd64_CallWithFakeFrame(catchAddr, frame, spillSize, argsSize, exceptionObject);
}

scriptContext->GetThreadContext()->SetHasBailedOutBitPtr(nullptr);
return continuation;
}

Expand All @@ -154,6 +159,7 @@ namespace Js
{
void *tryContinuation = nullptr;
JavascriptExceptionObject *exception = nullptr;
scriptContext->GetThreadContext()->SetHasBailedOutBitPtr((bool*)((char*)frame + hasBailedOutOffset));

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

Expand Down Expand Up @@ -189,6 +195,7 @@ namespace Js
// If we have bailed out, this exception is coming from the interpreter. It should not have been caught;
// it so happens that this catch was on the stack and caught the exception.
// Re-throw!
scriptContext->GetThreadContext()->SetHasBailedOutBitPtr(nullptr);
JavascriptExceptionOperators::DoThrow(exception, scriptContext);
}

Expand All @@ -197,6 +204,7 @@ namespace Js
return continuation;
}

scriptContext->GetThreadContext()->SetHasBailedOutBitPtr(nullptr);
return tryContinuation;
}

Expand Down Expand Up @@ -250,6 +258,7 @@ namespace Js
void *continuation = nullptr;
JavascriptExceptionObject *exception = nullptr;
void * tryCatchFrameAddr = nullptr;
scriptContext->GetThreadContext()->SetHasBailedOutBitPtr((bool*)((char*)localsPtr + hasBailedOutOffset));

PROBE_STACK(scriptContext, Constants::MinStackDefault + argsSize);
{
Expand Down Expand Up @@ -295,8 +304,10 @@ namespace Js
// If we have bailed out, this exception is coming from the interpreter. It should not have been caught;
// it so happens that this catch was on the stack and caught the exception.
// Re-throw!
scriptContext->GetThreadContext()->SetHasBailedOutBitPtr(nullptr);
JavascriptExceptionOperators::DoThrow(exception, scriptContext);
}

Var exceptionObject = exception->GetThrownObject(scriptContext);
AssertMsg(exceptionObject, "Caught object is null.");
#if defined(_M_ARM)
Expand All @@ -306,6 +317,7 @@ namespace Js
#endif
}

scriptContext->GetThreadContext()->SetHasBailedOutBitPtr(nullptr);
return continuation;
}

Expand All @@ -320,9 +332,9 @@ namespace Js
{
void *tryContinuation = nullptr;
JavascriptExceptionObject *exception = nullptr;
scriptContext->GetThreadContext()->SetHasBailedOutBitPtr((bool*)((char*)localsPtr + hasBailedOutOffset));

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

try
{
#if defined(_M_ARM)
Expand Down Expand Up @@ -355,8 +367,10 @@ namespace Js
// If we have bailed out, this exception is coming from the interpreter. It should not have been caught;
// it so happens that this catch was on the stack and caught the exception.
// Re-throw!
scriptContext->GetThreadContext()->SetHasBailedOutBitPtr(nullptr);
JavascriptExceptionOperators::DoThrow(exception, scriptContext);
}

scriptContext->GetThreadContext()->SetPendingFinallyException(exception);
#if defined(_M_ARM)
void * finallyContinuation = arm_CallEhFrame(finallyAddr, framePtr, localsPtr, argsSize);
Expand All @@ -366,6 +380,7 @@ namespace Js
return finallyContinuation;
}

scriptContext->GetThreadContext()->SetHasBailedOutBitPtr(nullptr);
return tryContinuation;
}

Expand Down Expand Up @@ -429,6 +444,7 @@ namespace Js
void* continuationAddr = NULL;
Js::JavascriptExceptionObject* pExceptionObject = NULL;
void *tryCatchFrameAddr = nullptr;
scriptContext->GetThreadContext()->SetHasBailedOutBitPtr((bool*)((char*)framePtr + hasBailedOutOffset));

PROBE_STACK(scriptContext, Constants::MinStackDefault);
{
Expand Down Expand Up @@ -526,8 +542,10 @@ namespace Js
// If we have bailed out, this exception is coming from the interpreter. It should not have been caught;
// it so happens that this catch was on the stack and caught the exception.
// Re-throw!
scriptContext->GetThreadContext()->SetHasBailedOutBitPtr(nullptr);
JavascriptExceptionOperators::DoThrow(pExceptionObject, scriptContext);
}

Var catchObject = pExceptionObject->GetThrownObject(scriptContext);
AssertMsg(catchObject, "Caught object is NULL");
#ifdef _M_IX86
Expand Down Expand Up @@ -581,14 +599,15 @@ namespace Js
#endif
}

scriptContext->GetThreadContext()->SetHasBailedOutBitPtr(nullptr);
return continuationAddr;
}

void* JavascriptExceptionOperators::OP_TryFinally(void* tryAddr, void* handlerAddr, void* framePtr, int hasBailedOutOffset, ScriptContext *scriptContext)
{
Js::JavascriptExceptionObject* pExceptionObject = NULL;
void* continuationAddr = NULL;

scriptContext->GetThreadContext()->SetHasBailedOutBitPtr((bool*)((char*)framePtr + hasBailedOutOffset));
PROBE_STACK(scriptContext, Constants::MinStackDefault);

try
Expand Down Expand Up @@ -676,8 +695,10 @@ namespace Js
// If we have bailed out, this exception is coming from the interpreter. It should not have been caught;
// it so happens that this catch was on the stack and caught the exception.
// Re-throw!
scriptContext->GetThreadContext()->SetHasBailedOutBitPtr(nullptr);
JavascriptExceptionOperators::DoThrow(pExceptionObject, scriptContext);
}

scriptContext->GetThreadContext()->SetPendingFinallyException(pExceptionObject);

void* newContinuationAddr = NULL;
Expand Down Expand Up @@ -733,6 +754,8 @@ namespace Js
#endif
return newContinuationAddr;
}

scriptContext->GetThreadContext()->SetHasBailedOutBitPtr(nullptr);
return continuationAddr;
}

Expand Down
1 change: 1 addition & 0 deletions deps/chakrashim/core/test/EH/hasBailedOutBug.baseline
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
TypeError: Assignment to read-only properties is not allowed in strict mode
48 changes: 48 additions & 0 deletions deps/chakrashim/core/test/EH/hasBailedOutBug.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
//-------------------------------------------------------------------------------------------------------
// Copyright (C) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
//-------------------------------------------------------------------------------------------------------

var shouldBailout = false;
function test0() {
var obj0 = {};
var func0 = function () {
};
var func1 = function () {
(function () {
'use strict';
try {
function func8() {
obj0.prop2;
}
var uniqobj4 = func8();
} catch (ex) {
return 'somestring';
} finally {
}
func0(ary.push(ary.unshift(Object.prototype.length = protoObj0)));
}(shouldBailout ? (Object.defineProperty(Object.prototype, 'length', {
get: function () {
}
})) : arguments));
};
var ary = Array();
var protoObj0 = Object();
({
prop7: shouldBailout ? (Object.defineProperty(obj0, 'prop2', {
set: function () {
}
})) : Object
});
for (; func1(); ) {
}
}
test0();
test0();
shouldBailout = true;
try {
test0();
}
catch(ex) {
print(ex);
}
6 changes: 6 additions & 0 deletions deps/chakrashim/core/test/EH/rlexe.xml
Original file line number Diff line number Diff line change
Expand Up @@ -176,4 +176,10 @@
<compile-flags> -force:inline </compile-flags>
</default>
</test>
<test>
<default>
<files>hasBailedOutBug.js</files>
<baseline>hasBailedOutBug.baseline</baseline>
</default>
</test>
</regress-exe>

0 comments on commit 246f843

Please sign in to comment.