Initialize optional members of ELFYAML types.

[vzakhari]

Summary

This change-set initializes optional memrbers of ELFYAML types to their default values (as defined in ELFYAML.cpp mapping traits).

For example, since Section has a user-defined constructor, one cannot neither default-initialize (e.g. with Section MySection;) nor zero-initilize it (e.g. with Section MySection{};) completely. The optional members will be uninitialized. The same is true for MipsABIFlags.

Moreover, YamlIO documentation says that the native types must have default constructors that must initialize the optional members: https://llvm.org/docs/YamlIO.html#default-values

This change-set is almost no-op, since I have not found uses that output ELFYAML structures as YAML. The new unit test seems to be the first such use. The unit test will fail in an undefined way, if the optional members are not properly initialized.

[rahmanl]

BBAddrMap-related changes LGTM.

[MaskRay]

llvm/include/llvm/ObjectYAML/ELFYAML.h

661

Let strong typedef Hex16/MIPS_AFL_REG initialize the base value

[vzakhari]

llvm/include/llvm/ObjectYAML/ELFYAML.h

661

Sorry, I do not understand this.

Are you suggesting to let Hex16's default constructor to initialize the actual value? I believe there is no default constructor that would do that.

#define LLVM_YAML_STRONG_TYPEDEF(_base, _type)                                 \
    struct _type {                                                             \
        _type() = default;                                                     \
        _type(const _base v) : value(v) {}                                     \
        _type(const _type &v) = default;                                       \
        _type &operator=(const _type &rhs) = default;                          \
        _type &operator=(const _base &rhs) { value = rhs; return *this; }      \
        operator const _base & () const { return value; }                      \
        bool operator==(const _type &rhs) const { return value == rhs.value; } \
        bool operator==(const _base &rhs) const { return value == rhs; }       \
        bool operator<(const _type &rhs) const { return value < rhs.value; }   \
        _base value;                                                           \
        using BaseType = _base;                                                \
    };

Default construction will leave value uninitialized for POD _base type. Zero-initializaton will set value to 0.

[vzakhari]

llvm/include/llvm/ObjectYAML/ELFYAML.h

661

Just to clarify, I preferred value-initialization over zero-initialization to make all the initializations consistent, and since not all default values may be 0.

[MaskRay]

llvm/include/llvm/ObjectYAML/ELFYAML.h

661

You can initialize _base value; , then you don't need to spread {0} everywhere.

[vzakhari]

llvm/include/llvm/ObjectYAML/ELFYAML.h

661

Can you please suggest the exact change? Do you mean something like _base value = _base()?

In general, _base type may have a deleted default constructor, so this may not work.

[vzakhari]

ping

[jhenderson]

llvm/include/llvm/ObjectYAML/ELFYAML.h

661

Are there any actual cases in current practice where the default constructor has been deleted?

[vzakhari]

llvm/include/llvm/ObjectYAML/ELFYAML.h

661

I tried adding the in-class initialization as _base value = _base() and hit the following issue:

