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

Tue Mar 17 04:27:49 PDT 2020

ikudrin marked an inline comment as done.
ikudrin added a comment.

I am mostly OK with the changes at the moment. Please reach the consent with @jhenderson about code duplication and other topics he mentioned.

================
Comment at: llvm/test/DebugInfo/X86/debug-macro-macinfo.s:42
+	.byte	1                       # File Number
+	.ascii	"DWARF_VERSION"        # Macro String
+	.byte	32                      # Space
----------------
SouraVX wrote:
> ikudrin wrote:
> > 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.
> >  And you have not divided the definition for "DWARF_VERSION 5" despite it follows the same rules.
> Are you concerned regarding representation ?? 
> DW_MACRO_define_strp forms requires the entire macro and definition(value) "DWARF_VERSION 5" to be kept as a string in debug_str section. 
> That's what I've done here. Am I missing something here ?
Both strings, "DWARF_VERSION 4" and "DWARF_VERSION 5", are OK now.

================
Comment at: llvm/test/DebugInfo/X86/debug-macro-macinfo.s:47
+.Linfo_string0:
+	.asciz	"DWARF_VERSION 5"       # string offset=149
----------------
Remove the comment "string offset=149". It is both useless and misleading. The real offset is `0`.

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

https://reviews.llvm.org/D73086