[DebugInfo] Simplify DWARFDebugAddr.

The patch removes unnecessary members of DWARFDebugAddr and further
simplifies the implementation by separating parsing methods of tables
in the DWARFv5 and pre-standard formats.

Differential Revision: https://reviews.llvm.org/D74197

diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugAddr.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugAddr.h
index 4539b9c..fd9c5dc 100644 (file)
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugAddr.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugAddr.h
@@ -27,69 +27,53 @@ class raw_ostream;
 /// The table consists of a header followed by an array of address values from
 /// .debug_addr section.
 class DWARFDebugAddrTable {
-public:
-  struct Header {
-    /// The total length of the entries for this table, not including the length
-    /// field itself.
-    uint32_t Length = 0;
-    /// The DWARF version number.
-    uint16_t Version = 5;
-    /// The size in bytes of an address on the target architecture. For
-    /// segmented addressing, this is the size of the offset portion of the
-    /// address.
-    uint8_t AddrSize;
-    /// The size in bytes of a segment selector on the target architecture.
-    /// If the target system uses a flat address space, this value is 0.
-    uint8_t SegSize = 0;
-  };
-
-private:
   dwarf::DwarfFormat Format;
-  uint64_t HeaderOffset;
-  Header HeaderData;
-  uint32_t DataSize = 0;
+  uint64_t Offset;
+  /// The total length of the entries for this table, not including the length
+  /// field itself.
+  uint32_t Length = 0;
+  /// The DWARF version number.
+  uint16_t Version;
+  /// The size in bytes of an address on the target architecture. For
+  /// segmented addressing, this is the size of the offset portion of the
+  /// address.
+  uint8_t AddrSize;
+  /// The size in bytes of a segment selector on the target architecture.
+  /// If the target system uses a flat address space, this value is 0.
+  uint8_t SegSize;
   std::vector<uint64_t> Addrs;
 
+  /// Invalidate Length field to stop further processing.
+  void invalidateLength() { Length = 0; }
+
+  Error extractAddresses(const DWARFDataExtractor &Data, uint64_t *OffsetPtr,
+                         uint64_t EndOffset);
+
 public:
-  void clear();
 
-  /// Extract an entire table, including all addresses.
-  Error extract(DWARFDataExtractor Data, uint64_t *OffsetPtr,
-                uint16_t Version, uint8_t AddrSize,
+  /// Extract the entire table, including all addresses.
+  Error extract(const DWARFDataExtractor &Data, uint64_t *OffsetPtr,
+                uint16_t CUVersion, uint8_t CUAddrSize,
                 std::function<void(Error)> WarnCallback);
 
-  uint64_t getHeaderOffset() const { return HeaderOffset; }
-  uint8_t getAddrSize() const { return HeaderData.AddrSize; }
+  /// Extract a DWARFv5 address table.
+  Error extractV5(const DWARFDataExtractor &Data, uint64_t *OffsetPtr,
+                  uint8_t CUAddrSize, std::function<void(Error)> WarnCallback);
+
+  /// Extract a pre-DWARFv5 address table. Such tables do not have a header
+  /// and consist only of a series of addresses.
+  /// See https://gcc.gnu.org/wiki/DebugFission for details.
+  Error extractPreStandard(const DWARFDataExtractor &Data, uint64_t *OffsetPtr,
+                           uint16_t CUVersion, uint8_t CUAddrSize);
+
   void dump(raw_ostream &OS, DIDumpOptions DumpOpts = {}) const;
 
   /// Return the address based on a given index.
   Expected<uint64_t> getAddrEntry(uint32_t Index) const;
 
-  /// Return the size of the table header including the length
-  /// but not including the addresses.
-  uint8_t getHeaderSize() const {
-    switch (Format) {
-    case dwarf::DwarfFormat::DWARF32:
-      return 8; // 4 + 2 + 1 + 1
-    case dwarf::DwarfFormat::DWARF64:
-      return 16; // 12 + 2 + 1 + 1
-    }
-    llvm_unreachable("Invalid DWARF format (expected DWARF32 or DWARF64)");
-  }
-
-  /// Returns the length of this table, including the length field, or 0 if the
-  /// length has not been determined (e.g. because the table has not yet been
-  /// parsed, or there was a problem in parsing).
-  uint32_t getLength() const;
-
-  /// Verify that the given length is valid for this table.
-  bool hasValidLength() const { return getLength() != 0; }
-
-  /// Invalidate Length field to stop further processing.
-  void invalidateLength() { HeaderData.Length = 0; }
-
-  /// Returns the length of the array of addresses.
-  uint32_t getDataSize() const;
+  /// Return the full length of this table, including the length field.
+  /// Return None if the length cannot be identified reliably.
+  Optional<uint64_t> getFullLength() const;
 };
 
 } // end namespace llvm