llvm/include/llvm/ObjectYAML/WasmYAML.h:86:8: note: ‘llvm::WasmYAML::Import::Import()’ is implicitly deleted because the default definition would be ill-formed:
 struct Import {
        ^~~~~~
llvm/include/llvm/ObjectYAML/WasmYAML.h:92:12: error: union member ‘llvm::WasmYAML::Import::<unnamed union>::GlobalImport’ with non-trivial ‘llvm::WasmYAML::Global::Global()’
     Global GlobalImport;
            ^~~~~~~~~~~~
llvm/include/llvm/ObjectYAML/WasmYAML.h:93:11: error: union member ‘llvm::WasmYAML::Import::<unnamed union>::TableImport’ with non-trivial ‘llvm::WasmYAML::Table::Table()’
     Table TableImport;
           ^~~~~~~~~~~
llvm/include/llvm/ObjectYAML/WasmYAML.h:94:12: error: union member ‘llvm::WasmYAML::Import::<unnamed union>::Memory’ with non-trivial ‘constexpr llvm::WasmYAML::Limits::Limits()’
     Limits Memory;
            ^~~~~~

[vzakhari]

llvm/include/llvm/ObjectYAML/ELFYAML.h

661

I believe this is a reproducer:

struct A {
  int x = int();
};

struct B {
  union {
    A a;
  };
};

void foo() { B b = B(); }

ctr.cpp:11:20: error: call to implicitly-deleted default constructor of 'B'
void foo() { B b = B(); }
                   ^
ctr.cpp:7:7: note: default constructor of 'B' is implicitly deleted because variant field 'a' has a non-trivial default constructor
    A a;
      ^
1 error generated.

A corresponds to the strongdef'ed type.

[jhenderson]

@vzakhari, what's the motivation for doing this? These data structures are used within yaml2obj/obj2yaml, although I'm not particularly familiar with the plumbing to identify whether it matters. My suspicion is that it doesn't.

llvm/include/llvm/ObjectYAML/ELFYAML.h

661

@MaskRay, do you have any suggestions on how to improve this. It looks to me like you had something in mind, but @vzakhari has been unable to identify it.

[MaskRay]

llvm/include/llvm/ObjectYAML/ELFYAML.h

661

We can change LLVM_YAML_STRONG_TYPEDEF or add a similar macro for integeral types - initialized to 0.

Then we can avoid {0} here and there.

[vzakhari]

In D101341#2753240, @jhenderson wrote:

@vzakhari, what's the motivation for doing this? These data structures are used within yaml2obj/obj2yaml, although I'm not particularly familiar with the plumbing to identify whether it matters. My suspicion is that it doesn't.

@jhenderson, there are two ways to create a YAML file using YAML mappings. The first one is to create a YAML file from a string bufffer (see line 38 in llvm/unittests/ObjectYAML/ELFYAMLTest.cpp). The second way is to populate the ELFYAML:: data structures and stream them into yaml::Output. Both ways are valid. The second way is a little bit more convenient for programmatic ELF-YAML creation in my downstream application. When I try to use ELFYAML:: data structures, there is no convenient way to make sure that all optional members in them are initialized. Neither default nor zero-initialization works. This is why I made these changes to make sure that all optional members are initialized properly. Note that the default values are not necessarily zeroes. The default values for this particular case are defined by the mappings in llvm/lib/ObjectYAML/ELFYAML.cpp. So I am not sure that adjusting LLVM_YAML_STRONG_TYPEDEF to use zero values always is the right approach.

[vzakhari]

ping

[jhenderson]

In D101341#2755605, @vzakhari wrote:

In D101341#2753240, @jhenderson wrote:

@vzakhari, what's the motivation for doing this? These data structures are used within yaml2obj/obj2yaml, although I'm not particularly familiar with the plumbing to identify whether it matters. My suspicion is that it doesn't.

@jhenderson, there are two ways to create a YAML file using YAML mappings. The first one is to create a YAML file from a string bufffer (see line 38 in llvm/unittests/ObjectYAML/ELFYAMLTest.cpp). The second way is to populate the ELFYAML:: data structures and stream them into yaml::Output. Both ways are valid. The second way is a little bit more convenient for programmatic ELF-YAML creation in my downstream application. When I try to use ELFYAML:: data structures, there is no convenient way to make sure that all optional members in them are initialized. Neither default nor zero-initialization works. This is why I made these changes to make sure that all optional members are initialized properly. Note that the default values are not necessarily zeroes.

Which cases don't have zero as their default value? The ones you've modified in this patch all are (even the Mips enums are - the enum values are value 0).

Frankly speaking, I don't like the style of the change: it looks somewhat ugly (= 0 would be better than {0} in my opinion at least), but more importantly there's no guarantee that it will continue to work - more fields could be added at a later date, for example, without this initialisation, especially as it's not clear when it needs to be explicitly initialised (this change for example doesn't change the Size field of the StackSizeEntry struct). It sounds like there's no internal-to-LLVM usage of this approach, so it could easily be considered unnecessary and get dropped at a later date too.

