[1.8>1.9] [MERGE #4812 @akroshg] ChakraCore 2018-03 Security updates

Merge pull request #4812 from akroshg:test1803_1

Pushing 18-03 changes.

Build/Common.Build.props

@@ -78,6 +78,9 @@
       <!-- Separate global variable for linker -->
       <AdditionalOptions>%(AdditionalOptions) /Gw</AdditionalOptions>
 
+      <!-- Enable warnings not included in W4 by default -->
+      <AdditionalOptions>%(AdditionalOptions) /w44242 /w44254</AdditionalOptions>
+
       <ProgramDataBaseFileName Condition="'$(ConfigurationType)'=='StaticLibrary'">$(IntDir)$(TargetName).pdb</ProgramDataBaseFileName>
       <ProgramDataBaseFileName Condition="'$(ConfigurationType)'!='StaticLibrary'">$(IntDir)</ProgramDataBaseFileName>
 

bin/NativeTests/NativeTests.cpp

@@ -7,6 +7,7 @@
 #define CATCH_CONFIG_RUNNER
 #pragma warning(push)
 // conversion from 'int' to 'char', possible loss of data
+#pragma warning(disable:4242)
 #pragma warning(disable:4244)
 #include "catch.hpp"
 #pragma warning(pop)

lib/Backend/BailOut.cpp

@@ -1301,8 +1301,11 @@ BailOutRecord::BailOutInlinedHelper(Js::JavascriptCallStackLayout * layout, Bail
         InlineeFrameRecord* inlineeFrameRecord = entryPointInfo->FindInlineeFrame(returnAddress);
         if (inlineeFrameRecord)
         {
+            // While bailing out, RestoreFrames should box all Vars on the stack. If there are multiple Vars pointing to the same
+            // object, the cached version (that was previously boxed) will be reused to maintain pointer identity and correctness
+            // after the transition to the interpreter.
             InlinedFrameLayout* outerMostFrame = (InlinedFrameLayout *)(((uint8 *)Js::JavascriptCallStackLayout::ToFramePointer(layout)) - entryPointInfo->frameHeight);
-            inlineeFrameRecord->RestoreFrames(functionBody, outerMostFrame, layout, false /* deepCopy */);
+            inlineeFrameRecord->RestoreFrames(functionBody, outerMostFrame, layout, true /* boxArgs */);
         }
     }
 

lib/Backend/GlobOpt.cpp

@@ -2481,8 +2481,11 @@ GlobOpt::OptInstr(IR::Instr *&instr, bool* isInstrRemoved)
         CurrentBlockData()->KillStateForGeneratorYield();
     }
 
-    // Change LdFld on arrays, strings, and 'arguments' to LdLen when we're accessing the .length field
-    this->TryReplaceLdLen(instr);
+    if (!IsLoopPrePass())
+    {
+        // Change LdFld on arrays, strings, and 'arguments' to LdLen when we're accessing the .length field
+        this->TryReplaceLdLen(instr);
+    }
 
     // Consider: Do we ever get post-op bailout here, and if so is the FillBailOutInfo call in the right place?
     if (instr->HasBailOutInfo() && !this->IsLoopPrePass())
@@ -13440,6 +13443,9 @@ GlobOpt::OptArraySrc(IR::Instr * *const instrRef)
         return;
     }
 
