Debugger support for split param/body scope case during default.

For the split scope we were still using the older way of getting information from the function body when we walk the slot array walker. However that is not true as the propertyidslotarraycontainer was meant for slots for body's variable and also the regslotcontainer will not have right information as well.
Fixed that by
The variable which are copied from param scope to body scope (due to split scope) will be put regslotcontainer if needed.
While walking we need to distinguish that the scope is split and we need to make use of DebuggerScope.
Also the paramscope could be activation object so we need to have DiagParamScopeInObject to distinguish while locals walking.
Linear scan gave the problem as for the split case the formals are duplicated as variables. Our intention was to restore only formals, so I fixed that by limiting the loop till the formalsCount only.

lib/Backend/LinearScan.cpp

@@ -1626,7 +1626,9 @@ LinearScan::FillBailOutRecord(IR::Instr * instr)
 
         if (hasFormalArgs)
         {
-            for (uint32 index = functionBody->GetFirstNonTempLocalIndex(); index < functionBody->GetEndNonTempLocalIndex(); index++)
+            Assert(functionBody->GetInParamsCount() > 0);
+            uint32 endIndex = min(functionBody->GetFirstNonTempLocalIndex() + functionBody->GetInParamsCount() - 1, functionBody->GetEndNonTempLocalIndex());
+            for (uint32 index = functionBody->GetFirstNonTempLocalIndex(); index < endIndex; index++)
             {
                 StackSym * stackSym = this->func->m_symTable->FindStackSym(index);
                 if (stackSym != nullptr)

lib/Runtime/Base/FunctionBody.cpp

@@ -5318,6 +5318,8 @@ namespace Js
             return _u("DiagWithScope");
         case DiagExtraScopesType::DiagParamScope:
             return _u("DiagParamScope");
+        case DiagExtraScopesType::DiagParamScopeInObject:
+            return _u("DiagParamScopeInObject");
         default:
             AssertMsg(false, "Missing a debug scope type.");
             return _u("");
@@ -5493,6 +5495,12 @@ namespace Js
             || this->scopeType == Js::DiagCatchScopeInSlot;
     }
 
+    bool DebuggerScope::IsParamScope() const
+    {
+        return this->scopeType == Js::DiagParamScope
+            || this->scopeType == Js::DiagParamScopeInObject;
+    }
+
     // Gets whether or not the scope has any properties in it.
     bool DebuggerScope::HasProperties() const
     {
@@ -5681,7 +5689,7 @@ namespace Js
         {
             Js::DebuggerScope *debuggerScope = pScopeChain->Item(i);
             DebuggerScopeProperty debuggerScopeProperty;
-            if (debuggerScope->scopeType != DiagParamScope && debuggerScope->TryGetProperty(propertyId, location, &debuggerScopeProperty))
+            if (!debuggerScope->IsParamScope() && debuggerScope->TryGetProperty(propertyId, location, &debuggerScopeProperty))
             {
                 bool isOffsetInScope = debuggerScope->IsOffsetInScope(offset);
 

lib/Runtime/Base/FunctionBody.h

@@ -67,6 +67,7 @@ namespace Js
         DiagBlockScopeInObject,     // Block scope in activation object
         DiagBlockScopeRangeEnd,     // Used to end a block scope range.
         DiagParamScope,             // The scope represents symbols at formals
+        DiagParamScopeInObject,     // The scope represents symbols at formals and formal scope in activation object
     };
 
     class PropertyGuard
@@ -3475,6 +3476,7 @@ namespace Js
         bool IsCatchScope() const;
         bool IsWithScope() const;
         bool IsSlotScope() const;
+        bool IsParamScope() const;
         bool HasProperties() const;
         bool IsAncestorOf(const DebuggerScope* potentialChildScope);
         bool AreAllPropertiesInDeadZone(int byteCodeOffset) const;

lib/Runtime/ByteCode/ByteCodeEmitter.cpp

@@ -2773,7 +2773,7 @@ void ByteCodeGenerator::PopulateFormalsScope(uint beginOffset, FuncInfo *funcInf
         {
             if (debuggerScope == nullptr)
             {
-                debuggerScope = RecordStartScopeObject(pnode, Js::DiagParamScope);
+                debuggerScope = RecordStartScopeObject(pnode, funcInfo->paramScope && funcInfo->paramScope->GetIsObject() ? Js::DiagParamScopeInObject : Js::DiagParamScope);
                 debuggerScope->SetBegin(beginOffset);
             }
 
@@ -3274,7 +3274,7 @@ void ByteCodeGenerator::EmitOneFunction(ParseNode *pnode)
         {
             // Emit bytecode to copy the initial values from param names to their corresponding body bindings.
             // We have to do this after the rest param is marked as false for need declaration.
-            paramScope->ForEachSymbol([this, funcInfo, paramScope](Symbol* param) {
+            paramScope->ForEachSymbol([this, funcInfo, paramScope, byteCodeFunction](Symbol* param) {
                 Symbol* varSym = funcInfo->GetBodyScope()->FindLocalSymbol(param->GetName());
                 Assert(varSym || param->GetIsArguments());
                 Assert(param->GetIsArguments() || param->IsInSlot(funcInfo));
@@ -3289,6 +3289,12 @@ void ByteCodeGenerator::EmitOneFunction(ParseNode *pnode)
                     slot = slot + (paramScope->GetIsObject() ? 0 : Js::ScopeSlots::FirstSlotIndex);
 
                     this->m_writer.SlotI1(op, tempReg, slot, profileId);
+
+                    if (ShouldTrackDebuggerMetadata() && !varSym->GetIsArguments() && !varSym->IsInSlot(funcInfo))
+                    {
+                        byteCodeFunction->InsertSymbolToRegSlotList(varSym->GetName(), varSym->GetLocation(), funcInfo->varRegsCount);
+                    }
+
                     this->EmitPropStore(tempReg, varSym, varSym->GetPid(), funcInfo);
                     funcInfo->ReleaseTmpRegister(tempReg);
                 }

lib/Runtime/Debug/DiagObjectModel.cpp

@@ -546,53 +546,16 @@ namespace Js
         return !*isPropertyInDebuggerScope;
     }
 
-    // Gets an adjusted offset for the current bytecode location based on which stack frame we're in.
-    // If we're in the top frame (leaf node), then the byte code offset should remain as is, to reflect
-    // the current position of the instruction pointer.  If we're not in the top frame, we need to subtract
-    // 1 as the byte code location will be placed at the next statement to be executed at the top frame.
-    // In the case of block scoping, this is an inaccurate location for viewing variables since the next
-    // statement could be beyond the current block scope.  For inspection, we want to remain in the
-    // current block that the function was called from.
-    // An example is this:
-    // function foo() { ... }   // Frame 0 (with breakpoint inside)
-    // function bar() {         // Frame 1
-    //     {
-    //         let a = 0;
-    //         foo(); // <-- Inspecting here, foo is already evaluated.
-    //     }
-    //     foo(); // <-- Byte code offset is now here, so we need to -1 to get back in the block scope.
     int VariableWalkerBase::GetAdjustedByteCodeOffset() const
     {
-        Assert(pFrame);
-        int offset = pFrame->GetByteCodeOffset();
-        if (!pFrame->IsTopFrame() && pFrame->IsInterpreterFrame())
-        {
-            // Native frames are already adjusted so just need to adjust interpreted
-            // frames that are not the top frame.
-            --offset;
-        }
-
-        return offset;
+        return LocalsWalker::GetAdjustedByteCodeOffset(pFrame);
     }
 
     DebuggerScope * VariableWalkerBase::GetScopeWhenHaltAtFormals()
     {
         if (IsWalkerForCurrentFrame())
         {
-            Js::ScopeObjectChain * scopeObjectChain = pFrame->GetJavascriptFunction()->GetFunctionBody()->GetScopeObjectChain();
-
-            if (scopeObjectChain != nullptr && scopeObjectChain->pScopeChain != nullptr)
-            {
-                int currentOffset = GetAdjustedByteCodeOffset();
-                for (int i = 0; i < scopeObjectChain->pScopeChain->Count(); i++)
-                {
-                    Js::DebuggerScope * scope = scopeObjectChain->pScopeChain->Item(i);
-                    if (scope->scopeType == Js::DiagParamScope && scope->GetEnd() > currentOffset)
-                    {
-                        return scope;
-                    }
-                }
-            }
+            return LocalsWalker::GetScopeWhenHaltAtFormals(pFrame);
         }
 
         return nullptr;
@@ -631,13 +594,35 @@ namespace Js
 
             if (slotArray.IsFunctionScopeSlotArray())
             {
+                DebuggerScope *formalScope = GetScopeWhenHaltAtFormals();
                 Js::FunctionBody *pFBody = slotArray.GetFunctionBody();
-                if (pFBody->GetPropertyIdsForScopeSlotArray() != nullptr)
+                uint slotArrayCount = slotArray.GetCount();
+
+                if (formalScope != nullptr && !pFBody->IsParamAndBodyScopeMerged())
                 {
-                    uint slotArrayCount = slotArray.GetCount();
+                    Assert(pFBody->paramScopeSlotArraySize > 0);
                     pMembersList = JsUtil::List<DebuggerPropertyDisplayInfo *, ArenaAllocator>::New(arena, slotArrayCount);
 
-                    DebuggerScope *formalScope = GetScopeWhenHaltAtFormals();
+                    for (ulong i = 0; i < slotArrayCount; i++)
+                    {
+                        Js::DebuggerScopeProperty scopeProperty = formalScope->scopeProperties->Item(i);
+
+                        Var value = slotArray.Get(i);
+                        bool isInDeadZone = pFrame->GetScriptContext()->IsUndeclBlockVar(value);
+
+                        DebuggerPropertyDisplayInfo *pair = AllocateNewPropertyDisplayInfo(
+                            scopeProperty.propId,
+                            value,
+                            false/*isConst*/,
+                            isInDeadZone);
+
+                        Assert(pair != nullptr);
+                        pMembersList->Add(pair);
+                    }
+                }
+                else if (pFBody->GetPropertyIdsForScopeSlotArray() != nullptr)
+                {
+                    pMembersList = JsUtil::List<DebuggerPropertyDisplayInfo *, ArenaAllocator>::New(arena, slotArrayCount);
 
                     for (ulong i = 0; i < slotArrayCount; i++)
                     {
@@ -970,7 +955,7 @@ namespace Js
                 Js::DebuggerScope *debuggerScope = pScopeObjectChain->pScopeChain->Item(i);
                 bool isScopeInRange = debuggerScope->IsOffsetInScope(bytecodeOffset);
                 if (isScopeInRange
-                    && debuggerScope->scopeType != DiagParamScope
+                    && !debuggerScope->IsParamScope()
                     && (debuggerScope->IsOwnScope() || (debuggerScope->scopeType == DiagBlockScopeDirect && debuggerScope->HasProperties())))
                 {
                     switch (debuggerScope->scopeType)
@@ -1107,27 +1092,46 @@ namespace Js
                 pVarWalkers->Add(Anew(arena, RootObjectVariablesWalker, pFrame, pFrame->GetRootObject(), UIGroupType_None));
             }
 
-            DWORD localsType = GetCurrentFramesLocalsType(pFrame);
-            if (localsType & FramesLocalType::LocalType_Reg)
-            {
-                pVarWalkers->Add(Anew(arena, RegSlotVariablesWalker, pFrame, nullptr /*not debugger scope*/, UIGroupType_None, !!(frameWalkerFlags & FrameWalkerFlags::FW_AllowSuperReference)));
-            }
-            if (localsType & FramesLocalType::LocalType_InObject)
-            {
-                Assert(scopeCount > 0);
-                pVarWalker = Anew(arena, ObjectVariablesWalker, pFrame, pDisplay->GetItem(nextStartIndex++), UIGroupType_None, !!(frameWalkerFlags & FrameWalkerFlags::FW_AllowLexicalThis), !!(frameWalkerFlags & FrameWalkerFlags::FW_AllowSuperReference));
-            }
-            else if (localsType & FramesLocalType::LocalType_InSlot)
+            DebuggerScope *formalScope = GetScopeWhenHaltAtFormals(pFrame);
+
+            // If we are halted at formal place, and param and body scopes are splitted we need to make use of formal debugger scope to walk those variables.
+            if (!pFBody->IsParamAndBodyScopeMerged() && formalScope != nullptr)
             {
                 Assert(scopeCount > 0);
-                pVarWalker = Anew(arena, SlotArrayVariablesWalker, pFrame, (Js::Var *)pDisplay->GetItem(nextStartIndex++), UIGroupType_None, !!(frameWalkerFlags & FrameWalkerFlags::FW_AllowLexicalThis), !!(frameWalkerFlags & FrameWalkerFlags::FW_AllowSuperReference));
+                if (formalScope->scopeType == Js::DiagParamScopeInObject)
+                {
+                    pVarWalker = Anew(arena, ObjectVariablesWalker, pFrame, (Js::Var *)pDisplay->GetItem(nextStartIndex++), UIGroupType_None, !!(frameWalkerFlags & FrameWalkerFlags::FW_AllowLexicalThis), !!(frameWalkerFlags & FrameWalkerFlags::FW_AllowSuperReference));
+                }
+                else
+                {
+                    Assert(pFBody->paramScopeSlotArraySize > 0);
+                    pVarWalker = Anew(arena, SlotArrayVariablesWalker, pFrame, (Js::Var *)pDisplay->GetItem(nextStartIndex++), UIGroupType_None, !!(frameWalkerFlags & FrameWalkerFlags::FW_AllowLexicalThis), !!(frameWalkerFlags & FrameWalkerFlags::FW_AllowSuperReference));
+                }
             }
-            else if (scopeCount > 0 && pFBody->GetFrameDisplayRegister() != 0)
+            else
             {
-                Assert((Var)pDisplay->GetItem(0) == pFrame->GetScriptContext()->GetLibrary()->GetNull());
+                DWORD localsType = GetCurrentFramesLocalsType(pFrame);
+                if (localsType & FramesLocalType::LocalType_Reg)
+                {
+                    pVarWalkers->Add(Anew(arena, RegSlotVariablesWalker, pFrame, nullptr /*not debugger scope*/, UIGroupType_None, !!(frameWalkerFlags & FrameWalkerFlags::FW_AllowSuperReference)));
+                }
+                if (localsType & FramesLocalType::LocalType_InObject)
+                {
+                    Assert(scopeCount > 0);
+                    pVarWalker = Anew(arena, ObjectVariablesWalker, pFrame, pDisplay->GetItem(nextStartIndex++), UIGroupType_None, !!(frameWalkerFlags & FrameWalkerFlags::FW_AllowLexicalThis), !!(frameWalkerFlags & FrameWalkerFlags::FW_AllowSuperReference));
+                }
+                else if (localsType & FramesLocalType::LocalType_InSlot)
+                {
+                    Assert(scopeCount > 0);
+                    pVarWalker = Anew(arena, SlotArrayVariablesWalker, pFrame, (Js::Var *)pDisplay->GetItem(nextStartIndex++), UIGroupType_None, !!(frameWalkerFlags & FrameWalkerFlags::FW_AllowLexicalThis), !!(frameWalkerFlags & FrameWalkerFlags::FW_AllowSuperReference));
+                }
+                else if (scopeCount > 0 && pFBody->GetFrameDisplayRegister() != 0 )
+                {
+                    Assert((Var)pDisplay->GetItem(0) == pFrame->GetScriptContext()->GetLibrary()->GetNull() || !pFBody->IsParamAndBodyScopeMerged());
 
-                // A dummy scope with nullptr register is created. Skip this.
-                nextStartIndex++;
+                    // A dummy scope with nullptr register is created. Skip this.
+                    nextStartIndex++;
+                }
             }
 
             if (pVarWalker)
@@ -1366,6 +1370,55 @@ namespace Js
         return totalLocalsCount;
     }
 
+    /*static*/
+    DebuggerScope * LocalsWalker::GetScopeWhenHaltAtFormals(DiagStackFrame* frame)
+    {
+        Js::ScopeObjectChain * scopeObjectChain = frame->GetJavascriptFunction()->GetFunctionBody()->GetScopeObjectChain();
+
+        if (scopeObjectChain != nullptr && scopeObjectChain->pScopeChain != nullptr)
+        {
+            int currentOffset = GetAdjustedByteCodeOffset(frame);
+            for (int i = 0; i < scopeObjectChain->pScopeChain->Count(); i++)
+            {
+                Js::DebuggerScope * scope = scopeObjectChain->pScopeChain->Item(i);
+                if (scope->IsParamScope() && scope->GetEnd() > currentOffset)
+                {
+                    return scope;
+                }
+            }
+        }
+
+        return nullptr;
+    }
+
+    // Gets an adjusted offset for the current bytecode location based on which stack frame we're in.
+    // If we're in the top frame (leaf node), then the byte code offset should remain as is, to reflect
+    // the current position of the instruction pointer.  If we're not in the top frame, we need to subtract
+    // 1 as the byte code location will be placed at the next statement to be executed at the top frame.
+    // In the case of block scoping, this is an inaccurate location for viewing variables since the next
+    // statement could be beyond the current block scope.  For inspection, we want to remain in the
+    // current block that the function was called from.
+    // An example is this:
+    // function foo() { ... }   // Frame 0 (with breakpoint inside)
+    // function bar() {         // Frame 1
+    //     {
+    //         let a = 0;
+    //         foo(); // <-- Inspecting here, foo is already evaluated.
+    //     }
+    //     foo(); // <-- Byte code offset is now here, so we need to -1 to get back in the block scope.
+    int LocalsWalker::GetAdjustedByteCodeOffset(DiagStackFrame* frame)
+    {
+        int offset = frame->GetByteCodeOffset();
+        if (!frame->IsTopFrame() && frame->IsInterpreterFrame())
+        {
+            // Native frames are already adjusted so just need to adjust interpreted
+            // frames that are not the top frame.
+            --offset;
+        }
+
+        return offset;
+    }
+
     /*static*/
     DWORD LocalsWalker::GetCurrentFramesLocalsType(DiagStackFrame* frame)
     {

lib/Runtime/Debug/DiagObjectModel.h

@@ -316,6 +316,8 @@ namespace Js
         virtual BOOL GetGroupObject(ResolvedObject* pResolvedObject) {return FALSE; }
 
         static DWORD GetCurrentFramesLocalsType(DiagStackFrame* frame);
+        static DebuggerScope * GetScopeWhenHaltAtFormals(DiagStackFrame* frame);
+        static int GetAdjustedByteCodeOffset(DiagStackFrame* frame);
 
         IDiagObjectAddress * FindPropertyAddress(PropertyId propId, bool& isConst) override;