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

[Adrian Prantl via Phabricator via llvm-commits]

aprantl added inline comments.


================
Comment at: include/llvm/DebugInfo/DWARF/DWARFVerifier.h:60
                         bool &isUnitDWARF64);
 
   bool verifyUnitContents(DWARFUnit Unit);
----------------
separate commit and perhaps use the opportunity to document verifyUnitContents? (no need for separate review)


================
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;
----------------
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.


https://reviews.llvm.org/D35643