Unreviewed, rolling out r191190.
authorkeith_miller@apple.com <keith_miller@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 16 Oct 2015 19:47:04 +0000 (19:47 +0000)
committerkeith_miller@apple.com <keith_miller@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 16 Oct 2015 19:47:04 +0000 (19:47 +0000)
Patch needs some design changes.

Reverted changeset:

"Fix some issues with TypedArrays"
https://bugs.webkit.org/show_bug.cgi?id=150216
http://trac.webkit.org/changeset/191190

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@191193 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGOperations.cpp
Source/JavaScriptCore/runtime/JSGenericTypedArrayViewConstructorInlines.h
Source/JavaScriptCore/runtime/JSGenericTypedArrayViewPrototypeFunctions.h
Source/JavaScriptCore/tests/stress/typedarray-construct-iterator.js [deleted file]

index 7da2f9e..2c24787 100644 (file)
@@ -1,3 +1,15 @@
+2015-10-16  Keith Miller  <keith_miller@apple.com>
+
+        Unreviewed, rolling out r191190.
+
+        Patch needs some design changes.
+
+        Reverted changeset:
+
+        "Fix some issues with TypedArrays"
+        https://bugs.webkit.org/show_bug.cgi?id=150216
+        http://trac.webkit.org/changeset/191190
+
 2015-10-16  Mark Lam  <mark.lam@apple.com>
 
         Move all the probe trampolines into their respective MacroAssembler files.
index fdbb348..cf6911b 100644 (file)
@@ -142,6 +142,64 @@ char* newTypedArrayWithSize(ExecState* exec, Structure* structure, int32_t size)
     return bitwise_cast<char*>(ViewClass::create(exec, structure, size));
 }
 
