[lld] b9e2b59 - [yaml2obj][ELF] - Simplify the code that performs sections validation.

Georgii Rymar via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 20 01:28:43 PDT 2020


Author: Georgii Rymar
Date: 2020-10-20T11:28:23+03:00
New Revision: b9e2b59680ad1bbfd2b9110b3ebf3d2b22cad51b

URL: https://github.com/llvm/llvm-project/commit/b9e2b59680ad1bbfd2b9110b3ebf3d2b22cad51b
DIFF: https://github.com/llvm/llvm-project/commit/b9e2b59680ad1bbfd2b9110b3ebf3d2b22cad51b.diff

LOG: [yaml2obj][ELF] - Simplify the code that performs sections validation.

This:
1) Changes the return type of `MappingTraits<T>>::validate` to `std::string`
instead of `StringRef`. It allows to create more complex error messages.

2) It introduces std::vector<std::pair<StringRef, bool>> getEntries():
a new virtual method of Section, which is the base class for all sections.
It returns names of special section specific keys (e.g. "Entries") and flags that
says if them exist in a YAML. The code in validate() uses this list of entries
descriptions to generalize validation.
This approach was discussed in the D89039 thread.

Differential revision: https://reviews.llvm.org/D89463

Added: 
    

Modified: 
    lld/lib/ReaderWriter/MachO/MachONormalizedFileYAML.cpp
    llvm/include/llvm/ObjectYAML/DWARFYAML.h
    llvm/include/llvm/ObjectYAML/ELFYAML.h
    llvm/include/llvm/ObjectYAML/MachOYAML.h
    llvm/include/llvm/ObjectYAML/MinidumpYAML.h
    llvm/include/llvm/Support/YAMLTraits.h
    llvm/lib/ObjectYAML/DWARFYAML.cpp
    llvm/lib/ObjectYAML/ELFYAML.cpp
    llvm/lib/ObjectYAML/MachOYAML.cpp
    llvm/lib/ObjectYAML/MinidumpYAML.cpp
    llvm/test/tools/yaml2obj/ELF/gnu-hash-section.yaml
    llvm/unittests/Support/YAMLIOTest.cpp

Removed: 
    


################################################################################
diff  --git a/lld/lib/ReaderWriter/MachO/MachONormalizedFileYAML.cpp b/lld/lib/ReaderWriter/MachO/MachONormalizedFileYAML.cpp
index 7f53faaeaea3..3826e97d62b9 100644
--- a/lld/lib/ReaderWriter/MachO/MachONormalizedFileYAML.cpp
+++ b/lld/lib/ReaderWriter/MachO/MachONormalizedFileYAML.cpp
@@ -740,9 +740,7 @@ struct MappingTraits<NormalizedFile> {
     io.mapOptional("exports",          file.exportInfo);
     io.mapOptional("dataInCode",       file.dataInCode);
   }
-  static StringRef validate(IO &io, NormalizedFile &file) {
-    return StringRef();
-  }
+  static std::string validate(IO &io, NormalizedFile &file) { return {}; }
 };
 
 } // namespace llvm

