Bug 1290566 - RadixSort must not access buffer directly through properties draft
authorSumit Tiwari <sumi29@gmail.com>
Tue, 09 Aug 2016 23:53:11 -0400
changeset 399429 3d2729adeabb9e67d6648ef9a0da8c33545b88f8
parent 398385 ce48c048a83877a7a0c264b2dba13cbc79569b40
child 399756 c5d0411de2eaff035580a83af62e41e219b76dc6
child 399757 3cd26d65a8eff1bfbe330aea088094e93d54955a
child 399758 bad21d1b155a6aabcb798d2c253aecef998fcb90
child 400316 7c07144b8b775f2da499ca81cf9740ea4db36341
child 400447 f7c9ff75dfdca02aeb74c4393d4444cf14aa9b70
child 400448 4ac1c54a2ef80c5632b19ac1abfbec3bf453eb9a
push id25828
push userbmo:sumi29@gmail.com
push dateThu, 11 Aug 2016 01:47:58 +0000
bugs1290566
milestone51.0a1
Bug 1290566 - RadixSort must not access buffer directly through properties MozReview-Commit-ID: DO3SvNHIQpW
js/src/builtin/Sorting.js
js/src/builtin/TypedArray.js
js/src/tests/ecma_6/TypedArray/sorting_buffer_access.js
--- a/js/src/builtin/Sorting.js
+++ b/js/src/builtin/Sorting.js
@@ -88,35 +88,38 @@ function SortByColumn(array, len, aux, c
     // Copy back
     for (let i = 0; i < len; i++) {
         array[i] = aux[i];
     }
 }
 
 // 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) {
         QuickSort(array, len, 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;
     }
 
     let view = array;
     let signMask = 1 << nbytes * 8 - 1;
 
     // Preprocess
     if (floating) {
-        view = new Int32Array(array.buffer);
+        view = new Int32Array(buffer);
 
         // Flip sign bit for positive numbers; flip all bits for negative
         // numbers
         for (let i = 0; i < len; i++) {
             if (view[i] & signMask) {
                 view[i] ^= 0xFFFFFFFF;
             } else {
                 view[i] ^= signMask
--- a/js/src/builtin/TypedArray.js
+++ b/js/src/builtin/TypedArray.js
@@ -1039,20 +1039,22 @@ function TypedArraySort(comparefn) {
 
     // Step 1.
     var obj = this;
 
     // Step 2.
     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;
     if (isTypedArray) {
         len = TypedArrayLength(obj);
     } else {
         len = callFunction(CallTypedArrayMethodIfWrapped, obj, obj, "TypedArrayLength");
     }
@@ -1060,25 +1062,25 @@ function TypedArraySort(comparefn) {
     if (comparefn === undefined) {
         comparefn = TypedArrayCompare;
         // CountingSort doesn't invoke the comparefn
         if (IsUint8TypedArray(obj)) {
             return CountingSort(obj, len, false /* signed */);
         } 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);
         }
     }
 
     // To satisfy step 2 from TypedArray SortCompare described in 22.2.3.26
     // the user supplied comparefn is wrapped.
     var wrappedCompareFn = comparefn;
     comparefn = function(x, y) {
         // Step a.
new file mode 100644
--- /dev/null
+++ b/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);
\ No newline at end of file