Title: [97791] trunk
Revision
97791
Author
[email protected]
Date
2011-10-18 14:57:59 -0700 (Tue, 18 Oct 2011)

Log Message

Array.prototype methods missing exception checks
https://bugs.webkit.org/show_bug.cgi?id=70360

Reviewed by Geoff Garen.

Missing exception checks after calls to the static getProperty helper,
these may result in the wrong exception being thrown (or an ASSERT being hit,
as is currently the case running test-262).

Source/_javascript_Core: 

No performance impact.

* runtime/ArrayPrototype.cpp:
(JSC::arrayProtoFuncConcat):
(JSC::arrayProtoFuncReverse):
(JSC::arrayProtoFuncShift):
(JSC::arrayProtoFuncSlice):
(JSC::arrayProtoFuncSplice):
(JSC::arrayProtoFuncUnShift):
(JSC::arrayProtoFuncReduce):
(JSC::arrayProtoFuncReduceRight):
(JSC::arrayProtoFuncIndexOf):
(JSC::arrayProtoFuncLastIndexOf):

LayoutTests: 

* fast/js/array-proto-func-property-getter-except-expected.txt:
* fast/js/script-tests/array-proto-func-property-getter-except.js:
    - The test results that change were only testing for an absence of a crash; the new results are correct
      (the first exception thrown should be the one returned).
* sputnik/Conformance/15_Native_Objects/15.5_String/15.5.4/15.5.4.10_String.prototype.match/S15.5.4.10_A1_T4-expected.txt:
* sputnik/Conformance/15_Native_Objects/15.5_String/15.5.4/15.5.4.10_String.prototype.match/S15.5.4.10_A1_T6-expected.txt:
* sputnik/Conformance/15_Native_Objects/15.5_String/15.5.4/15.5.4.10_String.prototype.match/S15.5.4.10_A1_T7-expected.txt:
* sputnik/Conformance/15_Native_Objects/15.5_String/15.5.4/15.5.4.10_String.prototype.match/S15.5.4.10_A1_T8-expected.txt:
* sputnik/Conformance/15_Native_Objects/15.5_String/15.5.4/15.5.4.10_String.prototype.match/S15.5.4.10_A1_T9-expected.txt:
* sputnik/Conformance/15_Native_Objects/15.5_String/15.5.4/15.5.4.11_String.prototype.replace/S15.5.4.11_A1_T11-expected.txt:
* sputnik/Conformance/15_Native_Objects/15.5_String/15.5.4/15.5.4.11_String.prototype.replace/S15.5.4.11_A1_T12-expected.txt:
* sputnik/Conformance/15_Native_Objects/15.5_String/15.5.4/15.5.4.12_String.prototype.search/S15.5.4.12_A1_T4-expected.txt:
    - Check in passing results.

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (97790 => 97791)


