Breakpoint position should be inside function body.

Review URL: http://codereview.chromium.org/2883042

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@5145 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/arm/full-codegen-arm.cc b/src/arm/full-codegen-arm.cc
index 7d90ed9..1d26aba 100644 (file)
--- a/src/arm/full-codegen-arm.cc
+++ b/src/arm/full-codegen-arm.cc
@@ -224,7 +224,7 @@ void FullCodeGenerator::EmitReturnSequence() {
       // Here we use masm_-> instead of the __ macro to avoid the code coverage
       // tool from instrumenting as we rely on the code size here.
       int32_t sp_delta = (scope()->num_parameters() + 1) * kPointerSize;
-      CodeGenerator::RecordPositions(masm_, function()->end_position());
+      CodeGenerator::RecordPositions(masm_, function()->end_position() - 1);
       __ RecordJSReturn();
       masm_->mov(sp, fp);
       masm_->ldm(ia_w, sp, fp.bit() | lr.bit());

diff --git a/src/codegen.cc b/src/codegen.cc
index 84b73a4..444698c 100644 (file)
--- a/src/codegen.cc
+++ b/src/codegen.cc
@@ -433,7 +433,7 @@ void CodeGenerator::CodeForFunctionPosition(FunctionLiteral* fun) {
 
 
 void CodeGenerator::CodeForReturnPosition(FunctionLiteral* fun) {
-  if (FLAG_debug_info) RecordPositions(masm(), fun->end_position(), false);
+  if (FLAG_debug_info) RecordPositions(masm(), fun->end_position() - 1, false);
 }
 
 

diff --git a/src/debug.cc b/src/debug.cc
index 7288135..0957cbc 100644 (file)
--- a/src/debug.cc
+++ b/src/debug.cc
@@ -170,7 +170,7 @@ void BreakLocationIterator::Next() {
       // Set the positions to the end of the function.
       if (debug_info_->shared()->HasSourceCode()) {
         position_ = debug_info_->shared()->end_position() -
-                    debug_info_->shared()->start_position();
+                    debug_info_->shared()->start_position() - 1;
       } else {
         position_ = 0;
       }

diff --git a/src/full-codegen.cc b/src/full-codegen.cc
index 8a8b39b..aaf2f99 100644 (file)
--- a/src/full-codegen.cc
+++ b/src/full-codegen.cc
@@ -770,7 +770,7 @@ void FullCodeGenerator::SetFunctionPosition(FunctionLiteral* fun) {
 
 void FullCodeGenerator::SetReturnPosition(FunctionLiteral* fun) {
   if (FLAG_debug_info) {
-    CodeGenerator::RecordPositions(masm_, fun->end_position());
+    CodeGenerator::RecordPositions(masm_, fun->end_position() - 1);
   }
 }
 

diff --git a/src/ia32/full-codegen-ia32.cc b/src/ia32/full-codegen-ia32.cc
index b2ff1fd..8f20108 100644 (file)
--- a/src/ia32/full-codegen-ia32.cc
+++ b/src/ia32/full-codegen-ia32.cc
@@ -207,7 +207,7 @@ void FullCodeGenerator::EmitReturnSequence() {
     Label check_exit_codesize;
     masm_->bind(&check_exit_codesize);
 #endif
-    CodeGenerator::RecordPositions(masm_, function()->end_position());
+    CodeGenerator::RecordPositions(masm_, function()->end_position() - 1);
     __ RecordJSReturn();
     // Do not use the leave instruction here because it is too short to
     // patch with the code required by the debugger.

diff --git a/src/runtime.cc b/src/runtime.cc
index 4063a0f..4b14643 100644 (file)
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -9387,13 +9387,6 @@ static Object* Runtime_SetScriptBreakPoint(Arguments args) {
     }
     Debug::SetBreakPoint(shared, break_point_object_arg, &position);
     position += shared->start_position();
-
-    // The result position may become beyond script source end.
-    // This is expected when the function is toplevel. This may become
-    // a problem later when actual position gets converted into line/column.
-    if (shared->is_toplevel() && position == shared->end_position()) {
-      position = shared->end_position() - 1;
-    }
     return Smi::FromInt(position);
   }
   return  Heap::undefined_value();

diff --git a/src/x64/full-codegen-x64.cc b/src/x64/full-codegen-x64.cc
index 0b3b7c4..a8626b3 100644 (file)
--- a/src/x64/full-codegen-x64.cc
+++ b/src/x64/full-codegen-x64.cc
@@ -207,7 +207,7 @@ void FullCodeGenerator::EmitReturnSequence() {
     Label check_exit_codesize;
     masm_->bind(&check_exit_codesize);
 #endif
-    CodeGenerator::RecordPositions(masm_, function()->end_position());
+    CodeGenerator::RecordPositions(masm_, function()->end_position() - 1);
     __ RecordJSReturn();
     // Do not use the leave instruction here because it is too short to
     // patch with the code required by the debugger.

diff --git a/test/cctest/test-debug.cc b/test/cctest/test-debug.cc
index 6a94bed..5f8b826 100644 (file)
--- a/test/cctest/test-debug.cc
+++ b/test/cctest/test-debug.cc
@@ -1175,11 +1175,11 @@ TEST(BreakPointReturn) {
   foo->Call(env->Global(), 0, NULL);
   CHECK_EQ(1, break_point_hit_count);
   CHECK_EQ(0, last_source_line);
-  CHECK_EQ(16, last_source_column);
+  CHECK_EQ(15, last_source_column);
   foo->Call(env->Global(), 0, NULL);
   CHECK_EQ(2, break_point_hit_count);
   CHECK_EQ(0, last_source_line);
-  CHECK_EQ(16, last_source_column);
+  CHECK_EQ(15, last_source_column);
 
   // Run without breakpoints.
   ClearBreakPoint(bp);

diff --git a/test/mjsunit/debug-breakpoints.js b/test/mjsunit/debug-breakpoints.js
index c332f1e..0bc349c 100644 (file)
--- a/test/mjsunit/debug-breakpoints.js
+++ b/test/mjsunit/debug-breakpoints.js
@@ -43,12 +43,12 @@ bp1 = Debug.setBreakPoint(f, 0, 8);
 assertEquals("() {a=1;[B0]b=2}", Debug.showBreakPoints(f));
 bp2 = Debug.setBreakPoint(f, 0, 4);
 assertEquals("() {[B0]a=1;[B1]b=2}", Debug.showBreakPoints(f));
-bp3 = Debug.setBreakPoint(f, 0, 12);
-assertEquals("() {[B0]a=1;[B1]b=2}[B2]", Debug.showBreakPoints(f));
+bp3 = Debug.setBreakPoint(f, 0, 11);
+assertEquals("() {[B0]a=1;[B1]b=2[B2]}", Debug.showBreakPoints(f));
 Debug.clearBreakPoint(bp1);
-assertEquals("() {[B0]a=1;b=2}[B1]", Debug.showBreakPoints(f));
+assertEquals("() {[B0]a=1;b=2[B1]}", Debug.showBreakPoints(f));
 Debug.clearBreakPoint(bp2);
-assertEquals("() {a=1;b=2}[B0]", Debug.showBreakPoints(f));
+assertEquals("() {a=1;b=2[B0]}", Debug.showBreakPoints(f));
 Debug.clearBreakPoint(bp3);
 assertEquals("() {a=1;b=2}", Debug.showBreakPoints(f));
 
@@ -96,21 +96,21 @@ bp3 = Debug.setBreakPoint(g, 3, 0);
 // }[B2]
 assertTrue(Debug.showBreakPoints(g).indexOf("[B0]a=1;") > 0);
 assertTrue(Debug.showBreakPoints(g).indexOf("[B1]b=2;") > 0);
-assertTrue(Debug.showBreakPoints(g).indexOf("}[B2]") > 0);
+assertTrue(Debug.showBreakPoints(g).indexOf("[B2]}") > 0);
 Debug.clearBreakPoint(bp1);
 // function g() {
 //   [B0]a=1;
 //   b=2;
 // }[B1]
 assertTrue(Debug.showBreakPoints(g).indexOf("[B0]a=1;") > 0);
-assertTrue(Debug.showBreakPoints(g).indexOf("}[B1]") > 0);
+assertTrue(Debug.showBreakPoints(g).indexOf("[B1]}") > 0);
 assertTrue(Debug.showBreakPoints(g).indexOf("[B2]") < 0);
 Debug.clearBreakPoint(bp2);
 // function g() {
 //   a=1;
 //   b=2;
 // }[B0]
-assertTrue(Debug.showBreakPoints(g).indexOf("}[B0]") > 0);
+assertTrue(Debug.showBreakPoints(g).indexOf("[B0]}") > 0);
 assertTrue(Debug.showBreakPoints(g).indexOf("[B1]") < 0);
 Debug.clearBreakPoint(bp3);
 // function g() {

diff --git a/test/mjsunit/debug-conditional-breakpoints.js b/test/mjsunit/debug-conditional-breakpoints.js
index bd4cdd1..6248437 100644 (file)
--- a/test/mjsunit/debug-conditional-breakpoints.js
+++ b/test/mjsunit/debug-conditional-breakpoints.js
@@ -136,7 +136,7 @@ Debug.clearBreakPoint(bp);
 
 // Conditional breakpoint which checks a local variable.
 break_point_hit_count = 0;
-bp = Debug.setBreakPoint(h, 0, 23, 'a % 2 == 0');
+bp = Debug.setBreakPoint(h, 0, 22, 'a % 2 == 0');
 for (var i = 0; i < 10; i++) {
   g();
 }
@@ -146,8 +146,8 @@ Debug.clearBreakPoint(bp);
 
 // Multiple conditional breakpoint which the same condition.
 break_point_hit_count = 0;
-bp1 = Debug.setBreakPoint(h, 0, 23, 'a % 2 == 0');
-bp2 = Debug.setBreakPoint(h, 0, 23, 'a % 2 == 0');
+bp1 = Debug.setBreakPoint(h, 0, 22, 'a % 2 == 0');
+bp2 = Debug.setBreakPoint(h, 0, 22, 'a % 2 == 0');
 for (var i = 0; i < 10; i++) {
   g();
 }
@@ -159,8 +159,8 @@ Debug.clearBreakPoint(bp2);
 
 // Multiple conditional breakpoint which different conditions.
 break_point_hit_count = 0;
-bp1 = Debug.setBreakPoint(h, 0, 23, 'a % 2 == 0');
-bp2 = Debug.setBreakPoint(h, 0, 23, '(a + 1) % 2 == 0');
+bp1 = Debug.setBreakPoint(h, 0, 22, 'a % 2 == 0');
+bp2 = Debug.setBreakPoint(h, 0, 22, '(a + 1) % 2 == 0');
 for (var i = 0; i < 10; i++) {
   g();
 }

diff --git a/test/mjsunit/debug-return-value.js b/test/mjsunit/debug-return-value.js
index a9ac520..3982ea9 100644 (file)
--- a/test/mjsunit/debug-return-value.js
+++ b/test/mjsunit/debug-return-value.js
@@ -101,7 +101,7 @@ function listener(event, exec_state, event_data, data) {
         exec_state.prepareStep(Debug.StepAction.StepIn, 1);
       } else {
         // Position at the end of the function.
-        assertEquals(debugger_source_position + 51,
+        assertEquals(debugger_source_position + 50,
         exec_state.frame(0).sourcePosition());
         
         // Just about to return from the function.