[PATCH] D35698: [DWARF] Generalized verification of .debug_abbrev to be applicable to both .debug_abbrev and .debug_abbrev.dwo sections.

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: lib/DebugInfo/DWARF/DWARFVerifier.cpp:135
+  if (noDebugAbbrev && noDebugAbbrevDWO) {
+    OS << "Warning: .debug_abbrev  and .debug_abbrev.dwo are empty.\n";
+    return true;
----------------
General feedback: probably want to consistently use lowercase 'warning' and 'error' (or better yet/as well, maybe refactor error/warning handling to use a single function for printing warnings and errors).

Also there are two spaces between ".debug_abbrev" and "and" - should only be one.

Is there an existing warning about there being no debug_info section? And about debug_info not having a valid debug_abbrev? Then maybe this warning about missing debug_abbrev isn't necessary?


================
Comment at: test/tools/llvm-dwarfdump/X86/verify_debug_abbrev.s:1
+# RUN: llvm-mc %s -filetype obj -triple x86_64-unknown-linux-gnu -o - \
+# RUN: | not llvm-dwarfdump -verify - \
----------------
This seems like a really big/long test - what's in basic.c? Does it need to be that complicated? (could it be simpler/shorter/etc?)


================
Comment at: test/tools/llvm-dwarfdump/X86/verify_debug_abbrev.s:6-7
+# CHECK: Verifying .debug_abbrev...
+# CHECK-NEXT: Error: Abbreviation declaration with code 1 contains multiple DW_AT_low_pc attributes.
+# CHECK-NEXT: Error: Abbreviation declaration with code 1 contains multiple DW_AT_language attributes.
+
----------------
Maybe worth mentioning the DIE tag? Or dumping the whole abbrev after/with the diagnostic?


https://reviews.llvm.org/D35698