[DWARFYAML] Refactor emitDebugInfo() to make the length be inferred.

[Higuoxing]

Summary

This patch refactors emitDebugInfo() to make the length field be
inferred from its content. Besides, the Visitor class is removed in
this patch. The original Visitor class helps us determine an
appropriate length and emit the .debug_info section. These two
processes can be merged into one process. Besides, the length field
should be inferred when it's missing rather than when it's zero.

[Higuoxing]

I'll add some tests later.

[MaskRay]

The number of changed tests is large. Is it worth moving the IO.mapOptional("Length", Unit.Length); change to a separate patch to make the refactoring more focused? Thanks

[Higuoxing]

In D84008#2160426, @MaskRay wrote:

The number of changed tests is large. Is it worth moving the IO.mapOptional("Length", Unit.Length); change to a separate patch to make the refactoring more focused? Thanks

This patch is intended to make the length field be inferred when emitting the .debug_info section. If we move the IO.mapOption("Length", Unit.Length); change to a separate change, we might not be able to know when to infer the length? There are two visitors, DumpVisitor which is used to emit the .debug_info section and DIEFixupVisitor which is used to calculate the length field for us. Do you mean that we keep the DIEFixupVisitor class and remove the DumpVisitor class in this patch?

[labath]

In D84008#2161243, @Higuoxing wrote:

In D84008#2160426, @MaskRay wrote:

The number of changed tests is large. Is it worth moving the IO.mapOptional("Length", Unit.Length); change to a separate patch to make the refactoring more focused? Thanks

This patch is intended to make the length field be inferred when emitting the .debug_info section. If we move the IO.mapOption("Length", Unit.Length); change to a separate change, we might not be able to know when to infer the length? There are two visitors, DumpVisitor which is used to emit the .debug_info section and DIEFixupVisitor which is used to calculate the length field for us. Do you mean that we keep the DIEFixupVisitor class and remove the DumpVisitor class in this patch?

I think that should work if you make it so that this other patch comes before the functional change in this patch. That other patch could change the encoding to hex (uint64_t Length; -> yaml::Hex64 Length;) and make it default to zero (IO.mapRequired("Length", Unit.Length); -> IO.mapOptional("Length", Unit.Length, 0);). That should have no functional change (I think), but allow you to make the changes in all the yaml files. The visitation stuff could come after that.

[Higuoxing]

In D84008#2161461, @labath wrote:

In D84008#2161243, @Higuoxing wrote:

In D84008#2160426, @MaskRay wrote:

The number of changed tests is large. Is it worth moving the IO.mapOptional("Length", Unit.Length); change to a separate patch to make the refactoring more focused? Thanks

This patch is intended to make the length field be inferred when emitting the .debug_info section. If we move the IO.mapOption("Length", Unit.Length); change to a separate change, we might not be able to know when to infer the length? There are two visitors, DumpVisitor which is used to emit the .debug_info section and DIEFixupVisitor which is used to calculate the length field for us. Do you mean that we keep the DIEFixupVisitor class and remove the DumpVisitor class in this patch?

I think that should work if you make it so that this other patch comes before the functional change in this patch. That other patch could change the encoding to hex (uint64_t Length; -> yaml::Hex64 Length;) and make it default to zero (IO.mapRequired("Length", Unit.Length); -> IO.mapOptional("Length", Unit.Length, 0);). That should have no functional change (I think), but allow you to make the changes in all the yaml files. The visitation stuff could come after that.

Thank you @labath, It sounds good to me!

[jhenderson]

In principle, this approach looks fine, but please do as @labath suggests to reduce the amount of changes in one go.

[Higuoxing]

Address comments & Rebase & Add one test.

[jhenderson]

llvm/test/tools/yaml2obj/ELF/DWARF/debug-info.yaml

743

Should this be INFER-LENGTH? (The test is currently failing...)

748–750

I don't think these comments are particularly helpful, since they can be inferred from the byte-count/offset at the start of the line. I think if you want comments, you should say what the bytes represent, like you do elsewhere.

[Higuoxing]

Address comments.

llvm/test/tools/yaml2obj/ELF/DWARF/debug-info.yaml

743

Oops, sorry, I forget to modify this line.

[jhenderson]

LGTM, thanks, but might be best to get a second opinion.

[labath]

Yeah, looks good. Some small suggestions inline, though some of them may be better off as separate patches...

llvm/lib/ObjectYAML/DWARFEmitter.cpp

211–343

Maybe this is for a separate patch, since you're just moving these, but this code already exists in BinaryFormat/Dwarf.h. It would be nice if DWARFYaml::Unit had a dwarf::FormParams field or a getter, and then these things could be retrieved by querying that object...

355–359

Since the comments repeat the entire expression, it would be less repetitive to just structure the code in a way that comments can be attached to the actual expressions. Maybe something like:

Lenght = 3; // version + address_size
Length += (Unit.Version >= 5 ? 1 : 0); // unit_type
Length += Unit.getFormParams().getDwarfOffsetByteSize(); // abbrev_offset

385–387

a writeDwarfOffset function might be handy.

[Higuoxing]

Address comments. Thanks for reviewing!

llvm/lib/ObjectYAML/DWARFEmitter.cpp

211–343

Thanks! I'll do it in a separate patch.

[jhenderson]

llvm/lib/ObjectYAML/DWARFEmitter.cpp

385–387

I'm guessing the new function can be used elsewhere too (I expect in places like .debug_aranges for example). In another patch, please consider looking at that.

[Higuoxing]

llvm/lib/ObjectYAML/DWARFEmitter.cpp

385–387

Yeah, it can be used in the .debug_str_offsets/debug_rnglists/debug_loclists/debug_aranges sections, etc. I've applied it in this patch and will apply it to other sections in a follow-up patch.

lldb/unittests/TestingSupport/Symbol/YAMLModuleTester.cpp

public:
   bool ParseHeader() override { return false; }
   Type CalculateType() override { return {}; }
   Strata CalculateStrata() override { return {}; }
   /// \}
 };
 
 YAMLModuleTester::YAMLModuleTester(llvm::StringRef yaml_data,
                                    llvm::StringRef triple) {
-  auto sections_map = llvm::DWARFYAML::emitDebugSections(yaml_data, true);
+  auto sections_map = llvm::DWARFYAML::emitDebugSections(yaml_data);
   if (!sections_map)
     return;
   m_sections_map = std::move(*sections_map);
   ArchSpec arch(triple);
   m_module_sp = std::make_shared<YAMLModule>(arch);
   m_objfile_sp = std::make_shared<YAMLObjectFile>(m_module_sp, m_sections_map);
   static_cast<YAMLModule *>(m_module_sp.get())->SetObjectFile(m_objfile_sp);
 

llvm/include/llvm/ObjectYAML/DWARFEmitter.h

Error emitPubSection(raw_ostream &OS, const PubSection &Sect,
                      bool IsLittleEndian, bool IsGNUPubSec = false);
 Error emitDebugInfo(raw_ostream &OS, const Data &DI);
 Error emitDebugLine(raw_ostream &OS, const Data &DI);
 Error emitDebugAddr(raw_ostream &OS, const Data &DI);
 Error emitDebugStrOffsets(raw_ostream &OS, const Data &DI);
 Error emitDebugRnglists(raw_ostream &OS, const Data &DI);
 
 Expected<StringMap<std::unique_ptr<MemoryBuffer>>>
-emitDebugSections(StringRef YAMLString, bool ApplyFixups = false,
+emitDebugSections(StringRef YAMLString,
                   bool IsLittleEndian = sys::IsLittleEndianHost);
 } // end namespace DWARFYAML
 } // end namespace llvm
 
 #endif // LLVM_OBJECTYAML_DWARFEMITTER_H

llvm/include/llvm/ObjectYAML/DWARFYAML.h

 /// Class that contains helpful context information when mapping YAML into DWARF
 /// data structures.
 struct DWARFContext {
   bool IsGNUPubSec = false;
 };
 
 struct Unit {
   dwarf::DwarfFormat Format;
-  yaml::Hex64 Length;
+  Optional<yaml::Hex64> Length;
   uint16_t Version;
   llvm::dwarf::UnitType Type; // Added in DWARF 5
   yaml::Hex64 AbbrOffset;
   uint8_t AddrSize;
   std::vector<Entry> Entries;
 };
 
 struct File {

llvm/lib/ObjectYAML/CMakeLists.txt

 add_llvm_component_library(LLVMObjectYAML
   CodeViewYAMLDebugSections.cpp
   CodeViewYAMLSymbols.cpp
   CodeViewYAMLTypeHashing.cpp
   CodeViewYAMLTypes.cpp
   COFFEmitter.cpp
   COFFYAML.cpp
   DWARFEmitter.cpp
-  DWARFVisitor.cpp
   DWARFYAML.cpp
   ELFEmitter.cpp
   ELFYAML.cpp
   MachOEmitter.cpp
   MachOYAML.cpp
   ObjectYAML.cpp
   MinidumpEmitter.cpp
   MinidumpYAML.cpp

llvm/lib/ObjectYAML/DWARFEmitter.cpp

 //===- DWARFEmitter - Convert YAML to DWARF binary data -------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 ///
 /// \file
 /// The DWARF component of yaml2obj. Provided as library code for tests.
 ///
 //===----------------------------------------------------------------------===//
 
 #include "llvm/ObjectYAML/DWARFEmitter.h"
-#include "DWARFVisitor.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/BinaryFormat/Dwarf.h"
 #include "llvm/ObjectYAML/DWARFYAML.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/Host.h"
if (IsGNUPubSec)
       writeInteger((uint8_t)Entry.Descriptor, OS, IsLittleEndian);
     OS.write(Entry.Name.data(), Entry.Name.size());
     OS.write('\0');
   }
 
   return Error::success();
 }
 
