From dc1661239358efb8144a179d6706d7faec7fec2f Mon Sep 17 00:00:00 2001 From: Igor Kudrin Date: Thu, 6 Feb 2020 14:14:12 +0700 Subject: [PATCH] [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 --- llvm/include/llvm/DebugInfo/DWARF/DWARFDebugAddr.h | 86 +++---- llvm/lib/DebugInfo/DWARF/DWARFContext.cpp | 11 +- llvm/lib/DebugInfo/DWARF/DWARFDebugAddr.cpp | 246 ++++++++++----------- .../X86/debug_addr_address_size_not_multiple.s | 2 +- .../X86/debug_addr_unsupported_version.s | 11 + .../X86/debug_addr_version_mismatch.s | 42 ---- 6 files changed, 173 insertions(+), 225 deletions(-) delete mode 100644 llvm/test/tools/llvm-dwarfdump/X86/debug_addr_version_mismatch.s diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugAddr.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFDebugAddr.h index 4539b9c..fd9c5dc 100644 --- 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 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 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 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 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 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 --- 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 --- 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 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 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 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 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 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 --- 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 --- 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 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 -- 2.7.4