فهرست منبع

[CVE-2018-8359] Edge - Chakra OOB Write on ProxyEntryPointInfo - Internal

Seth Brenith 7 سال پیش
والد
کامیت
f8bdb180c4

+ 29 - 2
lib/Backend/BailOut.cpp

@@ -506,6 +506,13 @@ uint32 BailOutRecord::GetArgumentsObjectOffset()
     return argumentsObjectOffset;
 }
 
+Js::FunctionEntryPointInfo *BailOutRecord::GetFunctionEntryPointInfo() const
+{
+    Js::EntryPointInfo* result = this->globalBailOutRecordTable->entryPointInfo;
+    AssertOrFailFast(result->IsFunctionEntryPointInfo());
+    return (Js::FunctionEntryPointInfo*)result;
+}
+
 Js::Var BailOutRecord::EnsureArguments(Js::InterpreterStackFrame * newInstance, Js::JavascriptCallStackLayout * layout, Js::ScriptContext* scriptContext, Js::Var* pArgumentsObject) const
 {
     Assert(globalBailOutRecordTable->hasStackArgOpt);
@@ -1706,7 +1713,27 @@ void BailOutRecord::ScheduleFunctionCodeGen(Js::ScriptFunction * function, Js::S
     BailOutRecord * bailOutRecordNotConst = (BailOutRecord *)(void *)bailOutRecord;
     bailOutRecordNotConst->bailOutCount++;
 
-    Js::FunctionEntryPointInfo *entryPointInfo = function->GetFunctionEntryPointInfo();
+    Js::FunctionEntryPointInfo *entryPointInfo = bailOutRecord->GetFunctionEntryPointInfo();
+
+#if DBG
+    // BailOutRecord is not recycler-allocated, so make sure something the recycler can see was keeping the entry point info alive.
+    // We expect the entry point to be kept alive as follows:
+    // 1. The function's current type might still have the same entry point info as when we entered the function (easy case)
+    // 2. The function might have moved to a successor path type, which still keeps the previous type and its entry point info alive
+    // 3. The entry point info might be held by the ThreadContext (QueueFreeOldEntryPointInfoIfInScript):
+    //   a. If the entry point info was replaced on the type that used to hold it (ScriptFunction::ChangeEntryPoint)
+    //   b. If the function's last-added property was deleted and it moved to a previous type (ScriptFunction::ReplaceTypeWithPredecessorType)
+    //   c. If the function's path type got replaced with a dictionary, then all previous entry point infos in that path are queued on the ThreadContext (ScriptFunction::PrepareForConversionToNonPathType)
+    bool foundEntryPoint = false;
+    executeFunction->MapEntryPointsUntil([&](int index, Js::FunctionEntryPointInfo* info)
+    {
+        foundEntryPoint = info == entryPointInfo;
+        return foundEntryPoint;
+    });
+    foundEntryPoint = foundEntryPoint || function->GetScriptContext()->GetThreadContext()->IsOldEntryPointInfo(entryPointInfo);
+    Assert(foundEntryPoint);
+#endif
+
     uint8 callsCount = entryPointInfo->callsCount > 255 ? 255 : static_cast<uint8>(entryPointInfo->callsCount);
     RejitReason rejitReason = RejitReason::None;
     bool reThunk = false;
@@ -2235,7 +2262,7 @@ void BailOutRecord::ScheduleFunctionCodeGen(Js::ScriptFunction * function, Js::S
         if (bailOutRecord->IsForLoopTop() && IR::IsTypeCheckBailOutKind(bailOutRecord->bailOutKind))
         {
             // Disable FieldPRE if we're triggering a type check rejit due to a bailout at the loop top.
-            // Most likely this was caused by a CheckFixedFld that was hoisted from a branch block where 
+            // Most likely this was caused by a CheckFixedFld that was hoisted from a branch block where
             // only certain types flowed, to the loop top, where more types (different or non-equivalent)
             // were flowing in.
             profileInfo->DisableFieldPRE();

+ 4 - 1
lib/Backend/BailOut.h

@@ -209,6 +209,8 @@ public:
     void SetType(BailoutRecordType type) { this->type = type; }
     bool IsShared() const { return type == Shared || type == SharedForLoopTop; }
     bool IsForLoopTop() const { return type == SharedForLoopTop; }
+
+    Js::FunctionEntryPointInfo *GetFunctionEntryPointInfo() const;
 protected:
     struct BailOutReturnValue
     {
@@ -237,7 +239,7 @@ protected:
 
     static void UpdatePolymorphicFieldAccess(Js::JavascriptFunction *  function, BailOutRecord const * bailOutRecord);
 
-    static void ScheduleFunctionCodeGen(Js::ScriptFunction * function, Js::ScriptFunction * innerMostInlinee, BailOutRecord const * bailOutRecord, IR::BailOutKind bailOutKind, 
+    static void ScheduleFunctionCodeGen(Js::ScriptFunction * function, Js::ScriptFunction * innerMostInlinee, BailOutRecord const * bailOutRecord, IR::BailOutKind bailOutKind,
                                         uint32 actualBailOutOffset, Js::ImplicitCallFlags savedImplicitCallFlags, void * returnAddress);
     static void ScheduleLoopBodyCodeGen(Js::ScriptFunction * function, Js::ScriptFunction * innerMostInlinee, BailOutRecord const * bailOutRecord, IR::BailOutKind bailOutKind);
     static void CheckPreemptiveRejit(Js::FunctionBody* executeFunction, IR::BailOutKind bailOutKind, BailOutRecord* bailoutRecord, uint8& callsOrIterationsCount, int loopNumber);
@@ -416,6 +418,7 @@ struct GlobalBailOutRecordDataTable
     // The offset to 'registerSaveSpace' is hard-coded in LinearScanMD::SaveAllRegisters, so let this be the first member variable
     Js::Var *registerSaveSpace;
     GlobalBailOutRecordDataRow *globalBailOutRecordDataRows;
+    Js::EntryPointInfo *entryPointInfo;
     uint32 length;
     uint32 size;
     int32  firstActualStackOffset;

+ 8 - 1
lib/Backend/FunctionJITTimeInfo.cpp

@@ -27,6 +27,7 @@ FunctionJITTimeInfo::BuildJITTimeData(
     jitData->isInlined = codeGenData->GetIsInlined();
     jitData->weakFuncRef = (intptr_t)codeGenData->GetWeakFuncRef();
     jitData->inlineesBv = (BVFixedIDL*)(const BVFixed*)codeGenData->inlineesBv;
+    jitData->entryPointInfoAddr = (intptr_t)codeGenData->GetEntryPointInfo();
 
     if (!codeGenData->GetFunctionBody() || !codeGenData->GetFunctionBody()->GetByteCode())
     {
@@ -62,7 +63,7 @@ FunctionJITTimeInfo::BuildJITTimeData(
             Assert(defaultEntryPointInfo->IsFunctionEntryPointInfo());
             Js::FunctionEntryPointInfo *functionEntryPointInfo = static_cast<Js::FunctionEntryPointInfo*>(defaultEntryPointInfo);
             jitData->callsCountAddress = (intptr_t)&functionEntryPointInfo->callsCount;
-                
+
             jitData->sharedPropertyGuards = codeGenData->sharedPropertyGuards;
             jitData->sharedPropGuardCount = codeGenData->sharedPropertyGuardCount;
         }
@@ -203,6 +204,12 @@ FunctionJITTimeInfo::GetFunctionInfoAddr() const
     return m_data.functionInfoAddr;
 }
 
+intptr_t
+FunctionJITTimeInfo::GetEntryPointInfoAddr() const
+{
+    return m_data.entryPointInfoAddr;
+}
+
 intptr_t
 FunctionJITTimeInfo::GetWeakFuncRef() const
 {

+ 1 - 0
lib/Backend/FunctionJITTimeInfo.h

@@ -27,6 +27,7 @@ public:
     JITTimeFunctionBody * GetBody() const;
     bool IsPolymorphicCallSite(Js::ProfileId profiledCallSiteId) const;
     intptr_t GetFunctionInfoAddr() const;
+    intptr_t GetEntryPointInfoAddr() const;
     intptr_t GetWeakFuncRef() const;
     uint GetLocalFunctionId() const;
     uint GetSourceContextId() const;

+ 5 - 4
lib/Backend/LinearScan.cpp

@@ -1288,6 +1288,7 @@ LinearScan::EnsureGlobalBailOutRecordTable(Func *func)
     if (globalBailOutRecordDataTable == nullptr)
     {
         globalBailOutRecordDataTable = globalBailOutRecordTables[inlineeID] = NativeCodeDataNew(allocator, GlobalBailOutRecordDataTable);
+        globalBailOutRecordDataTable->entryPointInfo = (Js::EntryPointInfo*)func->GetWorkItem()->GetJITTimeInfo()->GetEntryPointInfoAddr();
         globalBailOutRecordDataTable->length = globalBailOutRecordDataTable->size = 0;
         globalBailOutRecordDataTable->isInlinedFunction = !isTopFunc;
         globalBailOutRecordDataTable->hasNonSimpleParams = func->GetHasNonSimpleParams();
@@ -2603,14 +2604,14 @@ LinearScan::FindReg(Lifetime *newLifetime, IR::RegOpnd *regOpnd, bool force)
                 // Avoid the temp reg that we have loaded in this basic block
                 regsBvNoTemps.Minus(this->tempRegs);
             }
-            
+
             BitVector regsBvNoTempsNoCallee = regsBvNoTemps;
             // Try to find a non-callee saved reg so that we don't have to save it in prolog
             regsBvNoTempsNoCallee.Minus(this->calleeSavedRegs);
 
             // Allocate a non-callee saved reg from the other end of the bit vector so that it can keep live for longer
             regIndex = regsBvNoTempsNoCallee.GetPrevBit();
-            
+
             if (regIndex == BVInvalidIndex)
             {
                 // If we don't have any non-callee saved reg then get the first available callee saved reg so that prolog can store adjacent registers
@@ -3730,7 +3731,7 @@ LinearScan::ProcessSecondChanceBoundaryHelper(IR::BranchInstr *branchInstr, IR::
                     }
                     else
                     {
-                        // Dead code after the unconditional branch causes the currentBlock data to be freed later on...  
+                        // Dead code after the unconditional branch causes the currentBlock data to be freed later on...
                         // Deep copy in this case.
                         branchLabel->m_loweredBasicBlock = this->currentBlock->Clone(this->tempAlloc);
                     }
@@ -4730,7 +4731,7 @@ IR::Instr * LinearScan::GetIncInsertionPoint(IR::Instr *instr)
 }
 
 void LinearScan::DynamicStatsInstrument()
-{    
+{
     {
         IR::Instr *firstInstr = this->func->m_headInstr;
     IR::MemRefOpnd *memRefOpnd = IR::MemRefOpnd::New(this->func->GetJITFunctionBody()->GetCallCountStatsAddr(), TyUint32, this->func);

+ 1 - 0
lib/JITIDL/JITTypes.h

@@ -680,6 +680,7 @@ typedef struct FunctionJITTimeDataIDL
     CHAKRA_PTR functionInfoAddr;
     CHAKRA_PTR callsCountAddress;
     CHAKRA_PTR weakFuncRef;
+    CHAKRA_PTR entryPointInfoAddr;
 } FunctionJITTimeDataIDL;
 
 #if !FLOATVAR

+ 0 - 27
lib/Runtime/Base/FunctionBody.cpp

@@ -2088,33 +2088,6 @@ namespace Js
         this->SetAuxPtr<AuxPointerType::FunctionObjectTypeList>(list);
     }
 
-    template <typename Fn>
-    void FunctionProxy::MapFunctionObjectTypes(Fn func)
-    {
-        FunctionTypeWeakRefList* functionObjectTypeList = this->GetFunctionObjectTypeList();
-        if (functionObjectTypeList != nullptr)
-        {
-            functionObjectTypeList->Map([&](int, FunctionTypeWeakRef* typeWeakRef)
-            {
-                if (typeWeakRef)
-                {
-                    ScriptFunctionType* type = typeWeakRef->Get();
-                    if (type)
-                    {
-                        func(type);
-                    }
-                }
-            });
-        }
-
-        if (this->deferredPrototypeType)
-        {
-            func(this->deferredPrototypeType);
-        }
-        // NOTE: We deliberately do not map the undeferredFunctionType here, since it's in the list
-        // of registered function object types we processed above.
-    }
-
     FunctionProxy::FunctionTypeWeakRefList* FunctionProxy::EnsureFunctionObjectTypeList()
     {
         FunctionTypeWeakRefList* functionObjectTypeList = this->GetFunctionObjectTypeList();

+ 33 - 7
lib/Runtime/Base/FunctionBody.h

@@ -222,7 +222,7 @@ namespace Js
     // main and JIT threads.
     class EntryPointInfo : public ProxyEntryPointInfo
     {
-        
+
     private:
         enum State : BYTE
         {
@@ -296,7 +296,6 @@ namespace Js
 
     public:
         virtual void Finalize(bool isShutdown) override;
-        virtual bool IsFunctionEntryPointInfo() const override { return true; }
 
 #if ENABLE_NATIVE_CODEGEN
         NativeEntryPointData * EnsureNativeEntryPointData();
@@ -572,9 +571,9 @@ namespace Js
         void ResetOnLazyBailoutFailure();
         void OnNativeCodeInstallFailure();
         virtual void ResetOnNativeCodeInstallFailure() = 0;
-               
-        void FreeJitTransferData();        
-        bool ClearEquivalentTypeCaches();        
+
+        void FreeJitTransferData();
+        bool ClearEquivalentTypeCaches();
 
         virtual void Invalidate(bool prolongEntryPoint) { Assert(false); }
         InlineeFrameRecord* FindInlineeFrame(void* returnAddress);
@@ -624,6 +623,8 @@ namespace Js
     public:
         FunctionEntryPointInfo(FunctionProxy * functionInfo, Js::JavascriptMethod method, ThreadContext* context);
 
+        virtual bool IsFunctionEntryPointInfo() const override { return true; }
+
         bool ExecutedSinceCallCountCollection() const;
         void CollectCallCounts();
 
@@ -1063,8 +1064,33 @@ namespace Js
         FunctionTypeWeakRefList* GetFunctionObjectTypeList() const;
         void SetFunctionObjectTypeList(FunctionTypeWeakRefList* list);
         void RegisterFunctionObjectType(ScriptFunctionType* functionType);
+
         template <typename Fn>
-        void MapFunctionObjectTypes(Fn func);
+        void MapFunctionObjectTypes(Fn func)
+        {
+            FunctionTypeWeakRefList* functionObjectTypeList = this->GetFunctionObjectTypeList();
+            if (functionObjectTypeList != nullptr)
+            {
+                functionObjectTypeList->Map([&](int, FunctionTypeWeakRef* typeWeakRef)
+                {
+                    if (typeWeakRef)
+                    {
+                        ScriptFunctionType* type = typeWeakRef->Get();
+                        if (type)
+                        {
+                            func(type);
+                        }
+                    }
+                });
+            }
+
+            if (this->deferredPrototypeType)
+            {
+                func(this->deferredPrototypeType);
+            }
+            // NOTE: We deliberately do not map the undeferredFunctionType here, since it's in the list
+            // of registered function object types we processed above.
+        }
 
         static uint GetOffsetOfDeferredPrototypeType() { return static_cast<uint>(offsetof(Js::FunctionProxy, deferredPrototypeType)); }
         static Js::ScriptFunctionType * EnsureFunctionProxyDeferredPrototypeType(FunctionProxy * proxy)
@@ -2196,7 +2222,7 @@ namespace Js
 #if DYNAMIC_INTERPRETER_THUNK
         void GenerateDynamicInterpreterThunk();
 #endif
-      
+
         Js::JavascriptMethod GetEntryPoint(ProxyEntryPointInfo* entryPoint) const { return entryPoint->jsMethod; }
         void CaptureDynamicProfileState(FunctionEntryPointInfo* entryPointInfo);
 #if ENABLE_DEBUG_CONFIG_OPTIONS

+ 8 - 3
lib/Runtime/Base/ThreadContext.cpp

@@ -721,7 +721,7 @@ bool ThreadContext::ThreadContextRecyclerTelemetryHostInterface::TransmitTelemet
 
 bool ThreadContext::ThreadContextRecyclerTelemetryHostInterface::IsThreadBound() const
 {
-    return this->tc->IsThreadBound(); 
+    return this->tc->IsThreadBound();
 }
 
 
@@ -2641,11 +2641,16 @@ ThreadContext::DoExpirableCollectModeStackWalk()
             if (javascriptFunction != nullptr && Js::ScriptFunction::Test(javascriptFunction))
             {
                 Js::ScriptFunction* scriptFunction = (Js::ScriptFunction*) javascriptFunction;
-                Js::FunctionEntryPointInfo* entryPointInfo =  scriptFunction->GetFunctionEntryPointInfo();
-                entryPointInfo->SetIsObjectUsed();
+
                 scriptFunction->GetFunctionBody()->MapEntryPoints([](int index, Js::FunctionEntryPointInfo* entryPoint){
                     entryPoint->SetIsObjectUsed();
                 });
+
+                // Make sure we marked the current one when iterating all entry points
+                Js::ProxyEntryPointInfo* entryPointInfo = scriptFunction->GetEntryPointInfo();
+                Assert(entryPointInfo == nullptr
+                    || !entryPointInfo->IsFunctionEntryPointInfo()
+                    || ((Js::FunctionEntryPointInfo*)entryPointInfo)->IsObjectUsed());
             }
         }
     }

+ 12 - 0
lib/Runtime/Base/ThreadContext.h

@@ -1466,6 +1466,18 @@ public:
         }
     }
 
+    bool IsOldEntryPointInfo(Js::ProxyEntryPointInfo* entryPointInfo)
+    {
+        Js::FunctionEntryPointInfo* current = this->recyclableData->oldEntryPointInfo;
+        while (current != nullptr)
+        {
+            if (current == entryPointInfo)
+                return true;
+            current = current->nextEntryPoint;
+        }
+        return false;
+    }
+
     static bool IsOnStack(void const *ptr);
     _NOINLINE bool IsStackAvailable(size_t size, bool* isInterrupt = nullptr);
     _NOINLINE bool IsStackAvailableNoThrow(size_t size = Js::Constants::MinStackDefault);

+ 38 - 1
lib/Runtime/Library/ScriptFunction.cpp

@@ -202,6 +202,29 @@ using namespace Js;
         return type;
     }
 
+    void ScriptFunction::PrepareForConversionToNonPathType()
+    {
+        // We have a path type handler that is currently responsible for holding some number of entry point infos alive.
+        // The last one will be copied on to the new dictionary type handler, but if any previous instances in the path
+        // are holding different entry point infos, those need to be copied to somewhere safe.
+        // The number of entry points is likely low compared to length of path, so iterate those instead.
+
+        ProxyEntryPointInfo* entryPointInfo = this->GetScriptFunctionType()->GetEntryPointInfo();
+
+        this->GetFunctionProxy()->MapFunctionObjectTypes([&](ScriptFunctionType* functionType)
+        {
+            CopyEntryPointInfoToThreadContextIfNecessary(functionType->GetEntryPointInfo(), entryPointInfo);
+        });
+    }
+
+    void ScriptFunction::ReplaceTypeWithPredecessorType(DynamicType * previousType)
+    {
+        ProxyEntryPointInfo* oldEntryPointInfo = this->GetScriptFunctionType()->GetEntryPointInfo();
+        __super::ReplaceTypeWithPredecessorType(previousType);
+        ProxyEntryPointInfo* newEntryPointInfo = this->GetScriptFunctionType()->GetEntryPointInfo();
+        CopyEntryPointInfoToThreadContextIfNecessary(oldEntryPointInfo, newEntryPointInfo);
+    }
+
     bool ScriptFunction::HasFunctionBody()
     {
         // for asmjs we want to first check if the FunctionObject has a function body. Check that the function is not deferred
@@ -221,6 +244,20 @@ using namespace Js;
         this->GetScriptFunctionType()->ChangeEntryPoint(entryPointInfo, entryPoint, isAsmJS);
     }
 
+    void ScriptFunction::CopyEntryPointInfoToThreadContextIfNecessary(ProxyEntryPointInfo* oldEntryPointInfo, ProxyEntryPointInfo* newEntryPointInfo)
+    {
+        if (oldEntryPointInfo
+            && oldEntryPointInfo != newEntryPointInfo
+            && oldEntryPointInfo->SupportsExpiration())
+        {
+            // The old entry point could be executing so we need root it to make sure
+            // it isn't prematurely collected. The rooting is done by queuing it up on the threadContext
+            ThreadContext* threadContext = ThreadContext::GetContextForCurrentThread();
+
+            threadContext->QueueFreeOldEntryPointInfoIfInScript((FunctionEntryPointInfo*)oldEntryPointInfo);
+        }
+    }
+
     FunctionProxy * ScriptFunction::GetFunctionProxy() const
     {
         Assert(this->functionInfo->HasBody());
@@ -586,7 +623,7 @@ using namespace Js;
 
         TTD::NSSnapObjects::StdExtractSetKindSpecificInfo<TTD::NSSnapObjects::SnapScriptFunctionInfo*, TTD::NSSnapObjects::SnapObjectType::SnapScriptFunctionObject>(objData, ssfi);
     }
-    
+
     // TODO:  Fixup function definition - something funky w/ header file includes - cycles?
     void ScriptFunction::ExtractSnapObjectDataIntoSnapScriptFunctionInfo(/*TTD::NSSnapObjects::SnapScriptFunctionInfo* */ void* snapScriptFunctionInfo, TTD::SlabAllocator& alloc)
     {

+ 8 - 2
lib/Runtime/Library/ScriptFunction.h

@@ -86,11 +86,15 @@ namespace Js
         static ScriptFunction * OP_NewScFunc(FrameDisplay *environment, FunctionInfoPtrPtr infoRef);
         static ScriptFunction * OP_NewScFuncHomeObj(FrameDisplay *environment, FunctionInfoPtrPtr infoRef, Var homeObj);
 
+        static void CopyEntryPointInfoToThreadContextIfNecessary(ProxyEntryPointInfo* oldEntryPointInfo, ProxyEntryPointInfo* newEntryPointInfo);
+
         ProxyEntryPointInfo* GetEntryPointInfo() const;
         FunctionEntryPointInfo* GetFunctionEntryPointInfo() const
         {
             Assert(this->GetFunctionProxy()->IsDeferred() == FALSE);
-            return (FunctionEntryPointInfo*) this->GetEntryPointInfo();
+            ProxyEntryPointInfo* result = this->GetEntryPointInfo();
+            Assert(result->IsFunctionEntryPointInfo());
+            return (FunctionEntryPointInfo*)result;
         }
 
         FunctionProxy * GetFunctionProxy() const;
@@ -112,6 +116,8 @@ namespace Js
         JavascriptMethod UpdateUndeferredBody(FunctionBody* newFunctionInfo);
 
         virtual ScriptFunctionType * DuplicateType() override;
+        virtual void PrepareForConversionToNonPathType() override;
+        virtual void ReplaceTypeWithPredecessorType(DynamicType * previousType) override;
 
         virtual Var GetSourceString() const;
         virtual JavascriptString * EnsureSourceString();
@@ -199,7 +205,7 @@ namespace Js
         WebAssemblyMemory* GetWebAssemblyMemory() const;
 
         virtual bool IsWasmFunction() const override { return true; }
-    protected:        
+    protected:
         DEFINE_VTABLE_CTOR(WasmScriptFunction, AsmJsScriptFunction);
         DEFINE_MARSHAL_OBJECT_TO_SCRIPT_CONTEXT(WasmScriptFunction);
     private:

+ 6 - 1
lib/Runtime/Types/DynamicObject.cpp

@@ -128,7 +128,7 @@ namespace Js
         }
         else
         {
-            // Otherwise, assert that there is either 
+            // Otherwise, assert that there is either
             // - no object array to deep copy
             // - an object array, but no deep copy needed
             // - data in the objectArray member, but it is inline slot data
@@ -526,6 +526,11 @@ namespace Js
         return RecyclerNew(GetRecycler(), DynamicType, this->GetDynamicType());
     }
 
+    void DynamicObject::PrepareForConversionToNonPathType()
+    {
+        // Nothing to do in base class
+    }
+
     /*
     *   DynamicObject::IsTypeHandlerCompatibleForObjectHeaderInlining
     *   -   Checks if the TypeHandlers are compatible for transition from oldTypeHandler to newTypeHandler

+ 6 - 5
lib/Runtime/Types/DynamicObject.h

@@ -69,7 +69,7 @@ namespace Js
         friend class JavascriptNativeArray; // for xplat offsetof field access
         friend class JavascriptOperators;
         friend class JavascriptLibrary;
-        friend class ModuleNamespace; // for slot setting.       
+        friend class ModuleNamespace; // for slot setting.
 
 #if ENABLE_OBJECT_SOURCE_TRACKING
     public:
@@ -152,7 +152,7 @@ namespace Js
         void EnsureSlots(int oldCount, int newCount, ScriptContext * scriptContext, DynamicTypeHandler * newTypeHandler = nullptr);
         void EnsureSlots(int newCount, ScriptContext *scriptContext);
         void ReplaceType(DynamicType * type);
-        void ReplaceTypeWithPredecessorType(DynamicType * previousType);
+        virtual void ReplaceTypeWithPredecessorType(DynamicType * previousType);
 
         DynamicTypeHandler * GetTypeHandler() const;
 
@@ -304,6 +304,7 @@ namespace Js
         virtual BOOL IsCrossSiteObject() const { return FALSE; }
 
         virtual DynamicType* DuplicateType();
+        virtual void PrepareForConversionToNonPathType();
         static bool IsTypeHandlerCompatibleForObjectHeaderInlining(DynamicTypeHandler * oldTypeHandler, DynamicTypeHandler * newTypeHandler);
 
         void ChangeType();
@@ -338,7 +339,7 @@ namespace Js
         void SetArrayCallSiteIndex(ProfileId profileId);
 
         static DynamicObject * BoxStackInstance(DynamicObject * instance, bool deepCopy);
-        
+
     private:
         ArrayObject* EnsureObjectArray();
         ArrayObject* GetObjectArrayOrFlagsAsArray() const { return objectArray; }
@@ -375,8 +376,8 @@ namespace Js
     public:
         virtual VTableValue DummyVirtualFunctionToHinderLinkerICF()
         {
-            // This virtual function hinders linker to do ICF vtable of this class with other classes. 
-            // ICF vtable causes unexpected behavior in type check code. Objects uses vtable as identify should 
+            // This virtual function hinders linker to do ICF vtable of this class with other classes.
+            // ICF vtable causes unexpected behavior in type check code. Objects uses vtable as identify should
             // override this function and return a unique value.
             return VTableValue::VtableDynamicObject;
         }

+ 25 - 21
lib/Runtime/Types/PathTypeHandler.cpp

@@ -145,7 +145,7 @@ namespace Js
         }
     }
 
-    PathTypeMultiSuccessorInfo::PathTypeMultiSuccessorInfo(Recycler * recycler, const PathTypeSuccessorKey key, RecyclerWeakReference<DynamicType> * typeWeakRef) 
+    PathTypeMultiSuccessorInfo::PathTypeMultiSuccessorInfo(Recycler * recycler, const PathTypeSuccessorKey key, RecyclerWeakReference<DynamicType> * typeWeakRef)
     {
         this->propertySuccessors = RecyclerNew(recycler, PropertySuccessorsMap, recycler, 3);
         this->propertySuccessors->Item(key, typeWeakRef);
@@ -250,7 +250,7 @@ namespace Js
     {
 #if DBG
         DynamicType * successorType = typeWeakRef->Get();
-        AssertMsg(!successorType || !successorType->GetTypeHandler()->IsPathTypeHandler() || 
+        AssertMsg(!successorType || !successorType->GetTypeHandler()->IsPathTypeHandler() ||
                   PathTypeHandlerBase::FromTypeHandler(successorType->GetTypeHandler())->GetPredecessorType() == type, "We're using a successor that has a different predecessor?");
 #endif
         if (this->successorInfo == nullptr)
@@ -456,7 +456,7 @@ namespace Js
 
         if(typePath->GetIsUsedFixedFieldAt(slotIndex, objectSlotCount))
         {
-            // We are adding a new value where some other instance already has an existing value.  If this is a fixed 
+            // We are adding a new value where some other instance already has an existing value.  If this is a fixed
             // field we must clear the bit. If the value was hard coded in the JIT-ed code, we must invalidate the guards.
 
             // Invalidate any JIT-ed code that hard coded this method. No need to invalidate store field
@@ -472,16 +472,16 @@ namespace Js
         Assert(HasSingletonInstanceOnlyIfNeeded(/*typePath*/));
         if(objectSlotCount == typePath->GetMaxInitializedLength())
         {
-            // We have now reached the most advanced instance along this path.  If this instance is not the singleton instance, 
-            // then the former singleton instance (if any) is no longer a singleton.  This instance could be the singleton 
+            // We have now reached the most advanced instance along this path.  If this instance is not the singleton instance,
+            // then the former singleton instance (if any) is no longer a singleton.  This instance could be the singleton
             // instance, if we just happen to set (overwrite) its last property.
 
             // This is perhaps the most fragile point of fixed fields on path types.  If we cleared the singleton instance
             // while some fields remained fixed, the instance would be collectible, and yet some code would expect to see
-            // values and call methods on it.  Clearly, a recipe for disaster.  We rely on the fact that we always add 
-            // properties to (pre-initialized) type handlers in the order they appear on the type path.  By the time 
-            // we reach the singleton instance, all fixed fields will have been invalidated.  Otherwise, some fields 
-            // could remain fixed (or even uninitialized) and we would have to spin off a loop here to invalidate any 
+            // values and call methods on it.  Clearly, a recipe for disaster.  We rely on the fact that we always add
+            // properties to (pre-initialized) type handlers in the order they appear on the type path.  By the time
+            // we reach the singleton instance, all fixed fields will have been invalidated.  Otherwise, some fields
+            // could remain fixed (or even uninitialized) and we would have to spin off a loop here to invalidate any
             // remaining fixed fields - a rather unfortunate overhead.
             typePath->ClearSingletonInstance();
         }
@@ -1073,7 +1073,7 @@ namespace Js
     BOOL PathTypeHandlerBase::SetAccessorsHelper(DynamicObject* instance, PropertyId propertyId, ObjectSlotAttributes * attributes, PathTypeSetterSlotIndex * setters, Var getter, Var setter, PropertyOperationFlags flags)
     {
         if (instance->GetType()->IsExternal() || instance->GetScriptContext()->IsScriptContextInDebugMode() || PHASE_OFF1(ShareAccessorTypesPhase))
-        {         
+        {
 #ifdef PROFILE_TYPES
             instance->GetScriptContext()->convertPathToDictionaryAccessorsCount++;
 #endif
@@ -1120,7 +1120,7 @@ namespace Js
             {
                 getter = CanonicalizeAccessor(getter, library);
                 setter = CanonicalizeAccessor(setter, library);
-  
+
                 if (!setters || setters[propertyIndex] == NoSetterSlot)
                 {
                     // We'll add 1 property to the type, so check the limit.
@@ -1307,7 +1307,7 @@ namespace Js
             Assert(CanConvertToSimpleDictionaryType());
 
             // Convert to new shared type with shared simple dictionary type handler and call operation on it.
-            SimpleDictionaryTypeHandlerWithNonExtensibleSupport * newTypeHandler = 
+            SimpleDictionaryTypeHandlerWithNonExtensibleSupport * newTypeHandler =
                 ConvertToSimpleDictionaryType<SimpleDictionaryTypeHandlerWithNonExtensibleSupport>(instance, this->GetPathLength(), true);
 
             Assert(newTypeHandler->GetMayBecomeShared() && !newTypeHandler->GetIsShared());
@@ -1368,6 +1368,8 @@ namespace Js
         ScriptContext* scriptContext = instance->GetScriptContext();
         Recycler* recycler = scriptContext->GetRecycler();
 
+        instance->PrepareForConversionToNonPathType();
+
         PathTypeHandlerBase * oldTypeHandler;
 
         // Ideally 'this' and oldTypeHandler->GetTypeHandler() should be same
@@ -1548,6 +1550,8 @@ namespace Js
         ScriptContext* scriptContext = instance->GetScriptContext();
         Recycler* recycler = scriptContext->GetRecycler();
 
+        instance->PrepareForConversionToNonPathType();
+
         // Ideally 'this' and oldTypeHandler->GetTypeHandler() should be same
         // But we can have calls from external DOM objects, which requests us to replace the type of the
         // object with a new type. And in such cases, this API gets called with oldTypeHandler and the
@@ -2036,13 +2040,13 @@ namespace Js
                     }
                 }
             }
-            else 
+            else
             {
                 if (key.GetAttributes() != ObjectSlotAttr_Default && oldAttributes == nullptr)
                 {
                     newAttributes = this->UpdateAttributes(recycler, nullptr, oldPathSize, newTypePath->GetPathSize());
                 }
-            
+
                 if ((key.GetAttributes() == ObjectSlotAttr_Setter) && oldSetters == nullptr)
                 {
                     newSetters = this->UpdateSetterSlots(recycler, nullptr, oldPathSize, newTypePath->GetPathSize());
@@ -2590,7 +2594,7 @@ namespace Js
         }
         clonedTypeHandler->SetMayBecomeShared();
         clonedTypeHandler->CopyPropertyTypes(PropertyTypesWritableDataOnly | PropertyTypesWritableDataOnlyDetection | PropertyTypesHasSpecialProperties, this->GetPropertyTypes());
-        
+
         return clonedTypeHandler;
     }
 
@@ -2690,7 +2694,7 @@ namespace Js
                 Js::PropertyId propertyId = GetPropertyId(scriptContext, propertyIndex);
                 ObjectSlotAttributes attr = attributes ? attributes[propertyIndex] : ObjectSlotAttr_Default;
                 cachedDynamicType = newTypeHandler->PromoteType<false>(cachedDynamicType, PathTypeSuccessorKey(propertyId, attr), true, scriptContext, instance, &propertyIndex);
-                newTypeHandler = PathTypeHandlerBase::FromTypeHandler(cachedDynamicType->GetTypeHandler());             
+                newTypeHandler = PathTypeHandlerBase::FromTypeHandler(cachedDynamicType->GetTypeHandler());
                 if (attr == ObjectSlotAttr_Setter)
                 {
                     newTypeHandler->SetSetterSlot(newTypeHandler->GetTypePath()->LookupInline(propertyId, newTypeHandler->GetPathLength()), (PathTypeSetterSlotIndex)(newTypeHandler->GetPathLength() - 1));
@@ -3354,7 +3358,7 @@ namespace Js
     {
         uint32 plength = this->GetPathLength();
         ObjectSlotAttributes * attributes = this->GetAttributeArray();
-        
+
         for(uint32 index = 0; index < plength; ++index)
         {
             ObjectSlotAttributes attr = attributes ? attributes[index] : ObjectSlotAttr_Default;
@@ -3493,7 +3497,7 @@ namespace Js
             return true;
         }
 
-        ObjectSlotAttributes attr = 
+        ObjectSlotAttributes attr =
             (ObjectSlotAttributes)(value ? (attributes[propertyIndex] | ObjectSlotAttr_Configurable) : (attributes[propertyIndex] & ~ObjectSlotAttr_Configurable));
         return SetAttributesHelper(instance, propertyId, propertyIndex, attributes, attr);
     }
@@ -3520,7 +3524,7 @@ namespace Js
             return true;
         }
 
-        ObjectSlotAttributes attr = 
+        ObjectSlotAttributes attr =
             (ObjectSlotAttributes)(value ? (attributes[propertyIndex] | ObjectSlotAttr_Enumerable) : (attributes[propertyIndex] & ~ObjectSlotAttr_Enumerable));
         return SetAttributesHelper(instance, propertyId, propertyIndex, attributes, attr);
     }
@@ -3547,7 +3551,7 @@ namespace Js
             return true;
         }
 
-        ObjectSlotAttributes attr = 
+        ObjectSlotAttributes attr =
             (ObjectSlotAttributes)(value ? (attributes[propertyIndex] | ObjectSlotAttr_Writable) : (attributes[propertyIndex] & ~ObjectSlotAttr_Writable));
         return SetAttributesHelper(instance, propertyId, propertyIndex, attributes, attr);
     }
@@ -3652,7 +3656,7 @@ namespace Js
         {
             // PropertyAttributes is only one byte so it can't carry out data about whether this is an accessor.
             // Accessors must be cached differently than normal properties, so if we want to cache this we must
-            // do so here rather than in the caller. However, caching here would require passing originalInstance and 
+            // do so here rather than in the caller. However, caching here would require passing originalInstance and
             // requestContext through a wide variety of call paths to this point (like we do for GetProperty), for
             // very little improvement. For now, just block caching this case.
             PropertyValueInfo::SetNoCache(info, instance);

+ 1 - 10
lib/Runtime/Types/ScriptFunctionType.cpp

@@ -60,16 +60,7 @@ namespace Js
         }
 
         ProxyEntryPointInfo* oldEntryPointInfo = this->GetEntryPointInfo();
-        if (oldEntryPointInfo
-            && oldEntryPointInfo != entryPointInfo
-            && oldEntryPointInfo->SupportsExpiration())
-        {
-            // The old entry point could be executing so we need root it to make sure
-            // it isn't prematurely collected. The rooting is done by queuing it up on the threadContext
-            ThreadContext* threadContext = ThreadContext::GetContextForCurrentThread();
-
-            threadContext->QueueFreeOldEntryPointInfoIfInScript((FunctionEntryPointInfo*)oldEntryPointInfo);
-        }
+        ScriptFunction::CopyEntryPointInfoToThreadContextIfNecessary(oldEntryPointInfo, entryPointInfo);
 
         this->SetEntryPointInfo(entryPointInfo);
     }