-namespace {
-/// An extension of the DWARFYAML::ConstVisitor which writes compile
-/// units and DIEs to a stream.
-class DumpVisitor : public DWARFYAML::ConstVisitor {
-  raw_ostream &OS;
-
-protected:
-  void onStartCompileUnit(const DWARFYAML::Unit &CU) override {
-    writeInitialLength(CU.Format, CU.Length, OS, DebugInfo.IsLittleEndian);
-    writeInteger((uint16_t)CU.Version, OS, DebugInfo.IsLittleEndian);
-    if (CU.Version >= 5) {
-      writeInteger((uint8_t)CU.Type, OS, DebugInfo.IsLittleEndian);
-      writeInteger((uint8_t)CU.AddrSize, OS, DebugInfo.IsLittleEndian);
-      cantFail(writeVariableSizedInteger(CU.AbbrOffset,
-                                         CU.Format == dwarf::DWARF64 ? 8 : 4,
-                                         OS, DebugInfo.IsLittleEndian));
-    } else {
-      cantFail(writeVariableSizedInteger(CU.AbbrOffset,
-                                         CU.Format == dwarf::DWARF64 ? 8 : 4,
-                                         OS, DebugInfo.IsLittleEndian));
-      writeInteger((uint8_t)CU.AddrSize, OS, DebugInfo.IsLittleEndian);
-    }
-  }
+static unsigned getOffsetSize(const DWARFYAML::Unit &Unit) {
+  return Unit.Format == dwarf::DWARF64 ? 8 : 4;
 
-  void onStartDIE(const DWARFYAML::Unit &CU,
-                  const DWARFYAML::Entry &DIE) override {
-    encodeULEB128(DIE.AbbrCode, OS);
-  }
+static unsigned getRefSize(const DWARFYAML::Unit &Unit) {
+  if (Unit.Version == 2)
+    return Unit.AddrSize;
+  return getOffsetSize(Unit);
 
-  void onValue(const uint8_t U) override {
-    writeInteger(U, OS, DebugInfo.IsLittleEndian);
-  }
+static Expected<uint64_t> writeDIE(ArrayRef<DWARFYAML::Abbrev> AbbrevDecls,
+                                   const DWARFYAML::Unit &Unit,
+                                   const DWARFYAML::Entry &Entry,
+                                   raw_ostream &OS, bool IsLittleEndian) {
+  uint64_t EntryBegin = OS.tell();
+  encodeULEB128(Entry.AbbrCode, OS);
+  uint32_t AbbrCode = Entry.AbbrCode;
+  if (AbbrCode == 0 || Entry.Values.empty())
+    return OS.tell() - EntryBegin;
 
-  void onValue(const uint16_t U) override {
-    writeInteger(U, OS, DebugInfo.IsLittleEndian);
+  if (AbbrCode > AbbrevDecls.size())
+    return createStringError(
+        errc::invalid_argument,
+        "abbrev code must be less than or equal to the number of "
+        "entries in abbreviation table");
+  const DWARFYAML::Abbrev &Abbrev = AbbrevDecls[AbbrCode - 1];
+  auto FormVal = Entry.Values.begin();
+  auto AbbrForm = Abbrev.Attributes.begin();
+  for (; FormVal != Entry.Values.end() && AbbrForm != Abbrev.Attributes.end();
+       ++FormVal, ++AbbrForm) {
+    dwarf::Form Form = AbbrForm->Form;
+    bool Indirect;
+    do {
+      Indirect = false;
+      switch (Form) {
+      case dwarf::DW_FORM_addr:
+        // TODO: Test this error.
+        if (Error Err = writeVariableSizedInteger(FormVal->Value, Unit.AddrSize,
+                                                  OS, IsLittleEndian))
+          return std::move(Err);
+        break;
+      case dwarf::DW_FORM_ref_addr:
+        // TODO: Test this error.
+        if (Error Err = writeVariableSizedInteger(
+                FormVal->Value, getRefSize(Unit), OS, IsLittleEndian))
+          return std::move(Err);
+        break;
+      case dwarf::DW_FORM_exprloc:
+      case dwarf::DW_FORM_block:
+        encodeULEB128(FormVal->BlockData.size(), OS);
+        OS.write((const char *)FormVal->BlockData.data(),
+                 FormVal->BlockData.size());
+        break;
+      case dwarf::DW_FORM_block1: {
+        writeInteger((uint8_t)FormVal->BlockData.size(), OS, IsLittleEndian);
+        OS.write((const char *)FormVal->BlockData.data(),
+                 FormVal->BlockData.size());
+        break;
+      }
+      case dwarf::DW_FORM_block2: {
+        writeInteger((uint16_t)FormVal->BlockData.size(), OS, IsLittleEndian);
+        OS.write((const char *)FormVal->BlockData.data(),
+                 FormVal->BlockData.size());
+        break;
+      }
+      case dwarf::DW_FORM_block4: {
+        writeInteger((uint32_t)FormVal->BlockData.size(), OS, IsLittleEndian);
+        OS.write((const char *)FormVal->BlockData.data(),
+                 FormVal->BlockData.size());
+        break;
+      }
+      case dwarf::DW_FORM_strx:
+      case dwarf::DW_FORM_addrx:
+      case dwarf::DW_FORM_rnglistx:
+      case dwarf::DW_FORM_loclistx:
+      case dwarf::DW_FORM_udata:
+      case dwarf::DW_FORM_ref_udata:
+      case dwarf::DW_FORM_GNU_addr_index:
+      case dwarf::DW_FORM_GNU_str_index:
+        encodeULEB128(FormVal->Value, OS);
+        break;
+      case dwarf::DW_FORM_data1:
+      case dwarf::DW_FORM_ref1:
+      case dwarf::DW_FORM_flag:
+      case dwarf::DW_FORM_strx1:
+      case dwarf::DW_FORM_addrx1:
+        writeInteger((uint8_t)FormVal->Value, OS, IsLittleEndian);
+        break;
+      case dwarf::DW_FORM_data2:
+      case dwarf::DW_FORM_ref2:
+      case dwarf::DW_FORM_strx2:
+      case dwarf::DW_FORM_addrx2:
+        writeInteger((uint16_t)FormVal->Value, OS, IsLittleEndian);
+        break;
+      case dwarf::DW_FORM_data4:
+      case dwarf::DW_FORM_ref4:
+      case dwarf::DW_FORM_ref_sup4:
+      case dwarf::DW_FORM_strx4:
+      case dwarf::DW_FORM_addrx4:
+        writeInteger((uint32_t)FormVal->Value, OS, IsLittleEndian);
+        break;
+      case dwarf::DW_FORM_data8:
+      case dwarf::DW_FORM_ref8:
+      case dwarf::DW_FORM_ref_sup8:
+      case dwarf::DW_FORM_ref_sig8:
+        writeInteger((uint64_t)FormVal->Value, OS, IsLittleEndian);
+        break;
+      case dwarf::DW_FORM_sdata:
+        encodeSLEB128(FormVal->Value, OS);
+        break;
+      case dwarf::DW_FORM_string:
+        OS.write(FormVal->CStr.data(), FormVal->CStr.size());
+        OS.write('\0');
+        break;
+      case dwarf::DW_FORM_indirect:
+        encodeULEB128(FormVal->Value, OS);
+        Indirect = true;
+        Form = static_cast<dwarf::Form>((uint64_t)FormVal->Value);
+        ++FormVal;
+        break;
+      case dwarf::DW_FORM_strp:
+      case dwarf::DW_FORM_sec_offset:
+      case dwarf::DW_FORM_GNU_ref_alt:
+      case dwarf::DW_FORM_GNU_strp_alt:
+      case dwarf::DW_FORM_line_strp:
+      case dwarf::DW_FORM_strp_sup:
+        // TODO: Test this error.
+        if (Error Err = writeVariableSizedInteger(
+                FormVal->Value, getOffsetSize(Unit), OS, IsLittleEndian))
+          return std::move(Err);
+        break;
+      default:
+        break;
+      }
+    } while (Indirect);
   }
 
-  void onValue(const uint32_t U) override {
-    writeInteger(U, OS, DebugInfo.IsLittleEndian);
-  }
+  return OS.tell() - EntryBegin;
 
-  void onValue(const uint64_t U, const bool LEB = false) override {
-    if (LEB)
-      encodeULEB128(U, OS);
-    else
-      writeInteger(U, OS, DebugInfo.IsLittleEndian);
-  }
+static void writeDWARFOffset(uint64_t Offset, dwarf::DwarfFormat Format,
+                             raw_ostream &OS, bool IsLittleEndian) {
+  cantFail(writeVariableSizedInteger(Offset, Format == dwarf::DWARF64 ? 8 : 4,
+                                     OS, IsLittleEndian));
 
-  void onValue(const int64_t S, const bool LEB = false) override {
-    if (LEB)
-      encodeSLEB128(S, OS);
+Error DWARFYAML::emitDebugInfo(raw_ostream &OS, const DWARFYAML::Data &DI) {
+  for (const DWARFYAML::Unit &Unit : DI.CompileUnits) {
+    uint64_t Length = 3; // sizeof(version) + sizeof(address_size)
+    Length += Unit.Version >= 5 ? 1 : 0; // sizeof(unit_type)
+    Length +=
+        Unit.Format == dwarf::DWARF64 ? 8 : 4; // sizeof(debug_abbrev_offset)
+
+    // Since the length of the current compilation unit is undetermined yet, we
+    // firstly write the content of the compilation unit to a buffer to
+    // calculate it and then serialize the buffer content to the actual output
+    // stream.
+    std::string EntryBuffer;
+    raw_string_ostream EntryBufferOS(EntryBuffer);
+
+    for (const DWARFYAML::Entry &Entry : Unit.Entries) {
+      if (Expected<uint64_t> EntryLength = writeDIE(
+              DI.AbbrevDecls, Unit, Entry, EntryBufferOS, DI.IsLittleEndian))
+        Length += *EntryLength;
       else
-      writeInteger(S, OS, DebugInfo.IsLittleEndian);
+        return EntryLength.takeError();
     }
 
-  void onValue(const StringRef String) override {
-    OS.write(String.data(), String.size());
-    OS.write('\0');
-  }
+    // If the length is specified in the YAML description, we use it instead of
+    // the actual length.
+    if (Unit.Length)
+      Length = *Unit.Length;
 
-  void onValue(const MemoryBufferRef MBR) override {
-    OS.write(MBR.getBufferStart(), MBR.getBufferSize());
+    writeInitialLength(Unit.Format, Length, OS, DI.IsLittleEndian);
+    writeInteger((uint16_t)Unit.Version, OS, DI.IsLittleEndian);
+    if (Unit.Version >= 5) {
+      writeInteger((uint8_t)Unit.Type, OS, DI.IsLittleEndian);
+      writeInteger((uint8_t)Unit.AddrSize, OS, DI.IsLittleEndian);
+      writeDWARFOffset(Unit.AbbrOffset, Unit.Format, OS, DI.IsLittleEndian);
+    } else {
+      writeDWARFOffset(Unit.AbbrOffset, Unit.Format, OS, DI.IsLittleEndian);
+      writeInteger((uint8_t)Unit.AddrSize, OS, DI.IsLittleEndian);
     }
 
-public:
-  DumpVisitor(const DWARFYAML::Data &DI, raw_ostream &Out)
-      : DWARFYAML::ConstVisitor(DI), OS(Out) {}
-};
-} // namespace
+    OS.write(EntryBuffer.data(), EntryBuffer.size());
+  }
 
-Error DWARFYAML::emitDebugInfo(raw_ostream &OS, const DWARFYAML::Data &DI) {
-  DumpVisitor Visitor(DI, OS);
-  return Visitor.traverseDebugInfo();
+  return Error::success();
 }
 
 static void emitFileEntry(raw_ostream &OS, const DWARFYAML::File &File) {
   OS.write(File.Name.data(), File.Name.size());
   OS.write('\0');
   encodeULEB128(File.DirIdx, OS);
   encodeULEB128(File.ModTime, OS);
   encodeULEB128(File.Length, OS);
if (Error Err = EmitFunc(DebugInfoStream, DI))
     return Err;
   DebugInfoStream.flush();
   if (!Data.empty())
     OutputBuffers[Sec] = MemoryBuffer::getMemBufferCopy(Data);
 
   return Error::success();
 }
 
-namespace {
-class DIEFixupVisitor : public DWARFYAML::Visitor {
-  uint64_t Length;
-
-public:
-  DIEFixupVisitor(DWARFYAML::Data &DI) : DWARFYAML::Visitor(DI){};
-
-protected:
-  void onStartCompileUnit(DWARFYAML::Unit &CU) override {
-    // Size of the unit header, excluding the length field itself.
-    Length = CU.Version >= 5 ? 8 : 7;
-  }
-
-  void onEndCompileUnit(DWARFYAML::Unit &CU) override { CU.Length = Length; }
-
-  void onStartDIE(DWARFYAML::Unit &CU, DWARFYAML::Entry &DIE) override {
-    Length += getULEB128Size(DIE.AbbrCode);
-  }
-
-  void onValue(const uint8_t U) override { Length += 1; }
-  void onValue(const uint16_t U) override { Length += 2; }
-  void onValue(const uint32_t U) override { Length += 4; }
-  void onValue(const uint64_t U, const bool LEB = false) override {
-    if (LEB)
-      Length += getULEB128Size(U);
-    else
-      Length += 8;
-  }
-  void onValue(const int64_t S, const bool LEB = false) override {
-    if (LEB)
-      Length += getSLEB128Size(S);
-    else
-      Length += 8;
-  }
-  void onValue(const StringRef String) override { Length += String.size() + 1; }
-
-  void onValue(const MemoryBufferRef MBR) override {
-    Length += MBR.getBufferSize();
-  }
-};
-} // namespace
-
 Expected<StringMap<std::unique_ptr<MemoryBuffer>>>
-DWARFYAML::emitDebugSections(StringRef YAMLString, bool ApplyFixups,
-                             bool IsLittleEndian) {
+DWARFYAML::emitDebugSections(StringRef YAMLString, bool IsLittleEndian) {
   auto CollectDiagnostic = [](const SMDiagnostic &Diag, void *DiagContext) {
     *static_cast<SMDiagnostic *>(DiagContext) = Diag;
   };
 
   SMDiagnostic GeneratedDiag;
   yaml::Input YIn(YAMLString, /*Ctxt=*/nullptr, CollectDiagnostic,
                   &GeneratedDiag);
 
   DWARFYAML::Data DI;
   DI.IsLittleEndian = IsLittleEndian;
   YIn >> DI;
   if (YIn.error())
     return createStringError(YIn.error(), GeneratedDiag.getMessage());
 
-  if (ApplyFixups) {
-    DIEFixupVisitor DIFixer(DI);
-    if (Error Err = DIFixer.traverseDebugInfo())
-      return std::move(Err);
-  }
-
   StringMap<std::unique_ptr<MemoryBuffer>> DebugSections;
   Error Err = emitDebugSectionImpl(DI, &DWARFYAML::emitDebugInfo, "debug_info",
                                    DebugSections);
   Err = joinErrors(std::move(Err),
                    emitDebugSectionImpl(DI, &DWARFYAML::emitDebugLine,
                                         "debug_line", DebugSections));
   Err = joinErrors(std::move(Err),
                    emitDebugSectionImpl(DI, &DWARFYAML::emitDebugStr,

llvm/lib/ObjectYAML/DWARFVisitor.h

-//===--- DWARFVisitor.h -----------------------------------------*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef LLVM_OBJECTYAML_DWARFVISITOR_H
-#define LLVM_OBJECTYAML_DWARFVISITOR_H
-
-#include "llvm/ADT/StringRef.h"
-#include "llvm/BinaryFormat/Dwarf.h"
-#include "llvm/Support/MemoryBuffer.h"
-
-namespace llvm {
-class Error;
-
-namespace DWARFYAML {
-
-struct Data;
-struct Unit;
-struct Entry;
-struct FormValue;
-struct AttributeAbbrev;
-
-/// A class to visits DWARFYAML Compile Units and DIEs in preorder.
-///
-/// Extensions of this class can either maintain const or non-const references
-/// to the DWARFYAML::Data object.
-template <typename T> class VisitorImpl {
-protected:
-  T &DebugInfo;
-
-  /// Visitor Functions
-  /// @{
-  virtual void onStartCompileUnit(Unit &CU) {}
-  virtual void onEndCompileUnit(Unit &CU) {}
-  virtual void onStartDIE(Unit &CU, Entry &DIE) {}
-  virtual void onEndDIE(Unit &CU, Entry &DIE) {}
-  virtual void onForm(AttributeAbbrev &AttAbbrev, FormValue &Value) {}
-  /// @}
-
-  /// Const Visitor Functions
-  /// @{
-  virtual void onStartCompileUnit(const Unit &CU) {}
-  virtual void onEndCompileUnit(const Unit &CU) {}
-  virtual void onStartDIE(const Unit &CU, const Entry &DIE) {}
-  virtual void onEndDIE(const Unit &CU, const Entry &DIE) {}
-  virtual void onForm(const AttributeAbbrev &AttAbbrev,
-                      const FormValue &Value) {}
-  /// @}
-
-  /// Value visitors
-  /// @{
-  virtual void onValue(const uint8_t U) {}
-  virtual void onValue(const uint16_t U) {}
-  virtual void onValue(const uint32_t U) {}
-  virtual void onValue(const uint64_t U, const bool LEB = false) {}
-  virtual void onValue(const int64_t S, const bool LEB = false) {}
-  virtual void onValue(const StringRef String) {}
-  virtual void onValue(const MemoryBufferRef MBR) {}
-  /// @}
-
-public:
-  VisitorImpl(T &DI) : DebugInfo(DI) {}
-
-  virtual ~VisitorImpl() {}
-
-  Error traverseDebugInfo();
-
-private:
-  void onVariableSizeValue(uint64_t U, unsigned Size);
-};
-
-// Making the visior instantiations extern and explicit in the cpp file. This
-// prevents them from being instantiated in every compile unit that uses the
-// visitors.
-extern template class VisitorImpl<DWARFYAML::Data>;
-extern template class VisitorImpl<const DWARFYAML::Data>;
-
-class Visitor : public VisitorImpl<Data> {
-public:
-  Visitor(Data &DI) : VisitorImpl<Data>(DI) {}
-};
-
-class ConstVisitor : public VisitorImpl<const Data> {
-public:
-  ConstVisitor(const Data &DI) : VisitorImpl<const Data>(DI) {}
-};
-
-} // namespace DWARFYAML
-} // namespace llvm
-
-#endif

llvm/lib/ObjectYAML/DWARFVisitor.cpp

-//===--- DWARFVisitor.cpp ---------------------------------------*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-//
-//===----------------------------------------------------------------------===//
-
-#include "DWARFVisitor.h"
-#include "llvm/BinaryFormat/Dwarf.h"
-#include "llvm/ObjectYAML/DWARFYAML.h"
-#include "llvm/Support/Errc.h"
-#include "llvm/Support/Error.h"
-
-using namespace llvm;
-
-template <typename T>
-void DWARFYAML::VisitorImpl<T>::onVariableSizeValue(uint64_t U, unsigned Size) {
-  switch (Size) {
-  case 8:
-    onValue((uint64_t)U);
-    break;
-  case 4:
-    onValue((uint32_t)U);
-    break;
-  case 2:
-    onValue((uint16_t)U);
-    break;
-  case 1:
-    onValue((uint8_t)U);
-    break;
-  default:
-    llvm_unreachable("Invalid integer write size.");
-  }
-}
-
-static unsigned getOffsetSize(const DWARFYAML::Unit &Unit) {
-  return Unit.Format == dwarf::DWARF64 ? 8 : 4;
-}
-
-static unsigned getRefSize(const DWARFYAML::Unit &Unit) {
-  if (Unit.Version == 2)
-    return Unit.AddrSize;
-  return getOffsetSize(Unit);
-}
-
-template <typename T> Error DWARFYAML::VisitorImpl<T>::traverseDebugInfo() {
-  for (auto &Unit : DebugInfo.CompileUnits) {
-    onStartCompileUnit(Unit);
-
-    for (auto &Entry : Unit.Entries) {
-      onStartDIE(Unit, Entry);
-      uint32_t AbbrCode = Entry.AbbrCode;
-      if (AbbrCode == 0 || Entry.Values.empty())
-        continue;
-
-      if (AbbrCode > DebugInfo.AbbrevDecls.size())
-        return createStringError(
-            errc::invalid_argument,
-            "abbrev code must be less than or equal to the number of "
-            "entries in abbreviation table");
-      const DWARFYAML::Abbrev &Abbrev = DebugInfo.AbbrevDecls[AbbrCode - 1];
-      auto FormVal = Entry.Values.begin();
-      auto AbbrForm = Abbrev.Attributes.begin();
-      for (;
-           FormVal != Entry.Values.end() && AbbrForm != Abbrev.Attributes.end();
-           ++FormVal, ++AbbrForm) {
-        onForm(*AbbrForm, *FormVal);
-        dwarf::Form Form = AbbrForm->Form;
-        bool Indirect;
-        do {
-          Indirect = false;
-          switch (Form) {
-          case dwarf::DW_FORM_addr:
-            onVariableSizeValue(FormVal->Value, Unit.AddrSize);
-            break;
-          case dwarf::DW_FORM_ref_addr:
-            onVariableSizeValue(FormVal->Value, getRefSize(Unit));
-            break;
-          case dwarf::DW_FORM_exprloc:
-          case dwarf::DW_FORM_block:
-            onValue((uint64_t)FormVal->BlockData.size(), true);
-            onValue(
-                MemoryBufferRef(StringRef((const char *)&FormVal->BlockData[0],
-                                          FormVal->BlockData.size()),
-                                ""));
-            break;
-          case dwarf::DW_FORM_block1: {
-            auto writeSize = FormVal->BlockData.size();
-            onValue((uint8_t)writeSize);
-            onValue(
-                MemoryBufferRef(StringRef((const char *)&FormVal->BlockData[0],
-                                          FormVal->BlockData.size()),
-                                ""));
-            break;
-          }
-          case dwarf::DW_FORM_block2: {
-            auto writeSize = FormVal->BlockData.size();
-            onValue((uint16_t)writeSize);
-            onValue(
-                MemoryBufferRef(StringRef((const char *)&FormVal->BlockData[0],
-                                          FormVal->BlockData.size()),
-                                ""));
-            break;
-          }
-          case dwarf::DW_FORM_block4: {
-            auto writeSize = FormVal->BlockData.size();
-            onValue((uint32_t)writeSize);
-            onValue(
-                MemoryBufferRef(StringRef((const char *)&FormVal->BlockData[0],
-                                          FormVal->BlockData.size()),
-                                ""));
-            break;
-          }
-          case dwarf::DW_FORM_strx:
-          case dwarf::DW_FORM_addrx:
-          case dwarf::DW_FORM_rnglistx:
-          case dwarf::DW_FORM_loclistx:
-          case dwarf::DW_FORM_udata:
-          case dwarf::DW_FORM_ref_udata:
-          case dwarf::DW_FORM_GNU_addr_index:
-          case dwarf::DW_FORM_GNU_str_index:
-            onValue((uint64_t)FormVal->Value, /*LEB=*/true);
-            break;
-          case dwarf::DW_FORM_data1:
-          case dwarf::DW_FORM_ref1:
-          case dwarf::DW_FORM_flag:
-          case dwarf::DW_FORM_strx1:
-          case dwarf::DW_FORM_addrx1:
-            onValue((uint8_t)FormVal->Value);
-            break;
-          case dwarf::DW_FORM_data2:
-          case dwarf::DW_FORM_ref2:
-          case dwarf::DW_FORM_strx2:
-          case dwarf::DW_FORM_addrx2:
-            onValue((uint16_t)FormVal->Value);
-            break;
-          case dwarf::DW_FORM_data4:
-          case dwarf::DW_FORM_ref4:
-          case dwarf::DW_FORM_ref_sup4:
-          case dwarf::DW_FORM_strx4:
-          case dwarf::DW_FORM_addrx4:
-            onValue((uint32_t)FormVal->Value);
-            break;
-          case dwarf::DW_FORM_data8:
-          case dwarf::DW_FORM_ref8:
-          case dwarf::DW_FORM_ref_sup8:
-          case dwarf::DW_FORM_ref_sig8:
-            onValue((uint64_t)FormVal->Value);
-            break;
-          case dwarf::DW_FORM_sdata:
-            onValue((int64_t)FormVal->Value, true);
-            break;
-          case dwarf::DW_FORM_string:
-            onValue(FormVal->CStr);
-            break;
-          case dwarf::DW_FORM_indirect:
-            onValue((uint64_t)FormVal->Value, true);
-            Indirect = true;
-            Form = static_cast<dwarf::Form>((uint64_t)FormVal->Value);
-            ++FormVal;
-            break;
-          case dwarf::DW_FORM_strp:
-          case dwarf::DW_FORM_sec_offset:
-          case dwarf::DW_FORM_GNU_ref_alt:
-          case dwarf::DW_FORM_GNU_strp_alt:
-          case dwarf::DW_FORM_line_strp:
-          case dwarf::DW_FORM_strp_sup:
-            onVariableSizeValue(FormVal->Value, getOffsetSize(Unit));
-            break;
-          default:
-            break;
-          }
-        } while (Indirect);
-      }
-      onEndDIE(Unit, Entry);
-    }
-    onEndCompileUnit(Unit);
-  }
-
-  return Error::success();
-}
-
-// Explicitly instantiate the two template expansions.
-template class DWARFYAML::VisitorImpl<DWARFYAML::Data>;
-template class DWARFYAML::VisitorImpl<const DWARFYAML::Data>;

llvm/lib/ObjectYAML/DWARFYAML.cpp

void MappingTraits<DWARFYAML::PubSection>::mapping(
   IO.mapRequired("Version", Section.Version);
   IO.mapRequired("UnitOffset", Section.UnitOffset);
   IO.mapRequired("UnitSize", Section.UnitSize);
   IO.mapRequired("Entries", Section.Entries);
 }
 
 void MappingTraits<DWARFYAML::Unit>::mapping(IO &IO, DWARFYAML::Unit &Unit) {
   IO.mapOptional("Format", Unit.Format, dwarf::DWARF32);
-  IO.mapOptional("Length", Unit.Length, 0);
+  IO.mapOptional("Length", Unit.Length);
   IO.mapRequired("Version", Unit.Version);
   if (Unit.Version >= 5)
     IO.mapRequired("UnitType", Unit.Type);
   IO.mapRequired("AbbrOffset", Unit.AbbrOffset);
   IO.mapRequired("AddrSize", Unit.AddrSize);
   IO.mapOptional("Entries", Unit.Entries);
 }
 

llvm/test/tools/yaml2obj/ELF/DWARF/debug-info.yaml

 DWARF:
   debug_info:
     - Format:     DWARF64
       Length:     0x0c
       Version:    4
       AbbrOffset: 0x1234
       AddrSize:   8
       Entries:    []
+
+## k) Test that yaml2obj is able to emit a correct length for compilation units.
+
+# RUN: yaml2obj --docnum=13 %s -o %t13.o
+# RUN: llvm-readelf --hex-dump=.debug_info %t13.o | \
+# RUN:   FileCheck %s --check-prefix=INFER-LENGTH
+
+#      INFER-LENGTH: Hex dump of section '.debug_info':
+# INFER-LENGTH-NEXT: 0x00000000 37000000 04000000 00000801 00000000 7...............
+##                              ^-------                            unit_length (0x37)
+##                                       ^---                       version (2-byte)
+##                                           ^--------              debug_abbrev_offset (4-byte)
+##                                                    ^-            address_size (1-byte)
+##                                                      ^-          abbrev code (ULEB128) 0x01
+##                                                         ^------- Form: DW_FORM_strp (4-byte)
+# INFER-LENGTH-NEXT: 0x00000010 0c001600 00000000 00001e00 00002011 .............. .
+##                              ^---                                Form: DW_FORM_data2 (2-byte)
+##                                  ^--------                       Form: DW_FORM_strp (4-byte)
+##                                           ^--------              Form: DW_FORM_sec_offset (4-byte)
+##                                                    ^--------     Form: DW_FORM_strp (4-byte)
+##                                                             ^--- Form: DW_FORM_addr (8-byte)
+# INFER-LENGTH-NEXT: 0x00000020 00000000 00003300 00000220 11000000 ......3.... ....
+##                              -------------
+##                                           ^--------              Form: DW_FORM_data4 (4-byte)
+##                                                    ^-            abbrev code (ULEB128) 0x02
+##                                                      ^---------- Form: DW_FORM_addr (8-byte)
+# INFER-LENGTH-NEXT: 0x00000030 00000006 00000038 000000            .......8...
+##                              ------
+##                                    ^--------                     Form: DW_FORM_data4 (4-byte)
+##                                             ^--------            Form: DW_FORM_strp (4-byte)
+
+## The handwritten DIEs should look like:
+
+## 0x0000000b: DW_TAG_compile_unit [1] *
+##               DW_AT_producer [DW_FORM_strp]        ( .debug_str[0x00000000] = "clang version 10.0.0 ")
+##               DW_AT_language [DW_FORM_data2]       (DW_LANG_C99)
+##               DW_AT_name [DW_FORM_strp]            ( .debug_str[0x00000016] = "hello.c")
+##               DW_AT_stmt_list [DW_FORM_sec_offset] (0x00000000)
+##               DW_AT_comp_dir [DW_FORM_strp]        ( .debug_str[0x0000001e] = "/home/v/x/llvm/playground")
+##               DW_AT_low_pc [DW_FORM_addr]          (0x0000000000001120)
+##               DW_AT_high_pc [DW_FORM_data4]        (0x00000033)
+## 
+## 0x0000002a:   DW_TAG_subprogram [2]  
+##                 DW_AT_low_pc [DW_FORM_addr]        (0x0000000000001120)
+##                 DW_AT_high_pc [DW_FORM_data4]      (0x00000006)
+##                 DW_AT_name [DW_FORM_strp]          ( .debug_str[0x00000038] = "main")
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_EXEC
+  Machine: EM_X86_64
+DWARF:
+  debug_str:
+    - "clang version 10.0.0 "
+    - "hello.c"
+    - "/home/v/x/llvm/playground"
+    - "main"
+  debug_abbrev:
+    - Code:     1
+      Tag:      DW_TAG_compile_unit
+      Children: DW_CHILDREN_yes
+      Attributes:
+        - Attribute: DW_AT_producer
+          Form:      DW_FORM_strp
+        - Attribute: DW_AT_language
+          Form:      DW_FORM_data2
+        - Attribute: DW_AT_name
+          Form:      DW_FORM_strp
+        - Attribute: DW_AT_stmt_list
+          Form:      DW_FORM_sec_offset
+        - Attribute: DW_AT_comp_dir
+          Form:      DW_FORM_strp
+        - Attribute: DW_AT_low_pc
+          Form:      DW_FORM_addr
+        - Attribute: DW_AT_high_pc
+          Form:      DW_FORM_data4
+    - Code: 2
+      Tag:  DW_TAG_subprogram
+      Children: DW_CHILDREN_no
+      Attributes:
+        - Attribute: DW_AT_low_pc
+          Form:      DW_FORM_addr
+        - Attribute: DW_AT_high_pc
+          Form:      DW_FORM_data4
+        - Attribute: DW_AT_name
+          Form:      DW_FORM_strp
+  debug_info:
+    - Version:    4
+      AbbrOffset: 0x00
+      AddrSize:   0x08
+      Entries:
+        - AbbrCode: 1
+          Values:
+            - Value: 0x00   ## DW_AT_producer [DW_FORM_strp]
+            - Value: 0x0c   ## DW_LANG_C99, DW_AT_language [DW_FORM_data2]
+            - Value: 0x16   ## DW_AT_name [DW_FORM_strp]
+            - Value: 0x00   ## DW_AT_stmt_list [DW_FORM_sec_offset]
+            - Value: 0x1e   ## DW_AT_comp_dir [DW_FORM_strp]
+            - Value: 0x1120 ## DW_AT_low_pc [DW_FORM_addr]
+            - Value: 0x33   ## DW_AT_high_pc [DW_FORM_data4]
+        - AbbrCode: 2
+          Values:
+            - Value: 0x1120 ## DW_AT_low_pc [DW_FORM_addr]
+            - Value: 0x06   ## DW_AT_high_pc [DW_FORM_data4]
+            - Value: 0x38   ## DW_AT_name [DW_FORM_strp]

llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp

const char *yamldata = "debug_abbrev:\n"
                          "    AbbrOffset:      0\n"
                          "    AddrSize:        8\n"
                          "    Entries:\n"
                          "      - AbbrCode:        0x00000001\n"
                          "        Values:\n"
                          "      - AbbrCode:        0x00000000\n"
                          "        Values:\n";
 
-  auto ErrOrSections = DWARFYAML::emitDebugSections(StringRef(yamldata), true);
+  auto ErrOrSections = DWARFYAML::emitDebugSections(StringRef(yamldata));
   ASSERT_TRUE((bool)ErrOrSections);
   std::unique_ptr<DWARFContext> DwarfContext =
       DWARFContext::create(*ErrOrSections, 8);
 
   // Verify the number of compile units is correct.
   uint32_t NumCUs = DwarfContext->getNumCompileUnits();
   EXPECT_EQ(NumCUs, 1u);
   DWARFCompileUnit *U =

llvm/unittests/DebugInfo/DWARF/DWARFDieTest.cpp

debug_info:
             Values:
               - Value:           12
               - Value:           0x0000000000000001
                 BlockData:       [ 0x47 ]
               - Value:           20
               - Value:           25
   )";
   Expected<StringMap<std::unique_ptr<MemoryBuffer>>> Sections =
-      DWARFYAML::emitDebugSections(StringRef(yamldata), /*ApplyFixups=*/true,
+      DWARFYAML::emitDebugSections(StringRef(yamldata),
                                    /*IsLittleEndian=*/true);
   ASSERT_THAT_EXPECTED(Sections, Succeeded());
   std::vector<uint8_t> Loclists{
       // Header
       0, 0, 0, 0, // Length
       5, 0,       // Version
       4,          // Address size
       0,          // Segment selector size