[llvm-dwp] Add support for rnglists and loclists

[kimanh]

Summary

This patch updates llvm-dwp to include rnglists and loclists
when parsing debug sections.

[kimanh]

Minor change: edit comment in test

[kimanh]

Minor: use wildcard in test.

[dblaikie]

Sounds OK - test might be able to be simplified a bit.

llvm/test/tools/llvm-dwp/X86/loclists.s

26–31

No need for actual .text in these files - these files are only dwo files, so they can have arbitrary hardcoded constants for the starting/ending offsets.

llvm/test/tools/llvm-dwp/X86/rnglists.s

63–77

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))

[kimanh]

Simplify tests:

• use hardocoded constants as offsets
• simplify abbrev section and remove str section

[kimanh]

llvm/test/tools/llvm-dwp/X86/rnglists.s

63–77

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.

[dblaikie]

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).

[kimanh]

In 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?

[dblaikie]

In D101894#2753779, @kimanh wrote:

In 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?

I don't think there's anything you /have/ to do here - my suggestion was more a "could be nice", sorry for the tone/confusion.

I think in principle it would be nice if we didn't parse stuff we didn't need to - yeah, that means in DWARFv4 we still need to parse the CU's main DIE to get the dwo_id. But in DWARFv5 we can get away with only reading the header - then if there's a duplicate dwo_id /then/ we could try to parse the DIE to get the dwo name, etc.

Just a nice to have, maybe, at some point. Though there are far bigger performance problems to deal with with llvm-dwp anyway, so not to worry. Sorry for the diversion.

[kimanh]

In D101894#2758200, @dblaikie wrote:

In D101894#2753779, @kimanh wrote:

In 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?

I don't think there's anything you /have/ to do here - my suggestion was more a "could be nice", sorry for the tone/confusion.

I think in principle it would be nice if we didn't parse stuff we didn't need to - yeah, that means in DWARFv4 we still need to parse the CU's main DIE to get the dwo_id. But in DWARFv5 we can get away with only reading the header - then if there's a duplicate dwo_id /then/ we could try to parse the DIE to get the dwo name, etc.

Just a nice to have, maybe, at some point. Though there are far bigger performance problems to deal with with llvm-dwp anyway, so not to worry. Sorry for the diversion.

Oh, no worries at all, and thanks for the clarification! :) Ok, I'll keep it this way then. Since this patch is dependent on the others I'll ping this thread again as soon as the parent patches are ready.

[kimanh]

Rebase and reupload to trigger premerge checks.

llvm/test/tools/llvm-dwp/X86/loclists.s

