[PATCH] D45170: Cleanup DWARFCompileUnit and DWARFUnit in preparation for adding DWARFTypeUnit

Tue Apr 10 15:22:45 PDT 2018

> On Apr 10, 2018, at 2:38 PM, Jan Kratochvil <jan.kratochvil at redhat.com> wrote:
> 
> On Tue, 10 Apr 2018 17:46:55 +0200, Greg Clayton wrote:
>>> On Apr 7, 2018, at 2:04 AM, Jan Kratochvil <jan.kratochvil at redhat.com> wrote:
>>> DW_TAG_partial_unit gets read in (by
>>> DWARFDebugInfo::ParseCompileUnitHeadersIfNeeded) as DWARFCompileUnit because
>>> there is no quick enough way to find the difference.  It would require reading
>>> the first DIE tag which means to read and decode .debug_abbrev for each unit
>>> being scanned.
>> 
>> If there is no structural difference other than the first DW_TAG, is there
>> a reason you would need to know the difference?
> 
> Not really, it also currently does work with DWARFCompileUnit.
> Just one could assert that nobody tries to use DIEs directly from that
> DWARFCompileUnit without their remapping to DWARFPartialUnit.

If it come down to really just getting a unique and lldb::user_id_t, there are probably easier ways. I always try to leave the DWARFCompileUnit and those level classes out of the loop if possible so no changes are needed for them when possible. Sounds like from what you said below there aren't any new methods needed on DWARFCompileUnit for it to be used as a DWARFPartialUnit and hopefully DWARFPartialUnit won't need to exist if we do things correctly. More details below...

> 
>> It would be fine to just add extra methods on DWARFCompileUnit that do
>> partial unit kind of things if it is a partial unit?
> 
> There are not really any extra methods.
> 
> 
>>> DWARFPartialUnit is used only as a remapping of DWARFCompileUnit to a new
>>> offset without any new data (there is stored only a new remapped offset whose
>>> value is only made up internally in LLDB) at the moment someone uses
>>> DW_TAG_imported_unit for it - at that moment we easily know that unit has to
>>> be DW_TAG_partial_unit.
>> 
>> It is remapped to a new offset just to keep LLDB's lldb::user_id_t stuff
>> happy?
> 
> Yes.
> 
>> A partial unit can refer to an external file on disk. The remapping is
>> solely making a unique offset by adding an offset to the offset of the
>> external file?
> 
> Not only that. Even when DWZ does not use any external file then one
> DW_TAG_partial_unit is included into many DW_TAG_compile_unit and so DIEs from
> that DW_TAG_partial_unit would no longer have unique lldb::user_id_t.
> (Although maybe that can be solved by that AST import which I haven't tried.)

Why would we need to inline anything if the partial unit is along side the same compile units? Why wouldn't you just use DW_FORM_ref_addr to refer directly to any DIEs in a partial unit from a compile unit? I must be missing something. You you walk me through creating this kind of scenario? I have the multiple files that refer to an external file with one or more DW_TAG_partial_unit tags (or is there only ever just one DW_TAG_partial_unit?). But I would like to see this inlined DW_TAG_partial_unit along side DWARF. Why would and compile unit just not refer directly to the DIEs? They shouldn't have to import anything?
> 
> 
> 
>>> Therefore DWARFCompileUnit and DWARFTypeUnit both contain some their own data.
>>> But DWARFPartialUnit is just a remapping of DWARFCompileUnit (containing
>>> DW_TAG_partial_unit) to a new offset without any new data. Particularly
>>> m_die_array is not in DWARFPartialUnit.
>> 
>> In reading the DWZ it sounded like you can only have 1 external debug info
>> file and that external debug info file can't itself refer to another?
> 
> Right.
> 
>> Is this what the DWARFPartialUnit would be for and this is the only
>> remapping that needs to happen?
> 
> One also needs to remap all the uses of DW_TAG_partial_unit, which is even
> used recursively (one DW_TAG_partial_unit uses DW_TAG_imported_unit for
> another DW_TAG_partial_unit etc.) otherwise there happen ambiguous
> lldb::user_id_t again.

If this is all in the same file, then the offsets are all in the .debug_info? What am I missing?
> 
> 
>>> (1) Your DWARFTypeUnit patch - it makes it more aligned to LLVM DWARFUnits.
>>> (2) My DWZ patch - it is a new feature with no counterpart in LLVM DWARFUnits.
>>> (3) Replacement of LLDB DWARFUnits with LLVM DWARFUnits.
> ...
>> We need to solve (1) first so we can move onto (2). (3) can wait IMHO, but
>> if someone really wants to tackle that it is fine.
> 
> Great, thanks. So I would like to somehow adjust (1) so that (2) can be built
> on top of it (and not having undo it or create similar parallel
> functionality).
> 
> 
>> I think I need to see any example of this DWZ so I can intelligently comment
>> on thing. The brief description of DWZ at
>> http://www.dwarfstd.org/ShowIssue.php?issue=120604.1&type=open
>> <http://www.dwarfstd.org/ShowIssue.php?issue=120604.1&type=open> doesn't
>> help me see how this is actually laid out on disk and who refers to who and
>> how things are laid out in the DWARF itself.
> 
> You said in another mail you already have DWZ sample.  That dwarfstd reference
> above I have commented at:
> 	https://reviews.llvm.org/D32167#1063666
> 
> 
> Thanks,
> Jan