Page MenuHomePhabricator

[lldb/DWARF] Don't hold a unique SymbolFileDWARFDwo in a DWARFUnit
ClosedPublic

Authored by labath on Jan 31 2020, 6:08 AM.

Details

Summary

This is the second dwp preparatory patch. When a SymbolFileDWARFDwo will
hold more than one split unit, it will not be able to be uniquely owned
by a single DWARFUnit. I achieve this by changing the
unique_ptr<SymbolFileDWARFDwo> member of DWARFUnit to
shared_ptr<DWARFUnit>. The shared_ptr points to a DWARFUnit, but it is
in fact holding the entire SymbolFileDWARFDwo alive. This is the same
method used by llvm DWARFUnit (except that is uses the DWARFContext
class).

Diff Detail

Event Timeline

labath created this revision.Jan 31 2020, 6:08 AM
Herald added a project: Restricted Project. · View Herald Transcript
clayborg added inline comments.Jan 31 2020, 10:35 AM
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
56

How often does this function get called? Should we cache the DW_AT_GNU_dwo_id in the DWARFCompileUnit to avoid extracting the DW_AT_GNU_dwo_id attribute maybe multiple times?

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
67

Curious: what is a single compile unit? A bit of a comment in header doc here might be nice.

labath marked 4 inline comments as done.Feb 6 2020, 8:27 AM
labath added inline comments.
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
56

In a dwo file, this function will be called exactly once. With a DWP file (the next patch) it will get called once for each compile unit, but it will return a different compile unit each time, so for a single compile unit, the dwo_id attribute will still be accessed only once.

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
67

I've added a simple comment. The idea is that a dwo file will normally(*) contain exactly one compile unit. This function will find it and return it.

(*) This may not be 100% as I believe llvm can produce multiple CUs in a dwo file under some circumstances, but I'm not sure if this is fully conforming, and it is definitely not something that works in lldb right now.

labath updated this revision to Diff 242915.Feb 6 2020, 8:28 AM
labath marked 2 inline comments as done.
  • clang-format
  • add some doxygen

Do you have any more comments on this patch, Greg?

clayborg accepted this revision.Feb 13 2020, 8:09 AM

LGTM, sorry for the delay!

This revision is now accepted and ready to land.Feb 13 2020, 8:09 AM
This revision was automatically updated to reflect the committed changes.