[MERGE #6184 @boingoing] Assert in JavascriptArray::EntryReverse when length > Uint32Max

Merge pull request #6184 from boingoing:6179

Turns out it's pretty easy to make the length property of a TypedArray greater than Uint32Max. Passing a TypedArray object like that to Array#reverse hits an assert verifying the length is <= Uint32Max.

This is a valid situation to get into, though, so instead of asserting we should fall into the object (non-Array, non-TypedArray) case and use the slow path which supports larger lengths.

Fixes #6179

Author: boingoing

lib/Runtime/Library/JavascriptArray.cpp

@@ -5533,7 +5533,6 @@ using namespace Js;
         {
             JS_REENTRANT_UNLOCK(jsReentLock, return JavascriptArray::ReverseHelper(pArr, nullptr, obj, length.GetSmallIndex(), scriptContext));
         }
-        Assert(pArr == nullptr || length.IsUint32Max()); // if pArr is not null lets make sure length is safe to cast, which will only happen if length is a uint32max
 
         JS_REENTRANT_UNLOCK(jsReentLock, return JavascriptArray::ReverseHelper(pArr, nullptr, obj, length.GetBigIndex(), scriptContext));
         JIT_HELPER_END(Array_Reverse);
@@ -5624,6 +5623,7 @@ using namespace Js;
 
         if (useNoSideEffectReverse)
         {
+            Assert(length <= JavascriptArray::MaxArrayLength);
             Recycler * recycler = scriptContext->GetRecycler();
 
             if (length <= 1)
@@ -5774,9 +5774,8 @@ using namespace Js;
 
             failFastOnError.Completed();
         }
-        else if (typedArrayBase)
+        else if (typedArrayBase && length <= JavascriptArray::MaxArrayLength)
         {
-            Assert(length <= JavascriptArray::MaxArrayLength);
             if (typedArrayBase->GetLength() == length)
             {
                 // If typedArrayBase->length == length then we know that the TypedArray will have all items < length

test/Bugs/bug_6179.js

@@ -0,0 +1,60 @@
+//-------------------------------------------------------------------------------------------------------
+// Copyright (C) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
+//-------------------------------------------------------------------------------------------------------
+
+WScript.LoadScriptFile("..\\UnitTestFramework\\UnitTestFramework.js");
+
+var tests = [
+    {
+        name: "#6179 - Assert in JavascriptArray::EntryReverse when length > Uint32Max for TypedArray with Array prototype",
+        body: function () {
+            var ua = new Uint32Array(0x10);
+            ua.__proto__ = new Array(0xffffffff);
+            ua.length = 0xffffffff*2;
+
+            assert.throws(()=>ua.reverse(), TypeError, "Array#reverse tries to delete a property on the TypedArray but that throws");
+        }
+    },
+    {
+        name: "#6179 - Assert in JavascriptArray::EntryReverse when length > Uint32Max for TypedArray with own length property",
+        body: function () {
+            var ua = new Uint32Array(0x10);
+            Object.defineProperty(ua, 'length', {value: 0xffffffff*2 });
+
+            assert.throws(()=>Array.prototype.reverse.call(ua), TypeError, "Array#reverse tries to delete a property on the TypedArray but that throws");
+        }
+    },
+    {
+        name: "#6179 - Assert in JavascriptArray::EntryReverse when length > Uint32Max for an object with length property",
+        body: function () {
+            let getCount = 0;
+            let setCount = 0;
+            var ua = {
+                length: 0xffffffff*2,
+                set [0xffffffff*2-1](v) {
+                    assert.areEqual(1, getCount, "1 === getCount");
+                    assert.areEqual(0, setCount, "0 === setCount");
+                    setCount++
+                },
+                get '0'() {
+                    assert.areEqual(0, getCount, "0 === getCount");
+                    assert.areEqual(0, setCount, "0 === setCount");
+                    getCount++
+                },
+                get '1'() {
+                    assert.areEqual(1, getCount, "1 === getCount");
+                    assert.areEqual(1, setCount, "1 === setCount");
+                    throw 123; 
+                }
+            };
+
+            assert.throws(()=>Array.prototype.reverse.call(ua), 123, "Array#reverse will throw above when we try and get property '1'");
+
+            assert.areEqual(1, getCount, "1 === getCount");
+            assert.areEqual(1, setCount, "1 === setCount");
+        }
+    },
+];
+
+testRunner.runTests(tests, { verbose: WScript.Arguments[0] != "summary" });

test/Bugs/rlexe.xml

@@ -602,5 +602,11 @@ Re-enable after fixing ScopeInfo::SaveEnclosingScopeInfo to handle named functio
       <files>Bug19948792.js</files>
       <compile-flags>-maxinterpretcount:1 -bgjit- -oopjit- -loopinterpretcount:1 -maxsimplejitruncount:2</compile-flags>
     </default>
-  </test> 
+  </test>
+  <test>
+    <default>
+      <files>bug_6179.js</files>
+      <compile-flags>-args summary -endargs</compile-flags>
+    </default>
+  </test>
 </regress-exe>