[interpreter] statement position always overwrites earlier positions.

If a statement or expression does not produce any bytecode, it's position
should always be overwritten by a following statement position.

R=mstarzinger@chromium.org, vogelheim@chromium.org
BUG=v8:4680
LOG=N

Review URL: https://codereview.chromium.org/1854113002

Cr-Commit-Position: refs/heads/master@{#35252}

src/interpreter/bytecode-array-builder.cc

@@ -1075,9 +1075,8 @@ size_t BytecodeArrayBuilder::GetConstantPoolEntry(Handle<Object> object) {
 void BytecodeArrayBuilder::SetReturnPosition() {
   if (return_position_ == RelocInfo::kNoPosition) return;
   if (exit_seen_in_block_) return;
-  source_position_table_builder_.AddStatementPosition(
-      bytecodes_.size(), return_position_,
-      SourcePositionTableBuilder::OVERWRITE_DUPLICATE);
+  source_position_table_builder_.AddStatementPosition(bytecodes_.size(),
+                                                      return_position_);
 }
 
 void BytecodeArrayBuilder::SetStatementPosition(Statement* stmt) {

src/interpreter/source-position-table.cc

@@ -115,35 +115,23 @@ void DecodeEntry(ByteArray* bytes, int* index, PositionTableEntry* entry) {
 
 }  // namespace
 
-void SourcePositionTableBuilder::AddStatementPosition(
-    size_t bytecode_offset, int source_position,
-    SourcePositionTableBuilder::OnDuplicateCodeOffset on_duplicate) {
+void SourcePositionTableBuilder::AddStatementPosition(size_t bytecode_offset,
+                                                      int source_position) {
   int offset = static_cast<int>(bytecode_offset);
-  AddEntry({offset, source_position, true}, on_duplicate);
-  LOG_CODE_EVENT(isolate_, CodeLinePosInfoAddStatementPositionEvent(
-                               jit_handler_data_, offset, source_position));
-  LOG_CODE_EVENT(isolate_, CodeLinePosInfoAddPositionEvent(
-                               jit_handler_data_, offset, source_position));
+  AddEntry({offset, source_position, true});
 }
 
 void SourcePositionTableBuilder::AddExpressionPosition(size_t bytecode_offset,
                                                        int source_position) {
   int offset = static_cast<int>(bytecode_offset);
   AddEntry({offset, source_position, false});
-  LOG_CODE_EVENT(isolate_, CodeLinePosInfoAddPositionEvent(
-                               jit_handler_data_, offset, source_position));
 }
 
-void SourcePositionTableBuilder::AddEntry(
-    const PositionTableEntry& entry,
-    SourcePositionTableBuilder::OnDuplicateCodeOffset on_duplicate) {
+void SourcePositionTableBuilder::AddEntry(const PositionTableEntry& entry) {
   // Don't encode a new entry if this bytecode already has a source position
   // assigned.
   if (candidate_.bytecode_offset == entry.bytecode_offset) {
-    if ((!candidate_.is_statement && entry.is_statement) ||
-        on_duplicate == OVERWRITE_DUPLICATE) {
-      candidate_ = entry;
-    }
+    if (entry.is_statement) candidate_ = entry;
     return;
   }
 
@@ -158,6 +146,15 @@ void SourcePositionTableBuilder::CommitEntry() {
   EncodeEntry(bytes_, tmp);
   previous_ = candidate_;
 
+  if (candidate_.is_statement) {
+    LOG_CODE_EVENT(isolate_, CodeLinePosInfoAddStatementPositionEvent(
+                                 jit_handler_data_, candidate_.bytecode_offset,
+                                 candidate_.source_position));
+  }
+  LOG_CODE_EVENT(isolate_, CodeLinePosInfoAddPositionEvent(
+                               jit_handler_data_, candidate_.bytecode_offset,
+                               candidate_.source_position));
+
 #ifdef ENABLE_SLOW_DCHECKS
   raw_entries_.push_back(candidate_);
 #endif

src/interpreter/source-position-table.h

@@ -36,12 +36,7 @@ struct PositionTableEntry {
 
 class SourcePositionTableBuilder : public PositionsRecorder {
  public:
-  enum OnDuplicateCodeOffset {
-    DISCARD_DUPLICATE,
-    OVERWRITE_DUPLICATE,
-  };
-
-  explicit SourcePositionTableBuilder(Isolate* isolate, Zone* zone)
+  SourcePositionTableBuilder(Isolate* isolate, Zone* zone)
       : isolate_(isolate),
         bytes_(zone),
 #ifdef ENABLE_SLOW_DCHECKS
@@ -50,17 +45,14 @@ class SourcePositionTableBuilder : public PositionsRecorder {
         candidate_(kUninitializedCandidateOffset, 0, false) {
   }
 
-  void AddStatementPosition(
-      size_t bytecode_offset, int source_position,
-      OnDuplicateCodeOffset on_duplicate = DISCARD_DUPLICATE);
+  void AddStatementPosition(size_t bytecode_offset, int source_position);
   void AddExpressionPosition(size_t bytecode_offset, int source_position);
   Handle<ByteArray> ToSourcePositionTable();
 
  private:
   static const int kUninitializedCandidateOffset = -1;
 
-  void AddEntry(const PositionTableEntry& entry,
-                OnDuplicateCodeOffset on_duplicate = DISCARD_DUPLICATE);
+  void AddEntry(const PositionTableEntry& entry);
   void CommitEntry();
 
   Isolate* isolate_;

test/mjsunit/mjsunit.status

@@ -798,9 +798,6 @@
   # objects.
   'es6/mirror-collections': [FAIL],
 
-  # TODO(mythria, 4680): possibly problem with line numbers.
-  'es6/regress/regress-468661': [FAIL],
-
   # TODO(mythria, 4680): Fails with context_register_count_ > 0 (0 vs. 0) when
   # trying to get a context register in BytecodeGenerator.
   'harmony/regress/regress-4658': [FAIL, ['mode == release and dcheck_always_on == False', PASS],],

test/unittests/interpreter/source-position-table-unittest.cc

@@ -37,15 +37,7 @@ TEST_F(SourcePositionTableTest, EncodeStatementDuplicates) {
   SourcePositionTableBuilder builder(isolate(), zone());
   for (int i = 0; i < arraysize(offsets); i++) {
     builder.AddStatementPosition(offsets[i], offsets[i]);
-    builder.AddStatementPosition(
-        offsets[i], offsets[i],
-        (i % 2 == 0) ? SourcePositionTableBuilder::OVERWRITE_DUPLICATE
-                     : SourcePositionTableBuilder::DISCARD_DUPLICATE);
-    builder.AddStatementPosition(offsets[i], offsets[i]);
-    builder.AddStatementPosition(
-        offsets[i], offsets[i],
-        (i % 2 == 0) ? SourcePositionTableBuilder::OVERWRITE_DUPLICATE
-                     : SourcePositionTableBuilder::DISCARD_DUPLICATE);
+    builder.AddStatementPosition(offsets[i], offsets[i] + 1);
   }
 
   // To test correctness, we rely on the assertions in ToSourcePositionTable().