+    const bool isLikelyVirtualTypedArray = baseValueType.IsLikelyOptimizedVirtualTypedArray();
+    Assert(!(isLikelyJsArray && isLikelyVirtualTypedArray));
+
     ValueType newBaseValueType(baseValueType.ToDefiniteObject());
     if(isLikelyJsArray && newBaseValueType.HasNoMissingValues() && !DoArrayMissingValueCheckHoist())
     {
@@ -13770,7 +13776,7 @@ GlobOpt::OptArraySrc(IR::Instr * *const instrRef)
             {
                 const JsArrayKills loopKills(loop->jsArrayKills);
                 Value *baseValueInLoopLandingPad = nullptr;
-                if((isLikelyJsArray && loopKills.KillsValueType(newBaseValueType)) ||
+                if(((isLikelyJsArray || isLikelyVirtualTypedArray) && loopKills.KillsValueType(newBaseValueType)) ||
                     !OptIsInvariant(baseOpnd->m_sym, currentBlock, loop, baseValue, true, true, &baseValueInLoopLandingPad) ||
                     !(doArrayChecks || baseValueInLoopLandingPad->GetValueInfo()->IsObject()))
                 {
@@ -17384,7 +17390,9 @@ GlobOpt::DoArrayCheckHoist(const ValueType baseValueType, Loop* loop, IR::Instr
         return false;
     }
 
-    if(!baseValueType.IsLikelyArrayOrObjectWithArray() ||
+    // This includes typed arrays, but not virtual typed arrays, whose vtable can change if the buffer goes away.
+    // Note that in the virtual case the vtable check is the only way to catch this, since there's no bound check.
+    if(!(baseValueType.IsLikelyArrayOrObjectWithArray() || baseValueType.IsLikelyOptimizedVirtualTypedArray()) ||
         (loop ? ImplicitCallFlagsAllowOpts(loop) : ImplicitCallFlagsAllowOpts(func)))
     {
         return true;

lib/Backend/GlobOpt.h

@@ -332,12 +332,16 @@ public:
 public:
     bool KillsValueType(const ValueType valueType) const
     {
-        Assert(valueType.IsArrayOrObjectWithArray());
+        Assert(valueType.IsArrayOrObjectWithArray() || valueType.IsOptimizedVirtualTypedArray());
 
         return
             killsAllArrays ||
-            (killsArraysWithNoMissingValues && valueType.HasNoMissingValues()) ||
-            (killsNativeArrays && !valueType.HasVarElements());
+            (valueType.IsArrayOrObjectWithArray() && 
+             (
+              (killsArraysWithNoMissingValues && valueType.HasNoMissingValues()) ||
+              (killsNativeArrays && !valueType.HasVarElements())
+             )
+            );
     }
 
     bool AreSubsetOf(const JsArrayKills &other) const

lib/Backend/InlineeFrameInfo.cpp

@@ -199,14 +199,14 @@ void InlineeFrameRecord::Finalize(Func* inlinee, uint32 currentOffset)
     Assert(this->inlineDepth != 0);
 }
 
-void InlineeFrameRecord::Restore(Js::FunctionBody* functionBody, InlinedFrameLayout *inlinedFrame, Js::JavascriptCallStackLayout * layout, bool deepCopy) const
+void InlineeFrameRecord::Restore(Js::FunctionBody* functionBody, InlinedFrameLayout *inlinedFrame, Js::JavascriptCallStackLayout * layout, bool boxValues) const
 {
     Assert(this->inlineDepth != 0);
     Assert(inlineeStartOffset != 0);
 
     BAILOUT_VERBOSE_TRACE(functionBody, _u("Restore function object: "));
     // No deepCopy needed for just the function
-    Js::Var varFunction = this->Restore(this->functionOffset, /*isFloat64*/ false, /*isInt32*/ false, layout, functionBody, /*deepCopy*/ false);
+    Js::Var varFunction = this->Restore(this->functionOffset, /*isFloat64*/ false, /*isInt32*/ false, layout, functionBody, boxValues);
     Assert(Js::ScriptFunction::Is(varFunction));
 
     Js::ScriptFunction* function = Js::ScriptFunction::FromVar(varFunction);
@@ -222,9 +222,9 @@ void InlineeFrameRecord::Restore(Js::FunctionBody* functionBody, InlinedFrameLay
 
         // Forward deepCopy flag for the arguments in case their data must be guaranteed
         // to have its own lifetime
-        Js::Var var = this->Restore(this->argOffsets[i], isFloat64, isInt32, layout, functionBody, deepCopy);
+        Js::Var var = this->Restore(this->argOffsets[i], isFloat64, isInt32, layout, functionBody, boxValues);
 #if DBG
-        if (!Js::TaggedNumber::Is(var))
+        if (boxValues && !Js::TaggedNumber::Is(var))
         {
             Js::RecyclableObject *const recyclableObject = Js::RecyclableObject::FromVar(var);
             Assert(!ThreadContext::IsOnStack(recyclableObject));
@@ -236,7 +236,10 @@ void InlineeFrameRecord::Restore(Js::FunctionBody* functionBody, InlinedFrameLay
     BAILOUT_FLUSH(functionBody);
 }
 
-void InlineeFrameRecord::RestoreFrames(Js::FunctionBody* functionBody, InlinedFrameLayout* outerMostFrame, Js::JavascriptCallStackLayout* callstack, bool deepCopy)
+// Note: the boxValues parameter should be true when this is called from a Bailout codepath to ensure that multiple vars to
+// the same object reuse the cached value during the transition to the interpreter.
+// Otherwise, this parameter should be false as the values are not required to be moved to the heap to restore the frame.
+void InlineeFrameRecord::RestoreFrames(Js::FunctionBody* functionBody, InlinedFrameLayout* outerMostFrame, Js::JavascriptCallStackLayout* callstack, bool boxValues)
 {
     InlineeFrameRecord* innerMostRecord = this;
     class AutoReverse
@@ -274,7 +277,7 @@ void InlineeFrameRecord::RestoreFrames(Js::FunctionBody* functionBody, InlinedFr
 
     while (currentRecord)
     {
-        currentRecord->Restore(functionBody, currentFrame, callstack, deepCopy);
+        currentRecord->Restore(functionBody, currentFrame, callstack, boxValues);
         currentRecord = currentRecord->parent;
         currentFrame = currentFrame->Next();
     }
@@ -283,10 +286,10 @@ void InlineeFrameRecord::RestoreFrames(Js::FunctionBody* functionBody, InlinedFr
     currentFrame->callInfo.Count = 0;
 }
 
-Js::Var InlineeFrameRecord::Restore(int offset, bool isFloat64, bool isInt32, Js::JavascriptCallStackLayout * layout, Js::FunctionBody* functionBody, bool deepCopy) const
+Js::Var InlineeFrameRecord::Restore(int offset, bool isFloat64, bool isInt32, Js::JavascriptCallStackLayout * layout, Js::FunctionBody* functionBody, bool boxValue) const
 {
     Js::Var value;
-    bool boxStackInstance = true;
+    bool boxStackInstance = boxValue;
     double dblValue;
     if (offset >= 0)
     {
@@ -324,8 +327,11 @@ Js::Var InlineeFrameRecord::Restore(int offset, bool isFloat64, bool isInt32, Js
         BAILOUT_VERBOSE_TRACE(functionBody, _u(", value: 0x%p"), value);
         if (boxStackInstance)
         {
+            // Do not deepCopy in this call to BoxStackInstance because this should be used for
+            // bailing out, where a shallow copy that is cached is needed to ensure that multiple
+            // vars pointing to the same boxed object reuse the new boxed value.
             Js::Var oldValue = value;
-            value = Js::JavascriptOperators::BoxStackInstance(oldValue, functionBody->GetScriptContext(), /* allowStackFunction */ true, deepCopy);
+            value = Js::JavascriptOperators::BoxStackInstance(oldValue, functionBody->GetScriptContext(), /* allowStackFunction */ true, false /* deepCopy */);
 
 #if ENABLE_DEBUG_CONFIG_OPTIONS
             if (oldValue != value)

lib/Backend/InlineeFrameInfo.h

@@ -108,7 +108,7 @@ struct InlineeFrameRecord
     }
 
     void PopulateParent(Func* func);
-    void RestoreFrames(Js::FunctionBody* functionBody, InlinedFrameLayout* outerMostInlinee, Js::JavascriptCallStackLayout* callstack, bool deepCopy);
+    void RestoreFrames(Js::FunctionBody* functionBody, InlinedFrameLayout* outerMostInlinee, Js::JavascriptCallStackLayout* callstack, bool boxValues);
     void Finalize(Func* inlinee, uint currentOffset);
 #if DBG_DUMP
     void Dump() const;
@@ -123,8 +123,8 @@ struct InlineeFrameRecord
     }
 
 private:
-    void Restore(Js::FunctionBody* functionBody, InlinedFrameLayout *outerMostFrame, Js::JavascriptCallStackLayout * layout, bool deepCopy) const;
-    Js::Var Restore(int offset, bool isFloat64, bool isInt32, Js::JavascriptCallStackLayout * layout, Js::FunctionBody* functionBody, bool deepCopy) const;
+    void Restore(Js::FunctionBody* functionBody, InlinedFrameLayout *outerMostFrame, Js::JavascriptCallStackLayout * layout, bool boxValues) const;
+    Js::Var Restore(int offset, bool isFloat64, bool isInt32, Js::JavascriptCallStackLayout * layout, Js::FunctionBody* functionBody, bool boxValue) const;
     InlineeFrameRecord* Reverse();
 };
 

lib/Common/CommonDefines.h

@@ -320,7 +320,10 @@
 
 #ifndef NTBUILD
 #define DELAYLOAD_SECTIONAPI 1
-#else
+#define DELAYLOAD_UNLOCKMEMORY 1
+#endif
+
+#ifdef NTBUILD
 #define ENABLE_PROJECTION
 #define ENABLE_FOUNDATION_OBJECT
 #define ENABLE_EXPERIMENTAL_FLAGS

lib/Common/Core/DelayLoadLibrary.cpp

@@ -317,4 +317,43 @@ NtdllLibrary::NTSTATUS NtdllLibrary::Close(_In_ HANDLE Handle)
 #endif
 }
 
+#ifndef DELAYLOAD_UNLOCKMEMORY
+extern "C"
+WINBASEAPI
+NtdllLibrary::NTSTATUS
+WINAPI
+NtUnlockVirtualMemory(
+    _In_ HANDLE ProcessHandle,
+    _Inout_ PVOID *BaseAddress,
+    _Inout_ PSIZE_T RegionSize,
+    _In_ ULONG MapType
+);
+#endif
+
+NtdllLibrary::NTSTATUS NtdllLibrary::UnlockVirtualMemory(
+    _In_ HANDLE ProcessHandle,
+    _Inout_ PVOID *BaseAddress,
+    _Inout_ PSIZE_T RegionSize,
+    _In_ ULONG MapType)
+{
+#ifdef DELAYLOAD_UNLOCKMEMORY
+    if (m_hModule)
+    {
+        if (unlock == nullptr)
+        {
+            unlock = (PFnNtUnlockVirtualMemory)GetFunction("NtUnlockVirtualMemory");
+            if (unlock == nullptr)
+            {
+                Assert(false);
+                return -1;
+            }
+        }
+        return unlock(ProcessHandle, BaseAddress, RegionSize, MapType);
+    }
+    return -1;
+#else
+    return NtUnlockVirtualMemory(ProcessHandle, BaseAddress, RegionSize, MapType);
+#endif
+}
+
 #endif // _WIN32

lib/Common/Core/DelayLoadLibrary.h

@@ -34,6 +34,7 @@ class NtdllLibrary : protected DelayLoadLibrary
 public:
     // needed for InitializeObjectAttributes
     static const ULONG OBJ_KERNEL_HANDLE = 0x00000200;
+    static const ULONG MAP_PROCESS = 1;
 
     typedef struct _UNICODE_STRING {
         USHORT Length;
@@ -105,6 +106,13 @@ private:
     typedef NTSTATUS(NTAPI *PFnNtClose)(_In_ HANDLE Handle);
     PFnNtClose close;
 
+    typedef NTSTATUS(NTAPI *PFnNtUnlockVirtualMemory)(
+        _In_ HANDLE ProcessHandle,
+        _Inout_ PVOID *BaseAddress,
+        _Inout_ PSIZE_T RegionSize,
+        _In_ ULONG MapType);
+    PFnNtUnlockVirtualMemory unlock;
+
 public:
     static NtdllLibrary* Instance;
 
@@ -117,7 +125,8 @@ public:
         createSection(NULL),
         mapViewOfSection(NULL),
         unmapViewOfSection(NULL),
-        close(NULL)
+        close(NULL),
+        unlock(nullptr)
     {
         this->EnsureFromSystemDirOnly();
     }
@@ -176,5 +185,12 @@ public:
     NTSTATUS Close(
         _In_ HANDLE Handle
     );
+
+    NTSTATUS UnlockVirtualMemory(
+        _In_ HANDLE ProcessHandle,
+        _Inout_ PVOID *BaseAddress,
+        _Inout_ PSIZE_T RegionSize,
+        _In_ ULONG MapType
+    );
 };
 #endif

lib/Common/Memory/RecyclerSweep.cpp

@@ -290,7 +290,7 @@ RecyclerSweep::EndSweep()
 
     // Clean up the HeapBlockMap.
     // This will release any internal structures that are no longer needed after Sweep.
-    recycler->heapBlockMap.Cleanup(!recycler->IsMemProtectMode());
+    recycler->heapBlockMap.Cleanup(recycler->IsMemProtectMode());
 }
 
 #if ENABLE_CONCURRENT_GC

lib/Common/Memory/SectionAllocWrapper.cpp

@@ -10,11 +10,21 @@
 #ifdef NTDDI_WIN10_RS2
 #if (NTDDI_VERSION >= NTDDI_WIN10_RS2)
 #define USEFILEMAP2 1
+#define USEVIRTUALUNLOCKEX 1
 #endif
 #endif
 
 namespace Memory
 {
+    
+void UnlockMemory(HANDLE process, LPVOID address, SIZE_T size)
+{
+#if USEVIRTUALUNLOCKEX
+    VirtualUnlockEx(process, address, size);
+#else
+    NtdllLibrary::Instance->UnlockVirtualMemory(process, &address, &size, NtdllLibrary::MAP_PROCESS);
+#endif
+}
 
 void CloseSectionHandle(HANDLE handle)
 {
@@ -593,12 +603,6 @@ SectionAllocWrapper::AllocPages(LPVOID requestAddress, size_t pageCount, DWORD a
             return nullptr;
         }
         address = requestAddress;
-
-        if ((allocationType & MEM_COMMIT) == MEM_COMMIT)
-        {
-            const DWORD allocProtectFlags = AutoSystemInfo::Data.IsCFGEnabled() ? PAGE_EXECUTE_RO_TARGETS_INVALID : PAGE_EXECUTE_READ;
-            address = VirtualAllocEx(this->process, address, dwSize, MEM_COMMIT, allocProtectFlags);
-        }
     }
 
     return address;
@@ -661,11 +665,7 @@ BOOL SectionAllocWrapper::Free(LPVOID lpAddress, size_t dwSize, DWORD dwFreeType
             ZeroMemory(localAddr, AutoSystemInfo::PageSize);
             FreeLocal(localAddr);
         }
-        DWORD oldFlags = NULL;
-        if (!VirtualProtectEx(this->process, lpAddress, dwSize, PAGE_NOACCESS, &oldFlags))
-        {
-            return FALSE;
-        }
+        UnlockMemory(this->process, lpAddress, dwSize);
     }
 
     return TRUE;
@@ -924,37 +924,15 @@ LPVOID PreReservedSectionAllocWrapper::AllocPages(LPVOID lpAddress, DECLSPEC_GUA
         AssertMsg(freeSegmentsBVIndex < PreReservedAllocationSegmentCount, "Invalid BitVector index calculation?");
         AssertMsg(dwSize % AutoSystemInfo::PageSize == 0, "COMMIT is managed at AutoSystemInfo::PageSize granularity");
 
-        char * allocatedAddress = nullptr;
-
-        if ((allocationType & MEM_COMMIT) != 0)
-        {
-#if defined(ENABLE_JIT_CLAMP)
-            AutoEnableDynamicCodeGen enableCodeGen;
-#endif
-
-            const DWORD allocProtectFlags = AutoSystemInfo::Data.IsCFGEnabled() ? PAGE_EXECUTE_RO_TARGETS_INVALID : PAGE_EXECUTE_READ;
-            allocatedAddress = (char *)VirtualAllocEx(this->process, addressToReserve, dwSize, MEM_COMMIT, allocProtectFlags);
-            if (allocatedAddress == nullptr)
-            {
-                MemoryOperationLastError::RecordLastError();
-            }
-        }
-        else
-        {
-            // Just return the uncommitted address if we didn't ask to commit it.
-            allocatedAddress = addressToReserve;
-        }
-
         // Keep track of the committed pages within the preReserved Memory Region
-        if (lpAddress == nullptr && allocatedAddress != nullptr)
+        if (lpAddress == nullptr)
         {
-            Assert(allocatedAddress == addressToReserve);
             Assert(requestedNumOfSegments != 0);
             freeSegments.ClearRange(freeSegmentsBVIndex, static_cast<uint>(requestedNumOfSegments));
-        }
+    }
 
-        PreReservedHeapTrace(_u("MEM_COMMIT: StartAddress: 0x%p of size: 0x%x * 0x%x bytes \n"), allocatedAddress, requestedNumOfSegments, AutoSystemInfo::Data.GetAllocationGranularityPageSize());
-        return allocatedAddress;
+        PreReservedHeapTrace(_u("MEM_COMMIT: StartAddress: 0x%p of size: 0x%x * 0x%x bytes \n"), addressToReserve, requestedNumOfSegments, AutoSystemInfo::Data.GetAllocationGranularityPageSize());
+        return addressToReserve;
     }
 }
 
@@ -989,11 +967,7 @@ PreReservedSectionAllocWrapper::Free(LPVOID lpAddress, size_t dwSize, DWORD dwFr
         FreeLocal(localAddr);
     }
 
-    DWORD oldFlags = NULL;
-    if(!VirtualProtectEx(this->process, lpAddress, dwSize, PAGE_NOACCESS, &oldFlags))
-    {
-        return FALSE;
-    }
+    UnlockMemory(this->process, lpAddress, dwSize);
 
     size_t requestedNumOfSegments = dwSize / AutoSystemInfo::Data.GetAllocationGranularityPageSize();
     Assert(requestedNumOfSegments <= MAXUINT32);

lib/Parser/Parse.cpp

@@ -931,21 +931,6 @@ Symbol* Parser::AddDeclForPid(ParseNodePtr pnode, IdentPtr pid, SymbolType symbo
             CheckRedeclarationErrorForBlockId(pid, pnodeFnc->sxFnc.pnodeScopes->sxBlock.blockId);
         }
 
-        if ((scope->GetScopeType() == ScopeType_FunctionBody || scope->GetScopeType() == ScopeType_Parameter) && symbolType != STFunction)
-        {
-            AnalysisAssert(pnodeFnc);
-            if (pnodeFnc->sxFnc.pnodeName &&
-                pnodeFnc->sxFnc.pnodeName->nop == knopVarDecl &&
-                pnodeFnc->sxFnc.pnodeName->sxVar.pid == pid &&
-                (pnodeFnc->sxFnc.IsBodyAndParamScopeMerged() || scope->GetScopeType() == ScopeType_Parameter))
-            {
-                // Named function expression has its name hidden by a local declaration.
-                // This is important to know if we don't know whether nested deferred functions refer to it,
-                // because if the name has a non-local reference then we have to create a scope object.
-                m_currentNodeFunc->sxFnc.SetNameIsHidden();
-            }
-        }
-
         if (!sym)
         {
             const char16 *name = reinterpret_cast<const char16*>(pid->Psz());
@@ -4002,7 +3987,16 @@ ParseNodePtr Parser::ParsePostfixOperators(
         case tkStrTmplBasic:
         case tkStrTmplBegin:
             {
-                ParseNode* templateNode = ParseStringTemplateDecl<buildAST>(pnode);
+                ParseNode* templateNode = nullptr;
+                if (pnode != nullptr)
+                {
+                    AutoMarkInParsingArgs autoMarkInParsingArgs(this);
+                    templateNode = ParseStringTemplateDecl<buildAST>(pnode);
+                }
+                else
+                {
+                    templateNode = ParseStringTemplateDecl<buildAST>(pnode);
+                }
 
                 if (!buildAST)
                 {
@@ -6523,15 +6517,6 @@ bool Parser::ParseFncNames(ParseNodePtr pnodeFnc, ParseNodePtr pnodeFncParent, u
         *pFncNamePid = pidBase;
     }
 
-    if (fDeclaration &&
-        pnodeFncParent &&
-        pnodeFncParent->sxFnc.pnodeName &&
-        pnodeFncParent->sxFnc.pnodeName->nop == knopVarDecl &&
-        pnodeFncParent->sxFnc.pnodeName->sxVar.pid == pidBase)
-    {
-        pnodeFncParent->sxFnc.SetNameIsHidden();
-    }
-
     if (buildAST)
     {
         AnalysisAssert(pnodeFnc);
@@ -7603,7 +7588,7 @@ LPCOLESTR Parser::ConstructFinalHintNode(IdentPtr pClassName, IdentPtr pMemberNa
     }
 
     LPCOLESTR pFinalName = isComputedName? pMemberNameHint : pMemberName->Psz();
-    uint32 fullNameHintLength = 0;
+    uint32 fullNameHintLength = (uint32)wcslen(pFinalName);
     uint32 shortNameOffset = 0;
     if (!isStatic)
     {
@@ -7633,15 +7618,9 @@ LPCOLESTR Parser::ConstructFinalHintNode(IdentPtr pClassName, IdentPtr pMemberNa
         }
 
     }
-    if (fullNameHintLength > *nameLength)
-    {
-        *nameLength = fullNameHintLength;
-    }
 
-    if (shortNameOffset > *pShortNameOffset)
-    {
-        *pShortNameOffset = shortNameOffset;
-    }
+    *nameLength = fullNameHintLength;
+    *pShortNameOffset = shortNameOffset;
 
     return pFinalName;
 }
@@ -8241,6 +8220,7 @@ ParseNodePtr Parser::ParseStringTemplateDecl(ParseNodePtr pnodeTagFnc)
 
             // We need to set the arg count explicitly
             pnodeStringTemplate->sxCall.argCount = stringConstantCount;
+            pnodeStringTemplate->sxCall.hasDestructuring = m_hasDestructuringPattern;
         }
     }
 
@@ -12826,6 +12806,10 @@ ParseNodePtr Parser::GetRightSideNodeFromPattern(ParseNodePtr pnode)
         {
             TrackAssignment<true>(pnode, nullptr);
         }
+        else if (op == knopAsg)
+        {
+            TrackAssignment<true>(pnode->sxBin.pnode1, nullptr);
+        }
     }
 
     return rightNode;

lib/Parser/ptree.h

@@ -190,7 +190,7 @@ enum FncFlags : uint
     kFunctionIsClassConstructor                 = 1 << 18, // function is a class constructor
     kFunctionIsBaseClassConstructor             = 1 << 19, // function is a base class constructor
     kFunctionIsClassMember                      = 1 << 20, // function is a class member
-    kFunctionNameIsHidden                       = 1 << 21, // True if a named function expression has its name hidden from nested functions
+    // Free = 1 << 21,
     kFunctionIsGeneratedDefault                 = 1 << 22, // Is the function generated by us as a default (e.g. default class constructor)
     kFunctionHasDefaultArguments                = 1 << 23, // Function has one or more ES6 default arguments
     kFunctionIsStaticMember                     = 1 << 24,
@@ -311,7 +311,6 @@ public:
     void SetIsLambda(bool set = true) { SetFlags(kFunctionIsLambda, set); }
     void SetIsMethod(bool set = true) { SetFlags(kFunctionIsMethod, set); }
     void SetIsStaticMember(bool set = true) { SetFlags(kFunctionIsStaticMember, set); }
-    void SetNameIsHidden(bool set = true) { SetFlags(kFunctionNameIsHidden, set); }
     void SetNested(bool set = true) { SetFlags(kFunctionNested, set); }
     void SetStrictMode(bool set = true) { SetFlags(kFunctionStrictMode, set); }
     void SetIsModule(bool set = true) { SetFlags(kFunctionIsModule, set); }
@@ -352,7 +351,6 @@ public:
     bool IsNested() const { return HasFlags(kFunctionNested); }
     bool IsStaticMember() const { return HasFlags(kFunctionIsStaticMember); }
     bool IsModule() const { return HasFlags(kFunctionIsModule); }
-    bool NameIsHidden() const { return HasFlags(kFunctionNameIsHidden); }
     bool UsesArguments() const { return HasFlags(kFunctionUsesArguments); }
     bool IsDefaultModuleExport() const { return HasFlags(kFunctionIsDefaultModuleExport); }
     bool NestedFuncEscapes() const { return nestedFuncEscapes; }
@@ -364,7 +362,6 @@ public:
         kFunctionNested |
         kFunctionDeclaration |
         kFunctionStrictMode |
-        kFunctionNameIsHidden |
         kFunctionHasReferenceableBuiltInArguments |
         kFunctionHasNonThisStmt |
         // todo:: we shouldn't accept kFunctionHasAnyWriteToFormals on the asm module, but it looks like a bug is setting that flag incorrectly

lib/Runtime/Base/CallInfo.h

@@ -50,7 +50,7 @@ namespace Js
             AssertOrFailFastMsg(count < CallInfo::kMaxCountArgs, "Argument list too large");
         }
 
-        CallInfo(CallFlags flags, ArgSlot count)
+        CallInfo(CallFlags flags, uint count)
             : Flags(flags)
             , Count(count)
 #ifdef TARGET_64
@@ -60,16 +60,6 @@ namespace Js
             // Keeping this version to avoid the assert
         }
 
-        // The bool is used to avoid the signature confusion between the ArgSlot and uint version of the constructor
-        CallInfo(CallFlags flags, uint count, bool unusedBool)
-            : Flags(flags)
-            , Count(count)
-#ifdef TARGET_64
-            , unused(0)
-#endif
-        {
-            AssertOrFailFastMsg(count < CallInfo::kMaxCountArgs, "Argument list too large");
-        }
 
         CallInfo(VirtualTableInfoCtorEnum v)
         {
@@ -107,7 +97,7 @@ namespace Js
         }
 
         // New target value is passed as an extra argument which is nto included in the Count
-        static Var GetNewTarget(CallFlags flag, Var* values, ArgSlot count)
+        static Var GetNewTarget(CallFlags flag, Var* values, uint count)
         {
             if (HasNewTarget(flag))
             {

lib/Runtime/Base/Debug.cpp

@@ -43,7 +43,7 @@ WCHAR* DumpCallStackFull(uint frameCount, bool print)
 
             for (uint i = 0; i < callInfo.Count; i++)
             {
-                StringCchPrintf(buffer, _countof(buffer), _u(", 0x%p"), walker.GetJavascriptArgs()[i]);
+                StringCchPrintf(buffer, _countof(buffer), _u(", 0x%p"), walker.GetJavascriptArgs(false /*boxArgs*/)[i]);
                 sb.AppendSz(buffer);
             }
             StringCchPrintf(buffer, _countof(buffer), _u(")[%s (%u, %d)]\n"), sourceFileName, line + 1, column + 1);

lib/Runtime/ByteCode/ByteCodeGenerator.cpp

@@ -2682,27 +2682,12 @@ FuncInfo* PostVisitFunction(ParseNode* pnode, ByteCodeGenerator* byteCodeGenerat
             Assert(CONFIG_FLAG(DeferNested));
             byteCodeGenerator->ProcessCapturedSym(sym);
 
-            if (!top->root->sxFnc.NameIsHidden())
+            top->SetFuncExprNameReference(true);
+            if (pnode->sxFnc.pnodeBody)
             {
-                top->SetFuncExprNameReference(true);
-                if (pnode->sxFnc.pnodeBody)
-                {
-                    top->GetParsedFunctionBody()->SetFuncExprNameReference(true);
-                }
-                if (!sym->GetScope()->GetIsObject())
-                {
-                    // The function expression symbol will be emitted in the param/body scope.
-                    if (top->GetParamScope())
-                    {
-                        top->GetParamScope()->SetHasOwnLocalInClosure(true);
-                    }
-                    else
-                    {
-                        top->GetBodyScope()->SetHasOwnLocalInClosure(true);
-                    }
-                    top->SetHasLocalInClosure(true);
-                }
+                top->GetParsedFunctionBody()->SetFuncExprNameReference(true);
             }
+            byteCodeGenerator->ProcessScopeWithCapturedSym(sym->GetScope());
         }
     }
 

lib/Runtime/Language/DynamicProfileInfo.cpp

@@ -505,7 +505,7 @@ namespace Js
     }
 #endif
 
-    void DynamicProfileInfo::RecordCallSiteInfo(FunctionBody* functionBody, ProfileId callSiteId, FunctionInfo* calleeFunctionInfo, JavascriptFunction* calleeFunction, ArgSlot actualArgCount, bool isConstructorCall, InlineCacheIndex ldFldInlineCacheId)
+    void DynamicProfileInfo::RecordCallSiteInfo(FunctionBody* functionBody, ProfileId callSiteId, FunctionInfo* calleeFunctionInfo, JavascriptFunction* calleeFunction, uint actualArgCount, bool isConstructorCall, InlineCacheIndex ldFldInlineCacheId)
     {
 #if DBG_DUMP || defined(DYNAMIC_PROFILE_STORAGE) || defined(RUNTIME_DATA_COLLECTION)
         // If we persistsAcrossScriptContext, the dynamic profile info may be referred to by multiple function body from

lib/Runtime/Language/DynamicProfileInfo.h

@@ -389,7 +389,7 @@ namespace Js
 #ifdef ASMJS_PLAT
         void RecordAsmJsCallSiteInfo(FunctionBody* callerBody, ProfileId callSiteId, FunctionBody* calleeBody);
 #endif
-        void RecordCallSiteInfo(FunctionBody* functionBody, ProfileId callSiteId, FunctionInfo * calleeFunctionInfo, JavascriptFunction* calleeFunction, ArgSlot actualArgCount, bool isConstructorCall, InlineCacheIndex ldFldInlineCacheId = Js::Constants::NoInlineCacheIndex);
+        void RecordCallSiteInfo(FunctionBody* functionBody, ProfileId callSiteId, FunctionInfo * calleeFunctionInfo, JavascriptFunction* calleeFunction, uint actualArgCount, bool isConstructorCall, InlineCacheIndex ldFldInlineCacheId = Js::Constants::NoInlineCacheIndex);
         void RecordConstParameterAtCallSite(ProfileId callSiteId, int argNum);
         static bool HasCallSiteInfo(FunctionBody* functionBody);
         bool HasCallSiteInfo(FunctionBody* functionBody, ProfileId callSiteId); // Does a particular callsite have ProfileInfo?

lib/Runtime/Language/JavascriptStackWalker.cpp

@@ -263,14 +263,14 @@ namespace Js
         this->GetCurrentArgv()[JavascriptFunctionArgIndex_ArgumentsObject] = args;
     }
 
-    Js::Var * JavascriptStackWalker::GetJavascriptArgs() const
+    Js::Var * JavascriptStackWalker::GetJavascriptArgs(bool boxArgsAndDeepCopy) const
     {
         Assert(this->IsJavascriptFrame());
 
 #if ENABLE_NATIVE_CODEGEN
         if (inlinedFramesBeingWalked)
         {
-            return inlinedFrameWalker.GetArgv(/* includeThis = */ false);
+            return inlinedFrameWalker.GetArgv(/* includeThis */ false, boxArgsAndDeepCopy);
         }
         else
 #endif
@@ -450,7 +450,7 @@ namespace Js
             // are inlined frames on the stack the InlineeCallInfo of the first inlined frame
             // has the native offset of the current physical frame.
             Assert(!*inlinee);
-            InlinedFrameWalker::FromPhysicalFrame(tmpFrameWalker, currentFrame, ScriptFunction::FromVar(parentFunction), PreviousInterpreterFrameIsFromBailout(), loopNum, this, useInternalFrameInfo, false /*noAlloc*/, false /*deepCopy*/);
+            InlinedFrameWalker::FromPhysicalFrame(tmpFrameWalker, currentFrame, ScriptFunction::FromVar(parentFunction), PreviousInterpreterFrameIsFromBailout(), loopNum, this, useInternalFrameInfo, false /*noAlloc*/);
             inlineeOffset = tmpFrameWalker.GetBottomMostInlineeOffset();
             tmpFrameWalker.Close();
         }
@@ -555,7 +555,7 @@ namespace Js
                         }
 
                         bool hasInlinedFramesOnStack = InlinedFrameWalker::FromPhysicalFrame(inlinedFrameWalker, currentFrame,
-                            ScriptFunction::FromVar(function), true /*fromBailout*/, loopNum, this, false /*useInternalFrameInfo*/, false /*noAlloc*/, this->deepCopyForArgs);
+                            ScriptFunction::FromVar(function), true /*fromBailout*/, loopNum, this, false /*useInternalFrameInfo*/, false /*noAlloc*/);
 
                         if (hasInlinedFramesOnStack)
                         {
@@ -611,8 +611,7 @@ namespace Js
                         -1,     // loopNum
                         nullptr,// walker
                         false,  // useInternalFrameInfo
-                        false,  // noAlloc
-                        this->deepCopyForArgs
+                        false   // noAlloc
                     );
                     if (inlinedFramesFound)
                     {
@@ -658,8 +657,7 @@ namespace Js
     _NOINLINE
     JavascriptStackWalker::JavascriptStackWalker(ScriptContext * scriptContext, bool useEERContext, PVOID returnAddress, bool _forceFullWalk /*=false*/) :
         inlinedFrameCallInfo(CallFlags_None, 0), shouldDetectPartiallyInitializedInterpreterFrame(true), forceFullWalk(_forceFullWalk),
-        previousInterpreterFrameIsFromBailout(false), previousInterpreterFrameIsForLoopBody(false), hasInlinedFramesOnStack(false),
-        deepCopyForArgs(false)
+        previousInterpreterFrameIsFromBailout(false), previousInterpreterFrameIsForLoopBody(false), hasInlinedFramesOnStack(false)
     {
         if (scriptContext == NULL)
         {
@@ -955,7 +953,7 @@ namespace Js
                 Assert(this->interpreterFrame->TestFlags(Js::InterpreterStackFrameFlags_FromBailOut));
                 InlinedFrameWalker tmpFrameWalker;
                 Assert(InlinedFrameWalker::FromPhysicalFrame(tmpFrameWalker, currentFrame, Js::ScriptFunction::FromVar(argv[JavascriptFunctionArgIndex_Function]),
-                    true /*fromBailout*/, this->tempInterpreterFrame->GetCurrentLoopNum(), this, false /*useInternalFrameInfo*/, true /*noAlloc*/, false /*deepCopy*/));
+                    true /*fromBailout*/, this->tempInterpreterFrame->GetCurrentLoopNum(), this, false /*useInternalFrameInfo*/, true /*noAlloc*/));
                 tmpFrameWalker.Close();
             }
 #endif //DBG
@@ -1002,7 +1000,7 @@ namespace Js
             {
                 if (includeInlineFrames &&
                     InlinedFrameWalker::FromPhysicalFrame(inlinedFrameWalker, currentFrame, Js::ScriptFunction::FromVar(argv[JavascriptFunctionArgIndex_Function]),
-                        false /*fromBailout*/, this->tempInterpreterFrame->GetCurrentLoopNum(), this, false /*useInternalFrameInfo*/, false /*noAlloc*/, this->deepCopyForArgs))
+                        false /*fromBailout*/, this->tempInterpreterFrame->GetCurrentLoopNum(), this, false /*useInternalFrameInfo*/, false /*noAlloc*/))
                 {
                     // Found inlined frames in a jitted loop body. We dont want to skip the inlined frames; walk all of them before setting codeAddress on lastInternalFrameInfo.
                     // DeepCopy here because, if there is an inlinee in a loop body, FromPhysicalFrame won't be called from UpdateFrame
@@ -1246,7 +1244,7 @@ namespace Js
 
 #if ENABLE_NATIVE_CODEGEN
     bool InlinedFrameWalker::FromPhysicalFrame(InlinedFrameWalker& self, StackFrame& physicalFrame, Js::ScriptFunction *parent, bool fromBailout,
-        int loopNum, const JavascriptStackWalker * const stackWalker, bool useInternalFrameInfo, bool noAlloc, bool deepCopy)
+        int loopNum, const JavascriptStackWalker * const stackWalker, bool useInternalFrameInfo, bool noAlloc)
     {
         bool inlinedFramesFound = false;
         FunctionBody* parentFunctionBody = parent->GetFunctionBody();
@@ -1299,7 +1297,7 @@ namespace Js
 
                 if (record)
                 {
-                    record->RestoreFrames(parent->GetFunctionBody(), outerMostFrame, JavascriptCallStackLayout::FromFramePointer(framePointer), deepCopy);
+                    record->RestoreFrames(parent->GetFunctionBody(), outerMostFrame, JavascriptCallStackLayout::FromFramePointer(framePointer), false /* boxValues */);
                 }
             }
 
@@ -1366,26 +1364,46 @@ namespace Js
         return currentFrame->callInfo.Count;
     }
 
-    Js::Var *InlinedFrameWalker::GetArgv(bool includeThis /* = true */) const
+    // Note: the boxArgsAndDeepCopy parameter should be true when a copy of the JS args must be ensured to
+    // be on the heap. This results in a new array of Vars with deep copied boxed values (where
+    // appropriate).
+    // Otherwise, this parameter should be false. For instance, if the args will only be used
+    // internally to gather type info, the values are not boxed (so, some Vars may still be on
+    // the stack) and the array of the current frame is returned.
+    Js::Var *InlinedFrameWalker::GetArgv(bool includeThis, bool boxArgsAndDeepCopy) const
     {
         InlinedFrameWalker::InlinedFrame *const currentFrame = GetCurrentFrame();
         Assert(currentFrame);
 
         uint firstArg = includeThis ? InlinedFrameArgIndex_This : InlinedFrameArgIndex_SecondScriptArg;
-        Js::Var *args = &currentFrame->argv[firstArg];
+        size_t argCount = this->GetArgc() - firstArg;
+        Js::Var *args;
+        if (!boxArgsAndDeepCopy)
+        {
+            args = &currentFrame->argv[firstArg];
 
-        this->FinalizeStackValues(args, this->GetArgc() - firstArg);
+        }
+        else
+        {
+            args = RecyclerNewArray(parentFunction->GetScriptContext()->GetRecycler(), Js::Var, argCount);
+            for (size_t i = 0; i < argCount; i++)
+            {
+                args[i] = currentFrame->argv[firstArg + i];
+            }
+
+            this->FinalizeStackValues(args, argCount, true /*deepCopy*/);
+        }
 
         return args;
     }
 
-    void InlinedFrameWalker::FinalizeStackValues(__in_ecount(argCount) Js::Var args[], size_t argCount) const
+    void InlinedFrameWalker::FinalizeStackValues(__in_ecount(argCount) Js::Var args[], size_t argCount, bool deepCopy) const
     {
         ScriptContext *scriptContext = this->GetFunctionObject()->GetScriptContext();
 
         for (size_t i = 0; i < argCount; i++)
         {
-            args[i] = Js::JavascriptOperators::BoxStackInstance(args[i], scriptContext, false /*allowStackFunction*/, false /*deepCopy*/);
+            args[i] = Js::JavascriptOperators::BoxStackInstance(args[i], scriptContext, false /*allowStackFunction*/, deepCopy);
         }
     }
 
@@ -1443,15 +1461,20 @@ namespace Js
     {
         Assert(!IsTopMostFrame());
         Assert(currentIndex);
-
+#pragma warning(push)
+#pragma warning(disable: 4254)
         return GetFrameAtIndex(currentIndex - 1)->callInfo.InlineeStartOffset;
+#pragma warning(pop)
     }
 
     uint32 InlinedFrameWalker::GetBottomMostInlineeOffset() const
     {
         Assert(frameCount);
 
+#pragma warning(push)
+#pragma warning(disable: 4254)
         return GetFrameAtIndex(frameCount - 1)->callInfo.InlineeStartOffset;
+#pragma warning(pop)
     }
 
     Js::JavascriptFunction *InlinedFrameWalker::GetBottomMostFunctionObject() const

lib/Runtime/Language/JavascriptStackWalker.h

@@ -97,11 +97,11 @@ namespace Js
         }
 
         static bool             FromPhysicalFrame(InlinedFrameWalker& self, StackFrame& physicalFrame, Js::ScriptFunction *parent, bool fromBailout,
-                                                  int loopNum, const JavascriptStackWalker * const walker, bool useInternalFrameInfo, bool noAlloc, bool deepCopy);
+                                                  int loopNum, const JavascriptStackWalker * const walker, bool useInternalFrameInfo, bool noAlloc);
         void                    Close();
         bool                    Next(CallInfo& callInfo);
         size_t                  GetArgc() const;
