llvm-dwarfdump --verify: Add support for .debug_str_offsets[.dwo]

Had a couple of issues lately causing corrupted strings due to
problematic str_offsets (overflow due to >4GB .debug_str.dwo section in
a dwp and the dwp tool silently overflowing the 32 bit offsets updated
in the .debug_str_offsets.dwo section, and then more recently two CUs in
a dwo caused the dwp tool to reapply the offset adjustment twice
corrupting str_offsets.dwo as well) - so let's check that the offsets
are valid.

This assumes no suffix merging - if anyone implements that, then this
checking should just be removed for the most part (we could still check
the offsets are within the bounds of .debug_str[.dwo], but nothing more
- any offset in the range would be valid, the offsets wouldn't have to
land at the start of a string)

Author: dwblaikie

llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h

@@ -338,6 +338,17 @@ class DWARFVerifier {
   /// \returns true if the existing Apple-style accelerator tables verify
   /// successfully, false otherwise.
   bool handleAccelTables();
+
+  /// Verify the information in the .debug_str_offsets[.dwo].
+  ///
+  /// Any errors are reported to the stream that was this object was
+  /// constructed with.
+  ///
+  /// \returns true if the .debug_line verifies successfully, false otherwise.
+  bool handleDebugStrOffsets();
+  bool verifyDebugStrOffsets(
+      StringRef SectionName, const DWARFSection &Section, StringRef StrData,
+      void (DWARFObject::*)(function_ref<void(const DWARFSection &)>) const);
 };
 
 static inline bool operator<(const DWARFVerifier::DieRangeInfo &LHS,

llvm/lib/DebugInfo/DWARF/DWARFContext.cpp

@@ -776,6 +776,8 @@ bool DWARFContext::verify(raw_ostream &OS, DIDumpOptions DumpOpts) {
     Success &= verifier.handleDebugInfo();
   if (DumpOpts.DumpType & DIDT_DebugLine)
     Success &= verifier.handleDebugLine();
+  if (DumpOpts.DumpType & DIDT_DebugStrOffsets)
+    Success &= verifier.handleDebugStrOffsets();
   Success &= verifier.handleAccelTables();
   return Success;
 }

llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp

@@ -1629,6 +1629,116 @@ bool DWARFVerifier::handleAccelTables() {
   return NumErrors == 0;
 }
 
+bool DWARFVerifier::handleDebugStrOffsets() {
+  OS << "Verifying .debug_str_offsets...\n";
+  const DWARFObject &DObj = DCtx.getDWARFObj();
+  bool Success = true;
+  Success &= verifyDebugStrOffsets(
+      ".debug_str_offsets.dwo", DObj.getStrOffsetsDWOSection(),
+      DObj.getStrDWOSection(), &DWARFObject::forEachInfoDWOSections);
+  Success &= verifyDebugStrOffsets(
+      ".debug_str_offsets", DObj.getStrOffsetsSection(), DObj.getStrSection(),
+      &DWARFObject::forEachInfoSections);
+  return Success;
+}
+
+bool DWARFVerifier::verifyDebugStrOffsets(
+    StringRef SectionName, const DWARFSection &Section, StringRef StrData,
+    void (DWARFObject::*VisitInfoSections)(
+        function_ref<void(const DWARFSection &)>) const) {
+  const DWARFObject &DObj = DCtx.getDWARFObj();
+  uint16_t InfoVersion = 0;
+  DwarfFormat InfoFormat = DwarfFormat::DWARF32;
+  (DObj.*VisitInfoSections)([&](const DWARFSection &S) {
+    if (InfoVersion)
+      return;
+    DWARFDataExtractor DebugInfoData(DObj, S, DCtx.isLittleEndian(), 0);
+    uint64_t Offset = 0;
+    InfoFormat = DebugInfoData.getInitialLength(&Offset).second;
+    InfoVersion = DebugInfoData.getU16(&Offset);
+  });
+
+  DWARFDataExtractor DA(DObj, Section, DCtx.isLittleEndian(), 0);
+
+  DataExtractor::Cursor C(0);
+  uint64_t NextUnit = 0;
+  bool Success = true;
+  while (C.seek(NextUnit), C.tell() < DA.getData().size()) {
+    DwarfFormat Format;
+    uint64_t Length;
+    uint64_t StartOffset = C.tell();
+    if (InfoVersion == 4) {
+      Format = InfoFormat;
+      Length = DA.getData().size();
+      NextUnit = C.tell() + Length;
+    } else {
+      std::tie(Length, Format) = DA.getInitialLength(C);
+      if (!C)
+        break;
+      if (C.tell() + Length > DA.getData().size()) {
+        error() << formatv(
+            "{0}: contribution {1:X}: length exceeds available space "
+            "(contribution "
+            "offset ({1:X}) + length field space ({2:X}) + length ({3:X}) == "
+            "{4:X} > section size {5:X})\n",
+            SectionName, StartOffset, C.tell() - StartOffset, Length,
+            C.tell() + Length, DA.getData().size());
+        Success = false;
+        // Nothing more to do - no other contributions to try.
+        break;
+      }
+      NextUnit = C.tell() + Length;
+      uint8_t Version = DA.getU16(C);
+      if (C && Version != 5) {
+        error() << formatv("{0}: contribution {1:X}: invalid version {2}\n",
+                           SectionName, StartOffset, Version);
+        Success = false;
+        // Can't parse the rest of this contribution, since we don't know the
+        // version, but we can pick up with the next contribution.
+        continue;
+      }
+      (void)DA.getU16(C); // padding
+    }
+    uint64_t OffsetByteSize = getDwarfOffsetByteSize(Format);
+    DA.setAddressSize(OffsetByteSize);
+    uint64_t Remainder = (Length - 4) % OffsetByteSize;
+    if (Remainder != 0) {
+      error() << formatv(
+          "{0}: contribution {1:X}: invalid length ((length ({2:X}) "
+          "- header (0x4)) % offset size {3:X} == {4:X} != 0)\n",
+          SectionName, StartOffset, Length, OffsetByteSize, Remainder);
+      Success = false;
+    }
+    for (uint64_t Index = 0; C && C.tell() + OffsetByteSize <= NextUnit; ++Index) {
+      uint64_t OffOff = C.tell();
+      uint64_t StrOff = DA.getAddress(C);
+      // check StrOff refers to the start of a string
+      if (StrOff == 0)
+        continue;
+      if (StrData.size() <= StrOff) {
+        error() << formatv(
+            "{0}: contribution {1:X}: index {2:X}: invalid string "
+            "offset *{3:X} == {4:X}, is beyond the bounds of the string section of length {5:X}\n",
+            SectionName, StartOffset, Index, OffOff, StrOff, StrData.size());
+        continue;
+      }
+      if (StrData[StrOff - 1] == '\0')
+        continue;
+      error() << formatv("{0}: contribution {1:X}: index {2:X}: invalid string "
+                         "offset *{3:X} == {4:X}, is neither zero nor "
+                         "immediately following a null character\n",
+                         SectionName, StartOffset, Index, OffOff, StrOff);
+      Success = false;
+    }
+  }
+
+  if (Error E = C.takeError()) {
+    error() << SectionName << ": " << toString(std::move(E)) << '\n';
+    return false;
+  }
+  return Success;
+}
+
 raw_ostream &DWARFVerifier::error() const { return WithColor::error(OS); }
 
 raw_ostream &DWARFVerifier::warn() const { return WithColor::warning(OS); }

llvm/test/tools/llvm-dwarfdump/X86/verify_dwarf5_debug_line.yaml

@@ -41,8 +41,13 @@ main:                                   # @main
 .Linfo_string2:
 	.asciz	"/tmp"                          # string offset=110
 	.section	.debug_str_offsets,"",@progbits
+        .long   .Lstr_off_end - .Lstr_off_begin # Length of String Offsets Set
+.Lstr_off_begin:
+        .short  5
+        .short  0
 	.long	.Linfo_string0
 	.long	.Linfo_string1
 	.long	.Linfo_string2
+.Lstr_off_end:
 	.ident	"clang version 17.0.0"
 .Lline_table_start0:

llvm/test/tools/llvm-dwarfdump/X86/verify_invalid_str_offsets.yaml

@@ -0,0 +1,59 @@
+# RUN: rm -rf %t && split-file %s %t
+# RUN: yaml2obj %t/v5.yaml -o %t/v5.o
+# RUN: not llvm-dwarfdump -debug-str-offsets -verify %t/v5.o | FileCheck %s
+# RUN: yaml2obj %t/v4.yaml -o %t/v4.o
+# RUN: not llvm-dwarfdump -debug-str-offsets -verify %t/v4.o | FileCheck --check-prefix=V4 %s
+
+#      CHECK: Verifying .debug_abbrev...
+#      CHECK: Verifying .debug_str_offsets...
+# CHECK-NEXT: error: .debug_str_offsets: contribution 0x0: index 0x2: invalid string offset *0x10 == 0x1, is neither zero nor immediately following a null character
+# CHECK-NEXT: error: .debug_str_offsets: contribution 0x0: index 0x3: invalid string offset *0x14 == 0x42, is beyond the bounds of the string section of length 0x8
+# CHECK-NEXT: error: .debug_str_offsets: contribution 0x18: invalid version 42
+# CHECK-NEXT: error: .debug_str_offsets: contribution 0x20: invalid length ((length (0x5) - header (0x4)) % offset size 0x4 == 0x1 != 0)
+# CHECK-NEXT: error: .debug_str_offsets: contribution 0x29: length exceeds available space (contribution offset (0x29) + length field space (0x4) + length (0x5000000) == 0x500002D > section size 0x30)
+# Errors detected.
+
+# V4: error: .debug_str_offsets: contribution 0x0: index 0x2: invalid string offset *0x8 == 0x2, is neither zero nor immediately following a null character
+
+
+#--- v4.yaml
+--- !ELF
+FileHeader:
+  Class: ELFCLASS64
+  Data:  ELFDATA2LSB
+  Type:  ET_EXEC
+DWARF:
+  debug_str:
+    - 'foo'
+    - 'bar'
+  debug_info:
+    - Version: 4
+      AddrSize: 4
+Sections:
+  - Name: '.debug_str_offsets'
+    Type: SHT_PROGBITS
+    Content: "000000000400000002000000"
+
+#--- v5.yaml
+--- !ELF
+FileHeader:
+  Class: ELFCLASS64
+  Data:  ELFDATA2LSB
+  Type:  ET_EXEC
+DWARF:
+  debug_str:
+    - 'foo'
+    - 'bar'
+  debug_info:
+    - Version: 5
+      UnitType:        DW_UT_compile
+      AddrSize:        8
+  debug_str_offsets:
+    - Offsets:
+        - 0x00000000
+        - 0x00000004
+        - 0x00000001
+        - 0x00000042
+    - Version: 42
+    - Length: 5
+    - Length: 8