[PATCH] D35853: [DWARF] Generalized verification of .apple_names accelerator table to be applicable to any acceleration table. Added verification for .apple_types, .apple_namespaces and .apple_objc sections.

[Spyridoula Gravani via Phabricator via llvm-commits]

sgravani added inline comments.


================
Comment at: include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h:52
 
+  uint32_t HdrDataLengthOffset = offsetof(Header, HeaderDataLength);
+
----------------
aprantl wrote:
> I'm not sure this needs to be part of the interface — and if it is this should be a static constant or a constexpr instead of a data member.
Removed it. I can use getSizeHdr() instead.


================
Comment at: lib/DebugInfo/DWARF/DWARFContext.cpp:430
   }
-  if (DumpType == DIDT_All || DumpType == DIDT_AppleNames) {
-    if (!verifier.handleAppleNames())
-      Success = false;
-  }
+  Success &= verifier.handleAccelTables();
   return Success;
----------------
aprantl wrote:
> Could you just update all of the other cases in a separate commit?
Yes, I'll do that.


================
Comment at: lib/DebugInfo/DWARF/DWARFVerifier.cpp:482
+  // Verify that the section is not too short.
+  uint32_t Offset = 8;
+  if (AccelSectionData.getU32(&Offset) > 0 && !AccelTable.extract()) {
----------------
aprantl wrote:
> why initialize to 8?
I thought I need to verify whether num_of_buckets > 0 (and that field would be at offset 8 from the beginning of the section).
However, after the first check (for the size of the fixed part of the header),  if we just check the return value of extract(), it suffices 
to perform the second check (for the size of the section).

I modified the code to perform only extract() for the second check.


https://reviews.llvm.org/D35853