[PATCH] [DWARF] Fix a bug in line info handling

[Alexey Samsonov]

REPOSITORY
  rL LLVM

================
Comment at: lib/DebugInfo/DWARF/DWARFDebugLine.cpp:525
@@ -524,1 +524,3 @@
 
+static uint32_t unknown_index = UINT32_MAX;
+uint32_t DWARFDebugLine::LineTable::findRowInSeq(
----------------
consider moving this constant to LineTable struct in the header (and rename to smth. like `UnknownRowIndex`)

================
Comment at: lib/DebugInfo/DWARF/DWARFDebugLine.cpp:571
@@ -545,3 +570,3 @@
   }
   if (!found_seq.containsPC(address))
     return unknown_index;
----------------
Looks like this condition should be checked in `findRowInSequence` function above.

================
Comment at: lib/DebugInfo/DWARF/DWARFDebugLine.cpp:603
@@ -596,20 +602,3 @@
     DWARFDebugLine::Sequence cur_seq = *seq_pos;
-    uint32_t first_row_index;
-    uint32_t last_row_index;
-    if (seq_pos == start_pos) {
-      // For the first sequence, we need to find which row in the sequence is the
-      // first in our range. Rows are stored in a vector, so we may use
-      // arithmetical operations with iterators.
-      DWARFDebugLine::Row row;
-      row.Address = address;
-      RowIter first_row = Rows.begin() + cur_seq.FirstRowIndex;
-      RowIter last_row = Rows.begin() + cur_seq.LastRowIndex;
-      RowIter row_pos = std::upper_bound(first_row, last_row, row,
-                                         DWARFDebugLine::Row::orderByAddress);
-      // The 'row_pos' iterator references the first row that is greater than
-      // our start address. Unless that's the first row, we want to start at
-      // the row before that.
-      first_row_index = cur_seq.FirstRowIndex + (row_pos - first_row);
-      if (row_pos != first_row)
-        --first_row_index;
-    } else
+    uint32_t first_row_index = unknown_index;
+    uint32_t last_row_index = unknown_index;
----------------
I'd rather not provide default value for variables, if we're unconditionally initializing them later.

================
Comment at: lib/DebugInfo/DWARF/DWARFDebugLine.cpp:608
@@ +607,3 @@
+    if (seq_pos == start_pos)
+      first_row_index = findRowInSeq(cur_seq, address);
+    if (first_row_index == unknown_index)
----------------
Just a ternary operator?
  uint32_t first_row_index = (seq_pos == start_pos) ? findRowInSeq(cur_seq, address) : cur_seq.FirstRowIndex;

================
Comment at: lib/DebugInfo/DWARF/DWARFDebugLine.cpp:614
@@ -619,14 +613,3 @@
     // range.  For all other sequences we can go to the end of the sequence.
-    if (cur_seq.HighPC > end_addr) {
-      DWARFDebugLine::Row row;
-      row.Address = end_addr;
-      RowIter first_row = Rows.begin() + cur_seq.FirstRowIndex;
-      RowIter last_row = Rows.begin() + cur_seq.LastRowIndex;
-      RowIter row_pos = std::upper_bound(first_row, last_row, row,
-                                         DWARFDebugLine::Row::orderByAddress);
-      // The 'row_pos' iterator references the first row that is greater than
-      // our end address.  The row before that is the last row we want.
-      last_row_index = cur_seq.FirstRowIndex + (row_pos - first_row) - 1;
-    } else
-      // Contrary to what you might expect, DWARFDebugLine::SequenceLastRowIndex
-      // isn't a valid index within the current sequence.  It's that plus one.
+    if (cur_seq.HighPC > end_addr)
+      last_row_index = findRowInSeq(cur_seq, end_addr-1);
----------------
Fun fact: if you add the call to `Sequence::containsPC()` at the beginning of `findRowInSequence`, as suggested above, you won't need this special case:
  uint32_t last_row_index = findRowInSequnce(cur_seq, end_addr - 1);
  if (last_row_index == UnknownRowIndex)
    last_row_index = cur_seq.LastRowIndex - 1;

================
Comment at: lib/DebugInfo/DWARF/DWARFDebugLine.cpp:621
@@ -634,2 +620,2 @@
 
     for (uint32_t i = first_row_index; i <= last_row_index; ++i) {
----------------
assert that both first_row_index and last_row_index are valid? Or return false in this case.

http://reviews.llvm.org/D9925

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/