Fix line table resolution near the end of a section

[labath]

Summary

lld r367537 changed the way the linker organizes sections and segments.
This exposed an lldb bug and caused some tests to fail.

In all of the failing tests the root cause was the same -- when we were
trying to resolve the last address in the line_table section, we failed
because it pointed past the end of the section.

This is similar to the problem encountered in D32022, where we had a
problem with unwinding of functions near the end of the section/module.
In that patch, we added a flag which allowed these end addresses to
resolve to the section, but we only did so for load address computations
(SectionLoadList). line table uses file addresses, and so we are not
able to take advantage of that here.

This patch adds an equivalent flag to the SectionList class, and changes
the line table resolving code to use it for end-of-sequence entries (as
those are the only ones that can validly point outside of a section).

I also revert the linker flags which were added to the failing tests to
restore previous behavior, and add a unit test to exercise the new API
more directly.

[clayborg]

I vote to not add any API calls with allow_section_end and just take care of this in source/Symbol/LineTable.cpp. All of my inline comments will reflect this notion.

include/lldb/Core/Module.h

683–684 ↗

(On Diff #213020)

revert

include/lldb/Core/Section.h

74–80 ↗

(On Diff #213020)

revert

133–137 ↗

(On Diff #213020)

revert

source/Core/Module.cpp

426–440 ↗

(On Diff #213020)

revert

source/Core/Section.cpp

270–276 ↗

(On Diff #213020)

revert

590–603 ↗

(On Diff #213020)

revert

source/Symbol/LineTable.cpp

247–249 ↗

(On Diff #213020)

Take care of backing up the entry address if this is the last entry so we don't need any of the other API changes:

lldb::addr_t file_addr = entry.file_addr - (entry.is_terminal_entry ? 1 : 0);
if (module_sp && module_sp->ResolveFileAddress(file_addr, line_entry.range.GetBaseAddress())) {
  if (entry.is_terminal_entry) 
    line_entry.range.GetBaseAddress().Slide(1);

[labath]

• redo the patch to keep the logic inside the line table code. I've been

considering whether to do that my self. On one hand, this functionality seems
like it could be useful in other places, but OTOH, the bool argument api is not
really nice. I guess we can keep it this way until we come up with a better way
to do that.

I've kept the module unit test as it seems useful regardless of the fact that
I'm not changing that code.

[labath]

(I eventually decided to skip the test, as without the extra flag, the same functionality can be exercised in a much more conventional way with "image lookup -a")

lldb/trunk/lit/SymbolFile/DWARF/debug-line-basic.s

 # REQUIRES: lld, x86
 
 # RUN: llvm-mc -triple x86_64-pc-linux %s -filetype=obj > %t.o
-# RUN: ld.lld -z separate-code %t.o -o %t
+# RUN: ld.lld %t.o -o %t
 # RUN: %lldb %t -o "image dump line-table -v a.c" -o exit | FileCheck %s
 
 
   .text
   .globl  _start
 _start:
 # CHECK: Line table for /tmp/a.c
   .file 1 "/tmp/b.c"

lldb/trunk/lit/SymbolFile/DWARF/dir-separator-no-comp-dir-relative-name.s

 # Test that parsing of line tables works reasonably. In this case the debug info
 # does not have enough information for our heuristics to determine the path
 # style, so we will just treat them as native host paths.
 
 # REQUIRES: lld, x86
 
 # RUN: llvm-mc -triple x86_64-pc-linux %s -filetype=obj > %t.o
-# RUN: ld.lld -z separate-code %t.o -o %t
+# RUN: ld.lld %t.o -o %t
 # RUN: %lldb %t -s %S/Inputs/dir-separator-no-comp-dir-relative-name.lldbinit -o exit | FileCheck %s
 
 # CHECK-LABEL: image dump line-table a.c
 # CHECK: Line table for foo{{.}}a.c
 # CHECK-NEXT: 0x0000000000201000: foo{{.}}a.c:1
 # CHECK-NEXT: 0x0000000000201001: foo{{.}}b.c:1
 # CHECK-NEXT: 0x0000000000201002: foo{{.}}b.c:1
 # CHECK-EMPTY:

lldb/trunk/lit/SymbolFile/DWARF/dir-separator-no-comp-dir.s

 # Test that we properly determine the path syntax of a compile unit even if the
 # compile unit does not have a DW_AT_comp_dir attribute.
 
 # REQUIRES: lld, x86
 
 # RUN: llvm-mc -triple x86_64-pc-linux %s -filetype=obj > %t.o
-# RUN: ld.lld -z separate-code %t.o -o %t
+# RUN: ld.lld %t.o -o %t
 # RUN: %lldb %t -s %S/Inputs/dir-separator-windows.lldbinit -o exit | FileCheck %s
 
 # CHECK-LABEL: image dump line-table a.c
 # CHECK: Line table for C:\tmp\a.c
 # CHECK-NEXT: 0x0000000000201000: C:\tmp\a.c:1
 # CHECK-NEXT: 0x0000000000201001: C:\tmp\b.c:1
 # CHECK-NEXT: 0x0000000000201002: C:\tmp\b.c:1
 # CHECK-EMPTY:

lldb/trunk/lit/SymbolFile/DWARF/dir-separator-posix.s

 # Test that parsing of line tables works reasonably, even if the host directory
 # separator does not match the separator of the compile unit.
 
 # REQUIRES: lld, x86
 
 # RUN: llvm-mc -triple x86_64-pc-linux %s -filetype=obj > %t.o
-# RUN: ld.lld -z separate-code %t.o -o %t
+# RUN: ld.lld %t.o -o %t
 # RUN: %lldb %t -s %S/Inputs/dir-separator-posix.lldbinit -o exit | FileCheck %s
 
 # CHECK-LABEL: image dump line-table a.c
 # CHECK: Line table for /tmp/a.c
 # CHECK-NEXT: 0x0000000000201000: /tmp/a.c:1
 # CHECK-NEXT: 0x0000000000201001: /tmp/b.c:1
 # CHECK-NEXT: 0x0000000000201002: /tmp/b.c:1
 # CHECK-EMPTY:

lldb/trunk/lit/SymbolFile/DWARF/dir-separator-windows.s

 # Test that parsing of line tables works reasonably, even if the host directory
 # separator does not match the separator of the compile unit.
 
 # REQUIRES: lld, x86
 
 # RUN: llvm-mc -triple x86_64-pc-linux %s -filetype=obj > %t.o
-# RUN: ld.lld -z separate-code %t.o -o %t
+# RUN: ld.lld %t.o -o %t
 # RUN: %lldb %t -s %S/Inputs/dir-separator-windows.lldbinit -o exit | FileCheck %s
 
 # CHECK-LABEL: image dump line-table a.c
 # CHECK: Line table for C:\tmp\a.c
 # CHECK-NEXT: 0x0000000000201000: C:\tmp\a.c:1
 # CHECK-NEXT: 0x0000000000201001: C:\tmp\b.c:1
 # CHECK-NEXT: 0x0000000000201002: C:\tmp\b.c:1
 # CHECK-EMPTY:

lldb/trunk/source/Symbol/LineTable.cpp

if (search_entry.file_addr != LLDB_INVALID_ADDRESS) {
       }
     }
   }
   return success;
 }
 
 bool LineTable::ConvertEntryAtIndexToLineEntry(uint32_t idx,
                                                LineEntry &line_entry) {
-  if (idx < m_entries.size()) {
-    const Entry &entry = m_entries[idx];
-    ModuleSP module_sp(m_comp_unit->GetModule());
-    if (module_sp &&
-        module_sp->ResolveFileAddress(entry.file_addr,
-                                      line_entry.range.GetBaseAddress())) {
-      if (!entry.is_terminal_entry && idx + 1 < m_entries.size())
-        line_entry.range.SetByteSize(m_entries[idx + 1].file_addr -
-                                     entry.file_addr);
-      else
-        line_entry.range.SetByteSize(0);
+  if (idx >= m_entries.size())
+    return false;
+
+  if (!module_sp)
+    return false;
+
+  addr_t file_addr = entry.file_addr;
+
+  // A terminal entry can point outside of a module or a section. Decrement the
+  // address to ensure it resolves correctly.
+  if (entry.is_terminal_entry)
+    --file_addr;
+
+  if (!module_sp->ResolveFileAddress(file_addr,
+                                     line_entry.range.GetBaseAddress()))
+    return false;
+
+  // Now undo the decrement above.
+  if (entry.is_terminal_entry)
+    line_entry.range.GetBaseAddress().Slide(1);
+
 
-      line_entry.file =
-          m_comp_unit->GetSupportFiles().GetFileSpecAtIndex(entry.file_idx);
-      line_entry.original_file =
-          m_comp_unit->GetSupportFiles().GetFileSpecAtIndex(entry.file_idx);
-      line_entry.line = entry.line;
-      line_entry.column = entry.column;
-      line_entry.is_start_of_statement = entry.is_start_of_statement;
-      line_entry.is_start_of_basic_block = entry.is_start_of_basic_block;
-      line_entry.is_prologue_end = entry.is_prologue_end;
-      line_entry.is_epilogue_begin = entry.is_epilogue_begin;
-      line_entry.is_terminal_entry = entry.is_terminal_entry;
-      return true;
-    }
-  }
-  return false;
-}
 
 uint32_t LineTable::FindLineEntryIndexByFileIndex(
     uint32_t start_idx, const std::vector<uint32_t> &file_indexes,
     uint32_t line, bool exact, LineEntry *line_entry_ptr) {
 
   const size_t count = m_entries.size();
   std::vector<uint32_t>::const_iterator begin_pos = file_indexes.begin();
   std::vector<uint32_t>::const_iterator end_pos = file_indexes.end();