[lldb] Don't put compile unit name into the support file list and support DWARF5 line tables

[labath]

Summary

Lldb's "format-independent" debug info made use of the fact that DWARF
(<=4) did not use the file index zero, and reused the support file index
zero for storing the compile unit name.

While this provided some convenience for DWARF<=4, it meant that the PDB
plugin needed to artificially remap file indices in order to free up
index 0. Furthermore, DWARF v5 make file index 0 legal, which meant that
similar remapping would be needed in the dwarf plugin too.

What this patch does instead is remove the requirement of having the
compile unit name in the index 0. It is not that useful since the name
can always be fetched from the CompileUnit object. Remapping code in the
pdb plugin(s) has been removed or simplified.

DWARF plugin has started inserting an empty FileSpec at index 0 to
ensure the indices keep matching up (in case of DWARF<=4). For DWARF5,
we insert the file 0 from the line table.

I add a test to ensure we can correctly lookup line table entries
referencing file 0, and in particular the case where the file 0 is also
duplicated in another file entry, as this is how clang produces line
tables in some circumstances (see pr44170). Though this is probably a
bug in clang, this is not forbidden by DWARF, and lldb already has
support for that in some (but not all) cases -- this adds a test for the
code path which was not fixed in this patch.

[teemperor]

This seems to break Shell/SymbolFile/DWARF/debug-types-address-ranges.s on my machine: https://teemperor.de/pub/D70954.log

[labath]

It seems one dwarf5 test is failing with this change (or rather, it was accidentally passing before as it picked up the fake line 0 entry). Since this patch is not that big, I'll just fold the dwarf5 followup into it. Planning changes == I need to come up with a test.

[labath]

Add the dwarf5 part + a test.

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

GetFileByIndex(const llvm::DWARFDebugLine::Prologue &prologue, size_t idx,
   // Otherwise ask for a relative path.
   std::string rel_path;
   auto relative = llvm::DILineInfoSpecifier::FileLineInfoKind::Default;
   if (!prologue.getFileNameByIndex(idx, compile_dir, relative, rel_path, style))
     return {};
   return std::move(rel_path);
 }
 
