[lldb/SymbolFile] Don't parse the whole line table for the support files

[JDevlieghere]

Summary

Greg noticed that line table parsing has regressed by 5x in Xcode 11.4.1. Rephrased from our (off list) e-mail conversation:

Prior to my patch of using the LLVM line table parsing code, SymbolFileDWARF::ParseSupportFiles would only parse the line table prologues to get the file list for any files that could be in the line table. If we found the file that someone is setting the breakpoint in in the support files list, we would get a valid index. If we didn't, we would not look any further. So someone sets a breakpoint one "MyFile.cpp:12" and if we find "MyFile.cpp" in the support file list for the compile unit, then and only then would we get the entire line table for that compile unit. Now, no matter what, we always fully parse the line table for all compile units any time any file and line breakpoint is set. This creates a serious problem when debugging a large DWARF in .o file project.

This patch re-instates the old behavior. Unfortunately it means we might end up parsing to prologue twice, but I don't think that outweighs the cost of trying to cache/reuse it.

[clayborg]

Yes this fixes the regression and actually speeds things up a bit. My results were a speed up of 17% to 30% versus 11.3.1 LLDB.

Just one nit in the way we are getting the DW_AT_stmt_list and not adding in the line table offset in the inline comments and this is good to go.

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

893–896

DWARFContext::getLineTableForUnit() in llvm/lib/DebugInfo/DWARF/DWARFContext.cpp grabs the DW_AT_stmt_list like this:

auto Offset = toSectionOffset(UnitDIE.find(DW_AT_stmt_list));
if (!Offset)
  return false; // No line table for this compile unit.

uint64_t stmtOffset = *Offset + U->getLineTableOffset();

Modifying this a bit would be a good idea to make sure we are compatible with all DWARF. Or we can put a function into DWARFUnit that does this correctly and switch DWARFContext::getLineTableForUnit() and our code over to use it to avoid duplicated code.

[JDevlieghere]

Use GetLineTableOffset()

[clayborg]

Nice! Our DWARFUnit::GetLineTableOffset() isn't adding some extra offset to the line table offset that the LLVM parser is adding, but we can do this in a separate patch.

[labath]

If this didn't show up on the lldb benchmark charts (https://teemperor.de/lldb-bench/static.html), it would be nice to add a benchmark for this. What exactly were you measuring here?

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

191–192

It would be good to also add a {0} somewhere to print the actual error we received.

[clayborg]

Hey Jonas, any idea if this could be cherry picked into a 11.5.1 update at any point? The performance is hindering 11.4.1 and 11.5 clients at Facebook.

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

ParseLLVMLineTable(lldb_private::DWARFContext &context,
   if (!line_table) {
     LLDB_LOG_ERROR(log, line_table.takeError(),
                    "SymbolFileDWARF::ParseLineTable failed to parse");
     return nullptr;
   }
   return *line_table;
 }
 