-        Js::Var                *GetArgv(bool includeThis = true) const;
+        Js::Var                *GetArgv(bool includeThis, bool boxArgsAndDeepCopy) const;
         Js::JavascriptFunction *GetFunctionObject() const;
         void                    SetFunctionObject(Js::JavascriptFunction * function);
         Js::Var                 GetArgumentsObject() const;
@@ -113,7 +113,7 @@ namespace Js
         uint32                  GetCurrentInlineeOffset() const;
         uint32                  GetBottomMostInlineeOffset() const;
         Js::JavascriptFunction *GetBottomMostFunctionObject() const;
-        void                    FinalizeStackValues(__in_ecount(argCount) Js::Var args[], size_t argCount) const;
+        void                    FinalizeStackValues(__in_ecount(argCount) Js::Var args[], size_t argCount, bool deepCopy) const;
         int32                   GetFrameCount() { return frameCount; }
 
     private:
@@ -218,7 +218,7 @@ namespace Js
         CallInfo GetCallInfo(bool includeInlinedFrames = true) const;
         CallInfo GetCallInfoFromPhysicalFrame() const;
         bool GetThis(Var *pThis, int moduleId) const;
-        Js::Var * GetJavascriptArgs() const;
+        Js::Var * GetJavascriptArgs(bool boxArgsAndDeepCopy) const;
         void **GetCurrentArgv() const;
 
         ScriptContext* GetCurrentScriptContext() const;