-static FileSpecList ParseSupportFilesFromPrologue(
-    const lldb::ModuleSP &module,
-    const llvm::DWARFDebugLine::Prologue &prologue, FileSpec::Style style,
-    llvm::StringRef compile_dir = {}, FileSpec first_file = {}) {
+static FileSpecList
+ParseSupportFilesFromPrologue(const lldb::ModuleSP &module,
+                              const llvm::DWARFDebugLine::Prologue &prologue,
+                              FileSpec::Style style,
+                              llvm::StringRef compile_dir = {}) {
   FileSpecList support_files;
-  support_files.Append(first_file);
+  size_t first_file = 0;
+  if (prologue.getVersion() <= 4) {
+    // File index 0 is not valid before DWARF v5. Add a dummy entry to ensure
+    // support file list indices match those we get from the debug info and line
+    // tables.
+    support_files.Append(FileSpec());
+    first_file = 1;
+  }
 
   const size_t number_of_files = prologue.FileNames.size();
-  for (size_t idx = 1; idx <= number_of_files; ++idx) {
+  for (size_t idx = first_file; idx <= number_of_files; ++idx) {
     std::string remapped_file;
     if (auto file_path = GetFileByIndex(prologue, idx, compile_dir, style))
       if (!module->RemapSourceFile(llvm::StringRef(*file_path), remapped_file))
         remapped_file = std::move(*file_path);
 
     // Unconditionally add an entry, so the indices match up.
     support_files.EmplaceBack(remapped_file, style);
   }
if (SymbolFileDWARFDebugMap *debug_map_symfile = GetDebugMapSymfile()) {
     comp_unit.SetLineTable(
         debug_map_symfile->LinkOSOLineTable(this, line_table_up.get()));
   } else {
     comp_unit.SetLineTable(line_table_up.release());
   }
 
   comp_unit.SetSupportFiles(ParseSupportFilesFromPrologue(
       comp_unit.GetModule(), line_table->Prologue, dwarf_cu->GetPathStyle(),
-      dwarf_cu->GetCompilationDirectory().GetCString(),
-      comp_unit.GetPrimaryFile()));
+      dwarf_cu->GetCompilationDirectory().GetCString()));
 
   return true;
 }
 
 lldb_private::DebugMacrosSP
 SymbolFileDWARF::ParseDebugMacros(lldb::offset_t *offset) {
   auto iter = m_debug_macros_map.find(*offset);
   if (iter != m_debug_macros_map.end())

lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp

for (const LineColumnEntry &group : lines) {
       if (!efn) {
         llvm::consumeError(efn.takeError());
         continue;
       }
 
       // LLDB wants the index of the file in the list of support files.
       auto fn_iter = llvm::find(cci->m_file_list, *efn);
       lldbassert(fn_iter != cci->m_file_list.end());
-      // LLDB support file indices are 1-based.
-      uint32_t file_index =
-          1 + std::distance(cci->m_file_list.begin(), fn_iter);
+      uint32_t file_index = std::distance(cci->m_file_list.begin(), fn_iter);
 
       std::unique_ptr<LineSequence> sequence(
           line_table->CreateLineSequenceContainer());
       lldbassert(!group.LineNumbers.empty());
 
       for (const LineNumberEntry &entry : group.LineNumbers) {
         AppendLineEntryToSequence(*line_table, *sequence, *cci, virtual_addr,
                                   file_index, *lfh, entry);
bool SymbolFileNativePDB::ParseSupportFiles(CompileUnit &comp_unit,
   lldbassert(cci);
 
   for (llvm::StringRef f : cci->m_file_list) {
     FileSpec::Style style =
         f.startswith("/") ? FileSpec::Style::posix : FileSpec::Style::windows;
     FileSpec spec(f, style);
     support_files.Append(spec);
   }
-
-  llvm::SmallString<64> main_source_file =
-      m_index->compilands().GetMainSourceFile(*cci);
-  FileSpec::Style style = main_source_file.startswith("/")
-                              ? FileSpec::Style::posix
-                              : FileSpec::Style::windows;
-  FileSpec spec(main_source_file, style);
-  support_files.Insert(0, spec);
   return true;
 }
 
 bool SymbolFileNativePDB::ParseImportedModules(
     const SymbolContext &sc, std::vector<SourceModule> &imported_modules) {
   // PDB does not yet support module debug info
   return false;
 }

lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp

bool SymbolFilePDB::ParseSupportFiles(
   if (!files || files->getChildCount() == 0)
     return false;
 
   while (auto file = files->getNext()) {
     FileSpec spec(file->getFileName(), FileSpec::Style::windows);
     support_files.AppendIfUnique(spec);
   }
 
-  // LLDB uses the DWARF-like file numeration (one based),
-  // the zeroth file is the compile unit itself
-  support_files.Insert(0, comp_unit.GetPrimaryFile());
-
   return true;
 }
 
 bool SymbolFilePDB::ParseImportedModules(
     const lldb_private::SymbolContext &sc,
     std::vector<SourceModule> &imported_modules) {
   // PDB does not yet support module debug info
   return false;
void SymbolFilePDB::BuildSupportFileIdToSupportFileIndexMap(
   // the support files array.  We don't want to do path comparisons to avoid
   // basename / full path issues that may or may not even be a problem, so we
   // use the globally unique source file identifiers.  Ideally we could use the
   // global identifiers everywhere, but LineEntry currently assumes indices.
   auto source_files = m_session_up->getSourceFilesForCompiland(compiland);
   if (!source_files)
     return;
 
-  // LLDB uses the DWARF-like file numeration (one based)
-  int index = 1;
-
+  int index = 0;
   while (auto file = source_files->getNext()) {
     uint32_t source_id = file->getUniqueId();
     index_map[source_id] = index++;
   }
 }
 
 lldb::CompUnitSP SymbolFilePDB::GetCompileUnitContainsAddress(
     const lldb_private::Address &so_addr) {

lldb/source/Symbol/CompileUnit.cpp

if (m_variables.get() == nullptr && can_create) {
     CalculateSymbolContext(&sc);
     assert(sc.module_sp);
     sc.module_sp->GetSymbolFile()->ParseVariablesForContext(sc);
   }
 
   return m_variables;
 }
 
+std::vector<uint32_t> FindFileIndexes(const FileSpecList &files, const FileSpec &file) {
+  std::vector<uint32_t> result;
+  uint32_t idx = -1;
+  while ((idx = files.FindFileIndex(idx + 1, file, /*full=*/true)) !=
+         UINT32_MAX)
+    result.push_back(idx);
+  return result;
+}
+
 uint32_t CompileUnit::FindLineEntry(uint32_t start_idx, uint32_t line,
                                     const FileSpec *file_spec_ptr, bool exact,
                                     LineEntry *line_entry_ptr) {
-  uint32_t file_idx = 0;
-
-  if (file_spec_ptr) {
-    file_idx = GetSupportFiles().FindFileIndex(1, *file_spec_ptr, true);
-    if (file_idx == UINT32_MAX)
-      return UINT32_MAX;
-  } else {
-    // All the line table entries actually point to the version of the Compile
-    // Unit that is in the support files (the one at 0 was artificially added.)
-    // So prefer the one further on in the support files if it exists...
-    const FileSpecList &support_files = GetSupportFiles();
-    const bool full = true;
-    file_idx = support_files.FindFileIndex(
-        1, support_files.GetFileSpecAtIndex(0), full);
-    if (file_idx == UINT32_MAX)
-      file_idx = 0;
-  }
+  if (!file_spec_ptr)
+    file_spec_ptr = &GetPrimaryFile();
+  std::vector<uint32_t> file_indexes = FindFileIndexes(GetSupportFiles(), *file_spec_ptr);
+  if (file_indexes.empty())
+
   LineTable *line_table = GetLineTable();
   if (line_table)
-    return line_table->FindLineEntryIndexByFileIndex(start_idx, file_idx, line,
-                                                     exact, line_entry_ptr);
+    return line_table->FindLineEntryIndexByFileIndex(
+        start_idx, file_indexes, line, exact, line_entry_ptr);
   return UINT32_MAX;
 }
 
 void CompileUnit::ResolveSymbolContext(const FileSpec &file_spec,
                                        uint32_t line, bool check_inlines,
                                        bool exact,
                                        SymbolContextItem resolve_scope,
                                        SymbolContextList &sc_list) {
   // First find all of the file indexes that match our "file_spec". If
   // "file_spec" has an empty directory, then only compare the basenames when
   // finding file indexes
   std::vector<uint32_t> file_indexes;
   bool file_spec_matches_cu_file_spec =
       FileSpec::Match(file_spec, this->GetPrimaryFile());
 
   // If we are not looking for inlined functions and our file spec doesn't
   // match then we are done...
   if (!file_spec_matches_cu_file_spec && !check_inlines)
     return;
 
   uint32_t file_idx =
-      GetSupportFiles().FindFileIndex(1, file_spec, true);
+      GetSupportFiles().FindFileIndex(0, file_spec, true);
   while (file_idx != UINT32_MAX) {
     file_indexes.push_back(file_idx);
     file_idx = GetSupportFiles().FindFileIndex(file_idx + 1, file_spec, true);
   }
 
   const size_t num_file_indexes = file_indexes.size();
   if (num_file_indexes == 0)
     return;

lldb/test/Shell/SymbolFile/DWARF/dwarf5-debug_line.s

+# Test handling of DWARF5 line tables. In particular, test that we handle files
+# which are present in the line table more than once.
+
+# REQUIRES: x86
+
+# RUN: llvm-mc -filetype=obj -o %t -triple x86_64-pc-linux %s
+# RUN: %lldb %t -o "source info -f file0.c" -o "source info -f file1.c" \
+# RUN:   -o "breakpoint set -f file0.c -l 42" \
+# RUN:   -o "breakpoint set -f file0.c -l 47" \
+# RUN:   -o exit | FileCheck %s
+
+# CHECK-LABEL: source info -f file0.c
+# CHECK: [0x0000000000000000-0x0000000000000001): /file0.c:42
+# CHECK-LABEL: source info -f file1.c
+# CHECK: [0x0000000000000001-0x0000000000000002): /file1.c:47
+# CHECK-LABEL: breakpoint set -f file0.c -l 42
+# CHECK: Breakpoint 1: {{.*}}`foo,
+# CHECK-LABEL: breakpoint set -f file0.c -l 47
+# CHECK: Breakpoint 2: {{.*}}`foo + 2,
+
+        .text
+        .globl  foo
+foo:
+        nop
+        nop
+        nop
+.Lfoo_end:
+
+        .section        .debug_abbrev,"",@progbits
+        .byte   1                       # Abbreviation Code
+        .byte   17                      # DW_TAG_compile_unit
+        .byte   0                       # DW_CHILDREN_no
+        .byte   37                      # DW_AT_producer
+        .byte   8                       # DW_FORM_string
+        .byte   19                      # DW_AT_language
+        .byte   5                       # DW_FORM_data2
+        .byte   3                       # DW_AT_name
+        .byte   8                       # DW_FORM_string
+        .byte   16                      # DW_AT_stmt_list
+        .byte   23                      # DW_FORM_sec_offset
+        .byte   27                      # DW_AT_comp_dir
+        .byte   8                       # DW_FORM_string
+        .byte   17                      # DW_AT_low_pc
+        .byte   1                       # DW_FORM_addr
+        .byte   18                      # DW_AT_high_pc
+        .byte   6                       # DW_FORM_data4
+        .byte   0                       # EOM(1)
+        .byte   0                       # EOM(2)
+        .byte   0                       # EOM(3)
+
+        .section        .debug_info,"",@progbits
+.Lcu_begin0:
+        .long   .Ldebug_info_end0-.Ldebug_info_start0 # Length of Unit
+.Ldebug_info_start0:
+        .short  5                       # DWARF version number
+        .byte   1                       # DWARF Unit Type
+        .byte   8                       # Address Size (in bytes)
+        .long   .debug_abbrev           # Offset Into Abbrev. Section
+        .byte   1                       # Abbrev [1] 0xc:0x23 DW_TAG_compile_unit
+        .asciz  "Hand-written DWARF"    # DW_AT_producer
+        .short  12                      # DW_AT_language
+        .asciz  "file0.c"               # DW_AT_name
+        .long   .Lline_table_begin      # DW_AT_stmt_list
+        .asciz  "/"                     # DW_AT_comp_dir
+        .quad   foo                     # DW_AT_low_pc
+        .long   .Lfoo_end-foo           # DW_AT_high_pc
+.Ldebug_info_end0:
+
+        .section        .debug_line,"",@progbits
+.Lline_table_begin:
+        .long .Lline_end-.Lline_start
+.Lline_start:
+        .short  5                       # DWARF version number
+        .byte   8                       # Address Size (in bytes)
+        .byte   0                       # Segment Selector Size
+        .long   .Lheader_end-.Lheader_start
+.Lheader_start:
+        .byte   1                       # Minimum Instruction Length
+        .byte   1                       # Maximum Operations per Instruction
+        .byte   1                       # Default is_stmt
+        .byte   0                       # Line Base
+        .byte   0                       # Line Range
+        .byte   5                       # Opcode Base
+        .byte   0, 1, 1, 1              # Standard Opcode Lengths
+
+        # Directory table format
+        .byte   1                       # One element per directory entry
+        .byte   1                       # DW_LNCT_path
+        .byte   0x08                    # DW_FORM_string
+
+        # Directory table entries
+        .byte   1                       # 1 directory
+        .asciz  "/"
+
+        # File table format
+        .byte   2                       # 2 elements per file entry
+        .byte   1                       # DW_LNCT_path
+        .byte   0x08                    # DW_FORM_string
+        .byte   2                       # DW_LNCT_directory_index
+        .byte   0x0b                    # DW_FORM_data1
+
+        # File table entries
+        .byte   3                       # 3 files
+        .asciz  "file0.c"
+        .byte   0
+        .asciz  "file1.c"
+        .byte   0
+        .asciz  "file0.c"
+        .byte   0
+.Lheader_end:
+
+        .byte   4, 0                    # DW_LNS_set_file 0
+        .byte   0, 9, 2                 # DW_LNE_set_address
+        .quad   foo
+        .byte   3, 41                   # DW_LNS_advance_line 41
+        .byte   1                       # DW_LNS_copy
+
+        .byte   4, 1                    # DW_LNS_set_file 1
+        .byte   2, 1                    # DW_LNS_advance_pc 1
+        .byte   3, 5                    # DW_LNS_advance_line 5
+        .byte   1                       # DW_LNS_copy
+
+        .byte   4, 2                    # DW_LNS_set_file 2
+        .byte   2, 1                    # DW_LNS_advance_pc 1
+        .byte   1                       # DW_LNS_copy
+
+        .byte   2, 1                    # DW_LNS_advance_pc 1
+        .byte   0, 1, 1                 # DW_LNE_end_sequence
+.Lline_end: