ok, so I was able to create a DWZ example and see how this is laid out and I 
now understand the format. I will see if I can modify this patch in a way that 
will work for DWZ and for .debug_types. 


> On Apr 10, 2018, at 8:46 AM, Greg Clayton <clayb...@gmail.com> wrote:
> 
> 
> 
>> On Apr 7, 2018, at 2:04 AM, Jan Kratochvil <jan.kratoch...@redhat.com 
>> <mailto:jan.kratoch...@redhat.com>> wrote:
>> 
>> On Sat, 07 Apr 2018 00:52:26 +0200, Greg Clayton wrote:
>>> Take look at how LLVM does it. I believe my changes mirror that approach.
>> 
>> LLVM does not support partial units so there is nothing to look at there.
>> 
>> 
>>> DWARFUnit should be the base class for anything that needs to hand out DIEs.
>> 
>> That's OK for partial units.
>> 
>> 
>>> Any specializations should be inheriting from DWARFUnit, like both
>>> DWARFCompileUnit and DWARFTypeUnit.
>> 
>> I see now the source of misunderstanding:
>> 
>> DWARFCompileUnit = DW_TAG_compile_unit
>> DWARFTypeUnit = DW_TAG_type_unit
>> DWARFPartialUnit != DW_TAG_partial_unit
>>                ^^^^
>> BTW DW_TAG_imported_unit is importing (="caller") tag, not a unit 
>> (="callee").
>> 
>> 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? It would be fine to just add 
> extra methods on DWARFCompileUnit that do partial unit kind of things if it 
> is a partial unit? Let me know if I am missing something?
> 
>> 
>> 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? 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?
> 
>> 
>> 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? Is this 
> what the DWARFPartialUnit would be for and this is the only remapping that 
> needs to happen?
> 
>> 
>> DWARFTypeUnit can be recognized easily as it is either in .debug_types
>> (<=DWARF-4) or the unit header contains DW_UT_type (>=DWARF-5).
>> DWARFPartialUnit (for DW_TAG_partial_unit) cannot be recognized easily first.
>> Besides that one would need then some DWARFRemappedPartialUnit for what I use
>> DWARFPartialUnit now.
>> 
>> I have implemented it according to your advice from this mail - at least
>> according to how I understood it:
>>      [lldb-dev] RFC for DWZ = DW_TAG_imported_unit + DWARF-5 supplementary 
>> files
>>      https://lists.llvm.org/pipermail/lldb-dev/2017-August/012664.html 
>> <https://lists.llvm.org/pipermail/lldb-dev/2017-August/012664.html>
>> 
>> It does not try to share anything at AST level, it only shares DWARF data 
>> (and
>> thus DWARFCompileUnit). Given that DWZ finds arbitrary unique DWARF subtrees
>> I find more logical to decode it at DWARF (and not at AST) level.
>> 
>> You wrote:
>> # The drawback is this won't allow sharing /tmp/shared1 or /tmp/shared2
>> # between two different top level DWARF files, but it does allow one
>> # clang::ASTContext to be used between all of them.
>> 
>> In my implementation /tmp/shared1
>> (/usr/lib/debug/.dwz/coreutils-8.27-20.fc27.x86_64) is shared between 
>> multiple
>> *.so files (which use DW_TAG_imported_unit) at DWARF level, also
>> clang::ASTContext is shared.
>> 
>> 
>> # SymbolFileDWARFDebugMap makes it lldb::user_id_t contain the CU index in 
>> the
>> # top 32 bits, and the DIE offset within that .o file's DWARF in the bottom 
>> 32
>> # bits. You could do something similar in your case where the top 32 bits is
>> # the index of the DWARF file in the "dwarf[]" array that would be maintained
>> # in a new SymbolFileDWARFDWZ subclass.
>> 
>> DW_TAG_imported_unit+DW_TAG_partial_unit can be also used for optimization of
>> a single file (without /usr/lib/debug/.dwz/* file which is used exclusively
>> for DW_TAG_partial_unit entries). Additionally the tags can be also used for
>> recursive inclusion. I haven't found how to use the top 32 bits for that.
>> I just reserve new remapped offset space for each DW_TAG_imported_unit (in 
>> the
>> bottom 32 bits).
>> 
>> I tried first to implement dw_offset_t caller (=unit with
>> DW_TAG_imported_unit) to be tracked along any dw_offset_t DIE offset but that
>> would require huge changes of the DWARF parsing code everywhere.  Also it
>> cannot work well given the inclusion is recursive (so we would need
>> std::vector<dw_offset_t> of the callers stack).
>> 
>> 
>> 
>>> I have no idea what are in your other patches
>> 
>> OK, so there was a gross misunderstanding and my DWARFUnit implemented
>> something very different from what you expected+approved. I am sure fine with
>> that but I needed to understand first why you did that big screw up of my
>> carefully coded patch.
>> 
>> 
>>>> This is how DWARFPartialUnit works, it is only a DWARFCompileUnit remapped 
>>>> to
>>>> new offset.  I do not see how to implement it transparently without the
>>>> accessor (and without needlessly copying all the data fields many times 
>>>> into
>>>> each DWARFPartialUnit instance).
>>> 
>>> What extra functions are needed for a partial unit that can't be done in
>>> a DWARFCompileUnit? Seems like they both contain things, but the partial
>>> unit can be referenced from compile units. 
>> 
>> DWARFPartialUnit is only a remapping, not really a representation of DWARF
>> file data.  So DWARFPartialUnit cannot contain its own m_die_array, m_version
>> and other data members you have moved back to DWARFUnit.
>> 
>> 
>>> As we are saying, we are trying to make the layering more like LLVM's
>>> layering, so that is what I meant by "fix the layering". I believe we should
>>> strive for being more like LLVM so that any transition can happen without
>>> major re-organization of the DWARF code later. So I would like the get the
>>> ok the revert the revert if you on board with what my suggestions are in the
>>> paragraph. I know this will require modifications to your patches and
>>> apologize for that.
>>> 
>>> Let me know what you think,
>> 
>> I would like to know what is the approved plan / upstreaming order regarding
>> all the planned changes:
>> 
>> (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.
>> 
>> I can sure work even on (3) but after half a year of work on DWZ support 
>> which
>> completely blocks LLDB for Red Hat usage (as Red Hat requires "upstream 
>> first"
>> to prevent heavy forks like what happened for Red Hat GDB) it makes the DWZ
>> upstreaming possibility too far for me to start refactoring LLDB for (3) 
>> first
>> - before upstreaming (2).
>> 
>> 
>> Thanks,
>> Jan
> 
> 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.
> 
> 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. Can you somehow make some simple 
> executables available to me so I can see the DWARF in the main compile unit, 
> the partial unit and the info in the external DWZ file? I know you know what 
> you have done and fully understand this feature, but I would like to take a 
> look at an example so I can see how we can make both the .debug_types and 
> your feature work using similar work arounds.
> 
> One idea that might work is to expand the DWARFDataExtractor so it can 
> contain enough information to create a lldb::user_id_t that would work for 
> .debug_info, .debug_types and for the alternate debug file. It would also 
> need to be able to get us to the right debug info when a SymbolFileDWARF API 
> was handed a lldb::user_id_t and be able to turn that back into something. 
> The current offset hack that was used for .debug_types could be used for this 
> purpose as well. But I really would like to see an example program that 
> refers to an external DWZ file so I can make sure what I recommend is viable.
> 
> Greg Clayton

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
          • Re: [Lldb... Jan Kratochvil via lldb-commits
            • Re: ... Davide Italiano via lldb-commits
            • Re: ... Greg Clayton via lldb-commits
              • ... Jan Kratochvil via lldb-commits
              • ... via lldb-commits
              • ... Greg Clayton via lldb-commits
              • ... Jan Kratochvil via lldb-commits
              • ... Pavel Labath via lldb-commits
              • ... Jan Kratochvil via lldb-commits
              • ... Greg Clayton via lldb-commits
              • ... Greg Clayton via lldb-commits
              • ... Jan Kratochvil via lldb-commits
              • ... Greg Clayton via lldb-commits
              • ... Jan Kratochvil via lldb-commits
              • ... Jan Kratochvil via lldb-commits
              • ... Jan Kratochvil via lldb-commits
              • ... Jan Kratochvil via lldb-commits
              • ... Adrian Prantl via lldb-commits
  • [Lldb-commits] [PATCH] D45... Jan Kratochvil via Phabricator via lldb-commits

Reply via email to