[lldb] Allow SymbolFileDWARFDebugMap to register multiple compile units

[augusto2112]

Summary

Currently, SymbolFileDWARFDebugMap works on the assumption that there is
only one compile unit per object file. This patch documents this
limitation (when using the general SymbolFile API), and allows users of
the concrete SymbolFileDWARFDebugMap class to find out about these extra
compile units.

[augusto2112]

Remote extra semi-colon

[labath]

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

55

This isn't a valid requirement for general code. It essentially means that this code will only run for the first compile unit in an elf file (or, if type units are in use, it may not run at all).

[aprantl]

Could we add a test for this that either uses full-LTO in an API test or some assembler code and and lldb-test test?

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

55

Maybe it's better to filter out skeleton units specifically instead? (If that's what the condition is for)

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

651

is std::find() shorter here?

1274–1281

is std::find() shorter / more readable?

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h

179

nit: convert these to ///

180

Why not only store a llvm::SmallVector<lldb::CompUnitSP, 1> and use the first element to store compile_unit_sp?

[augusto2112]

Added test, changed DWARFCompileUnit::BuildAddressRangeTable to not stop looking at oso ranges at offset 0.

[augusto2112]

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

651

I think it'd be worse, because we're comparing pointers so we'd need to pass in a lambda, which would make things more confusing imo (ditto for the next comment).

[aprantl]

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

1277

This is technically a O(n^2) algorithm. What do you think about adding a SmallDenseMap<cu_id, cu_idx> into compile_unit_infos?

lldb/test/API/lang/c/full_lto_stepping/TestFullLtoStepping.py

2

I think this docstring looks copied&pasted?
test that debugging from object files with multiple compile units works.

14

You might want to mark the test as @skipUnlessDarwin since it's about debug maps.

27

If you use lldbutil.run_to_name_breakpoint I think you can delete most of the setup code?

[augusto2112]

Add a map from compile unit id to index in vector.

[aprantl]

LGTM, with two tiny new suggestions.

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h

179

IIUC, the common case is 1 unit (clang), 2 units (swift), n units (lto). Should we reserve 2 instead of one to cover the majority of use-cases?

181

nit: I think we should use the same size as above llvm::SmallDenseMap<uint64_t, uint64_t, 1>, just to keep them in sync?

[augusto2112]

Updated default stack size of the cu containers

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