+template<typename ViewClass>
+char* newTypedArrayWithOneArgument(
+    ExecState* exec, Structure* structure, EncodedJSValue encodedValue)
+{
+    VM& vm = exec->vm();
+    NativeCallFrameTracer tracer(&vm, exec);
+    
+    JSValue value = JSValue::decode(encodedValue);
+    
+    if (JSArrayBuffer* jsBuffer = jsDynamicCast<JSArrayBuffer*>(value)) {
+        RefPtr<ArrayBuffer> buffer = jsBuffer->impl();
+        
+        if (buffer->byteLength() % ViewClass::elementSize) {
+            vm.throwException(exec, createRangeError(exec, ASCIILiteral("ArrayBuffer length minus the byteOffset is not a multiple of the element size")));
+            return 0;
+        }
+        return bitwise_cast<char*>(
+            ViewClass::create(
+                exec, structure, buffer, 0, buffer->byteLength() / ViewClass::elementSize));
+    }
+    
+    if (JSObject* object = jsDynamicCast<JSObject*>(value)) {
+        unsigned length = object->get(exec, vm.propertyNames->length).toUInt32(exec);
+        if (exec->hadException())
+            return 0;
+        
+        ViewClass* result = ViewClass::createUninitialized(exec, structure, length);
+        if (!result)
+            return 0;
+        
+        if (!result->set(exec, object, 0, length))
+            return 0;
+        
+        return bitwise_cast<char*>(result);
+    }
+    
+    int length;
+    if (value.isInt32())
+        length = value.asInt32();
+    else if (!value.isNumber()) {
+        vm.throwException(exec, createTypeError(exec, ASCIILiteral("Invalid array length argument")));
+        return 0;
+    } else {
+        length = static_cast<int>(value.asNumber());
+        if (length != value.asNumber()) {
+            vm.throwException(exec, createTypeError(exec, ASCIILiteral("Invalid array length argument (fractional lengths not allowed)")));
+            return 0;
+        }
+    }
+    
+    if (length < 0) {
+        vm.throwException(exec, createRangeError(exec, ASCIILiteral("Requested length is negative")));
+        return 0;
+    }
+    
+    return bitwise_cast<char*>(ViewClass::create(exec, structure, length));
+}
+
 extern "C" {
 
 EncodedJSValue JIT_OPERATION operationToThis(ExecState* exec, EncodedJSValue encodedOp)
@@ -606,7 +664,7 @@ char* JIT_OPERATION operationNewInt8ArrayWithSize(
 char* JIT_OPERATION operationNewInt8ArrayWithOneArgument(
     ExecState* exec, Structure* structure, EncodedJSValue encodedValue)
 {
-    return bitwise_cast<char*>(constructGenericTypedArrayViewWithFirstArgument<JSInt8Array>(exec, structure, encodedValue));
+    return newTypedArrayWithOneArgument<JSInt8Array>(exec, structure, encodedValue);
 }
 
 char* JIT_OPERATION operationNewInt16ArrayWithSize(
@@ -618,7 +676,7 @@ char* JIT_OPERATION operationNewInt16ArrayWithSize(
 char* JIT_OPERATION operationNewInt16ArrayWithOneArgument(
     ExecState* exec, Structure* structure, EncodedJSValue encodedValue)
 {
-    return bitwise_cast<char*>(constructGenericTypedArrayViewWithFirstArgument<JSInt16Array>(exec, structure, encodedValue));
+    return newTypedArrayWithOneArgument<JSInt16Array>(exec, structure, encodedValue);
 }
 
 char* JIT_OPERATION operationNewInt32ArrayWithSize(
@@ -630,7 +688,7 @@ char* JIT_OPERATION operationNewInt32ArrayWithSize(
 char* JIT_OPERATION operationNewInt32ArrayWithOneArgument(
     ExecState* exec, Structure* structure, EncodedJSValue encodedValue)
 {
-    return bitwise_cast<char*>(constructGenericTypedArrayViewWithFirstArgument<JSInt32Array>(exec, structure, encodedValue));
+    return newTypedArrayWithOneArgument<JSInt32Array>(exec, structure, encodedValue);
 }
 
 char* JIT_OPERATION operationNewUint8ArrayWithSize(
@@ -642,7 +700,7 @@ char* JIT_OPERATION operationNewUint8ArrayWithSize(
 char* JIT_OPERATION operationNewUint8ArrayWithOneArgument(
     ExecState* exec, Structure* structure, EncodedJSValue encodedValue)
 {
-    return bitwise_cast<char*>(constructGenericTypedArrayViewWithFirstArgument<JSUint8Array>(exec, structure, encodedValue));
+    return newTypedArrayWithOneArgument<JSUint8Array>(exec, structure, encodedValue);
 }
 
 char* JIT_OPERATION operationNewUint8ClampedArrayWithSize(
@@ -654,7 +712,7 @@ char* JIT_OPERATION operationNewUint8ClampedArrayWithSize(
 char* JIT_OPERATION operationNewUint8ClampedArrayWithOneArgument(
     ExecState* exec, Structure* structure, EncodedJSValue encodedValue)
 {
-    return bitwise_cast<char*>(constructGenericTypedArrayViewWithFirstArgument<JSUint8ClampedArray>(exec, structure, encodedValue));
+    return newTypedArrayWithOneArgument<JSUint8ClampedArray>(exec, structure, encodedValue);
 }
 
 char* JIT_OPERATION operationNewUint16ArrayWithSize(
@@ -666,7 +724,7 @@ char* JIT_OPERATION operationNewUint16ArrayWithSize(
 char* JIT_OPERATION operationNewUint16ArrayWithOneArgument(
     ExecState* exec, Structure* structure, EncodedJSValue encodedValue)
 {
-    return bitwise_cast<char*>(constructGenericTypedArrayViewWithFirstArgument<JSUint16Array>(exec, structure, encodedValue));
+    return newTypedArrayWithOneArgument<JSUint16Array>(exec, structure, encodedValue);
 }
 
 char* JIT_OPERATION operationNewUint32ArrayWithSize(
@@ -678,7 +736,7 @@ char* JIT_OPERATION operationNewUint32ArrayWithSize(
 char* JIT_OPERATION operationNewUint32ArrayWithOneArgument(
     ExecState* exec, Structure* structure, EncodedJSValue encodedValue)
 {
-    return bitwise_cast<char*>(constructGenericTypedArrayViewWithFirstArgument<JSUint32Array>(exec, structure, encodedValue));
+    return newTypedArrayWithOneArgument<JSUint32Array>(exec, structure, encodedValue);
 }
 
 char* JIT_OPERATION operationNewFloat32ArrayWithSize(
@@ -690,7 +748,7 @@ char* JIT_OPERATION operationNewFloat32ArrayWithSize(
 char* JIT_OPERATION operationNewFloat32ArrayWithOneArgument(
     ExecState* exec, Structure* structure, EncodedJSValue encodedValue)
 {
-    return bitwise_cast<char*>(constructGenericTypedArrayViewWithFirstArgument<JSFloat32Array>(exec, structure, encodedValue));
+    return newTypedArrayWithOneArgument<JSFloat32Array>(exec, structure, encodedValue);
 }
 
 char* JIT_OPERATION operationNewFloat64ArrayWithSize(
@@ -702,7 +760,7 @@ char* JIT_OPERATION operationNewFloat64ArrayWithSize(
 char* JIT_OPERATION operationNewFloat64ArrayWithOneArgument(
     ExecState* exec, Structure* structure, EncodedJSValue encodedValue)
 {
-    return bitwise_cast<char*>(constructGenericTypedArrayViewWithFirstArgument<JSFloat64Array>(exec, structure, encodedValue));
+    return newTypedArrayWithOneArgument<JSFloat64Array>(exec, structure, encodedValue);
 }
 
 JSCell* JIT_OPERATION operationCreateActivationDirect(ExecState* exec, Structure* structure, JSScope* scope, SymbolTable* table, EncodedJSValue initialValueEncoded)
index 1e1f9e7..d5e5a02 100644 (file)
@@ -77,23 +77,23 @@ Structure* JSGenericTypedArrayViewConstructor<ViewClass>::createStructure(
 }
 
 template<typename ViewClass>
-static JSObject* constructGenericTypedArrayViewFromIterator(ExecState* exec, Structure* structure, JSValue iterator)
+static EncodedJSValue constructGenericTypedArrayViewFromIterator(ExecState* exec, Structure* structure, JSValue iterator)
 {
     if (!iterator.isObject())
-        return throwTypeError(exec, "Symbol.Iterator for the first argument did not return an object.");
+        return JSValue::encode(throwTypeError(exec, "Symbol.Iterator for the first argument did not return an object."));
 
     MarkedArgumentBuffer storage;
     while (true) {
         JSValue next = iteratorStep(exec, iterator);
         if (exec->hadException())
-            return nullptr;
+            return JSValue::encode(jsUndefined());
 
         if (next.isFalse())
             break;
 
         JSValue nextItem = iteratorValue(exec, next);
         if (exec->hadException())
-            return nullptr;
+            return JSValue::encode(jsUndefined());
 
         storage.append(nextItem);
     }
@@ -101,143 +101,129 @@ static JSObject* constructGenericTypedArrayViewFromIterator(ExecState* exec, Str
     ViewClass* result = ViewClass::createUninitialized(exec, structure, storage.size());
     if (!result) {
         ASSERT(exec->hadException());
-        return nullptr;
+        return JSValue::encode(jsUndefined());
     }
 
     for (unsigned i = 0; i < storage.size(); ++i) {
         if (!result->setIndex(exec, i, storage.at(i))) {
             ASSERT(exec->hadException());
-            return nullptr;
+            return JSValue::encode(jsUndefined());
         }
     }
 
-    return result;
+    return JSValue::encode(result);
 }
 
-template<typename ViewClass, bool checkForOtherArguments = false>
-static JSObject* constructGenericTypedArrayViewWithFirstArgument(ExecState* exec, Structure* structure, EncodedJSValue firstArgument)
+template<typename ViewClass>
+static EncodedJSValue JSC_HOST_CALL constructGenericTypedArrayView(ExecState* exec)
 {
-    JSValue firstValue = JSValue::decode(firstArgument);
+    Structure* structure =
+        asInternalFunction(exec->callee())->globalObject()->typedArrayStructure(
+            ViewClass::TypedArrayStorageType);
+
     VM& vm = exec->vm();
 
-    if (JSArrayBuffer* jsBuffer = jsDynamicCast<JSArrayBuffer*>(firstValue)) {
+    if (!exec->argumentCount()) {
+        if (ViewClass::TypedArrayStorageType == TypeDataView)
+            return throwVMError(exec, createTypeError(exec, "DataView constructor requires at least one argument."));
+        
+        // Even though the documentation doesn't say so, it's correct to say
+        // "new Int8Array()". This is the same as allocating an array of zero
+        // length.
+        return JSValue::encode(ViewClass::create(exec, structure, 0));
+    }
+    
+    if (JSArrayBuffer* jsBuffer = jsDynamicCast<JSArrayBuffer*>(exec->argument(0))) {
         RefPtr<ArrayBuffer> buffer = jsBuffer->impl();
-
-        unsigned offset = 0;
+        
+        unsigned offset = (exec->argumentCount() > 1) ? exec->uncheckedArgument(1).toUInt32(exec) : 0;
+        if (exec->hadException())
+            return JSValue::encode(jsUndefined());
         unsigned length = 0;
-        if (checkForOtherArguments && exec->argumentCount() > 1) {
-            offset = exec->uncheckedArgument(1).toUInt32(exec);
-            if (exec->hadException())
-                return nullptr;
-        }
-
-        if (checkForOtherArguments && exec->argumentCount() > 2) {
+        if (exec->argumentCount() > 2) {
             length = exec->uncheckedArgument(2).toUInt32(exec);
             if (exec->hadException())
-                return nullptr;
+                return JSValue::encode(jsUndefined());
         } else {
             if ((buffer->byteLength() - offset) % ViewClass::elementSize)
-                return throwRangeError(exec, "ArrayBuffer length minus the byteOffset is not a multiple of the element size");
+                return throwVMError(exec, createRangeError(exec, "ArrayBuffer length minus the byteOffset is not a multiple of the element size"));
             length = (buffer->byteLength() - offset) / ViewClass::elementSize;
-
         }
-
-        return ViewClass::create(exec, structure, buffer, offset, length);
+        return JSValue::encode(ViewClass::create(exec, structure, buffer, offset, length));
     }
     
     if (ViewClass::TypedArrayStorageType == TypeDataView)
-        return throwTypeError(exec, "Expected ArrayBuffer for the first argument.");
+        return throwVMError(exec, createTypeError(exec, "Expected ArrayBuffer for the first argument."));
     
     // For everything but DataView, we allow construction with any of:
     // - Another array. This creates a copy of the of that array.
     // - An integer. This creates a new typed array of that length and zero-initializes it.
+    
+    if (JSObject* object = jsDynamicCast<JSObject*>(exec->uncheckedArgument(0))) {
+        PropertySlot lengthSlot(object);
+        object->getPropertySlot(exec, vm.propertyNames->length, lengthSlot);
 
-    if (JSObject* object = jsDynamicCast<JSObject*>(firstValue)) {
-        unsigned length;
-
-        if (isTypedView(object->classInfo()->typedArrayStorageType))
-            length = jsCast<JSArrayBufferView*>(object)->length();
-        else {
-            PropertySlot lengthSlot(object);
-            object->getPropertySlot(exec, vm.propertyNames->length, lengthSlot);
-
+        if (!isTypedView(object->classInfo()->typedArrayStorageType)) {
             JSValue iteratorFunc = object->get(exec, vm.propertyNames->iteratorSymbol);
             if (exec->hadException())
-                return nullptr;
+                return JSValue::encode(jsUndefined());
 
             // We would like not use the iterator as it is painfully slow. Fortunately, unless
             // 1) The iterator is not a known iterator.
             // 2) The base object does not have a length getter.
-            // 3) The base object might have indexed getters.
+            // 3) Bad times are being had.
             // it should not be observable that we do not use the iterator.
 
             if (!iteratorFunc.isUndefined()
-                && (iteratorFunc != object->globalObject()->arrayProtoValuesFunction()
+                && (iteratorFunc != exec->lexicalGlobalObject()->arrayProtoValuesFunction()
                     || lengthSlot.isAccessor() || lengthSlot.isCustom()
-                    || hasAnyArrayStorage(object->indexingType()))) {
+                    || exec->lexicalGlobalObject()->isHavingABadTime())) {
 
                     CallData callData;
                     CallType callType = getCallData(iteratorFunc, callData);
                     if (callType == CallTypeNone)
-                        return throwTypeError(exec, "Symbol.Iterator for the first argument cannot be called.");
+                        return JSValue::encode(throwTypeError(exec, "Symbol.Iterator for the first argument cannot be called."));
 
                     ArgList arguments;
                     JSValue iterator = call(exec, iteratorFunc, callType, callData, object, arguments);
                     if (exec->hadException())
-                        return nullptr;
+                        return JSValue::encode(jsUndefined());
 
                     return constructGenericTypedArrayViewFromIterator<ViewClass>(exec, structure, iterator);
-            }
 
-            length = lengthSlot.isUnset() ? 0 : lengthSlot.getValue(exec, vm.propertyNames->length).toUInt32(exec);
-            if (exec->hadException())
-                return nullptr;
+            }
         }
 
+        unsigned length = lengthSlot.getValue(exec, vm.propertyNames->length).toUInt32(exec);
+        if (exec->hadException())
+            return JSValue::encode(jsUndefined());
         
         ViewClass* result = ViewClass::createUninitialized(exec, structure, length);
         if (!result) {
             ASSERT(exec->hadException());
-            return nullptr;
+            return JSValue::encode(jsUndefined());
         }
         
         if (!result->set(exec, object, 0, length))
-            return nullptr;
+            return JSValue::encode(jsUndefined());
         
-        return result;
+        return JSValue::encode(result);
     }
     
     int length;
-    if (firstValue.isInt32())
-        length = firstValue.asInt32();
-    else if (!firstValue.isNumber())
-        return throwTypeError(exec, "Invalid array length argument");
+    if (exec->uncheckedArgument(0).isInt32())
+        length = exec->uncheckedArgument(0).asInt32();
+    else if (!exec->uncheckedArgument(0).isNumber())
+        return throwVMError(exec, createTypeError(exec, "Invalid array length argument"));
     else {
-        length = static_cast<int>(firstValue.asNumber());
-        if (length != firstValue.asNumber())
-            return throwTypeError(exec, "Invalid array length argument (fractional lengths not allowed)");
+        length = static_cast<int>(exec->uncheckedArgument(0).asNumber());
+        if (length != exec->uncheckedArgument(0).asNumber())
+            return throwVMError(exec, createTypeError(exec, "Invalid array length argument (fractional lengths not allowed)"));
     }
 
     if (length < 0)
-        return throwRangeError(exec, "Requested length is negative");
-    return ViewClass::create(exec, structure, length);
-}
-
-template<typename ViewClass>
-static EncodedJSValue JSC_HOST_CALL constructGenericTypedArrayView(ExecState* exec)
-{
-    Structure* structure =
-        asInternalFunction(exec->callee())->globalObject()->typedArrayStructure(
-            ViewClass::TypedArrayStorageType);
-
-    if (!exec->argumentCount()) {
-        if (ViewClass::TypedArrayStorageType == TypeDataView)
-            return throwVMError(exec, createTypeError(exec, "DataView constructor requires at least one argument."));
-
-        return JSValue::encode(ViewClass::create(exec, structure, 0));
-    }
-
-    return JSValue::encode(constructGenericTypedArrayViewWithFirstArgument<ViewClass, true>(exec, structure, JSValue::encode(exec->uncheckedArgument(0))));
+        return throwVMError(exec, createRangeError(exec, "Requested length is negative"));
+    return JSValue::encode(ViewClass::create(exec, structure, length));
 }
 
 template<typename ViewClass>
index b13493b..a090869 100644 (file)
@@ -66,6 +66,10 @@ EncodedJSValue JSC_HOST_CALL genericTypedArrayViewProtoFuncSet(ExecState* exec)
     if (!exec->argumentCount())
         return throwVMError(exec, createTypeError(exec, "Expected at least one argument"));
 
+    JSObject* sourceArray = jsDynamicCast<JSObject*>(exec->uncheckedArgument(0));
+    if (!sourceArray)
+        return throwVMError(exec, createTypeError(exec, "First argument should be an object"));
+
     unsigned offset;
     if (exec->argumentCount() >= 2) {
         offset = exec->uncheckedArgument(1).toUInt32(exec);
@@ -74,16 +78,7 @@ EncodedJSValue JSC_HOST_CALL genericTypedArrayViewProtoFuncSet(ExecState* exec)
     } else
         offset = 0;
 
-    JSObject* sourceArray = jsDynamicCast<JSObject*>(exec->uncheckedArgument(0));
-    if (!sourceArray)
-        return throwVMError(exec, createTypeError(exec, "First argument should be an object"));
-
-    unsigned length;
-    if (isTypedView(sourceArray->classInfo()->typedArrayStorageType))
-        length = jsDynamicCast<JSArrayBufferView*>(sourceArray)->length();
-    else
-        length = sourceArray->get(exec, exec->vm().propertyNames->length).toUInt32(exec);
-
+    unsigned length = sourceArray->get(exec, exec->vm().propertyNames->length).toUInt32(exec);
     if (exec->hadException())
         return JSValue::encode(jsUndefined());
 
@@ -288,6 +283,7 @@ EncodedJSValue JSC_HOST_CALL genericTypedArrayViewProtoGetterFuncByteOffset(Exec
     return JSValue::encode(jsNumber(thisObject->byteOffset()));
 }
 
+
 template<typename ViewClass>
 EncodedJSValue JSC_HOST_CALL genericTypedArrayViewProtoFuncReverse(ExecState* exec)
 {
diff --git a/Source/JavaScriptCore/tests/stress/typedarray-construct-iterator.js b/Source/JavaScriptCore/tests/stress/typedarray-construct-iterator.js
deleted file mode 100644 (file)
index d1d80d5..0000000
+++ /dev/null
@@ -1,66 +0,0 @@
-// Test a bunch of things about typed array constructors with iterators.
-
-// Test that the dfg actually respects iterators.
-let foo = [1,2,3,4];
-
-function iterator() {
-    return { i: 0,
-             next: function() {
-                 if (this.i < foo.length/2) {
-                     return { done: false,
-                              value: foo[this.i++]
-                            };
-                 }
-                 return { done: true };
-             }
-           };
-}
-
-foo[Symbol.iterator] = iterator;
-
-(function body() {
-
-    for (var i = 1; i < 100000; i++) {
-        if (new Int32Array(foo).length !== 2)
-            throw "iterator did not run";
-    }
-
-})();
-
-// Test that the optimizations used for iterators during construction is valid.
-
-foo = { 0:0, 1:1, 2:2, 3:3 };
-count = 4;
-foo.__defineGetter__("length", function() {
-    return count--;
-});
-
-foo[Symbol.iterator] = Array.prototype[Symbol.iterator];
-
-if (new Int32Array(foo).length !== 2)
-    throw "iterator did not run";
-
-// Test that we handle length is unset... whoops.
-
-foo = { 0:0, 2:2, 3:3 };
-
-if (new Int32Array(foo).length !== 0)
-    throw "did not handle object with unset length";
-
-// Test that we handle prototypes with accessors.
-
-foo = { 0:0, 2:2, 3:3 };
-foo[Symbol.iterator] = Array.prototype[Symbol.iterator];
-foo.length = 4;
-bar = { };
-
-bar.__defineGetter__("1", function() {
-    foo.length = 0;
-    return 1;
-});
-
-
-foo.__proto__ = bar;
-
-if (new Int32Array(foo).length !== 2)
-    throw "did not handle object with accessor on prototype";