[PATCH] D35643: [DWARF] Added check that verifies that no abbreviation declaration has more than one attribute with the same name.

[Spyridoula Gravani via Phabricator via llvm-commits]

sgravani added inline comments.


================
Comment at: lib/DebugInfo/DWARF/DWARFVerifier.cpp:97-105
+    std::set<dwarf::Attribute> AttributeSet;
     for (auto AttrValue : Die.attributes()) {
+      if (AttributeSet.find(AttrValue.Attr) != AttributeSet.end()) {
+        OS << format("Error: Die at offset 0x%08x ", Die.getOffset());
+        OS << "contains multiple " << AttributeString(AttrValue.Attr)
+           << " attributes.\n";
+        ++NumUnitErrors;
----------------
aprantl wrote:
> friss wrote:
> > While this is really simple, I feel like it's also really overkill to build a set for every DIE. Every DIE using the same abbreviation number will expose the same issue. Can we somehow prevent the duplicated work?
> Could we iterate over the abbreviations instead of the DIEs?
> It also may be faster to use a SmallDenseSet over a std::set since the number of attributes will be typically very small.
Done; verifying .debug_abbrev instead.


================
Comment at: test/tools/llvm-dwarfdump/X86/verify_debug_info.s:84
 	.byte	1                       ## DW_FORM_addr
-	.byte	18                      ## DW_AT_high_pc
+	.byte	17                      ## DW_AT_high_pc -- Error: Die at offset 0x0000002b contains multiple DW_AT_low_pc attributes.
 	.byte	6                       ## DW_FORM_data4
----------------
friss wrote:
> By adding this just to the abrrev table, I think you completely break the DIE tree decoding. Wouldn't it be better to add it to the debug_info section as well? 
I'm not sure what you mean.. Can you explain?


https://reviews.llvm.org/D35643