@@ -310,7 +310,6 @@ namespace Js
             return previousInterpreterFrameIsFromBailout;
         }
 
-        void SetDeepCopyForArguments() { deepCopyForArgs = true; }
 #if DBG
         static bool ValidateTopJitFrame(Js::ScriptContext* scriptContext);
 #endif
@@ -335,7 +334,6 @@ namespace Js
         bool                    previousInterpreterFrameIsFromBailout : 1;
         bool                    previousInterpreterFrameIsForLoopBody : 1;
         bool                    forceFullWalk               : 1; // ignoring hasCaller
-        bool                    deepCopyForArgs             : 1; // indicates when Var's data should be deep-copied when gathering Arguments for the frame
 
         Var GetThisFromFrame() const;                   // returns 'this' object from the physical frame
         Var GetCurrentArgumentsObject() const;          // returns arguments object from the current frame, which may be virtual (belonging to an inlinee)

lib/Runtime/Language/StackTraceArguments.cpp

@@ -73,7 +73,9 @@ namespace Js {
             if (numberOfArguments > 0) numberOfArguments --; // Don't consider 'this'
             for (int64 j = 0; j < numberOfArguments && j < MaxNumberOfDisplayedArgumentsInStack; j ++)
             {
-                types |= ObjectToTypeCode(walker.GetJavascriptArgs()[j]) << 3*j; // maximal code is 7, so we can use 3 bits to store it
+                // Since the Args are only used to get the type, no need to box the Vars to
+                // move them to the heap from the stack
+                types |= ObjectToTypeCode(walker.GetJavascriptArgs(false /*boxArgsAndDeepCopy*/)[j]) << 3 * j; // maximal code is 7, so we can use 3 bits to store it
             }
             if (numberOfArguments > MaxNumberOfDisplayedArgumentsInStack)
             {

lib/Runtime/Library/ArgumentsObject.cpp

@@ -26,78 +26,7 @@ namespace Js
         stringBuilder->AppendCppLiteral(_u("Object, (Arguments)"));
         return TRUE;
     }
-
-    Var ArgumentsObject::GetCaller(ScriptContext * scriptContext)
-    {
-        JavascriptStackWalker walker(scriptContext);
-
-        if (!this->AdvanceWalkerToArgsFrame(&walker))
-        {
-            return scriptContext->GetLibrary()->GetNull();
-        }
-
-        return ArgumentsObject::GetCaller(scriptContext, &walker, false);
-    }
-
-    Var ArgumentsObject::GetCaller(ScriptContext * scriptContext, JavascriptStackWalker *walker, bool skipGlobal)
-    {
-        // The arguments.caller property is equivalent to callee.caller.arguments - that is, it's the
-        // caller's arguments object (if any). Just fetch the caller and compute its arguments.
-        JavascriptFunction* funcCaller = nullptr;
-
-        while (walker->GetCaller(&funcCaller))
-        {
-            if (walker->IsCallerGlobalFunction())
-            {
-                // Caller is global/eval. If we're in IE9 mode, and the caller is eval,
-                // keep looking. Otherwise, caller is null.
-                if (skipGlobal || walker->IsEvalCaller())
-                {
-                    continue;
-                }
-                funcCaller = nullptr;
-            }
-            break;
-        }
-
-        if (funcCaller == nullptr)
-        {
-            return scriptContext->GetLibrary()->GetNull();
-        }
-
-        AssertMsg(JavascriptOperators::GetTypeId(funcCaller) == TypeIds_Function, "non function caller");
-
-        const CallInfo callInfo = walker->GetCallInfo();
-        uint32 paramCount = callInfo.Count;
-        CallFlags flags = callInfo.Flags;
-
-        if (paramCount == 0 || (flags & CallFlags_Eval))
-        {
-            // The caller is the "global function" or eval, so we return "null".
-            return scriptContext->GetLibrary()->GetNull();
-        }
-
-        if (!walker->GetCurrentFunction()->IsScriptFunction())
-        {
-            // builtin function do not have an argument object - return null.
-            return scriptContext->GetLibrary()->GetNull();
-        }
-
-        // Create new arguments object, everytime this is requested for, with the actuals value.
-        Var args = nullptr;
-
-        args = JavascriptOperators::LoadHeapArguments(
-            funcCaller,
-            paramCount - 1,
-            walker->GetJavascriptArgs(),
-            scriptContext->GetLibrary()->GetNull(),
-            scriptContext->GetLibrary()->GetNull(),
-            scriptContext,
-            /* formalsAreLetDecls */ false);
-
-        return args;
-    }
-
+    
     bool ArgumentsObject::Is(Var aValue)
     {
         return JavascriptOperators::GetTypeId(aValue) == TypeIds_Arguments;

lib/Runtime/Library/ArgumentsObject.h

@@ -18,8 +18,6 @@ namespace Js
         {
             Assert(type->GetTypeId() == TypeIds_Arguments);
         }
-        Var GetCaller(ScriptContext * scriptContext);
-        static Var GetCaller(ScriptContext * scriptContext, JavascriptStackWalker *walker, bool skipGlobal);
 
         static bool Is(Var aValue);
 

lib/Runtime/Library/JavascriptArray.cpp

@@ -4137,6 +4137,10 @@ namespace Js
                     return i;
                 }
             }
