[debugger] use absolute source positions for break locations.

There is simply no point in converting between relative and absolute source
positions in both C++ and Javascript code.

R=jgruber@chromium.org

Review-Url: https://codereview.chromium.org/2169463002
Cr-Commit-Position: refs/heads/master@{#37916}

src/debug/debug.cc

@@ -86,11 +86,11 @@ BreakLocation::BreakLocation(Handle<DebugInfo> debug_info, DebugBreakType type,
     SharedFunctionInfo* shared = debug_info->shared();
     if (shared->HasSourceCode()) {
       return_position =
-          std::max(shared->end_position() - shared->start_position() - 1, 0);
+          std::max(shared->end_position() - 1, shared->start_position());
     }
     // TODO(yangguo): find out why return position is wrong for liveedit.
     position_ = return_position;
-    statement_position = return_position;
+    statement_position_ = return_position;
   }
 }
 
@@ -104,10 +104,10 @@ BreakLocation::Iterator* BreakLocation::GetIterator(
 }
 
 BreakLocation::Iterator::Iterator(Handle<DebugInfo> debug_info)
-    : debug_info_(debug_info),
-      break_index_(-1),
-      position_(1),
-      statement_position_(1) {}
+    : debug_info_(debug_info), break_index_(-1) {
+  position_ = debug_info->shared()->start_position();
+  statement_position_ = position_;
+}
 
 BreakLocation::CodeIterator::CodeIterator(Handle<DebugInfo> debug_info,
                                           BreakLocatorType type)
