<div dir="ltr">+Alexey</div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, May 21, 2015 at 4:24 PM, Keno Fischer <span dir="ltr"><<a href="mailto:kfischer@college.harvard.edu" target="_blank">kfischer@college.harvard.edu</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi echristo, friss, dblaikie,<br>
<br>
This fixes a bug in the line info handling in the dwarf code, based on a<br>
problem I when implementing RelocVisitor support for MachO.<br>
Since addr+size will give the first address past the end of the function,<br>
we need to back up one line table entry. However, if we are doing the<br>
comparison with < rather than <=, we actually end up with the second<br>
entry past the end of the current function, if the next function<br>
starts at exactly addr+size (the first entry for the next<br>
function is at addr+size, so the next entry greater than it<br>
is the second entry for the next function).<br>
This comes up on MachO much more than on ELF, since MachO<br>
doesn't store the symbol size separately, hence making<br>
said situation always occur.<br>
<br>
REPOSITORY<br>
  rL LLVM<br>
<br>
<a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D9925&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=ERudFU9LDDz-74eT4u9THkmtJj709Na0tGisG-lYjO4&s=Xf-vDIH4sFwiTEe1JL1sUjPJEl-GiKzJJGGNzdLjMzo&e=" target="_blank">http://reviews.llvm.org/D9925</a><br>
<br>
Files:<br>
  include/llvm/DebugInfo/DWARF/DWARFDebugLine.h<br>
  lib/DebugInfo/DWARF/DWARFDebugLine.cpp<br>
  test/DebugInfo/debuglineinfo-macho.test<br>
<br>
Index: include/llvm/DebugInfo/DWARF/DWARFDebugLine.h<br>
===================================================================<br>
--- include/llvm/DebugInfo/DWARF/DWARFDebugLine.h<br>
+++ include/llvm/DebugInfo/DWARF/DWARFDebugLine.h<br>
@@ -94,6 +94,10 @@<br>
       return LHS.Address < RHS.Address;<br>
     }<br>
<br>
+    static bool orderByAddressLeq(const Row& LHS, const Row& RHS) {<br>
+      return LHS.Address <= RHS.Address;<br>
+    }<br>
+<br>
     // The program-counter value corresponding to a machine instruction<br>
     // generated by the compiler.<br>
     uint64_t Address;<br>
Index: lib/DebugInfo/DWARF/DWARFDebugLine.cpp<br>
===================================================================<br>
--- lib/DebugInfo/DWARF/DWARFDebugLine.cpp<br>
+++ lib/DebugInfo/DWARF/DWARFDebugLine.cpp<br>
@@ -605,13 +605,10 @@<br>
       RowIter first_row = Rows.begin() + cur_seq.FirstRowIndex;<br>
       RowIter last_row = Rows.begin() + cur_seq.LastRowIndex;<br>
       RowIter row_pos = std::upper_bound(first_row, last_row, row,<br>
-                                         DWARFDebugLine::Row::orderByAddress);<br>
+                                         DWARFDebugLine::Row::orderByAddressLeq);<br>
       // The 'row_pos' iterator references the first row that is greater than<br>
-      // our start address. Unless that's the first row, we want to start at<br>
-      // the row before that.<br>
+      // our or equal to start address.<br>
       first_row_index = cur_seq.FirstRowIndex + (row_pos - first_row);<br>
-      if (row_pos != first_row)<br>
-        --first_row_index;<br>
     } else<br>
       first_row_index = cur_seq.FirstRowIndex;<br>
<br>
@@ -623,9 +620,11 @@<br>
       RowIter first_row = Rows.begin() + cur_seq.FirstRowIndex;<br>
       RowIter last_row = Rows.begin() + cur_seq.LastRowIndex;<br>
       RowIter row_pos = std::upper_bound(first_row, last_row, row,<br>
-                                         DWARFDebugLine::Row::orderByAddress);<br>
+                                         DWARFDebugLine::Row::orderByAddressLeq);<br>
       // The 'row_pos' iterator references the first row that is greater than<br>
-      // our end address.  The row before that is the last row we want.<br>
+      // or equal our end address. The row before that is the last row we want,<br>
+      // since end_address is the first address past the end of the range we're<br>
+      // looking up.<br>
       last_row_index = cur_seq.FirstRowIndex + (row_pos - first_row) - 1;<br>
     } else<br>
       // Contrary to what you might expect, DWARFDebugLine::SequenceLastRowIndex<br>
