[PATCH] D35963: [DWARF] Added verification check for tags in accelerator tables. This patch verifies that the atom tag is actually the same with the tag of the DIE that we retrieve from the table.

[Adrian Prantl via Phabricator via llvm-commits]

aprantl added a comment.

This needs a testcase.



================
Comment at: include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h:68
+  /// DieTag is the tag of the DIE
+  std::tuple<uint32_t, dwarf::Tag> readAtoms(uint32_t &HashDataOffset);
   void dump(raw_ostream &OS) const;
----------------
I would use a `std::pair<>` or a simple struct here.


================
Comment at: lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp:102
+    case dwarf::DW_ATOM_die_tag:
+      DieTag = (dwarf::Tag)*FormValue.getAsUnsignedConstant();
+      break;
----------------
A dwarf::Tag is a uint_16 — do we also need to check that the value is within range, or is this impossible because we already verified the FORM?


================
Comment at: lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp:108
   }
-  return DieOffset;
+  return std::make_tuple(DieOffset, DieTag);
 }
----------------
`return {DieOffset, DieTag};`

should also work here


================
Comment at: lib/DebugInfo/DWARF/DWARFVerifier.cpp:550
 
           ++NumErrors;
+        } else if ((std::get<1>(Atoms) != dwarf::DW_TAG_null) &&
----------------
maybe `continue;` here and get rid of the else? not sure if this will be better :-)


================
Comment at: lib/DebugInfo/DWARF/DWARFVerifier.cpp:554
+          OS << "\terror: Tag " << dwarf::TagString(std::get<1>(Atoms))
+             << " is not a valid tag for DIE[" << HashDataIdx << "].\n";
         }
----------------
Could we be more specific in the error message? 
perhaps: "Tag in accelerator table does not match Tag of DIE" 


https://reviews.llvm.org/D35963