[DebugInfo]: Added preliminary support for DWARFv5 in llvm-dwp utility.

[SouraVX]

Summary

Summary: This patch adds preliminary support in llvm-dwp to package dwarfv5 DWO objects. Their are numerous updates in section contents in dwarfv5. This patch teaches llvm-dwp to parse revised debug_info and debug_str_offsets section based on the version information present in CU header.

[aprantl]

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

94

nit: this should be

// Emit the version.
Out.EmitIntValue(5, 2); 
// Emit two bytes padding.
Out.EmitIntValue(0, 2);

591

//

592

on *the* DWARF version

[SouraVX]

Thank You @aprantl
Incorporated some nits comments by @aprantl

[dblaikie]

Please split this into smaller/more incremental patches - it should be possible to implement the new support for parsing DWARFv5 unit headers, signature, etc, without the new string header support. (you'd have to modify/hand-craft the dwo file assembly though, since LLVM's generated DWARFv5 will always use FORM_strx - but if you change the dwo to use FORM_str (direct strings, no indirection) it should mean you can make a small but valid test case without the need for strx support)

llvm/test/tools/llvm-dwp/X86/simple-v5.test

1

While previous testing has used checked in binary object files (& I wrote those tests... ) - it seems a lot of testing even of tools like this has moved ot checking in assembly files and assuming/relying on llvm-mc as an assembler to make tests more legible.

Could you do that in this case? Use/add assembly rather than binary files, and have the test assemble those files then run them through llvm-dwp?

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

593–600

This is a rather strange construct - a named local lambda that's called only once. It doesn't seem appropriate.

If you especially want to reduce the scope of the extra variables, some options might include:

auto DwarfVersion;
{
  DataExtractor InfoData(InfoSection, true, 0);
  uint64_t Offset = 4;
  DwarfVersion = InfoData.getU16(&Offset);
}

This is usually only done if there's repeated use of similarly named variables, a large scope where these variables might get confused with others, etc. Which doesn't seem to be the case here.

or

auto DwarfVersion = [] {
  DataExtractor InfoData(InfoSection, true, 0);
  uint64_t Offset = 4;
  return InfoData.getU16(&Offset);
}();

The latter is usually only for when the variable being initialized is 'const'.

I think it's probably best to just write it more directly:

DataExtractor InfoData(InfoSection, true, 0);
uint64_t InfoVersionOffset = 4;
writeStringsAndOffsets(..., InfoData.getU16(&InfoVersionOffset));

[SouraVX]

In D74312#1869752, @dblaikie wrote:

Please split this into smaller/more incremental patches - it should be possible to implement the new support for parsing DWARFv5 unit headers, signature, etc, without the new string header support. (you'd have to modify/hand-craft the dwo file assembly though, since LLVM's generated DWARFv5 will always use FORM_strx - but if you change the dwo to use FORM_str (direct strings, no indirection) it should mean you can make a small but valid test case without the need for strx support)

Hi Thanks for review!
Sounds good to me, will segregate this in 2 patches.

1. With DWARFv5 Info section parsing support.
2. With DWARFv String Offset parsing support.

Regarding, testing I'm considering adding the test cases to be written and tested with llvm-mc in this directory llvm/test/tools/llvm-dwp/ . Didn't notice any debug-info related test case in llvm/test/MC/X86 directory. Do you have any other suitable place in mind, Please share.

[dblaikie]

In D74312#1869834, @SouraVX wrote:

In D74312#1869752, @dblaikie wrote:

Please split this into smaller/more incremental patches - it should be possible to implement the new support for parsing DWARFv5 unit headers, signature, etc, without the new string header support. (you'd have to modify/hand-craft the dwo file assembly though, since LLVM's generated DWARFv5 will always use FORM_strx - but if you change the dwo to use FORM_str (direct strings, no indirection) it should mean you can make a small but valid test case without the need for strx support)

Hi Thanks for review!
Sounds good to me, will segregate this in 2 patches.

1. With DWARFv5 Info section parsing support.
2. With DWARFv String Offset parsing support.

Regarding, testing I'm considering adding the test cases to be written and tested with llvm-mc in this directory llvm/test/tools/llvm-dwp/ . Didn't notice any debug-info related test case in llvm/test/MC/X86 directory. Do you have any other suitable place in mind, Please share.