Index: test/DebugInfo/debuglineinfo-macho.test<br>
===================================================================<br>
--- test/DebugInfo/debuglineinfo-macho.test<br>
+++ test/DebugInfo/debuglineinfo-macho.test<br>
@@ -3,3 +3,39 @@<br>
 RUN: llvm-dwarfdump %p/Inputs/test-multiple-macho.o | FileCheck %s<br>
<br>
 CHECK-NOT: error: failed to compute relocation: X86_64_RELOC_UNSIGNED<br>
+<br>
+# Check that relocations get applied correctly<br>
+RUN: llvm-rtdyld -printobjline %p/Inputs/test-simple-macho.o \<br>
+RUN:   | FileCheck %s -check-prefix TEST_SIMPLE<br>
+RUN: llvm-rtdyld -printline %p/Inputs/test-simple-macho.o \<br>
+RUN:   | FileCheck %s -check-prefix TEST_SIMPLE<br>
+RUN: llvm-rtdyld -printobjline %p/Inputs/test-multiple-macho.o \<br>
+RUN:   | FileCheck %s -check-prefix TEST_MULTIPLE<br>
+RUN: llvm-rtdyld -printline %p/Inputs/test-multiple-macho.o \<br>
+RUN:   | FileCheck %s -check-prefix TEST_MULTIPLE<br>
+<br>
+TEST_SIMPLE:      Function: _foo, Size = 11<br>
+TEST_SIMPLE-NEXT:  Line info @ 0: simple.c, line:1<br>
+TEST_SIMPLE-NEXT:  Line info @ 7: simple.c, line:2<br>
+TEST_SIMPLE-NEXT:  Line info @ 11: simple.c, line:2<br>
+<br>
+TEST_MULTIPLE:      Function: _bar, Size = 48<br>
+TEST_MULTIPLE-NEXT:   Line info @ 0: multiple.c, line:5<br>
+TEST_MULTIPLE-NEXT:   Line info @ 7: multiple.c, line:6<br>
+TEST_MULTIPLE-NEXT:   Line info @ 16: multiple.c, line:9<br>
+TEST_MULTIPLE-NEXT:   Line info @ 21: multiple.c, line:9<br>
+TEST_MULTIPLE-NEXT:   Line info @ 26: multiple.c, line:7<br>
+TEST_MULTIPLE-NEXT:   Line info @ 33: multiple.c, line:10<br>
+TEST_MULTIPLE-NOT:    Line info @ 48: multiple.c, line:12<br>
+TEST_MULTIPLE-NEXT: Function: _foo, Size = 16<br>
+TEST_MULTIPLE-NEXT:   Line info @ 0: multiple.c, line:1<br>
+TEST_MULTIPLE-NEXT:   Line info @ 7: multiple.c, line:2<br>
+TEST_MULTIPLE-NOT:    Line info @ 16: multiple.c, line:5<br>
+TEST_MULTIPLE-NEXT: Function: _fubar, Size = 46<br>
+TEST_MULTIPLE-NEXT:   Line info @ 0: multiple.c, line:12<br>
+TEST_MULTIPLE-NEXT:   Line info @ 7: multiple.c, line:13<br>
+TEST_MULTIPLE-NEXT:   Line info @ 12: multiple.c, line:17<br>
+TEST_MULTIPLE-NEXT:   Line info @ 25: multiple.c, line:15<br>
+TEST_MULTIPLE-NEXT:   Line info @ 34: multiple.c, line:19<br>
+TEST_MULTIPLE-NEXT:   Line info @ 41: multiple.c, line:21<br>
+TEST_MULTIPLE-NEXT:   Line info @ 46: multiple.c, line:21<br>
<br>
EMAIL PREFERENCES<br>
  <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_settings_panel_emailpreferences_&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=ERudFU9LDDz-74eT4u9THkmtJj709Na0tGisG-lYjO4&s=DcumszKA2iXtcJxiYujWG7MxpS45weZJsyjobo61CbU&e=" target="_blank">http://reviews.llvm.org/settings/panel/emailpreferences/</a><br>
</blockquote></div><br></div>