diff --git a/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp b/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
index eb736b3..62bc513 100644 (file)
--- a/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
@@ -252,12 +252,13 @@ static void dumpAddrSection(raw_ostream &OS, DWARFDataExtractor &AddrData,
       WithColor::error() << toString(std::move(Err)) << '\n';
       // Keep going after an error, if we can, assuming that the length field
       // could be read. If it couldn't, stop reading the section.
-      if (!AddrTable.hasValidLength())
-        break;
-      Offset = TableOffset + AddrTable.getLength();
-    } else {
-      AddrTable.dump(OS, DumpOpts);
+      if (auto TableLength = AddrTable.getFullLength()) {
+        Offset = TableOffset + *TableLength;
+        continue;
+      }
+      break;
     }
+    AddrTable.dump(OS, DumpOpts);
   }
 }
 

diff --git a/llvm/lib/DebugInfo/DWARF/DWARFDebugAddr.cpp b/llvm/lib/DebugInfo/DWARF/DWARFDebugAddr.cpp
index eb7070f..d2344e7 100644 (file)
--- a/llvm/lib/DebugInfo/DWARF/DWARFDebugAddr.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFDebugAddr.cpp
@@ -12,145 +12,146 @@
 
 using namespace llvm;
 
-void DWARFDebugAddrTable::clear() {
-  HeaderData = {};
+Error DWARFDebugAddrTable::extractAddresses(const DWARFDataExtractor &Data,
+                                            uint64_t *OffsetPtr,
+                                            uint64_t EndOffset) {
+  assert(EndOffset >= *OffsetPtr);
+  uint64_t DataSize = EndOffset - *OffsetPtr;
+  assert(Data.isValidOffsetForDataOfSize(*OffsetPtr, DataSize));
+  if (AddrSize != 4 && AddrSize != 8)
+    return createStringError(errc::not_supported,
+                             "address table at offset 0x%" PRIx64
+                             " has unsupported address size %" PRIu8
+                             " (4 and 8 are supported)",
+                             Offset, AddrSize);
+  if (DataSize % AddrSize != 0) {
+    invalidateLength();
+    return createStringError(errc::invalid_argument,
+                             "address table at offset 0x%" PRIx64
+                             " contains data of size 0x%" PRIx64
+                             " which is not a multiple of addr size %" PRIu8,
+                             Offset, DataSize, AddrSize);
+  }
   Addrs.clear();
-  invalidateLength();
+  size_t Count = DataSize / AddrSize;
+  Addrs.reserve(Count);
+  while (Count--)
+    Addrs.push_back(Data.getRelocatedValue(AddrSize, OffsetPtr));
+  return Error::success();
 }
 
-Error DWARFDebugAddrTable::extract(DWARFDataExtractor Data,
-                                   uint64_t *OffsetPtr,
-                                   uint16_t Version,
-                                   uint8_t AddrSize,
-                                   std::function<void(Error)> WarnCallback) {
-  clear();
-  HeaderOffset = *OffsetPtr;
-  // Read and verify the length field.
-  if (!Data.isValidOffsetForDataOfSize(*OffsetPtr, sizeof(uint32_t)))
+Error DWARFDebugAddrTable::extractV5(const DWARFDataExtractor &Data,
+                                     uint64_t *OffsetPtr, uint8_t CUAddrSize,
+                                     std::function<void(Error)> WarnCallback) {
+  Offset = *OffsetPtr;
+  // Check that we can read the unit length field.
+  if (!Data.isValidOffsetForDataOfSize(Offset, 4))
     return createStringError(errc::invalid_argument,
-                       "section is not large enough to contain an "
-                       "address table length at offset 0x%"
-                       PRIx64, *OffsetPtr);
-  uint16_t UnitVersion;
-  if (Version == 0) {
-    WarnCallback(createStringError(errc::invalid_argument,
-                       "DWARF version is not defined in CU,"
-                       " assuming version 5"));
-    UnitVersion = 5;
-  } else {
-    UnitVersion = Version;
-  }
+                             "section is not large enough to contain an "
+                             "address table length at offset 0x%" PRIx64,
+                             Offset);
   // TODO: Add support for DWARF64.
   Format = dwarf::DwarfFormat::DWARF32;
-  if (UnitVersion >= 5) {
-    HeaderData.Length = Data.getU32(OffsetPtr);
-    if (HeaderData.Length == dwarf::DW_LENGTH_DWARF64) {
-      invalidateLength();
-      return createStringError(errc::not_supported,
-          "DWARF64 is not supported in .debug_addr at offset 0x%" PRIx64,
-          HeaderOffset);
-    }
-    if (HeaderData.Length + sizeof(uint32_t) < sizeof(Header)) {
-      uint32_t TmpLength = HeaderData.Length;
-      invalidateLength();
-      return createStringError(errc::invalid_argument,
-                         "address table at offset 0x%" PRIx64
-                         " has a unit_length value of 0x%" PRIx32
-                         ", which is too small to contain a complete header",
-                         HeaderOffset, TmpLength);
-    }
-    uint64_t End = HeaderOffset + getLength();
-    if (!Data.isValidOffsetForDataOfSize(HeaderOffset, End - HeaderOffset)) {
-      uint32_t TmpLength = HeaderData.Length;
-      invalidateLength();
-      return createStringError(
-          errc::invalid_argument,
-          "section is not large enough to contain an address table "
-          "at offset 0x%" PRIx64 " with a unit_length value of 0x%" PRIx32,
-          HeaderOffset, TmpLength);
-    }
-
-    HeaderData.Version = Data.getU16(OffsetPtr);
-    HeaderData.AddrSize = Data.getU8(OffsetPtr);
-    HeaderData.SegSize = Data.getU8(OffsetPtr);
-    DataSize = getDataSize();
-  } else {
-    HeaderData.Version = UnitVersion;
-    HeaderData.AddrSize = AddrSize;
-    // TODO: Support for non-zero SegSize.
-    HeaderData.SegSize = 0;
-    DataSize = Data.size();
+  Length = Data.getU32(OffsetPtr);
+  if (Length == dwarf::DW_LENGTH_DWARF64) {
+    invalidateLength();
+    return createStringError(
+        errc::not_supported,
+        "DWARF64 is not supported in .debug_addr at offset 0x%" PRIx64, Offset);
+  }
+
+  if (!Data.isValidOffsetForDataOfSize(*OffsetPtr, Length)) {
+    uint32_t DiagnosticLength = Length;
+    invalidateLength();
+    return createStringError(
+        errc::invalid_argument,
+        "section is not large enough to contain an address table "
+        "at offset 0x%" PRIx64 " with a unit_length value of 0x%" PRIx32,
+        Offset, DiagnosticLength);
+  }
+  uint64_t EndOffset = *OffsetPtr + Length;
+  // Ensure that we can read the remaining header fields.
+  if (Length < 4) {
+    uint32_t DiagnosticLength = Length;
+    invalidateLength();
+    return createStringError(
+        errc::invalid_argument,
+        "address table at offset 0x%" PRIx64
+        " has a unit_length value of 0x%" PRIx32
+        ", which is too small to contain a complete header",
+        Offset, DiagnosticLength);
   }
 
-  // Perform basic validation of the remaining header fields.
+  Version = Data.getU16(OffsetPtr);
+  AddrSize = Data.getU8(OffsetPtr);
+  SegSize = Data.getU8(OffsetPtr);
 
-  // We support DWARF version 5 for now as well as pre-DWARF5
-  // implementations of .debug_addr table, which doesn't contain a header
-  // and consists only of a series of addresses.
-  if (HeaderData.Version > 5) {
+  // Perform a basic validation of the header fields.
+  if (Version != 5)
     return createStringError(errc::not_supported,
                              "address table at offset 0x%" PRIx64
                              " has unsupported version %" PRIu16,
-                             HeaderOffset, HeaderData.Version);
-  }
-  // FIXME: For now we just treat version mismatch as an error,
-  // however the correct way to associate a .debug_addr table
-  // with a .debug_info table is to look at the DW_AT_addr_base
-  // attribute in the info table.
-  if (HeaderData.Version != UnitVersion)
-    return createStringError(errc::invalid_argument,
-                       "address table at offset 0x%" PRIx64
-                       " has version %" PRIu16
-                       " which is different from the version suggested"
-                       " by the DWARF unit header: %" PRIu16,
-                       HeaderOffset, HeaderData.Version, UnitVersion);
-  if (HeaderData.AddrSize != 4 && HeaderData.AddrSize != 8)
+                             Offset, Version);
+  // TODO: add support for non-zero segment selector size.
+  if (SegSize != 0)
     return createStringError(errc::not_supported,
-                       "address table at offset 0x%" PRIx64
-                       " has unsupported address size %" PRIu8
-                       " (4 and 8 are supported)",
-                       HeaderOffset, HeaderData.AddrSize);
-  if (HeaderData.AddrSize != AddrSize && AddrSize != 0)
+                             "address table at offset 0x%" PRIx64
+                             " has unsupported segment selector size %" PRIu8,
+                             Offset, SegSize);
+
+  if (Error Err = extractAddresses(Data, OffsetPtr, EndOffset))
+    return Err;
+  if (CUAddrSize && AddrSize != CUAddrSize) {
     WarnCallback(createStringError(
         errc::invalid_argument,
         "address table at offset 0x%" PRIx64 " has address size %" PRIu8
         " which is different from CU address size %" PRIu8,
-        HeaderOffset, HeaderData.AddrSize, AddrSize));
-
-  // TODO: add support for non-zero segment selector size.
-  if (HeaderData.SegSize != 0)
-    return createStringError(errc::not_supported,
-                       "address table at offset 0x%" PRIx64
-                       " has unsupported segment selector size %" PRIu8,
-                       HeaderOffset, HeaderData.SegSize);
-  if (DataSize % HeaderData.AddrSize != 0) {
-    invalidateLength();
-    return createStringError(errc::invalid_argument,
-                       "address table at offset 0x%" PRIx64
-                       " contains data of size %" PRIu32
-                       " which is not a multiple of addr size %" PRIu8,
-                       HeaderOffset, DataSize, HeaderData.AddrSize);
+        Offset, AddrSize, CUAddrSize));
   }
