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

[Keno Fischer]

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(
----------------
samsonov wrote:
> consider moving this constant to LineTable struct in the header (and rename to smth. like `UnknownRowIndex`)
Will do.

================
Comment at: lib/DebugInfo/DWARF/DWARFDebugLine.cpp:571
@@ -545,3 +570,3 @@
   }
   if (!found_seq.containsPC(address))
     return unknown_index;
----------------
samsonov wrote:
> Looks like this condition should be checked in `findRowInSequence` function above.
Sure, it wasn't there before below, so I didn't pull it, but seems reasonable.

================
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;
----------------
samsonov wrote:
> I'd rather not provide default value for variables, if we're unconditionally initializing them later.
We're not, this also handles the case when seq_pos == start_pos is false.

================
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)
----------------
samsonov wrote:
> Just a ternary operator?
>   uint32_t first_row_index = (seq_pos == start_pos) ? findRowInSeq(cur_seq, address) : cur_seq.FirstRowIndex;
But that wouldn't capture the case where findRowInSeq returns an `UnknownRowIndex`.

================
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);
----------------
samsonov wrote:
> 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;
Sounds good.

http://reviews.llvm.org/D9925

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