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

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

kimanh added inline comments.


================
Comment at: llvm/test/tools/llvm-dwp/X86/rnglists.s:62-76
+	.asciz	"y_"                            # string offset=9
+	.section	.debug_abbrev.dwo,"e", at progbits
+	.byte	1                              # Abbreviation Code
+	.byte	17                              # DW_TAG_compile_unit
+	.byte	1                               # DW_CHILDREN_yes
+	.byte	37                              # DW_AT_producer
+	.byte	37                              # DW_FORM_strx1
----------------
dblaikie wrote:
> Strange to have a complicated abbreviation that's not actually used - maybe if this abbrev was simplified to compile_unit, CHILDREN_no, with no attributes - then the TU/CU could be a bit more valid by only including that one DIE without any attributes/children - rather than missing their unit DIE entirely.
> 
> (I guess this was tested this way because something in llvm-dwp ended up requiring that the CU had a valid DIE? Maybe we could fix that so there's no need for there to be that empty/degenerate DIE in the CU? (so it can 'work' the same as the TU - without any DIEs at all))
I have simplified the abbrev section now, as you suggested: With CHILDREN_no, and no attributes, and removed the debug_str section that it was referring to too.

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101894