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

[Adrian Prantl via Phabricator via llvm-commits]

aprantl added inline comments.


================
Comment at: include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h:52
 
+  uint32_t HdrDataLengthOffset = offsetof(Header, HeaderDataLength);
+
----------------
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.


================
Comment at: include/llvm/DebugInfo/DWARF/DWARFVerifier.h:175
+  /// \returns true if the existing accelerator tables verify successfully,
+  /// false otherwise.
+  bool handleAccelTables();
----------------
... existing *Apple-style* accelerator tables ...
DWARF v5 accelerator tables are not yet supported.


================
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;
----------------
Could you just update all of the other cases in a separate commit?


================
Comment at: lib/DebugInfo/DWARF/DWARFVerifier.cpp:478
+  if (!AccelSectionData.isValidOffset(AccelTable.HdrDataLengthOffset + 4)) {
+    OS << "\terror: Header too short.\n";
+    return 1;
----------------
`"Section" << Sectionname << " is too small to fit a section header\n"`


================
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()) {
----------------
why initialize to 8?


================
Comment at: lib/DebugInfo/DWARF/DWARFVerifier.cpp:484
+  if (AccelSectionData.getU32(&Offset) > 0 && !AccelTable.extract()) {
+    OS << "\terror: Section too short.\n";
+    return 1;
----------------
`"Section" << Sectionname << " is smaller than size described in section header\n"`


================
Comment at: lib/DebugInfo/DWARF/DWARFVerifier.cpp:569
+  unsigned NumErrors = 0;
+  if (!noAppleNames)
+    NumErrors +=
----------------
`if (!D.getAppleNamesSection().Data.empty())`


================
Comment at: lib/DebugInfo/DWARF/DWARFVerifier.cpp:572
+        verifyAccelTable(&D.getAppleNamesSection(), &StrData, ".apple_names");
+  if (!noAppleTypes)
+    NumErrors +=
----------------
...


================
Comment at: lib/DebugInfo/DWARF/DWARFVerifier.cpp:583
+}
\ No newline at end of file

----------------
add one :-)


https://reviews.llvm.org/D35853