[vzakhari]

In D101341#2781231, @jhenderson wrote:

In D101341#2755605, @vzakhari wrote:

In D101341#2753240, @jhenderson wrote:

@vzakhari, what's the motivation for doing this? These data structures are used within yaml2obj/obj2yaml, although I'm not particularly familiar with the plumbing to identify whether it matters. My suspicion is that it doesn't.

@jhenderson, there are two ways to create a YAML file using YAML mappings. The first one is to create a YAML file from a string bufffer (see line 38 in llvm/unittests/ObjectYAML/ELFYAMLTest.cpp). The second way is to populate the ELFYAML:: data structures and stream them into yaml::Output. Both ways are valid. The second way is a little bit more convenient for programmatic ELF-YAML creation in my downstream application. When I try to use ELFYAML:: data structures, there is no convenient way to make sure that all optional members in them are initialized. Neither default nor zero-initialization works. This is why I made these changes to make sure that all optional members are initialized properly. Note that the default values are not necessarily zeroes.

Which cases don't have zero as their default value? The ones you've modified in this patch all are (even the Mips enums are - the enum values are value 0).

Sorry, I do not see where I said that there are no default values. I said that neither default nor zero-initialization works - in this phrase I referred to initialization of POD members of objects, when we use default or zero-initialization. For example, look at struct MipsABIFlags: if we declare an object of this type using default initialization/construction, then members like ISARevision will be default constructed, which in turn causes their value to be undefined (because they use the default constructor of the type defined by LLVM_YAML_STRONG_TYPEDEF). Value/zero-initialization will also not work for such objects, because struct MipsABIFlags has a user-defined constructor.

Frankly speaking, I don't like the style of the change: it looks somewhat ugly (= 0 would be better than {0} in my opinion at least), but more importantly there's no guarantee that it will continue to work - more fields could be added at a later date, for example, without this initialisation, especially as it's not clear when it needs to be explicitly initialised (this change for example doesn't change the Size field of the StackSizeEntry struct). It sounds like there's no internal-to-LLVM usage of this approach, so it could easily be considered unnecessary and get dropped at a later date too.

I can try to change {0} to = 0, if this helps getting this merged. Please let me know.
Yes, there is not guarantee that newly added optional fields are properly initialized. There is no way to assert this, just as many other things developers do. Why does it matter?
The added unit test verifies that the existing optional fields are properly initialized, so this should be enough to make sure that the current changes are no dropped accidentally.

Again, I am just following the specification of YamlIO, which says: "That native class must have a default constructor. Whatever value the default constructor initially sets for an optional field will be that field’s value."
I believe this implies that the default constructors of YamlIO objects must set their optional fields to some not undefined value.

llvm/include/llvm/ObjectYAML/ELFYAML.h

 #ifndef LLVM_OBJECTYAML_ELFYAML_H
 #define LLVM_OBJECTYAML_ELFYAML_H
 
 #include "llvm/ADT/StringRef.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/Object/ELFTypes.h"
 #include "llvm/ObjectYAML/DWARFYAML.h"
 #include "llvm/ObjectYAML/YAML.h"