+# This test checks if llvm-dwp outputs .debug_loclists.
+
+# RUN: llvm-mc -triple x86_64-unknown-linux %s -filetype=obj -o %t.o \
+# RUN:         -split-dwarf-file=%t.dwo -dwarf-version=5
+# RUN: llvm-dwp %t.dwo -o %t.dwp
+# RUN: llvm-dwarfdump -debug-loclists -debug-cu-index -debug-tu-index %t.dwp | FileCheck %s
+
+# CHECK-DAG: .debug_loclists.dwo contents:
+# CHECK: locations list header: length = 0x00000019, format = DWARF32, version = 0x0005, addr_size = 0x08, seg_size = 0x00, offset_entry_count = 0x00000001
+# CHECK-NEXT: offsets: [
+# CHECK-NEXT: 0x00000004
+# CHECK-NEXT: ]
+# CHECK: DW_LLE_base_addressx   (0x0000000000000000)
+# CHECK-NEXT: DW_LLE_offset_pair     (0x0000000000000000, 0x0000000000000004): DW_OP_reg5 RDI
+# CHECK-NEXT: DW_LLE_offset_pair     (0x0000000000000004, 0x0000000000000008): DW_OP_reg3 RBX
+
+# CHECK-DAG: .debug_cu_index contents:
+# CHECK: Index Signature          INFO                     ABBREV                   LOCLISTS
+# CHECK:     1 {{.*}} [0x00000018, 0x0000002d) [0x00000000, 0x00000004) [0x00000000, 0x0000001d)
+
+# CHECK-DAG: .debug_tu_index contents:
+# CHECK: Index Signature          INFO                     ABBREV                   LOCLISTS
+# CHECK:     2 {{.*}} [0x00000000, 0x00000018) [0x00000000, 0x00000004) [0x00000000, 0x0000001d)
+
+  .section  .debug_info.dwo,"e",@progbits
+  .long .Ldebug_info_dwo_end0-.Ldebug_info_dwo_start0 # Length of Unit
+.Ldebug_info_dwo_start0:
+  .short  5                               # DWARF version number
+  .byte 6                               # DWARF Unit Type
+  .byte 8                               # Address Size (in bytes)
+  .long 0                               # Offset Into Abbrev. Section
+  .quad -4287463584810542331            # Type Signature
+  .long 31                              # Type DIE Offset
+.Ldebug_info_dwo_end0:
+  .section  .debug_info.dwo,"e",@progbits
+  .long .Ldebug_info_dwo_end3-.Ldebug_info_dwo_start3 # Length of Unit
+.Ldebug_info_dwo_start3:
+  .short  5                               # DWARF version number
+  .byte 5                               # DWARF Unit Type
+  .byte 8                               # Address Size (in bytes)
+  .long 0                               # Offset Into Abbrev. Section
+  .quad 1152943841751211454
+  .byte 1                              # Abbrev [1] 0x14:0x349 DW_TAG_compile_unit
+.Ldebug_info_dwo_end3:
+  .section  .debug_loclists.dwo,"e",@progbits
+  .long .Ldebug_list_header_end0-.Ldebug_list_header_start0 # Length
+.Ldebug_list_header_start0:
+  .short  5                               # Version
+  .byte 8                               # Address size
+  .byte 0                               # Segment selector size
+  .long 1                               # Offset entry count
+.Lloclists_table_base0:
+  .long .Ldebug_loc0-.Lloclists_table_base0
+.Ldebug_loc0:
+  .byte 1                               # DW_LLE_base_addressx
+  .byte 0                               #   base address index
+  .byte 4                               # DW_LLE_offset_pair
+  .uleb128 0                            #   starting offset
+  .uleb128 4                            #   ending offset
+  .byte 1                               # Loc expr size
+  .byte 85                              # DW_OP_reg5
+  .byte 4                               # DW_LLE_offset_pair
+  .uleb128 4                            #   starting offset
+  .uleb128 8                            #   ending offset
+  .byte 1                               # Loc expr size
+  .byte 83                              # DW_OP_reg3
+  .byte 0                               # DW_LLE_end_of_list
+.Ldebug_list_header_end0:
+  .section  .debug_abbrev.dwo,"e",@progbits
+  .byte 1                              # Abbreviation Code
+  .byte 17                              # DW_TAG_compile_unit
+  .byte 0                               # DW_CHILDREN_no
+  .byte 0                               # EOM(1)

llvm/test/tools/llvm-dwp/X86/rnglists.s

