[MERGE #1720 @agarwal-sandeep] Debugging: Top frame should be the frame on which break happened

Merge pull request #1720 from agarwal-sandeep:portfixto13

Porting fix from master to release/1.3

JsRT debugging, shim code can execute after break in user code and it may
not be the top frame when stack walker happens. Top frame should be the
frame on which break happened and set accordingly.

lib/Jsrt/JsrtDebuggerObject.cpp

@@ -851,6 +851,7 @@ Js::JavascriptArray * JsrtDebugStackFrames::StackFrames(Js::ScriptContext * scri
 
         stackList->Map([&](int index, Js::DiagStackFrame* stackFrame)
         {
+            AssertMsg(index != 0 || stackFrame->IsTopFrame(), "Index 0 frame is not marked as top frame");
             Js::DynamicObject* stackTraceObject = this->GetStackFrame(stackFrame, index);
             Js::Var marshaledObj = Js::CrossSite::MarshalVar(scriptContext, stackTraceObject);
             stackTraceArray->DirectSetItemAt(index, marshaledObj);

lib/Runtime/Debug/DebugManager.cpp

@@ -107,6 +107,14 @@ namespace Js
         }
     }
 
+    bool DebugManager::IsMatchTopFrameStackAddress(DiagStackFrame* frame) const
+    {
+        return (frame != nullptr) && 
+            (this->pCurrentInterpreterLocation != nullptr) &&
+            (this->pCurrentInterpreterLocation->topFrame != nullptr) &&
+            (this->pCurrentInterpreterLocation->topFrame->GetStackAddress() == frame->GetStackAddress());
+    }
+
 #ifdef ENABLE_MUTATION_BREAKPOINT
     MutationBreakpoint* DebugManager::GetActiveMutationBreakpoint() const
     {

lib/Runtime/Debug/DebugManager.h

@@ -54,6 +54,7 @@ namespace Js
         DWORD_PTR AllocateSecondaryHostSourceContext();
         void SetCurrentInterpreterLocation(InterpreterHaltState* pHaltState);
         void UnsetCurrentInterpreterLocation();
+        bool IsMatchTopFrameStackAddress(DiagStackFrame* frame) const;
         uint32 GetDebugSessionNumber() const { return debugSessionNumber; }
 #ifdef ENABLE_MUTATION_BREAKPOINT
         MutationBreakpoint* GetActiveMutationBreakpoint() const;

lib/Runtime/Debug/DiagStackFrame.cpp

@@ -9,16 +9,20 @@
 
 namespace Js
 {
-    DiagStackFrame::DiagStackFrame(int frameIndex) :
-        frameIndex(frameIndex)
+    DiagStackFrame::DiagStackFrame():
+        isTopFrame(false)
     {
-        Assert(frameIndex >= 0);
     }
 
     // Returns whether or not this frame is on the top of the callstack.
     bool DiagStackFrame::IsTopFrame()
     {
-        return this->frameIndex == 0 && GetScriptContext()->GetDebugContext()->GetProbeContainer()->IsPrimaryBrokenToDebuggerContext();
+        return this->isTopFrame && GetScriptContext()->GetDebugContext()->GetProbeContainer()->IsPrimaryBrokenToDebuggerContext();
+    }
+
+    void DiagStackFrame::SetIsTopFrame()
+    {
+        this->isTopFrame = true;
     }
 
     ScriptFunction* DiagStackFrame::GetScriptFunction()
@@ -394,8 +398,7 @@ namespace Js
         return GetNonVarRegValue(location);
     }
 
-    DiagInterpreterStackFrame::DiagInterpreterStackFrame(InterpreterStackFrame* frame, int frameIndex) :
-        DiagStackFrame(frameIndex),
+    DiagInterpreterStackFrame::DiagInterpreterStackFrame(InterpreterStackFrame* frame) :
         m_interpreterFrame(frame)
     {
         Assert(m_interpreterFrame != NULL);
@@ -475,9 +478,7 @@ namespace Js
         ScriptFunction* function,
         int byteCodeOffset,
         void* stackAddr,
-        void *codeAddr,
-        int frameIndex) :
-        DiagStackFrame(frameIndex),
+        void *codeAddr) :
         m_function(function),
         m_byteCodeOffset(byteCodeOffset),
         m_stackAddr(stackAddr),
@@ -600,8 +601,7 @@ namespace Js
 #endif
 
 
-    DiagRuntimeStackFrame::DiagRuntimeStackFrame(JavascriptFunction* function, PCWSTR displayName, void* stackAddr, int frameIndex):
-        DiagStackFrame(frameIndex),
+    DiagRuntimeStackFrame::DiagRuntimeStackFrame(JavascriptFunction* function, PCWSTR displayName, void* stackAddr):
         m_function(function),
         m_displayName(displayName),
         m_stackAddr(stackAddr)

lib/Runtime/Debug/DiagStackFrame.h

@@ -36,6 +36,7 @@ namespace Js
         virtual Var GetInnerScopeFromRegSlot(RegSlot location);
 
         bool IsTopFrame();
+        void SetIsTopFrame();
         ScriptFunction* GetScriptFunction();
         FunctionBody* GetFunction();
 
@@ -49,10 +50,10 @@ namespace Js
         Js::Var DoEval(Js::ScriptFunction* pfuncScript);
 
     protected:
-        DiagStackFrame(int frameIndex);
+        DiagStackFrame();
 
     private:
-        int frameIndex;
+        bool isTopFrame;
     };
 
     class DiagInterpreterStackFrame : public DiagStackFrame
@@ -60,7 +61,7 @@ namespace Js
         InterpreterStackFrame* m_interpreterFrame;
 
     public:
-        DiagInterpreterStackFrame(InterpreterStackFrame* frame, int frameIndex);
+        DiagInterpreterStackFrame(InterpreterStackFrame* frame);
         virtual JavascriptFunction* GetJavascriptFunction() override;
         virtual ScriptContext* GetScriptContext() override;
         virtual int GetByteCodeOffset() override;
@@ -88,7 +89,7 @@ namespace Js
         static const int32 InvalidOffset = -1;
 
     public:
-        DiagNativeStackFrame(ScriptFunction* function, int byteCodeOffset, void* stackAddr, void *codeAddr, int frameIndex);
+        DiagNativeStackFrame(ScriptFunction* function, int byteCodeOffset, void* stackAddr, void *codeAddr);
         virtual JavascriptFunction* GetJavascriptFunction() override;
         virtual ScriptContext* GetScriptContext() override;
         virtual int GetByteCodeOffset() override;
@@ -112,7 +113,7 @@ namespace Js
         void* m_stackAddr;
 
     public:
-        DiagRuntimeStackFrame(JavascriptFunction* function, PCWSTR displayName, void* stackAddr, int frameIndex);
+        DiagRuntimeStackFrame(JavascriptFunction* function, PCWSTR displayName, void* stackAddr);
         virtual JavascriptFunction* GetJavascriptFunction() override;
         virtual int GetByteCodeOffset() override;
         virtual DWORD_PTR GetStackAddress() override;

lib/Runtime/Debug/ProbeContainer.cpp

@@ -127,7 +127,7 @@ namespace Js
                     {
                         if (dispatchHaltFrameAddress == 0 || interpreterFrame->GetStackAddress() > dispatchHaltFrameAddress)
                         {
-                            frm = Anew(pDiagArena, DiagInterpreterStackFrame, interpreterFrame, frameIndex);
+                            frm = Anew(pDiagArena, DiagInterpreterStackFrame, interpreterFrame);
                         }
                     }
                     else
@@ -139,14 +139,14 @@ namespace Js
                             if (func->IsScriptFunction())
                             {
                                 frm = Anew(pDiagArena, DiagNativeStackFrame,
-                                    ScriptFunction::FromVar(walker.GetCurrentFunction()), walker.GetByteCodeOffset(), stackAddress, walker.GetCurrentCodeAddr(), frameIndex);
+                                    ScriptFunction::FromVar(walker.GetCurrentFunction()), walker.GetByteCodeOffset(), stackAddress, walker.GetCurrentCodeAddr());
                             }
                             else
 #else
                             Assert(!func->IsScriptFunction());
 #endif
                             {
-                                frm = Anew(pDiagArena, DiagRuntimeStackFrame, func, walker.GetCurrentNativeLibraryEntryName(), stackAddress, frameIndex);
+                                frm = Anew(pDiagArena, DiagRuntimeStackFrame, func, walker.GetCurrentNativeLibraryEntryName(), stackAddress);
                             }
                         }
                     }
