[PATCH] D45549: [DWARF v5] improved support for .debug_rnglists/consumer

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: test/DebugInfo/X86/dwarfdump-rnglists.s:6-8
+# There are 2 full CUs with DW_AT_rnglists_base, one with a DW_AT_ranges
+# attribute using DW_FORM_sec_offset, the other with DW_AT_ranges using
+# DW_FORM_rnglistx.
----------------
You could use one CU with rnglists_base (but maybe it's better to test the case where there is no rnglists_base, as you are doing?) - the spec says that rnglistx /may/ be used in the presence of rngilsts_base, but doesn't have to be used.

(the indirection of indexing into a table of offsets in rnglists, and the fact that the rnglists_base points to the bytes /after/ the header seem both a bit weird/unfortunate, but I guess they were picked that way for a reason (the latter seems problematic because it looks to me like that would mean the section can't be versioned effectively - if a newer version of the table changed the layout, how would that be parsed? The header would have to remain the same size & the parser would have to walk backwards from rnglists_base to see if the version is the same? :/)


================
Comment at: test/DebugInfo/X86/dwarfdump-rnglists.s:135
+Rnglist_Table0_Rnglist0_dwo:
+        .byte 0                                     # DW_RLE_start_end
+Rnglist_Table0_Rnglist1_dwo:
----------------
This comment should be DW_RLE_end_of_list, perhaps?


================
Comment at: test/DebugInfo/X86/dwarfdump-rnglists.s:155-176
+# CHECK:      .debug_info contents:
+# CHECK:      Compile Unit: 
+# CHECK-NOT:  Compile Unit:
+# CHECK:      DW_TAG_compile_unit
+# CHECK-NEXT: DW_AT_rnglists_base [DW_FORM_sec_offset]  (0x0000000c)
+# CHECK-NEXT: DW_AT_ranges [DW_FORM_sec_offset] (0x00000014
+# CHECK-NEXT: [0x00000014, 0x0000001e) ".text"
----------------
Usually we put the CHECK lines at the start, or sometimes interleaved with the content they're checking - perhaps you could do that? (you can even have multiple section chunks so you could keep the ranges for a specific CU next to the CU to make the tests relatively isolated rather than interleaved, perhaps?)


https://reviews.llvm.org/D45549