+            else if (SparseArraySegment<Var>::IsMissingItem(&element))
+            {
+                AssertOrFailFast(false);
+            }
             else if (includesAlgorithm && JavascriptConversion::SameValueZero(element, search))
             {
                 //Array.prototype.includes
@@ -6667,6 +6671,8 @@ Case0:
             ClearSegmentMap(); // Dump the segmentMap again in case user compare function rebuilds it
             if (hasException)
             {
+                // The current array might have affected due to callbacks. As we have got the exception we should be resetting the missing value.
+                SetHasNoMissingValues(false);
                 head = startSeg;
                 this->InvalidateLastUsedSegment();
             }
@@ -11673,17 +11679,32 @@ Case0:
     T * JavascriptArray::BoxStackInstance(T * instance, bool deepCopy)
     {
         Assert(ThreadContext::IsOnStack(instance));
-        // On the stack, the we reserved a pointer before the object as to store the boxed value
-        T ** boxedInstanceRef = ((T **)instance) - 1;
-        T * boxedInstance = *boxedInstanceRef;
-        if (boxedInstance)
+        T * boxedInstance;
+        T ** boxedInstanceRef;
+        if (!deepCopy)
+        {
+            // On the stack, the we reserved a pointer before the object as to store the boxed value
+            boxedInstanceRef = ((T **)instance) - 1;
+            boxedInstance = *boxedInstanceRef;
+            if (boxedInstance)
+            {
+                return boxedInstance;
+            }
+        }
+        else
         {
-            return boxedInstance;
+            // When doing a deep copy, do not cache the boxed value to ensure that only shallow copies
+            // are reused
+            boxedInstance = nullptr;
+            boxedInstanceRef = nullptr;
         }
 
         const size_t inlineSlotsSize = instance->GetTypeHandler()->GetInlineSlotsSize();
-        if (ThreadContext::IsOnStack(instance->head))
+        if (ThreadContext::IsOnStack(instance->head) || deepCopy)
         {
+            // Reallocate both the object as well as the head segment when the head is on the stack or
+            // when a deep copy is needed. This is to prevent a scenario where box may leave either one
+            // on the stack when both must be on the heap.
             boxedInstance = RecyclerNewPlusZ(instance->GetRecycler(),
                 inlineSlotsSize + sizeof(Js::SparseArraySegmentBase) + instance->head->size * sizeof(typename T::TElement),
                 T, instance, true, deepCopy);
@@ -11697,7 +11718,10 @@ Case0:
             boxedInstance = RecyclerNew(instance->GetRecycler(), T, instance, false, false);
         }
 
-        *boxedInstanceRef = boxedInstance;
+        if (boxedInstanceRef != nullptr)
+        {
+            *boxedInstanceRef = boxedInstance;
+        }
         return boxedInstance;
     }
 

lib/Runtime/Library/JavascriptExternalFunction.cpp

@@ -273,6 +273,14 @@ namespace Js
 
         ScriptContext * scriptContext = externalFunction->type->GetScriptContext();
         AnalysisAssert(scriptContext);
+
+        if (args.Info.Count > USHORT_MAX)
+        {
+            // Due to compat reasons, stdcall external functions expect a ushort count of args.
+            // To support more than this we will need a new API.
+            Js::JavascriptError::ThrowTypeError(scriptContext, JSERR_ArgListTooLarge);
+        }
+
         Var result = nullptr;
         Assert(callInfo.Count > 0);
 
@@ -291,14 +299,14 @@ namespace Js
         {
             BEGIN_LEAVE_SCRIPT(scriptContext)
             {
-                result = externalFunction->stdCallNativeMethod(function, args.Values, args.Info.Count, &info, externalFunction->callbackState);
+                result = externalFunction->stdCallNativeMethod(function, args.Values, static_cast<USHORT>(args.Info.Count), &info, externalFunction->callbackState);
             }
             END_LEAVE_SCRIPT(scriptContext);
         }
 #else
         BEGIN_LEAVE_SCRIPT(scriptContext)
         {
-            result = externalFunction->stdCallNativeMethod(function, args.Values, args.Info.Count, &info, externalFunction->callbackState);
+            result = externalFunction->stdCallNativeMethod(function, args.Values, static_cast<USHORT>(args.Info.Count), &info, externalFunction->callbackState);
         }
         END_LEAVE_SCRIPT(scriptContext);
 #endif
@@ -424,6 +432,13 @@ namespace Js
         }
         else
         {
+            if (args.Info.Count > USHORT_MAX)
+            {
+                // Due to compat reasons, stdcall external functions expect a ushort count of args.
+                // To support more than this we will need a new API.
+                Js::JavascriptError::ThrowTypeError(scriptContext, JSERR_ArgListTooLarge);
+            }
+
             TTDAssert(scriptContext->ShouldPerformRecordAction(), "Check either record/replay before calling!!!");
 
             TTD::EventLog* elog = scriptContext->GetThreadContext()->TTDLog;
@@ -439,7 +454,7 @@ namespace Js
 
             BEGIN_LEAVE_SCRIPT(scriptContext)
             {
-                result = externalFunction->stdCallNativeMethod(function, args.Values, args.Info.Count, &info, externalFunction->callbackState);
+                result = externalFunction->stdCallNativeMethod(function, args.Values, static_cast<ushort>(args.Info.Count), &info, externalFunction->callbackState);
             }
             END_LEAVE_SCRIPT(scriptContext);
 

lib/Runtime/Library/JavascriptFunction.cpp

@@ -878,7 +878,7 @@ namespace Js
         if (overridingNewTarget != nullptr)
         {
             ScriptFunction * scriptFunctionObj = JavascriptOperators::TryFromVar<ScriptFunction>(functionObj);
-            ushort newCount = args.Info.Count;
+            uint newCount = args.Info.Count;
             if (scriptFunctionObj && scriptFunctionObj->GetFunctionInfo()->IsClassConstructor())
             {
                 thisAlreadySpecified = true;
@@ -1097,7 +1097,7 @@ namespace Js
         uint32 actualLength = CallInfo::GetLargeArgCountWithExtraArgs(args.Info.Flags, spreadSize);
 
         // Allocate (if needed) space for the expanded arguments.
-        Arguments outArgs(CallInfo(args.Info.Flags, spreadSize, /* unUsedBool */ false), nullptr);
+        Arguments outArgs(CallInfo(args.Info.Flags, spreadSize), nullptr);
         Var stackArgs[STACK_ARGS_ALLOCA_THRESHOLD];
         size_t outArgsSize = 0;
         if (actualLength > STACK_ARGS_ALLOCA_THRESHOLD)
@@ -2871,7 +2871,6 @@ LABEL1:
         // and foo.arguments[n] will be maintained after this object is returned.
 
         JavascriptStackWalker walker(scriptContext);
-        walker.SetDeepCopyForArguments();
 
         if (walker.WalkToTarget(this))
         {
@@ -2882,12 +2881,13 @@ LABEL1:
             else
             {
                 Var args = nullptr;
-                //Create a copy of the arguments and return it.
+                // Since the arguments will be returned back to script, box the arguments to ensure a copy of
+                // them with their own lifetime (as well as move any from the stack to the heap).
 
                 const CallInfo callInfo = walker.GetCallInfo();
                 args = JavascriptOperators::LoadHeapArguments(
                     this, callInfo.Count - 1,
-                    walker.GetJavascriptArgs(),
+                    walker.GetJavascriptArgs(true /* boxArgsAndDeepCopy */),
                     scriptContext->GetLibrary()->GetNull(),
                     scriptContext->GetLibrary()->GetNull(),
                     scriptContext,

lib/Runtime/Library/JavascriptRegExpConstructor.cpp

@@ -355,8 +355,12 @@ namespace Js
         case PropertyIds::input:
         case PropertyIds::$_:
             //TODO: review: although the 'input' property is marked as readonly, it has a set on V5.8. There is no spec on this.
-            EnsureValues(); // The last match info relies on the last input. Use it before it is changed.
-            this->lastInput = JavascriptConversion::ToString(value, this->GetScriptContext());
+            {
+                auto tempInput = JavascriptConversion::ToString(value, this->GetScriptContext());
+                // Above toString call can cause user code to be called, which may call .match to invalidate our state, ensure that we have proper values in case that happens.
+                EnsureValues();  // The last match info relies on the last input. Use it before it is changed.
+                this->lastInput = tempInput;
+            }
             *result = true;
             return true;
         case PropertyIds::lastMatch:

lib/Runtime/Library/SubString.cpp

@@ -33,7 +33,7 @@ namespace Js
 
         Recycler* recycler = scriptContext->GetRecycler();
 
-        Assert(string->GetLength() >= start + length);
+        AssertOrFailFast(string->GetLength() >= start + length);
         const char16 * subString = string->GetString() + start;
         void const * originalFullStringReference = string->GetOriginalStringReference();
 

test/Function/evenMoreFuncExpr3.baseline

@@ -9,3 +9,4 @@ obj[0].z : proto[0].z
 obj[0].w : proto[0].w
 obj[1].z : undefined
 obj[1].w : undefined
+pass

test/Function/evenMoreFuncExpr3.js

@@ -40,5 +40,11 @@ var a = function x() {
         "use strict";
         x = 1;
     };
-}
-
+};
+
+(function __f_997(__v_4351 = function () {
+        WScript.Echo('pass');
+        return __f_997;
+    }()) {
+    function __f_997() {}
+})();