Fix a bug in the handling of debug break in CallIC
authorsgjesse@chromium.org <sgjesse@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Wed, 25 Aug 2010 05:57:02 +0000 (05:57 +0000)
committersgjesse@chromium.org <sgjesse@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Wed, 25 Aug 2010 05:57:02 +0000 (05:57 +0000)
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
src/ia32/debug-ia32.cc
src/x64/debug-x64.cc
test/cctest/test-debug.cc

index 3a94845..5c1c149 100644 (file)
@@ -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());
 }
 
 
index b57cf3d..3d3f511 100644 (file)
@@ -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);
 }
 
 
index d5b7e77..fb4429d 100644 (file)
@@ -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);
 }
 
 
index 1e93bf5..3e53b74 100644 (file)
@@ -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;