Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DWARFYAML] Implement debug_names support #79666

Merged
merged 6 commits into from
Feb 12, 2024

Conversation

felipepiovezan
Copy link
Contributor

This commit brings support for debug_names in DWARFYAML. It parses YAML and generates emits a DWARF5 Accelerator table with the following limitations:

  1. All forms must have a fixed length (zero length is also ok).
  2. Hard-coded support for DWARF 5 and DWARF32.
  3. The generated table does not contain a hash index

All of these limitations can be lifted in the future, but for now this is good enough to enable testing.

This commit brings support for debug_names in DWARFYAML. It parses YAML and
generates emits a DWARF5 Accelerator table with the following limitations:

1. All forms must have a fixed length (zero length is also ok).
2. Hard-coded support for DWARF 5 and DWARF32.
3. The generated table does not contain a hash index

All of these limitations can be lifted in the future, but for now this is good
enough to enable testing.
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 26, 2024

@llvm/pr-subscribers-debuginfo

@llvm/pr-subscribers-objectyaml

Author: Felipe de Azevedo Piovezan (felipepiovezan)

Changes

This commit brings support for debug_names in DWARFYAML. It parses YAML and generates emits a DWARF5 Accelerator table with the following limitations:

  1. All forms must have a fixed length (zero length is also ok).
  2. Hard-coded support for DWARF 5 and DWARF32.
  3. The generated table does not contain a hash index

All of these limitations can be lifted in the future, but for now this is good enough to enable testing.


Full diff: https://github.com/llvm/llvm-project/pull/79666.diff

5 Files Affected:

  • (modified) llvm/include/llvm/ObjectYAML/DWARFEmitter.h (+1)
  • (modified) llvm/include/llvm/ObjectYAML/DWARFYAML.h (+49)
  • (modified) llvm/lib/ObjectYAML/DWARFEmitter.cpp (+186)
  • (modified) llvm/lib/ObjectYAML/DWARFYAML.cpp (+29)
  • (modified) llvm/unittests/DebugInfo/DWARF/DWARFAcceleratorTableTest.cpp (+161)
diff --git a/llvm/include/llvm/ObjectYAML/DWARFEmitter.h b/llvm/include/llvm/ObjectYAML/DWARFEmitter.h
index ee421b2efc72bc..5e1b88f4fef649 100644
--- a/llvm/include/llvm/ObjectYAML/DWARFEmitter.h
+++ b/llvm/include/llvm/ObjectYAML/DWARFEmitter.h
@@ -42,6 +42,7 @@ Error emitDebugAddr(raw_ostream &OS, const Data &DI);
 Error emitDebugStrOffsets(raw_ostream &OS, const Data &DI);
 Error emitDebugRnglists(raw_ostream &OS, const Data &DI);
 Error emitDebugLoclists(raw_ostream &OS, const Data &DI);
+Error emitDebugNames(raw_ostream &OS, const Data &DI);
 
 std::function<Error(raw_ostream &, const Data &)>
 getDWARFEmitterByName(StringRef SecName);
diff --git a/llvm/include/llvm/ObjectYAML/DWARFYAML.h b/llvm/include/llvm/ObjectYAML/DWARFYAML.h
index a70ddf3a180a2d..5d1213d9ff965b 100644
--- a/llvm/include/llvm/ObjectYAML/DWARFYAML.h
+++ b/llvm/include/llvm/ObjectYAML/DWARFYAML.h
@@ -118,6 +118,28 @@ struct Unit {
   std::vector<Entry> Entries;
 };
 
+struct IdxForm {
+  llvm::dwarf::Index Idx;
+  llvm::dwarf::Form Form;
+};
+
+struct DebugNameAbbreviation {
+  llvm::yaml::Hex64 Code;
+  llvm::dwarf::Tag Tag;
+  std::vector<IdxForm> Indices;
+};
+
+struct DebugNameEntry {
+  llvm::yaml::Hex32 NameStrp;
+  llvm::yaml::Hex64 Code;
+  std::vector<llvm::yaml::Hex64> Values;
+};
+
+struct DebugNamesSection {
+  std::vector<DebugNameAbbreviation> Abbrevs;
+  std::vector<DebugNameEntry> Entries;
+};
+
 struct File {
   StringRef Name;
   uint64_t DirIdx;
@@ -228,6 +250,7 @@ struct Data {
   std::vector<LineTable> DebugLines;
   std::optional<std::vector<ListTable<RnglistEntry>>> DebugRnglists;
   std::optional<std::vector<ListTable<LoclistEntry>>> DebugLoclists;
+  std::optional<DebugNamesSection> DebugNames;
 
   bool isEmpty() const;
 
@@ -276,10 +299,26 @@ LLVM_YAML_IS_SEQUENCE_VECTOR(
     llvm::DWARFYAML::ListEntries<DWARFYAML::LoclistEntry>)
 LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::DWARFYAML::LoclistEntry)
 LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::DWARFYAML::DWARFOperation)
+LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::DWARFYAML::DebugNameEntry)
+LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::DWARFYAML::DebugNameAbbreviation)
+LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::DWARFYAML::IdxForm)
 
 namespace llvm {
 namespace yaml {
 
+template <> struct MappingTraits<DWARFYAML::DebugNamesSection> {
+  static void mapping(IO &IO, DWARFYAML::DebugNamesSection &);
+};
+template <> struct MappingTraits<DWARFYAML::DebugNameEntry> {
+  static void mapping(IO &IO, DWARFYAML::DebugNameEntry &);
+};
+template <> struct MappingTraits<DWARFYAML::DebugNameAbbreviation> {
+  static void mapping(IO &IO, DWARFYAML::DebugNameAbbreviation &);
+};
+template <> struct MappingTraits<DWARFYAML::IdxForm> {
+  static void mapping(IO &IO, DWARFYAML::IdxForm &);
+};
+
 template <> struct MappingTraits<DWARFYAML::Data> {
   static void mapping(IO &IO, DWARFYAML::Data &DWARF);
 };
@@ -437,6 +476,16 @@ template <> struct ScalarEnumerationTraits<dwarf::Form> {
   }
 };
 
+#define HANDLE_DW_IDX(unused, name)                                            \
+  io.enumCase(value, "DW_IDX_" #name, dwarf::DW_IDX_##name);
+
+template <> struct ScalarEnumerationTraits<dwarf::Index> {
+  static void enumeration(IO &io, dwarf::Index &value) {
+#include "llvm/BinaryFormat/Dwarf.def"
+    io.enumFallback<Hex16>(value);
+  }
+};
+
 #define HANDLE_DW_UT(unused, name)                                             \
   io.enumCase(value, "DW_UT_" #name, dwarf::DW_UT_##name);
 
diff --git a/llvm/lib/ObjectYAML/DWARFEmitter.cpp b/llvm/lib/ObjectYAML/DWARFEmitter.cpp
index a26e93f65ed7c5..bf01ade9156dbe 100644
--- a/llvm/lib/ObjectYAML/DWARFEmitter.cpp
+++ b/llvm/lib/ObjectYAML/DWARFEmitter.cpp
@@ -691,6 +691,191 @@ Error DWARFYAML::emitDebugStrOffsets(raw_ostream &OS, const Data &DI) {
   return Error::success();
 }
 
+namespace {
+/// Emits the header for a DebugNames section.
+void emitDebugNamesHeader(raw_ostream &OS, bool IsLittleEndian,
+                          uint32_t NameCount, uint32_t AbbrevSize,
+                          uint32_t CombinedSizeOtherParts) {
+  StringRef AugmentationString = "LLVM0700";
+  auto TotalSize = CombinedSizeOtherParts + 5 * sizeof(uint32_t) +
+                   2 * sizeof(uint16_t) + sizeof(NameCount) +
+                   sizeof(AbbrevSize) + AugmentationString.size();
+  writeInteger(uint32_t(TotalSize), OS, IsLittleEndian); // Unit length
+
+  // Everything below is included in total size.
+  writeInteger(uint16_t(5), OS, IsLittleEndian); // Version
+  writeInteger(uint16_t(0), OS, IsLittleEndian); // Padding
+  writeInteger(uint32_t(1), OS, IsLittleEndian); // Compilation Unit count
+  writeInteger(uint32_t(0), OS, IsLittleEndian); // Local Type Unit count
+  writeInteger(uint32_t(0), OS, IsLittleEndian); // Foreign Type Unit count
+  writeInteger(uint32_t(0), OS, IsLittleEndian); // Bucket count
+  writeInteger(NameCount, OS, IsLittleEndian);
+  writeInteger(AbbrevSize, OS, IsLittleEndian);
+  writeInteger(uint32_t(AugmentationString.size()), OS, IsLittleEndian);
+  OS.write(AugmentationString.data(), AugmentationString.size());
+  return;
+}
+
+/// Emits the abbreviations for a DebugNames section.
+std::string
+emitDebugNamesAbbrev(ArrayRef<DWARFYAML::DebugNameAbbreviation> Abbrevs) {
+  std::string Data;
+  llvm::raw_string_ostream OS(Data);
+  for (const auto &Abbrev : Abbrevs) {
+    encodeULEB128(Abbrev.Code, OS);
+    encodeULEB128(Abbrev.Tag, OS);
+    for (auto [Idx, Form] : Abbrev.Indices) {
+      encodeULEB128(Idx, OS);
+      encodeULEB128(Form, OS);
+    }
+    encodeULEB128(0, OS);
+    encodeULEB128(0, OS);
+  }
+  encodeULEB128(0, OS);
+  return Data;
+}
+
+/// Emits a simple CU offsets list for a DebugNames section containing a single
+/// CU at offset 0.
+std::string emitDebugNamesCUOffsets(bool IsLittleEndian) {
+  std::string Data;
+  llvm::raw_string_ostream OS(Data);
+  writeInteger(uint32_t(0), OS, IsLittleEndian);
+  return Data;
+}
+
+/// Emits the "NameTable" for a DebugNames section; according to the spec, it
+/// consists of two arrays: an array of string offsets, followed immediately by
+/// an array of entry offsets. The string offsets are emitted in the order
+/// provided in `Entries`.
+std::string emitDebugNamesNameTable(
+    bool IsLittleEndian,
+    const std::map<uint32_t, std::vector<DWARFYAML::DebugNameEntry>> &Entries,
+    ArrayRef<uint32_t> EntryPoolOffsets) {
+  assert(Entries.size() == EntryPoolOffsets.size());
+
+  std::string Data;
+  llvm::raw_string_ostream OS(Data);
+
+  for (auto Strp : make_first_range(Entries))
+    writeInteger(Strp, OS, IsLittleEndian);
+  for (auto PoolOffset : EntryPoolOffsets)
+    writeInteger(uint32_t(PoolOffset), OS, IsLittleEndian);
+  return Data;
+}
+
+/// Groups entries based on their name (strp) code and returns a sorted map.
+std::map<uint32_t, std::vector<DWARFYAML::DebugNameEntry>>
+groupEntries(ArrayRef<DWARFYAML::DebugNameEntry> Entries) {
+  std::map<uint32_t, std::vector<DWARFYAML::DebugNameEntry>> Ans;
+  for (const auto &Entry : Entries)
+    Ans[Entry.NameStrp].push_back(Entry);
+  return Ans;
+}
+
+/// Finds the abbreviation whose code is AbbrevCode and returns a list
+/// containing the expected size of all non-zero-length forms.
+Expected<SmallVector<uint8_t>>
+getNonZeroDataSizesFor(uint32_t AbbrevCode,
+                       ArrayRef<DWARFYAML::DebugNameAbbreviation> Abbrevs) {
+  const auto *AbbrevIt = find_if(Abbrevs, [&](const auto &Abbrev) {
+    return Abbrev.Code.value == AbbrevCode;
+  });
+  if (AbbrevIt == Abbrevs.end())
+    return createStringError(inconvertibleErrorCode(),
+                             "Did not find an Abbreviation for this code");
+
+  SmallVector<uint8_t> DataSizes;
+  dwarf::FormParams Params{5 /*Version*/, 4 /*AddrSize*/, dwarf::DWARF32};
+  for (auto [Idx, Form] : AbbrevIt->Indices) {
+    auto FormSize = dwarf::getFixedFormByteSize(Form, Params);
+    if (FormSize == std::nullopt)
+      return createStringError(inconvertibleErrorCode(),
+                               "Unsupported Form for YAML debug_names emitter");
+    if (FormSize == 0)
+      continue;
+    DataSizes.push_back(*FormSize);
+  }
+  return DataSizes;
+}
+
+struct PoolOffsetsAndData {
+  std::string PoolData;
+  std::vector<uint32_t> PoolOffsets;
+};
+
+/// Emits the entry pool and returns an array of offsets containing the start
+/// offset for the entries of each unique name.
+/// Verifies that the provided number of data values match those expected by
+/// the abbreviation table.
+Expected<PoolOffsetsAndData> emitDebugNamesEntryPool(
+    bool IsLittleEndian,
+    const std::map<uint32_t, std::vector<DWARFYAML::DebugNameEntry>>
+        &SortedEntries,
+    ArrayRef<DWARFYAML::DebugNameAbbreviation> Abbrevs) {
+  PoolOffsetsAndData Ans;
+  llvm::raw_string_ostream OS(Ans.PoolData);
+
+  for (ArrayRef<DWARFYAML::DebugNameEntry> EntriesWithSameName :
+       llvm::make_second_range(SortedEntries)) {
+    Ans.PoolOffsets.push_back(Ans.PoolData.size());
+
+    for (const auto &Entry : EntriesWithSameName) {
+      encodeULEB128(Entry.Code, OS);
+
+      auto DataSizes = getNonZeroDataSizesFor(Entry.Code, Abbrevs);
+      if (!DataSizes)
+        return DataSizes.takeError();
+      if (DataSizes->size() != Entry.Values.size())
+        return createStringError(
+            inconvertibleErrorCode(),
+            "Mismatch between provided and required number of values");
+
+      for (auto [Value, ValueSize] : zip_equal(Entry.Values, *DataSizes))
+        if (auto E =
+                writeVariableSizedInteger(Value, ValueSize, OS, IsLittleEndian))
+          return std::move(E);
+    }
+    encodeULEB128(0, OS);
+  }
+
+  return Ans;
+}
+} // namespace
+
+Error DWARFYAML::emitDebugNames(raw_ostream &OS, const Data &DI) {
+  assert(DI.DebugNames && "unexpected emitDebugNames() call");
+  const auto DebugNames = DI.DebugNames.value();
+
+  auto SortedEntries = groupEntries(DebugNames.Entries);
+
+  // Emit all sub-sections into individual strings so that we may compute
+  // relative offsets and sizes.
+  Expected<PoolOffsetsAndData> PoolInfo = emitDebugNamesEntryPool(
+      DI.IsLittleEndian, SortedEntries, DebugNames.Abbrevs);
+  if (!PoolInfo)
+    return PoolInfo.takeError();
+  auto NamesTableData = emitDebugNamesNameTable(
+      DI.IsLittleEndian, SortedEntries, PoolInfo->PoolOffsets);
+
+  auto AbbrevData = emitDebugNamesAbbrev(DebugNames.Abbrevs);
+  auto CUOffsetsData = emitDebugNamesCUOffsets(DI.IsLittleEndian);
+
+  auto TotalSize = PoolInfo->PoolData.size() + NamesTableData.size() +
+                   AbbrevData.size() + CUOffsetsData.size();
+
+  // Start real emission by combining all individual strings.
+  emitDebugNamesHeader(OS, DI.IsLittleEndian, SortedEntries.size(),
+                       AbbrevData.size(), TotalSize);
+  OS.write(CUOffsetsData.data(), CUOffsetsData.size());
+  // No local TUs, no foreign TUs, no hash lookups table.
+  OS.write(NamesTableData.data(), NamesTableData.size());
+  OS.write(AbbrevData.data(), AbbrevData.size());
+  OS.write(PoolInfo->PoolData.data(), PoolInfo->PoolData.size());
+
+  return Error::success();
+}
+
 static Error checkOperandCount(StringRef EncodingString,
                                ArrayRef<yaml::Hex64> Values,
                                uint64_t ExpectedOperands) {
@@ -1024,6 +1209,7 @@ DWARFYAML::getDWARFEmitterByName(StringRef SecName) {
           .Case("debug_rnglists", DWARFYAML::emitDebugRnglists)
           .Case("debug_str", DWARFYAML::emitDebugStr)
           .Case("debug_str_offsets", DWARFYAML::emitDebugStrOffsets)
+          .Case("debug_names", DWARFYAML::emitDebugNames)
           .Default([&](raw_ostream &, const DWARFYAML::Data &) {
             return createStringError(errc::not_supported,
                                      SecName + " is not supported");
diff --git a/llvm/lib/ObjectYAML/DWARFYAML.cpp b/llvm/lib/ObjectYAML/DWARFYAML.cpp
index 2bddeed4641353..5207671f57d930 100644
--- a/llvm/lib/ObjectYAML/DWARFYAML.cpp
+++ b/llvm/lib/ObjectYAML/DWARFYAML.cpp
@@ -52,6 +52,8 @@ SetVector<StringRef> DWARFYAML::Data::getNonEmptySectionNames() const {
     SecNames.insert("debug_rnglists");
   if (DebugLoclists)
     SecNames.insert("debug_loclists");
+  if (DebugNames)
+    SecNames.insert("debug_names");
   return SecNames;
 }
 
@@ -105,6 +107,7 @@ void MappingTraits<DWARFYAML::Data>::mapping(IO &IO, DWARFYAML::Data &DWARF) {
   IO.mapOptional("debug_str_offsets", DWARF.DebugStrOffsets);
   IO.mapOptional("debug_rnglists", DWARF.DebugRnglists);
   IO.mapOptional("debug_loclists", DWARF.DebugLoclists);
+  IO.mapOptional("debug_names", DWARF.DebugNames);
   IO.setContext(OldContext);
 }
 
@@ -122,6 +125,32 @@ void MappingTraits<DWARFYAML::Abbrev>::mapping(IO &IO,
   IO.mapOptional("Attributes", Abbrev.Attributes);
 }
 
+void MappingTraits<DWARFYAML::IdxForm>::mapping(IO &IO,
+                                                DWARFYAML::IdxForm &IdxForm) {
+  IO.mapRequired("Idx", IdxForm.Idx);
+  IO.mapRequired("Form", IdxForm.Form);
+}
+
+void MappingTraits<DWARFYAML::DebugNameAbbreviation>::mapping(
+    IO &IO, DWARFYAML::DebugNameAbbreviation &DebugNameAbbreviation) {
+  IO.mapRequired("Code", DebugNameAbbreviation.Code);
+  IO.mapRequired("Tag", DebugNameAbbreviation.Tag);
+  IO.mapRequired("Indices", DebugNameAbbreviation.Indices);
+}
+
+void MappingTraits<DWARFYAML::DebugNameEntry>::mapping(
+    IO &IO, DWARFYAML::DebugNameEntry &DebugNameEntry) {
+  IO.mapRequired("Name", DebugNameEntry.NameStrp);
+  IO.mapRequired("Code", DebugNameEntry.Code);
+  IO.mapOptional("Values", DebugNameEntry.Values);
+}
+
+void MappingTraits<DWARFYAML::DebugNamesSection>::mapping(
+    IO &IO, DWARFYAML::DebugNamesSection &DebugNames) {
+  IO.mapRequired("Abbreviations", DebugNames.Abbrevs);
+  IO.mapRequired("Entries", DebugNames.Entries);
+}
+
 void MappingTraits<DWARFYAML::AttributeAbbrev>::mapping(
     IO &IO, DWARFYAML::AttributeAbbrev &AttAbbrev) {
   IO.mapRequired("Attribute", AttAbbrev.Attribute);
diff --git a/llvm/unittests/DebugInfo/DWARF/DWARFAcceleratorTableTest.cpp b/llvm/unittests/DebugInfo/DWARF/DWARFAcceleratorTableTest.cpp
index 38f3e076ace213..404bddebb4acdc 100644
--- a/llvm/unittests/DebugInfo/DWARF/DWARFAcceleratorTableTest.cpp
+++ b/llvm/unittests/DebugInfo/DWARF/DWARFAcceleratorTableTest.cpp
@@ -8,6 +8,7 @@
 
 #include "llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h"
 #include "llvm/DebugInfo/DWARF/DWARFContext.h"
+#include "llvm/ObjectYAML/DWARFEmitter.h"
 #include "llvm/Testing/Support/Error.h"
 #include "gtest/gtest.h"
 
@@ -46,4 +47,164 @@ TEST(DWARFDebugNames, TooSmallForDWARF64) {
                         "data at offset 0x2b while reading [0x28, 0x2c)"));
 }
 
+TEST(DWARFDebugNames, BasicTestEntries) {
+  const char *Yamldata = R"(
+--- !ELF
+  debug_str:
+    - 'NameType1'
+    - 'NameType2'
+
+  debug_names:
+    Abbreviations:
+    - Code:   0x1
+      Tag: DW_TAG_namespace
+      Indices:
+        - Idx:   DW_IDX_compile_unit
+          Form:  DW_FORM_data4
+        - Idx:   DW_IDX_die_offset
+          Form:  DW_FORM_ref4
+    Entries:
+    - Name:   0x0  # strp to NameType1
+      Code:   0x1
+      Values:
+        - 0x0      # Compile unit
+        - 0x0      # DIE Offset
+    - Name:   0xa  # strp to NameType2
+      Code:   0x1
+      Values:
+        - 0x1      # Compile unit
+        - 0x1      # DIE Offset
+    - Name:   0x0  # strp to NameType1
+      Code:   0x1
+      Values:
+        - 0x2     # Compile unit
+        - 0x2     # DIE Offset
+
+)";
+
+  Expected<StringMap<std::unique_ptr<MemoryBuffer>>> Sections =
+      DWARFYAML::emitDebugSections(StringRef(Yamldata),
+                                   /*IsLittleEndian=*/true,
+                                   /*Is64BitAddrSize=*/true);
+  ASSERT_THAT_EXPECTED(Sections, Succeeded());
+  auto Ctx = DWARFContext::create(*Sections, 4, /*isLittleEndian=*/true);
+  const DWARFDebugNames &DebugNames = Ctx->getDebugNames();
+  ASSERT_NE(DebugNames.begin(), DebugNames.end());
+  const auto &NameIndex = *DebugNames.begin();
+
+  ASSERT_EQ(NameIndex.getNameCount(), 2u);
+  ASSERT_EQ(NameIndex.getBucketCount(), 0u);
+  ASSERT_EQ(NameIndex.getCUCount(), 1u);
+  ASSERT_EQ(NameIndex.getCUOffset(0), 0u);
+  ASSERT_EQ(NameIndex.getForeignTUCount(), 0u);
+  ASSERT_EQ(NameIndex.getLocalTUCount(), 0u);
+
+  // Check "NameEntries": there should be one per unique name.
+  // These are indexed starting on 1.
+  auto FirstEntry = NameIndex.getNameTableEntry(1);
+  ASSERT_EQ(FirstEntry.getString(), StringRef("NameType1"));
+  auto SecondEntry = NameIndex.getNameTableEntry(2);
+  ASSERT_EQ(SecondEntry.getString(), StringRef("NameType2"));
+
+  auto FirstNameEntries = llvm::to_vector_of<DWARFDebugNames::Entry>(
+      NameIndex.equal_range("NameType1"));
+  ASSERT_EQ(FirstNameEntries.size(), 2u);
+  ASSERT_EQ(FirstNameEntries[0].getCUIndex(), 0u);
+  ASSERT_EQ(FirstNameEntries[1].getCUIndex(), 0x2);
+  ASSERT_EQ(FirstNameEntries[0].getDIEUnitOffset(), 0x0);
+  ASSERT_EQ(FirstNameEntries[1].getDIEUnitOffset(), 0x2);
+
+  auto SecondNameEntries = llvm::to_vector_of<DWARFDebugNames::Entry>(
+      NameIndex.equal_range("NameType2"));
+  ASSERT_EQ(SecondNameEntries.size(), 1u);
+  ASSERT_EQ(SecondNameEntries[0].getCUIndex(), 0x1);
+  ASSERT_EQ(SecondNameEntries[0].getDIEUnitOffset(), 0x1);
+}
+
+TEST(DWARFDebugNames, ParentEntries) {
+  const char *Yamldata = R"(
+--- !ELF
+  debug_str:
+    - 'Name1'
+    - 'Name2'
+    - 'Name3'
+    - 'Name4'
+  debug_names:
+    Abbreviations:
+    - Code:   0x11
+      Tag: DW_TAG_namespace
+      Indices:
+        - Idx:   DW_IDX_parent
+          Form:  DW_FORM_flag_present
+        - Idx:   DW_IDX_die_offset
+          Form:  DW_FORM_ref4
+    - Code:   0x22
+      Tag: DW_TAG_namespace
+      Indices:
+        - Idx:   DW_IDX_parent
+          Form:  DW_FORM_ref4
+        - Idx:   DW_IDX_die_offset
+          Form:  DW_FORM_ref4
+    - Code:   0x33
+      Tag: DW_TAG_namespace
+      Indices:
+        - Idx:   DW_IDX_die_offset
+          Form:  DW_FORM_ref4
+    Entries:
+    - Name:   0x0  # strp to Name1
+      Code:   0x11
+      Values:
+        - 0x0      # Die offset
+    - Name:   0x6  # strp to Name2
+      Code:   0x22
+      Values:
+        - 0x0      # Parent = First entry
+        - 0x1      # Die offset
+    - Name:   0xc  # strp to Name3
+      Code:   0x22
+      Values:
+        - 0x6      # Parent = Second entry
+        - 0x1      # Die offset
+    - Name:   0x12  # strp to Name4
+      Code:   0x33
+      Values:
+        - 0x1      # Die offset
+)";
+
+  Expected<StringMap<std::unique_ptr<MemoryBuffer>>> Sections =
+      DWARFYAML::emitDebugSections(StringRef(Yamldata),
+                                   /*IsLittleEndian=*/true,
+                                   /*Is64BitAddrSize=*/true);
+  ASSERT_THAT_EXPECTED(Sections, Succeeded());
+  auto Ctx = DWARFContext::create(*Sections, 4, /*isLittleEndian=*/true);
+  const DWARFDebugNames &DebugNames = Ctx->getDebugNames();
+  ASSERT_NE(DebugNames.begin(), DebugNames.end());
+  const auto &NameIndex = *DebugNames.begin();
+
+  auto Name1Entries = llvm::to_vector_of<DWARFDebugNames::Entry>(
+      NameIndex.equal_range("Name1"));
+  ASSERT_EQ(Name1Entries.size(), 1u);
+  auto Name1Parent = Name1Entries[0].getParentDIEEntry();
+  ASSERT_THAT_EXPECTED(Name1Parent, Succeeded());
+  ASSERT_EQ(*Name1Parent, std::nullopt); // Name1 has no parent
+
+  auto Name2Entries = llvm::to_vector_of<DWARFDebugNames::Entry>(
+      NameIndex.equal_range("Name2"));
+  ASSERT_EQ(Name2Entries.size(), 1u);
+  auto Name2Parent = Name2Entries[0].getParentDIEEntry();
+  ASSERT_THAT_EXPECTED(Name2Parent, Succeeded());
+  ASSERT_EQ((**Name2Parent).getDIEUnitOffset(), 0x0); // Name2 parent == Name1
+
+  auto Name3Entries = llvm::to_vector_of<DWARFDebugNames::Entry>(
+      NameIndex.equal_range("Name3"));
+  ASSERT_EQ(Name3Entries.size(), 1u);
+  auto Name3Parent = Name3Entries[0].getParentDIEEntry();
+  ASSERT_THAT_EXPECTED(Name3Parent, Succeeded());
+  ASSERT_EQ((**Name3Parent).getDIEUnitOffset(), 0x1); // Name3 parent == Name2
+
+  auto Name4Entries = llvm::to_vector_of<DWARFDebugNames::Entry>(
+      NameIndex.equal_range("Name4"));
+  ASSERT_EQ(Name4Entries.size(), 1u);
+  ASSERT_FALSE(Name4Entries[0].hasParentInformation());
+}
 } // end anonymous namespace

Copy link
Collaborator

@adrian-prantl adrian-prantl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! I'm sure there could be more customized (parsed instead of hardcoded) in the future, but I think this is a great starting point.

@jh7370
Copy link
Collaborator

jh7370 commented Jan 30, 2024

Thanks for looking at this. I plan on reviewing this in the next day or two (dependent on how busy I am with other things), so please hold off merging for a few days until I can make my review too.

@felipepiovezan
Copy link
Contributor Author

Thanks for looking at this. I plan on reviewing this in the next day or two (dependent on how busy I am with other things), so please hold off merging for a few days until I can make my review too.

Sounds good, thanks for the heads up

@felipepiovezan
Copy link
Contributor Author

Polite ping

llvm/include/llvm/ObjectYAML/DWARFYAML.h Show resolved Hide resolved
uint32_t NameCount, uint32_t AbbrevSize,
uint32_t CombinedSizeOtherParts) {
StringRef AugmentationString = "LLVM0700";
auto TotalSize = CombinedSizeOtherParts + 5 * sizeof(uint32_t) +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specific type? Also in other places.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed most autos from the patch

@jh7370
Copy link
Collaborator

jh7370 commented Feb 5, 2024

Polite ping

Apologies - I ended up off sick last week. Looking into this now.

Copy link
Collaborator

@jh7370 jh7370 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not massively familiar with the DWARF5 spec, so I haven't attempted to review that aspect of this PR and have restricted myself to coding style comments and more general YAML comments.

The testing seems a bit light to me. Are all new code paths tested? If nothing else, I don't see any testing of the error cases.

llvm/include/llvm/ObjectYAML/DWARFYAML.h Outdated Show resolved Hide resolved
llvm/include/llvm/ObjectYAML/DWARFYAML.h Outdated Show resolved Hide resolved
llvm/lib/ObjectYAML/DWARFEmitter.cpp Show resolved Hide resolved
llvm/lib/ObjectYAML/DWARFEmitter.cpp Outdated Show resolved Hide resolved
llvm/lib/ObjectYAML/DWARFEmitter.cpp Outdated Show resolved Hide resolved
"Did not find an Abbreviation for this code");

SmallVector<uint8_t> DataSizes;
dwarf::FormParams Params{5 /*Version*/, 4 /*AddrSize*/, dwarf::DWARF32};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The canonical form of these sorts of comments is /*Version=*/5, /*AddrSize=*/4 (clang-format actually recognises that specific format too).

llvm/lib/ObjectYAML/DWARFEmitter.cpp Outdated Show resolved Hide resolved
for (auto [Idx, Form] : AbbrevIt->Indices) {
auto FormSize = dwarf::getFixedFormByteSize(Form, Params);
if (FormSize == std::nullopt)
return createStringError(inconvertibleErrorCode(),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to my above comment, could we instead permit this and just assume some arbitrary size (0 seems like a reasonable bet), to allow explicit specification of an invalid Form value, which in turn would allow users to be able to test invalid Form handling in the DWARF reading code?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed with the idea, but please see my answer in the previous thread.

llvm/lib/ObjectYAML/DWARFEmitter.cpp Outdated Show resolved Hide resolved
)";

Expected<StringMap<std::unique_ptr<MemoryBuffer>>> Sections =
DWARFYAML::emitDebugSections(StringRef(Yamldata),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the explicit StringRef construction actually necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, it's not! I thought that ctor was explicit, but it's not!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, being explicit would inhibit one of the main purposes of StringRef, which is to be essentially a drop-in replacement for const std::string & in function signatures (which is of course implicitly constructible from std::string or const char *). It's the same as the C++17 std::string_view class (but I believe we haven't switched because StringRef has some additional functionality that we've implemented).

Copy link
Contributor Author

@felipepiovezan felipepiovezan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed review comments

The testing seems a bit light to me. Are all new code paths tested?

W.r.t. to the parsing and emitting, yes, all code paths should be exercised, with the exception of the error paths.

If nothing else, I don't see any testing of the error cases.

I will add a few in the next commit

llvm/include/llvm/ObjectYAML/DWARFYAML.h Outdated Show resolved Hide resolved
llvm/lib/ObjectYAML/DWARFEmitter.cpp Show resolved Hide resolved
uint32_t NameCount, uint32_t AbbrevSize,
uint32_t CombinedSizeOtherParts) {
StringRef AugmentationString = "LLVM0700";
auto TotalSize = CombinedSizeOtherParts + 5 * sizeof(uint32_t) +
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed most autos from the patch

llvm/lib/ObjectYAML/DWARFEmitter.cpp Outdated Show resolved Hide resolved
llvm/lib/ObjectYAML/DWARFEmitter.cpp Outdated Show resolved Hide resolved
});
if (AbbrevIt == Abbrevs.end())
return createStringError(inconvertibleErrorCode(),
"Did not find an Abbreviation for this code");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to lower case first letters

llvm/lib/ObjectYAML/DWARFEmitter.cpp Outdated Show resolved Hide resolved
for (auto [Idx, Form] : AbbrevIt->Indices) {
auto FormSize = dwarf::getFixedFormByteSize(Form, Params);
if (FormSize == std::nullopt)
return createStringError(inconvertibleErrorCode(),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed with the idea, but please see my answer in the previous thread.

llvm/lib/ObjectYAML/DWARFEmitter.cpp Outdated Show resolved Hide resolved
)";

Expected<StringMap<std::unique_ptr<MemoryBuffer>>> Sections =
DWARFYAML::emitDebugSections(StringRef(Yamldata),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, it's not! I thought that ctor was explicit, but it's not!

@felipepiovezan
Copy link
Contributor Author

Addressed all review comments


SmallVector<uint8_t> DataSizes;
dwarf::FormParams Params{5 /*Version*/, 4 /*AddrSize*/, dwarf::DWARF32};
dwarf::FormParams Params{/*Version*/ 5, /*AddrSize*/ 4, dwarf::DWARF32};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still missing the = from my original suggestion (i.e. /*Version=*/5 and /*AddrSize=*/4).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, fixed!

llvm/lib/ObjectYAML/DWARFEmitter.cpp Outdated Show resolved Hide resolved
Copy link

github-actions bot commented Feb 8, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Collaborator

@jh7370 jh7370 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@felipepiovezan felipepiovezan merged commit a8b4c11 into llvm:main Feb 12, 2024
3 of 4 checks passed
@felipepiovezan felipepiovezan deleted the felipe/debug_names_yaml branch February 12, 2024 17:24
felipepiovezan added a commit to felipepiovezan/llvm-project that referenced this pull request Feb 13, 2024
This commit brings support for debug_names in DWARFYAML. It parses YAML
and generates emits a DWARF5 Accelerator table with the following
limitations:

1. All forms must have a fixed length (zero length is also ok).
2. Hard-coded support for DWARF 5 and DWARF32.
3. The generated table does not contain a hash index

All of these limitations can be lifted in the future, but for now this
is good enough to enable testing.

(cherry picked from commit a8b4c11)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants