Fix crash in JSPromise::Resolve when 'then' getter is terminating

The crash scenario is as follows:
  1) Add a getter for 'then' to the Object prototype that is
     considered side-effecting.
  2) Evaluate a simple string using 'REPL' mode with side-effect checks
     enabled.
     Note: REPL mode is not strictly necessary, but it causes a 'then'
     lookup as the evaluation result is not a promise.
  3) Calling the 'then' getter causes a termination exception, due
     to the side-effect check. JSPromise::Resolve then tries to
     put the termination exception as the reject reason, which causes
     a CHECK failure.

The solution is to check for termination in the "abrupt completion"
case when 'then' was retrieved.

Bug: chromium:1140845
Change-Id: I72b644cd49355cea40f599fcbe80264e99ed7bd6
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2501283Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Commit-Queue: Simon Zünd <szuend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70785}

src/objects/objects.cc

@@ -5446,6 +5446,11 @@ MaybeHandle<Object> JSPromise::Resolve(Handle<JSPromise> promise,
   // 10. If then is an abrupt completion, then
   Handle<Object> then_action;
   if (!then.ToHandle(&then_action)) {
+    // The "then" lookup can cause termination.
+    if (!isolate->is_catchable_by_javascript(isolate->pending_exception())) {
+      return kNullMaybeHandle;
+    }
+
     // a. Return RejectPromise(promise, then.[[Value]]).
     Handle<Object> reason(isolate->pending_exception(), isolate);
     isolate->clear_pending_exception();

test/inspector/runtime/regression-1140845-expected.txt

+Regression test for crbug.com/1140845. Check that a "then" gettter on the object prototype does not crash V8
+Evaluating a simple string 'foo' does not cause a crash, but a side-effect exception.
+{
+    id : <messageId>
+    result : {
+        exceptionDetails : {
+            columnNumber : -1
+            exception : {
+                className : EvalError
+                description : EvalError: Possible side-effect in debug-evaluate
+                objectId : <objectId>
+                subtype : error
+                type : object
+            }
+            exceptionId : <exceptionId>
+            lineNumber : -1
+            scriptId : <scriptId>
+            text : Uncaught
+        }
+        result : {
+            className : EvalError
+            description : EvalError: Possible side-effect in debug-evaluate
+            objectId : <objectId>
+            subtype : error
+            type : object
+        }
+    }
+}
+Evaluating a simple string 'foo' with side-effets should give us the string.
+{
+    id : <messageId>
+    result : {
+        result : {
+            type : string
+            value : foo
+        }
+    }
+}

test/inspector/runtime/regression-1140845.js

+// Copyright 2020 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+let {session, contextGroup, Protocol} =
+    InspectorTest.start('Regression test for crbug.com/1140845. Check that a "then" gettter on the object prototype does not crash V8');
+
+const setupScript = `
+  let obj = Object.prototype;
+  obj.__defineGetter__('then', function() {console.log("foo")});
+`;
+
+(async function() {
+  await Protocol.Debugger.enable();
+
+  // Set a custom `then` method on the Object prototype. This causes termination
+  // when 'then' is retrieved, as the 'then' getter is side-effecting.
+  await Protocol.Runtime.evaluate({
+    expression: setupScript,
+  });
+
+  InspectorTest.log(`Evaluating a simple string 'foo' does not cause a crash, but a side-effect exception.`);
+  InspectorTest.logMessage(await Protocol.Runtime.evaluate({
+    expression: `"foo"`,
+    replMode: true,
+    throwOnSideEffect: true,
+  }));
+
+  InspectorTest.log(`Evaluating a simple string 'foo' with side-effets should give us the string.`);
+  InspectorTest.logMessage(await Protocol.Runtime.evaluate({
+    expression: `"foo"`,
+    replMode: true,
+  }));
+
+  InspectorTest.completeTest();
+})();