Yes, llvm/test/tools/llvm-dwp would be the right place for the test (see similar tests using llvm-mc in llvm/test/tools/dsymutil or llvm/test/tools/llvm-dwarfdump etc)

llvm/test/tools/llvm-dwp/X86/simple-v5.test

+RUN: llvm-dwp %p/../Inputs/simple/notypes/a-v5.dwo %p/../Inputs/simple/notypes/b-v5.dwo -o %t
+RUN: llvm-dwarfdump -v %t | FileCheck %s
+
+a-v5.c:
+  struct foo { };
+  foo a;
+
+b-v5.c:
+  struct bar { };
+  void b(bar) {
+  }
+
+CHECK-LABEL: .debug_abbrev.dwo contents:
+CHECK-LABEL: Abbrev table for offset:
+CHECK: 0x0000[[AAOFF:.*]]
+CHECK: DW_TAG_compile_unit
+CHECK: DW_TAG_variable
+CHECK: DW_TAG_structure_type
+CHECK-LABEL: Abbrev table for offset:
+CHECK: 0x0000[[BAOFF:.*]]
+CHECK: DW_TAG_compile_unit
+CHECK: DW_TAG_subprogram
+CHECK: DW_TAG_formal_parameter
+CHECK: DW_TAG_structure_type
+
+CHECK: .debug_info.dwo contents:
+CHECK: [[AOFF:0x[0-9a-f]*]]:
+CHECK-LABEL: Compile Unit: length = {{.*}} version = 0x0005 unit_type = DW_UT_split_compile abbr_offset =
+CHECK:         0x[[AAOFF]] addr_size = 0x08 DWO_id = [[DWOA:.*]] (next unit at [[BOFF:.*]])
+CHECK: DW_TAG_compile_unit
+CHECK:   DW_AT_name {{.*}} "a-v5.c"
+CHECK:   DW_TAG_variable
+CHECK:     DW_AT_name {{.*}} "a"
+CHECK:   DW_TAG_structure_type
+CHECK:     DW_AT_name {{.*}} "foo"
+
+CHECK: [[BOFF]]:
+CHECK-LABEL: Compile Unit: length = {{.*}} version = 0x0005 unit_type = DW_UT_split_compile abbr_offset =
+CHECK:         0x[[BAOFF]] addr_size = 0x08 DWO_id = [[DWOB:.*]] (next unit at [[XOFF:.*]])
+CHECK:   DW_AT_name {{.*}} "b-v5.c"
+CHECK:   DW_TAG_subprogram
+CHECK:     DW_AT_name {{.*}} "b"
+CHECK:     DW_TAG_formal_parameter
+CHECK:   DW_TAG_structure_type
+CHECK:     DW_AT_name {{.*}} "bar"
+
+CHECK-LABEL: .debug_cu_index contents:
+CHECK: Index Signature         INFO                      ABBREV                             STR_OFFSETS
+CHECK:  1     [[DWOA]]  {{\[}}[[AOFF]], [[BOFF]]) [0x0000[[AAOFF]], 0x0000[[BAOFF]]) [0x00000000, 0x0000001c)
+CHECK:  3     [[DWOB]]  {{\[}}[[BOFF]], [[XOFF]]) [0x0000[[BAOFF]], 0x0000006b)      [0x0000001c, 0x0000003c)
+
+CHECK-LABEL: .debug_str.dwo contents:
+CHECK: "clang version
+CHECK: 0x[[AC:.*]]: "a-v5.c"
+CHECK-NOT: "clang version
+CHECK: 0x[[BC:.*]]: "b-v5.c"
+
+CHECK-LABEL: .debug_str_offsets.dwo contents:
+CHECK-NEXT: 0x00000000: Contribution size = 24, Format = DWARF32, Version = 5
+CHECK: : [[AC]]
+CHECK-LABEL: 0x0000001c: Contribution size = 28, Format = DWARF32, Version = 5
+CHECK: : [[BC]]

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

 static cl::opt<std::string> OutputFilename(cl::Required, "o",
                                            cl::desc("Specify the output file."),
                                            cl::value_desc("filename"),
                                            cl::cat(DwpCategory));
 
 static void writeStringsAndOffsets(MCStreamer &Out, DWPStringPool &Strings,
                                    MCSection *StrOffsetSection,
                                    StringRef CurStrSection,
-                                   StringRef CurStrOffsetSection) {
+                                   StringRef CurStrOffsetSection,
+                                   uint16_t Version) {
   // 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 Offset = 0;
   uint64_t Size = CurStrOffsetSection.size();
+  // Write DWARFv5 debug_str_offsets section header.
+  if (Version >= 5) {
+    // Unit_length field should not include the length field(4 bytes) it self.
+    Out.EmitIntValue(Size - 4, 4);
+    // Emit the Version.
+    Out.EmitIntValue(5, 2);
+    // Emit two bytes padding.
+    Out.EmitIntValue(0, 2);
+  }
+  uint64_t Offset = Version >= 5 ? 8 : 0;
   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) {
+  uint64_t StrOffsetsOffset = 0;
   if (Form == dwarf::DW_FORM_string)
     return InfoData.getCStr(&InfoOffset);
-  if (Form != dwarf::DW_FORM_GNU_str_index)
+  if (Form != dwarf::DW_FORM_strx1 && Form != dwarf::DW_FORM_strx2 &&
+      Form != dwarf::DW_FORM_strx3 && Form != dwarf::DW_FORM_strx4 &&
+      Form != dwarf::DW_FORM_GNU_str_index)
     return make_error<DWPError>(
-        "string field encoded without DW_FORM_string or DW_FORM_GNU_str_index");
+        "string field encoded without DW_FORM_string or DW_FORM_GNU_str_index \
+  or DW_FORM_strx*");
   auto StrIndex = InfoData.getULEB128(&InfoOffset);
   DataExtractor StrOffsetsData(StrOffsets, true, 0);