-  Data.setAddressSize(HeaderData.AddrSize);
-  uint32_t AddrCount = DataSize / HeaderData.AddrSize;
-  for (uint32_t I = 0; I < AddrCount; ++I)
-    Addrs.push_back(Data.getRelocatedAddress(OffsetPtr));
   return Error::success();
 }
 
+Error DWARFDebugAddrTable::extractPreStandard(const DWARFDataExtractor &Data,
+                                              uint64_t *OffsetPtr,
+                                              uint16_t CUVersion,
+                                              uint8_t CUAddrSize) {
+  assert(CUVersion > 0 && CUVersion < 5);
+
+  Offset = *OffsetPtr;
+  Length = 0;
+  Version = CUVersion;
+  AddrSize = CUAddrSize;
+  SegSize = 0;
+
+  return extractAddresses(Data, OffsetPtr, Data.size());
+}
+
+Error DWARFDebugAddrTable::extract(const DWARFDataExtractor &Data,
+                                   uint64_t *OffsetPtr,
+                                   uint16_t CUVersion,
+                                   uint8_t CUAddrSize,
+                                   std::function<void(Error)> WarnCallback) {
+  if (CUVersion > 0 && CUVersion < 5)
+    return extractPreStandard(Data, OffsetPtr, CUVersion, CUAddrSize);
+  if (CUVersion == 0)
+    WarnCallback(createStringError(errc::invalid_argument,
+                                   "DWARF version is not defined in CU,"
+                                   " assuming version 5"));
+  return extractV5(Data, OffsetPtr, CUAddrSize, WarnCallback);
+}
+
 void DWARFDebugAddrTable::dump(raw_ostream &OS, DIDumpOptions DumpOpts) const {
   if (DumpOpts.Verbose)
-    OS << format("0x%8.8" PRIx32 ": ", HeaderOffset);
-  if (HeaderData.Length)
+    OS << format("0x%8.8" PRIx32 ": ", Offset);
+  if (Length)
     OS << format("Address table header: length = 0x%8.8" PRIx32
-                 ", version = 0x%4.4" PRIx16 ", "
-                 "addr_size = 0x%2.2" PRIx8 ", seg_size = 0x%2.2" PRIx8 "\n",
-                 HeaderData.Length, HeaderData.Version, HeaderData.AddrSize,
-                 HeaderData.SegSize);
+                 ", version = 0x%4.4" PRIx16 ", addr_size = 0x%2.2" PRIx8
+                 ", seg_size = 0x%2.2" PRIx8 "\n",
+                 Length, Version, AddrSize, SegSize);
 
   if (Addrs.size() > 0) {
-    const char *AddrFmt = (HeaderData.AddrSize == 4) ? "0x%8.8" PRIx64 "\n"
-                                                     : "0x%16.16" PRIx64 "\n";
+    const char *AddrFmt =
+        (AddrSize == 4) ? "0x%8.8" PRIx64 "\n" : "0x%16.16" PRIx64 "\n";
     OS << "Addrs: [\n";
     for (uint64_t Addr : Addrs)
       OS << format(AddrFmt, Addr);
@@ -162,22 +163,15 @@ Expected<uint64_t> DWARFDebugAddrTable::getAddrEntry(uint32_t Index) const {
   if (Index < Addrs.size())
     return Addrs[Index];
   return createStringError(errc::invalid_argument,
-                           "Index %" PRIu32 " is out of range in the "
+                           "Index %" PRIu32 " is out of range of the "
                            "address table at offset 0x%" PRIx64,
-                           Index, HeaderOffset);
+                           Index, Offset);
 }
 