--- trunk/LayoutTests/ChangeLog	2011-10-18 21:07:01 UTC (rev 97790)
+++ trunk/LayoutTests/ChangeLog	2011-10-18 21:57:59 UTC (rev 97791)
@@ -1,3 +1,28 @@
+2011-10-18  Gavin Barraclough  <[email protected]>
+
+        Array.prototype methods missing exception checks
+        https://bugs.webkit.org/show_bug.cgi?id=70360
+
+        Reviewed by Geoff Garen.
+
+        Missing exception checks after calls to the static getProperty helper,
+        these may result in the wrong exception being thrown (or an ASSERT being hit,
+        as is currently the case running test-262).
+
+        * fast/js/array-proto-func-property-getter-except-expected.txt:
+        * fast/js/script-tests/array-proto-func-property-getter-except.js:
+            - The test results that change were only testing for an absence of a crash; the new results are correct
+              (the first exception thrown should be the one returned).
+        * sputnik/Conformance/15_Native_Objects/15.5_String/15.5.4/15.5.4.10_String.prototype.match/S15.5.4.10_A1_T4-expected.txt:
+        * sputnik/Conformance/15_Native_Objects/15.5_String/15.5.4/15.5.4.10_String.prototype.match/S15.5.4.10_A1_T6-expected.txt:
+        * sputnik/Conformance/15_Native_Objects/15.5_String/15.5.4/15.5.4.10_String.prototype.match/S15.5.4.10_A1_T7-expected.txt:
+        * sputnik/Conformance/15_Native_Objects/15.5_String/15.5.4/15.5.4.10_String.prototype.match/S15.5.4.10_A1_T8-expected.txt:
+        * sputnik/Conformance/15_Native_Objects/15.5_String/15.5.4/15.5.4.10_String.prototype.match/S15.5.4.10_A1_T9-expected.txt:
+        * sputnik/Conformance/15_Native_Objects/15.5_String/15.5.4/15.5.4.11_String.prototype.replace/S15.5.4.11_A1_T11-expected.txt:
+        * sputnik/Conformance/15_Native_Objects/15.5_String/15.5.4/15.5.4.11_String.prototype.replace/S15.5.4.11_A1_T12-expected.txt:
+        * sputnik/Conformance/15_Native_Objects/15.5_String/15.5.4/15.5.4.12_String.prototype.search/S15.5.4.12_A1_T4-expected.txt:
+            - Check in passing results.
+
 2011-10-18  Dirk Pranke  <[email protected]>
 
         Add missing baselines for fast/selectors/visited-descendant.

Modified: trunk/LayoutTests/fast/js/array-proto-func-property-getter-except-expected.txt (97790 => 97791)


--- trunk/LayoutTests/fast/js/array-proto-func-property-getter-except-expected.txt	2011-10-18 21:07:01 UTC (rev 97790)
+++ trunk/LayoutTests/fast/js/array-proto-func-property-getter-except-expected.txt	2011-10-18 21:57:59 UTC (rev 97791)
@@ -14,11 +14,11 @@
 PASS test(Array.prototype.join) is true
 PASS test(Array.prototype.pop) is true
 PASS test(Array.prototype.push) is false
-PASS test(Array.prototype.reverse) is false
-PASS test(Array.prototype.shift) is false
+PASS test(Array.prototype.reverse) is true
+PASS test(Array.prototype.shift) is true
 PASS test(Array.prototype.slice) is true
 PASS test(Array.prototype.splice) is true
-PASS test(Array.prototype.unshift) is false
+PASS test(Array.prototype.unshift) is true
 PASS test(Array.prototype.indexOf) is true
 PASS test(Array.prototype.lastIndexOf) is true
 PASS successfullyParsed is true

Modified: trunk/LayoutTests/fast/js/script-tests/array-proto-func-property-getter-except.js (97790 => 97791)


--- trunk/LayoutTests/fast/js/script-tests/array-proto-func-property-getter-except.js	2011-10-18 21:07:01 UTC (rev 97790)
+++ trunk/LayoutTests/fast/js/script-tests/array-proto-func-property-getter-except.js	2011-10-18 21:57:59 UTC (rev 97791)
@@ -36,11 +36,11 @@
 shouldBeTrue("test(Array.prototype.join)");
 shouldBeTrue("test(Array.prototype.pop)");
 shouldBeFalse("test(Array.prototype.push)");
-shouldBeFalse("test(Array.prototype.reverse)");
-shouldBeFalse("test(Array.prototype.shift)");
+shouldBeTrue("test(Array.prototype.reverse)");
+shouldBeTrue("test(Array.prototype.shift)");
 shouldBeTrue("test(Array.prototype.slice)");
 shouldBeTrue("test(Array.prototype.splice)");
-shouldBeFalse("test(Array.prototype.unshift)");
+shouldBeTrue("test(Array.prototype.unshift)");
 shouldBeTrue("test(Array.prototype.indexOf)");
 shouldBeTrue("test(Array.prototype.lastIndexOf)");
 