-  uint64_t StrOffsetsOffset = 4 * StrIndex;
+  if (Version >= 5)
+    StrOffsetsOffset = 4 * StrIndex + 8 /*Header Size*/;
+  else
+    StrOffsetsOffset = 4 * StrIndex;
   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;
   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>(
+          "unit type DW_UT_split_compile type not found in debug_info header");
+  }
   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);
 
   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;
-  CompileUnitIdentifiers ID;
-  Optional<uint64_t> Signature = None;
   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);
+          getIndexedString(Form, InfoData, Offset, StrOffsets, Str, Version);
       if (!EName)
         return EName.takeError();
       ID.Name = *EName;
       break;
     }
-    case dwarf::DW_AT_GNU_dwo_name: {
+    case dwarf::DW_AT_GNU_dwo_name:
+    case dwarf::DW_AT_dwo_name: {
       Expected<const char *> EName =
-          getIndexedString(Form, InfoData, Offset, StrOffsets, Str);
+          getIndexedString(Form, InfoData, Offset, StrOffsets, Str, Version);
       if (!EName)
         return EName.takeError();
       ID.DWOName = *EName;
       break;
     }
     case dwarf::DW_AT_GNU_dwo_id:
       Signature = InfoData.getU64(&Offset);
       break;
for (const auto &Section : Obj.sections())
               UncompressedSections, ContributionOffsets, CurEntry,
               CurStrSection, CurStrOffsetSection, CurTypesSection, InfoSection,
               AbbrevSection, CurCUIndexSection, CurTUIndexSection))
         return Err;
 
     if (InfoSection.empty())
       continue;
 
+    // Here we're extracting Dwarf Version from the Info Section to pass it
+    // for reconstruction of debug_str_offset section based on the Dwarf Version.
+    auto getVersion = [&]() -> uint16_t {
+      DataExtractor InfoData(InfoSection, true, 0);
+      // Version field is located just after length field in Info section
+      // header. For DWARF32 length field is of 4 bytes.
+      uint64_t Offset = 4;
+      return InfoData.getU16(&Offset);
+    };
+    auto DwarfVersion = getVersion();
     writeStringsAndOffsets(Out, Strings, StrOffsetSection, CurStrSection,
-                           CurStrOffsetSection);
+                           CurStrOffsetSection, DwarfVersion);
 
     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));