[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 inline comments.


================
Comment at: lib/DebugInfo/DWARF/DWARFVerifier.cpp:550
 
           ++NumErrors;
+        } else if ((std::get<1>(Atoms) != dwarf::DW_TAG_null) &&
----------------
sgravani wrote:
> aprantl wrote:
> > maybe `continue;` here and get rid of the else? not sure if this will be better :-)
> .....why do that? I'm not sure if this is better either :P
The LLVM coding style prefers early exits over nested ifs to improve readability. I'll leave this up to you.


================
Comment at: lib/DebugInfo/DWARF/DWARFVerifier.cpp:534
+        auto Atoms = AccelTable.readAtoms(HashDataOffset);
+        auto Die = DCtx.getDIEForOffset(std::get<0>(Atoms));
+        if (!Die) {
----------------
I think something like this would be more readable:
```
unsigned Offset;
unsigned Tag;
std::tie(Offset, Tag) = AccelTable.readAtoms(HashDataOffset);
```


================
Comment at: test/tools/llvm-dwarfdump/X86/apple_types_verify_tag.s:121
+	.long	52
+	.short	31						## error: Tag DW_TAG_ptr_to_member_type in accelerator table does not match Tag DW_TAG_base_type of DIE[0].
+	.byte	0
----------------
Where's the CHECK?


https://reviews.llvm.org/D35963