Debugger: correctly redirect code with no stack check.

This fix makes the redirect mechanism a bit more stable.

R=mvstanton@chromium.org

Review URL: https://codereview.chromium.org/1269733002

Cr-Commit-Position: refs/heads/master@{#29936}

src/debug.cc

@@ -1311,13 +1311,10 @@ void Debug::ClearStepNext() {
 }
 
 
-// We start counting call sites from the stack check because the declaration
-// code at the start of the function may have changed on recompile.
-static void SkipToStackCheck(Isolate* isolate, RelocIterator* it) {
-  while (Code::GetCodeFromTargetAddress(it->rinfo()->target_address()) !=
-         *isolate->builtins()->StackCheck()) {
-    it->next();
-  }
+bool MatchingCodeTargets(Code* target1, Code* target2) {
+  if (target1 == target2) return true;
+  if (target1->kind() != target2->kind()) return false;
+  return target1->is_handler() || target1->is_inline_cache_stub();
 }
 
 
@@ -1328,24 +1325,42 @@ static Address ComputeNewPcForRedirect(Code* new_code, Code* old_code,
   DCHECK_EQ(old_code->kind(), Code::FUNCTION);
   DCHECK_EQ(new_code->kind(), Code::FUNCTION);
   DCHECK(new_code->has_debug_break_slots());
-  int mask = RelocInfo::kCodeTargetMask;
-  int index = 0;
+  static const int mask = RelocInfo::kCodeTargetMask;
+
+  // Find the target of the current call.
+  Code* target = NULL;
   intptr_t delta = 0;
-  Isolate* isolate = new_code->GetIsolate();
-  RelocIterator old_it(old_code, mask);
-  for (SkipToStackCheck(isolate, &old_it); !old_it.done(); old_it.next()) {
-    RelocInfo* rinfo = old_it.rinfo();
+  for (RelocIterator it(old_code, mask); !it.done(); it.next()) {
+    RelocInfo* rinfo = it.rinfo();
     Address current_pc = rinfo->pc();
     // The frame PC is behind the call instruction by the call instruction size.
     if (current_pc > old_pc) break;
-    index++;
     delta = old_pc - current_pc;
+    target = Code::GetCodeFromTargetAddress(rinfo->target_address());
+  }
+
+  // Count the number of calls to the same target before the current call.
+  int index = 0;
+  for (RelocIterator it(old_code, mask); !it.done(); it.next()) {
+    RelocInfo* rinfo = it.rinfo();
+    Address current_pc = rinfo->pc();
+    if (current_pc > old_pc) break;
+    Code* current = Code::GetCodeFromTargetAddress(rinfo->target_address());
+    if (MatchingCodeTargets(target, current)) index++;
+  }
+
+  DCHECK(index > 0);
+
+  // Repeat the count on the new code to find corresponding call.
+  for (RelocIterator it(new_code, mask); !it.done(); it.next()) {
+    RelocInfo* rinfo = it.rinfo();
+    Code* current = Code::GetCodeFromTargetAddress(rinfo->target_address());
+    if (MatchingCodeTargets(target, current)) index--;
+    if (index == 0) return rinfo->pc() + delta;
   }
 
-  RelocIterator new_it(new_code, mask);
-  SkipToStackCheck(isolate, &new_it);
-  for (int i = 1; i < index; i++) new_it.next();
-  return new_it.rinfo()->pc() + delta;
+  UNREACHABLE();
+  return NULL;
 }
 
 

src/full-codegen/arm/full-codegen-arm.cc

@@ -348,6 +348,11 @@ void FullCodeGenerator::Generate() {
       VisitDeclarations(scope()->declarations());
     }
 
+    // Assert that the declarations do not use ICs. Otherwise the debugger
+    // won't be able to redirect a PC at an IC to the correct IC in newly
+    // recompiled code.
+    DCHECK_EQ(0, ic_total_count_);
+
     { Comment cmnt(masm_, "[ Stack check");
       PrepareForBailoutForId(BailoutId::Declarations(), NO_REGISTERS);
       Label ok;

src/full-codegen/arm64/full-codegen-arm64.cc

@@ -353,6 +353,11 @@ void FullCodeGenerator::Generate() {
       VisitDeclarations(scope()->declarations());
     }
 
+    // Assert that the declarations do not use ICs. Otherwise the debugger
+    // won't be able to redirect a PC at an IC to the correct IC in newly
+    // recompiled code.
+    DCHECK_EQ(0, ic_total_count_);
+
     {
       Comment cmnt(masm_, "[ Stack check");
       PrepareForBailoutForId(BailoutId::Declarations(), NO_REGISTERS);

src/full-codegen/ia32/full-codegen-ia32.cc

@@ -350,6 +350,11 @@ void FullCodeGenerator::Generate() {
       VisitDeclarations(scope()->declarations());
     }
 
+    // Assert that the declarations do not use ICs. Otherwise the debugger
+    // won't be able to redirect a PC at an IC to the correct IC in newly
+    // recompiled code.
+    DCHECK_EQ(0, ic_total_count_);
+
     { Comment cmnt(masm_, "[ Stack check");
       PrepareForBailoutForId(BailoutId::Declarations(), NO_REGISTERS);
       Label ok;

src/full-codegen/mips/full-codegen-mips.cc

@@ -366,6 +366,11 @@ void FullCodeGenerator::Generate() {
       VisitDeclarations(scope()->declarations());
     }
 
+    // Assert that the declarations do not use ICs. Otherwise the debugger
+    // won't be able to redirect a PC at an IC to the correct IC in newly
+    // recompiled code.
+    DCHECK_EQ(0, ic_total_count_);
+
     { Comment cmnt(masm_, "[ Stack check");
       PrepareForBailoutForId(BailoutId::Declarations(), NO_REGISTERS);
       Label ok;

src/full-codegen/mips64/full-codegen-mips64.cc

@@ -360,6 +360,12 @@ void FullCodeGenerator::Generate() {
     { Comment cmnt(masm_, "[ Declarations");
       VisitDeclarations(scope()->declarations());
     }
+
+    // Assert that the declarations do not use ICs. Otherwise the debugger
+    // won't be able to redirect a PC at an IC to the correct IC in newly
+    // recompiled code.
+    DCHECK_EQ(0, ic_total_count_);
+
     { Comment cmnt(masm_, "[ Stack check");
       PrepareForBailoutForId(BailoutId::Declarations(), NO_REGISTERS);
       Label ok;

src/full-codegen/ppc/full-codegen-ppc.cc

@@ -360,6 +360,11 @@ void FullCodeGenerator::Generate() {
       VisitDeclarations(scope()->declarations());
     }
 
+    // Assert that the declarations do not use ICs. Otherwise the debugger
+    // won't be able to redirect a PC at an IC to the correct IC in newly
+    // recompiled code.
+    DCHECK_EQ(0, ic_total_count_);
+
     {
       Comment cmnt(masm_, "[ Stack check");
       PrepareForBailoutForId(BailoutId::Declarations(), NO_REGISTERS);

src/full-codegen/x64/full-codegen-x64.cc

@@ -348,6 +348,11 @@ void FullCodeGenerator::Generate() {
       VisitDeclarations(scope()->declarations());
     }
 
+    // Assert that the declarations do not use ICs. Otherwise the debugger
+    // won't be able to redirect a PC at an IC to the correct IC in newly
+    // recompiled code.
+    DCHECK_EQ(0, ic_total_count_);
+
     { Comment cmnt(masm_, "[ Stack check");
       PrepareForBailoutForId(BailoutId::Declarations(), NO_REGISTERS);
        Label ok;

src/full-codegen/x87/full-codegen-x87.cc

@@ -347,6 +347,11 @@ void FullCodeGenerator::Generate() {
       VisitDeclarations(scope()->declarations());
     }
 
+    // Assert that the declarations do not use ICs. Otherwise the debugger
+    // won't be able to redirect a PC at an IC to the correct IC in newly
+    // recompiled code.
+    DCHECK_EQ(0, ic_total_count_);
+
     { Comment cmnt(masm_, "[ Stack check");
       PrepareForBailoutForId(BailoutId::Declarations(), NO_REGISTERS);
       Label ok;

test/mjsunit/regress/regress-debugger-redirect.js

+// Copyright 2015 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: --expose-debug-as debug
+
+function f(x) {
+  // This function compiles into code that only throws a redeclaration
+  // error. It contains no stack check and has no function body.
+  const x = 0;
+  return x;
+}
+
+function g() {
+  f(0);
+}
+
+var exception = null;
+var called = false;
+var Debug = debug.Debug;
+Debug.setBreakOnException();
+
+function listener(event, exec_state, event_data, data) {
+  if (event != Debug.DebugEvent.Exception) return;
+  try {
+    called = true;
+    Debug.setBreakPoint(f, 1);
+  } catch (e) {
+    exception = e;
+  }
+}
+
+Debug.setListener(listener);
+
+assertThrows(g);
+assertNull(exception);
+assertTrue(called);