Modified: trunk/Source/_javascript_Core/ChangeLog (97790 => 97791)


--- trunk/Source/_javascript_Core/ChangeLog	2011-10-18 21:07:01 UTC (rev 97790)
+++ trunk/Source/_javascript_Core/ChangeLog	2011-10-18 21:57:59 UTC (rev 97791)
@@ -1,3 +1,28 @@
+2011-10-18  Gavin Barraclough  <[email protected]>
+
+        Array.prototype methods missing exception checks
+        https://bugs.webkit.org/show_bug.cgi?id=70360
+
+        Reviewed by Geoff Garen.
+
+        Missing exception checks after calls to the static getProperty helper,
+        these may result in the wrong exception being thrown (or an ASSERT being hit,
+        as is currently the case running test-262).
+
+        No performance impact.
+
+        * runtime/ArrayPrototype.cpp:
+        (JSC::arrayProtoFuncConcat):
+        (JSC::arrayProtoFuncReverse):
+        (JSC::arrayProtoFuncShift):
+        (JSC::arrayProtoFuncSlice):
+        (JSC::arrayProtoFuncSplice):
+        (JSC::arrayProtoFuncUnShift):
+        (JSC::arrayProtoFuncReduce):
+        (JSC::arrayProtoFuncReduceRight):
+        (JSC::arrayProtoFuncIndexOf):
+        (JSC::arrayProtoFuncLastIndexOf):
+
 2011-10-18  Adam Barth  <[email protected]>
 
         Always enable ENABLE(XPATH)

Modified: trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp (97790 => 97791)


--- trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp	2011-10-18 21:07:01 UTC (rev 97790)
+++ trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp	2011-10-18 21:57:59 UTC (rev 97791)
@@ -350,7 +350,10 @@
             unsigned length = curArg.get(exec, exec->propertyNames().length).toUInt32(exec);
             JSObject* curObject = curArg.toObject(exec);
             for (unsigned k = 0; k < length; ++k) {
-                if (JSValue v = getProperty(exec, curObject, k))
+                JSValue v = getProperty(exec, curObject, k);
+                if (exec->hadException())
+                    return JSValue::encode(jsUndefined());
+                if (v)
                     arr->putVirtual(exec, n, v);
                 n++;
             }