@@ -115,8 +115,7 @@ BreakLocation::CodeIterator::CodeIterator(Handle<DebugInfo> debug_info,
       reloc_iterator_(debug_info->abstract_code()->GetCode(),
                       GetModeMask(type)),
       source_position_iterator_(
-          debug_info->abstract_code()->GetCode()->source_position_table()),
-      start_position_(debug_info_->shared()->start_position()) {
+          debug_info->abstract_code()->GetCode()->source_position_table()) {
   // There is at least one break location.
   DCHECK(!Done());
   Next();
@@ -150,7 +149,7 @@ void BreakLocation::CodeIterator::Next() {
   int offset = code_offset();
   while (!source_position_iterator_.done() &&
          source_position_iterator_.code_offset() <= offset) {
-    position_ = source_position_iterator_.source_position() - start_position_;
+    position_ = source_position_iterator_.source_position();
     if (source_position_iterator_.is_statement()) {
       statement_position_ = position_;
     }
@@ -189,8 +188,7 @@ BreakLocation::BytecodeArrayIterator::BytecodeArrayIterator(
       source_position_iterator_(debug_info->abstract_code()
                                     ->GetBytecodeArray()
                                     ->source_position_table()),
-      break_locator_type_(type),
-      start_position_(debug_info->shared()->start_position()) {
+      break_locator_type_(type) {
   // There is at least one break location.
   DCHECK(!Done());
   Next();
@@ -204,7 +202,7 @@ void BreakLocation::BytecodeArrayIterator::Next() {
     if (!first) source_position_iterator_.Advance();
     first = false;
     if (Done()) return;
-    position_ = source_position_iterator_.source_position() - start_position_;
+    position_ = source_position_iterator_.source_position();
     if (source_position_iterator_.is_statement()) {
       statement_position_ = position_;
     }
@@ -806,28 +804,22 @@ bool Debug::SetBreakPointForScript(Handle<Script> script,
 
   // Find position within function. The script position might be before the
   // source position of the first function.
-  int position;
   if (shared->start_position() > *source_position) {
-    position = 0;
-  } else {
-    position = *source_position - shared->start_position();
+    *source_position = shared->start_position();
   }
 
   Handle<DebugInfo> debug_info(shared->GetDebugInfo());
-  // Source positions starts with zero.
-  DCHECK(position >= 0);
 
   // Find the break point and change it.
   BreakLocation location =
-      BreakLocation::FromPosition(debug_info, position, alignment);
+      BreakLocation::FromPosition(debug_info, *source_position, alignment);
   location.SetBreakPoint(break_point_object);
 
   feature_tracker()->Track(DebugFeatureTracker::kBreakPoint);
 
-  position = (alignment == STATEMENT_ALIGNED) ? location.statement_position()
-                                              : location.position();
-
-  *source_position = position + shared->start_position();
+  *source_position = (alignment == STATEMENT_ALIGNED)
+                         ? location.statement_position()
+                         : location.position();
 
   // At least one active break point now.
   DCHECK(debug_info->GetBreakPointCount() > 0);

src/debug/debug.h

@@ -178,7 +178,6 @@ class BreakLocation {
 
     RelocIterator reloc_iterator_;
     SourcePositionTableIterator source_position_iterator_;
-    int start_position_;
     DISALLOW_COPY_AND_ASSIGN(CodeIterator);
   };
 
@@ -200,7 +199,6 @@ class BreakLocation {
 
     SourcePositionTableIterator source_position_iterator_;
     BreakLocatorType break_locator_type_;
-    int start_position_;
     DISALLOW_COPY_AND_ASSIGN(BytecodeArrayIterator);
   };
 

src/debug/debug.js

@@ -472,13 +472,6 @@ Debug.setListener = function(listener, opt_data) {
 };
 
 
-Debug.breakLocations = function(f, opt_position_aligment) {
-  if (!IS_FUNCTION(f)) throw MakeTypeError(kDebuggerType);
-  var position_aligment = IS_UNDEFINED(opt_position_aligment)
-      ? Debug.BreakPositionAlignment.Statement : opt_position_aligment;
-  return %GetBreakLocations(f, position_aligment);
-};
-
 // Returns a Script object. If the parameter is a function the return value
 // is the script in which the function is defined. If the parameter is a string
 // the return value is the script for which the script name has that string
@@ -588,10 +581,9 @@ Debug.setBreakPoint = function(func, opt_line, opt_column, opt_condition) {
   if (%FunctionIsAPIFunction(func)) {
     throw MakeError(kDebugger, 'Cannot set break point in native code.');
   }
-  // Find source position relative to start of the function
-  var break_position =
+  // Find source position.
+  var source_position =
       this.findFunctionSourceLocation(func, opt_line, opt_column).position;
-  var source_position = break_position - this.sourcePosition(func);
   // Find the script for the function.
   var script = %FunctionGetScript(func);
   // Break in builtin JavaScript code is not supported.
@@ -601,8 +593,6 @@ Debug.setBreakPoint = function(func, opt_line, opt_column, opt_condition) {
   // If the script for the function has a name convert this to a script break
   // point.
   if (script && script.id) {
-    // Adjust the source position to be script relative.
-    source_position += %FunctionGetScriptSourcePosition(func);
     // Find line and column for the position in the script and set a script
     // break point from that.
     var location = script.locationFromPosition(source_position, false);
@@ -614,7 +604,6 @@ Debug.setBreakPoint = function(func, opt_line, opt_column, opt_condition) {
     var break_point = MakeBreakPoint(source_position);
     var actual_position =
         %SetFunctionBreakPoint(func, source_position, break_point);
-    actual_position += this.sourcePosition(func);
     var actual_location = script.locationFromPosition(actual_position, true);
     break_point.actual_location = { line: actual_location.line,
                                     column: actual_location.column,
@@ -829,8 +818,10 @@ Debug.isBreakOnUncaughtException = function() {
 Debug.showBreakPoints = function(f, full, opt_position_alignment) {
   if (!IS_FUNCTION(f)) throw MakeError(kDebuggerType);
   var source = full ? this.scriptSource(f) : this.source(f);
-  var offset = full ? this.sourcePosition(f) : 0;
-  var locations = this.breakLocations(f, opt_position_alignment);
+  var offset = full ? 0 : this.sourcePosition(f);
+  var position_alignment = IS_UNDEFINED(opt_position_alignment)
+      ? Debug.BreakPositionAlignment.Statement : opt_position_alignment;
+  var locations = %GetBreakLocations(f, position_alignment);
   if (!locations) return source;
   locations.sort(function(x, y) { return x - y; });
   var result = "";

test/cctest/heap/test-heap.cc

@@ -1539,7 +1539,7 @@ TEST(TestCodeFlushingIncrementalAbort) {
   // Enable the debugger and add a breakpoint while incremental marking
   // is running so that incremental marking aborts and code flushing is
   // disabled.
-  int position = 0;
+  int position = function->shared()->start_position();
   Handle<Object> breakpoint_object(Smi::FromInt(0), isolate);
   EnableDebugger(CcTest::isolate());
   isolate->debug()->SetBreakPoint(function, breakpoint_object, &position);

test/cctest/test-debug.cc

@@ -157,29 +157,23 @@ static bool HasDebugInfo(v8::Local<v8::Function> fun) {
   return shared->HasDebugInfo();
 }
 
-
-// Set a break point in a function and return the associated break point
-// number.
-static int SetBreakPoint(Handle<v8::internal::JSFunction> fun, int position) {
+// Set a break point in a function with a position relative to function start,
+// and return the associated break point number.
+static int SetBreakPoint(v8::Local<v8::Function> fun, int position) {
+  i::Handle<i::JSFunction> function =
+      i::Handle<i::JSFunction>::cast(v8::Utils::OpenHandle(*fun));
+  position += function->shared()->start_position();
   static int break_point = 0;
-  v8::internal::Isolate* isolate = fun->GetIsolate();
+  v8::internal::Isolate* isolate = function->GetIsolate();
   v8::internal::Debug* debug = isolate->debug();
   debug->SetBreakPoint(
-      fun,
+      function,
       Handle<Object>(v8::internal::Smi::FromInt(++break_point), isolate),
       &position);
   return break_point;
 }
 
 
-// Set a break point in a function and return the associated break point
-// number.
-static int SetBreakPoint(v8::Local<v8::Function> fun, int position) {
-  return SetBreakPoint(
-      i::Handle<i::JSFunction>::cast(v8::Utils::OpenHandle(*fun)), position);
-}
-
-
 // Set a break point in a function using the Debug object and return the
 // associated break point number.
 static int SetBreakPointFromJS(v8::Isolate* isolate,
@@ -8039,29 +8033,29 @@ TEST(BreakLocationIterator) {
   TestBreakLocation::Iterator* iterator =
       TestBreakLocation::GetIterator(debug_info, i::ALL_BREAK_LOCATIONS);
   CHECK(iterator->GetBreakLocation().IsDebuggerStatement());
-  CHECK_EQ(7, iterator->GetBreakLocation().position());
+  CHECK_EQ(17, iterator->GetBreakLocation().position());
   iterator->Next();
   CHECK(iterator->GetBreakLocation().IsDebugBreakSlot());
-  CHECK_EQ(22, iterator->GetBreakLocation().position());
+  CHECK_EQ(32, iterator->GetBreakLocation().position());
   iterator->Next();
   CHECK(iterator->GetBreakLocation().IsCall());
-  CHECK_EQ(22, iterator->GetBreakLocation().position());
+  CHECK_EQ(32, iterator->GetBreakLocation().position());
   iterator->Next();
   CHECK(iterator->GetBreakLocation().IsDebuggerStatement());
-  CHECK_EQ(37, iterator->GetBreakLocation().position());
+  CHECK_EQ(47, iterator->GetBreakLocation().position());
   iterator->Next();
   CHECK(iterator->GetBreakLocation().IsReturn());
-  CHECK_EQ(50, iterator->GetBreakLocation().position());
+  CHECK_EQ(60, iterator->GetBreakLocation().position());
   iterator->Next();
   CHECK(iterator->Done());
   delete iterator;
 
   iterator = TestBreakLocation::GetIterator(debug_info, i::CALLS_AND_RETURNS);
   CHECK(iterator->GetBreakLocation().IsCall());
-  CHECK_EQ(22, iterator->GetBreakLocation().position());
+  CHECK_EQ(32, iterator->GetBreakLocation().position());
   iterator->Next();
   CHECK(iterator->GetBreakLocation().IsReturn());
-  CHECK_EQ(50, iterator->GetBreakLocation().position());
+  CHECK_EQ(60, iterator->GetBreakLocation().position());
   iterator->Next();
   CHECK(iterator->Done());
   delete iterator;