[inspector] Add throwOnSideEffect to Runtime.callFunctionOn.

In order to implement eager (side effect free) evaluation of arbitrary
accessor properties correctly, we need the ability to call getters while
guaranteeing that we don't trigger side effects. This is accomplished by
adding a `throwOnSideEffect` flag to the `Runtime.callFunctionOn` API,
similar to what's already available with the `Runtime.evaluate` and the
`Debugger.evaluateOnCallFrame` APIs.

Bug: chromium:1076820, chromium:1119900, chromium:1222114
Change-Id: If2d6c51376669cbc71a9dd3c79403d24d62aee43
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3001360
Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Yang Guo <yangguo@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#75556}

include/js_protocol.pdl

@@ -1347,6 +1347,8 @@ domain Runtime
       # Symbolic group name that can be used to release multiple objects. If objectGroup is not
       # specified and objectId is, objectGroup will be inherited from object.
       optional string objectGroup
+      # Whether to throw an exception if side effect cannot be ruled out during evaluation.
+      experimental optional boolean throwOnSideEffect
     returns
       # Call result.
       RemoteObject result

src/debug/debug-interface.cc

@@ -939,6 +939,31 @@ v8::Local<GeneratorObject> GeneratorObject::Cast(v8::Local<v8::Value> value) {
   return ToApiHandle<GeneratorObject>(Utils::OpenHandle(*value));
 }
 