@@ -432,7 +435,11 @@
     for (unsigned k = 0; k < middle; k++) {
         unsigned lk1 = length - k - 1;
         JSValue obj2 = getProperty(exec, thisObj, lk1);
+        if (exec->hadException())
+            return JSValue::encode(jsUndefined());
         JSValue obj = getProperty(exec, thisObj, k);
+        if (exec->hadException())
+            return JSValue::encode(jsUndefined());
 
         if (obj2)
             thisObj->putVirtual(exec, k, obj2);
@@ -464,7 +471,10 @@
             ((JSArray *)thisObj)->shiftCount(exec, 1);
         else {
             for (unsigned k = 1; k < length; k++) {
-                if (JSValue obj = getProperty(exec, thisObj, k))
+                JSValue obj = getProperty(exec, thisObj, k);
+                if (exec->hadException())
+                    return JSValue::encode(jsUndefined());
+                if (obj)
                     thisObj->putVirtual(exec, k - 1, obj);
                 else
                     thisObj->deletePropertyVirtual(exec, k - 1);
@@ -493,7 +503,10 @@
 
     unsigned n = 0;
     for (unsigned k = begin; k < end; k++, n++) {
-        if (JSValue v = getProperty(exec, thisObj, k))
+        JSValue v = getProperty(exec, thisObj, k);
+        if (exec->hadException())
+            return JSValue::encode(jsUndefined());
+        if (v)
             resObj->putVirtual(exec, n, v);
     }
     resObj->setLength(n);
@@ -588,8 +601,12 @@
     JSArray* resObj = JSArray::create(exec->globalData(), exec->lexicalGlobalObject()->arrayStructure(), deleteCount, CreateCompact);
     JSValue result = resObj;
     JSGlobalData& globalData = exec->globalData();
-    for (unsigned k = 0; k < deleteCount; k++)
-        resObj->uncheckedSetIndex(globalData, k, getProperty(exec, thisObj, k + begin));
+    for (unsigned k = 0; k < deleteCount; k++) {
+        JSValue v = getProperty(exec, thisObj, k + begin);
+        if (exec->hadException())
+            return JSValue::encode(jsUndefined());
+        resObj->uncheckedSetIndex(globalData, k, v);
+    }
 
     resObj->setLength(deleteCount);
 
@@ -600,7 +617,10 @@
                 ((JSArray *)thisObj)->shiftCount(exec, deleteCount - additionalArgs);
             else {
                 for (unsigned k = begin; k < length - deleteCount; ++k) {
-                    if (JSValue v = getProperty(exec, thisObj, k + deleteCount))
+                    JSValue v = getProperty(exec, thisObj, k + deleteCount);
+                    if (exec->hadException())
+                        return JSValue::encode(jsUndefined());
+                    if (v)
                         thisObj->putVirtual(exec, k + additionalArgs, v);
                     else
                         thisObj->deletePropertyVirtual(exec, k + additionalArgs);
@@ -613,7 +633,10 @@
                 ((JSArray *)thisObj)->unshiftCount(exec, additionalArgs - deleteCount);
             else {
                 for (unsigned k = length - deleteCount; k > begin; --k) {
-                    if (JSValue obj = getProperty(exec, thisObj, k + deleteCount - 1))
+                    JSValue obj = getProperty(exec, thisObj, k + deleteCount - 1);
+                    if (exec->hadException())
+                        return JSValue::encode(jsUndefined());
+                    if (obj)
                         thisObj->putVirtual(exec, k + additionalArgs - 1, obj);
                     else
                         thisObj->deletePropertyVirtual(exec, k + additionalArgs - 1);
@@ -643,7 +666,10 @@
             ((JSArray *)thisObj)->unshiftCount(exec, nrArgs);
         else {
             for (unsigned k = length; k > 0; --k) {
-                if (JSValue v = getProperty(exec, thisObj, k - 1))
+                JSValue v = getProperty(exec, thisObj, k - 1);
+                if (exec->hadException())
+                    return JSValue::encode(jsUndefined());
+                if (v)
                     thisObj->putVirtual(exec, k + nrArgs - 1, v);
                 else
                     thisObj->deletePropertyVirtual(exec, k + nrArgs - 1);
@@ -973,6 +999,8 @@
     } else {
         for (i = 0; i < length; i++) {
             rv = getProperty(exec, thisObj, i);
+            if (exec->hadException())
+                return JSValue::encode(jsUndefined());
             if (rv)
                 break;
         }
@@ -1048,6 +1076,8 @@
     } else {
         for (i = 0; i < length; i++) {
             rv = getProperty(exec, thisObj, length - i - 1);
+            if (exec->hadException())
+                return JSValue::encode(jsUndefined());
             if (rv)
                 break;
         }
@@ -1104,6 +1134,8 @@
     JSValue searchElement = exec->argument(0);
     for (; index < length; ++index) {
         JSValue e = getProperty(exec, thisObj, index);
+        if (exec->hadException())
+            return JSValue::encode(jsUndefined());
         if (!e)
             continue;
         if (JSValue::strictEqual(exec, searchElement, e))
@@ -1138,6 +1170,8 @@
     do {
         ASSERT(index < length);
         JSValue e = getProperty(exec, thisObj, index);
+        if (exec->hadException())
+            return JSValue::encode(jsUndefined());
         if (!e)
             continue;
         if (JSValue::strictEqual(exec, searchElement, e))
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to