[PATCH] D73086: [DWARF5] Added support for debug_macro section parsing and dumping in llvm-dwarfdump.

[Igor Kudrin via Phabricator via llvm-commits]

ikudrin added inline comments.


================
Comment at: llvm/test/DebugInfo/X86/debug-macro-macinfo.s:38
+	.byte	0                       # Line Number
+	.file	1 "/macro" "a-v4.c"     # File Number
+	.byte	1                       # DW_MACINFO_define
----------------
Remove this `.file` line. I guess you may expect it to contribute the "File Numer" to this section, but that is not true. It has no influence on the content of this section.

In fact, the byte you described as "DW_MACINFO_define" is the actual file number. The next byte is that directive and the following byte is the line number for the entry. You should know that `DW_MACINFO_define` does not have an operand for file number.

I guess it would be better to use some different values for the file number and for the line number here to avoid that misinterpretation.


================
Comment at: llvm/test/DebugInfo/X86/debug-macro-macinfo.s:42
+	.byte	1                       # File Number
+	.ascii	"DWARF_VERSION"        # Macro String
+	.byte	32                      # Space
----------------
You may just use `.asciz "DWARF_VERSION 4"`. It is the same. The fact that the macro string consists of the name, space, and value is not important for this test. And you have not divided the definition for "DWARF_VERSION 5" despite it follows the same rules.


================
Comment at: llvm/test/DebugInfo/X86/debug-macro-v5.s:31
+	.byte	1                       # Line Number
+	.file	1 "." "foo.h" md5 0x0f0cd0e22b44f49d3944992c8dc28661 # File Number
+	.byte	1
----------------
Not needed for the test. Remove.


================
Comment at: llvm/test/DebugInfo/X86/unsupported-dwarf64-debug-macro-v5.s:13
+	.byte	3                       # Flags: 64 bit, debug_line_offset present
+	.long	0                       # debug_line_offset
----------------
For the consistency, this should be `.quad`, not `.long`.


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

https://reviews.llvm.org/D73086