Changeset 185904 in webkit

Timestamp:

Jun 24, 2015, 1:14:14 AM (10 years ago)

Author:

Darin Adler

Message:

2015-06-24 Darin Adler <Darin Adler>

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.

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• runtime/ArrayPrototype.cpp (modified) (4 diffs)
• runtime/JSArray.cpp (modified) (4 diffs)
• runtime/JSArray.h (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/JavaScriptCore/runtime/ArrayPrototype.cpp

@@ -202 +202 @@
     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;
     }
@@ -291 +291 @@
     ASSERT(isJSArray(thisValue));
     JSArray* thisArray = asArray(thisValue);
-    
+
     unsigned length = thisArray->length();
 
@@ -415 +415 @@
 
     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)
@@ -435 +438 @@
     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());

trunk/Source/JavaScriptCore/runtime/JSArray.cpp

@@ -112 +112 @@
 
         // 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())
@@ -161 +162 @@
         // 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.");
@@ -1091 +1093 @@
         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));
@@ -1102 +1105 @@
     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:

trunk/Source/JavaScriptCore/runtime/JSArray.h

@@ -67 +67 @@
 
     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);