[Lldb-commits] [PATCH] D136114: [lldb] Allow SymbolFileDWARFDebugMap to register multiple compile units

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

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?



================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp:55
 
-  if (debug_aranges->GetNumRanges() == num_debug_aranges) {
+  if (debug_aranges->GetNumRanges() == num_debug_aranges && cu_offset == 0) {
     // We got nothing from the debug info, maybe we have a line tables only
----------------
labath wrote:
> 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).
Maybe it's better to filter out skeleton units specifically instead? (If that's what the condition is for)


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp:644
+       if (&comp_unit == extra.get())
+         return &m_compile_unit_infos[i];
   }
----------------
is std::find() shorter here?


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp:1276
 
-        return m_compile_unit_infos[cu_idx].compile_unit_sp;
+        for (auto &cu : m_compile_unit_infos[cu_idx].extra_compile_units_sps)
+          if (cu->GetID() == dwarf_cu.GetID())
----------------
is std::find() shorter / more readable?


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h:180
     lldb::CompUnitSP compile_unit_sp;
+    // The rest of the compile units that an object file contains.
+    llvm::SmallVector<lldb::CompUnitSP, 1> extra_compile_units_sps;
----------------
nit: convert these to `///`


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h:181
+    // The rest of the compile units that an object file contains.
+    llvm::SmallVector<lldb::CompUnitSP, 1> extra_compile_units_sps;
     uint32_t first_symbol_index = UINT32_MAX;
----------------
Why not only store a llvm::SmallVector<lldb::CompUnitSP, 1> and use the first element to store compile_unit_sp?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136114/new/

https://reviews.llvm.org/D136114