+#include "llvm/Support/MipsABIFlags.h"
 #include "llvm/Support/YAMLTraits.h"
 #include <cstdint>
 #include <memory>
 #include <vector>
 
 namespace llvm {
 namespace ELFYAML {
 
unsigned getDefaultShEntSize(unsigned EMachine, ELF_SHT SecType,
   }
 }
 
 // For now, hardcode 64 bits everywhere that 32 or 64 would be needed
 // since 64-bit can hold 32-bit values too.
 struct FileHeader {
   ELF_ELFCLASS Class;
   ELF_ELFDATA Data;
-  ELF_ELFOSABI OSABI;
-  llvm::yaml::Hex8 ABIVersion;
+  ELF_ELFOSABI OSABI{0};
+  llvm::yaml::Hex8 ABIVersion{0};
   ELF_ET Type;
   Optional<ELF_EM> Machine;
-  ELF_EF Flags;
-  llvm::yaml::Hex64 Entry;
+  ELF_EF Flags{0};
+  llvm::yaml::Hex64 Entry{0};
 
   Optional<llvm::yaml::Hex64> EPhOff;
   Optional<llvm::yaml::Hex16> EPhEntSize;
   Optional<llvm::yaml::Hex16> EPhNum;
   Optional<llvm::yaml::Hex16> EShEntSize;
   Optional<llvm::yaml::Hex64> EShOff;
   Optional<llvm::yaml::Hex16> EShNum;
   Optional<llvm::yaml::Hex16> EShStrNdx;
 };
 
 struct SectionHeader {
   StringRef Name;
 };
 
 struct Symbol {
   StringRef Name;
-  ELF_STT Type;
+  ELF_STT Type{0};
   Optional<StringRef> Section;
   Optional<ELF_SHN> Index;
-  ELF_STB Binding;
+  ELF_STB Binding{0};
   Optional<llvm::yaml::Hex64> Value;
   Optional<llvm::yaml::Hex64> Size;
   Optional<uint8_t> Other;
 
   Optional<uint32_t> StName;
 };
 
 struct SectionOrType {
   StringRef sectionNameOrType;
 };
 
 struct DynamicEntry {
   ELF_DYNTAG Tag;
   llvm::yaml::Hex64 Val;
 };
 
 struct BBAddrMapEntry {
   struct BBEntry {
     llvm::yaml::Hex64 AddressOffset;
     llvm::yaml::Hex64 Size;
     llvm::yaml::Hex64 Metadata;
   };
-  llvm::yaml::Hex64 Address;
+  llvm::yaml::Hex64 Address{0};
   Optional<uint64_t> NumBlocks;
   Optional<std::vector<BBEntry>> BBEntries;
 };
 
 struct StackSizeEntry {
-  llvm::yaml::Hex64 Address;
+  llvm::yaml::Hex64 Address{0};
   llvm::yaml::Hex64 Size;
 };
 
 struct NoteEntry {
   StringRef Name;
   yaml::BinaryRef Desc;
   ELF_NT Type;
 };
struct Chunk {
   virtual ~Chunk();
 };
 
 struct Section : public Chunk {
   ELF_SHT Type;
   Optional<ELF_SHF> Flags;
   Optional<llvm::yaml::Hex64> Address;
   Optional<StringRef> Link;
-  llvm::yaml::Hex64 AddressAlign;
+  llvm::yaml::Hex64 AddressAlign{0};
   Optional<llvm::yaml::Hex64> EntSize;
 
   Optional<yaml::BinaryRef> Content;
   Optional<llvm::yaml::Hex64> Size;
 
   // Holds the original section index.
-  unsigned OriginalSecNdx;
+  unsigned OriginalSecNdx{0};
 
   Section(ChunkKind Kind, bool IsImplicit = false) : Chunk(Kind, IsImplicit) {}
 
   static bool classof(const Chunk *S) {
     return S->Kind < ChunkKind::SpecialChunksStart;
   }
 
   // Some derived sections might have their own special entries. This method
struct GroupSection : Section {
   std::vector<std::pair<StringRef, bool>> getEntries() const override {
     return {{"Members", Members.hasValue()}};
   };
 
   static bool classof(const Chunk *S) { return S->Kind == ChunkKind::Group; }
 };
 
 struct Relocation {
-  llvm::yaml::Hex64 Offset;
-  YAMLIntUInt Addend;
+  llvm::yaml::Hex64 Offset{0};
+  YAMLIntUInt Addend{0};
   ELF_REL Type;
   Optional<StringRef> Symbol;
 };
 
 struct RelocationSection : Section {
   Optional<std::vector<Relocation>> Relocations;
   StringRef RelocatableSec; /* Info */
 
struct ARMIndexTableSection : Section {
 
   static bool classof(const Chunk *S) {
     return S->Kind == ChunkKind::ARMIndexTable;
   }
 };
 
 // Represents .MIPS.abiflags section
 struct MipsABIFlags : Section {
-  llvm::yaml::Hex16 Version;
+  llvm::yaml::Hex16 Version{0};
   MIPS_ISA ISALevel;
-  llvm::yaml::Hex8 ISARevision;
-  MIPS_AFL_REG GPRSize;
-  MIPS_AFL_REG CPR1Size;
-  MIPS_AFL_REG CPR2Size;
-  MIPS_ABI_FP FpABI;
-  MIPS_AFL_EXT ISAExtension;
-  MIPS_AFL_ASE ASEs;
-  MIPS_AFL_FLAGS1 Flags1;
-  llvm::yaml::Hex32 Flags2;
+  llvm::yaml::Hex8 ISARevision{0};
+  MIPS_AFL_REG GPRSize{Mips::AFL_REG_NONE};
+  MIPS_AFL_REG CPR1Size{Mips::AFL_REG_NONE};
+  MIPS_AFL_REG CPR2Size{Mips::AFL_REG_NONE};
+  MIPS_ABI_FP FpABI{Mips::Val_GNU_MIPS_ABI_FP_ANY};
+  MIPS_AFL_EXT ISAExtension{Mips::AFL_EXT_NONE};
+  MIPS_AFL_ASE ASEs{0};
+  MIPS_AFL_FLAGS1 Flags1{0};
+  llvm::yaml::Hex32 Flags2{0};
 
   MipsABIFlags() : Section(ChunkKind::MipsABIFlags) {}
 
   static bool classof(const Chunk *S) {
     return S->Kind == ChunkKind::MipsABIFlags;
   }
 };
 
 struct ProgramHeader {
   ELF_PT Type;
-  ELF_PF Flags;
-  llvm::yaml::Hex64 VAddr;
-  llvm::yaml::Hex64 PAddr;
+  ELF_PF Flags{0};
+  llvm::yaml::Hex64 VAddr{0};
+  llvm::yaml::Hex64 PAddr{0};
   Optional<llvm::yaml::Hex64> Align;
   Optional<llvm::yaml::Hex64> FileSize;
   Optional<llvm::yaml::Hex64> MemSize;
   Optional<llvm::yaml::Hex64> Offset;
   Optional<StringRef> FirstSec;
   Optional<StringRef> LastSec;
 
   // This vector contains all chunks from [FirstSec, LastSec].

llvm/unittests/ObjectYAML/ELFYAMLTest.cpp

 //===- ELFYAMLTest.cpp - Tests for ELFYAML.cpp ----------------------------===//
 //
 // 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 "llvm/ObjectYAML/ELFYAML.h"
 #include "llvm/Object/ELF.h"
 #include "llvm/Object/ELFObjectFile.h"
 #include "llvm/Object/ELFTypes.h"
 #include "llvm/ObjectYAML/yaml2obj.h"
+#include "llvm/Support/LEB128.h"
 #include "llvm/Support/YAMLTraits.h"
 #include "llvm/Testing/Support/Error.h"
 #include "gtest/gtest.h"
+#include <initializer_list>
 
 using namespace llvm;
 using namespace llvm::object;
 
 template <class ELFT>
 static Expected<ELFObjectFile<ELFT>> toBinary(SmallVectorImpl<char> &Storage,
                                               StringRef Yaml) {
   Storage.clear();
break
         NAME_CHECK(R_VE_CALL_LO32);
       default:
         FAIL() << "Found unexpected relocation type: " + Twine(R.getType());
         break;
       }
     }
   }
 }
+
+TEST(ELFOutput, OptionalMembersInit) {
+  // Test that the optional members of ELFYAML types are initialized
+  // to their default values in the constructors.
+  ELFYAML::FileHeader FileHeader;
+  FileHeader.Class = ELF::ELFCLASS64;
+  FileHeader.Data = ELF::ELFDATA2LSB;
+  FileHeader.Type = ELF::ET_REL;
+
+  ELFYAML::ProgramHeader ProgramHeader;
+  ProgramHeader.Type = ELF::PT_NOTE;
+
+  ELFYAML::StackSizeEntry SSEntry;
+  SSEntry.Size = llvm::yaml::Hex64(0x777);
+  ELFYAML::StackSizesSection SSSection;
+  SSSection.Type = ELF::SHT_PROGBITS;
+  SSSection.Name = ".stack_sizes";
+  SSSection.Entries.emplace(
+      std::initializer_list<ELFYAML::StackSizeEntry>{SSEntry});
+
+  ELFYAML::BBAddrMapEntry BBAMEntry;
+  ELFYAML::BBAddrMapSection BBAMSection;
+  BBAMSection.Type = ELF::SHT_LLVM_BB_ADDR_MAP;
+  BBAMSection.Name = ".whatever";
+  BBAMSection.Entries.emplace(
+      std::initializer_list<ELFYAML::BBAddrMapEntry>{BBAMEntry});
+
+  ELFYAML::Symbol Symbol;
+  Symbol.Name = "__some_symbol";
+  // For some reason Other is not optional.
+  Symbol.Other.emplace(0);
+
+  ELFYAML::Relocation Relocation;
+  Relocation.Type = ELF::R_X86_64_64;
+  Relocation.Symbol.emplace(Symbol.Name);
+
+  ELFYAML::RelocationSection RSection;
+  RSection.Type = ELF::SHT_RELA;
+  RSection.Name = ".rela.section";
+  RSection.Relocations.emplace(
+      std::initializer_list<ELFYAML::Relocation>{Relocation});
+
+  ELFYAML::Object Object;
+  Object.Header = FileHeader;
+  Object.ProgramHeaders.push_back(ProgramHeader);
+
+  Object.Chunks.push_back(
+      std::make_unique<ELFYAML::StackSizesSection>(std::move(SSSection)));
+  Object.Chunks.push_back(
+      std::make_unique<ELFYAML::BBAddrMapSection>(std::move(BBAMSection)));
+  Object.Symbols.emplace(std::initializer_list<ELFYAML::Symbol>{Symbol});
+  Object.Chunks.push_back(
+      std::make_unique<ELFYAML::RelocationSection>(std::move(RSection)));
+
+  SmallString<0> YAMLStorage;
+  raw_svector_ostream YAMLOS(YAMLStorage);
+  {
+    yaml::Output YAMLOut(YAMLOS);
+    YAMLOut << Object;
+  }
+
+  SmallString<0> ELFStorage;
+  Expected<ELFObjectFile<ELF64LE>> ExpectedFile =
+      toBinary<ELF64LE>(ELFStorage, YAMLStorage);
+  ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+  const ELFObjectFile<ELF64LE> &File = *ExpectedFile;
+
+  // Check FileHeader:
+  const auto &EFile = File.getELFFile();
+  const auto &EHeader = EFile.getHeader();
+  const auto &EIdent = EHeader.e_ident;
+  EXPECT_EQ(ELF::ELFCLASS64, EIdent[ELF::EI_CLASS]);
+  EXPECT_EQ(ELF::ELFDATA2LSB, EIdent[ELF::EI_DATA]);
+  EXPECT_EQ(ELF::ET_REL, EHeader.e_type);
+  EXPECT_EQ(0, EIdent[ELF::EI_OSABI]);
+  EXPECT_EQ(0, EIdent[ELF::EI_ABIVERSION]);
+  EXPECT_EQ(ELF::EM_NONE, EHeader.e_machine);
+  EXPECT_EQ(0u, EHeader.e_flags);
+  EXPECT_EQ(0u, EHeader.e_entry);
+
+  // Check ProgramHeader:
+  Expected<ELFObjectFile<ELF64LE>::Elf_Phdr_Range> ExpectedPHeaders =
+      EFile.program_headers();
+  ASSERT_THAT_EXPECTED(ExpectedPHeaders, Succeeded());
+  for (const auto &PHdr : *ExpectedPHeaders) {
+    EXPECT_EQ(ELF::PT_NOTE, PHdr.p_type);
+    EXPECT_EQ(0u, PHdr.p_flags);
+    EXPECT_EQ(0u, PHdr.p_vaddr);
+    EXPECT_EQ(0u, PHdr.p_paddr);
+  }
+
+  Expected<ELFObjectFile<ELF64LE>::Elf_Shdr_Range> ExpectedSHeaders =
+      EFile.sections();
+  ASSERT_THAT_EXPECTED(ExpectedSHeaders, Succeeded());
+
+  bool StackSizesSectionSeen = false;
+  bool BBAddrMapSectionSeen = false;
+
+  for (const auto &SHdr : *ExpectedSHeaders) {
+    Expected<StringRef> ExpectedSName = EFile.getSectionName(SHdr);
+    ASSERT_THAT_EXPECTED(ExpectedSName, Succeeded());
+    if (*ExpectedSName == ".stack_sizes") {
+      // Required:
+      EXPECT_EQ(ELF::SHT_PROGBITS, SHdr.sh_type);
+      EXPECT_EQ(0u, SHdr.sh_addralign);
+
+      Expected<ArrayRef<uint8_t>> ExpectedSContents =
+          EFile.getSectionContents(SHdr);
+      ASSERT_THAT_EXPECTED(ExpectedSContents, Succeeded());
+      uint64_t Address = llvm::support::endian::read64le(
+          reinterpret_cast<const void *>(ExpectedSContents->data()));
+      EXPECT_EQ(0u, Address);
+      uint64_t Size = decodeULEB128(ExpectedSContents->data() + 8);
+      EXPECT_EQ(0x777u, Size);
+      StackSizesSectionSeen = true;
+    } else if (*ExpectedSName == ".whatever") {
+      EXPECT_EQ(ELF::SHT_LLVM_BB_ADDR_MAP, SHdr.sh_type);
+      EXPECT_EQ(0u, SHdr.sh_addralign);
+
+      Expected<ArrayRef<uint8_t>> ExpectedSContents =
+          EFile.getSectionContents(SHdr);
+      ASSERT_THAT_EXPECTED(ExpectedSContents, Succeeded());
+      uint64_t Address = llvm::support::endian::read64le(
+          reinterpret_cast<const void *>(ExpectedSContents->data()));
+      EXPECT_EQ(0u, Address);
+      BBAddrMapSectionSeen = true;
+    }
+  }
+
+  EXPECT_EQ(true, StackSizesSectionSeen);
+  EXPECT_EQ(true, BBAddrMapSectionSeen);
+
+  bool SomeSymbolSeen = false;
+  for (const auto &SymRef : File.symbols()) {
+    Expected<StringRef> ExpectedSName = SymRef.getName();
+    ASSERT_THAT_EXPECTED(ExpectedSName, Succeeded());
+    if (*ExpectedSName == "__some_symbol") {
+      EXPECT_EQ(ELF::STT_NOTYPE, SymRef.getELFType());
+      EXPECT_EQ(ELF::STB_LOCAL, SymRef.getBinding());
+      SomeSymbolSeen = true;
+    }
+  }
+  EXPECT_EQ(true, SomeSymbolSeen);
+
+  bool RelaSectionSeen = false;
+  for (SectionRef SecRef : File.sections()) {
+    Expected<StringRef> ExpectedSName = SecRef.getName();
+    ASSERT_THAT_EXPECTED(ExpectedSName, Succeeded());
+    if (*ExpectedSName == ".rela.section") {
+      RelaSectionSeen = true;
+      for (ELFRelocationRef RelocRef : SecRef.relocations()) {
+        EXPECT_EQ(0u, RelocRef.getOffset());
+        Expected<int64_t> ExpectedAddend = RelocRef.getAddend();
+        ASSERT_THAT_EXPECTED(ExpectedAddend, Succeeded());
+        EXPECT_EQ(0u, *ExpectedAddend);
+        EXPECT_EQ(ELF::R_X86_64_64, RelocRef.getType());
+      }
+    }
+  }
+
+  EXPECT_EQ(true, RelaSectionSeen);
+}
+
+TEST(ELFOutput, OptionalMembersInitMips) {
+  // Test that the optional members of ELFYAML types are initialized
+  // to their default values in the constructors.
+  ELFYAML::FileHeader FileHeader;
+  FileHeader.Class = ELF::ELFCLASS64;
+  FileHeader.Data = ELF::ELFDATA2LSB;
+  FileHeader.Type = ELF::ET_REL;
+  FileHeader.Machine = ELF::EM_MIPS;
+
+  ELFYAML::MipsABIFlags MipsSection;
+  MipsSection.Type = ELF::SHT_MIPS_ABIFLAGS;
+  MipsSection.Name = ".MIPS.abiflags";
+  MipsSection.ISALevel = 1; // MIPS1
+
+  ELFYAML::Object Object;
+  Object.Header = FileHeader;
+
+  Object.Chunks.push_back(
+      std::make_unique<ELFYAML::MipsABIFlags>(std::move(MipsSection)));
+
+  SmallString<0> YAMLStorage;
+  raw_svector_ostream YAMLOS(YAMLStorage);
+  {
+    yaml::Output YAMLOut(YAMLOS);
+    YAMLOut << Object;
+  }
+
+  SmallString<0> ELFStorage;
+  Expected<ELFObjectFile<ELF64LE>> ExpectedFile =
+      toBinary<ELF64LE>(ELFStorage, YAMLStorage);
+  ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+  const ELFObjectFile<ELF64LE> &File = *ExpectedFile;
+
+  bool MipsSectionSeen = false;
+  for (SectionRef SecRef : File.sections()) {
+    Expected<StringRef> ExpectedSName = SecRef.getName();
+    ASSERT_THAT_EXPECTED(ExpectedSName, Succeeded());
+    if (*ExpectedSName == ".MIPS.abiflags") {
+      MipsSectionSeen = true;
+      Expected<StringRef> ExpectedContents = SecRef.getContents();
+      ASSERT_THAT_EXPECTED(ExpectedContents, Succeeded());
+      EXPECT_EQ(false, ExpectedContents->empty());
+      // For some reason ELFEmitter writes these flags
+      // with the endianness of the compilation host.
+      const object::Elf_Mips_ABIFlags<ELF64LE> *Flags =
+          reinterpret_cast<const object::Elf_Mips_ABIFlags<ELF64LE> *>(
+              ExpectedContents->data());
+      EXPECT_EQ(0u, Flags->version);
+      EXPECT_EQ(1u, Flags->isa_level);
+      EXPECT_EQ(0u, Flags->isa_rev);
+      EXPECT_EQ(0u, Flags->gpr_size);
+      EXPECT_EQ(0u, Flags->cpr1_size);
+      EXPECT_EQ(0u, Flags->cpr2_size);
+      EXPECT_EQ(0u, Flags->fp_abi);
+      EXPECT_EQ(0u, Flags->isa_ext);
+      EXPECT_EQ(0u, Flags->ases);
+      EXPECT_EQ(0u, Flags->flags1);
+      EXPECT_EQ(0u, Flags->flags2);
+      (void)Flags;
+    }
+  }
+
+  EXPECT_EQ(true, MipsSectionSeen);
+}