Bug 1290566 - RadixSort must not access buffer directly through properties; r=mrrrgn

--HG--
extra : rebase_source : 61a605a3a3002c6a4c8bb814aefbbab3860408df
extra : amend_source : 77f1eefdaa03439894fef15320bef119a73149b7

js/src/builtin/Sorting.js

@@ -93,7 +93,7 @@ function SortByColumn(array, len, aux, col) {
 
 // Sorts integers and float32. |signed| is true for int16 and int32, |floating|
 // is true for float32.
-function RadixSort(array, len, nbytes, signed, floating, comparefn) {
+function RadixSort(array, len, buffer, nbytes, signed, floating, comparefn) {
 
     // Determined by performance testing.
     if (len < 128) {
@@ -101,6 +101,9 @@ function RadixSort(array, len, nbytes, signed, floating, comparefn) {
         return array;
     }
 
+    // Verify that the buffer is non-null
+    assert(buffer !== null, "Attached data buffer should be reified when array length is >= 128.");
+
     let aux = new List();
     for (let i = 0; i < len; i++) {
         aux[i] = 0;
@@ -111,7 +114,7 @@ function RadixSort(array, len, nbytes, signed, floating, comparefn) {
 
     // Preprocess
     if (floating) {
-        view = new Int32Array(array.buffer);
+        view = new Int32Array(buffer);
 
         // Flip sign bit for positive numbers; flip all bits for negative
         // numbers

js/src/builtin/TypedArray.js

@@ -1044,10 +1044,12 @@ function TypedArraySort(comparefn) {
     var isTypedArray = IsObject(obj) && IsTypedArray(obj);
 
     var buffer;
-    if (isTypedArray)
+    if (isTypedArray) {
         buffer = GetAttachedArrayBuffer(obj);
-    else
+    }
+    else {
         buffer = callFunction(CallTypedArrayMethodIfWrapped, obj, obj, "GetAttachedArrayBuffer");
+    }
 
     // Step 3.
     var len;
@@ -1065,15 +1067,15 @@ function TypedArraySort(comparefn) {
         } else if (IsInt8TypedArray(obj)) {
             return CountingSort(obj, len, true /* signed */);
         } else if (IsUint16TypedArray(obj)) {
-            return RadixSort(obj, len, 2 /* nbytes */, false /* signed */, false /* floating */, comparefn);
+            return RadixSort(obj, len, buffer, 2 /* nbytes */, false /* signed */, false /* floating */, comparefn);
         } else if (IsInt16TypedArray(obj)) {
-            return RadixSort(obj, len, 2 /* nbytes */, true /* signed */, false /* floating */, comparefn);
+            return RadixSort(obj, len, buffer, 2 /* nbytes */, true /* signed */, false /* floating */, comparefn);
         } else if (IsUint32TypedArray(obj)) {
-            return RadixSort(obj, len, 4 /* nbytes */, false /* signed */, false /* floating */, comparefn);
+            return RadixSort(obj, len, buffer, 4 /* nbytes */, false /* signed */, false /* floating */, comparefn);
         } else if (IsInt32TypedArray(obj)) {
-            return RadixSort(obj, len, 4 /* nbytes */, true /* signed */, false /* floating */, comparefn);
+            return RadixSort(obj, len, buffer, 4 /* nbytes */, true /* signed */, false /* floating */, comparefn);
         } else if (IsFloat32TypedArray(obj)) {
-            return RadixSort(obj, len, 4 /* nbytes */, true /* signed */, true /* floating */, comparefn);
+            return RadixSort(obj, len, buffer, 4 /* nbytes */, true /* signed */, true /* floating */, comparefn);
         }
     }
 

js/src/tests/ecma_6/TypedArray/sorting_buffer_access.js

@@ -0,0 +1,26 @@
+// Ensure that when sorting arrays of size greater than 128, which
+// calls RadixSort under the hood, we don't access the 'buffer' 
+// property of the typed array directly. 
+
+
+// The buggy behavior in the RadixSort is only exposed when we use
+// float arrays, but checking everything just to be sure.
+const constructors = [
+    Int8Array,
+    Uint8Array,
+    Uint8ClampedArray,
+    Int16Array,
+    Uint16Array,
+    Int32Array,
+    Uint32Array,
+    Float32Array,
+    Float64Array ];
+
+for (var ctor of constructors) {
+    var testArray = new ctor(1024);
+    Object.defineProperty(testArray, "buffer", { get() { throw new Error("FAIL: Buffer accessed directly"); }  });
+    testArray.sort();
+}
+
+if (typeof reportCompare === "function")
+    reportCompare(true, true);