Skip to content

Commit

Permalink
Clear inlinee callinfo
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
meg-gupta committed Sep 28, 2017
1 parent c626cf7 commit 932131e
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 1 deletion.
17 changes: 17 additions & 0 deletions lib/Runtime/Language/JavascriptExceptionOperators.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -876,6 +876,15 @@ namespace Js
JavascriptExceptionOperators::ThrowExceptionObject(exceptionObject, scriptContext, /*considerPassingToDebugger=*/ true, /*returnAddress=*/ nullptr, resetStack);
}

void JavascriptExceptionOperators::WalkStackForCleaningUpInlineeInfo(ScriptContext *scriptContext, PVOID returnAddress)
{
JavascriptStackWalker walker(scriptContext, true, returnAddress);

// We have to walk the inlinee frames and clear callinfo count on them on an exception
// At this point inlinedFrameWalker is closed, so we should build it again by calling InlinedFrameWalker::FromPhysicalFrame
walker.WalkAndClearInlineeFrameCallInfoOnException();
}

void
JavascriptExceptionOperators::WalkStackForExceptionContext(ScriptContext& scriptContext, JavascriptExceptionContext& exceptionContext, Var thrownObject, uint64 stackCrawlLimit, PVOID returnAddress, bool isThrownException, bool resetSatck)
{
Expand Down Expand Up @@ -1090,6 +1099,14 @@ namespace Js
WalkStackForExceptionContext(*scriptContext, exceptionContext, thrownObject, StackCrawlLimitOnThrow(thrownObject, *scriptContext), returnAddress, /*isThrownException=*/ true, resetStack);
exceptionObject->FillError(exceptionContext, scriptContext);
AddStackTraceToObject(thrownObject, exceptionContext.GetStackTrace(), *scriptContext, /*isThrownException=*/ true, resetStack);

// We need to clear callinfo on inlinee virtual frames on an exception.
// We now allow inlining of functions into callers that have try-catch/try-finally.
// When there is an exception inside the inlinee with caller having a try-catch, clear the inlinee callinfo by walking the stack.
// If not, we might have the try-catch inside a loop, and when we execute the loop next time in the interpreter on BailOnException,
// 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,
// and we haven't cleared those bits due to the exception
WalkStackForCleaningUpInlineeInfo(scriptContext, returnAddress);
}
Assert(!scriptContext ||
// If we disabled implicit calls and we did record an implicit call, do not throw.
Expand Down
1 change: 1 addition & 0 deletions lib/Runtime/Language/JavascriptExceptionOperators.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ namespace Js
static Var ThrowTypeErrorRestrictedPropertyAccessor(RecyclableObject* function, CallInfo callInfo, ...);
static Var StackTraceAccessor(RecyclableObject* function, CallInfo callInfo, ...);
static void WalkStackForExceptionContext(ScriptContext& scriptContext, JavascriptExceptionContext& exceptionContext, Var thrownObject, uint64 stackCrawlLimit, PVOID returnAddress, bool isThrownException = true, bool resetSatck = false);
static void WalkStackForCleaningUpInlineeInfo(ScriptContext *scriptContext, PVOID returnAddress);
static void AddStackTraceToObject(Var obj, JavascriptExceptionContext::StackTrace* stackTrace, ScriptContext& scriptContext, bool isThrownException = true, bool resetSatck = false);
static uint64 StackCrawlLimitOnThrow(Var thrownObject, ScriptContext& scriptContext);

Expand Down
23 changes: 23 additions & 0 deletions lib/Runtime/Language/JavascriptStackWalker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -617,6 +617,29 @@ namespace Js
return nullptr;
}

void JavascriptStackWalker::WalkAndClearInlineeFrameCallInfoOnException()
{
// Walk the stack and when we find the first JavascriptFrame, we clear the inlinee's callinfo for this frame
// It is sufficient we stop at the first Javascript frame, because we still don't inline functions that have try-catch/try-finally into other functions
while (this->Walk(true))
{
if (this->IsJavascriptFrame())
{
if (!this->isNativeLibraryFrame)
{
if (HasInlinedFramesOnStack())
{
for (int index = inlinedFrameWalker.frameCount - 1; index >= 0; index--)
{
inlinedFrameWalker.frames[index]->callInfo.Clear();
}
}
break;
}
}
}
}

// Note: noinline is to make sure that when we unwind to the unwindToAddress, there is at least one frame to unwind.
_NOINLINE
JavascriptStackWalker::JavascriptStackWalker(ScriptContext * scriptContext, bool useEERContext, PVOID returnAddress, bool _forceFullWalk /*=false*/) :
Expand Down
3 changes: 2 additions & 1 deletion lib/Runtime/Language/JavascriptStackWalker.h
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ namespace Js
void MoveNext();
InlinedFrame *const GetCurrentFrame() const;
InlinedFrame *const GetFrameAtIndex(signed index) const;

public:
Js::ScriptFunction *parentFunction;
InlinedFrame **frames;
int32 currentIndex;
Expand Down Expand Up @@ -234,6 +234,7 @@ namespace Js
void SetCachedInternalFrameInfo(InternalFrameType frameType, JavascriptFunction* function, bool hasInlinedFramesOnStack, bool prevIntFrameIsFromBailout);
InternalFrameInfo GetCachedInternalFrameInfo() const { return this->lastInternalFrameInfo; }
#endif
void WalkAndClearInlineeFrameCallInfoOnException();
bool IsCurrentPhysicalFrameForLoopBody() const;

// noinline, we want to use own stack frame.
Expand Down

0 comments on commit 932131e

Please sign in to comment.