Exception check for OOM is a bit too late in JSBigInt::exponentiate.
authorrmorisset@apple.com <rmorisset@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 13 May 2020 03:09:50 +0000 (03:09 +0000)
committerrmorisset@apple.com <rmorisset@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 13 May 2020 03:09:50 +0000 (03:09 +0000)
https://bugs.webkit.org/show_bug.cgi?id=211823

Reviewed by Mark Lam.

JSTests:

Just add one more case so that this test now catches this bug (the control-flow through JSBigInt::exponentiate is pretty convoluted).

* stress/bigint-exponential-oom.js:
(shouldThrow):

Source/JavaScriptCore:

We were doing multiplyImpl(...).payload.asHeapBigInt(), but multiplyImpl can return a null payload if it causes an exception.
So we must first check whether an exception was raised, and only if not can we do asHeapBigInt.

* runtime/JSBigInt.cpp:
(JSC::JSBigInt::exponentiateImpl):

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

JSTests/ChangeLog
JSTests/stress/bigint-exponential-oom.js
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/JSBigInt.cpp

index 861dba7..38a8b35 100644 (file)
@@ -1,3 +1,15 @@
+2020-05-12  Robin Morisset  <rmorisset@apple.com>
+
+        Exception check for OOM is a bit too late in JSBigInt::exponentiate.
+        https://bugs.webkit.org/show_bug.cgi?id=211823
+
+        Reviewed by Mark Lam.
+
+        Just add one more case so that this test now catches this bug (the control-flow through JSBigInt::exponentiate is pretty convoluted).
+
+        * stress/bigint-exponential-oom.js:
+        (shouldThrow):
+
 2020-05-12  Saam Barati  <sbarati@apple.com>
 
         handling of Check in VarargsForwardingPhase is too pessimistic
index b9f906a..e7b55ac 100644 (file)
@@ -28,3 +28,6 @@ shouldThrow(() => {
 shouldThrow(() => {
     2n ** 0xfffffffffffffffn;
 }, `Error: Out of memory: BigInt generated from this operation is too big`);
+shouldThrow(() => {
+    10n ** 1000000n;
+}, `Error: Out of memory: BigInt generated from this operation is too big`);
index ed3a8ba..cf00ea8 100644 (file)
@@ -1,3 +1,16 @@
+2020-05-12  Robin Morisset  <rmorisset@apple.com>
+
+        Exception check for OOM is a bit too late in JSBigInt::exponentiate.
+        https://bugs.webkit.org/show_bug.cgi?id=211823
+
+        Reviewed by Mark Lam.
+
+        We were doing multiplyImpl(...).payload.asHeapBigInt(), but multiplyImpl can return a null payload if it causes an exception.
+        So we must first check whether an exception was raised, and only if not can we do asHeapBigInt.
+
+        * runtime/JSBigInt.cpp:
+        (JSC::JSBigInt::exponentiateImpl):
+
 2020-05-12  Saam Barati  <sbarati@apple.com>
 
         handling of Check in VarargsForwardingPhase is too pessimistic
index 82b7fb1..aff15bf 100644 (file)
@@ -482,15 +482,21 @@ JSBigInt::ImplResult JSBigInt::exponentiateImpl(JSGlobalObject* globalObject, Bi
 
     n >>= 1;
     for (; n; n >>= 1) {
-        JSBigInt* maybeResult = JSBigInt::multiplyImpl(globalObject, HeapBigIntImpl { runningSquare }, HeapBigIntImpl { runningSquare }).payload.asHeapBigInt();
+        ImplResult temp = JSBigInt::multiplyImpl(globalObject, HeapBigIntImpl { runningSquare }, HeapBigIntImpl { runningSquare });
         RETURN_IF_EXCEPTION(scope, nullptr);
+        ASSERT(temp.payload);
+        ASSERT(temp.payload.isHeapBigInt());
+        JSBigInt* maybeResult = temp.payload.asHeapBigInt();
         runningSquare = maybeResult;
         if (n & 1) {
             if (!result)
                 result = runningSquare;
             else {
-                maybeResult = JSBigInt::multiplyImpl(globalObject, HeapBigIntImpl { result }, HeapBigIntImpl { runningSquare }).payload.asHeapBigInt();
+                temp = JSBigInt::multiplyImpl(globalObject, HeapBigIntImpl { result }, HeapBigIntImpl { runningSquare });
                 RETURN_IF_EXCEPTION(scope, nullptr);
+                ASSERT(temp.payload);
+                ASSERT(temp.payload.isHeapBigInt());
+                maybeResult = temp.payload.asHeapBigInt();
                 result = maybeResult;
             }
         }