diff  --git a/llvm/include/llvm/ObjectYAML/DWARFYAML.h b/llvm/include/llvm/ObjectYAML/DWARFYAML.h
index edb5f00c5e80..856cea9a1535 100644
--- a/llvm/include/llvm/ObjectYAML/DWARFYAML.h
+++ b/llvm/include/llvm/ObjectYAML/DWARFYAML.h
@@ -359,8 +359,8 @@ struct MappingTraits<DWARFYAML::ListTable<EntryType>> {
 template <typename EntryType>
 struct MappingTraits<DWARFYAML::ListEntries<EntryType>> {
   static void mapping(IO &IO, DWARFYAML::ListEntries<EntryType> &ListEntries);
-  static StringRef validate(IO &IO,
-                            DWARFYAML::ListEntries<EntryType> &ListEntries);
+  static std::string validate(IO &IO,
+                              DWARFYAML::ListEntries<EntryType> &ListEntries);
 };
 
 template <> struct MappingTraits<DWARFYAML::RnglistEntry> {

diff  --git a/llvm/include/llvm/ObjectYAML/ELFYAML.h b/llvm/include/llvm/ObjectYAML/ELFYAML.h
index af6d934d611e..5d794bae991c 100644
--- a/llvm/include/llvm/ObjectYAML/ELFYAML.h
+++ b/llvm/include/llvm/ObjectYAML/ELFYAML.h
@@ -193,6 +193,12 @@ struct Section : public Chunk {
 
   static bool classof(const Chunk *S) { return S->Kind != ChunkKind::Fill; }
 
+  // Some derived sections might have own special entries. This method returns
+  // vector of <entry name, is used> pairs. It is used for sections validation.
+  virtual std::vector<std::pair<StringRef, bool>> getEntries() const {
+    return {};
+  };
+
   // The following members are used to override section fields which is
   // useful for creating invalid objects.
 
@@ -235,6 +241,10 @@ struct StackSizesSection : Section {
 
   StackSizesSection() : Section(ChunkKind::StackSizes) {}
 
+  std::vector<std::pair<StringRef, bool>> getEntries() const override {
+    return {{"Entries", !!Entries}};
+  };
+
   static bool classof(const Chunk *S) {
     return S->Kind == ChunkKind::StackSizes;
   }
@@ -249,6 +259,10 @@ struct DynamicSection : Section {
 
   DynamicSection() : Section(ChunkKind::Dynamic) {}
 
+  std::vector<std::pair<StringRef, bool>> getEntries() const override {
+    return {{"Entries", !!Entries}};
+  };
+
   static bool classof(const Chunk *S) { return S->Kind == ChunkKind::Dynamic; }
 };
 
@@ -276,6 +290,10 @@ struct NoteSection : Section {
 
   NoteSection() : Section(ChunkKind::Note) {}
 
+  std::vector<std::pair<StringRef, bool>> getEntries() const override {
+    return {{"Notes", !!Notes}};
+  };
+
   static bool classof(const Chunk *S) { return S->Kind == ChunkKind::Note; }
 };
 
@@ -283,6 +301,10 @@ struct HashSection : Section {
   Optional<std::vector<uint32_t>> Bucket;
   Optional<std::vector<uint32_t>> Chain;
 
+  std::vector<std::pair<StringRef, bool>> getEntries() const override {
+    return {{"Bucket", !!Bucket}, {"Chain", !!Chain}};
+  };
+
   // The following members are used to override section fields.
   // This is useful for creating invalid objects.
   Optional<llvm::yaml::Hex64> NBucket;
@@ -321,6 +343,13 @@ struct GnuHashSection : Section {
 
   GnuHashSection() : Section(ChunkKind::GnuHash) {}
 
+  std::vector<std::pair<StringRef, bool>> getEntries() const override {
+    return {{"Header", !!Header},
+            {"BloomFilter", !!BloomFilter},
+            {"HashBuckets", !!HashBuckets},
+            {"HashValues", !!HashValues}};
+  };
+
   static bool classof(const Chunk *S) { return S->Kind == ChunkKind::GnuHash; }
 };
 
@@ -343,6 +372,10 @@ struct VerneedSection : Section {
 
   VerneedSection() : Section(ChunkKind::Verneed) {}
 
+  std::vector<std::pair<StringRef, bool>> getEntries() const override {
+    return {{"Dependencies", !!VerneedV}};
+  };
+
   static bool classof(const Chunk *S) {
     return S->Kind == ChunkKind::Verneed;
   }
@@ -353,6 +386,10 @@ struct AddrsigSection : Section {
 
   AddrsigSection() : Section(ChunkKind::Addrsig) {}
 
+  std::vector<std::pair<StringRef, bool>> getEntries() const override {
+    return {{"Symbols", !!Symbols}};
+  };
+
   static bool classof(const Chunk *S) { return S->Kind == ChunkKind::Addrsig; }
 };
 
@@ -366,6 +403,10 @@ struct LinkerOptionsSection : Section {
 
   LinkerOptionsSection() : Section(ChunkKind::LinkerOptions) {}
 
+  std::vector<std::pair<StringRef, bool>> getEntries() const override {
+    return {{"Options", !!Options}};
+  };
+
   static bool classof(const Chunk *S) {
     return S->Kind == ChunkKind::LinkerOptions;
   }
@@ -376,6 +417,10 @@ struct DependentLibrariesSection : Section {
 
   DependentLibrariesSection() : Section(ChunkKind::DependentLibraries) {}
 
+  std::vector<std::pair<StringRef, bool>> getEntries() const override {
+    return {{"Libraries", !!Libs}};
+  };
+
   static bool classof(const Chunk *S) {
     return S->Kind == ChunkKind::DependentLibraries;
   }
@@ -396,6 +441,10 @@ struct CallGraphProfileSection : Section {
 
   CallGraphProfileSection() : Section(ChunkKind::CallGraphProfile) {}
 
+  std::vector<std::pair<StringRef, bool>> getEntries() const override {
+    return {{"Entries", !!Entries}};
+  };
+
   static bool classof(const Chunk *S) {
     return S->Kind == ChunkKind::CallGraphProfile;
   }
@@ -406,6 +455,10 @@ struct SymverSection : Section {
 
   SymverSection() : Section(ChunkKind::Symver) {}
 
+  std::vector<std::pair<StringRef, bool>> getEntries() const override {
+    return {{"Entries", !!Entries}};
+  };
+
   static bool classof(const Chunk *S) { return S->Kind == ChunkKind::Symver; }
 };
 
@@ -424,6 +477,10 @@ struct VerdefSection : Section {
 
   VerdefSection() : Section(ChunkKind::Verdef) {}
 
+  std::vector<std::pair<StringRef, bool>> getEntries() const override {
+    return {{"Entries", !!Entries}};
+  };
+
   static bool classof(const Chunk *S) { return S->Kind == ChunkKind::Verdef; }
 };
 
@@ -435,6 +492,10 @@ struct GroupSection : Section {
 
   GroupSection() : Section(ChunkKind::Group) {}
 
+  std::vector<std::pair<StringRef, bool>> getEntries() const override {
+    return {{"Members", !!Members}};
+  };
+
   static bool classof(const Chunk *S) { return S->Kind == ChunkKind::Group; }
 };
 
@@ -451,6 +512,10 @@ struct RelocationSection : Section {
 
   RelocationSection() : Section(ChunkKind::Relocation) {}
 
+  std::vector<std::pair<StringRef, bool>> getEntries() const override {
+    return {{"Relocations", !!Relocations}};
+  };
+
   static bool classof(const Chunk *S) {
     return S->Kind == ChunkKind::Relocation;
   }
@@ -461,6 +526,10 @@ struct RelrSection : Section {
 
   RelrSection() : Section(ChunkKind::Relr) {}
 
+  std::vector<std::pair<StringRef, bool>> getEntries() const override {
+    return {{"Entries", !!Entries}};
+  };
+
   static bool classof(const Chunk *S) {
     return S->Kind == ChunkKind::Relr;
   }
@@ -471,6 +540,10 @@ struct SymtabShndxSection : Section {
 
   SymtabShndxSection() : Section(ChunkKind::SymtabShndxSection) {}
 
+  std::vector<std::pair<StringRef, bool>> getEntries() const override {
+    return {{"Entries", !!Entries}};
+  };
+
   static bool classof(const Chunk *S) {
     return S->Kind == ChunkKind::SymtabShndxSection;
   }
@@ -486,6 +559,10 @@ struct ARMIndexTableSection : Section {
 
   ARMIndexTableSection() : Section(ChunkKind::ARMIndexTable) {}
 
+  std::vector<std::pair<StringRef, bool>> getEntries() const override {
+    return {{"Entries", !!Entries}};
+  };
+
   static bool classof(const Chunk *S) {
     return S->Kind == ChunkKind::ARMIndexTable;
   }
@@ -699,7 +776,7 @@ struct MappingTraits<ELFYAML::FileHeader> {
 
 template <> struct MappingTraits<ELFYAML::SectionHeaderTable> {
   static void mapping(IO &IO, ELFYAML::SectionHeaderTable &SecHdrTable);
-  static StringRef validate(IO &IO, ELFYAML::SectionHeaderTable &SecHdrTable);
+  static std::string validate(IO &IO, ELFYAML::SectionHeaderTable &SecHdrTable);
 };
 
 template <> struct MappingTraits<ELFYAML::SectionHeader> {
@@ -713,7 +790,7 @@ template <> struct MappingTraits<ELFYAML::ProgramHeader> {
 template <>
 struct MappingTraits<ELFYAML::Symbol> {
   static void mapping(IO &IO, ELFYAML::Symbol &Symbol);
-  static StringRef validate(IO &IO, ELFYAML::Symbol &Symbol);
+  static std::string validate(IO &IO, ELFYAML::Symbol &Symbol);
 };
 
 template <> struct MappingTraits<ELFYAML::StackSizeEntry> {
@@ -762,7 +839,7 @@ template <> struct MappingTraits<ELFYAML::ARMIndexTableEntry> {
 
 template <> struct MappingTraits<std::unique_ptr<ELFYAML::Chunk>> {
   static void mapping(IO &IO, std::unique_ptr<ELFYAML::Chunk> &C);
-  static StringRef validate(IO &io, std::unique_ptr<ELFYAML::Chunk> &C);
+  static std::string validate(IO &io, std::unique_ptr<ELFYAML::Chunk> &C);
 };
 
 template <>

diff  --git a/llvm/include/llvm/ObjectYAML/MachOYAML.h b/llvm/include/llvm/ObjectYAML/MachOYAML.h
index fb6780b6d0ed..94e66c5ae787 100644
--- a/llvm/include/llvm/ObjectYAML/MachOYAML.h
+++ b/llvm/include/llvm/ObjectYAML/MachOYAML.h
@@ -220,7 +220,7 @@ template <> struct MappingTraits<MachOYAML::Relocation> {
 
 template <> struct MappingTraits<MachOYAML::Section> {
   static void mapping(IO &IO, MachOYAML::Section &Section);
-  static StringRef validate(IO &io, MachOYAML::Section &Section);
+  static std::string validate(IO &io, MachOYAML::Section &Section);
 };
 
 template <> struct MappingTraits<MachOYAML::NListEntry> {

diff  --git a/llvm/include/llvm/ObjectYAML/MinidumpYAML.h b/llvm/include/llvm/ObjectYAML/MinidumpYAML.h
index c1711a28dd84..b0cee541cef2 100644
--- a/llvm/include/llvm/ObjectYAML/MinidumpYAML.h
+++ b/llvm/include/llvm/ObjectYAML/MinidumpYAML.h
@@ -236,7 +236,7 @@ template <> struct BlockScalarTraits<MinidumpYAML::BlockStringRef> {
 
 template <> struct MappingTraits<std::unique_ptr<MinidumpYAML::Stream>> {
   static void mapping(IO &IO, std::unique_ptr<MinidumpYAML::Stream> &S);
-  static StringRef validate(IO &IO, std::unique_ptr<MinidumpYAML::Stream> &S);
+  static std::string validate(IO &IO, std::unique_ptr<MinidumpYAML::Stream> &S);
 };
 
 template <> struct MappingContextTraits<minidump::MemoryDescriptor, BinaryRef> {

diff  --git a/llvm/include/llvm/Support/YAMLTraits.h b/llvm/include/llvm/Support/YAMLTraits.h
index f32a6ca92d29..bdfe7f7cbfcb 100644
--- a/llvm/include/llvm/Support/YAMLTraits.h
+++ b/llvm/include/llvm/Support/YAMLTraits.h
@@ -61,7 +61,7 @@ struct MappingTraits {
   // Must provide:
   // static void mapping(IO &io, T &fields);
   // Optionally may provide:
-  // static StringRef validate(IO &io, T &fields);
+  // static std::string validate(IO &io, T &fields);
   //
   // The optional flow flag will cause generated YAML to use a flow mapping
   // (e.g. { a: 0, b: 1 }):
@@ -83,7 +83,7 @@ template <class T, class Context> struct MappingContextTraits {
   // Must provide:
   // static void mapping(IO &io, T &fields, Context &Ctx);
   // Optionally may provide:
-  // static StringRef validate(IO &io, T &fields, Context &Ctx);
+  // static std::string validate(IO &io, T &fields, Context &Ctx);
   //
   // The optional flow flag will cause generated YAML to use a flow mapping
   // (e.g. { a: 0, b: 1 }):
@@ -421,7 +421,7 @@ template <class T> struct has_MappingTraits<T, EmptyContext> {
 
 // Test if MappingContextTraits<T>::validate() is defined on type T.
 template <class T, class Context> struct has_MappingValidateTraits {
-  using Signature_validate = StringRef (*)(class IO &, T &, Context &);
+  using Signature_validate = std::string (*)(class IO &, T &, Context &);
 
   template <typename U>
   static char test(SameType<Signature_validate, &U::validate>*);
@@ -435,7 +435,7 @@ template <class T, class Context> struct has_MappingValidateTraits {
 
 // Test if MappingTraits<T>::validate() is defined on type T.
 template <class T> struct has_MappingValidateTraits<T, EmptyContext> {
-  using Signature_validate = StringRef (*)(class IO &, T &);
+  using Signature_validate = std::string (*)(class IO &, T &);
 
   template <typename U>
   static char test(SameType<Signature_validate, &U::validate> *);
@@ -1041,7 +1041,7 @@ yamlize(IO &io, T &Val, bool, Context &Ctx) {
   else
     io.beginMapping();
   if (io.outputting()) {
-    StringRef Err = MappingTraits<T>::validate(io, Val);
+    std::string Err = MappingTraits<T>::validate(io, Val);
     if (!Err.empty()) {
       errs() << Err << "\n";
       assert(Err.empty() && "invalid struct trying to be written as yaml");
@@ -1049,7 +1049,7 @@ yamlize(IO &io, T &Val, bool, Context &Ctx) {
   }
   detail::doMapping(io, Val, Ctx);
   if (!io.outputting()) {
-    StringRef Err = MappingTraits<T>::validate(io, Val);
+    std::string Err = MappingTraits<T>::validate(io, Val);
     if (!Err.empty())
       io.setError(Err);
   }

diff  --git a/llvm/lib/ObjectYAML/DWARFYAML.cpp b/llvm/lib/ObjectYAML/DWARFYAML.cpp
index 36da74f904ce..2591bf4d5af4 100644
--- a/llvm/lib/ObjectYAML/DWARFYAML.cpp
+++ b/llvm/lib/ObjectYAML/DWARFYAML.cpp
@@ -304,11 +304,11 @@ void MappingTraits<DWARFYAML::ListEntries<EntryType>>::mapping(
 }
 
 template <typename EntryType>
-StringRef MappingTraits<DWARFYAML::ListEntries<EntryType>>::validate(
+std::string MappingTraits<DWARFYAML::ListEntries<EntryType>>::validate(
     IO &IO, DWARFYAML::ListEntries<EntryType> &ListEntries) {
   if (ListEntries.Entries && ListEntries.Content)
     return "Entries and Content can't be used together";
-  return StringRef();
+  return "";
 }
 
 template <typename EntryType>

diff  --git a/llvm/lib/ObjectYAML/ELFYAML.cpp b/llvm/lib/ObjectYAML/ELFYAML.cpp
index b3dfd7f73945..a26ffa86a790 100644
--- a/llvm/lib/ObjectYAML/ELFYAML.cpp
+++ b/llvm/lib/ObjectYAML/ELFYAML.cpp
@@ -864,14 +864,14 @@ void MappingTraits<ELFYAML::SectionHeaderTable>::mapping(
   IO.mapOptional("NoHeaders", SectionHeader.NoHeaders);
 }
 
-StringRef MappingTraits<ELFYAML::SectionHeaderTable>::validate(
+std::string MappingTraits<ELFYAML::SectionHeaderTable>::validate(
     IO &IO, ELFYAML::SectionHeaderTable &SecHdrTable) {
   if (SecHdrTable.NoHeaders && (SecHdrTable.Sections || SecHdrTable.Excluded))
     return "NoHeaders can't be used together with Sections/Excluded";
   if (!SecHdrTable.NoHeaders && !SecHdrTable.Sections && !SecHdrTable.Excluded)
     return "SectionHeaderTable can't be empty. Use 'NoHeaders' key to drop the "
            "section header table";
-  return StringRef();
+  return {};
 }
 
 void MappingTraits<ELFYAML::FileHeader>::mapping(IO &IO,
@@ -1089,11 +1089,11 @@ void MappingTraits<ELFYAML::Symbol>::mapping(IO &IO, ELFYAML::Symbol &Symbol) {
   IO.mapOptional("Other", Keys->Other);
 }
 
-StringRef MappingTraits<ELFYAML::Symbol>::validate(IO &IO,
-                                                   ELFYAML::Symbol &Symbol) {
+std::string MappingTraits<ELFYAML::Symbol>::validate(IO &IO,
+                                                     ELFYAML::Symbol &Symbol) {
   if (Symbol.Index && Symbol.Section.data())
     return "Index and Section cannot both be specified for Symbol";
-  return StringRef();
+  return {};
 }
 
 static void commonSectionMapping(IO &IO, ELFYAML::Section &Section) {
@@ -1422,7 +1422,7 @@ void MappingTraits<std::unique_ptr<ELFYAML::Chunk>>::mapping(
   }
 }
 
-StringRef MappingTraits<std::unique_ptr<ELFYAML::Chunk>>::validate(
+std::string MappingTraits<std::unique_ptr<ELFYAML::Chunk>>::validate(
     IO &io, std::unique_ptr<ELFYAML::Chunk> &C) {
   if (const auto *F = dyn_cast<ELFYAML::Fill>(C.get())) {
     if (F->Pattern && F->Pattern->binary_size() != 0 && !F->Size)
@@ -1435,117 +1435,36 @@ StringRef MappingTraits<std::unique_ptr<ELFYAML::Chunk>>::validate(
       (uint64_t)(*Sec.Size) < Sec.Content->binary_size())
     return "Section size must be greater than or equal to the content size";
 
-  if (const auto *RawSection = dyn_cast<ELFYAML::RawContentSection>(C.get())) {
-    if (RawSection->Flags && RawSection->ShFlags)
-      return "ShFlags and Flags cannot be used together";
-    return {};
-  }
-
-  if (const auto *SS = dyn_cast<ELFYAML::StackSizesSection>(C.get())) {
-    if ((SS->Content || SS->Size) && SS->Entries)
-      return "\"Entries\" cannot be used with \"Content\" or \"Size\"";
-    return {};
-  }
-
-  if (const auto *HS = dyn_cast<ELFYAML::HashSection>(C.get())) {
-    if ((HS->Content || HS->Size) && (HS->Bucket || HS->Chain))
-      return "\"Bucket\" and \"Chain\" cannot be used with \"Content\" or "
-             "\"Size\"";
-    if ((HS->Bucket && !HS->Chain) || (!HS->Bucket && HS->Chain))
-      return "\"Bucket\" and \"Chain\" must be used together";
-    return {};
-  }
-
-  if (const auto *Sec = dyn_cast<ELFYAML::AddrsigSection>(C.get())) {
-    if ((Sec->Content || Sec->Size) && Sec->Symbols)
-      return "\"Symbols\" cannot be used with \"Content\" or \"Size\"";
-    return {};
-  }
-
-  if (const auto *NS = dyn_cast<ELFYAML::NoteSection>(C.get())) {
-    if ((NS->Content || NS->Size) && NS->Notes)
-      return "\"Notes\" cannot be used with \"Content\" or \"Size\"";
-    return {};
-  }
-
-  if (const auto *Sec = dyn_cast<ELFYAML::GnuHashSection>(C.get())) {
-    const bool HasSpecialFields =
-        Sec->Header || Sec->BloomFilter || Sec->HashBuckets || Sec->HashValues;
-    if (HasSpecialFields && (Sec->Content || Sec->Size))
-      return "\"Header\", \"BloomFilter\", "
-             "\"HashBuckets\" and \"HashValues\" can't be used together with "
-             "\"Content\" or \"Size\"";
-
-    if (HasSpecialFields && (!Sec->Header || !Sec->BloomFilter ||
-                             !Sec->HashBuckets || !Sec->HashValues))
-      return "\"Header\", \"BloomFilter\", "
-             "\"HashBuckets\" and \"HashValues\" must be used together";
-    return {};
-  }
-
-  if (const auto *Sec = dyn_cast<ELFYAML::LinkerOptionsSection>(C.get())) {
-    if ((Sec->Content || Sec->Size) && Sec->Options)
-      return "\"Options\" cannot be used with \"Content\" or \"Size\"";
-    return {};
-  }
-
-  if (const auto *Sec = dyn_cast<ELFYAML::DependentLibrariesSection>(C.get())) {
-    if ((Sec->Content || Sec->Size) && Sec->Libs)
-      return "\"Libraries\" cannot be used with \"Content\" or \"Size\"";
-    return {};
-  }
-
-  if (const auto *VD = dyn_cast<ELFYAML::VerdefSection>(C.get())) {
-    if ((VD->Content || VD->Size) && VD->Entries)
-      return "\"Entries\" cannot be used with \"Content\" or \"Size\"";
-    return {};
-  }
-
-  if (const auto *VN = dyn_cast<ELFYAML::VerneedSection>(C.get())) {
-    if ((VN->Content || VN->Size) && VN->VerneedV)
-      return "\"Dependencies\" cannot be used with \"Content\" or \"Size\"";
-    return {};
-  }
-
-  if (const auto *SV = dyn_cast<ELFYAML::SymverSection>(C.get())) {
-    if ((SV->Content || SV->Size) && SV->Entries)
-      return "\"Entries\" cannot be used with \"Content\" or \"Size\"";
-    return {};
-  }
-
-  if (const auto *RS = dyn_cast<ELFYAML::RelrSection>(C.get())) {
-    if ((RS->Content || RS->Size) && RS->Entries)
-      return "\"Entries\" cannot be used with \"Content\" or \"Size\"";
-    return {};
-  }
-
-  if (const auto *CGP = dyn_cast<ELFYAML::CallGraphProfileSection>(C.get())) {
-    if ((CGP->Content || CGP->Size) && CGP->Entries)
-      return "\"Entries\" cannot be used with \"Content\" or \"Size\"";
-    return {};
-  }
+  auto BuildErrPrefix = [](ArrayRef<std::pair<StringRef, bool>> EntV) {
+    std::string Msg;
+    for (size_t I = 0; I < EntV.size(); ++I) {
+      StringRef Name = EntV[I].first;
+      if (I == 0) {
+        Msg = "\"" + Name.str() + "\"";
+        continue;
+      }
+      if (I != EntV.size() - 1)
+        Msg += ", \"" + Name.str() + "\"";
+      else
+        Msg += " and \"" + Name.str() + "\"";
+    }
+    return Msg;
+  };
 
-  if (const auto *IT = dyn_cast<ELFYAML::ARMIndexTableSection>(C.get())) {
-    if ((IT->Content || IT->Size) && IT->Entries)
-      return "\"Entries\" cannot be used with \"Content\" or \"Size\"";
-    return {};
-  }
+  std::vector<std::pair<StringRef, bool>> Entries = Sec.getEntries();
+  const size_t NumUsedEntries = llvm::count_if(
+      Entries, [](const std::pair<StringRef, bool> &P) { return P.second; });
 
-  if (const auto *RS = dyn_cast<ELFYAML::RelocationSection>(C.get())) {
-    if ((RS->Content || RS->Size) && RS->Relocations)
-      return "\"Relocations\" cannot be used with \"Content\" or \"Size\"";
-    return {};
-  }
+  if ((Sec.Size || Sec.Content) && NumUsedEntries > 0)
+    return BuildErrPrefix(Entries) +
+           " cannot be used with \"Content\" or \"Size\"";
 
-  if (const auto *SS = dyn_cast<ELFYAML::SymtabShndxSection>(C.get())) {
-    if ((SS->Content || SS->Size) && SS->Entries)
-      return "\"Entries\" cannot be used with \"Content\" or \"Size\"";
-    return {};
-  }
+  if (NumUsedEntries > 0 && Entries.size() != NumUsedEntries)
+    return BuildErrPrefix(Entries) + " must be used together";
 
-  if (const auto *GS = dyn_cast<ELFYAML::GroupSection>(C.get())) {
-    if ((GS->Content || GS->Size) && GS->Members)
-      return "\"Members\" cannot be used with \"Content\" or \"Size\"";
+  if (const auto *RawSection = dyn_cast<ELFYAML::RawContentSection>(C.get())) {
+    if (RawSection->Flags && RawSection->ShFlags)
+      return "ShFlags and Flags cannot be used together";
     return {};
   }
 
@@ -1555,12 +1474,6 @@ StringRef MappingTraits<std::unique_ptr<ELFYAML::Chunk>>::validate(
     return {};
   }
 
-  if (const auto *DS = dyn_cast<ELFYAML::DynamicSection>(C.get())) {
-    if ((DS->Content || DS->Size) && DS->Entries)
-      return "\"Entries\" cannot be used with \"Content\" or \"Size\"";
-    return {};
-  }
-
   if (const auto *MF = dyn_cast<ELFYAML::MipsABIFlags>(C.get())) {
     if (MF->Content)
       return "\"Content\" key is not implemented for SHT_MIPS_ABIFLAGS "

diff  --git a/llvm/lib/ObjectYAML/MachOYAML.cpp b/llvm/lib/ObjectYAML/MachOYAML.cpp
index 86aad0233767..adcc4f0f1d4d 100644
--- a/llvm/lib/ObjectYAML/MachOYAML.cpp
+++ b/llvm/lib/ObjectYAML/MachOYAML.cpp
@@ -305,7 +305,7 @@ void MappingTraits<MachOYAML::Section>::mapping(IO &IO,
   IO.mapOptional("relocations", Section.relocations);
 }
 
-StringRef
+std::string
 MappingTraits<MachOYAML::Section>::validate(IO &IO,
                                             MachOYAML::Section &Section) {
   if (Section.content && Section.size < Section.content->binary_size())

diff  --git a/llvm/lib/ObjectYAML/MinidumpYAML.cpp b/llvm/lib/ObjectYAML/MinidumpYAML.cpp
index 77ea42c41a4c..e1a80b98e449 100644
--- a/llvm/lib/ObjectYAML/MinidumpYAML.cpp
+++ b/llvm/lib/ObjectYAML/MinidumpYAML.cpp
@@ -292,7 +292,7 @@ static void streamMapping(yaml::IO &IO, RawContentStream &Stream) {
   IO.mapOptional("Size", Stream.Size, Stream.Content.binary_size());
 }
 
-static StringRef streamValidate(RawContentStream &Stream) {
+static std::string streamValidate(RawContentStream &Stream) {
   if (Stream.Size.value < Stream.Content.binary_size())
     return "Stream size must be greater or equal to the content size";
   return "";
@@ -434,7 +434,7 @@ void yaml::MappingTraits<std::unique_ptr<Stream>>::mapping(
   }
 }
 
-StringRef yaml::MappingTraits<std::unique_ptr<Stream>>::validate(
+std::string yaml::MappingTraits<std::unique_ptr<Stream>>::validate(
     yaml::IO &IO, std::unique_ptr<MinidumpYAML::Stream> &S) {
   switch (S->Kind) {
   case MinidumpYAML::Stream::StreamKind::RawContent:

diff  --git a/llvm/test/tools/yaml2obj/ELF/gnu-hash-section.yaml b/llvm/test/tools/yaml2obj/ELF/gnu-hash-section.yaml
index 3b58fa8ce407..607336a4cc84 100644
--- a/llvm/test/tools/yaml2obj/ELF/gnu-hash-section.yaml
+++ b/llvm/test/tools/yaml2obj/ELF/gnu-hash-section.yaml
@@ -231,7 +231,7 @@ Sections:
 
 # RUN: not yaml2obj --docnum=11 -DCONTENT="" %s 2>&1 | FileCheck %s --check-prefix=TOGETHER
 # RUN: not yaml2obj --docnum=11 -DSIZE=0 %s 2>&1 | FileCheck %s --check-prefix=TOGETHER
-# TOGETHER: error: "Header", "BloomFilter", "HashBuckets" and "HashValues" can't be used together with "Content" or "Size"
+# TOGETHER: error: "Header", "BloomFilter", "HashBuckets" and "HashValues" cannot be used with "Content" or "Size"
 
 --- !ELF
 FileHeader:

diff  --git a/llvm/unittests/Support/YAMLIOTest.cpp b/llvm/unittests/Support/YAMLIOTest.cpp
index 492d854ef812..c7df4b919a27 100644
--- a/llvm/unittests/Support/YAMLIOTest.cpp
+++ b/llvm/unittests/Support/YAMLIOTest.cpp
@@ -1782,10 +1782,10 @@ namespace yaml {
     static void mapping(IO &io, MyValidation &d) {
         io.mapRequired("value", d.value);
     }
-    static StringRef validate(IO &io, MyValidation &d) {
+    static std::string validate(IO &io, MyValidation &d) {
         if (d.value < 0)
           return "negative value";
-        return StringRef();
+        return {};
     }
   };
  }


        


More information about the llvm-commits mailing list