In DWARF v4 compile units go in .debug_info and type units go in
.debug_types. However, in v5 both kinds of units are in .debug_info.
Therefore we can't decide whether to use the CU or TU index just by
looking at which section we're reading from. We have to wait until we
have read the unit type from the header.
Details
Diff Detail
Event Timeline
Nice catch. Regarding the implementation, I think it might be slightly cleaner (and slightly more consistent with the llvm dwarf parser) if the index handling has moved to the DWARFUnit class (as there's nothing to be gained now by doing it higher up). In DWARFUnit::extract, you could do something like:
if (dwarf.GetDWARFContext().isDWO()) { cu_index = &dwarf.GetDWARFContext().GetAsLLVM().getCUIndex(); ...
lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp | ||
---|---|---|
808–809 | I guess this could be header.IsTypeUnit() (and !header.IsTypeUnit()))... |
lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp | ||
---|---|---|
808–809 | But !header.IsTypeUnit would also treat DW_UT_partial and DW_UT_skeleton as compile units, right? |
lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp | ||
---|---|---|
808–809 | That's true, but can either of those units legitimately appear in a dwp file? I took this idea from the equivalent llvm code: https://github.com/llvm/llvm-project/blob/main/llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp#L81 |
Changed logic that checked the type of unit to use header.IsTypeUnit() as suggested by reviewer.
I'm going to go ahead and commit this given that I have just made the last suggested modification and the patch has already been up for review without further comments for a long while (sorry for the late replies, work keeps getting in the way of work).
lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp | ||
---|---|---|
808–809 | Good point. Changed. |
I guess this could be header.IsTypeUnit() (and !header.IsTypeUnit()))...