@@ -160,6 +160,7 @@ namespace Js
 
             return false;
         });
+
         OUTPUT_TRACE(Js::DebuggerPhase, _u("ProbeContainer::UpdateFramePointers: detected %d frames (this=%p, fMatchWithCurrentScriptContext=%d)\n"),
             tempFramePointers->Count(), this, fMatchWithCurrentScriptContext);
 
@@ -175,6 +176,12 @@ namespace Js
         {
             UpdateFramePointers(/*fMatchWithCurrentScriptContext*/true, dispatchHaltFrameAddress);
             this->debugSessionNumber = debugManager->GetDebugSessionNumber();
+
+            if ((framePointers->Count() > 0) &&
+                debugManager->IsMatchTopFrameStackAddress(framePointers->Peek(0)))
+            {
+                framePointers->Peek(0)->SetIsTopFrame();
+            }
         }
 
         ReferencedArenaAdapter* pRefArena = debugManager->GetDiagnosticArena();
@@ -195,6 +202,7 @@ namespace Js
         if (pHaltState->framePointers->Count() > 0)
         {
             pHaltState->topFrame = pHaltState->framePointers->Peek(0);
+            pHaltState->topFrame->SetIsTopFrame();
         }
 
         OUTPUT_TRACE(Js::DebuggerPhase, _u("ProbeContainer::InitializeLocation (end): this=%p, pHaltState=%p, fMatch=%d, topFrame=%p\n"),