When doing a MemCopy, do the copy from only 1 segment to another and make sure they are both long enough.

lib/Runtime/Language/JavascriptOperators.cpp

@@ -4552,25 +4552,22 @@ CommonNumber:
                 break;
             }
             // Upper bounds check for source array
-            uint32 end;
-            if (UInt32Math::Add(srcStart, length, &end) || end > ((ArrayObject*)srcInstance)->GetLength())
-            {
-                return false;
-            }
+            JavascriptArray* srcArray = JavascriptArray::FromVar(srcInstance);
+            JavascriptArray* dstArray = JavascriptArray::FromVar(dstInstance);
             if (scriptContext->optimizationOverrides.IsEnabledArraySetElementFastPath())
             {
                 INT_PTR vt = VirtualTableInfoBase::GetVirtualTable(dstInstance);
                 if (instanceType == TypeIds_Array)
                 {
-                    returnValue = JavascriptArray::FromVar(dstInstance)->DirectSetItemAtRangeFromArray<Var>(dstStart, length, JavascriptArray::FromVar(srcInstance), srcStart);
+                    returnValue = dstArray->DirectSetItemAtRangeFromArray<Var>(dstStart, length, srcArray, srcStart);
                 }
                 else if (instanceType == TypeIds_NativeIntArray)
                 {
-                    returnValue = JavascriptArray::FromVar(dstInstance)->DirectSetItemAtRangeFromArray<int32>(dstStart, length, JavascriptArray::FromVar(srcInstance), srcStart);
+                    returnValue = dstArray->DirectSetItemAtRangeFromArray<int32>(dstStart, length, srcArray, srcStart);
                 }
                 else
                 {
-                    returnValue = JavascriptArray::FromVar(dstInstance)->DirectSetItemAtRangeFromArray<double>(dstStart, length, JavascriptArray::FromVar(srcInstance), srcStart);
+                    returnValue = dstArray->DirectSetItemAtRangeFromArray<double>(dstStart, length, srcArray, srcStart);
                 }
                 returnValue &= vt == VirtualTableInfoBase::GetVirtualTable(dstInstance);
             }

lib/Runtime/Library/JavascriptArray.inl

@@ -955,45 +955,46 @@ SECOND_PASS:
             return true;
         }
 
-        SparseArraySegment<T> *toSegment = PrepareSegmentForMemOp<T>(toStartIndex, length);
-        if (toSegment == nullptr)
+        const auto isSegmentValid = [length](Js::SparseArraySegment<T>* segment, uint32 startIndex) {
+            uint32 end, segmentEnd;
+            // Check the segment is long enough
+            return (
+                segment &&
+                !UInt32Math::Add(startIndex, length, &end) &&
+                !UInt32Math::Add(segment->left, segment->length, &segmentEnd) &&
+                startIndex >= segment->left &&
+                startIndex < segmentEnd &&
+                segmentEnd >= end
+            );
+        };
+        //Find the segment where itemIndex is present or is at the boundary
+        Js::SparseArraySegment<T>* fromSegment = (Js::SparseArraySegment<T>*)fromArray->GetBeginLookupSegment(fromStartIndex, false);
+        if (!isSegmentValid(fromSegment, fromStartIndex))
         {
             return false;
         }
-        Assert(fromArray->head);
-        Assert(fromArray->length >= (fromStartIndex + length));
 
-        //Find the segment where itemIndex is present or is at the boundary
-        SparseArraySegmentBase* current = fromArray->GetBeginLookupSegment(fromStartIndex, false);
-        Assert(current);
-        Assert(GetSegmentMap() == nullptr  && fromArray->GetSegmentMap() == nullptr);
-
-        while (current && length)
+        // Check the from segment first so we don't prepare the toSegment in case it is invalid
+        SparseArraySegment<T> *toSegment = PrepareSegmentForMemOp<T>(toStartIndex, length);
+        if (!isSegmentValid(toSegment, toStartIndex))
         {
-            int memcopySize = length;
-            int startOffset;
-            if (fromStartIndex >= current->left && fromStartIndex < (current->left + current->length))
-            {
-                startOffset = fromStartIndex - current->left;
-                if ((startOffset + length) > current->length)
-                {
-                    memcopySize = current->length - startOffset;
-                }
-
-
-                js_memcpy_s(toSegment->elements + toStartIndex, memcopySize * sizeof(T), (((Js::SparseArraySegment<T>*)current)->elements + startOffset), memcopySize * sizeof(T));
-
-                fromArray->SetLastUsedSegment(current);
-                fromStartIndex = fromStartIndex + memcopySize;
-                toStartIndex = toStartIndex + memcopySize;
-                length = length - memcopySize;
-
-            }
-            current = current->next;
+            return false;
         }
+        Assert(GetSegmentMap() == nullptr && fromArray->GetSegmentMap() == nullptr);
+
+        int memcopySize = length;
+        int toStartOffset = toStartIndex - toSegment->left;
+        int fromStartOffset = fromStartIndex - fromSegment->left;
+        Assert((fromStartOffset + length) <= fromSegment->length);
 
-        Assert(length == 0);
+        js_memcpy_s(
+            toSegment->elements + toStartOffset,
+            (toSegment->size - toStartOffset) * sizeof(T),
+            fromSegment->elements + fromStartOffset,
+            memcopySize * sizeof(T)
+        );
 
+        fromArray->SetLastUsedSegment(fromSegment);
         this->SetLastUsedSegment(toSegment);
 #if DBG
         if (Js::Configuration::Global.flags.MemOpMissingValueValidate)

test/Array/memcopy.js

@@ -54,7 +54,6 @@ let testCases = [
       end: end,
       size: end,
       test: function testReverse(a, src) {
-        // Currently this is not a valid memcopy pattern
         for(let i = 100; i >= 5; i--) {
           a[i] = src[i];
         }

test/Array/memcopy_length_bug.js

@@ -0,0 +1,29 @@
+//-------------------------------------------------------------------------------------------------------
+// Copyright (C) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
+//-------------------------------------------------------------------------------------------------------
+
+var func2 = function () {
+  var v5 = ary.length;
+  for (var i = -1; i < v5; i++) {
+    dst[i] = ary[i];
+  }
+};
+var dst = Array();
+var ary = Array();
+ary.length = 100;
+ary[0] = 15;
+ary[1] = 178;
+ary[2] = 987;
+
+func2();
+if (
+  dst.length !== 100 ||
+  dst[0] !== 15 ||
+  dst[1] !== 178 ||
+  dst[2] !== 987
+) {
+  print("FAILED");
+} else {
+  print("PASSED");
+}

test/Array/rlexe.xml

@@ -663,6 +663,12 @@
         <compile-flags>-mic:1 -off:simplejit -off:JITLoopBody -off:inline -off:globopt:1.18-1.30 -mmoc:0 -args float -endargs</compile-flags>
      </default>
   </test>
+  <test>
+     <default>
+        <files>memcopy_length_bug.js</files>
+        <compile-flags>-bgjit- -lic:0</compile-flags>
+     </default>
+  </test>
   <test>
      <default>
         <files>memop_alias.js</files>