+static bool ParseLLVMLineTablePrologue(lldb_private::DWARFContext &context,
+                                       llvm::DWARFDebugLine::Prologue &prologue,
+                                       dw_offset_t line_offset,
+                                       dw_offset_t unit_offset) {
+  Log *log = LogChannelDWARF::GetLogIfAll(DWARF_LOG_DEBUG_INFO);
+  bool success = true;
+  llvm::DWARFDataExtractor data = context.getOrLoadLineData().GetAsLLVM();
+  llvm::DWARFContext &ctx = context.GetAsLLVM();
+  uint64_t offset = line_offset;
+  llvm::Error error = prologue.parse(
+      data, &offset,
+      [&](llvm::Error e) {
+        success = false;
+        LLDB_LOG_ERROR(log, std::move(e),
+                       "SymbolFileDWARF::ParseSupportFiles failed to parse "
+                       "line table prologue");
+      },
+      ctx, nullptr);
+  if (error) {
+    LLDB_LOG_ERROR(log, std::move(error),
+                   "SymbolFileDWARF::ParseSupportFiles failed to parse line "
+                   "table prologue");
+    return false;
+  }
+  return success;
+}
+
 static llvm::Optional<std::string>
 GetFileByIndex(const llvm::DWARFDebugLine::Prologue &prologue, size_t idx,
                llvm::StringRef compile_dir, FileSpec::Style style) {
   // Try to get an absolute path first.
   std::string abs_path;
   auto absolute = llvm::DILineInfoSpecifier::FileLineInfoKind::AbsoluteFilePath;
   if (prologue.getFileNameByIndex(idx, compile_dir, absolute, abs_path, style))
     return std::move(abs_path);
for (std::size_t i = 0; i < module->GetNumCompileUnits(); ++i) {
         return true;
     }
   }
   return false;
 }
 
 bool SymbolFileDWARF::ParseSupportFiles(CompileUnit &comp_unit,
                                         FileSpecList &support_files) {
-  if (!comp_unit.GetLineTable())
-    ParseLineTable(comp_unit);
+  std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
+  DWARFUnit *dwarf_cu = GetDWARFCompileUnit(&comp_unit);
+  if (!dwarf_cu)
+    return false;
+
+  dw_offset_t offset = dwarf_cu->GetLineTableOffset();
+  if (offset == DW_INVALID_OFFSET)
+    return false;
+
+  llvm::DWARFDebugLine::Prologue prologue;
+  if (!ParseLLVMLineTablePrologue(m_context, prologue, offset,
+                                  dwarf_cu->GetOffset()))
+    return false;
+
+  comp_unit.SetSupportFiles(ParseSupportFilesFromPrologue(
+      comp_unit.GetModule(), prologue, dwarf_cu->GetPathStyle(),
+      dwarf_cu->GetCompilationDirectory().GetCString()));
+
   return true;
 }
 
 FileSpec SymbolFileDWARF::GetFile(DWARFUnit &unit, size_t file_idx) {
   if (auto *dwarf_cu = llvm::dyn_cast<DWARFCompileUnit>(&unit)) {
     if (CompileUnit *lldb_cu = GetCompUnitForDWARFCompUnit(*dwarf_cu))
       return lldb_cu->GetSupportFiles().GetFileSpecAtIndex(file_idx);
     return FileSpec();
bool SymbolFileDWARF::ParseLineTable(CompileUnit &comp_unit) {
   std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
   if (comp_unit.GetLineTable() != nullptr)
     return true;
 
   DWARFUnit *dwarf_cu = GetDWARFCompileUnit(&comp_unit);
   if (!dwarf_cu)
     return false;
 
-  const DWARFBaseDIE dwarf_cu_die = dwarf_cu->GetUnitDIEOnly();
-  if (!dwarf_cu_die)
-    return false;
-
-  const dw_offset_t cu_line_offset = dwarf_cu_die.GetAttributeValueAsUnsigned(
-      DW_AT_stmt_list, DW_INVALID_OFFSET);
-  if (cu_line_offset == DW_INVALID_OFFSET)
+  dw_offset_t offset = dwarf_cu->GetLineTableOffset();
+  if (offset == DW_INVALID_OFFSET)
     return false;
 
   llvm::DWARFDebugLine line;
-  const llvm::DWARFDebugLine::LineTable *line_table = ParseLLVMLineTable(
-      m_context, line, cu_line_offset, dwarf_cu->GetOffset());
+  const llvm::DWARFDebugLine::LineTable *line_table =
+      ParseLLVMLineTable(m_context, line, offset, dwarf_cu->GetOffset());
 
   if (!line_table)
     return false;
 
   // FIXME: Rather than parsing the whole line table and then copying it over
   // into LLDB, we should explore using a callback to populate the line table
   // while we parse to reduce memory usage.
   std::unique_ptr<LineSequence> sequence =
if (SymbolFileDWARFDebugMap *debug_map_symfile = GetDebugMapSymfile()) {
     // linked. We need to link the line table and convert the addresses that
     // are relative to the .o file into addresses for the main executable.
     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()));
-
   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())
     return iter->second;