Page MenuHomePhabricator

General cleanup to minimize the .debug_types patch
ClosedPublic

Authored by clayborg on May 8 2018, 3:08 PM.

Details

Summary

This cleanup is designed to make the https://reviews.llvm.org/D32167 patch smaller and easier to read.

Cleanup in this patch:

Allow DWARFUnit subclasses to hand out the data that should be used when decoding data for a DIE. The information might be in .debug_info or could be in .debug_types. There is a new virtual function on DWARFUnit that each subclass must override:

virtual const lldb_private::DWARFDataExtractor &DWARFUnit::GetData() const;

This allows DWARFCompileUnit and eventually DWARFTypeUnit to hand out different data to be used when decoding the DIE information.

Add a new pure virtual function to get the size of the DWARF unit header:

virtual uint32_t DWARFUnit::GetHeaderByteSize() const = 0;

This allows DWARFCompileUnit and eventually DWARFTypeUnit to hand out different offsets where the first DIE starts when decoding DIE information from the unit.

Added a new function to DWARFDataExtractor to get the size of an offset:

size_t DWARFDataExtractor::GetDWARFSizeOfOffset() const;

Removed dead dumping and parsing code in the DWARFDebugInfo class.
Inlined a bunch of calls in DWARFUnit for accessors that were just returning integer member variables.
Renamed DWARFUnit::Size() to DWARFUnit::GetHeaderByteSize() as it clearly states what it is doing and makes more sense.

Diff Detail

Repository
rL LLVM

Event Timeline

clayborg created this revision.May 8 2018, 3:08 PM

This looks fine to me. I am not clicking accept yet, so other debug info folks can take a look at this too.

source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp
28 ↗(On Diff #145794)

This is dead code. new will abort the program if it cannot allocate memory for the object.

clayborg updated this revision to Diff 145926.May 9 2018, 8:17 AM

Remove check for NULL cu_sp shared pointer as Pavel noted.

aprantl accepted this revision.May 9 2018, 8:49 AM

Looks like this is bringing the interface a *little* closer to the llvm one.

source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
40 ↗(On Diff #145926)

Looking at the LLVM implementation, this is only correct fro DWARF 4.
Out of curiosity, do we have any plans to ever support DWARF 5 in LLDB's DWARF parser or are we planning to get that from moving to the LLVM implementation? (I'd clearly prefer the second option).

This revision is now accepted and ready to land.May 9 2018, 8:49 AM
This revision was automatically updated to reflect the committed changes.