Changeset 253515 in webkit

Timestamp:

Dec 13, 2019, 5:51:00 PM (5 years ago)

Author:

mark.lam@apple.com

Message:

Fix bad exception assertion in ExceptionHelpers.cpp's createError().
​https://bugs.webkit.org/show_bug.cgi?id=205230
<rdar://problem/57875688>

Reviewed by Yusuke Suzuki.

JSTests:

• stress/test-exception-assert-in-ExceptionHelpers-createError.js: Added.

Source/JavaScriptCore:

The code in createError() was doing the following:

String valueDescription = errorDescriptionForValue(globalObject, value);

EXCEPTION_ASSERT(scope.exception()

!!valueDescription);

if (!valueDescription) {

scope.clearException();
return createOutOfMemoryError(globalObject);

}

If errorDescriptionForValue() throws an exception, then we expect the
valueDescription string to be null so that we can throw an OutOfMemoryError.
However, errorDescriptionForValue() can detect an imminent overflow in String
length and just return a null string without throwing an exception which fails
the above assertion.

The fix is to simply do an explicit exception check in addition to the null string
check and remove the assertion.

• runtime/ExceptionHelpers.cpp:

(JSC::createError):

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/stress/test-exception-assert-in-ExceptionHelpers-createError.js (added)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/runtime/ExceptionHelpers.cpp (modified) (1 diff)

trunk/JSTests/ChangeLog

@@ +1 @@
+2019-12-13  Mark Lam  <mark.lam@apple.com>
+
+        Fix bad exception assertion in ExceptionHelpers.cpp's createError().
+        https://bugs.webkit.org/show_bug.cgi?id=205230
+        <rdar://problem/57875688>
+
+        Reviewed by Yusuke Suzuki.
+
+        * stress/test-exception-assert-in-ExceptionHelpers-createError.js: Added.
+
 2019-12-12  Yusuke Suzuki  <ysuzuki@apple.com>
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2019-12-13  Mark Lam  <mark.lam@apple.com>
+
+        Fix bad exception assertion in ExceptionHelpers.cpp's createError().
+        https://bugs.webkit.org/show_bug.cgi?id=205230
+        <rdar://problem/57875688>
+
+        Reviewed by Yusuke Suzuki.
+
+        The code in createError() was doing the following:
+
+            String valueDescription = errorDescriptionForValue(globalObject, value);
+            EXCEPTION_ASSERT(scope.exception() || !!valueDescription);
+            if (!valueDescription) {
+                scope.clearException();
+                return createOutOfMemoryError(globalObject);
+            }
+
+        If errorDescriptionForValue() throws an exception, then we expect the
+        valueDescription string to be null so that we can throw an OutOfMemoryError.
+        However, errorDescriptionForValue() can detect an imminent overflow in String
+        length and just return a null string without throwing an exception which fails
+        the above assertion.
+
+        The fix is to simply do an explicit exception check in addition to the null string
+        check and remove the assertion.
+
+        * runtime/ExceptionHelpers.cpp:
+        (JSC::createError):
+
 2019-12-13  Saam Barati  <sbarati@apple.com>
 

trunk/Source/JavaScriptCore/runtime/ExceptionHelpers.cpp

@@ -267 +267 @@
 
     String valueDescription = errorDescriptionForValue(globalObject, value);
-    EXCEPTION_ASSERT(scope.exception() || !!valueDescription);
-    if (!valueDescription) {
+    if (scope.exception() || !valueDescription) {
+        // When we see an exception, we're not returning immediately because
+        // we're in a CatchScope, i.e. no exceptions are thrown past this scope.
+        // We're using a CatchScope because the contract for createError() is
+        // that it only creates an error object; it doesn't throw it.
         scope.clearException();
         return createOutOfMemoryError(globalObject);