[ignition, debugger] correctly set position for return with elided bytecode.

src/interpreter/source-position-table.cc

Index: src/interpreter/source-position-table.cc
diff --git a/src/interpreter/source-position-table.cc b/src/interpreter/source-position-table.cc
index 5d30f51be7ff032111dd893d2e27f3463a1a0956..66f2e741b4cb95264ff1b6e78e50d6cd40e3e2d3 100644
--- a/src/interpreter/source-position-table.cc
+++ b/src/interpreter/source-position-table.cc
@@ -115,10 +115,11 @@ void DecodeEntry(ByteArray* bytes, int* index, PositionTableEntry* entry) {
 
 }  // namespace
 
-void SourcePositionTableBuilder::AddStatementPosition(size_t bytecode_offset,
-                                                      int source_position) {
+void SourcePositionTableBuilder::AddStatementPosition(
+    size_t bytecode_offset, int source_position,
+    SourcePositionTableBuilder::OnDuplicateCodeOffset on_duplicate) {
   int offset = static_cast<int>(bytecode_offset);
-  AddEntry({offset, source_position, true});
+  AddEntry({offset, source_position, true}, on_duplicate);
   LOG_CODE_EVENT(isolate_, CodeLinePosInfoAddStatementPositionEvent(
                                jit_handler_data_, offset, source_position));
   LOG_CODE_EVENT(isolate_, CodeLinePosInfoAddPositionEvent(
@@ -133,24 +134,34 @@ void SourcePositionTableBuilder::AddExpressionPosition(size_t bytecode_offset,
                                jit_handler_data_, offset, source_position));
 }
 
-void SourcePositionTableBuilder::AddEntry(const PositionTableEntry& entry) {
+void SourcePositionTableBuilder::AddEntry(
+    const PositionTableEntry& entry,
+    SourcePositionTableBuilder::OnDuplicateCodeOffset on_duplicate) {
   // Don't encode a new entry if this bytecode already has a source position
   // assigned.
   if (bytes_.size() > 0 && previous_.bytecode_offset == entry.bytecode_offset) {

[vogelheim, 2016/03/14 10:41:15]

I'm not sure this is correct... This change always keeps the "current" entry in
candidate_ - much like earlier versions of this code did - but it compares
against previous_ - not candidate_ - to determine whether to write an entry.

So if we write entries A, B, C, I expect something like this to happen:

[p_ == previous_, c_ == candidate_]

- p_: {0,..}, c_: {-1,...}
- AddEntry(A)
  - p_: {0,..}, c_: A
- AddEntry(B)
  - p_: A, c_: B
- AddEntry(C)
  - ...

If B + C have the same offset and OVERWRITE_DUPLICATE, wouldn't it still write
B, since in the last step's AddEntry it will compare C against A, rather then B?



  

[Yang, 2016/03/14 11:32:34]

You are right. Fixed.

+    if (on_duplicate == OVERWRITE_DUPLICATE) candidate_ = entry;
     return;
   }
 
-  PositionTableEntry tmp(entry);
+  CommitEntry();
+  candidate_ = entry;
+}
+
+void SourcePositionTableBuilder::CommitEntry() {
+  if (candidate_.bytecode_offset == kUninitializedCandidateOffset) return;
+  PositionTableEntry tmp(candidate_);
   SubtractFromEntry(tmp, previous_);
   EncodeEntry(bytes_, tmp);
-  previous_ = entry;
+  previous_ = candidate_;
 
 #ifdef ENABLE_SLOW_DCHECKS
-  raw_entries_.push_back(entry);
+  raw_entries_.push_back(candidate_);
 #endif
 }
 
 Handle<ByteArray> SourcePositionTableBuilder::ToSourcePositionTable() {
+  CommitEntry();
   if (bytes_.empty()) return isolate_->factory()->empty_byte_array();
 
   Handle<ByteArray> table = isolate_->factory()->NewByteArray(