[PATCH] D101894: [llvm-dwp] Add support for rnglists and loclists

[Kim-Anh Tran via Phabricator via llvm-commits]

kimanh added a comment.

In D101894#2745302 <https://reviews.llvm.org/D101894#2745302>, @dblaikie wrote:

>> Yes exactly, for CU units there are "CUIdentifiers" (Signature, name, dwo id) that are recorded for error purposes, if a duplicate CU is detected. Before extracting these attributes there is a check whether this unit is actually is a CU unit or not, so we need the abbrev section to at least have info on the unit type. This check does currently not exist for TU units (for DWARF5 this will be checked using the unit type field, for DWARF4 type units this is however not checked at the moment). I think the check if it's a compile unit should stay where it is (given that we need to access the attributes for error purposes afterwards), so I'm not sure how we should change the code to allow for no DIEs at all.
>
> Ah, right right, I remember adding that - potentially that code could be refactored to not try to parse the name until the dwo ID collision occurs - then these tests wouldn't need any DIEs, for instance (which isn't, in and of itself, a valuable thing - but does show that usually the content of the DIE tree is not examined/used/relevant to the DWP tool, only a "nice to have" for error messages).

I had another look at the code: not all of the information that is used in the CUIdentifiers is actually needed at that point in time (although it is set, but it is set to null). The only piece that definitely is required is the signature of compile units if parsing DWARFv4, since the signature is used to build the index. The signature is taken from the .debug_abbrev section, so the check if it's actually a compile unit or not should stay (at least for v4), so I tend towards keeping it this way. WDYT?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101894/new/

https://reviews.llvm.org/D101894