if (num_ranges > 0) {
         debug_aranges->AppendRange(cu_offset, range.GetRangeBase(),
                                    range.GetRangeEnd());
       }
 
       return;
     }
   }
 
   if (debug_aranges->GetNumRanges() == num_debug_aranges) {
-    // We got nothing from the debug info, maybe we have a line tables only
-    // situation. Check the line tables and build the arange table from this.
+    // We got nothing from the debug info, try to build the arange table from
+    // the debug map OSO aranges.
     SymbolContext sc;
     sc.comp_unit = m_dwarf.GetCompUnitForDWARFCompUnit(*this);
     if (sc.comp_unit) {
       SymbolFileDWARFDebugMap *debug_map_sym_file =
           m_dwarf.GetDebugMapSymfile();
-      if (debug_map_sym_file == nullptr) {
-        if (LineTable *line_table = sc.comp_unit->GetLineTable()) {
-          LineTable::FileAddressRanges file_ranges;
-          const bool append = true;
-          const size_t num_ranges =
-              line_table->GetContiguousFileAddressRanges(file_ranges, append);
-          for (uint32_t idx = 0; idx < num_ranges; ++idx) {
-            const LineTable::FileAddressRanges::Entry &range =
-                file_ranges.GetEntryRef(idx);
-            debug_aranges->AppendRange(cu_offset, range.GetRangeBase(),
-                                       range.GetRangeEnd());
-          }
-        }
-      } else
+      if (debug_map_sym_file) {
+        auto *cu_info =
+            debug_map_sym_file->GetCompileUnitInfo(&GetSymbolFileDWARF());
+        // If there are extra compile units the OSO entries aren't a reliable
+        // source of information.
+        if (cu_info->compile_units_sps.empty())
           debug_map_sym_file->AddOSOARanges(&m_dwarf, debug_aranges);
       }
     }
+  }
 
   if (debug_aranges->GetNumRanges() == num_debug_aranges) {
     // We got nothing from the functions, maybe we have a line tables only
     // situation. Check the line tables and build the arange table from this.
     SymbolContext sc;
     sc.comp_unit = m_dwarf.GetCompUnitForDWARFCompUnit(*this);
     if (sc.comp_unit) {
       if (LineTable *line_table = sc.comp_unit->GetLineTable()) {

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

 
 lldb::CompUnitSP SymbolFileDWARF::ParseCompileUnit(DWARFCompileUnit &dwarf_cu) {
   CompUnitSP cu_sp;
   CompileUnit *comp_unit = (CompileUnit *)dwarf_cu.GetUserData();
   if (comp_unit) {
     // We already parsed this compile unit, had out a shared pointer to it
     cu_sp = comp_unit->shared_from_this();
   } else {
-    if (dwarf_cu.GetOffset() == 0 && GetDebugMapSymfile()) {
+    if (GetDebugMapSymfile()) {
       // Let the debug map create the compile unit
-      cu_sp = m_debug_map_symfile->GetCompileUnit(this);
+      cu_sp = m_debug_map_symfile->GetCompileUnit(this, dwarf_cu);
       dwarf_cu.SetUserData(cu_sp.get());
     } else {
       ModuleSP module_sp(m_objfile_sp->GetModule());
       if (module_sp) {
         auto initialize_cu = [&](const FileSpec &file_spec,
                                  LanguageType cu_language) {
           BuildCuTranslationTable();
           cu_sp = std::make_shared<CompileUnit>(

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h

 #include "llvm/Support/Chrono.h"
 #include <bitset>
 #include <map>
 #include <vector>
 
 #include "UniqueDWARFASTType.h"
 
 class SymbolFileDWARF;
+class DWARFCompileUnit;
 class DWARFDebugAranges;
 class DWARFDeclContext;
 
 class SymbolFileDWARFDebugMap : public lldb_private::SymbolFileCommon {
   /// LLVM RTTI support.
   static char ID;
 
 public:
protected:
 
   // Class specific types
   struct CompileUnitInfo {
     lldb_private::FileSpec so_file;
     lldb_private::ConstString oso_path;
     llvm::sys::TimePoint<> oso_mod_time;
     lldb_private::Status oso_load_error;
     OSOInfoSP oso_sp;
-    lldb::CompUnitSP compile_unit_sp;
+    /// The compile units that an object file contains.
+    llvm::SmallVector<lldb::CompUnitSP, 2> compile_units_sps;
+    /// A map from the compile unit ID to its index in the vector.
+    llvm::SmallDenseMap<uint64_t, uint64_t, 2> id_to_index_map;
     uint32_t first_symbol_index = UINT32_MAX;
     uint32_t last_symbol_index = UINT32_MAX;
     uint32_t first_symbol_id = UINT32_MAX;
     uint32_t last_symbol_id = UINT32_MAX;
     FileRangeMap file_range_map;
     bool file_range_map_valid = false;
 
-    CompileUnitInfo()
-        : so_file(), oso_path(), oso_mod_time(), oso_sp(), compile_unit_sp(),
-
-          file_range_map() {}
+    CompileUnitInfo() = default;
 
     const FileRangeMap &GetFileRangeMap(SymbolFileDWARFDebugMap *exe_symfile);
   };
 
   // Protected Member Functions
   void InitOSO();
 
+  /// This function actually returns the number of object files, which may be
+  /// less than the actual number of compile units, since an object file may
+  /// contain more than one compile unit. SymbolFileDWARFDebugMap looks up the
+  /// number of compile units by reading the nlist symbol table, which
+  /// currently, on macOS, only reports one compile unit per object file, and
+  /// there's no efficient way to calculate the actual number of compile units
+  /// upfront.
   uint32_t CalculateNumCompileUnits() override;
+
+  /// This function actually returns the first compile unit the object file at
+  /// the given index contains.
   lldb::CompUnitSP ParseCompileUnitAtIndex(uint32_t index) override;
 
   static uint32_t GetOSOIndexFromUserID(lldb::user_id_t uid) {
     return (uint32_t)((uid >> 32ull) - 1ull);
   }
 
   static SymbolFileDWARF *GetSymbolFileAsSymbolFileDWARF(SymbolFile *sym_file);
 
void PrivateFindGlobalVariables(
       lldb_private::ConstString name,
       const lldb_private::CompilerDeclContext &parent_decl_ctx,
       const std::vector<uint32_t> &name_symbol_indexes, uint32_t max_matches,
       lldb_private::VariableList &variables);
 
   void SetCompileUnit(SymbolFileDWARF *oso_dwarf,
                       const lldb::CompUnitSP &cu_sp);
 
-  lldb::CompUnitSP GetCompileUnit(SymbolFileDWARF *oso_dwarf);
+  /// Returns the compile unit associated with the dwarf compile unit. This may
+  /// be one of the extra compile units an object file contains which isn't
+  /// reachable by ParseCompileUnitAtIndex(uint32_t).
+  lldb::CompUnitSP GetCompileUnit(SymbolFileDWARF *oso_dwarf,
+                                  DWARFCompileUnit &dwarf_cu);
 
   CompileUnitInfo *GetCompileUnitInfo(SymbolFileDWARF *oso_dwarf);
 
   lldb::TypeSP
   FindDefinitionTypeForDWARFDeclContext(const DWARFDeclContext &die_decl_ctx);
 
   bool Supports_DW_AT_APPLE_objc_complete_type(SymbolFileDWARF *skip_dwarf_oso);
 

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

 //===-- SymbolFileDWARFDebugMap.cpp ---------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "SymbolFileDWARFDebugMap.h"
+#include "DWARFCompileUnit.h"
 #include "DWARFDebugAranges.h"
+#include "DWARFDebugInfo.h"
 
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleList.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/Section.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Utility/RangeMap.h"
 #include "lldb/Utility/RegularExpression.h"
 #include "lldb/Utility/Timer.h"
+#include "lldb/Utility/StreamString.h"
 
 //#define DEBUG_OSO_DMAP // DO NOT CHECKIN WITH THIS NOT COMMENTED OUT
 #if defined(DEBUG_OSO_DMAP)
 #include "lldb/Core/StreamFile.h"
 #endif
 
 #include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/LineTable.h"
uint32_t SymbolFileDWARFDebugMap::CalculateNumCompileUnits() {
   return m_compile_unit_infos.size();
 }
 
 CompUnitSP SymbolFileDWARFDebugMap::ParseCompileUnitAtIndex(uint32_t cu_idx) {
   CompUnitSP comp_unit_sp;
   const uint32_t cu_count = GetNumCompileUnits();
 
   if (cu_idx < cu_count) {
-    Module *oso_module = GetModuleByCompUnitInfo(&m_compile_unit_infos[cu_idx]);
+    auto &cu_info = m_compile_unit_infos[cu_idx];
+    Module *oso_module = GetModuleByCompUnitInfo(&cu_info);
     if (oso_module) {
       FileSpec so_file_spec;
       if (GetFileSpecForSO(cu_idx, so_file_spec)) {
         // User zero as the ID to match the compile unit at offset zero in each
-        // .o file since each .o file can only have one compile unit for now.
+        // .o file.
         lldb::user_id_t cu_id = 0;
-        m_compile_unit_infos[cu_idx].compile_unit_sp =
+        cu_info.compile_units_sps.push_back(
             std::make_shared<CompileUnit>(
                 m_objfile_sp->GetModule(), nullptr, so_file_spec, cu_id,
-                eLanguageTypeUnknown, eLazyBoolCalculate);
-
-        if (m_compile_unit_infos[cu_idx].compile_unit_sp) {
-          SetCompileUnitAtIndex(cu_idx,
-                                m_compile_unit_infos[cu_idx].compile_unit_sp);
+                eLanguageTypeUnknown, eLazyBoolCalculate));
+        cu_info.id_to_index_map.insert({0, 0});
+        SetCompileUnitAtIndex(cu_idx, cu_info.compile_units_sps[0]);
+        // If there's a symbol file also register all the extra compile units.
+        if (SymbolFileDWARF *oso_symfile =
+                GetSymbolFileByCompUnitInfo(&cu_info)) {
+          auto num_dwarf_units = oso_symfile->DebugInfo().GetNumUnits();
+          for (size_t i = 0; i < num_dwarf_units; ++i) {
+            auto *dwarf_unit = oso_symfile->DebugInfo().GetUnitAtIndex(i);
+            if (auto *dwarf_cu = llvm::dyn_cast<DWARFCompileUnit>(dwarf_unit)) {
+              // The "main" one was already registered.
+              if (dwarf_cu->GetID() == 0)
+                continue;
+              cu_info.compile_units_sps.push_back(std::make_shared<CompileUnit>(
+                  m_objfile_sp->GetModule(), nullptr, so_file_spec,
+                  dwarf_cu->GetID(), eLanguageTypeUnknown, eLazyBoolCalculate));
+              cu_info.id_to_index_map.insert(
+                  {dwarf_cu->GetID(), cu_info.compile_units_sps.size() - 1});
+            }
           }
         }
       }
-    comp_unit_sp = m_compile_unit_infos[cu_idx].compile_unit_sp;
+    }
+    if (!cu_info.compile_units_sps.empty())
+      comp_unit_sp = cu_info.compile_units_sps[0];
   }
 
   return comp_unit_sp;
 }
 
 SymbolFileDWARFDebugMap::CompileUnitInfo *
 SymbolFileDWARFDebugMap::GetCompUnitInfo(const SymbolContext &sc) {
   return GetCompUnitInfo(*sc.comp_unit);
 }
 
 SymbolFileDWARFDebugMap::CompileUnitInfo *
 SymbolFileDWARFDebugMap::GetCompUnitInfo(const CompileUnit &comp_unit) {
   const uint32_t cu_count = GetNumCompileUnits();
   for (uint32_t i = 0; i < cu_count; ++i) {
-    if (&comp_unit == m_compile_unit_infos[i].compile_unit_sp.get())
+    auto &id_to_index_map = m_compile_unit_infos[i].id_to_index_map;
+
+    auto it = id_to_index_map.find(comp_unit.GetID());
+    if (it != id_to_index_map.end() &&
+        &comp_unit ==
+            m_compile_unit_infos[i].compile_units_sps[it->getSecond()].get())
       return &m_compile_unit_infos[i];
   }
   return nullptr;
 }
 
 size_t SymbolFileDWARFDebugMap::GetCompUnitInfosForModule(
     const lldb_private::Module *module,
     std::vector<CompileUnitInfo *> &cu_infos) {
   const uint32_t cu_count = GetNumCompileUnits();
   for (uint32_t i = 0; i < cu_count; ++i) {
     if (module == GetModuleByCompUnitInfo(&m_compile_unit_infos[i]))
       cu_infos.push_back(&m_compile_unit_infos[i]);
ForEachSymbolFile([&s](SymbolFileDWARF *oso_dwarf) -> bool {
     // The underlying assumption is that DumpClangAST(...) will obtain the
     // AST from the underlying TypeSystem and therefore we only need to do
     // this once and can stop after the first iteration hence we return true.
     return true;
   });
 }
 
 lldb::CompUnitSP
-SymbolFileDWARFDebugMap::GetCompileUnit(SymbolFileDWARF *oso_dwarf) {
+SymbolFileDWARFDebugMap::GetCompileUnit(SymbolFileDWARF *oso_dwarf, DWARFCompileUnit &dwarf_cu) {
   if (oso_dwarf) {
     const uint32_t cu_count = GetNumCompileUnits();
     for (uint32_t cu_idx = 0; cu_idx < cu_count; ++cu_idx) {
       SymbolFileDWARF *oso_symfile =
           GetSymbolFileByCompUnitInfo(&m_compile_unit_infos[cu_idx]);
       if (oso_symfile == oso_dwarf) {
-        if (!m_compile_unit_infos[cu_idx].compile_unit_sp)
-          m_compile_unit_infos[cu_idx].compile_unit_sp =
-              ParseCompileUnitAtIndex(cu_idx);
+        if (m_compile_unit_infos[cu_idx].compile_units_sps.empty())
 
-        return m_compile_unit_infos[cu_idx].compile_unit_sp;
+        auto &id_to_index_map = m_compile_unit_infos[cu_idx].id_to_index_map;
+        auto it = id_to_index_map.find(dwarf_cu.GetID());
+        if (it != id_to_index_map.end())
+          return m_compile_unit_infos[cu_idx]
+              .compile_units_sps[it->getSecond()];
       }
     }
   }
   llvm_unreachable("this shouldn't happen");
 }
 
 SymbolFileDWARFDebugMap::CompileUnitInfo *
 SymbolFileDWARFDebugMap::GetCompileUnitInfo(SymbolFileDWARF *oso_dwarf) {

 void SymbolFileDWARFDebugMap::SetCompileUnit(SymbolFileDWARF *oso_dwarf,
                                              const CompUnitSP &cu_sp) {
   if (oso_dwarf) {
     const uint32_t cu_count = GetNumCompileUnits();
     for (uint32_t cu_idx = 0; cu_idx < cu_count; ++cu_idx) {
       SymbolFileDWARF *oso_symfile =
           GetSymbolFileByCompUnitInfo(&m_compile_unit_infos[cu_idx]);
       if (oso_symfile == oso_dwarf) {
-        if (m_compile_unit_infos[cu_idx].compile_unit_sp) {
-          assert(m_compile_unit_infos[cu_idx].compile_unit_sp.get() ==
+        if (!m_compile_unit_infos[cu_idx].compile_units_sps.empty()) {
+          assert(m_compile_unit_infos[cu_idx].compile_units_sps[0].get() ==
                  cu_sp.get());
         } else {
-          m_compile_unit_infos[cu_idx].compile_unit_sp = cu_sp;
+          assert(cu_sp->GetID() == 0 &&
+                 "Setting first compile unit but with id different than 0!");
+          auto &compile_units_sps = m_compile_unit_infos[cu_idx].compile_units_sps;
+          compile_units_sps.push_back(cu_sp);
+          m_compile_unit_infos[cu_idx].id_to_index_map.insert(
+              {cu_sp->GetID(), compile_units_sps.size() - 1});
+
           SetCompileUnitAtIndex(cu_idx, cu_sp);
         }
       }
     }
   }
 }
 
 CompilerDeclContext

lldb/test/API/lang/c/full_lto_stepping/Makefile

+C_SOURCES := main.c foo.c
+CFLAGS_EXTRAS = -flto=full -Wl,-object_path_lto -Wl,$(BUILDDIR)/lto.o
+
+include Makefile.rules
 No newline at end of file

lldb/test/API/lang/c/full_lto_stepping/TestFullLtoStepping.py

+"""Test that stepping in object files with multiple compile units works."""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+import lldbsuite.test.lldbutil as lldbutil
+
+
+class TestFullLtoStepping(TestBase):
+
+    @skipIf(compiler=no_match("clang"))
+    @skipUnlessDarwin
+    def test(self):
+        self.build()
+        target = self.createTestTarget()
+    
+        breakpoint = target.BreakpointCreateByName("main")
+        self.assertTrue(
+            breakpoint and breakpoint.IsValid(),
+            "Breakpoint is valid")
+
+        _, _, thread, _ = lldbutil.run_to_breakpoint_do_run(self, target, breakpoint)
+        name = thread.frames[0].GetFunctionName()
+        # Check that we start out in main.
+        self.assertEqual(name, 'main')
+        thread.StepInto()
+        name = thread.frames[0].GetFunctionName()
+        # Check that we stepped into f.
+        self.assertEqual(name, 'f')

lldb/test/API/lang/c/full_lto_stepping/foo.h

+int f(void);

lldb/test/API/lang/c/full_lto_stepping/foo.c

+int f() {
+  return 42;
+}

lldb/test/API/lang/c/full_lto_stepping/main.c

+#include "foo.h"
+
+int main(void) {
+  int i = f();
+  return 0;
+}