Handle DW_FORM_strx[1-4] in llvm-dwp

[kimanh]

Summary

Add suport for DW_FORM_STR[1-4] that are introduced in DWARF5. Adapted patch by tamur.

[kimanh]

Picking up patch by tamur: https://reviews.llvm.org/D75485

I've addressed the outstanding issues, i.e.:

1. Adding test cases for each error case in parseCompileUnitHeader(StringRef Info)
2. Changing the error message to reuse the message from Error in parseCompileUnitHeader(StringRef Info)
3. Adapted tests (minor change: test error message, and added required header info to unsupported_cu_index_version.s)

As it seems (https://reviews.llvm.org/D77143) llvm-dwp does not yet support a compile unit index version of 5, which is used in DWARF5. Will investigate (any input is welcome) later. However, with this patch (originally from tamur), we can now generate dwps.

PTAL :-)

[dblaikie]

Hmm - perhaps you could commandeer the previous review, then update it? That'd make it easier to see any changes you've made from the previous review.

[kimanh]

Updating the original review (https://reviews.llvm.org/D75485) and closing this one.

llvm/test/tools/llvm-dwp/X86/handle_strx.test

+RUN: llvm-dwp %p/../Inputs/handle_strx/dw5.dwo -o %t 2>/dev/null
+RUN: llvm-dwarfdump --verbose %t 2>/dev/null | FileCheck --check-prefix=READ_STRX %s
+
+RUN: not llvm-dwp %p/../Inputs/handle_strx/dw5.dwo %p/../Inputs/handle_strx/dw5.dwo -o %t 2>&1 | FileCheck --check-prefix=PARSE_STRX %s
+
+
+dw5.o is compiled from a file that contains a single empty void function,
+with options -gdwarf-5 and -gsplit-dwarf.
+
+READ_STRX: DW_AT_name [DW_FORM_strx1]{{.*}}dw5.cc
+
+PARSE_STRX: error: duplicate DWO ID ({{.*}}) in 'dw5.cc' and 'dw5.cc'
+

llvm/test/tools/llvm-dwp/X86/invalid_cu_header_length.s

+# RUN: llvm-mc -triple x86_64-unknown-linux %s -filetype=obj -o %t.o \
+# RUN:         -split-dwarf-file=%t.dwo -dwarf-version=5
+# RUN: not llvm-dwp %t.dwo -o %t.dwp 2>&1 | FileCheck %s
+
+# CHECK: error: Cannot parse compile unit length: unexpected end of data at offset 0x2 while reading [0x0, 0x4
+  .section  .debug_info.dwo,"e",@progbits
+  .short  0 # Length of Unit

llvm/test/tools/llvm-dwp/X86/invalid_cu_header_unittype.s

+# RUN: llvm-mc -triple x86_64-unknown-linux %s -filetype=obj -o %t.o \
+# RUN:         -split-dwarf-file=%t.dwo -dwarf-version=5
+# RUN: not llvm-dwp %t.dwo -o %t.dwp 2>&1 | FileCheck %s
+
+# CHECK: error: Cannot parse compile unit header: unexpected end of data at offset 0x6 while reading [0x6, 0x7)
+  .section  .debug_info.dwo,"e",@progbits
+  .long 2 # Length of Unit
+  .short 5

llvm/test/tools/llvm-dwp/X86/invalid_cu_header_version.s

+# RUN: llvm-mc -triple x86_64-unknown-linux %s -filetype=obj -o %t.o \
+# RUN:         -split-dwarf-file=%t.dwo -dwarf-version=5
+# RUN: not llvm-dwp %t.dwo -o %t.dwp 2>&1 | FileCheck %s
+
+# CHECK: error: Cannot parse compile unit version: unexpected end of data at offset 0x4 while reading [0x4, 0x6)
+  .section  .debug_info.dwo,"e",@progbits
+  .long 0 # Length of Unit

llvm/test/tools/llvm-dwp/X86/invalid_string_form.test

+RUN: not llvm-dwp %p/../Inputs/invalid_string_form.dwo -o %t 2>&1 | xargs echo
 RUN: not llvm-dwp %p/../Inputs/invalid_string_form.dwo -o %t 2>&1 | FileCheck %s
 
-CHECK: error: {{.*}}invalid_string_form.dwo': string field encoded without DW_FORM_string or DW_FORM_GNU_str_index
+CHECK: error: {{.*}}: string field must be encoded with one of the following:
+CHECK: DW_FORM_string, DW_FORM_strx, DW_FORM_strx1, DW_FORM_strx2, DW_FORM_strx3, DW_FORM_strx4, or DW_FORM_GNU_str_index.

llvm/test/tools/llvm-dwp/X86/unsupported_cu_index_version.s

 # RUN: llvm-mc -triple x86_64-unknown-linux %s -filetype=obj -o %t.dwp
 # RUN: not llvm-dwp %t.dwp -o %t 2>&1 | FileCheck %s
 
 # CHECK: error: unsupported cu_index version: 5 (only version 2 is supported)
 
 ## To reach the test point, no real data is needed in .debug_info.dwo,
 ## but the section should not be empty.
     .section .debug_info.dwo, "e", @progbits
-    .space 1
-
+    .long  .Ldebug_info_dwo_end0-.Ldebug_info_dwo_start0 # Length of Unit
+.Ldebug_info_dwo_start0:
+    .short 5                       # DWARF version number
+    .byte 5                        # DWARF Unit type
+    .byte 8                        # Address Size (in bytes)
+    .long 0                        # Offset Into Abbrev. Section
+    .quad -346972125991005518
+.Ldebug_info_dwo_end0:
     .section .debug_cu_index, "", @progbits
 ## Header:
     .short 5                        # Version
     .space 2                        # Padding
     .long 2                         # Section count
     .long 1                         # Unit count
     .long 2                         # Slot count
 ## Hash Table of Signatures:

llvm/tools/llvm-dwp/llvm-dwp.cpp

 //
 //===----------------------------------------------------------------------===//
 #include "DWPError.h"
 #include "DWPStringPool.h"
 #include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/DebugInfo/DWARF/DWARFContext.h"
+#include "llvm/DebugInfo/DWARF/DWARFDataExtractor.h"
 #include "llvm/DebugInfo/DWARF/DWARFFormValue.h"
 #include "llvm/DebugInfo/DWARF/DWARFUnitIndex.h"
 #include "llvm/MC/MCAsmBackend.h"
 #include "llvm/MC/MCAsmInfo.h"
 #include "llvm/MC/MCCodeEmitter.h"
 #include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCInstrInfo.h"
 #include "llvm/MC/MCObjectFileInfo.h"
static cl::list<std::string> ExecFilenames(
     cl::desc("Specify the executable/library files to get the list of *.dwo from"),
     cl::value_desc("filename"), cl::cat(DwpCategory));
 
 static cl::opt<std::string> OutputFilename(cl::Required, "o",
                                            cl::desc("Specify the output file."),
                                            cl::value_desc("filename"),
                                            cl::cat(DwpCategory));
 
+// Returns the size of debug_str_offsets section headers in bytes.
+static uint64_t debugStrOffsetsHeaderSize(DataExtractor StrOffsetsData,
+                                          uint16_t DwarfVersion) {
+  if (DwarfVersion <= 4)
+    return 0; // There is no header before dwarf 5.
+  uint64_t Offset = 0;
+  uint64_t Length = StrOffsetsData.getU32(&Offset);
+  if (Length == llvm::dwarf::DW_LENGTH_DWARF64)
+    return 16; // unit length: 12 bytes, version: 2 bytes, padding: 2 bytes.
+  return 8;    // unit length: 4 bytes, version: 2 bytes, padding: 2 bytes.
+}
+
+// Holds data for Skeleton and Split Compilation Unit Headers as defined in
+// Dwarf 5 specification, 7.5.1.2 and Dwarf 4 specification 7.5.1.1.
+struct CompileUnitHeader {
+  // unit_length field. Note that the type is uint64_t even in 32-bit dwarf.
+  uint64_t Length = 0;
+
+  // version field.
+  uint16_t Version = 0;
+
+  // unit_type field. Initialized only if Version >= 5.
+  uint8_t UnitType = 0;
+
+  // address_size field.
+  uint8_t AddrSize = 0;
+
+  // debug_abbrev_offset field. Note that the type is uint64_t even in 32-bit
+  // dwarf. It is assumed to be 0.
+  uint64_t DebugAbbrevOffset = 0;
+
+  // dwo_id field. This resides in the header only if Version >= 5.
+  // In earlier versions, it is read from DW_AT_GNU_dwo_id.
+  Optional<uint64_t> Signature = None;
+
+  // Derived from the length of Length field.
+  dwarf::DwarfFormat Format = dwarf::DwarfFormat::DWARF32;
+
+  // The size of the Header in bytes. This is derived while parsing the header,
+  // and is stored as a convenience.
+  uint8_t HeaderSize = 0;
+};
+
+// Parse and return the header of the compile unit.
+static Expected<CompileUnitHeader> parseCompileUnitHeader(StringRef Info) {
+  CompileUnitHeader Header;
+  Error Err = Error::success();
+  uint64_t Offset = 0;
+  DWARFDataExtractor InfoData(Info, true, 0);
+  std::tie(Header.Length, Header.Format) =
+      InfoData.getInitialLength(&Offset, &Err);
+  if (Err)
+    return make_error<DWPError>("Cannot parse compile unit length: " +
+                                llvm::toString(std::move(Err)) + "\n");
+  Header.Version = InfoData.getU16(&Offset, &Err);
+  if (Err)
+    return make_error<DWPError>("Cannot parse compile unit version: " +
+                                llvm::toString(std::move(Err)) + "\n");
+  if (Header.Version >= 5) {
+    Header.UnitType = InfoData.getU8(&Offset, &Err);
+    Header.AddrSize = InfoData.getU8(&Offset, &Err);
+    Header.DebugAbbrevOffset = InfoData.getU32(&Offset, &Err);
+    Header.Signature = InfoData.getU64(&Offset, &Err);
+  } else {
+    // Note that, address_size and debug_abbrev_offset fields have switched
+    // places between dwarf version 4 and 5.
+    Header.DebugAbbrevOffset = InfoData.getU32(&Offset, &Err);
+    Header.AddrSize = InfoData.getU8(&Offset, &Err);
+  }
+  if (Err)
+    return make_error<DWPError>("Cannot parse compile unit header: " +
+                                llvm::toString(std::move(Err)) + "\n");
+  Header.HeaderSize = Offset;
+  return Header;
+}
+
 static void writeStringsAndOffsets(MCStreamer &Out, DWPStringPool &Strings,
                                    MCSection *StrOffsetSection,
                                    StringRef CurStrSection,
-                                   StringRef CurStrOffsetSection) {
+                                   StringRef CurStrOffsetSection,
+                                   const CompileUnitHeader &Header) {
   // Could possibly produce an error or warning if one of these was non-null but
   // the other was null.
   if (CurStrSection.empty() || CurStrOffsetSection.empty())
     return;
 
   DenseMap<uint64_t, uint32_t> OffsetRemapping;
 
   DataExtractor Data(CurStrSection, true, 0);
   uint64_t LocalOffset = 0;
   uint64_t PrevOffset = 0;
   while (const char *s = Data.getCStr(&LocalOffset)) {
     OffsetRemapping[PrevOffset] =
         Strings.getOffset(s, LocalOffset - PrevOffset);
     PrevOffset = LocalOffset;
   }
 
   Data = DataExtractor(CurStrOffsetSection, true, 0);
 
   Out.SwitchSection(StrOffsetSection);
 
+  uint64_t HeaderSize = debugStrOffsetsHeaderSize(Data, Header.Version);
   uint64_t Offset = 0;
   uint64_t Size = CurStrOffsetSection.size();
+  // FIXME: This can be caused by bad input and should be handled as such.
+  assert(HeaderSize <= Size && "StrOffsetSection size is less than its header");
+  // Copy the header to the output.
+  Out.emitBytes(Data.getBytes(&Offset, HeaderSize));
   while (Offset < Size) {
     auto OldOffset = Data.getU32(&Offset);
     auto NewOffset = OffsetRemapping[OldOffset];
     Out.emitIntValue(NewOffset, 4);
   }
 }
 
 static uint64_t getCUAbbrev(StringRef Abbrev, uint64_t AbbrCode) {

 
 struct CompileUnitIdentifiers {
   uint64_t Signature = 0;
   const char *Name = "";
   const char *DWOName = "";
 };
 
 static Expected<const char *>
-getIndexedString(dwarf::Form Form, DataExtractor InfoData,
-                 uint64_t &InfoOffset, StringRef StrOffsets, StringRef Str) {
+getIndexedString(dwarf::Form Form, DataExtractor InfoData, uint64_t &InfoOffset,
+                 StringRef StrOffsets, StringRef Str, uint16_t Version) {
   if (Form == dwarf::DW_FORM_string)
     return InfoData.getCStr(&InfoOffset);
-  if (Form != dwarf::DW_FORM_GNU_str_index)
+  uint64_t StrIndex;
+  switch (Form) {
+  case dwarf::DW_FORM_strx1:
+    StrIndex = InfoData.getU8(&InfoOffset);
+    break;
+  case dwarf::DW_FORM_strx2:
+    StrIndex = InfoData.getU16(&InfoOffset);
+    break;
+  case dwarf::DW_FORM_strx3:
+    StrIndex = InfoData.getU24(&InfoOffset);
+    break;
+  case dwarf::DW_FORM_strx4:
+    StrIndex = InfoData.getU32(&InfoOffset);
+    break;
+  case dwarf::DW_FORM_strx:
+  case dwarf::DW_FORM_GNU_str_index:
+    StrIndex = InfoData.getULEB128(&InfoOffset);
+    break;
+  default:
     return make_error<DWPError>(
-        "string field encoded without DW_FORM_string or DW_FORM_GNU_str_index");
-  auto StrIndex = InfoData.getULEB128(&InfoOffset);
+        "string field must be encoded with one of the following: "
+        "DW_FORM_string, DW_FORM_strx, DW_FORM_strx1, DW_FORM_strx2, "
+        "DW_FORM_strx3, DW_FORM_strx4, or DW_FORM_GNU_str_index.");
+  }
   DataExtractor StrOffsetsData(StrOffsets, true, 0);
   uint64_t StrOffsetsOffset = 4 * StrIndex;
+  StrOffsetsOffset += debugStrOffsetsHeaderSize(StrOffsetsData, Version);
+
   uint64_t StrOffset = StrOffsetsData.getU32(&StrOffsetsOffset);
   DataExtractor StrData(Str, true, 0);
   return StrData.getCStr(&StrOffset);
 }
 
 static Expected<CompileUnitIdentifiers> getCUIdentifiers(StringRef Abbrev,
                                                          StringRef Info,
                                                          StringRef StrOffsets,
                                                          StringRef Str) {
-  uint64_t Offset = 0;
+  Expected<CompileUnitHeader> HeaderOrError = parseCompileUnitHeader(Info);
+  if (!HeaderOrError)
+    return HeaderOrError.takeError();
+  CompileUnitHeader &Header = *HeaderOrError;
   DataExtractor InfoData(Info, true, 0);
-  dwarf::DwarfFormat Format = dwarf::DwarfFormat::DWARF32;
-  uint64_t Length = InfoData.getU32(&Offset);
-  CompileUnitIdentifiers ID;
-  Optional<uint64_t> Signature = None;
-  // If the length is 0xffffffff, then this indictes that this is a DWARF 64
-  // stream and the length is actually encoded into a 64 bit value that follows.
-  if (Length == 0xffffffffU) {
-    Format = dwarf::DwarfFormat::DWARF64;
-    Length = InfoData.getU64(&Offset);
-  }
-  uint16_t Version = InfoData.getU16(&Offset);
-  if (Version >= 5) {
-    auto UnitType = InfoData.getU8(&Offset);
-    if (UnitType != dwarf::DW_UT_split_compile)
-      return make_error<DWPError>(
-          std::string("unit type DW_UT_split_compile type not found in "
-                      "debug_info header. Unexpected unit type 0x" +
-                      utostr(UnitType) + " found"));
-  }
-  InfoData.getU32(&Offset); // Abbrev offset (should be zero)
-  uint8_t AddrSize = InfoData.getU8(&Offset);
-  if (Version >= 5)
-    Signature = InfoData.getU64(&Offset);
-  uint32_t AbbrCode = InfoData.getULEB128(&Offset);
+  uint64_t Offset = Header.HeaderSize;
+  if (Header.Version >= 5 && Header.UnitType != dwarf::DW_UT_split_compile)
+                    utostr(Header.UnitType) + " found"));
 
+  CompileUnitIdentifiers ID;
+
+  uint32_t AbbrCode = InfoData.getULEB128(&Offset);
   DataExtractor AbbrevData(Abbrev, true, 0);
   uint64_t AbbrevOffset = getCUAbbrev(Abbrev, AbbrCode);
   auto Tag = static_cast<dwarf::Tag>(AbbrevData.getULEB128(&AbbrevOffset));
   if (Tag != dwarf::DW_TAG_compile_unit)
     return make_error<DWPError>("top level DIE is not a compile unit");
   // DW_CHILDREN
   AbbrevData.getU8(&AbbrevOffset);
   uint32_t Name;
   dwarf::Form Form;
   while ((Name = AbbrevData.getULEB128(&AbbrevOffset)) |
          (Form = static_cast<dwarf::Form>(AbbrevData.getULEB128(&AbbrevOffset))) &&
          (Name != 0 || Form != 0)) {
     switch (Name) {
     case dwarf::DW_AT_name: {
-      Expected<const char *> EName =
-          getIndexedString(Form, InfoData, Offset, StrOffsets, Str);
+      Expected<const char *> EName = getIndexedString(
+          Form, InfoData, Offset, StrOffsets, Str, Header.Version);
       if (!EName)
         return EName.takeError();
       ID.Name = *EName;
       break;
     }
     case dwarf::DW_AT_GNU_dwo_name:
     case dwarf::DW_AT_dwo_name: {
-      Expected<const char *> EName =
-          getIndexedString(Form, InfoData, Offset, StrOffsets, Str);
+      Expected<const char *> EName = getIndexedString(
+          Form, InfoData, Offset, StrOffsets, Str, Header.Version);
       if (!EName)
         return EName.takeError();
       ID.DWOName = *EName;
       break;
     }
     case dwarf::DW_AT_GNU_dwo_id:
-      Signature = InfoData.getU64(&Offset);
+      Header.Signature = InfoData.getU64(&Offset);
       break;
     default:
-      DWARFFormValue::skipValue(Form, InfoData, &Offset,
-                                dwarf::FormParams({Version, AddrSize, Format}));
+      DWARFFormValue::skipValue(
+          Form, InfoData, &Offset,
+          dwarf::FormParams({Header.Version, Header.AddrSize, Header.Format}));
     }
   }
-  if (!Signature)
+  if (!Header.Signature)
     return make_error<DWPError>("compile unit missing dwo_id");
-  ID.Signature = *Signature;
+  ID.Signature = *Header.Signature;
   return ID;
 }
 
 struct UnitIndexEntry {
   DWARFUnitIndex::Entry::SectionContribution Contributions[8];
   std::string Name;
   std::string DWOName;
   StringRef DWPName;
for (const auto &Section : Obj.sections())
               UncompressedSections, ContributionOffsets, CurEntry,
               CurStrSection, CurStrOffsetSection, CurTypesSection, InfoSection,
               AbbrevSection, CurCUIndexSection, CurTUIndexSection))
         return Err;
 
     if (InfoSection.empty())
       continue;
 
+    Expected<CompileUnitHeader> CompileUnitHeaderOrErr =
+        parseCompileUnitHeader(InfoSection);
+    if (!CompileUnitHeaderOrErr)
+      return CompileUnitHeaderOrErr.takeError();
+    CompileUnitHeader &CompileUnitHeader = *CompileUnitHeaderOrErr;
+
     writeStringsAndOffsets(Out, Strings, StrOffsetSection, CurStrSection,
-                           CurStrOffsetSection);
+                           CurStrOffsetSection, CompileUnitHeader);
 
     if (CurCUIndexSection.empty()) {
       Expected<CompileUnitIdentifiers> EID = getCUIdentifiers(
           AbbrevSection, InfoSection, CurStrOffsetSection, CurStrSection);
       if (!EID)
         return createFileError(Input, EID.takeError());
       const auto &ID = *EID;
       auto P = IndexEntries.insert(std::make_pair(ID.Signature, CurEntry));