[DebugInfo] Add support for DWARF64 into DWARFDebugAddr.

[aprantl]

llvm/lib/DebugInfo/DWARF/DWARFDebugAddr.cpp

68

DiagnosticLength?

161

unsigned?

[dblaikie]

llvm/lib/DebugInfo/DWARF/DWARFDebugAddr.cpp

55–75

@labath another instance of this 32/64 length parsing/error dance and subsequent "isValid" checking following it that'd be great to tidy up with a common utility for parsing the length and returning a length-constrained DWARFDataExtractor for further use.

[ikudrin]

llvm/lib/DebugInfo/DWARF/DWARFDebugAddr.cpp

68

Thanks! I'll change the name, as well as for the other similar instances.

161

Could you explain why unsinged is better than int here?

Note that according to http://www.cplusplus.com/reference/cstdio/printf/,

*: The width is not specified in the format string, but as an additional integer value argument preceding the argument that has to be formatted.

Not "an unsigned integer value".

[ikudrin]

• Update to reflect changes in parent revisions.
• TmpLength -> DiagnosticLength.
• Added a test for insufficient space for an extended length field.

[jhenderson]

llvm/lib/DebugInfo/DWARF/DWARFDebugAddr.cpp

57

IIRC, the DataExtractor class provides an optional second argument to its get... methods that can be used to specify an Error to pass any problems back in, including reading past the end. Would it make some sense to use that instead of the isValid... checks?

72–74

Maybe errc::not_supported?

161

I find the following link tends to be more precise and accurate in its wording:

https://en.cppreference.com/w/cpp/io/c/fprintf

(optional) integer value or * that specifies minimum field width. The result is padded with space characters (by default), if required, on the left when right-justified, or on the right if left-justified. In the case when * is used, the width is specified by an additional argument of type int. If the value of the argument is negative, it results with the - flag specified and positive field width. (Note: This is the minimum width: The value is never truncated.)

I don't have a copy of the standard to see whether the type is explicitly mentioned, but that quote definitively says int.

[aprantl]

llvm/lib/DebugInfo/DWARF/DWARFDebugAddr.cpp

161

Cool, I wasn't aware of that.

[ikudrin]

llvm/lib/DebugInfo/DWARF/DWARFDebugAddr.cpp

161

Oh, that reference is great. They even have a sample with a negative value for field width.

[jhenderson]

llvm/lib/DebugInfo/DWARF/DWARFDebugAddr.cpp

161

Huh, I didn't notice that, nor did I realise that left-justification was even possible!

[ikudrin]

llvm/lib/DebugInfo/DWARF/DWARFDebugAddr.cpp

57

Can't say I like that because the resulting diagnostic messages might not be that informative as with the current approach.

[ikudrin]

• Updated to reflect changes in parent revisions.
• Changed errc::invalid_argument to errc::not_supported for the message for a reserved unit length value.

[jhenderson]

LGTM.