-uint32_t DWARFDebugAddrTable::getLength() const {
-  if (HeaderData.Length == 0)
-    return 0;
+Optional<uint64_t> DWARFDebugAddrTable::getFullLength() const {
+  if (Length == 0)
+    return None;
   // TODO: DWARF64 support.
-  return HeaderData.Length + sizeof(uint32_t);
+  return Length + sizeof(uint32_t);
 }
 
-uint32_t DWARFDebugAddrTable::getDataSize() const {
-  if (DataSize != 0)
-    return DataSize;
-  if (getLength() == 0)
-    return 0;
-  return getLength() - getHeaderSize();
-}

diff --git a/llvm/test/tools/llvm-dwarfdump/X86/debug_addr_address_size_not_multiple.s b/llvm/test/tools/llvm-dwarfdump/X86/debug_addr_address_size_not_multiple.s
index eaae931..d758709 100644 (file)
--- a/llvm/test/tools/llvm-dwarfdump/X86/debug_addr_address_size_not_multiple.s
+++ b/llvm/test/tools/llvm-dwarfdump/X86/debug_addr_address_size_not_multiple.s
@@ -4,7 +4,7 @@
 
 # CHECK: .debug_addr contents:
 # CHECK-NOT: {{.}}
-# ERR: address table at offset 0x0 contains data of size 7 which is not a multiple of addr size 4
+# ERR: address table at offset 0x0 contains data of size 0x7 which is not a multiple of addr size 4
 # ERR-NOT: {{.}}
 
 # data size is not multiple of address_size

diff --git a/llvm/test/tools/llvm-dwarfdump/X86/debug_addr_unsupported_version.s b/llvm/test/tools/llvm-dwarfdump/X86/debug_addr_unsupported_version.s
index c2afea3..4eeeaa7 100644 (file)
--- a/llvm/test/tools/llvm-dwarfdump/X86/debug_addr_unsupported_version.s
+++ b/llvm/test/tools/llvm-dwarfdump/X86/debug_addr_unsupported_version.s
@@ -3,6 +3,7 @@
 # RUN: FileCheck %s -input-file %t.err -check-prefix=ERR
 
 # ERR: address table at offset 0x0 has unsupported version 6
+# ERR: address table at offset 0x20 has unsupported version 4
 # ERR-NOT: {{.}}
 
 # CHECK: .debug_addr contents
@@ -40,3 +41,13 @@
   .byte 0  # segment_selector_size
   .long 0x00000002
   .long 0x00000003
+
+       .section        .debug_addr,"",@progbits
+.Ldebug_addr2:
+  .long 12 # unit_length = .short + .byte + .byte + .long + .long
+  .short 4 # version
+  .byte 4  # address_size
+  .byte 0  # segment_selector_size
+  .long 0x00000000
+  .long 0x00000001
+

diff --git a/llvm/test/tools/llvm-dwarfdump/X86/debug_addr_version_mismatch.s b/llvm/test/tools/llvm-dwarfdump/X86/debug_addr_version_mismatch.s
deleted file mode 100644 (file)
index 788e102..0000000
--- a/llvm/test/tools/llvm-dwarfdump/X86/debug_addr_version_mismatch.s
+++ /dev/null
@@ -1,42 +0,0 @@
-# RUN: llvm-mc %s -filetype obj -triple i386-pc-linux -o - | \
-# RUN: llvm-dwarfdump -debug-addr - 2> %t.err | FileCheck %s
-# RUN: FileCheck %s -input-file %t.err -check-prefix=ERR
-
-# ERR: address table at offset 0x0 has version 4 which is different from the version suggested by the DWARF unit header: 5
-# ERR-NOT: {{.}}
-
-# CHECK: .debug_addr contents
-# CHECK-NEXT:     length = 0x0000000c, version = 0x0005, addr_size = 0x04, seg_size = 0x00
-# CHECK-NEXT:     Addrs: [
-# CHECK-NEXT:     0x00000000
-# CHECK-NEXT:     0x00000001
-# CHECK-NEXT:     ]
-# CHECK-NOT:      {{.}}
-
-       .section        .debug_abbrev,"",@progbits
-       .byte   1                       # Abbreviation Code
-       .section        .debug_info,"",@progbits
-.Lcu_begin0:
-       .long 8                       # Length of Unit
-       .short  5                     # DWARF version number
-  .byte 1                       # DWARF unit type
-       .byte   4                       # Address Size (in bytes)
-       .long   .debug_abbrev           # Offset Into Abbrev. Section
-
-       .section        .debug_addr,"",@progbits
-.Ldebug_addr0:
-  .long 12 # unit_length = .short + .byte + .byte + .long + .long
-  .short 4 # version
-  .byte 4  # address_size
-  .byte 0  # segment_selector_size
-  .long 0x00000000
-  .long 0x00000001
-
-       .section        .debug_addr,"",@progbits
-.Ldebug_addr1:
-  .long 12 # unit_length = .short + .byte + .byte + .long + .long
-  .short 5 # version
-  .byte 4  # address_size
-  .byte 0  # segment_selector_size
-  .long 0x00000000
-  .long 0x00000001