[debug] correctly tier down function for side effect check mode

hashseed · V8 LUCI CQ · commit dc49fe064725 · 2021-07-28T15:03:26.000Z
Previously we do not tier down from baseline to interpreter, which breaks per-bytecode side effect checks (to check whether e.g. we are mutating a temporary object, which is not considered a side effect). R=leszeks@chromium.org Bug: chromium:1233401 Change-Id: Ie08b5352aa4c124421b4c9abce18326938bbc822 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3056981 Reviewed-by: Leszek Swirski <leszeks@chromium.org> Commit-Queue: Yang Guo <yangguo@chromium.org> Cr-Commit-Position: refs/heads/master@{#75963}
diff --git a/src/runtime/runtime-debug.cc b/src/runtime/runtime-debug.cc
@@ -685,7 +685,8 @@ RUNTIME_FUNCTION(Runtime_DebugOnFunctionCall) {
   CONVERT_ARG_HANDLE_CHECKED(Object, receiver, 1);
   if (isolate->debug()->needs_check_on_function_call()) {
     // Ensure that the callee will perform debug check on function call too.
-    Deoptimizer::DeoptimizeFunction(*fun);
+    Handle<SharedFunctionInfo> shared(fun->shared(), isolate);
+    isolate->debug()->DeoptimizeFunction(shared);
     if (isolate->debug()->last_step_action() >= StepInto ||
         isolate->debug()->break_on_next_function_call()) {
       DCHECK_EQ(isolate->debug_execution_mode(), DebugInfo::kBreakpoints);
diff --git a/test/inspector/regress/regress-crbug-1233401-expected.txt b/test/inspector/regress/regress-crbug-1233401-expected.txt
@@ -0,0 +1,73 @@
+Execute it separately
+{
+    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
+        }
+    }
+}
+{
+    id : <messageId>
+    result : {
+        result : {
+            description : 1
+            type : number
+            value : 1
+        }
+    }
+}
+{
+    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
+        }
+    }
+}
+{
+    id : <messageId>
+    result : {
+        result : {
+            description : 2
+            type : number
+            value : 2
+        }
+    }
+}
diff --git a/test/inspector/regress/regress-crbug-1233401.js b/test/inspector/regress/regress-crbug-1233401.js
@@ -0,0 +1,54 @@
+// 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.
+
+// Flags: --sparkplug
+// Flags: --no-baseline-batch-compilation
+
+const baseScript = `
+var paths = [
+  [ 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 1],
+  [ 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 1],
+  [ 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 0]
+];
+
+function sink(arr) {
+  let i = arr.length - 2;
+  for (let j = 0; j < arr[i].length; j++) {
+    const node = arr[i][j];
+    const right = arr[i + 1][j + 1];
+    arr[i][j] = Math.max(node + right);
+  }
+  arr.pop();
+  return arr[i][arr[i].length-1];
+}
+`;
+
+let {session, contextGroup, Protocol} = InspectorTest.start(
+  'Evaluate with and without side effect checks');
+
+(async function test() {
+  await Protocol.Runtime.evaluate({
+    expression: baseScript,
+    replMode: true,
+  });
+  InspectorTest.logMessage(await Protocol.Runtime.evaluate({
+    expression: 'sink(paths);',
+    replMode: true,
+    throwOnSideEffect: true,
+  }));
+  InspectorTest.logMessage(await Protocol.Runtime.evaluate({
+    expression: 'sink(paths)',
+    replMode: true,
+  }));
+  InspectorTest.logMessage(await Protocol.Runtime.evaluate({
+    expression: 'sink(paths);',
+    replMode: true,
+    throwOnSideEffect: true,
+  }));
+  InspectorTest.logMessage(await Protocol.Runtime.evaluate({
+    expression: 'sink(paths);',
+    replMode: true,
+  }));
+  InspectorTest.completeTest();
+})();
