ChakraCore servicing update for 19-10

Build/NuGet/.pack-version

@@ -1 +1 @@
-1.11.13
+1.11.14

lib/Backend/BackwardPass.cpp

@@ -2195,6 +2195,13 @@ BackwardPass::DeadStoreTypeCheckBailOut(IR::Instr * instr)
         return;
     }
 
+    // By default, do not do this for stores, as it makes the presence of type checks unpredictable in the forward pass.
+    // For instance, we can't predict which stores may cause reallocation of aux slots.
+    if (instr->GetDst() && instr->GetDst()->IsSymOpnd())
+    {
+        return;
+    }
+
     IR::BailOutKind oldBailOutKind = instr->GetBailOutKind();
     if (!IR::IsTypeCheckBailOutKind(oldBailOutKind))
     {

lib/Backend/GlobOpt.cpp

@@ -2161,27 +2161,46 @@ GlobOpt::CollectMemOpInfo(IR::Instr *instrBegin, IR::Instr *instr, Value *src1Va
             return false;
         }
         break;
-    case Js::OpCode::Decr_A:
-        isIncr = false;
-    case Js::OpCode::Incr_A:
-        isChangedByOne = true;
-        goto MemOpCheckInductionVariable;
     case Js::OpCode::Sub_I4:
-    case Js::OpCode::Sub_A:
         isIncr = false;
-    case Js::OpCode::Add_A:
     case Js::OpCode::Add_I4:
     {
-MemOpCheckInductionVariable:
-        StackSym *sym = instr->GetSrc1()->GetStackSym();
-        if (!sym)
+        // The only case in which these OpCodes can contribute to an inductionVariableChangeInfo
+        // is when the induction variable is being modified and overwritten aswell (ex: j = j + 1)
+        // and not when the induction variable is modified but not overwritten (ex: k = j + 1).
+        // This can either be detected in IR as
+        // s1     = Add_I4 s1     1  // Case #1, can be seen with "j++".
+        // or as
+        // s4(s2) = Add_I4 s3(s1) 1  // Case #2, can be see with "j = j + 1".
+        // s1     = Ld_A   s2
+        bool isInductionVar = false;
+        IR::Instr* nextInstr = instr->m_next;
+        if (
+            // Checks for Case #1 and Case #2
+            instr->GetDst()->GetStackSym() != nullptr &&
+            instr->GetDst()->IsRegOpnd() &&
+            (
+                // Checks for Case #1
+                (instr->GetDst()->GetStackSym() == instr->GetSrc1()->GetStackSym()) ||
+
+                // Checks for Case #2
+                (nextInstr&& nextInstr->m_opcode == Js::OpCode::Ld_A &&
+                 nextInstr->GetSrc1()->IsRegOpnd() &&
+                 nextInstr->GetDst()->IsRegOpnd() &&
+                 GetVarSymID(instr->GetDst()->GetStackSym()) == nextInstr->GetSrc1()->GetStackSym()->m_id &&
+                 GetVarSymID(instr->GetSrc1()->GetStackSym()) == nextInstr->GetDst()->GetStackSym()->m_id)
+            )
+        )
         {
-            sym = instr->GetSrc2()->GetStackSym();
+            isInductionVar = true;
         }
+
+        // Even if dstIsInductionVar then dst == src1 so it's safe to use src1 as the induction sym always.
+        StackSym* sym = instr->GetSrc1()->GetStackSym();
 
         SymID inductionSymID = GetVarSymID(sym);
 
-        if (IsSymIDInductionVariable(inductionSymID, this->currentBlock->loop))
+        if (isInductionVar && IsSymIDInductionVariable(inductionSymID, this->currentBlock->loop))
         {
             if (!isChangedByOne)
             {
@@ -2246,7 +2265,6 @@ GlobOpt::CollectMemOpInfo(IR::Instr *instrBegin, IR::Instr *instr, Value *src1Va
                     {
                         inductionVariableChangeInfo.unroll++;
                     }
-
                     inductionVariableChangeInfo.isIncremental = isIncr;
                     loop->memOpInfo->inductionVariableChangeInfoMap->Item(inductionSymID, inductionVariableChangeInfo);
                 }
@@ -2284,6 +2302,27 @@ GlobOpt::CollectMemOpInfo(IR::Instr *instrBegin, IR::Instr *instr, Value *src1Va
             }
         }
         NEXT_INSTR_IN_RANGE;
+        IR::Instr* prevInstr = instr->m_prev;
+
+        // If an instr where the dst is an induction variable (and thus is being written to) is not caught by a case in the above
+        // switch statement (which implies that this instr does not contributes to a inductionVariableChangeInfo) and in the default
+        // case does not set doMemOp to false (which implies that this instr does not invalidate this MemOp), then FailFast as we
+        // should not be performing a MemOp under these conditions. 
+        AssertOrFailFast(!instr->GetDst() || instr->m_opcode == Js::OpCode::IncrLoopBodyCount || !loop->memOpInfo ||
+
+            // Refer to "Case #2" described above in this function. For the following IR:
+            // Line #1: s4(s2) = Add_I4 s3(s1) 1
+            // Line #2: s3(s1) = Ld_A   s4(s2)
+            // do not consider line #2 as a violating instr
+            (instr->m_opcode == Js::OpCode::Ld_I4 &&
+                prevInstr && (prevInstr->m_opcode == Js::OpCode::Add_I4 || prevInstr->m_opcode == Js::OpCode::Sub_I4) &&
+                instr->GetSrc1()->IsRegOpnd() &&
+                instr->GetDst()->IsRegOpnd() &&
+                prevInstr->GetDst()->IsRegOpnd() &&
+                instr->GetDst()->GetStackSym() == prevInstr->GetSrc1()->GetStackSym() &&
+                instr->GetSrc1()->GetStackSym() == prevInstr->GetDst()->GetStackSym()) ||
+
+            !loop->memOpInfo->inductionVariableChangeInfoMap->ContainsKey(GetVarSymID(instr->GetDst()->GetStackSym())));
     }
 
     return true;
@@ -3564,7 +3603,7 @@ GlobOpt::OptSrc(IR::Opnd *opnd, IR::Instr * *pInstr, Value **indirIndexValRef, I
 
         opnd->SetValueType(valueType);
 
-        if(!IsLoopPrePass() && opnd->IsSymOpnd() && valueType.IsDefinite())
+        if(!IsLoopPrePass() && opnd->IsSymOpnd() && (valueType.IsDefinite() || valueType.IsNotTaggedValue()))
         {
             if (opnd->AsSymOpnd()->m_sym->IsPropertySym())
             {

lib/Backend/GlobOpt.h

@@ -370,6 +370,7 @@ class JsArrayKills
             (valueType.IsArrayOrObjectWithArray() &&
              (
               (killsArraysWithNoMissingValues && valueType.HasNoMissingValues()) ||
+              (killsObjectArraysWithNoMissingValues && !valueType.IsArray() && valueType.HasNoMissingValues()) ||
               (killsNativeArrays && !valueType.HasVarElements())
              )
             );

lib/Backend/GlobOptFields.cpp

@@ -905,7 +905,7 @@ GlobOpt::FinishOptPropOp(IR::Instr *instr, IR::PropertySymOpnd *opnd, BasicBlock
 
         SymID opndId = opnd->HasObjectTypeSym() ? opnd->GetObjectTypeSym()->m_id : -1;
 
-        if (!isObjTypeChecked)
+        if (!isObjTypeSpecialized || opnd->IsBeingAdded())
         {
             if (block->globOptData.maybeWrittenTypeSyms == nullptr)
             {
@@ -1122,6 +1122,19 @@ GlobOpt::ProcessPropOpInTypeCheckSeq(IR::Instr* instr, IR::PropertySymOpnd *opnd
     Assert(opnd->IsTypeCheckSeqCandidate());
     Assert(opnd->HasObjectTypeSym());
 
+    if (opnd->HasTypeMismatch())
+    {
+        if (emitsTypeCheckOut != nullptr)
+        {
+            *emitsTypeCheckOut = false;
+        }
+        if (changesTypeValueOut != nullptr)
+        {
+            *changesTypeValueOut = false;
+        }
+        return false;
+    }
+
     bool isStore = opnd == instr->GetDst();
     bool isTypeDead = opnd->IsTypeDead();
     bool consumeType = makeChanges && !IsLoopPrePass();
@@ -1229,7 +1242,7 @@ GlobOpt::ProcessPropOpInTypeCheckSeq(IR::Instr* instr, IR::PropertySymOpnd *opnd
                 // a new type value here.
                 isSpecialized = false;
 
-                if (consumeType)
+                if (makeChanges)
                 {
                     opnd->SetTypeMismatch(true);
                 }
@@ -1273,7 +1286,7 @@ GlobOpt::ProcessPropOpInTypeCheckSeq(IR::Instr* instr, IR::PropertySymOpnd *opnd
                 // a new type value here.
                 isSpecialized = false;
 
-                if (consumeType)
+                if (makeChanges)
                 {
                     opnd->SetTypeMismatch(true);
                 }
@@ -1324,7 +1337,7 @@ GlobOpt::ProcessPropOpInTypeCheckSeq(IR::Instr* instr, IR::PropertySymOpnd *opnd
             {
                 // Indicates failure/mismatch
                 isSpecialized = false;
-                if (consumeType)
+                if (makeChanges)
                 {
                     opnd->SetTypeMismatch(true);
                 }
@@ -1423,7 +1436,7 @@ GlobOpt::ProcessPropOpInTypeCheckSeq(IR::Instr* instr, IR::PropertySymOpnd *opnd
             // a new type value here.
             isSpecialized = false;
 
-            if (consumeType)
+            if (makeChanges)
             {
                 opnd->SetTypeMismatch(true);
             }

lib/Backend/Lower.cpp

@@ -7420,9 +7420,6 @@ Lowerer::GenerateStFldWithCachedType(IR::Instr *instrStFld, bool* continueAsHelp
 
     if (hasTypeCheckBailout)
     {
-        AssertMsg(PHASE_ON1(Js::ObjTypeSpecIsolatedFldOpsWithBailOutPhase) || !propertySymOpnd->IsTypeDead() || propertySymOpnd->TypeCheckRequired(),
-            "Why does a field store have a type check bailout, if its type is dead?");
-
         if (instrStFld->GetBailOutInfo()->bailOutInstr != instrStFld)
         {
             // Set the cache index in the bailout info so that the generated code will write it into the
@@ -7482,7 +7479,7 @@ Lowerer::GenerateCachedTypeCheck(IR::Instr *instrChk, IR::PropertySymOpnd *prope
     // cache and no type check bailout. In the latter case, we can wind up doing expensive failed equivalence checks
     // repeatedly and never rejit.
     bool doEquivTypeCheck =
-        (instrChk->HasEquivalentTypeCheckBailOut() && propertySymOpnd->TypeCheckRequired()) ||
+        (instrChk->HasEquivalentTypeCheckBailOut() && (propertySymOpnd->TypeCheckRequired() || propertySymOpnd == instrChk->GetDst())) ||
         (propertySymOpnd->HasEquivalentTypeSet() &&
          !(propertySymOpnd->HasFinalType() && propertySymOpnd->HasInitialType()) &&
          !propertySymOpnd->MustDoMonoCheck() &&

lib/Common/ChakraCoreVersion.h

@@ -17,7 +17,7 @@
 // ChakraCore version number definitions (used in ChakraCore binary metadata)
 #define CHAKRA_CORE_MAJOR_VERSION 1
 #define CHAKRA_CORE_MINOR_VERSION 11
-#define CHAKRA_CORE_PATCH_VERSION 13
+#define CHAKRA_CORE_PATCH_VERSION 14
 #define CHAKRA_CORE_VERSION_RELEASE_QFE 0 // Redundant with PATCH_VERSION. Keep this value set to 0.
 
 // -------------

test/fieldopts/OS23440664.js

@@ -0,0 +1,17 @@
+//Reduced Switches: -printsystemexception -maxinterpretcount:1 -maxsimplejitruncount:1 -werexceptionsupport -oopjit- -bvt -off:bailonnoprofile -force:fixdataprops -forcejitloopbody 
+var shouldBailout = false;
+var IntArr0 = [];
+function test0() {
+  var loopInvariant = shouldBailout;
+  function makeArrayLength() {
+    return Math.floor();
+  }
+  makeArrayLength();
+  makeArrayLength();
+  prop0 = 1;
+  Object;
+  for (; shouldBailout ? (Object()) : (IntArr0[Object & 1] = '') ? Object : 0;) {
+  }
+}
+test0();
+WScript.Echo('pass');

test/fieldopts/rlexe.xml

@@ -852,4 +852,10 @@
       <files>argobjlengthhoist.js</files>
     </default>
   </test>
+  <test>
+    <default>
+      <files>OS23440664.js</files>
+      <compile-flags>-off:bailonnoprofile -force:fixdataprops -forcejitloopbody</compile-flags>
+    </default>
+  </test>
 </regress-exe>