2015-06-24 Darin Adler <darin@apple.com>
authordarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 24 Jun 2015 08:14:14 +0000 (08:14 +0000)
committerdarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 24 Jun 2015 08:14:14 +0000 (08:14 +0000)
        Fix Array.concat with RuntimeArray (regression from my last patch)

        * runtime/ArrayPrototype.cpp:
        (JSC::arrayProtoFuncConcat): Use getLength instead of JSArray::length.

        * runtime/JSArray.cpp:
        (JSC::JSArray::defineOwnProperty): Added comment about use of
        JSArray::length here that is incorrect (in a really non-obvious way).
        (JSC::JSArray::fillArgList): Ditto.
        (JSC::JSArray::copyToArguments): Ditto.

        * runtime/JSArray.h: Added a comment explaining that it is not always
        safe to use JSArray::length.

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/ArrayPrototype.cpp
Source/JavaScriptCore/runtime/JSArray.cpp
Source/JavaScriptCore/runtime/JSArray.h

index 42b5098c89f7259bac173d8c774d637c0f8b0762..ae11dc71c8bf618ec3dc3d470828f3d10a59e887 100644 (file)
@@ -1,3 +1,19 @@
+2015-06-24  Darin Adler  <darin@apple.com>
+
+        Fix Array.concat with RuntimeArray (regression from my last patch)
+
+        * runtime/ArrayPrototype.cpp:
+        (JSC::arrayProtoFuncConcat): Use getLength instead of JSArray::length.
+
+        * runtime/JSArray.cpp:
+        (JSC::JSArray::defineOwnProperty): Added comment about use of
+        JSArray::length here that is incorrect (in a really non-obvious way).
+        (JSC::JSArray::fillArgList): Ditto.
+        (JSC::JSArray::copyToArguments): Ditto.
+
+        * runtime/JSArray.h: Added a comment explaining that it is not always
+        safe to use JSArray::length.
+
 2015-06-23  Mark Lam  <mark.lam@apple.com>
 
         Gardening: Fixing 2 bad asserts from r185889.
index 29d2a4408c880fe6dab6a19086c8242d16f3d9d1..1bdb61921d40e2d481f4f01f5156e79bab674c49 100644 (file)
@@ -201,7 +201,7 @@ void shift(ExecState* exec, JSObject* thisObj, unsigned header, unsigned current
 
     if (isJSArray(thisObj)) {
         JSArray* array = asArray(thisObj);
-        if (array->length() == length && asArray(thisObj)->shiftCount<shiftCountMode>(exec, header, count))
+        if (array->length() == length && array->shiftCount<shiftCountMode>(exec, header, count))
             return;
     }
 
@@ -290,7 +290,7 @@ EncodedJSValue JSC_HOST_CALL arrayProtoFuncToString(ExecState* exec)
 
     ASSERT(isJSArray(thisValue));
     JSArray* thisArray = asArray(thisValue);
-    
+
     unsigned length = thisArray->length();
 
     StringRecursionChecker checker(exec, thisArray);
@@ -414,9 +414,12 @@ EncodedJSValue JSC_HOST_CALL arrayProtoFuncConcat(ExecState* exec)
     Checked<unsigned, RecordOverflow> finalArraySize = 0;
 
     for (unsigned i = 0; ; ++i) {
-        if (JSArray* currentArray = jsDynamicCast<JSArray*>(curArg))
-            finalArraySize += currentArray->length();
-        else
+        if (JSArray* currentArray = jsDynamicCast<JSArray*>(curArg)) {
+            // Can't use JSArray::length here because this might be a RuntimeArray!
+            finalArraySize += getLength(exec, currentArray);
+            if (exec->hadException())
+                return JSValue::encode(jsUndefined());
+        } else
             ++finalArraySize;
         if (i == argCount)
             break;
@@ -434,7 +437,8 @@ EncodedJSValue JSC_HOST_CALL arrayProtoFuncConcat(ExecState* exec)
     unsigned n = 0;
     for (unsigned i = 0; ; ++i) {
         if (JSArray* currentArray = jsDynamicCast<JSArray*>(curArg)) {
-            unsigned length = currentArray->length();
+            // Can't use JSArray::length here because this might be a RuntimeArray!
+            unsigned length = getLength(exec, currentArray);
             if (exec->hadException())
                 return JSValue::encode(jsUndefined());
             for (unsigned k = 0; k < length; ++k) {
index 1bad31e8f30e50410ec0f65a9cf7da004fdf0fe0..51914da67e8fb517ae1edf680b1399387d156eee 100644 (file)
@@ -111,6 +111,7 @@ bool JSArray::defineOwnProperty(JSObject* object, ExecState* exec, PropertyName
         }
 
         // Based on SameValue check in 8.12.9, this is always okay.
+        // FIXME: Nothing prevents this from being called on a RuntimeArray, and the length function will always return 0 in that case.
         if (newLen == array->length()) {
             if (descriptor.writablePresent())
                 array->setLengthWritable(exec, descriptor.writable());
@@ -160,6 +161,7 @@ bool JSArray::defineOwnProperty(JSObject* object, ExecState* exec, PropertyName
     if (Optional<uint32_t> optionalIndex = parseIndex(propertyName)) {
         // b. Reject if index >= oldLen and oldLenDesc.[[Writable]] is false.
         uint32_t index = optionalIndex.value();
+        // FIXME: Nothing prevents this from being called on a RuntimeArray, and the length function will always return 0 in that case.
         if (index >= array->length() && !array->isLengthWritable())
             return reject(exec, throwException, "Attempting to define numeric property on array with non-writable length property.");
         // c. Let succeeded be the result of calling the default [[DefineOwnProperty]] internal method (8.12.9) on A passing P, Desc, and false as arguments.
@@ -1090,7 +1092,8 @@ void JSArray::fillArgList(ExecState* exec, MarkedArgumentBuffer& args)
             break;
         args.append(v.get());
     }
-    
+
+    // FIXME: What prevents this from being called with a RuntimeArray? The length function will always return 0 in that case.
     for (; i < length(); ++i)
         args.append(get(exec, i));
 }
@@ -1101,7 +1104,10 @@ void JSArray::copyToArguments(ExecState* exec, VirtualRegister firstElementDest,
     WriteBarrier<Unknown>* vector;
     unsigned vectorEnd;
     length += offset; // We like to think of the length as being our length, rather than the output length.
+
+    // FIXME: What prevents this from being called with a RuntimeArray? The length function will always return 0 in that case.
     ASSERT(length == this->length());
+
     switch (indexingType()) {
     case ArrayClass:
         return;
index af061507c1c25707ae11621438eb4c3abf6e6d70..72c40305576df17e9b43b57036ac039d34608359 100644 (file)
@@ -66,9 +66,11 @@ public:
     JS_EXPORT_PRIVATE static bool getOwnPropertySlot(JSObject*, ExecState*, PropertyName, PropertySlot&);
 
     DECLARE_EXPORT_INFO;
-        
+
+    // OK if we know this is a JSArray, but not if it could be an object of a derived class; for RuntimeArray this always returns 0.
     unsigned length() const { return getArrayLength(); }
-    // OK to use on new arrays, but not if it might be a RegExpMatchArray.
+
+    // OK to use on new arrays, but not if it might be a RegExpMatchArray or RuntimeArray.
     JS_EXPORT_PRIVATE bool setLength(ExecState*, unsigned, bool throwException = false);
 
     JS_EXPORT_PRIVATE void push(ExecState*, JSValue);