From d7fa6b7a21973c2a6a7248a580ac0d8ec4a54a2d Mon Sep 17 00:00:00 2001
From: "sgjesse@chromium.org"
 <sgjesse@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Date: Wed, 25 Aug 2010 05:57:02 +0000
Subject: [PATCH] Fix a bug in the handling of debug break in CallIC

The change of calling convention in the CallIC was not reflected in the debug break code. Without the change to the debug break code the added test crashed.
Review URL: http://codereview.chromium.org/3167037

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@5333 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
---
 src/arm/debug-arm.cc      | 11 ++---------
 src/ia32/debug-ia32.cc    |  7 ++-----
 src/x64/debug-x64.cc      |  7 +++----
 test/cctest/test-debug.cc | 39 ++++++++++++++++++++++++++++++++++++---
 4 files changed, 43 insertions(+), 21 deletions(-)

diff --git a/src/arm/debug-arm.cc b/src/arm/debug-arm.cc
index 3a94845..5c1c149 100644
--- a/src/arm/debug-arm.cc
+++ b/src/arm/debug-arm.cc
@@ -225,16 +225,9 @@ void Debug::GenerateKeyedStoreICDebugBreak(MacroAssembler* masm) {
 void Debug::GenerateCallICDebugBreak(MacroAssembler* masm) {
   // Calling convention for IC call (from ic-arm.cc)
   // ----------- S t a t e -------------
-  //  -- r0: number of arguments
-  //  -- r1: receiver
-  //  -- lr: return address
+  //  -- r2: name
   // -----------------------------------
-  // Register r1 contains an object that needs to be pushed on the expression
-  // stack of the fake JS frame. r0 is the actual number of arguments not
-  // encoded as a smi, therefore it cannot be on the expression stack of the
-  // fake JS frame as it can easily be an invalid pointer (e.g. 1). r0 will be
-  // pushed on the stack of the C frame and restored from there.
-  Generate_DebugBreakCallHelper(masm, r1.bit());
+  Generate_DebugBreakCallHelper(masm, r2.bit());
 }
 
 
diff --git a/src/ia32/debug-ia32.cc b/src/ia32/debug-ia32.cc
index b57cf3d..3d3f511 100644
--- a/src/ia32/debug-ia32.cc
+++ b/src/ia32/debug-ia32.cc
@@ -183,8 +183,6 @@ void Debug::GenerateKeyedStoreICDebugBreak(MacroAssembler* masm) {
   //  -- ecx    : key
   //  -- edx    : receiver
   // -----------------------------------
-  // Register eax contains an object that needs to be pushed on the
-  // expression stack of the fake JS frame.
   Generate_DebugBreakCallHelper(masm, eax.bit() | ecx.bit() | edx.bit(), false);
 }
 
@@ -192,10 +190,9 @@ void Debug::GenerateKeyedStoreICDebugBreak(MacroAssembler* masm) {
 void Debug::GenerateCallICDebugBreak(MacroAssembler* masm) {
   // Register state for keyed IC call call (from ic-ia32.cc)
   // ----------- S t a t e -------------
-  //  -- eax: number of arguments
+  //  -- ecx: name
   // -----------------------------------
-  // The number of arguments in eax is not smi encoded.
-  Generate_DebugBreakCallHelper(masm, 0, false);
+  Generate_DebugBreakCallHelper(masm, ecx.bit(), false);
 }
 
 
diff --git a/src/x64/debug-x64.cc b/src/x64/debug-x64.cc
index d5b7e77..fb4429d 100644
--- a/src/x64/debug-x64.cc
+++ b/src/x64/debug-x64.cc
@@ -100,12 +100,11 @@ static void Generate_DebugBreakCallHelper(MacroAssembler* masm,
 
 
 void Debug::GenerateCallICDebugBreak(MacroAssembler* masm) {
-  // Register state for keyed IC call call (from ic-x64.cc)
+  // Register state for IC call call (from ic-x64.cc)
   // ----------- S t a t e -------------
-  //  -- rax: number of arguments
+  //  -- rcx: function name
   // -----------------------------------
-  // The number of arguments in rax is not smi encoded.
-  Generate_DebugBreakCallHelper(masm, 0, false);
+  Generate_DebugBreakCallHelper(masm, rcx.bit(), false);
 }
 
 
diff --git a/test/cctest/test-debug.cc b/test/cctest/test-debug.cc
index 1e93bf5..3e53b74 100644
--- a/test/cctest/test-debug.cc
+++ b/test/cctest/test-debug.cc
@@ -869,8 +869,8 @@ static void DebugEventBreakPointCollectGarbage(
       // Scavenge.
       Heap::CollectGarbage(0, v8::internal::NEW_SPACE);
     } else {
-      // Mark sweep (and perhaps compact).
-      Heap::CollectAllGarbage(false);
+      // Mark sweep compact.
+      Heap::CollectAllGarbage(true);
     }
   }
 }
@@ -1127,7 +1127,7 @@ TEST(BreakPointICCall) {
   foo->Call(env->Global(), 0, NULL);
   CHECK_EQ(0, break_point_hit_count);
 
-  // Run with breakpoint
+  // Run with breakpoint.
   int bp = SetBreakPoint(foo, 0);
   foo->Call(env->Global(), 0, NULL);
   CHECK_EQ(1, break_point_hit_count);
@@ -1144,6 +1144,39 @@ TEST(BreakPointICCall) {
 }
 
 
+// Test that a break point can be set at an IC call location and survive a GC.
+TEST(BreakPointICCallWithGC) {
+  break_point_hit_count = 0;
+  v8::HandleScope scope;
+  DebugLocalContext env;
+  v8::Debug::SetDebugEventListener(DebugEventBreakPointCollectGarbage,
+                                   v8::Undefined());
+  v8::Script::Compile(v8::String::New("function bar(){return 1;}"))->Run();
+  v8::Script::Compile(v8::String::New("function foo(){return bar();}"))->Run();
+  v8::Local<v8::Function> foo =
+      v8::Local<v8::Function>::Cast(env->Global()->Get(v8::String::New("foo")));
+
+  // Run without breakpoints.
+  CHECK_EQ(1, foo->Call(env->Global(), 0, NULL)->Int32Value());
+  CHECK_EQ(0, break_point_hit_count);
+
+  // Run with breakpoint.
+  int bp = SetBreakPoint(foo, 0);
+  CHECK_EQ(1, foo->Call(env->Global(), 0, NULL)->Int32Value());
+  CHECK_EQ(1, break_point_hit_count);
+  CHECK_EQ(1, foo->Call(env->Global(), 0, NULL)->Int32Value());
+  CHECK_EQ(2, break_point_hit_count);
+
+  // Run without breakpoints.
+  ClearBreakPoint(bp);
+  foo->Call(env->Global(), 0, NULL);
+  CHECK_EQ(2, break_point_hit_count);
+
+  v8::Debug::SetDebugEventListener(NULL);
+  CheckDebuggerUnloaded();
+}
+
+
 // Test that a break point can be set at a return store location.
 TEST(BreakPointReturn) {
   break_point_hit_count = 0;
-- 
2.7.4