+MaybeLocal<Value> CallFunctionOn(Local<Context> context,
+                                 Local<Function> function, Local<Value> recv,
+                                 int argc, Local<Value> argv[],
+                                 bool throw_on_side_effect) {
+  auto isolate = reinterpret_cast<i::Isolate*>(context->GetIsolate());
+  PREPARE_FOR_DEBUG_INTERFACE_EXECUTION_WITH_ISOLATE(isolate, Value);
+  auto self = Utils::OpenHandle(*function);
+  auto recv_obj = Utils::OpenHandle(*recv);
+  STATIC_ASSERT(sizeof(v8::Local<v8::Value>) == sizeof(i::Handle<i::Object>));
+  auto args = reinterpret_cast<i::Handle<i::Object>*>(argv);
+  // Disable breaks in side-effect free mode.
+  i::DisableBreak disable_break_scope(isolate->debug(), throw_on_side_effect);
+  if (throw_on_side_effect) {
+    isolate->debug()->StartSideEffectCheckMode();
+  }
+  Local<Value> result;
+  has_pending_exception = !ToLocal<Value>(
+      i::Execution::Call(isolate, self, recv_obj, argc, args), &result);
+  if (throw_on_side_effect) {
+    isolate->debug()->StopSideEffectCheckMode();
+  }
+  RETURN_ON_FAILED_EXECUTION(Value);
+  RETURN_ESCAPED(result);
+}
+
 MaybeLocal<v8::Value> EvaluateGlobal(v8::Isolate* isolate,
                                      v8::Local<v8::String> source,
                                      EvaluateGlobalMode mode, bool repl) {

src/debug/debug-interface.h

@@ -520,6 +520,11 @@ using RuntimeCallCounterCallback =
 void EnumerateRuntimeCallCounters(v8::Isolate* isolate,
                                   RuntimeCallCounterCallback callback);
 
+MaybeLocal<Value> CallFunctionOn(Local<Context> context,
+                                 Local<Function> function, Local<Value> recv,
+                                 int argc, Local<Value> argv[],
+                                 bool throw_on_side_effect);
+
 enum class EvaluateGlobalMode {
   kDefault,
   kDisableBreaks,

src/inspector/v8-runtime-agent-impl.cc

@@ -113,7 +113,7 @@ void innerCallFunctionOn(
     v8::Local<v8::Value> recv, const String16& expression,
     Maybe<protocol::Array<protocol::Runtime::CallArgument>> optionalArguments,
     bool silent, WrapMode wrapMode, bool userGesture, bool awaitPromise,
-    const String16& objectGroup,
+    const String16& objectGroup, bool throw_on_side_effect,
     std::unique_ptr<V8RuntimeAgentImpl::CallFunctionOnCallback> callback) {
   V8InspectorImpl* inspector = session->inspector();
 
@@ -178,8 +178,9 @@ void innerCallFunctionOn(
   {
     v8::MicrotasksScope microtasksScope(inspector->isolate(),
                                         v8::MicrotasksScope::kRunMicrotasks);
-    maybeResultValue = functionValue.As<v8::Function>()->Call(
-        scope.context(), recv, argc, argv.get());
+    maybeResultValue = v8::debug::CallFunctionOn(
+        scope.context(), functionValue.As<v8::Function>(), recv, argc,
+        argv.get(), throw_on_side_effect);
   }
   // Re-initialize after running client's code, as it could have destroyed
   // context or session.
@@ -361,6 +362,7 @@ void V8RuntimeAgentImpl::callFunctionOn(
     Maybe<bool> silent, Maybe<bool> returnByValue, Maybe<bool> generatePreview,
     Maybe<bool> userGesture, Maybe<bool> awaitPromise,
     Maybe<int> executionContextId, Maybe<String16> objectGroup,
+    Maybe<bool> throwOnSideEffect,
     std::unique_ptr<CallFunctionOnCallback> callback) {
   if (objectId.isJust() && executionContextId.isJust()) {
     callback->sendFailure(Response::ServerError(
@@ -382,13 +384,13 @@ void V8RuntimeAgentImpl::callFunctionOn(
       callback->sendFailure(response);
       return;
     }
-    innerCallFunctionOn(m_session, scope, scope.object(), expression,
-                        std::move(optionalArguments), silent.fromMaybe(false),
-                        mode, userGesture.fromMaybe(false),
-                        awaitPromise.fromMaybe(false),
-                        objectGroup.isJust() ? objectGroup.fromMaybe(String16())
-                                             : scope.objectGroupName(),
-                        std::move(callback));
+    innerCallFunctionOn(
+        m_session, scope, scope.object(), expression,
+        std::move(optionalArguments), silent.fromMaybe(false), mode,
+        userGesture.fromMaybe(false), awaitPromise.fromMaybe(false),
+        objectGroup.isJust() ? objectGroup.fromMaybe(String16())
+                             : scope.objectGroupName(),
+        throwOnSideEffect.fromMaybe(false), std::move(callback));
   } else {
     int contextId = 0;
     Response response = ensureContext(m_inspector, m_session->contextGroupId(),
@@ -404,11 +406,12 @@ void V8RuntimeAgentImpl::callFunctionOn(
       callback->sendFailure(response);
       return;
     }
-    innerCallFunctionOn(m_session, scope, scope.context()->Global(), expression,
-                        std::move(optionalArguments), silent.fromMaybe(false),
-                        mode, userGesture.fromMaybe(false),
-                        awaitPromise.fromMaybe(false),
-                        objectGroup.fromMaybe(""), std::move(callback));
+    innerCallFunctionOn(
+        m_session, scope, scope.context()->Global(), expression,
+        std::move(optionalArguments), silent.fromMaybe(false), mode,
+        userGesture.fromMaybe(false), awaitPromise.fromMaybe(false),
+        objectGroup.fromMaybe(""), throwOnSideEffect.fromMaybe(false),
+        std::move(callback));
   }
 }
 

src/inspector/v8-runtime-agent-impl.h

@@ -82,7 +82,7 @@ class V8RuntimeAgentImpl : public protocol::Runtime::Backend {
       Maybe<bool> silent, Maybe<bool> returnByValue,
       Maybe<bool> generatePreview, Maybe<bool> userGesture,
       Maybe<bool> awaitPromise, Maybe<int> executionContextId,
-      Maybe<String16> objectGroup,
+      Maybe<String16> objectGroup, Maybe<bool> throwOnSideEffect,
       std::unique_ptr<CallFunctionOnCallback>) override;
   Response releaseObject(const String16& objectId) override;
   Response getProperties(

test/inspector/runtime/call-function-on-side-effect-free-expected.txt

@@ -0,0 +1,9 @@
+Tests side-effect-free Runtime.callFunctionOn()
+
+Running test: testCallFunctionOnSideEffectFree
+function getA() { return this.a; }: ok
+function setA(a) { this.a = a; }: throws
+function getB() { return this.b; }: ok
+function setB(b) { this.b = b; }: throws
+function setSomeGlobal() { globalThis.someGlobal = this; }: throws
+function localSideEffect() { const date = new Date(); date.setDate(this.a); return date; }: ok

test/inspector/runtime/call-function-on-side-effect-free.js

@@ -0,0 +1,31 @@
+// Copyright 2021 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('Tests side-effect-free Runtime.callFunctionOn()');
+
+async function check(callable, object, ...args) {
+  const functionDeclaration = callable.toString();
+  const {result:{exceptionDetails}} = await Protocol.Runtime.callFunctionOn({
+    objectId: object.objectId,
+    functionDeclaration,
+    arguments: args.map(arg => (arg && arg.objectId) ? {objectId: arg.objectId} : {value: arg}),
+    throwOnSideEffect: true,
+  });
+  InspectorTest.log(`${functionDeclaration}: ${exceptionDetails ? 'throws' : 'ok'}`);
+};
+
+InspectorTest.runAsyncTestSuite([
+  async function testCallFunctionOnSideEffectFree() {
+    await Protocol.Runtime.enable();
+    const {result: {result: object}} = await Protocol.Runtime.evaluate({ expression: '({a: 1, b: ""})'});
+    await check(function getA() { return this.a; }, object);
+    await check(function setA(a) { this.a = a; }, object, 1);
+    await check(function getB() { return this.b; }, object);
+    await check(function setB(b) { this.b = b; }, object, "lala");
+    await check(function setSomeGlobal() { globalThis.someGlobal = this; }, object);
+    await check(function localSideEffect() { const date = new Date(); date.setDate(this.a); return date; }, object);
+    await Protocol.Runtime.disable();
+  }
+]);