DWARF: When parsing type unit headers take the length field into account.

[wolfgangp]

Summary

This patch is split off from https://reviews.llvm.org/D32779. It fixes a bug in the parsing of type unit headers. The bug will not manifest with compiler-generated TU dies due to their size, but it is there nonetheless and will show up with hand-crafted TU dies for testing purposes. This is in preparation for DWARF5 work.

The DWARF64 portion of the fix is not tested (yet), because in order to do so the DWARFUnit class would have to support DWARF64. This is not the case at the moment.

[dblaikie]

Might be worth keeping the DWARF64 change separately until it's testable/valid, so test coverage can properly be assessed at that point.

test/DebugInfo/Inputs/typeunit-header.s

11–12 ↗

(On Diff #98238)

Do you need to give it any attributes at all?

[wolfgangp]

test/DebugInfo/Inputs/typeunit-header.s

11–12 ↗

(On Diff #98238)

Technically no, but I thought it would lend a little more substance to the test.

[dblaikie]

test/DebugInfo/Inputs/typeunit-header.s

11–12 ↗

(On Diff #98238)

I'd probably skip it entirely - otherwise it leads to the question "why exactly one? is there some special/interesting case there?" which may make it harder to modify the test in the future for fear that the reader is missing something important.

llvm/trunk/lib/DebugInfo/DWARF/DWARFTypeUnit.cpp

 using namespace llvm;
 
 bool DWARFTypeUnit::extractImpl(DataExtractor debug_info,
                                 uint32_t *offset_ptr) {
   if (!DWARFUnit::extractImpl(debug_info, offset_ptr))
     return false;
   TypeHash = debug_info.getU64(offset_ptr);
   TypeOffset = debug_info.getU32(offset_ptr);
-  return TypeOffset < getLength();
+  // TypeOffset is relative to the beginning of the header,
+  // so we have to account for the leading length field.
+  // FIXME: The size of the length field is 12 in DWARF64.
+  unsigned SizeOfLength = 4;
+  return TypeOffset < getLength() + SizeOfLength;
 }
 
 void DWARFTypeUnit::dump(raw_ostream &OS, bool SummarizeTypes) {
   DWARFDie TD = getDIEForOffset(TypeOffset + getOffset());
   const char *Name = TD.getName(DINameKind::ShortName);
 
   if (SummarizeTypes) {
     OS << "name = '" << Name << "'"

llvm/trunk/test/DebugInfo/Inputs/typeunit-header.s

+# Test object with an artifically constructed type unit header to verify 
+# that the length field is correctly used to verify the validity of the
+# type_offset field.
+#
+# To generate the test object:
+# llvm-mc -triple x86_64-unknown-linux typeunit-header.s -filetype=obj \
+#         -o typeunit-header.elf-x86-64
+#
+# We only have an abbreviation for the type unit die which is all we need.
+# Real type unit dies have quite different attributes of course, but we
+# just need to demonstrate an issue with validating length, so we just give it
+# a single visibility attribute.
+        .section .debug_abbrev,"",@progbits
+        .byte 0x01  # Abbrev code
+        .byte 0x41  # DW_TAG_type_unit
+        .byte 0x01  # DW_CHILDREN_yes
+        .byte 0x17  # DW_AT_visibility
+        .byte 0x0b  # DW_FORM_data1
+        .byte 0x00  # EOM(1)
+        .byte 0x00  # EOM(2)
+        .byte 0x02  # Abbrev code
+        .byte 0x13  # DW_TAG_structure_type
+        .byte 0x00  # DW_CHILDREN_no (no members)
+        .byte 0x17  # DW_AT_visibility
+        .byte 0x0b  # DW_FORM_data1
+        .byte 0x00  # EOM(1)
+        .byte 0x00  # EOM(2)
+        .byte 0x00  # EOM(3)
+        
+        .section .debug_types,"",@progbits
+# DWARF v4 Type unit header - DWARF32 format.
+TU_4_32_start:
+        .long TU_4_32_end-TU_4_32_version  # Length of Unit
+TU_4_32_version:
+        .short 4               # DWARF version number
+        .long .debug_abbrev    # Offset Into Abbrev. Section
+        .byte 8                # Address Size (in bytes)
+        .quad 0x0011223344556677 # Type Signature
+        .long TU_4_32_type-TU_4_32_start # Type offset
+# The type-unit DIE, which has just a visibility attribute.
+        .byte 1                # Abbreviation code
+        .byte 1                # DW_VIS_local
+# The type DIE, which also just has a one-byte visibility attribute.
+TU_4_32_type:
+        .byte 2                # Abbreviation code
+        .byte 1                # DW_VIS_local
+        .byte 0 # NULL
+        .byte 0 # NULL
+TU_4_32_end:

llvm/trunk/test/DebugInfo/typeunit-header.test

+RUN: llvm-dwarfdump %p/Inputs/typeunit-header.elf-x86-64 | FileCheck %s
+
+This is testing a bugfix where parsing the type unit header was not 
+taking the unit's intial length field into account when validating.
+
+The input file is hand-coded assembler to generate a type unit stub,
+which only contains a type unit DIE with a sole visibility attribute.
+
+We make sure that llvm-dwarfdump is parsing the type unit header correctly
+and displays it. 
+
+CHECK: .debug_types contents:
+CHECK: 0x00000000: Type Unit: length = 0x00000019 version = 0x0004 abbr_offset = 0x0000 addr_size = 0x08 name = '' type_signature = 0x0011223344556677 type_offset = 0x0019 (next unit at 0x0000001d)
+CHECK: 0x00000017: DW_TAG_type_unit [1] *
+CHECK: DW_AT_visibility [DW_FORM_data1] (DW_VIS_local)