llvm/include/llvm/DebugInfo/DWARF/DWARFDebugAddr.h

 /// A class representing an address table as specified in DWARF v5.
 /// The table consists of a header followed by an array of address values from
 /// .debug_addr section.
 class DWARFDebugAddrTable {
   dwarf::DwarfFormat Format;
   uint64_t Offset;
   /// The total length of the entries for this table, not including the length
   /// field itself.
-  uint32_t Length = 0;
+  uint64_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.

llvm/lib/DebugInfo/DWARF/DWARFDebugAddr.cpp

Error DWARFDebugAddrTable::extractV5(const DWARFDataExtractor &Data,
                                      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,
                              Offset);
-  // TODO: Add support for DWARF64.
   Format = dwarf::DwarfFormat::DWARF32;
   Length = Data.getU32(OffsetPtr);
   if (Length == dwarf::DW_LENGTH_DWARF64) {
+    // Check that we can read the extended unit length field.
+    if (!Data.isValidOffsetForDataOfSize(*OffsetPtr, 8)) {
+      invalidateLength();
+      return createStringError(
+          errc::invalid_argument,
+          "section is not large enough to contain an extended length field "
+          "of the address table at offset 0x%" PRIx64,
+          Offset);
+    }
+    Format = dwarf::DwarfFormat::DWARF64;
+    Length = Data.getU64(OffsetPtr);
+  } else if (Length >= dwarf::DW_LENGTH_lo_reserved) {
+    uint64_t DiagnosticLength = Length;
     invalidateLength();
     return createStringError(
         errc::not_supported,
-        "DWARF64 is not supported in .debug_addr at offset 0x%" PRIx64, Offset);
+        "address table at offset 0x%" PRIx64
+        " has unsupported reserved unit length of value 0x%" PRIx64,
+        Offset, DiagnosticLength);
   }
 
   if (!Data.isValidOffsetForDataOfSize(*OffsetPtr, Length)) {
-    uint32_t DiagnosticLength = Length;
+    uint64_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,
+        "at offset 0x%" PRIx64 " with a unit_length value of 0x%" PRIx64,
         Offset, DiagnosticLength);
   }
   uint64_t EndOffset = *OffsetPtr + Length;
   // Ensure that we can read the remaining header fields.
   if (Length < 4) {
-    uint32_t DiagnosticLength = Length;
+    uint64_t DiagnosticLength = Length;
     invalidateLength();
     return createStringError(
         errc::invalid_argument,
         "address table at offset 0x%" PRIx64
-        " has a unit_length value of 0x%" PRIx32
+        " has a unit_length value of 0x%" PRIx64
         ", which is too small to contain a complete header",
         Offset, DiagnosticLength);
   }
 
   Version = Data.getU16(OffsetPtr);
   AddrSize = Data.getU8(OffsetPtr);
   SegSize = Data.getU8(OffsetPtr);
 
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 ": ", Offset);
-  if (Length)
-    OS << format("Address table header: length = 0x%8.8" PRIx32
+    OS << format("0x%8.8" PRIx64 ": ", Offset);
+  if (Length) {
+    int LengthFieldWidth = (Format == dwarf::DwarfFormat::DWARF64) ? 16 : 8;
+    OS << format("Address table header: length = 0x%0*" PRIx64
                  ", version = 0x%4.4" PRIx16 ", addr_size = 0x%2.2" PRIx8
                  ", seg_size = 0x%2.2" PRIx8 "\n",
-                 Length, Version, AddrSize, SegSize);
+                 LengthFieldWidth, Length, Version, AddrSize, SegSize);
+  }
 
   if (Addrs.size() > 0) {
     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);
     OS << "]\n";
   }
 }
 
 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 of the "
                            "address table at offset 0x%" PRIx64,
                            Index, Offset);
 }
 
 Optional<uint64_t> DWARFDebugAddrTable::getFullLength() const {
   if (Length == 0)
     return None;
-  // TODO: DWARF64 support.
-  return Length + sizeof(uint32_t);
+  return Length + dwarf::getUnitLengthFieldByteSize(Format);
 }
 

llvm/test/tools/llvm-dwarfdump/X86/debug_addr_dwarf64.s

-# 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
+# RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux -o - | \
+# RUN:   llvm-dwarfdump -debug-addr - | \
+# RUN:   FileCheck %s
 
-# CHECK: .debug_addr contents:
-# CHECK-NOT: {{.}}
-# ERR: DWARF64 is not supported in .debug_addr at offset 0x0
-# ERR-NOT: {{.}}
+# CHECK:      .debug_addr contents:
+# CHECK-NEXT: Address table header:
+# CHECK-SAME: length = 0x000000000000000c,
+# CHECK-SAME: version = 0x0005,
+# CHECK-SAME: addr_size = 0x04,
+# CHECK-SAME: seg_size = 0x00
+# CHECK-NEXT: Addrs: [
+# CHECK-NEXT: 0x00000000
+# CHECK-NEXT: 0x00001000
+# CHECK-NEXT: ]
 
-# DWARF64 table
-  .section  .debug_addr,"",@progbits
-.Ldebug_addr0:
-  .long 0xffffffff # unit_length DWARF64 mark
-  .quad 12         # unit_length
-  .short 5         # version
-  .byte 3          # address_size
-  .byte 0          # segment_selector_size
+    .section .debug_addr,"",@progbits
+    .long 0xffffffff                    # DWARF64 mark
+    .quad .LAddr0end-.LAddr0version     # Length
+.LAddr0version:
+    .short 5                            # Version
+    .byte 4                             # Address size
+    .byte 0                             # Segment selector size
     .long 0x00000000
-  .long 0x00000001
+    .long 0x00001000
+.LAddr0end:

llvm/test/tools/llvm-dwarfdump/X86/debug_addr_reserved_length.s

+# RUN: llvm-mc %s -filetype obj -triple x86_64-pc-linux -o - | \
+# RUN:   llvm-dwarfdump --debug-addr - 2>&1 | \
+# RUN:   FileCheck %s --implicit-check-not=error
+
+# CHECK: error: address table at offset 0x0 has unsupported reserved unit length of value 0xfffffff0
+
+.section .debug_addr,"",@progbits
+.long 0xfffffff0

llvm/test/tools/llvm-dwarfdump/X86/debug_addr_too_small_for_extended_length_field.s

+# 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
+
+# CHECK: .debug_addr contents:
+# CHECK-NOT: {{.}}
+# ERR: section is not large enough to contain an extended length field of the address table at offset 0x0
+# ERR-NOT: {{.}}
+
+# too small section to contain an extended length field of a DWARF64 address table.
+  .section  .debug_addr,"",@progbits
+  .long 0xffffffff      # DWARF64 mark
+  .space 7              # a DWARF64 unit length field takes 8 bytes.