+# This test checks if llvm-dwp outputs .debug_rnglists.
+
+# RUN: llvm-mc -triple x86_64-unknown-linux %s -filetype=obj -o %t.o \
+# RUN:         -split-dwarf-file=%t.dwo -dwarf-version=5
+# RUN: llvm-dwp %t.dwo -o %t.dwp
+# RUN: llvm-dwarfdump -debug-rnglists -debug-cu-index -debug-tu-index %t.dwp | FileCheck %s
+
+# CHECK-DAG: .debug_cu_index contents:
+# CHECK: Index Signature          INFO                     ABBREV                   RNGLISTS
+# CHECK:     1 {{.*}} [0x00000018, 0x0000002d) [0x00000000, 0x00000004) [0x00000000, 0x00000017)
+
+# CHECK-DAG: .debug_tu_index contents:
+# CHECK: Index Signature          INFO                     ABBREV                   RNGLISTS
+# CHECK:     2 {{.*}} [0x00000000, 0x00000018) [0x00000000, 0x00000004) [0x00000000, 0x00000017)
+
+# CHECK-DAG: .debug_rnglists.dwo contents:
+# range list header: length = 0x00000013, format = DWARF32, version = 0x0005, addr_size = 0x08, seg_size = 0x00, offset_entry_count = 0x00000001
+# CHECK: offsets: [
+# CHECK-NEXT: 0x00000004
+# CHECK-NEXT: ]
+# CHECK-NEXT: ranges:
+# CHECK-NEXT: [0x0000000000000004, 0x0000000000000008)
+# CHECK-NEXT: [0x000000000000000c, 0x0000000000000010)
+
+  .section  .debug_info.dwo,"e",@progbits
+  .long .Ldebug_info_dwo_end0-.Ldebug_info_dwo_start0 # Length of Unit
+.Ldebug_info_dwo_start0:
+  .short  5                               # DWARF version number
+  .byte 6                               # DWARF Unit Type
+  .byte 8                               # Address Size (in bytes)
+  .long 0                               # Offset Into Abbrev. Section
+  .quad -4287463584810542331            # Type Signature
+  .long 31                              # Type DIE Offset
+.Ldebug_info_dwo_end0:
+  .section  .debug_info.dwo,"e",@progbits
+  .long .Ldebug_info_dwo_end3-.Ldebug_info_dwo_start3 # Length of Unit
+.Ldebug_info_dwo_start3:
+  .short  5                               # DWARF version number
+  .byte 5                               # DWARF Unit Type
+  .byte 8                               # Address Size (in bytes)
+  .long 0                               # Offset Into Abbrev. Section
+  .quad 1152943841751211454
+  .byte 1                              # Abbrev [1] 0x14:0x349 DW_TAG_compile_unit
+.Ldebug_info_dwo_end3:
+  .section  .debug_abbrev.dwo,"e",@progbits
+  .byte 1                              # Abbreviation Code
+  .byte 17                              # DW_TAG_compile_unit
+  .byte 0                               # DW_CHILDREN_no
+  .byte 0                               # EOM(1)
+  .section  .debug_rnglists.dwo,"e",@progbits
+  .long .Ldebug_list_header_end1-.Ldebug_list_header_start1 # Length
+.Ldebug_list_header_start1:
+  .short  5                               # Version
+  .byte 8                               # Address size
+  .byte 0                               # Segment selector size
+  .long 1                               # Offset entry count
+.Lrnglists_dwo_table_base0:
+  .long .Ldebug_ranges0-.Lrnglists_dwo_table_base0
+.Ldebug_ranges0:
+  .byte 4                               # DW_RLE_offset_pair
+  .uleb128 4                            #   starting offset
+  .uleb128 8                            #   ending offset
+  .byte 4                               # DW_RLE_offset_pair
+  .uleb128 12                           #   starting offset
+  .uleb128 16                           #   ending offset
+  .byte 0                               # DW_RLE_end_of_list
+.Ldebug_list_header_end1:

llvm/tools/llvm-dwp/llvm-dwp.cpp

const StringMap<std::pair<MCSection *, DWARFSectionKind>> KnownSections = {
       {"debug_info.dwo", {InfoSection, DW_SECT_INFO}},
       {"debug_types.dwo", {MCOFI.getDwarfTypesDWOSection(), DW_SECT_EXT_TYPES}},
       {"debug_str_offsets.dwo", {StrOffsetSection, DW_SECT_STR_OFFSETS}},
       {"debug_str.dwo", {StrSection, static_cast<DWARFSectionKind>(0)}},
       {"debug_loc.dwo", {MCOFI.getDwarfLocDWOSection(), DW_SECT_EXT_LOC}},
       {"debug_line.dwo", {MCOFI.getDwarfLineDWOSection(), DW_SECT_LINE}},
       {"debug_macro.dwo", {MCOFI.getDwarfMacroDWOSection(), DW_SECT_MACRO}},
       {"debug_abbrev.dwo", {MCOFI.getDwarfAbbrevDWOSection(), DW_SECT_ABBREV}},
+      {"debug_loclists.dwo",
+       {MCOFI.getDwarfLoclistsDWOSection(), DW_SECT_LOCLISTS}},
+      {"debug_rnglists.dwo",
+       {MCOFI.getDwarfRnglistsDWOSection(), DW_SECT_RNGLISTS}},
       {"debug_cu_index", {CUIndexSection, static_cast<DWARFSectionKind>(0)}},
       {"debug_tu_index", {TUIndexSection, static_cast<DWARFSectionKind>(0)}}};
 
   MapVector<uint64_t, UnitIndexEntry> IndexEntries;
   MapVector<uint64_t, UnitIndexEntry> TypeIndexEntries;
 
   uint32_t ContributionOffsets[8] = {};
   uint16_t Version = 0;