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

[David Blaikie]

+Alexey

On Thu, May 21, 2015 at 4:24 PM, Keno Fischer <kfischer at college.harvard.edu>
wrote:

> Hi echristo, friss, dblaikie,
>
> This fixes a bug in the line info handling in the dwarf code, based on a
> problem I when implementing RelocVisitor support for MachO.
> Since addr+size will give the first address past the end of the function,
> we need to back up one line table entry. However, if we are doing the
> comparison with < rather than <=, we actually end up with the second
> entry past the end of the current function, if the next function
> starts at exactly addr+size (the first entry for the next
> function is at addr+size, so the next entry greater than it
> is the second entry for the next function).
> This comes up on MachO much more than on ELF, since MachO
> doesn't store the symbol size separately, hence making
> said situation always occur.
>
> REPOSITORY
>   rL LLVM
>
> http://reviews.llvm.org/D9925
>
> Files:
>   include/llvm/DebugInfo/DWARF/DWARFDebugLine.h
>   lib/DebugInfo/DWARF/DWARFDebugLine.cpp
>   test/DebugInfo/debuglineinfo-macho.test
>
> Index: include/llvm/DebugInfo/DWARF/DWARFDebugLine.h
> ===================================================================
> --- include/llvm/DebugInfo/DWARF/DWARFDebugLine.h
> +++ include/llvm/DebugInfo/DWARF/DWARFDebugLine.h
> @@ -94,6 +94,10 @@
>        return LHS.Address < RHS.Address;
>      }
>
> +    static bool orderByAddressLeq(const Row& LHS, const Row& RHS) {
> +      return LHS.Address <= RHS.Address;
> +    }
> +
>      // The program-counter value corresponding to a machine instruction
>      // generated by the compiler.
>      uint64_t Address;
> Index: lib/DebugInfo/DWARF/DWARFDebugLine.cpp
> ===================================================================
> --- lib/DebugInfo/DWARF/DWARFDebugLine.cpp
> +++ lib/DebugInfo/DWARF/DWARFDebugLine.cpp
> @@ -605,13 +605,10 @@
>        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);
> +
>  DWARFDebugLine::Row::orderByAddressLeq);
>        // 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.
> +      // our or equal to start address.
>        first_row_index = cur_seq.FirstRowIndex + (row_pos - first_row);
> -      if (row_pos != first_row)
> -        --first_row_index;
>      } else
>        first_row_index = cur_seq.FirstRowIndex;
>
> @@ -623,9 +620,11 @@
>        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);
> +
>  DWARFDebugLine::Row::orderByAddressLeq);
>        // 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.
> +      // or equal our end address. The row before that is the last row we
> want,
> +      // since end_address is the first address past the end of the range
> we're
> +      // looking up.
>        last_row_index = cur_seq.FirstRowIndex + (row_pos - first_row) - 1;
>      } else
>        // Contrary to what you might expect,
> DWARFDebugLine::SequenceLastRowIndex
> Index: test/DebugInfo/debuglineinfo-macho.test
> ===================================================================
> --- test/DebugInfo/debuglineinfo-macho.test
> +++ test/DebugInfo/debuglineinfo-macho.test
> @@ -3,3 +3,39 @@
>  RUN: llvm-dwarfdump %p/Inputs/test-multiple-macho.o | FileCheck %s
>
>  CHECK-NOT: error: failed to compute relocation: X86_64_RELOC_UNSIGNED
> +
> +# Check that relocations get applied correctly
> +RUN: llvm-rtdyld -printobjline %p/Inputs/test-simple-macho.o \
> +RUN:   | FileCheck %s -check-prefix TEST_SIMPLE
> +RUN: llvm-rtdyld -printline %p/Inputs/test-simple-macho.o \
> +RUN:   | FileCheck %s -check-prefix TEST_SIMPLE
> +RUN: llvm-rtdyld -printobjline %p/Inputs/test-multiple-macho.o \
> +RUN:   | FileCheck %s -check-prefix TEST_MULTIPLE
> +RUN: llvm-rtdyld -printline %p/Inputs/test-multiple-macho.o \
> +RUN:   | FileCheck %s -check-prefix TEST_MULTIPLE
> +
> +TEST_SIMPLE:      Function: _foo, Size = 11
> +TEST_SIMPLE-NEXT:  Line info @ 0: simple.c, line:1
> +TEST_SIMPLE-NEXT:  Line info @ 7: simple.c, line:2
> +TEST_SIMPLE-NEXT:  Line info @ 11: simple.c, line:2
> +
> +TEST_MULTIPLE:      Function: _bar, Size = 48
> +TEST_MULTIPLE-NEXT:   Line info @ 0: multiple.c, line:5
> +TEST_MULTIPLE-NEXT:   Line info @ 7: multiple.c, line:6
> +TEST_MULTIPLE-NEXT:   Line info @ 16: multiple.c, line:9
> +TEST_MULTIPLE-NEXT:   Line info @ 21: multiple.c, line:9
> +TEST_MULTIPLE-NEXT:   Line info @ 26: multiple.c, line:7
> +TEST_MULTIPLE-NEXT:   Line info @ 33: multiple.c, line:10
> +TEST_MULTIPLE-NOT:    Line info @ 48: multiple.c, line:12
> +TEST_MULTIPLE-NEXT: Function: _foo, Size = 16
> +TEST_MULTIPLE-NEXT:   Line info @ 0: multiple.c, line:1
> +TEST_MULTIPLE-NEXT:   Line info @ 7: multiple.c, line:2
> +TEST_MULTIPLE-NOT:    Line info @ 16: multiple.c, line:5
> +TEST_MULTIPLE-NEXT: Function: _fubar, Size = 46
> +TEST_MULTIPLE-NEXT:   Line info @ 0: multiple.c, line:12
> +TEST_MULTIPLE-NEXT:   Line info @ 7: multiple.c, line:13
> +TEST_MULTIPLE-NEXT:   Line info @ 12: multiple.c, line:17
> +TEST_MULTIPLE-NEXT:   Line info @ 25: multiple.c, line:15
> +TEST_MULTIPLE-NEXT:   Line info @ 34: multiple.c, line:19
> +TEST_MULTIPLE-NEXT:   Line info @ 41: multiple.c, line:21
> +TEST_MULTIPLE-NEXT:   Line info @ 46: multiple.c, line:21
>
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150521/c763ec05/attachment.html>