[lib/ObjectYAML] - Improve and cleanup error reporting in ELFState<ELFT> class.

[grimar]

Summary

The aim of this patch is to refactor how we handle and report error.

I suggest to use the same approach we use in LLD: delayed error reporting.
For that I introduced 'HasError' flag which triggers when we report an error.
Now we do not exit instantly on any error. The benefits are:

1. There are no more 'exit(1)' calls in the library code.
2. Code was simplified significantly in a few places.
3. It is now possible to print multiple errors instead of only one.

Also, I changed the messages to be lower case and removed a full stop.

[jhenderson]

Thanks for working on this. I think you need a few more tests to show that we no longer exit(1) where we previously were (i.e. that we can report multiple error messages). I see you have one for relocation references, but there are several other places that need testing too.

lib/ObjectYAML/ELFEmitter.cpp

274 ↗

(On Diff #218714)

at -> by

285 ↗

(On Diff #218714)

at -> by

416 ↗

(On Diff #218714)

Maybe you could have this function return void, and then have the caller check HasErrors?

453 ↗

(On Diff #218714)

Adding a test case to show that we can report multiple errors for this issue would be good.

589 ↗

(On Diff #218714)

This might be a bit of a bigger task than you had hoped for, and I'm okay if you want to push this suggestion to a later change, but it would be great if this could call some form of error handler passed in by the client of the library instead of directly calling WithColor::error(). That way the client can control how the errors are reported. See my lightning talk on error handling in libraries from the US conference a couple of years ago.

Regardless, setting HasError here seems fine, and obviously a default option of WithColor::error() is sensible.

606 ↗

(On Diff #218714)

Maybe this could be a continue instead of a return. That way all unknown sections will be picked up.

718–719 ↗

(On Diff #218714)

Maybe it's worth having something like this comment in toSymbolIndex?

[grimar]

• Addressed review comments, added test cases.
• Added single quotes around symbol name in the error message: "unknown section referenced ... by YAML symbol 'X'". This matches another messages tool reports.

lib/ObjectYAML/ELFEmitter.cpp

416 ↗

(On Diff #218714)

Yes, this is probably better.

453 ↗

(On Diff #218714)

Done.

589 ↗

(On Diff #218714)

I'll be happy to investigate it deeper and suggest a follow-up patch to add a custom error handler to this lib.

718–719 ↗

(On Diff #218714)

I added a comment.

[jhenderson]

lib/ObjectYAML/ELFEmitter.cpp

367 ↗

(On Diff #218905)

I think we need a test case showing this can be called even after an earlier sh_link fails (e.g. two sections with bad sh_link values).

380 ↗

(On Diff #218905)

Arguably, the same goes for each of the changes here too - if the writeSectionContent can fail for a given section type, we should have a test showing that the loop continues.

710 ↗

(On Diff #218905)

It might be interesting to have a test case showing that a bad sh_info and an unknown symbol index are both reported for the same relocation section. Up to you.

768 ↗

(On Diff #218905)

Perhaps worth a test for a bad symbol and multiple bad section indexes in the same section.

945 ↗

(On Diff #218905)

Could this just be a continue and this function return void?

960 ↗

(On Diff #218905)

Could this be a continue and return void?

test/tools/yaml2obj/elf-comdat-broken-info.yaml

1 ↗

(On Diff #218905)

produce an SHT_GROUP

30 ↗

(On Diff #218905)

when multiple unknown symbols
by SHT_GROUP sections.

[grimar]

• Addressed review comments.

lib/ObjectYAML/ELFEmitter.cpp

367 ↗

(On Diff #218905)

Done in section-link.yaml.

380 ↗

(On Diff #218905)

I am not sure it worth testing so hard honestly.

There is nothing like

if (HasError)
  return;

around, so I think no reason to assume that loop can terminate.
It feels that testing a single writeSectionContent should be enough.

However, the current situation is following.
writeSectionContent can fail for:

1. writeSectionContent(...const ELFYAML::DynamicSection &Section...)

It is tested in: dynamic-section-raw-content.yaml

2. writeSectionContent(...const ELFYAML::RelocationSection &Section...)

I added a test to reloc-sec-info.yaml

3. writeSectionContent(...const ELFYAML::Group &Section...)

It is tested in: elf-comdat-broken-info.yaml

710 ↗

(On Diff #218905)

I added a test to reloc-sec-info.yaml

768 ↗

(On Diff #218905)

I added elf-comdat-broken-members.yaml.

945 ↗

(On Diff #218905)

Yes, done.

960 ↗

(On Diff #218905)

Yes, done.

[jhenderson]

LGTM, with comment fixes

test/tools/yaml2obj/elf-comdat-broken-members.yaml

1 ↗

(On Diff #219077)

refereces(typo) unknown -> references an unknown
members list -> member list

test/tools/yaml2obj/reloc-sec-info.yaml

27 ↗

(On Diff #219077)

when a relocation section
via its Info

test/tools/yaml2obj/section-link.yaml

27 ↗

(On Diff #219077)

when an unknown
via a Link

[MaskRay]

lib/ObjectYAML/ELFEmitter.cpp

117 ↗

(On Diff #219077)

Instead of HasError, have you thought using an integer (int errorCount = 0;) ?

[MaskRay]

lib/ObjectYAML/ELFEmitter.cpp

117 ↗

(On Diff #219077)

Sorry, int ErrorCount = 0;

Is there a case that many errors may be accumulated? If yes, we may need an ErrorLimit. We certainly don't need ErrorLimit in this patch, but using ErrorCount may prevent a future rename HasError -> ErrorCount..

[MaskRay]

test/tools/yaml2obj/duplicate-symbol-names.test

44 ↗

(On Diff #219077)

CASE3-COUNT-2:

test/tools/yaml2obj/dynamic-section-raw-content.yaml

29 ↗

(On Diff #219077)

Can -o %t2 be deleted?

test/tools/yaml2obj/dynsymtab-implicit-sections-size-content.yaml

32 ↗

(On Diff #219077)

Can -o %t2 be deleted?

56 ↗

(On Diff #219077)

Can %t3 be deleted?

[grimar]

lib/ObjectYAML/ELFEmitter.cpp

117 ↗

(On Diff #219077)

I'd do this when we need it. It is not clear how much it is useful, since it is not the same case as we have in linker: I do not think there can be many errors in a hand written YAML document. And probably no need to limit them.
obj2yaml is assumed to produce only valid YAMLs too.

test/tools/yaml2obj/dynamic-section-raw-content.yaml

29 ↗

(On Diff #219077)

Yes, you're right here and below. I'll update the diff in my next week or tomorrow.

[MaskRay]

LG once you remove the redundant -o %t2 in a few places.

lib/ObjectYAML/ELFEmitter.cpp

117 ↗

(On Diff #219077)

I was just checking whether we'll eventually do that. So using the final variable name can prevent a future rename. You make the call. I haven't put much thought on that, either.

llvm/trunk/lib/ObjectYAML/ELFEmitter.cpp

template <class ELFT> class ELFState {
   /// The future ".dynstr" section.
   StringTableBuilder DotDynstr{StringTableBuilder::ELF};
 
   NameToIdxMap SN2I;
   NameToIdxMap SymN2I;
   NameToIdxMap DynSymN2I;
   ELFYAML::Object &Doc;
 
-  bool buildSectionIndex();
-  bool buildSymbolIndexes();
+  bool HasError = false;
+
+  std::vector<Elf_Sym> toELFSymbols(ArrayRef<ELFYAML::Symbol> Symbols,
+                                    const StringTableBuilder &Strtab);
+  unsigned toSectionIndex(StringRef S, StringRef LocSec, StringRef LocSym = "");
+  unsigned toSymbolIndex(StringRef S, StringRef LocSec, bool IsDynamic);
+  void reportError(const Twine &Msg);
+
+  void buildSectionIndex();
+  void buildSymbolIndexes();
   void initProgramHeaders(std::vector<Elf_Phdr> &PHeaders);
   bool initImplicitHeader(ContiguousBlobAccumulator &CBA, Elf_Shdr &Header,
                           StringRef SecName, ELFYAML::Section *YAMLSec);
-  bool initSectionHeaders(std::vector<Elf_Shdr> &SHeaders,
+  void initSectionHeaders(std::vector<Elf_Shdr> &SHeaders,
                           ContiguousBlobAccumulator &CBA);
   void initSymtabSectionHeader(Elf_Shdr &SHeader, SymtabType STType,
                                ContiguousBlobAccumulator &CBA,
                                ELFYAML::Section *YAMLSec);
   void initStrtabSectionHeader(Elf_Shdr &SHeader, StringRef Name,
                                StringTableBuilder &STB,
                                ContiguousBlobAccumulator &CBA,
                                ELFYAML::Section *YAMLSec);
   void setProgramHeaderLayout(std::vector<Elf_Phdr> &PHeaders,
                               std::vector<Elf_Shdr> &SHeaders);
   void finalizeStrings();
   void writeELFHeader(ContiguousBlobAccumulator &CBA, raw_ostream &OS);
-  bool writeSectionContent(Elf_Shdr &SHeader,
+  void writeSectionContent(Elf_Shdr &SHeader,
                            const ELFYAML::RawContentSection &Section,
                            ContiguousBlobAccumulator &CBA);
-  bool writeSectionContent(Elf_Shdr &SHeader,
+  void writeSectionContent(Elf_Shdr &SHeader,
                            const ELFYAML::RelocationSection &Section,
                            ContiguousBlobAccumulator &CBA);
-  bool writeSectionContent(Elf_Shdr &SHeader, const ELFYAML::Group &Group,
+  void writeSectionContent(Elf_Shdr &SHeader, const ELFYAML::Group &Group,
                            ContiguousBlobAccumulator &CBA);
-  bool writeSectionContent(Elf_Shdr &SHeader,
+  void writeSectionContent(Elf_Shdr &SHeader,
                            const ELFYAML::SymtabShndxSection &Shndx,
                            ContiguousBlobAccumulator &CBA);
-  bool writeSectionContent(Elf_Shdr &SHeader,
+  void writeSectionContent(Elf_Shdr &SHeader,
                            const ELFYAML::SymverSection &Section,
                            ContiguousBlobAccumulator &CBA);
-  bool writeSectionContent(Elf_Shdr &SHeader,
+  void writeSectionContent(Elf_Shdr &SHeader,
                            const ELFYAML::VerneedSection &Section,
                            ContiguousBlobAccumulator &CBA);
-  bool writeSectionContent(Elf_Shdr &SHeader,
+  void writeSectionContent(Elf_Shdr &SHeader,
                            const ELFYAML::VerdefSection &Section,
                            ContiguousBlobAccumulator &CBA);
-  bool writeSectionContent(Elf_Shdr &SHeader,
+  void writeSectionContent(Elf_Shdr &SHeader,
                            const ELFYAML::MipsABIFlags &Section,
                            ContiguousBlobAccumulator &CBA);
-  bool writeSectionContent(Elf_Shdr &SHeader,
+  void writeSectionContent(Elf_Shdr &SHeader,
                            const ELFYAML::DynamicSection &Section,
                            ContiguousBlobAccumulator &CBA);
   ELFState(ELFYAML::Object &D);
-
 public:
   static int writeELF(raw_ostream &OS, ELFYAML::Object &Doc);
 };
 } // end anonymous namespace
 
 template <class T> static size_t arrayDataSize(ArrayRef<T> A) {
   return A.size() * sizeof(T);
 }
for (const auto &YamlPhdr : Doc.ProgramHeaders) {
     Phdr.p_type = YamlPhdr.Type;
     Phdr.p_flags = YamlPhdr.Flags;
     Phdr.p_vaddr = YamlPhdr.VAddr;
     Phdr.p_paddr = YamlPhdr.PAddr;
     PHeaders.push_back(Phdr);
   }
 }
 
-static bool convertSectionIndex(NameToIdxMap &SN2I, StringRef SecName,
-                                StringRef IndexSrc, unsigned &IndexDest) {
-  if (!SN2I.lookup(IndexSrc, IndexDest) && !to_integer(IndexSrc, IndexDest)) {
-    WithColor::error() << "Unknown section referenced: '" << IndexSrc
-                       << "' at YAML section '" << SecName << "'.\n";
-    return false;
-  }
-  return true;
+template <class ELFT>
+unsigned ELFState<ELFT>::toSectionIndex(StringRef S, StringRef LocSec,
+                                        StringRef LocSym) {
+  unsigned Index;
+  if (SN2I.lookup(S, Index) || to_integer(S, Index))
+    return Index;
+
+  assert(LocSec.empty() || LocSym.empty());
+  if (!LocSym.empty())
+    reportError("unknown section referenced: '" + S + "' by YAML symbol '" +
+                LocSym + "'");
+  else
+    reportError("unknown section referenced: '" + S + "' by YAML section '" +
+                LocSec + "'");
+  return 0;
+
+template <class ELFT>
+unsigned ELFState<ELFT>::toSymbolIndex(StringRef S, StringRef LocSec,
+                                       bool IsDynamic) {
+  const NameToIdxMap &SymMap = IsDynamic ? DynSymN2I : SymN2I;
+  unsigned Index;
+  // Here we try to look up S in the symbol table. If it is not there,
+  // treat its value as a symbol index.
+  if (!SymMap.lookup(S, Index) && !to_integer(S, Index)) {
+    reportError("unknown symbol referenced: '" + S + "' by YAML section '" +
+                LocSec + "'");
+    return 0;
+  }
+  return Index;
 }
 
 template <class ELFT>
 bool ELFState<ELFT>::initImplicitHeader(ContiguousBlobAccumulator &CBA,
                                         Elf_Shdr &Header, StringRef SecName,
                                         ELFYAML::Section *YAMLSec) {
   // Check if the header was already initialized.
   if (Header.sh_offset)

 static StringRef dropUniqueSuffix(StringRef S) {
   size_t SuffixPos = S.rfind(" [");
   if (SuffixPos == StringRef::npos)
     return S;
   return S.substr(0, SuffixPos);
 }
 
 template <class ELFT>
-bool ELFState<ELFT>::initSectionHeaders(std::vector<Elf_Shdr> &SHeaders,
+void ELFState<ELFT>::initSectionHeaders(std::vector<Elf_Shdr> &SHeaders,
                                         ContiguousBlobAccumulator &CBA) {
   // Ensure SHN_UNDEF entry is present. An all-zero section header is a
   // valid SHN_UNDEF entry since SHT_NULL == 0.
   SHeaders.resize(Doc.Sections.size());
 
   for (size_t I = 0; I < Doc.Sections.size(); ++I) {
     ELFYAML::Section *Sec = Doc.Sections[I].get();
     if (I == 0 && Sec->IsImplicit)
for (size_t I = 0; I < Doc.Sections.size(); ++I) {
 
     SHeader.sh_name = DotShStrtab.getOffset(dropUniqueSuffix(Sec->Name));
     SHeader.sh_type = Sec->Type;
     if (Sec->Flags)
       SHeader.sh_flags = *Sec->Flags;
     SHeader.sh_addr = Sec->Address;
     SHeader.sh_addralign = Sec->AddressAlign;
 
-    if (!Sec->Link.empty()) {
-      unsigned Index;
-      if (!convertSectionIndex(SN2I, Sec->Name, Sec->Link, Index))
-        return false;
-      SHeader.sh_link = Index;
-    }
+    if (!Sec->Link.empty())
+      SHeader.sh_link = toSectionIndex(Sec->Link, Sec->Name);
 
     if (I == 0) {
       if (auto RawSec = dyn_cast<ELFYAML::RawContentSection>(Sec)) {
         // We do not write any content for special SHN_UNDEF section.
         if (RawSec->Size)
           SHeader.sh_size = *RawSec->Size;
         if (RawSec->Info)
           SHeader.sh_info = *RawSec->Info;
       }
       if (Sec->EntSize)
         SHeader.sh_entsize = *Sec->EntSize;
     } else if (auto S = dyn_cast<ELFYAML::RawContentSection>(Sec)) {
-      if (!writeSectionContent(SHeader, *S, CBA))
-        return false;
+      writeSectionContent(SHeader, *S, CBA);
     } else if (auto S = dyn_cast<ELFYAML::SymtabShndxSection>(Sec)) {
-      if (!writeSectionContent(SHeader, *S, CBA))
-        return false;
+      writeSectionContent(SHeader, *S, CBA);
     } else if (auto S = dyn_cast<ELFYAML::RelocationSection>(Sec)) {
-      if (!writeSectionContent(SHeader, *S, CBA))
-        return false;
+      writeSectionContent(SHeader, *S, CBA);
     } else if (auto S = dyn_cast<ELFYAML::Group>(Sec)) {
-      if (!writeSectionContent(SHeader, *S, CBA))
-        return false;
+      writeSectionContent(SHeader, *S, CBA);
     } else if (auto S = dyn_cast<ELFYAML::MipsABIFlags>(Sec)) {
-      if (!writeSectionContent(SHeader, *S, CBA))
-        return false;
+      writeSectionContent(SHeader, *S, CBA);
     } else if (auto S = dyn_cast<ELFYAML::NoBitsSection>(Sec)) {
       SHeader.sh_entsize = 0;
       SHeader.sh_size = S->Size;
       // SHT_NOBITS section does not have content
       // so just to setup the section offset.
       CBA.getOSAndAlignedOffset(SHeader.sh_offset, SHeader.sh_addralign);
     } else if (auto S = dyn_cast<ELFYAML::DynamicSection>(Sec)) {
-      if (!writeSectionContent(SHeader, *S, CBA))
-        return false;
+      writeSectionContent(SHeader, *S, CBA);
     } else if (auto S = dyn_cast<ELFYAML::SymverSection>(Sec)) {
-      if (!writeSectionContent(SHeader, *S, CBA))
-        return false;
+      writeSectionContent(SHeader, *S, CBA);
     } else if (auto S = dyn_cast<ELFYAML::VerneedSection>(Sec)) {
-      if (!writeSectionContent(SHeader, *S, CBA))
-        return false;
+      writeSectionContent(SHeader, *S, CBA);
     } else if (auto S = dyn_cast<ELFYAML::VerdefSection>(Sec)) {
-      if (!writeSectionContent(SHeader, *S, CBA))
-        return false;
-    } else
+      writeSectionContent(SHeader, *S, CBA);
+    } else {
       llvm_unreachable("Unknown section type");
+    }
 
     // Override the fields if requested.
     if (Sec) {
       if (Sec->ShName)
         SHeader.sh_name = *Sec->ShName;
       if (Sec->ShOffset)
         SHeader.sh_offset = *Sec->ShOffset;
       if (Sec->ShSize)
         SHeader.sh_size = *Sec->ShSize;
     }
   }
-
-  return true;
 }
 
 static size_t findFirstNonGlobal(ArrayRef<ELFYAML::Symbol> Symbols) {
   for (size_t I = 0; I < Symbols.size(); ++I)
     if (Symbols[I].Binding.value != ELF::STB_LOCAL)
       return I;
   return Symbols.size();
 }
static uint64_t writeRawSectionData(raw_ostream &OS,
   if (!RawSec.Size)
     return ContentSize;
 
   OS.write_zeros(*RawSec.Size - ContentSize);
   return *RawSec.Size;
 }
 
 template <class ELFT>
-static std::vector<typename ELFT::Sym>
-toELFSymbols(NameToIdxMap &SN2I, ArrayRef<ELFYAML::Symbol> Symbols,
+std::vector<typename ELFT::Sym>
+ELFState<ELFT>::toELFSymbols(ArrayRef<ELFYAML::Symbol> Symbols,
                              const StringTableBuilder &Strtab) {
-  using Elf_Sym = typename ELFT::Sym;
-
   std::vector<Elf_Sym> Ret;
   Ret.resize(Symbols.size() + 1);
 
   size_t I = 0;
   for (const auto &Sym : Symbols) {
     Elf_Sym &Symbol = Ret[++I];
 
     // If NameIndex, which contains the name offset, is explicitly specified, we
     // use it. This is useful for preparing broken objects. Otherwise, we add
     // the specified Name to the string table builder to get its offset.
     if (Sym.NameIndex)
       Symbol.st_name = *Sym.NameIndex;
     else if (!Sym.Name.empty())
       Symbol.st_name = Strtab.getOffset(dropUniqueSuffix(Sym.Name));
 
     Symbol.setBindingAndType(Sym.Binding, Sym.Type);
-    if (!Sym.Section.empty()) {
-      unsigned Index;
-      if (!SN2I.lookup(Sym.Section, Index)) {
-        WithColor::error() << "Unknown section referenced: '" << Sym.Section
-                           << "' by YAML symbol " << Sym.Name << ".\n";
-        exit(1);
-      }
-      Symbol.st_shndx = Index;
-    } else if (Sym.Index) {
+    if (!Sym.Section.empty())
+      Symbol.st_shndx = toSectionIndex(Sym.Section, "", Sym.Name);
+    else if (Sym.Index)
       Symbol.st_shndx = *Sym.Index;
-    }
-    // else Symbol.st_shndex == SHN_UNDEF (== 0), since it was zero'd earlier.
+
     Symbol.st_value = Sym.Value;
     Symbol.st_other = Sym.Other ? *Sym.Other : 0;
     Symbol.st_size = Sym.Size;
   }
 
   return Ret;
 }
 
 template <class ELFT>
 void ELFState<ELFT>::initSymtabSectionHeader(Elf_Shdr &SHeader,
                                              SymtabType STType,
                                              ContiguousBlobAccumulator &CBA,
                                              ELFYAML::Section *YAMLSec) {
 
   bool IsStatic = STType == SymtabType::Static;
   const auto &Symbols = IsStatic ? Doc.Symbols : Doc.DynamicSymbols;
 
   ELFYAML::RawContentSection *RawSec =
       dyn_cast_or_null<ELFYAML::RawContentSection>(YAMLSec);
   if (RawSec && !Symbols.empty() && (RawSec->Content || RawSec->Size)) {
     if (RawSec->Content)
-      WithColor::error() << "Cannot specify both `Content` and " +
-                                (IsStatic ? Twine("`Symbols`")
-                                          : Twine("`DynamicSymbols`")) +
-                                " for symbol table section '"
-                         << RawSec->Name << "'.\n";
+      reportError("cannot specify both `Content` and " +
+                  (IsStatic ? Twine("`Symbols`") : Twine("`DynamicSymbols`")) +
+                  " for symbol table section '" + RawSec->Name + "'");
     if (RawSec->Size)
-      WithColor::error() << "Cannot specify both `Size` and " +
-                                (IsStatic ? Twine("`Symbols`")
-                                          : Twine("`DynamicSymbols`")) +
-                                " for symbol table section '"
-                         << RawSec->Name << "'.\n";
-    exit(1);
+      reportError("cannot specify both `Size` and " +
+                  (IsStatic ? Twine("`Symbols`") : Twine("`DynamicSymbols`")) +
+                  " for symbol table section '" + RawSec->Name + "'");
+    return;
   }
 
   zero(SHeader);
   SHeader.sh_name = DotShStrtab.getOffset(IsStatic ? ".symtab" : ".dynsym");
 
   if (YAMLSec)
     SHeader.sh_type = YAMLSec->Type;
   else
     SHeader.sh_type = IsStatic ? ELF::SHT_SYMTAB : ELF::SHT_DYNSYM;
 
   if (RawSec && !RawSec->Link.empty()) {
     // If the Link field is explicitly defined in the document,
     // we should use it.
-    unsigned Index;
-    if (!convertSectionIndex(SN2I, RawSec->Name, RawSec->Link, Index))
-      return;
-    SHeader.sh_link = Index;
+    SHeader.sh_link = toSectionIndex(RawSec->Link, RawSec->Name);
   } else {
     // When we describe the .dynsym section in the document explicitly, it is
     // allowed to omit the "DynamicSymbols" tag. In this case .dynstr is not
     // added implicitly and we should be able to leave the Link zeroed if
     // .dynstr is not defined.
     unsigned Link = 0;
     if (IsStatic)
       Link = SN2I.get(".strtab");
void ELFState<ELFT>::initSymtabSectionHeader(Elf_Shdr &SHeader,
   auto &OS = CBA.getOSAndAlignedOffset(SHeader.sh_offset, SHeader.sh_addralign);
   if (RawSec && (RawSec->Content || RawSec->Size)) {
     assert(Symbols.empty());
     SHeader.sh_size = writeRawSectionData(OS, *RawSec);
     return;
   }
 
   std::vector<Elf_Sym> Syms =
-      toELFSymbols<ELFT>(SN2I, Symbols, IsStatic ? DotStrtab : DotDynstr);
+      toELFSymbols(Symbols, IsStatic ? DotStrtab : DotDynstr);
   writeArrayData(OS, makeArrayRef(Syms));
   SHeader.sh_size = arrayDataSize(makeArrayRef(Syms));
 }
 
 template <class ELFT>
 void ELFState<ELFT>::initStrtabSectionHeader(Elf_Shdr &SHeader, StringRef Name,
                                              StringTableBuilder &STB,
                                              ContiguousBlobAccumulator &CBA,
else if (Name == ".dynstr")
     SHeader.sh_flags = ELF::SHF_ALLOC;
 
   // If the section is explicitly described in the YAML
   // then we want to use its section address.
   if (YAMLSec)
     SHeader.sh_addr = YAMLSec->Address;
 }
 
+template <class ELFT> void ELFState<ELFT>::reportError(const Twine &Msg) {
+  WithColor::error() << Msg << "\n";
+  HasError = true;
+}
+
 template <class ELFT>
 void ELFState<ELFT>::setProgramHeaderLayout(std::vector<Elf_Phdr> &PHeaders,
                                             std::vector<Elf_Shdr> &SHeaders) {
   uint32_t PhdrIdx = 0;
   for (auto &YamlPhdr : Doc.ProgramHeaders) {
     Elf_Phdr &PHeader = PHeaders[PhdrIdx++];
 
     std::vector<Elf_Shdr *> Sections;
     for (const ELFYAML::SectionName &SecName : YamlPhdr.Sections) {
       unsigned Index;
       if (!SN2I.lookup(SecName.Section, Index)) {
-        WithColor::error() << "Unknown section referenced: '" << SecName.Section
-                           << "' by program header.\n";
-        exit(1);
+        reportError("unknown section referenced: '" + SecName.Section +
+                    "' by program header");
+        continue;
       }
       Sections.push_back(&SHeaders[Index]);
     }
 
     if (YamlPhdr.Offset) {
       PHeader.p_offset = *YamlPhdr.Offset;
     } else {
       if (YamlPhdr.Sections.size())
if (YamlPhdr.Align) {
       for (Elf_Shdr *SHeader : Sections)
         if (SHeader->sh_offset == PHeader.p_offset)
           PHeader.p_align = std::max(PHeader.p_align, SHeader->sh_addralign);
     }
   }
 }
 
 template <class ELFT>
-bool ELFState<ELFT>::writeSectionContent(
+void ELFState<ELFT>::writeSectionContent(
     Elf_Shdr &SHeader, const ELFYAML::RawContentSection &Section,
     ContiguousBlobAccumulator &CBA) {
   raw_ostream &OS =
       CBA.getOSAndAlignedOffset(SHeader.sh_offset, SHeader.sh_addralign);
   SHeader.sh_size = writeRawSectionData(OS, Section);
 
   if (Section.EntSize)
     SHeader.sh_entsize = *Section.EntSize;
   else if (Section.Type == llvm::ELF::SHT_RELR)
     SHeader.sh_entsize = sizeof(Elf_Relr);
   else
     SHeader.sh_entsize = 0;
 
   if (Section.Info)
     SHeader.sh_info = *Section.Info;
-
-  return true;
 }
 
 static bool isMips64EL(const ELFYAML::Object &Doc) {
   return Doc.Header.Machine == ELFYAML::ELF_EM(llvm::ELF::EM_MIPS) &&
          Doc.Header.Class == ELFYAML::ELF_ELFCLASS(ELF::ELFCLASS64) &&
          Doc.Header.Data == ELFYAML::ELF_ELFDATA(ELF::ELFDATA2LSB);
 }
 
 template <class ELFT>
-bool ELFState<ELFT>::writeSectionContent(
+void ELFState<ELFT>::writeSectionContent(
     Elf_Shdr &SHeader, const ELFYAML::RelocationSection &Section,
     ContiguousBlobAccumulator &CBA) {
   assert((Section.Type == llvm::ELF::SHT_REL ||
           Section.Type == llvm::ELF::SHT_RELA) &&
          "Section type is not SHT_REL nor SHT_RELA");
 
   bool IsRela = Section.Type == llvm::ELF::SHT_RELA;
   SHeader.sh_entsize = IsRela ? sizeof(Elf_Rela) : sizeof(Elf_Rel);
   SHeader.sh_size = SHeader.sh_entsize * Section.Relocations.size();
 
   // For relocation section set link to .symtab by default.
   if (Section.Link.empty())
     SHeader.sh_link = SN2I.get(".symtab");
 
-  unsigned Index = 0;
-  if (!Section.RelocatableSec.empty() &&
-      !convertSectionIndex(SN2I, Section.Name, Section.RelocatableSec, Index))
-    return false;
-  SHeader.sh_info = Index;
+  if (!Section.RelocatableSec.empty())
+    SHeader.sh_info = toSectionIndex(Section.RelocatableSec, Section.Name);
 
   auto &OS = CBA.getOSAndAlignedOffset(SHeader.sh_offset, SHeader.sh_addralign);
-
-  const NameToIdxMap &SymMap = Section.Link == ".dynsym" ? DynSymN2I : SymN2I;
   for (const auto &Rel : Section.Relocations) {
-    unsigned SymIdx = 0;
-    // If a relocation references a symbol, try to look one up in the symbol
-    // table. If it is not there, treat the value as a symbol index.
-    if (Rel.Symbol && !SymMap.lookup(*Rel.Symbol, SymIdx) &&
-        !to_integer(*Rel.Symbol, SymIdx)) {
-      WithColor::error() << "Unknown symbol referenced: '" << *Rel.Symbol
-                         << "' at YAML section '" << Section.Name << "'.\n";
-      return false;
-    }
-
+    unsigned SymIdx = Rel.Symbol ? toSymbolIndex(*Rel.Symbol, Section.Name,
+                                                 Section.Link == ".dynsym")
+                                 : 0;
     if (IsRela) {
       Elf_Rela REntry;
       zero(REntry);
       REntry.r_offset = Rel.Offset;
       REntry.r_addend = Rel.Addend;
       REntry.setSymbolAndType(SymIdx, Rel.Type, isMips64EL(Doc));
       OS.write((const char *)&REntry, sizeof(REntry));
     } else {
       Elf_Rel REntry;
       zero(REntry);
       REntry.r_offset = Rel.Offset;
       REntry.setSymbolAndType(SymIdx, Rel.Type, isMips64EL(Doc));
       OS.write((const char *)&REntry, sizeof(REntry));
     }
   }
-  return true;
 }
 
 template <class ELFT>
-bool ELFState<ELFT>::writeSectionContent(
+void ELFState<ELFT>::writeSectionContent(
     Elf_Shdr &SHeader, const ELFYAML::SymtabShndxSection &Shndx,
     ContiguousBlobAccumulator &CBA) {
   raw_ostream &OS =
       CBA.getOSAndAlignedOffset(SHeader.sh_offset, SHeader.sh_addralign);
 
   for (uint32_t E : Shndx.Entries)
     support::endian::write<uint32_t>(OS, E, ELFT::TargetEndianness);
 
   SHeader.sh_entsize = Shndx.EntSize ? (uint64_t)*Shndx.EntSize : 4;
   SHeader.sh_size = Shndx.Entries.size() * SHeader.sh_entsize;
-  return true;
 }
 
 template <class ELFT>
-bool ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
+void ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
                                          const ELFYAML::Group &Section,
                                          ContiguousBlobAccumulator &CBA) {
   assert(Section.Type == llvm::ELF::SHT_GROUP &&
          "Section type is not SHT_GROUP");
 
   SHeader.sh_entsize = 4;
   SHeader.sh_size = SHeader.sh_entsize * Section.Members.size();
-
-  unsigned SymIdx;
-  if (!SymN2I.lookup(Section.Signature, SymIdx) &&
-      !to_integer(Section.Signature, SymIdx)) {
-    WithColor::error() << "Unknown symbol referenced: '" << Section.Signature
-                       << "' at YAML section '" << Section.Name << "'.\n";
-    return false;
-  }
-  SHeader.sh_info = SymIdx;
+  SHeader.sh_info =
+      toSymbolIndex(Section.Signature, Section.Name, /*IsDynamic=*/false);
 
   raw_ostream &OS =
       CBA.getOSAndAlignedOffset(SHeader.sh_offset, SHeader.sh_addralign);
 
   for (const ELFYAML::SectionOrType &Member : Section.Members) {
     unsigned int SectionIndex = 0;
     if (Member.sectionNameOrType == "GRP_COMDAT")
       SectionIndex = llvm::ELF::GRP_COMDAT;
-    else if (!convertSectionIndex(SN2I, Section.Name, Member.sectionNameOrType,
-                                  SectionIndex))
-      return false;
+    else
+      SectionIndex = toSectionIndex(Member.sectionNameOrType, Section.Name);
     support::endian::write<uint32_t>(OS, SectionIndex, ELFT::TargetEndianness);
   }
-  return true;
 }
 
 template <class ELFT>
-bool ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
+void ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
                                          const ELFYAML::SymverSection &Section,
                                          ContiguousBlobAccumulator &CBA) {
   raw_ostream &OS =
       CBA.getOSAndAlignedOffset(SHeader.sh_offset, SHeader.sh_addralign);
   for (uint16_t Version : Section.Entries)
     support::endian::write<uint16_t>(OS, Version, ELFT::TargetEndianness);
 
   SHeader.sh_entsize = Section.EntSize ? (uint64_t)*Section.EntSize : 2;
   SHeader.sh_size = Section.Entries.size() * SHeader.sh_entsize;
-  return true;
 }
 
 template <class ELFT>
-bool ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
+void ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
                                          const ELFYAML::VerdefSection &Section,
                                          ContiguousBlobAccumulator &CBA) {
   typedef typename ELFT::Verdef Elf_Verdef;
   typedef typename ELFT::Verdaux Elf_Verdaux;
   raw_ostream &OS =
       CBA.getOSAndAlignedOffset(SHeader.sh_offset, SHeader.sh_addralign);
 
   uint64_t AuxCnt = 0;
for (size_t J = 0; J < E.VerNames.size(); ++J, ++AuxCnt) {
         VernAux.vda_next = sizeof(Elf_Verdaux);
       OS.write((const char *)&VernAux, sizeof(Elf_Verdaux));
     }
   }
 
   SHeader.sh_size = Section.Entries.size() * sizeof(Elf_Verdef) +
                     AuxCnt * sizeof(Elf_Verdaux);
   SHeader.sh_info = Section.Info;
-
-  return true;
 }
 
 template <class ELFT>
-bool ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
+void ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
                                          const ELFYAML::VerneedSection &Section,
                                          ContiguousBlobAccumulator &CBA) {
   typedef typename ELFT::Verneed Elf_Verneed;
   typedef typename ELFT::Vernaux Elf_Vernaux;
 
   auto &OS = CBA.getOSAndAlignedOffset(SHeader.sh_offset, SHeader.sh_addralign);
 
   uint64_t AuxCnt = 0;
for (size_t J = 0; J < VE.AuxV.size(); ++J, ++AuxCnt) {
         VernAux.vna_next = sizeof(Elf_Vernaux);
       OS.write((const char *)&VernAux, sizeof(Elf_Vernaux));
     }
   }
 
   SHeader.sh_size = Section.VerneedV.size() * sizeof(Elf_Verneed) +
                     AuxCnt * sizeof(Elf_Vernaux);
   SHeader.sh_info = Section.Info;
-
-  return true;
 }
 
 template <class ELFT>
-bool ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
+void ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
                                          const ELFYAML::MipsABIFlags &Section,
                                          ContiguousBlobAccumulator &CBA) {
   assert(Section.Type == llvm::ELF::SHT_MIPS_ABIFLAGS &&
          "Section type is not SHT_MIPS_ABIFLAGS");
 
   object::Elf_Mips_ABIFlags<ELFT> Flags;
   zero(Flags);
   SHeader.sh_entsize = sizeof(Flags);
   SHeader.sh_size = SHeader.sh_entsize;
 
   auto &OS = CBA.getOSAndAlignedOffset(SHeader.sh_offset, SHeader.sh_addralign);
   Flags.version = Section.Version;
   Flags.isa_level = Section.ISALevel;
   Flags.isa_rev = Section.ISARevision;
   Flags.gpr_size = Section.GPRSize;
   Flags.cpr1_size = Section.CPR1Size;
   Flags.cpr2_size = Section.CPR2Size;
   Flags.fp_abi = Section.FpABI;
   Flags.isa_ext = Section.ISAExtension;
   Flags.ases = Section.ASEs;
   Flags.flags1 = Section.Flags1;
   Flags.flags2 = Section.Flags2;
   OS.write((const char *)&Flags, sizeof(Flags));
-
-  return true;
 }
 
 template <class ELFT>
-bool ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
+void ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
                                          const ELFYAML::DynamicSection &Section,
                                          ContiguousBlobAccumulator &CBA) {
   typedef typename ELFT::uint uintX_t;
 
   assert(Section.Type == llvm::ELF::SHT_DYNAMIC &&
          "Section type is not SHT_DYNAMIC");
 
-  if (!Section.Entries.empty() && Section.Content) {
-    WithColor::error()
-        << "Cannot specify both raw content and explicit entries "
-           "for dynamic section '"
-        << Section.Name << "'.\n";
-    return false;
-  }
+  if (!Section.Entries.empty() && Section.Content)
+    reportError("cannot specify both raw content and explicit entries "
+                "for dynamic section '" +
+                Section.Name + "'");
 
   if (Section.Content)
     SHeader.sh_size = Section.Content->binary_size();
   else
     SHeader.sh_size = 2 * sizeof(uintX_t) * Section.Entries.size();
   if (Section.EntSize)
     SHeader.sh_entsize = *Section.EntSize;
   else
     SHeader.sh_entsize = sizeof(Elf_Dyn);
 
   raw_ostream &OS =
       CBA.getOSAndAlignedOffset(SHeader.sh_offset, SHeader.sh_addralign);
   for (const ELFYAML::DynamicEntry &DE : Section.Entries) {
     support::endian::write<uintX_t>(OS, DE.Tag, ELFT::TargetEndianness);
     support::endian::write<uintX_t>(OS, DE.Val, ELFT::TargetEndianness);
   }
   if (Section.Content)
     Section.Content->writeAsBinary(OS);
-
-  return true;
 }
 
-template <class ELFT> bool ELFState<ELFT>::buildSectionIndex() {
+template <class ELFT> void ELFState<ELFT>::buildSectionIndex() {
   for (unsigned I = 0, E = Doc.Sections.size(); I != E; ++I) {
     StringRef Name = Doc.Sections[I]->Name;
     if (Name.empty())
       continue;
 
     DotShStrtab.add(dropUniqueSuffix(Name));
-    if (!SN2I.addName(Name, I)) {
-      WithColor::error() << "Repeated section name: '" << Name
-                         << "' at YAML section number " << I << ".\n";
-      return false;
-    }
+    if (!SN2I.addName(Name, I))
+      reportError("repeated section name: '" + Name +
+                  "' at YAML section number " + Twine(I));
   }
 
   DotShStrtab.finalize();
-  return true;
 }
 
-static bool buildSymbolsMap(ArrayRef<ELFYAML::Symbol> V, NameToIdxMap &Map) {
+template <class ELFT> void ELFState<ELFT>::buildSymbolIndexes() {
+  auto Build = [this](ArrayRef<ELFYAML::Symbol> V, NameToIdxMap &Map) {
     for (size_t I = 0, S = V.size(); I < S; ++I) {
       const ELFYAML::Symbol &Sym = V[I];
-    if (Sym.Name.empty() || Map.addName(Sym.Name, I + 1))
-      continue;
-    WithColor::error() << "Repeated symbol name: '" << Sym.Name << "'.\n";
-    return false;
-  }
-  return true;
+      if (!Sym.Name.empty() && !Map.addName(Sym.Name, I + 1))
+        reportError("repeated symbol name: '" + Sym.Name + "'");
     }
+  };
 
-template <class ELFT> bool ELFState<ELFT>::buildSymbolIndexes() {
-  return buildSymbolsMap(Doc.Symbols, SymN2I) &&
-         buildSymbolsMap(Doc.DynamicSymbols, DynSymN2I);
+  Build(Doc.Symbols, SymN2I);
+  Build(Doc.DynamicSymbols, DynSymN2I);
 }
 
 template <class ELFT> void ELFState<ELFT>::finalizeStrings() {
   // Add the regular symbol names to .strtab section.
   for (const ELFYAML::Symbol &Sym : Doc.Symbols)
     DotStrtab.add(dropUniqueSuffix(Sym.Name));
   DotStrtab.finalize();
 

 int ELFState<ELFT>::writeELF(raw_ostream &OS, ELFYAML::Object &Doc) {
   ELFState<ELFT> State(Doc);
 
   // Finalize .strtab and .dynstr sections. We do that early because want to
   // finalize the string table builders before writing the content of the
   // sections that might want to use them.
   State.finalizeStrings();
 
-  if (!State.buildSectionIndex() || !State.buildSymbolIndexes())
-    return 1;
+  State.buildSectionIndex();
+  State.buildSymbolIndexes();
 
   std::vector<Elf_Phdr> PHeaders;
   State.initProgramHeaders(PHeaders);
 
   // XXX: This offset is tightly coupled with the order that we write
   // things to `OS`.
   const size_t SectionContentBeginOffset =
       sizeof(Elf_Ehdr) + sizeof(Elf_Phdr) * Doc.ProgramHeaders.size();
   ContiguousBlobAccumulator CBA(SectionContentBeginOffset);
 
   std::vector<Elf_Shdr> SHeaders;
-  if (!State.initSectionHeaders(SHeaders, CBA))
-    return 1;
+  State.initSectionHeaders(SHeaders, CBA);
 
   // Now we can decide segment offsets
   State.setProgramHeaderLayout(PHeaders, SHeaders);
 
+  if (State.HasError)
+    return 1;
+
   State.writeELFHeader(CBA, OS);
   writeArrayData(OS, makeArrayRef(PHeaders));
   CBA.writeBlobToStream(OS);
   writeArrayData(OS, makeArrayRef(SHeaders));
   return 0;
 }
 
 namespace llvm {

llvm/trunk/test/tools/yaml2obj/duplicate-section-names.test

- Name: '.foo [1]'
     Type: SHT_PROGBITS
   - Name: .foo2
     Type: SHT_PROGBITS
 
 ## Check that yaml2obj reports an error in case we have
 ## sections with equal names and suffixes.
 
 # RUN: not yaml2obj --docnum=2 %s 2>&1 | FileCheck %s --check-prefix=CASE2
-# CASE2: error: Repeated section name: '.foo [1]' at YAML section number 2.
+# CASE2: error: repeated section name: '.foo [1]' at YAML section number 2
+# CASE2: error: repeated section name: '.foo [1]' at YAML section number 3
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_REL
   Machine: EM_X86_64
 Sections:
   - Name: '.foo [1]'
     Type: SHT_PROGBITS
   - Name: '.foo [1]'
     Type: SHT_PROGBITS
+  - Name: '.foo [1]'
+    Type: SHT_PROGBITS
 
 ## Check that yaml2obj reports an error in case we have
 ## symbols without suffixes in the names and their
 ## names are equal.
 
 # RUN: not yaml2obj --docnum=3 %s 2>&1 | FileCheck %s --check-prefix=CASE3
-# CASE3: error: Repeated section name: '.foo' at YAML section number 2.
+# CASE3: error: repeated section name: '.foo' at YAML section number 2
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_REL
   Machine: EM_X86_64
 Sections:

llvm/trunk/test/tools/yaml2obj/duplicate-symbol-names.test

 Symbols:
   - Name: localfoo
   - Name: 'localfoo [1]'
 
 ## Check that yaml2obj reports an error when we have
 ## symbols with equal names and suffixes.
 
 # RUN: not yaml2obj --docnum=2 %s 2>&1| FileCheck %s --check-prefix=CASE2
-# CASE2: error: Repeated symbol name: 'localfoo [1]'.
+# CASE2-COUNT-2: error: repeated symbol name: 'localfoo [1]'
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_REL
   Machine: EM_X86_64
 Symbols:
   - Name: 'localfoo [1]'
   - Name: 'localfoo [1]'
+  - Name: 'localfoo [1]'
 
 ## Check that yaml2obj reports an error when we have
 ## symbols without suffixes in the names and their
 ## names are equal.
 
 # RUN: not yaml2obj --docnum=3 %s 2>&1| FileCheck %s --check-prefix=CASE3
-# CASE3: error: Repeated symbol name: 'localfoo'.
+# CASE3: error: repeated symbol name: 'localfoo'
+# CASE3: error: repeated symbol name: 'localfoo'
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_REL
   Machine: EM_X86_64
 Symbols:
-  - Name:    localfoo
-    Section: .text.foo.1
-  - Name:    localfoo
-    Section: .text.foo.2
+  - Name: localfoo
+  - Name: localfoo
+  - Name: localfoo
 
 ## Check that yaml2obj can produce correct relocations that
 ## reference symbols with name suffixes.
 
 # RUN: yaml2obj --docnum=4 %s -o %t4
 # RUN: llvm-readobj -r --expand-relocs %t4 | FileCheck %s --check-prefix=CASE4
 
 # CASE4:      Relocations [

llvm/trunk/test/tools/yaml2obj/dynamic-section-raw-content.yaml

 # RAW-NEXT: ]
 # RAW-NEXT: Address:
 # RAW-NEXT: Offset:
 # RAW-NEXT: Size: 16
 
 # RAW:      Hex dump of section '.dynamic':
 # RAW-NEXT: 0x00000000 01234567 89012345 67890000 00000000 {{.*}}
 
-# RUN: not yaml2obj --docnum=2 %s -o %t2 2>&1 | FileCheck %s --check-prefix=ERR
-# ERR: Cannot specify both raw content and explicit entries for dynamic section '.dynamic'.
-
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_EXEC
   Machine: EM_X86_64
 Sections:
   - Name: .dynamic
     Type: SHT_DYNAMIC
     Content: "01234567890123456789000000000000"
 
+# RUN: not yaml2obj --docnum=2 %s 2>&1 | FileCheck %s --check-prefix=ERR
+
+# ERR: cannot specify both raw content and explicit entries for dynamic section '.dynamic1'
+# ERR: cannot specify both raw content and explicit entries for dynamic section '.dynamic2'
+
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_EXEC
   Machine: EM_X86_64
 Sections:
-  - Name:    .dynamic
+  - Name:    .dynamic1
+    Type:    SHT_DYNAMIC
+    Content: "0123456789"
+    Entries:
+      - Tag:   DT_STRSZ
+        Value: 0
+  - Name:    .dynamic2
     Type:    SHT_DYNAMIC
     Content: "0123456789"
     Entries:
       - Tag:   DT_STRSZ
         Value: 0

llvm/trunk/test/tools/yaml2obj/dynamic-symbols.yaml

-# Ensures that implicitly added sections can be ordered within Sections.
-# RUN: yaml2obj %s -o %t
-# RUN: llvm-readobj --sections %t | FileCheck %s --check-prefix=SECTION
+## Ensures that implicitly added sections can be ordered within Sections.
+
+# RUN: yaml2obj --docnum=1 %s -o %t1
+# RUN: llvm-readobj --sections %t1 | FileCheck %s --check-prefix=SECTION
 # RUN: llvm-nm --dynamic %t | FileCheck %s --check-prefix=SYMBOL
 
-!ELF
+--- !ELF
 FileHeader:
   Class:           ELFCLASS64
   Data:            ELFDATA2LSB
   Type:            ET_DYN
   Machine:         EM_X86_64
 Sections:
   - Name: .data
     Type: SHT_PROGBITS

 # SECTION: Name: .dynstr
 # SECTION-NEXT: Type: SHT_STRTAB
 # SECTION-NEXT: Flags
 # SECTION-NEXT: SHF_ALLOC
 
 # SYMBOL-DAG: d dynlocal
 # SYMBOL-DAG: D dynglobal
 # SYMBOL-DAG: V dynweak
+
+## Check we can use numeric values to refer to sections.
+
+# RUN: yaml2obj --docnum=2 %s -o %t2
+# RUN: not llvm-readobj --dyn-symbols %t2 2>&1 | FileCheck -DFILE=%t2 %s --check-prefix=NUM
+
+# NUM:      Name: foo
+# NUM:      Section:
+# NUM-SAME: .data (0x1)
+
+# NUM:      Name: bar
+# NUM:      Section: 
+# NUM-SAME: .symtab (0x2)
+
+# NUM: error: '[[FILE]]': invalid section index: 255
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_X86_64
+Sections:
+  - Name: .data
+    Type: SHT_PROGBITS
+DynamicSymbols:
+  - Name: foo
+    Section: 1
+  - Name: bar
+    Section: 2
+  - Name: zed
+    Section: 0xff
+
+## Check we report errors when unknown sections are referenced by dynamic symbols.
+
+# RUN: not yaml2obj --docnum=3 %s 2>&1 | FileCheck -DFILE=%t3 %s --check-prefix=ERR
+
+# ERR: error: unknown section referenced: '.sec1' by YAML symbol 'foo'
+# ERR: error: unknown section referenced: '.sec2' by YAML symbol 'bar'
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_X86_64
+Sections:
+  - Name: .data
+    Type: SHT_PROGBITS
+DynamicSymbols:
+  - Name: foo
+    Section: .sec1
+  - Name: bar
+    Section: .sec2

llvm/trunk/test/tools/yaml2obj/dynsymtab-implicit-sections-size-content.yaml

FileHeader:
   Machine: EM_X86_64
 Sections:
   - Name: .dynsym
     Type: SHT_DYNSYM
 DynamicSymbols:
   - Name:    foo
     Binding: STB_GLOBAL
 
-## Specifying both `Size` and symbols at the same time is not allowed.
-# RUN: not yaml2obj --docnum=2 %s -o %t2 2>&1 | FileCheck %s --check-prefix=CASE2
+## Specifying both `Size` and symbols at the same time is not allowed for .dynsym.
 
-# CASE2: error: Cannot specify both `Size` and `DynamicSymbols` for symbol table section '.dynsym'.
+# RUN: not yaml2obj --docnum=2 %s 2>&1 | FileCheck %s --implicit-check-not=error --check-prefix=CASE2
+
+# CASE2: error: cannot specify both `Size` and `DynamicSymbols` for symbol table section '.dynsym'
+# CASE2: error: yaml2obj failed
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_DYN
   Machine: EM_X86_64
 Sections:
   - Name: .dynsym
     Type: SHT_DYNSYM
     Size: 0x100
+  - Name: .dynsym2
+    Type: SHT_DYNSYM
+    Size: 0x100
 DynamicSymbols:
   - Name:    foo
     Binding: STB_GLOBAL
 
-## Specifying both `Content` and symbols at the same time is not allowed.
-# RUN: not yaml2obj --docnum=3 %s -o %t3 2>&1 | FileCheck %s --check-prefix=CASE3
+## Specifying both `Content` and symbols at the same time is not allowed for .dynsym.
 
-# CASE3: error: Cannot specify both `Content` and `DynamicSymbols` for symbol table section '.dynsym'.
+# RUN: not yaml2obj --docnum=3 %s 2>&1 | FileCheck %s --implicit-check-not=error --check-prefix=CASE3
+
+# CASE3: error: cannot specify both `Content` and `DynamicSymbols` for symbol table section '.dynsym'
+# CASE3: error: yaml2obj failed
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_DYN
   Machine: EM_X86_64
 Sections:
   - Name: .dynsym
     Type: SHT_DYNSYM
     Content: "00"
+  - Name: .dynsym2
+    Type: SHT_DYNSYM
+    Content: "00"
 DynamicSymbols:
   - Name:    foo
     Binding: STB_GLOBAL
 
 ## Check we can use just `Content` to emit custom data in the symbol table section.
 # RUN: yaml2obj --docnum=4 %s -o %t4
 # RUN: llvm-readobj --section-data -S %t4 | FileCheck %s --check-prefix=CASE4
 

llvm/trunk/test/tools/yaml2obj/elf-comdat-broken-info.yaml

-# RUN: yaml2obj %s -o %t
-# RUN: llvm-readobj --sections %t | FileCheck %s
+## Check we are able to produce an SHT_GROUP section with a custom Info value (12345).
+
+# RUN: yaml2obj %s -o %t1
+# RUN: llvm-readobj --sections %t1 | FileCheck %s
 
 --- !ELF
 FileHeader:
   Class:               ELFCLASS64
   Data:                ELFDATA2LSB
   Type:                ET_REL
   Machine:             EM_X86_64
 Sections:
   - Name:              .group
     Type:              SHT_GROUP
     Link:              .symtab
     Info:              12345
     Members:
       - SectionOrType: GRP_COMDAT
 
-## Check we are able to produce SHT_GROUP section with a custom Info value (12345).
 # CHECK:      Name: .group
 # CHECK-NEXT: Type: SHT_GROUP
 # CHECK-NEXT: Flags [
 # CHECK-NEXT: ]
 # CHECK-NEXT: Address:
 # CHECK-NEXT: Offset:
 # CHECK-NEXT: Size:
 # CHECK-NEXT: Link:
 # CHECK-NEXT: Info: 12345
+
+## Check we report multiple errors when multiple unknown symbols are referenced by SHT_GROUP sections.
+
+# RUN: not yaml2obj --docnum=2 %s 2>&1 | FileCheck %s --check-prefix=ERR
+
+# ERR: error: unknown symbol referenced: 'foo' by YAML section '.group1'
+# ERR: error: unknown symbol referenced: 'bar' by YAML section '.group2'
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_REL
+  Machine: EM_X86_64
+Sections:
+  - Name: .group1
+    Type: SHT_GROUP
+    Link: .symtab
+    Info: foo
+    Members:
+      - SectionOrType: GRP_COMDAT
+  - Name: .group2
+    Type: SHT_GROUP
+    Link: .symtab
+    Info: bar
+    Members:
+      - SectionOrType: GRP_COMDAT

llvm/trunk/test/tools/yaml2obj/elf-comdat-broken-members.yaml

+## Check we report an error when SHT_GROUP references an unknown section in its member list.
+
+# RUN: not yaml2obj %s 2>&1 | FileCheck %s
+
+# CHECK: error: unknown section referenced: '.foo' by YAML section '.group'
+# CHECK: error: unknown section referenced: '.bar' by YAML section '.group'
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_REL
+  Machine: EM_X86_64
+Sections:
+  - Name: .group
+    Type: SHT_GROUP
+    Link: .symtab
+    Info: 0
+    Members:
+      - SectionOrType: GRP_COMDAT
+      - SectionOrType: .foo
+      - SectionOrType: .bar

llvm/trunk/test/tools/yaml2obj/elf-custom-null-section.yaml

- Type:         SHT_NULL
     Link:         0
   - Type: SHT_PROGBITS
     Name: 'foo'
 
 ## Check we report an error if null section sh_link field refers to an unknown section.
 
 # RUN: not yaml2obj --docnum=6 %s -o %t6 2>&1 | FileCheck %s --check-prefix=CASE4
 
-# CASE4: error: Unknown section referenced: '.foo' at YAML section ''.
+# CASE4: error: unknown section referenced: '.foo' by YAML section ''
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_REL
   Machine: EM_X86_64
 Sections:

llvm/trunk/test/tools/yaml2obj/program-header.yaml

 #CHECK:       }
 #CHECK-NEXT:  ProgramHeader {
 #CHECK-NEXT:    Type: PT_GNU_RELRO (0x6474E552)
 #CHECK:       }
 #CHECK-NEXT:]
 
 ## Check we do not allow referencing sections that do not exist.
 # RUN: not yaml2obj --docnum=2 %s -o %t 2>&1 | FileCheck %s --check-prefix=ERR
-# ERR: error: Unknown section referenced: '.foo' by program header.
+
+# ERR: error: unknown section referenced: '.foo' by program header
+# ERR: error: unknown section referenced: '.bar' by program header
 
 --- !ELF
 FileHeader:
-  Class:           ELFCLASS64
-  Data:            ELFDATA2LSB
-  Type:            ET_EXEC
-  Machine:         EM_X86_64
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_EXEC
+  Machine: EM_X86_64
 ProgramHeaders:
   - Type: PT_LOAD
     Sections:
       - Section: .foo
+      - Section: .bar

llvm/trunk/test/tools/yaml2obj/reloc-sec-info.yaml

-# RUN: yaml2obj %s -o %t
-# RUN: llvm-readobj --sections %t | FileCheck %s
+# RUN: yaml2obj --docnum=1 %s -o %t1
+# RUN: llvm-readobj --sections %t1 | FileCheck %s
 
 # CHECK:      Name: .rela.text
 # CHECK-NEXT: Type: SHT_RELA
 # CHECK-NEXT: Flags [
 # CHECK-NEXT: ]
 # CHECK-NEXT: Address:
 # CHECK-NEXT: Offset:
 # CHECK-NEXT: Size:
 # CHECK-NEXT: Link:
 # CHECK-NEXT: Info: 12345
 
 --- !ELF
 FileHeader:
   Class:           ELFCLASS64
   Data:            ELFDATA2LSB
   Type:            ET_REL
   Machine:         EM_X86_64
 Sections:
   - Name:            .rela.text
     Type:            SHT_RELA
     Link:            .symtab
     Info:            12345
     Relocations:
+
+## Check we report an error when a relocation section references an unknown section via its Info field.
+
+# RUN: not yaml2obj --docnum=2 %s 2>&1 | FileCheck %s --implicit-check-not=error --check-prefix=ERR
+
+# ERR: error: unknown section referenced: '.unknown1' by YAML section '.foo'
+# ERR: error: unknown section referenced: '.unknown2' by YAML section '.bar'
+# ERR: error: yaml2obj failed
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_REL
+  Machine: EM_X86_64
+Sections:
+  - Name: .foo
+    Type: SHT_RELA
+    Info: .unknown1
+    Relocations:
+  - Name: .bar
+    Type: SHT_RELA
+    Info: .unknown2
+    Relocations:
+  - Name: .zed
+    Type: SHT_RELA
+    Info: 0xFF
+    Relocations:

llvm/trunk/test/tools/yaml2obj/relocation-missing-symbol.yaml

-# Show that yaml2obj rejects a symbol reference from a relocation if the symbol
-# does not exist.
+## Show that yaml2obj rejects a symbol reference from a relocation if the symbol
+## does not exist.
 
 # RUN: not yaml2obj %s -o %t 2>&1 | FileCheck %s
 
-# CHECK: Unknown symbol referenced: 'does_not_exist' at YAML section '.rela.text'
+## Check we are able to report multiple errors.
+
+# CHECK: error: unknown symbol referenced: 'does_not_exist1' by YAML section '.rela.text'
+# CHECK: error: unknown symbol referenced: 'does_not_exist2' by YAML section '.rela.text'
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_REL
   Machine: EM_X86_64
 Sections:
   - Name: .text
     Type: SHT_PROGBITS
   - Name: .rela.text
     Type: SHT_RELA
     Info: .text
     Link: .symtab
     Relocations:
       - Type: R_X86_64_PC32
         Offset: 0
-        Symbol: does_not_exist
+        Symbol: does_not_exist1
+      - Type: R_X86_64_PC32
+        Offset: 0
+        Symbol: does_not_exist2

llvm/trunk/test/tools/yaml2obj/section-link.yaml

-# RUN: yaml2obj %s -o %t
-# RUN: llvm-readobj --sections %t | FileCheck %s
+# RUN: yaml2obj --docnum=1 %s -o %t1
+# RUN: llvm-readobj --sections %t1 | FileCheck %s
 
 # CHECK:      Name: .text
 # CHECK-NEXT: Type: SHT_PROGBITS
 # CHECK-NEXT: Flags [
 # CHECK-NEXT:   SHF_ALLOC
 # CHECK-NEXT:   SHF_EXECINSTR
 # CHECK-NEXT: ]
 # CHECK-NEXT: Address:
 # CHECK-NEXT: Offset:
 # CHECK-NEXT: Size:
 # CHECK-NEXT: Link: 12345
 
 --- !ELF
 FileHeader:
   Class:           ELFCLASS64
   Data:            ELFDATA2LSB
   Type:            ET_REL
   Machine:         EM_X86_64
 Sections:
   - Name:            .text
     Type:            SHT_PROGBITS
     Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
     Link:            12345
+
+## Check we report an error when an unknown section is referenced via a Link field.
+
+# RUN: not yaml2obj --docnum=2 %s 2>&1 | FileCheck %s --check-prefix=ERR
+
+# ERR: error: unknown section referenced: '.unknown1' by YAML section '.foo'
+# ERR: error: unknown section referenced: '.unknown2' by YAML section '.bar'
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_REL
+  Machine: EM_X86_64
+Sections:
+  - Name: .foo
+    Type: SHT_PROGBITS
+    Link: .unknown1
+  - Name: .bar
+    Type: SHT_PROGBITS
+    Link: .unknown2

llvm/trunk/test/tools/yaml2obj/symtab-implicit-sections-size-content.yaml

Sections:
   - Name: .symtab
     Type: SHT_SYMTAB
 Symbols:
   - Name: foo
 
 ## Specifying both `Size` and symbols at the same time is not allowed.
 # RUN: not yaml2obj --docnum=2 %s -o %t2 2>&1 | FileCheck %s --check-prefix=CASE2
 
-# CASE2: error: Cannot specify both `Size` and `Symbols` for symbol table section '.symtab'.
+# CASE2: error: cannot specify both `Size` and `Symbols` for symbol table section '.symtab'
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_DYN
   Machine: EM_X86_64
 Sections:
   - Name: .symtab
     Type: SHT_SYMTAB
     Size: 0x100
 Symbols:
   - Name: foo
 
 ## Specifying both `Content` and symbols at the same time is not allowed.
 # RUN: not yaml2obj --docnum=3 %s -o %t3 2>&1 | FileCheck %s --check-prefix=CASE3
 
-# CASE3: error: Cannot specify both `Content` and `Symbols` for symbol table section '.symtab'.
+# CASE3: error: cannot specify both `Content` and `Symbols` for symbol table section '.symtab'
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_DYN
   Machine: EM_X86_64
 Sections: