Page MenuHomePhabricator

Move the rest of the section loading over to DWARFContext
ClosedPublic

Authored by zturner on Mar 20 2019, 1:56 PM.

Details

Summary

This gets all of the remaining sections except those that are DWO-dependent. That will be a bit harder to do since we'll somehow have to get knowledge of DWO-ness into the DWARFContext.

This is mostly mechanical now after D59562.

Diff Detail

Repository
rL LLVM

Event Timeline

zturner created this revision.Mar 20 2019, 1:56 PM
clayborg requested changes to this revision.Mar 20 2019, 2:06 PM

Use reference to DWARFContext instead of pointers? Apply to entire patch and good to go

This revision now requires changes to proceed.Mar 20 2019, 2:06 PM

Use reference to DWARFContext instead of pointers? Apply to entire patch and good to go

Done. Since you conditionally LGTM'ed it based on that and I made that change I'll go ahead and submit

This revision was not accepted when it landed; it landed in state Needs Revision.Mar 21 2019, 9:33 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMar 21 2019, 9:33 AM

This broke the DWO flavours of some tests:

lldb-Suite :: lang/c/const_variables/TestConstVariables.py
lldb-Suite :: lang/c/local_variables/TestLocalVariables.py
lldb-Suite :: lang/c/vla/TestVLA.py

I believe the problem is that DWARFContext::LoadOrGetSection does not take DWO files into account. It was modelled based on SymbolFileDWARF's notion of loading sections, but you probably missed the fact that SymbolFileDWARFDwo overrides LoadSectionData to load the data from a different object file https://github.com/llvm-mirror/lldb/blob/master/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp#L32. The DWARFContext does not have that, and so it always loads the sections from the main module object file, which results in things going south.

I think the fix for this should be relatively simple. We just need to give SymbolFileDWARFDwo the opportunity to initialize the DWARFContext with a different set of sections. However, since this will require changing it's constructor to not accept a Module object (since dwo files do not have one -- the next best thing might be to take a SectionList), and change the DWARFContext ownership model (so that subclasses can customize it's initialization), I did not want to try make a hasty unreviewed fix for that. Instead I just reverted this patch for now. (The problem was kind of already introduced by the previous patch in this series, but I believe that one is fine to keep because there is no DWO flavour of debug_aranges).

Let me know if you have trouble reproducing these failures on your side.