Skip to content

Commit d119260

Browse files
author
meg-gupta
committed
Clear inlinee callinfo
When there is an exception in an inlinee which is inside an EH region, we end up leaving datastructures in stackwalker dirty. If the EH handler was a try catch, and if the same code is executed in a loop in the interprerter after BailOnNoException, the stack walker due to dirty datastructures assumes that there are inlinee frames on the stack.
1 parent cec77d7 commit d119260

9 files changed

+192
-4
lines changed

‎lib/Runtime/Base/ThreadContext.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ ThreadContext::ThreadContext(AllocationPolicyManager * allocationPolicyManager,
125125
entryExitRecord(nullptr),
126126
leafInterpreterFrame(nullptr),
127127
threadServiceWrapper(nullptr),
128+
tryCatchFrameAddr(nullptr),
128129
temporaryArenaAllocatorCount(0),
129130
temporaryGuestArenaAllocatorCount(0),
130131
crefSContextForDiag(0),

‎lib/Runtime/Base/ThreadContext.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -678,7 +678,7 @@ class ThreadContext sealed :
678678
ThreadServiceWrapper* threadServiceWrapper;
679679
uint functionCount;
680680
uint sourceInfoCount;
681-
681+
void * tryCatchFrameAddr;
682682
enum RedeferralState
683683
{
684684
InitialRedeferralState,
@@ -1253,6 +1253,9 @@ class ThreadContext sealed :
12531253
uint EnterScriptStart(Js::ScriptEntryExitRecord *, bool doCleanup);
12541254
void EnterScriptEnd(Js::ScriptEntryExitRecord *, bool doCleanup);
12551255

1256+
void * GetTryCatchFrameAddr() { return this->tryCatchFrameAddr; }
1257+
void SetTryCatchFrameAddr(void * frameAddr) { this->tryCatchFrameAddr = frameAddr; }
1258+
12561259
template <bool leaveForHost>
12571260
void LeaveScriptStart(void *);
12581261
template <bool leaveForHost>

‎lib/Runtime/Language/JavascriptExceptionOperators.cpp

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,18 @@ namespace Js
6060
m_threadContext->SetIsUserCode(m_previousCatchHandlerToUserCodeStatus);
6161
}
6262

63+
JavascriptExceptionOperators::TryCatchFrameAddrStack::TryCatchFrameAddrStack(ScriptContext* scriptContext, void *frameAddr)
64+
{
65+
m_threadContext = scriptContext->GetThreadContext();
66+
m_prevTryCatchFrameAddr = m_threadContext->GetTryCatchFrameAddr();
67+
scriptContext->GetThreadContext()->SetTryCatchFrameAddr(frameAddr);
68+
}
69+
70+
JavascriptExceptionOperators::TryCatchFrameAddrStack::~TryCatchFrameAddrStack()
71+
{
72+
m_threadContext->SetTryCatchFrameAddr(m_prevTryCatchFrameAddr);
73+
}
74+
6375
bool JavascriptExceptionOperators::CrawlStackForWER(Js::ScriptContext& scriptContext)
6476
{
6577
return Js::Configuration::Global.flags.WERExceptionSupport && !scriptContext.GetThreadContext()->HasCatchHandler();
@@ -87,6 +99,7 @@ namespace Js
8799
try
88100
{
89101
Js::JavascriptExceptionOperators::AutoCatchHandlerExists autoCatchHandlerExists(scriptContext);
102+
Js::JavascriptExceptionOperators::TryCatchFrameAddrStack tryCatchFrameAddrStack(scriptContext, frame);
90103
continuation = amd64_CallWithFakeFrame(tryAddr, frame, spillSize, argsSize);
91104
}
92105
catch (const Js::JavascriptException& err)
@@ -215,6 +228,7 @@ namespace Js
215228
try
216229
{
217230
Js::JavascriptExceptionOperators::AutoCatchHandlerExists autoCatchHandlerExists(scriptContext);
231+
Js::JavascriptExceptionOperators::TryCatchFrameAddrStack tryCatchFrameAddrStack(scriptContext, framePtr);
218232
#if defined(_M_ARM)
219233
continuation = arm_CallEhFrame(tryAddr, framePtr, localsPtr, argsSize);
220234
#elif defined(_M_ARM64)
@@ -365,6 +379,7 @@ namespace Js
365379
try
366380
{
367381
Js::JavascriptExceptionOperators::AutoCatchHandlerExists autoCatchHandlerExists(scriptContext);
382+
Js::JavascriptExceptionOperators::TryCatchFrameAddrStack tryCatchFrameAddrStack(scriptContext, framePtr);
368383

369384
// Adjust the frame pointer and call into the try.
370385
// If the try completes without raising an exception, it will pass back the continuation address.
@@ -875,7 +890,16 @@ namespace Js
875890

876891
JavascriptExceptionOperators::ThrowExceptionObject(exceptionObject, scriptContext, /*considerPassingToDebugger=*/ true, /*returnAddress=*/ nullptr, resetStack);
877892
}
893+
#if ENABLE_NATIVE_CODEGEN
894+
void JavascriptExceptionOperators::WalkStackForCleaningUpInlineeInfo(ScriptContext *scriptContext, PVOID returnAddress)
895+
{
896+
JavascriptStackWalker walker(scriptContext, true, returnAddress);
878897

898+
// We have to walk the inlinee frames and clear callinfo count on them on an exception
899+
// At this point inlinedFrameWalker is closed, so we should build it again by calling InlinedFrameWalker::FromPhysicalFrame
900+
walker.WalkAndClearInlineeFrameCallInfoOnException();
901+
}
902+
#endif
879903
void
880904
JavascriptExceptionOperators::WalkStackForExceptionContext(ScriptContext& scriptContext, JavascriptExceptionContext& exceptionContext, Var thrownObject, uint64 stackCrawlLimit, PVOID returnAddress, bool isThrownException, bool resetSatck)
881905
{
@@ -1090,6 +1114,20 @@ namespace Js
10901114
WalkStackForExceptionContext(*scriptContext, exceptionContext, thrownObject, StackCrawlLimitOnThrow(thrownObject, *scriptContext), returnAddress, /*isThrownException=*/ true, resetStack);
10911115
exceptionObject->FillError(exceptionContext, scriptContext);
10921116
AddStackTraceToObject(thrownObject, exceptionContext.GetStackTrace(), *scriptContext, /*isThrownException=*/ true, resetStack);
1117+
1118+
// We need to clear callinfo on inlinee virtual frames on an exception.
1119+
// We now allow inlining of functions into callers that have try-catch/try-finally.
1120+
// When there is an exception inside the inlinee with caller having a try-catch, clear the inlinee callinfo by walking the stack.
1121+
// If not, we might have the try-catch inside a loop, and when we execute the loop next time in the interpreter on BailOnException,
1122+
// we will see inlined frames as being present even though they are not, because we depend on FrameInfo's callinfo to tell if an inlinee is on the stack,
1123+
// and we haven't cleared those bits due to the exception
1124+
1125+
#if ENABLE_NATIVE_CODEGEN
1126+
if (scriptContext->GetThreadContext()->GetTryCatchFrameAddr() != nullptr)
1127+
{
1128+
WalkStackForCleaningUpInlineeInfo(scriptContext, returnAddress);
1129+
}
1130+
#endif
10931131
}
10941132
Assert(!scriptContext ||
10951133
// If we disabled implicit calls and we did record an implicit call, do not throw.

‎lib/Runtime/Language/JavascriptExceptionOperators.h

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,17 @@ namespace Js
4343
~AutoCatchHandlerExists();
4444
};
4545

46+
class TryCatchFrameAddrStack
47+
{
48+
private:
49+
void * m_prevTryCatchFrameAddr;
50+
ThreadContext* m_threadContext;
51+
52+
public:
53+
TryCatchFrameAddrStack(ScriptContext* scriptContext, void *frameAddr);
54+
~TryCatchFrameAddrStack();
55+
};
56+
4657
static void __declspec(noreturn) OP_Throw(Var object, ScriptContext* scriptContext);
4758
static void __declspec(noreturn) Throw(Var object, ScriptContext* scriptContext);
4859
static void __declspec(noreturn) ThrowExceptionObject(Js::JavascriptExceptionObject* exceptionObject, ScriptContext* scriptContext, bool considerPassingToDebugger = false, PVOID returnAddress = NULL, bool resetStack = false);
@@ -80,6 +91,9 @@ namespace Js
8091
static Var ThrowTypeErrorRestrictedPropertyAccessor(RecyclableObject* function, CallInfo callInfo, ...);
8192
static Var StackTraceAccessor(RecyclableObject* function, CallInfo callInfo, ...);
8293
static void WalkStackForExceptionContext(ScriptContext& scriptContext, JavascriptExceptionContext& exceptionContext, Var thrownObject, uint64 stackCrawlLimit, PVOID returnAddress, bool isThrownException = true, bool resetSatck = false);
94+
#if ENABLE_NATIVE_CODEGEN
95+
static void WalkStackForCleaningUpInlineeInfo(ScriptContext *scriptContext, PVOID returnAddress);
96+
#endif
8397
static void AddStackTraceToObject(Var obj, JavascriptExceptionContext::StackTrace* stackTrace, ScriptContext& scriptContext, bool isThrownException = true, bool resetSatck = false);
8498
static uint64 StackCrawlLimitOnThrow(Var thrownObject, ScriptContext& scriptContext);
8599

‎lib/Runtime/Language/JavascriptStackWalker.cpp

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -616,7 +616,34 @@ namespace Js
616616
}
617617
return nullptr;
618618
}
619-
619+
#if ENABLE_NATIVE_CODEGEN
620+
void JavascriptStackWalker::WalkAndClearInlineeFrameCallInfoOnException()
621+
{
622+
// Walk the stack and when we find the first JavascriptFrame, we clear the inlinee's callinfo for this frame
623+
// It is sufficient we stop at the first Javascript frame which had the enclosing try-catch
624+
while (this->Walk(true))
625+
{
626+
if (this->IsJavascriptFrame())
627+
{
628+
if (!this->isNativeLibraryFrame)
629+
{
630+
if (HasInlinedFramesOnStack())
631+
{
632+
for (int index = inlinedFrameWalker.GetFrameCount() - 1; index >= 0; index--)
633+
{
634+
auto inlinedFrame = inlinedFrameWalker.GetFrameAtIndex(index);
635+
inlinedFrame->callInfo.Clear();
636+
}
637+
}
638+
if (this->currentFrame.GetFrame() == this->scriptContext->GetThreadContext()->GetTryCatchFrameAddr())
639+
{
640+
break;
641+
}
642+
}
643+
}
644+
}
645+
}
646+
#endif
620647
// Note: noinline is to make sure that when we unwind to the unwindToAddress, there is at least one frame to unwind.
621648
_NOINLINE
622649
JavascriptStackWalker::JavascriptStackWalker(ScriptContext * scriptContext, bool useEERContext, PVOID returnAddress, bool _forceFullWalk /*=false*/) :

‎lib/Runtime/Language/JavascriptStackWalker.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ namespace Js
114114
uint32 GetBottomMostInlineeOffset() const;
115115
Js::JavascriptFunction *GetBottomMostFunctionObject() const;
116116
void FinalizeStackValues(__in_ecount(argCount) Js::Var args[], size_t argCount) const;
117+
int32 GetFrameCount() { return frameCount; }
117118

118119
private:
119120
enum {
@@ -135,11 +136,13 @@ namespace Js
135136

136137
};
137138

138-
void Initialize(int32 frameCount, __in_ecount(frameCount) InlinedFrame **frames, Js::ScriptFunction *parent);
139+
public:
140+
InlinedFrame *const GetFrameAtIndex(signed index) const;
139141

142+
private:
143+
void Initialize(int32 frameCount, __in_ecount(frameCount) InlinedFrame **frames, Js::ScriptFunction *parent);
140144
void MoveNext();
141145
InlinedFrame *const GetCurrentFrame() const;
142-
InlinedFrame *const GetFrameAtIndex(signed index) const;
143146

144147
Js::ScriptFunction *parentFunction;
145148
InlinedFrame **frames;
@@ -233,6 +236,7 @@ namespace Js
233236
void ClearCachedInternalFrameInfo();
234237
void SetCachedInternalFrameInfo(InternalFrameType frameType, JavascriptFunction* function, bool hasInlinedFramesOnStack, bool prevIntFrameIsFromBailout);
235238
InternalFrameInfo GetCachedInternalFrameInfo() const { return this->lastInternalFrameInfo; }
239+
void WalkAndClearInlineeFrameCallInfoOnException();
236240
#endif
237241
bool IsCurrentPhysicalFrameForLoopBody() const;
238242

‎test/EH/inlinestackwalkbug.js

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
//-------------------------------------------------------------------------------------------------------
2+
// Copyright (C) Microsoft. All rights reserved.
3+
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
4+
//-------------------------------------------------------------------------------------------------------
5+
6+
function shapeyConstructor() {
7+
y = iczqcn;
8+
}
9+
10+
function test1() {
11+
for (var w in [1,2]) {
12+
try {
13+
new shapeyConstructor(w);
14+
} catch (e) {
15+
}
16+
}
17+
}
18+
19+
function throwFunc() {
20+
// dummy try-catch so that this function does not get inlined
21+
try {
22+
}
23+
catch (ex) {
24+
}
25+
throw "ex" ;
26+
}
27+
28+
function caller() {
29+
throwFunc(w);
30+
}
31+
32+
function test2() {
33+
for (var w in [1,2]) {
34+
try {
35+
new caller();
36+
} catch (e) {
37+
}
38+
}
39+
}
40+
41+
test1();
42+
test2();
43+
WScript.Echo("PASS");
44+
45+

‎test/EH/nestedinlinestackwalkbug.js

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
//-------------------------------------------------------------------------------------------------------
2+
// Copyright (C) Microsoft. All rights reserved.
3+
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
4+
//-------------------------------------------------------------------------------------------------------
5+
6+
function throwFunc() {
7+
// dummy try-catch so that this function does not get inlined
8+
try {
9+
}
10+
catch (ex) {
11+
}
12+
throw "ex" ;
13+
}
14+
15+
function caller() {
16+
throwFunc(w);
17+
}
18+
19+
function shapeyConstructor() {
20+
y = iczqcn;
21+
}
22+
23+
function test() {
24+
for (var w in [1,2]) {
25+
try {
26+
new caller();
27+
} catch (e) {
28+
}
29+
}
30+
}
31+
32+
function toptest() {
33+
try {
34+
test();
35+
new shapeyConstructor();
36+
}
37+
catch (ex) {
38+
}
39+
}
40+
41+
toptest();
42+
toptest();
43+
toptest();
44+
WScript.Echo("PASS");
45+
46+

‎test/EH/rlexe.xml

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,4 +135,14 @@
135135
<compile-flags> -force:inline </compile-flags>
136136
</default>
137137
</test
138+
<test>
139+
<default>
140+
<files>inlinestackwalkbug.js</files>
141+
</default>
142+
</test>
143+
<test>
144+
<default>
145+
<files>nestedinlinestackwalkbug.js</files>
146+
</default>
147+
</test>
138148
</regress-exe>

0 commit comments

Comments
 (0)