[debug] do not leak optimized code into no-side-effect evaluate.

R=bmeurer@chromium.org

Bug: v8:7421
Change-Id: Iacdd8d294c02b7feb72e3a0bb397930e91197ae7
Reviewed-on: https://chromium-review.googlesource.com/926124
Commit-Queue: Yang Guo <yangguo@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51391}

src/compiler.cc

@@ -609,6 +609,11 @@ MaybeHandle<Code> GetOptimizedCode(Handle<JSFunction> function,
     function->ClearOptimizationMarker();
   }
 
+  if (isolate->debug()->needs_check_on_function_call()) {
+    // Do not optimize when debugger needs to hook into every call.
+    return MaybeHandle<Code>();
+  }
+
   Handle<Code> cached_code;
   if (GetCodeFromOptimizedCodeCache(function, osr_offset)
           .ToHandle(&cached_code)) {

src/debug/debug-evaluate.cc

@@ -358,7 +358,12 @@ bool IntrinsicHasNoSideEffect(Runtime::FunctionId id) {
   V(NewObject)                           \
   V(CompleteInobjectSlackTrackingForMap) \
   V(HasInPrototypeChain)                 \
-  V(StringMaxLength)
+  V(StringMaxLength)                     \
+  /* Test */                             \
+  V(OptimizeOsr)                         \
+  V(OptimizeFunctionOnNextCall)          \
+  V(UnblockConcurrentRecompilation)      \
+  V(GetOptimizationStatus)
 
 #define CASE(Name)       \
   case Runtime::k##Name: \

src/debug/debug.cc

@@ -2230,7 +2230,6 @@ bool Debug::PerformSideEffectCheck(Handle<JSFunction> function) {
       !Compiler::Compile(function, Compiler::KEEP_EXCEPTION)) {
     return false;
   }
-  Deoptimizer::DeoptimizeFunction(*function);
   if (!SharedFunctionInfo::HasNoSideEffect(handle(function->shared()))) {
     if (FLAG_trace_side_effect_free_debug_evaluate) {
       PrintF("[debug-evaluate] Function %s failed side effect check.\n",

src/debug/debug.h

@@ -358,6 +358,10 @@ class Debug {
   inline bool in_debug_scope() const {
     return !!base::Relaxed_Load(&thread_local_.current_debug_scope_);
   }
+  inline bool needs_check_on_function_call() const {
+    return hook_on_function_call_;
+  }
+
   void set_break_points_active(bool v) { break_points_active_ = v; }
   bool break_points_active() const { return break_points_active_; }
 

src/runtime/runtime-debug.cc

@@ -1824,17 +1824,21 @@ RUNTIME_FUNCTION(Runtime_DebugOnFunctionCall) {
   HandleScope scope(isolate);
   DCHECK_EQ(1, args.length());
   CONVERT_ARG_HANDLE_CHECKED(JSFunction, fun, 0);
-  if (isolate->debug()->last_step_action() >= StepIn) {
-    isolate->debug()->PrepareStepIn(fun);
+  if (isolate->debug()->needs_check_on_function_call()) {
+    // Ensure that the callee will perform debug check on function call too.
+    Deoptimizer::DeoptimizeFunction(*fun);
+    if (isolate->debug()->last_step_action() >= StepIn) {
+      isolate->debug()->PrepareStepIn(fun);
+    }
+    if (isolate->needs_side_effect_check() &&
+        !isolate->debug()->PerformSideEffectCheck(fun)) {
+      return isolate->heap()->exception();
+    }
   }
   if (fun->shared()->HasDebugInfo() &&
       fun->shared()->GetDebugInfo()->BreakAtEntry()) {
     isolate->debug()->Break(nullptr, fun);
   }
-  if (isolate->needs_side_effect_check() &&
-      !isolate->debug()->PerformSideEffectCheck(fun)) {
-    return isolate->heap()->exception();
-  }
   return isolate->heap()->undefined_value();
 }
 

test/debugger/regress/regress-7421.js

@@ -0,0 +1,78 @@
+// Copyright 2018 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: --block-concurrent-recompilation
+
+Debug = debug.Debug
+
+// Test that the side-effect check is not bypassed in optimized code.
+
+var exception = null;
+var counter = 0;
+
+function f1() {
+  counter++;
+}
+
+function wrapper1() {
+  for (var i = 0; i < 4; i++) {
+    // Get this function optimized before calling to increment.
+    // Check that that call performs the necessary side-effect checks.
+    %OptimizeOsr();
+  }
+  f1();
+}
+
+function f2() {
+  counter++;
+}
+
+function wrapper2(call) {
+  if (call) f2();
+}
+
+function listener(event, exec_state, event_data, data) {
+  if (event != Debug.DebugEvent.Break) return;
+  try {
+    function success(expectation, source) {
+      assertEquals(expectation,
+                   exec_state.frame(0).evaluate(source, true).value());
+    }
+    function fail(source) {
+      assertThrows(() => exec_state.frame(0).evaluate(source, true),
+                   EvalError);
+    }
+    wrapper1();
+    wrapper1();
+    fail("wrapper1()");
+
+    wrapper2(true);
+    wrapper2(false);
+    wrapper2(true);
+    %OptimizeFunctionOnNextCall(wrapper2);
+    wrapper2(false);
+    fail("wrapper2(true)");
+    fail("%OptimizeFunctionOnNextCall(wrapper2); wrapper2(true)");
+
+    %OptimizeFunctionOnNextCall(wrapper2, "concurrent");
+    wrapper2(false);
+    fail("%UnblockConcurrentRecompilation();" +
+         "%GetOptimizationStatus(wrapper2, 'sync');" +
+         "wrapper2(true);");
+  } catch (e) {
+    exception = e;
+    print(e, e.stack);
+  }
+};
+
+// Add the debug event listener.
+Debug.setListener(listener);
+
+function f() {
+  debugger;
+};
+
+f();
+
+assertNull(exception);

test/inspector/debugger/step-into-optimized-blackbox-expected.txt

@@ -0,0 +1,9 @@
+Tests stepping with blackboxing and inlining
+Paused in
+(...):1
+Paused in
+(...):1
+Paused in
+foo:2
+bar:2
+(...):1

test/inspector/debugger/step-into-optimized-blackbox.js

@@ -0,0 +1,46 @@
+// Copyright 2018 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: --allow-natives-syntax
+
+let {session, contextGroup, Protocol} =
+    InspectorTest.start('Tests stepping with blackboxing and inlining');
+
+contextGroup.addScript(
+`function bar() {
+  return 1 + foo();
+}
+//# sourceURL=bar.js`);
+
+contextGroup.addScript(
+`function foo() {
+  return "foo";
+}
+//# sourceURL=foo.js`);
+
+Protocol.Debugger.enable();
+Protocol.Debugger.setBlackboxPatterns({ patterns: [ "bar.js" ] });
+
+Protocol.Debugger.onPaused(PerformSteps);
+Protocol.Runtime.evaluate({
+  "expression": "bar(); bar(); %OptimizeFunctionOnNextCall(bar); bar()"
+});
+Protocol.Runtime.evaluate({ "expression": "debugger; bar();" });
+
+var commands = [ "stepInto", "stepInto" ];
+
+function PerformSteps(message) {
+  InspectorTest.log("Paused in");
+  var callFrames = message.params.callFrames;
+  for (var callFrame of callFrames) {
+    InspectorTest.log(
+        (callFrame.functionName || "(...)") + ":" + (callFrame.location.lineNumber + 1));
+  }
+  var command = commands.shift();
+  if (!command) {
+    InspectorTest.completeTest();
+    return;
+  }
+  Protocol.Debugger[command]();
+}