[yaml2obj/obj2yaml] - Refine handling of SHT_GNU_verdef sections.

[grimar]

Summary

This patch:

1. Makes Version, Flags, VersionNdx and Hash fields to be Optional<>.
2. Disallows dumping version definitions that have vd_version != 1. vd_version identifies the version of the structure itself. (https://refspecs.linuxfoundation.org/LSB_5.0.0/LSB-Core-generic/LSB-Core-generic/symversion.html , https://docs.oracle.com/cd/E19683-01/816-7777/chapter6-80869/index.html)
3. Stops dumping default values for Version, Flags, VersionNdx and Hash fields.
4. Refines testing.

[jhenderson]

llvm/test/tools/obj2yaml/ELF/verdef-section.yaml

70

llvm/test/tools/yaml2obj/ELF/verdef-section.yaml

62–73

82–86

What about unknown flags?

95

[grimar]

• Addressed review comments.

llvm/test/tools/yaml2obj/ELF/verdef-section.yaml

82–86

I've updated this case to cover both known and unknown cases.

[jhenderson]

LGTM, with nit.

llvm/test/tools/yaml2obj/ELF/verdef-section.yaml

82–87

How about: "an entry that has version predecessors and a selection of flags, (all known, some unknown)."

[MaskRay]

LSB 1.3 is superseded. The description can link to https://refspecs.linuxfoundation.org/LSB_5.0.0/LSB-Core-generic/LSB-Core-generic/symversion.html instead

[MaskRay]

Stops dumping default values for Version, Flags, VersionNdx and Hash fields.

https://maskray.me/blog/2020-11-26-all-about-symbol-versioning

In practice, one Verdef has only one auxilliary Verdaux entry. vd_hash is the ELF hash of the only vda_name and in theory the ELF hash may return 0.. But I think using 0 in obj2yaml is fine.

[grimar]

In D94659#2498626, @MaskRay wrote:

But I think using 0 in obj2yaml is fine.

Yeah, it is done for convenience - our tests often don't need to know about the hash. E.g. it is not important when printing versioned symbol names
Using 0 as the default value allows to reduce an unimportant noise in YAML descriptions.

llvm/include/llvm/ObjectYAML/ELFYAML.h

struct SymverSection : Section {
   std::vector<std::pair<StringRef, bool>> getEntries() const override {
     return {{"Entries", Entries.hasValue()}};
   };
 
   static bool classof(const Chunk *S) { return S->Kind == ChunkKind::Symver; }
 };
 
 struct VerdefEntry {
-  uint16_t Version;
-  uint16_t Flags;
-  uint16_t VersionNdx;
-  uint32_t Hash;
+  Optional<uint16_t> Version;
+  Optional<uint16_t> Flags;
+  Optional<uint16_t> VersionNdx;
+  Optional<uint32_t> Hash;
   std::vector<StringRef> VerNames;
 };
 
 struct VerdefSection : Section {
   Optional<std::vector<VerdefEntry>> Entries;
 
   llvm::yaml::Hex64 Info;
 

llvm/lib/ObjectYAML/ELFEmitter.cpp

void ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
   if (!Section.Entries)
     return;
 
   uint64_t AuxCnt = 0;
   for (size_t I = 0; I < Section.Entries->size(); ++I) {
     const ELFYAML::VerdefEntry &E = (*Section.Entries)[I];
 
     Elf_Verdef VerDef;
-    VerDef.vd_version = E.Version;
-    VerDef.vd_flags = E.Flags;
-    VerDef.vd_ndx = E.VersionNdx;
-    VerDef.vd_hash = E.Hash;
+    VerDef.vd_version = E.Version.getValueOr(1);
+    VerDef.vd_flags = E.Flags.getValueOr(0);
+    VerDef.vd_ndx = E.VersionNdx.getValueOr(0);
+    VerDef.vd_hash = E.Hash.getValueOr(0);
     VerDef.vd_aux = sizeof(Elf_Verdef);
     VerDef.vd_cnt = E.VerNames.size();
     if (I == Section.Entries->size() - 1)
       VerDef.vd_next = 0;
     else
       VerDef.vd_next =
           sizeof(Elf_Verdef) + E.VerNames.size() * sizeof(Elf_Verdaux);
     CBA.write((const char *)&VerDef, sizeof(Elf_Verdef));

llvm/lib/ObjectYAML/ELFYAML.cpp

void MappingTraits<ELFYAML::NoteEntry>::mapping(IO &IO, ELFYAML::NoteEntry &N) {
   IO.mapOptional("Desc", N.Desc);
   IO.mapRequired("Type", N.Type);
 }
 
 void MappingTraits<ELFYAML::VerdefEntry>::mapping(IO &IO,
                                                   ELFYAML::VerdefEntry &E) {
   assert(IO.getContext() && "The IO context is not initialized");
 
-  IO.mapRequired("Version", E.Version);
-  IO.mapRequired("Flags", E.Flags);
-  IO.mapRequired("VersionNdx", E.VersionNdx);
-  IO.mapRequired("Hash", E.Hash);
+  IO.mapOptional("Version", E.Version);
+  IO.mapOptional("Flags", E.Flags);
+  IO.mapOptional("VersionNdx", E.VersionNdx);
+  IO.mapOptional("Hash", E.Hash);
   IO.mapRequired("Names", E.VerNames);
 }
 
 void MappingTraits<ELFYAML::VerneedEntry>::mapping(IO &IO,
                                                    ELFYAML::VerneedEntry &E) {
   assert(IO.getContext() && "The IO context is not initialized");
 
   IO.mapRequired("Version", E.Version);

llvm/test/tools/obj2yaml/ELF/verdef-section.yaml

+## Check how we dump SHT_GNU_verdef sections.
+
 # RUN: yaml2obj %s -o %t
 # RUN: obj2yaml %t | FileCheck %s
 
-## Check we are able to yamalize SHT_GNU_verdef section.
-
-# CHECK:       - Name:            .gnu.version_d
-# CHECK-NEXT:     Type:            SHT_GNU_verdef
-# CHECK-NEXT:     Flags:           [ SHF_ALLOC ]
-# CHECK-NEXT:     Address:         0x230
-# CHECK-NEXT:     Link:            .dynstr
-# CHECK-NEXT:     AddressAlign:    0x4
-# CHECK-NEXT:     Info:            0x4
-# CHECK-NEXT:     Entries:
-# CHECK-NEXT:       - Version:         1
-# CHECK-NEXT:         Flags:           1
-# CHECK-NEXT:         VersionNdx:      1
-# CHECK-NEXT:         Hash:            170240160
-# CHECK-NEXT:         Names:
-# CHECK-NEXT:           - dso.so.0
-# CHECK-NEXT:       - Version:         1
-# CHECK-NEXT:         Flags:           2
-# CHECK-NEXT:         VersionNdx:      2
-# CHECK-NEXT:         Hash:            108387921
-# CHECK-NEXT:         Names:
-# CHECK-NEXT:           - VERSION_1
-# CHECK-NEXT:       - Version:         1
-# CHECK-NEXT:         Flags:           3
-# CHECK-NEXT:         VersionNdx:      3
-# CHECK-NEXT:         Hash:            108387922
-# CHECK-NEXT:         Names:
-# CHECK-NEXT:           - VERSION_2
-# CHECK-NEXT:           - VERSION_3
+# CHECK:       - Name:         .gnu.version_d
+# CHECK-NEXT:    Type:         SHT_GNU_verdef
+# CHECK-NEXT:    Flags:        [ SHF_ALLOC ]
+# CHECK-NEXT:    Address:      0x230
+# CHECK-NEXT:    Link:         .dynstr
+# CHECK-NEXT:    AddressAlign: 0x4
+# CHECK-NEXT:    Info:         0x4
+# CHECK-NEXT:    Entries:
+# CHECK-NEXT:      - Names:
+# CHECK-NEXT:          - VERSION_0
+# CHECK-NEXT:      - Flags:      2
+# CHECK-NEXT:        VersionNdx: 2
+# CHECK-NEXT:        Hash:       108387921
+# CHECK-NEXT:        Names:
+# CHECK-NEXT:          - VERSION_1
+# CHECK-NEXT:      - Flags:      3
+# CHECK-NEXT:        VersionNdx: 3
+# CHECK-NEXT:        Hash:       108387922
+# CHECK-NEXT:        Names:
+# CHECK-NEXT:          - VERSION_2
+# CHECK-NEXT:          - VERSION_3
+# CHECK-NEXT:          - VERSION_4
+# CHECK-NEXT:  - Name:
 
 --- !ELF
 FileHeader:
   Class: ELFCLASS64
   Data:  ELFDATA2LSB
   Type:  ET_DYN
-  Entry: 0x1000
 Sections:
   - Name:            .gnu.version_d
     Type:            SHT_GNU_verdef
     Flags:           [ SHF_ALLOC ]
     Address:         0x230
     Link:            .dynstr
     AddressAlign:    0x4
     Info:            0x4
     Entries:
-      - Version:         1
-        Flags:           1
-        VersionNdx:      1
-        Hash:            170240160
+## An entry that has all fields explicitly set to their default values.
+## Used to check that we don't dump them.
+      - Version:    [[VERSION=1]]
+        Flags:      0
+        VersionNdx: 0
+        Hash:       0
         Names:
-          - dso.so.0
-      - Version:         1
-        Flags:           2
-        VersionNdx:      2
-        Hash:            108387921
+          - VERSION_0
+## An entry with arbitrary values.
+      - Flags:      2
+        VersionNdx: 2
+        Hash:       108387921
         Names:
           - VERSION_1
-      - Version:         1
-        Flags:           3
-        VersionNdx:      3
-        Hash:            108387922
+## Another entry with arbitrary values and version predecessors.
+      - Flags:      3
+        VersionNdx: 3
+        Hash:       108387922
         Names:
           - VERSION_2
           - VERSION_3
-DynamicSymbols:
-  - Name:    foo
-    Binding: STB_GLOBAL
-...
+          - VERSION_4
+## Needed to emit the .dynstr section.
+DynamicSymbols: []
+
+## Document that we are not able to dump a version definition which
+## has a version revision (vd_version) that is not equal to 1.
+
+# RUN: yaml2obj %s -DVERSION=2 -o %t.version
+# RUN: not obj2yaml %t.version 2>&1 | \
+# RUN:   FileCheck %s -DFILE=%t.version --check-prefix=VERSION-ERR
+
+# VERSION-ERR: Error reading file: [[FILE]]: invalid SHT_GNU_verdef section version: 2

llvm/test/tools/yaml2obj/ELF/verdef-section.yaml

-## Check we are able to handle SHT_GNU_verdef sections.
+## Test how we create SHT_GNU_verdef sections.
 
 # RUN: yaml2obj --docnum=1 %s -o %t1
 # RUN: llvm-readobj -V %t1 | FileCheck %s
 
 # CHECK:      VersionDefinitions [
 # CHECK-NEXT:   Definition {
 # CHECK-NEXT:     Version: 1
+# CHECK-NEXT:     Flags [ (0x0)
+# CHECK-NEXT:     ]
+# CHECK-NEXT:     Index: 0
+# CHECK-NEXT:     Hash: 0
+# CHECK-NEXT:     Name:
+# CHECK-NEXT:     Predecessors: []
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Definition {
+# CHECK-NEXT:     Version: 1
 # CHECK-NEXT:     Flags [ (0x1)
 # CHECK-NEXT:       Base (0x1)
 # CHECK-NEXT:     ]
 # CHECK-NEXT:     Index: 1
 # CHECK-NEXT:     Hash: 170240160
 # CHECK-NEXT:     Name: dso.so.0
 # CHECK-NEXT:     Predecessors: []
 # CHECK-NEXT:   }
 # CHECK-NEXT:   Definition {
 # CHECK-NEXT:     Version: 1
 # CHECK-NEXT:     Flags [ (0x2)
 # CHECK-NEXT:       Weak (0x2)
 # CHECK-NEXT:     ]
 # CHECK-NEXT:     Index: 2
 # CHECK-NEXT:     Hash: 108387921
 # CHECK-NEXT:     Name: VERSION_1
 # CHECK-NEXT:     Predecessors: []
 # CHECK-NEXT:   }
 # CHECK-NEXT:   Definition {
 # CHECK-NEXT:     Version: 1
-# CHECK-NEXT:     Flags [ (0x3)
+# CHECK-NEXT:     Flags [ (0xFFFF)
 # CHECK-NEXT:       Base (0x1)
+# CHECK-NEXT:       Info (0x4)
 # CHECK-NEXT:       Weak (0x2)
 # CHECK-NEXT:     ]
 # CHECK-NEXT:     Index: 3
 # CHECK-NEXT:     Hash: 108387922
 # CHECK-NEXT:     Name: VERSION_2
-# CHECK-NEXT:     Predecessors: [VERSION_3]
+# CHECK-NEXT:     Predecessors: [VERSION_3, VERSION_4]
 # CHECK-NEXT:   }
 # CHECK-NEXT: ]
 
 --- !ELF
 FileHeader:
   Class: ELFCLASS64
   Data:  ELFDATA2LSB
   Type:  ET_DYN
-  Entry: 0x0000000000001000
 Sections:
-  - Name:            .gnu.version_d
-    Type:            SHT_GNU_verdef
-    Flags:           [ SHF_ALLOC ]
-    Address:         0x0000000000000230
-    Link:            .dynstr
-    AddressAlign:    0x0000000000000004
-    Info:            0x0000000000000003
+  - Name:  .gnu.version_d
+    Type:  SHT_GNU_verdef
+    Flags: [ SHF_ALLOC ]
+    Link:  .dynstr
+    Info:  0x4
     Entries:
-      - Version:         1
-        Flags:           1
-        VersionNdx:      1
-        Hash:            170240160
+## Case 1: an entry that has no Version, Flags, VersionNdx or Hash fields set.
+##         Used to check values that are written by default. Also shows
+##         that we are able to use the "=<none>" syntax for these fields.
+      - Version:    [[VERSION=<none>]]
+        Flags:      [[FLAGS=<none>]]
+        VersionNdx: [[VERNDX=<none>]]
+        Hash:       [[HASH=<none>]]
+        Names: []
+## Case 2: an arbitrary entry.
+      - Flags:      1
+        VersionNdx: 1
+        Hash:       170240160
         Names:
           - dso.so.0
-      - Version:         1
-        Flags:           2
-        VersionNdx:      2
-        Hash:            108387921
+## Case 3: one more arbitrary entry with different values.
+      - Flags:      2
+        VersionNdx: 2
+        Hash:       108387921
         Names:
           - VERSION_1
-      - Version:         1
-        Flags:           3
-        VersionNdx:      3
-        Hash:            108387922
+## Case 4: an entry that has version predecessors. Also, it sets
+##         all known flags as well as few unknown.
+      - Flags:      0xffff
+        VersionNdx: 3
+        Hash:       108387922
         Names:
           - VERSION_2
           - VERSION_3
+          - VERSION_4
 DynamicSymbols:
   - Name:    foo
     Binding: STB_GLOBAL
 
+## Check we are able to emit a version definition which has a version revision
+## (vd_version) field value that is not equal to 1.
+
+# RUN: yaml2obj --docnum=1 -DVERSION=2 %s -o %t.version
+# RUN: llvm-readobj -V %t.version 2>&1 | FileCheck %s --check-prefix=VERSION-ERR
+
+# VERSION-ERR: unable to dump SHT_GNU_verdef section with index 1: version 2 is not yet supported
+
 ## Check we can use "Content" to describe the content.
 
 # RUN: yaml2obj --docnum=2 %s -o %t2
 # RUN: llvm-readobj --sections --section-data %t2 | FileCheck %s --check-prefix=CONTENT
 
 # CONTENT:      Name: .gnu.version_d
 # CONTENT-NEXT: Type: SHT_GNU_verdef
 # CONTENT-NEXT: Flags [ (0x2)

llvm/tools/obj2yaml/elf2yaml.cpp

ELFDumper<ELFT>::dumpVerdefSection(const Elf_Shdr *Shdr) {
 
   S->Entries.emplace();
 
   llvm::ArrayRef<uint8_t> Data = *Contents;
   const uint8_t *Buf = Data.data();
   while (Buf) {
     const Elf_Verdef *Verdef = reinterpret_cast<const Elf_Verdef *>(Buf);
     ELFYAML::VerdefEntry Entry;
-    Entry.Version = Verdef->vd_version;
+    if (Verdef->vd_version != 1)
+      return createStringError(errc::invalid_argument,
+                               "invalid SHT_GNU_verdef section version: " +
+                                   Twine(Verdef->vd_version));
+
+    if (Verdef->vd_flags != 0)
       Entry.Flags = Verdef->vd_flags;
+
+    if (Verdef->vd_ndx != 0)
       Entry.VersionNdx = Verdef->vd_ndx;
+
+    if (Verdef->vd_hash != 0)
       Entry.Hash = Verdef->vd_hash;
 
     const uint8_t *BufAux = Buf + Verdef->vd_aux;
     while (BufAux) {
       const Elf_Verdaux *Verdaux =
           reinterpret_cast<const Elf_Verdaux *>(BufAux);
       Entry.VerNames.push_back(
           StringTableOrErr->drop_front(Verdaux->vda_name).data());
       BufAux = Verdaux->vda_next ? BufAux + Verdaux->vda_next : nullptr;