[1.11>master] [MERGE #5936 @MikeHolman] February 2019 Security Update

Merge pull request #5936 from MikeHolman:servicing/1902_1_11

February 2019 Security Update that addresses the following issues in ChakraCore:

CVE-2019-0590
CVE-2019-0591
CVE-2019-0593
CVE-2019-0605
CVE-2019-0607
CVE-2019-0610
CVE-2019-0640
CVE-2019-0642
CVE-2019-0644
CVE-2019-0648
CVE-2019-0649
CVE-2019-0651
CVE-2019-0655
CVE-2019-0658

Author: MikeHolman

lib/Backend/CodeGenWorkItem.cpp

@@ -210,19 +210,7 @@ void CodeGenWorkItem::OnWorkItemProcessFail(NativeCodeGenerator* codeGen)
 #if PDATA_ENABLED & defined(_WIN32)
         if (this->entryPointInfo)
         {
-            XDataAllocation * xdataAllocation = this->entryPointInfo->GetNativeEntryPointData()->GetXDataInfo();
-            if (xdataAllocation)
-            {
-                void* functionTable = xdataAllocation->functionTable;
-                if (functionTable)
-                {
-                    if (!DelayDeletingFunctionTable::AddEntry(functionTable))
-                    {
-                        PHASE_PRINT_TESTTRACE1(Js::XDataPhase, _u("[%d]OnWorkItemProcessFail: Failed to add to slist, table: %llx\n"), GetCurrentThreadId(), functionTable);
-                        DelayDeletingFunctionTable::DeleteFunctionTable(functionTable);
-                    }
-                }
-            }
+            this->entryPointInfo->GetNativeEntryPointData()->CleanupXDataInfo();
         }
 #endif
         codeGen->FreeNativeCodeGenAllocation(this->allocation->allocation->address);

lib/Backend/Func.cpp

@@ -1665,14 +1665,14 @@ Func::LinkCtorCacheToPropertyId(Js::PropertyId propertyId, JITTimeConstructorCac
 
 JITTimeConstructorCache* Func::GetConstructorCache(const Js::ProfileId profiledCallSiteId)
 {
-    Assert(profiledCallSiteId < GetJITFunctionBody()->GetProfiledCallSiteCount());
+    AssertOrFailFast(profiledCallSiteId < GetJITFunctionBody()->GetProfiledCallSiteCount());
     Assert(this->constructorCaches != nullptr);
     return this->constructorCaches[profiledCallSiteId];
 }
 
 void Func::SetConstructorCache(const Js::ProfileId profiledCallSiteId, JITTimeConstructorCache* constructorCache)
 {
-    Assert(profiledCallSiteId < GetJITFunctionBody()->GetProfiledCallSiteCount());
+    AssertOrFailFast(profiledCallSiteId < GetJITFunctionBody()->GetProfiledCallSiteCount());
     Assert(constructorCache != nullptr);
     Assert(this->constructorCaches != nullptr);
     Assert(this->constructorCaches[profiledCallSiteId] == nullptr);

lib/Backend/FunctionJITTimeInfo.cpp

@@ -327,11 +327,11 @@ FunctionJITTimeInfo::GetRuntimeInfo() const
 ObjTypeSpecFldInfo *
 FunctionJITTimeInfo::GetObjTypeSpecFldInfo(uint index) const
 {
-    AssertOrFailFast(index < GetBody()->GetInlineCacheCount());
     if (m_data.objTypeSpecFldInfoArray == nullptr)
     {
         return nullptr;
     }
+    AssertOrFailFast(index < m_data.objTypeSpecFldInfoCount);
 
     return reinterpret_cast<ObjTypeSpecFldInfo *>(m_data.objTypeSpecFldInfoArray[index]);
 }

lib/Backend/GlobOpt.cpp

@@ -2770,11 +2770,14 @@ GlobOpt::OptTagChecks(IR::Instr *instr)
                     ChangeValueType(nullptr, value, valueType.SetCanBeTaggedValue(false), true /*preserveSubClassInfo*/);
                     return false;
                 }
-                if (this->byteCodeUses)
+                if (!this->IsLoopPrePass())
                 {
-                    this->InsertByteCodeUses(instr);
+                    if (this->byteCodeUses)
+                    {
+                        this->InsertByteCodeUses(instr);
+                    }
+                    this->currentBlock->RemoveInstr(instr);
                 }
-                this->currentBlock->RemoveInstr(instr);
                 return true;
             }
 
@@ -5469,7 +5472,8 @@ GlobOpt::GetPrepassValueTypeForDst(
     IR::Instr *const instr,
     Value *const src1Value,
     Value *const src2Value,
-    bool const isValueInfoPrecise) const
+    bool const isValueInfoPrecise,
+    bool const isSafeToTransferInPrepass) const
 {
     // Values with definite types can be created in the loop prepass only when it is guaranteed that the value type will be the
     // same on any iteration of the loop. The heuristics currently used are:
@@ -5486,13 +5490,13 @@ GlobOpt::GetPrepassValueTypeForDst(
     Assert(IsLoopPrePass());
     Assert(instr);
 
-    if(!desiredValueType.IsDefinite())
-    {
-        return desiredValueType;
-    }
-
     if(!isValueInfoPrecise)
     {
+        if(!desiredValueType.IsDefinite())
+        {
+            return isSafeToTransferInPrepass ? desiredValueType : desiredValueType.SetCanBeTaggedValue(true);
+        }
+
         // If the desired value type is not precise, the value type of the destination is derived from the value types of the
         // sources. Since the value type of a source sym is not definite, the destination value type also cannot be definite.
         if(desiredValueType.IsInt() && OpCodeAttr::IsInt32(instr->m_opcode))
@@ -5505,6 +5509,7 @@ GlobOpt::GetPrepassValueTypeForDst(
             // The op always produces a number, but not always an int
             return desiredValueType.ToDefiniteAnyNumber();
         }
+        // Note: ToLikely() also sets CanBeTaggedValue
         return desiredValueType.ToLikely();
     }
 
@@ -5781,7 +5786,7 @@ GlobOpt::ValueNumberTransferDstInPrepass(IR::Instr *const instr, Value *const sr
     bool isSafeToTransferInPrepass = false;
     isValueInfoPrecise = IsPrepassSrcValueInfoPrecise(instr, src1Val, nullptr, &isSafeToTransferInPrepass);
 
-    const ValueType valueType(GetPrepassValueTypeForDst(src1ValueInfo->Type(), instr, src1Val, nullptr, isValueInfoPrecise));
+    const ValueType valueType(GetPrepassValueTypeForDst(src1ValueInfo->Type(), instr, src1Val, nullptr, isValueInfoPrecise, isSafeToTransferInPrepass));
     if(isValueInfoPrecise || isSafeToTransferInPrepass)
     {
         Assert(valueType == src1ValueInfo->Type());

lib/Backend/GlobOpt.h

@@ -559,7 +559,7 @@ class GlobOpt
     bool                    AreFromSameBytecodeFunc(IR::RegOpnd const* src1, IR::RegOpnd const* dst) const;
     Value *                 ValueNumberDst(IR::Instr **pInstr, Value *src1Val, Value *src2Val);
     Value *                 ValueNumberLdElemDst(IR::Instr **pInstr, Value *srcVal);
-    ValueType               GetPrepassValueTypeForDst(const ValueType desiredValueType, IR::Instr *const instr, Value *const src1Value, Value *const src2Value, bool const isValueInfoPreciseRef = false) const;
+    ValueType               GetPrepassValueTypeForDst(const ValueType desiredValueType, IR::Instr *const instr, Value *const src1Value, Value *const src2Value, bool const isValueInfoPreciseRef = false, bool const isSafeToTransferInPrepass = false) const;
     bool                    IsPrepassSrcValueInfoPrecise(IR::Opnd *const src, Value *const srcValue, bool * canTransferValueNumberToDst = nullptr) const;
     bool                    IsPrepassSrcValueInfoPrecise(IR::Instr *const instr, Value *const src1Value, Value *const src2Value, bool * canTransferValueNumberToDst = nullptr) const;
     bool                    IsSafeToTransferInPrepass(StackSym * const sym, ValueInfo *const srcValueInfo) const;

lib/Backend/GlobOptFields.cpp

@@ -228,6 +228,11 @@ GlobOpt::KillLiveElems(IR::IndirOpnd * indirOpnd, BVSparse<JitArenaAllocator> *
         this->KillAllFields(bv); // This also kills all property type values, as the same bit-vector tracks those stack syms
         SetAnyPropertyMayBeWrittenTo();
     }
+    else if (inGlobOpt && indexOpnd && !indexOpnd->GetValueType().IsInt() && !currentBlock->globOptData.IsInt32TypeSpecialized(indexOpnd->m_sym))
+    {
+        // Write/delete to a non-integer numeric index can't alias a name on the RHS of a dot, but it change object layout
+        this->KillAllObjectTypes(bv);
+    }
 }
 
 void
@@ -397,7 +402,8 @@ GlobOpt::ProcessFieldKills(IR::Instr *instr, BVSparse<JitArenaAllocator> *bv, bo
         KillLiveFields(this->lengthEquivBv, bv);
         if (inGlobOpt)
         {
-            KillObjectHeaderInlinedTypeSyms(this->currentBlock, false);
+            // Deleting an item, or pushing a property to a non-array, may change object layout
+            KillAllObjectTypes(bv);
         }
         break;
 
@@ -414,27 +420,32 @@ GlobOpt::ProcessFieldKills(IR::Instr *instr, BVSparse<JitArenaAllocator> *bv, bo
     case Js::OpCode::CallDirect:
         fnHelper = instr->GetSrc1()->AsHelperCallOpnd()->m_fnHelper;
 
-        // Kill length field for built-ins that can update it.
-        if(fnHelper == IR::JnHelperMethod::HelperArray_Shift 
-           || fnHelper == IR::JnHelperMethod::HelperArray_Splice
-           || fnHelper == IR::JnHelperMethod::HelperArray_Unshift)
+        switch (fnHelper)
         {
-            if (nullptr != this->lengthEquivBv)
-            {
-                KillLiveFields(this->lengthEquivBv, bv);
-            }
-            if (inGlobOpt)
-            {
-                KillObjectHeaderInlinedTypeSyms(this->currentBlock, false);
-            }
-        }
+            case IR::JnHelperMethod::HelperArray_Shift:
+            case IR::JnHelperMethod::HelperArray_Splice:
+            case IR::JnHelperMethod::HelperArray_Unshift:
+                // Kill length field for built-ins that can update it.
+                if (nullptr != this->lengthEquivBv)
+                {
+                    KillLiveFields(this->lengthEquivBv, bv);
+                }
+                // fall through
 
-        if ((fnHelper == IR::JnHelperMethod::HelperRegExp_Exec)
-           || (fnHelper == IR::JnHelperMethod::HelperString_Match)
-           || (fnHelper == IR::JnHelperMethod::HelperString_Replace))
-        {
-            // Consider: We may not need to kill all fields here.
-            this->KillAllFields(bv);
+            case IR::JnHelperMethod::HelperArray_Reverse:
+                // Deleting an item may change object layout
+                if (inGlobOpt)
+                {
+                    KillAllObjectTypes(bv);
+                }
+                break;
+
+            case IR::JnHelperMethod::HelperRegExp_Exec:
+            case IR::JnHelperMethod::HelperString_Match:
+            case IR::JnHelperMethod::HelperString_Replace:
+                // Consider: We may not need to kill all fields here.
+                this->KillAllFields(bv);
+                break;
         }
         break;
 

lib/Backend/IRBuilder.cpp

@@ -724,6 +724,8 @@ IRBuilder::Build()
                     if (!this->RegIsTemp(dstRegSlot) && !this->RegIsConstant(dstRegSlot))
                     {
                         SymID symId = dstSym->m_id;
+
+                        AssertOrFailFast(symId < m_stSlots->Length());
                         if (this->m_stSlots->Test(symId))
                         {
                             // For jitted loop bodies that are in a try block, we consider any symbol that has a
@@ -4076,7 +4078,12 @@ IRBuilder::EnsureLoopBodyLoadSlot(SymID symId, bool isCatchObjectSym)
         return;
     }
     StackSym * symDst = StackSym::FindOrCreate(symId, (Js::RegSlot)symId, m_func);
-    if (symDst->m_isCatchObjectSym || this->m_ldSlots->TestAndSet(symId))
+    if (symDst->m_isCatchObjectSym)
+    {
+        return;
+    }
+    AssertOrFailFast(symId < m_ldSlots->Length());
+    if (this->m_ldSlots->TestAndSet(symId))
     {
         return;
     }
@@ -4120,6 +4127,7 @@ IRBuilder::SetLoopBodyStSlot(SymID symID, bool isCatchObjectSym)
             return;
         }
     }
+    AssertOrFailFast(symID < m_stSlots->Length());
     this->m_stSlots->Set(symID);
 }
 

lib/Backend/IRBuilderAsmJs.cpp

@@ -3570,7 +3570,10 @@ IRBuilderAsmJs::BuildAsmJsLoopBodySlotOpnd(Js::RegSlot regSlot, IRType opndType)
 void
 IRBuilderAsmJs::EnsureLoopBodyAsmJsLoadSlot(Js::RegSlot regSlot, IRType type)
 {
-    if (GetJitLoopBodyData().GetLdSlots()->TestAndSet(regSlot))
+    BVFixed* ldSlotsBV = GetJitLoopBodyData().GetLdSlots();
+
+    AssertOrFailFast(regSlot < ldSlotsBV->Length());
+    if (ldSlotsBV->TestAndSet(regSlot))
     {
         return;
     }
@@ -3592,7 +3595,6 @@ void
 IRBuilderAsmJs::EnsureLoopBodyAsmJsStoreSlot(Js::RegSlot regSlot, IRType type)
 {
     Assert(!RegIsTemp(regSlot) || RegIsJitLoopYield(regSlot));
-    GetJitLoopBodyData().GetStSlots()->Set(regSlot);
     EnsureLoopBodyAsmJsLoadSlot(regSlot, type);
 }
 

lib/Backend/IRBuilderAsmJs.h

@@ -26,7 +26,6 @@ struct JitLoopBodyData
 {
 private:
     BVFixed* m_ldSlots = nullptr;
-    BVFixed* m_stSlots = nullptr;
     StackSym* m_loopBodyRetIPSym = nullptr;
     BVFixed* m_yieldRegs = nullptr;
     uint32 m_loopCurRegs[WAsmJs::LIMIT];
@@ -36,7 +35,6 @@ struct JitLoopBodyData
     {
         Assert(ldSlots && stSlots && loopBodyRetIPSym);
         m_ldSlots = ldSlots;
-        m_stSlots = stSlots;
         m_loopBodyRetIPSym = loopBodyRetIPSym;
     }
     // Use m_yieldRegs initialization to determine if m_loopCurRegs is initialized
@@ -57,14 +55,19 @@ struct JitLoopBodyData
     }
     bool IsYieldReg(Js::RegSlot reg) const
     {
-        return  m_yieldRegs && m_yieldRegs->Test(reg);
+        if (!m_yieldRegs)
+        {
+            return false;
+        }
+        AssertOrFailFast(reg < m_yieldRegs->Length());
+        return m_yieldRegs->Test(reg);
     }
     void SetRegIsYield(Js::RegSlot reg)
     {
         Assert(m_yieldRegs);
+        AssertOrFailFast(reg < m_yieldRegs->Length());
         m_yieldRegs->Set(reg);
     }
-    BVFixed* GetStSlots() const { return m_stSlots; }
     BVFixed* GetLdSlots() const { return m_ldSlots; }
     StackSym* GetLoopBodyRetIPSym() const { return m_loopBodyRetIPSym; }
 

lib/Backend/NativeEntryPointData.cpp

@@ -321,20 +321,14 @@ NativeEntryPointData::CleanupXDataInfo()
 {
     if (this->xdataInfo != nullptr)
     {
+        XDataAllocator::Unregister(this->xdataInfo);
 #ifdef _WIN32
         if (this->xdataInfo->functionTable
-            && !DelayDeletingFunctionTable::AddEntry(this->xdataInfo->functionTable))
+            && !DelayDeletingFunctionTable::AddEntry(this->xdataInfo))
         {
-            DelayDeletingFunctionTable::DeleteFunctionTable(this->xdataInfo->functionTable);
+            DelayDeletingFunctionTable::DeleteFunctionTable(this->xdataInfo);
         }
 #endif
-        XDataAllocator::Unregister(this->xdataInfo);
-#if defined(_M_ARM)
-        if (JITManager::GetJITManager()->IsOOPJITEnabled())
-#endif
-        {
-            HeapDelete(this->xdataInfo);
-        }
         this->xdataInfo = nullptr;
     }
 }