[obj2yaml,yaml2obj] - Refine how we set/dump the sh_entsize field.

This reuses the code from yaml2obj (moves it to ELFYAML.h).
With it we can set the `sh_entsize` in a single place in `obj2yaml`.

Note that it also fixes a bug of `yaml2obj`: we do not
set the `sh_entsize` field for the `SHT_ARM_EXIDX` section properly.

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

llvm/include/llvm/ObjectYAML/ELFYAML.h

@@ -16,6 +16,8 @@
 #define LLVM_OBJECTYAML_ELFYAML_H
 
 #include "llvm/ADT/StringRef.h"
+#include "llvm/BinaryFormat/ELF.h"
+#include "llvm/Object/ELFTypes.h"
 #include "llvm/ObjectYAML/DWARFYAML.h"
 #include "llvm/ObjectYAML/YAML.h"
 #include "llvm/Support/YAMLTraits.h"
@@ -69,6 +71,38 @@ LLVM_YAML_STRONG_TYPEDEF(uint32_t, MIPS_ISA)
 LLVM_YAML_STRONG_TYPEDEF(StringRef, YAMLFlowString)
 LLVM_YAML_STRONG_TYPEDEF(int64_t, YAMLIntUInt)
 
+template <class ELFT>
+unsigned getDefaultShEntSize(unsigned EMachine, ELF_SHT SecType,
+                             StringRef SecName) {
+  if (EMachine == ELF::EM_MIPS && SecType == ELF::SHT_MIPS_ABIFLAGS)
+    return sizeof(object::Elf_Mips_ABIFlags<ELFT>);
+
+  switch (SecType) {
+  case ELF::SHT_GROUP:
+    return sizeof(typename ELFT::Word);
+  case ELF::SHT_REL:
+    return sizeof(typename ELFT::Rel);
+  case ELF::SHT_RELA:
+    return sizeof(typename ELFT::Rela);
+  case ELF::SHT_RELR:
+    return sizeof(typename ELFT::Relr);
+  case ELF::SHT_DYNAMIC:
+    return sizeof(typename ELFT::Dyn);
+  case ELF::SHT_HASH:
+    return sizeof(typename ELFT::Word);
+  case ELF::SHT_SYMTAB_SHNDX:
+    return sizeof(typename ELFT::Word);
+  case ELF::SHT_GNU_versym:
+    return sizeof(typename ELFT::Half);
+  case ELF::SHT_LLVM_CALL_GRAPH_PROFILE:
+    return sizeof(object::Elf_CGProfile_Impl<ELFT>);
+  default:
+    if (SecName == ".debug_str")
+      return 1;
+    return 0;
+  }
+}
+
 // For now, hardcode 64 bits everywhere that 32 or 64 would be needed
 // since 64-bit can hold 32-bit values too.
 struct FileHeader {

llvm/lib/ObjectYAML/ELFEmitter.cpp

@@ -704,6 +704,12 @@ void ELFState<ELFT>::initSectionHeaders(std::vector<Elf_Shdr> &SHeaders,
     if (Sec->Link)
       SHeader.sh_link = toSectionIndex(*Sec->Link, Sec->Name);
 
+    if (Sec->EntSize)
+      SHeader.sh_entsize = *Sec->EntSize;
+    else
+      SHeader.sh_entsize = ELFYAML::getDefaultShEntSize<ELFT>(
+          Doc.Header.Machine.getValueOr(ELF::EM_NONE), Sec->Type, Sec->Name);
+
     if (IsFirstUndefSection) {
       if (auto RawSec = dyn_cast<ELFYAML::RawContentSection>(Sec)) {
         // We do not write any content for special SHN_UNDEF section.
@@ -712,8 +718,6 @@ void ELFState<ELFT>::initSectionHeaders(std::vector<Elf_Shdr> &SHeaders,
         if (RawSec->Info)
           SHeader.sh_info = *RawSec->Info;
       }
-      if (Sec->EntSize)
-        SHeader.sh_entsize = *Sec->EntSize;
 
       LocationCounter += SHeader.sh_size;
       overrideFields<ELFT>(Sec, SHeader);
@@ -1161,9 +1165,6 @@ template <class ELFT>
 void ELFState<ELFT>::writeSectionContent(
     Elf_Shdr &SHeader, const ELFYAML::RawContentSection &Section,
     ContiguousBlobAccumulator &CBA) {
-  if (Section.EntSize)
-    SHeader.sh_entsize = *Section.EntSize;
-
   if (Section.Info)
     SHeader.sh_info = *Section.Info;
 }
@@ -1182,12 +1183,6 @@ void ELFState<ELFT>::writeSectionContent(
           Section.Type == llvm::ELF::SHT_RELA) &&
          "Section type is not SHT_REL nor SHT_RELA");
 
-  bool IsRela = Section.Type == llvm::ELF::SHT_RELA;
-  if (Section.EntSize)
-    SHeader.sh_entsize = *Section.EntSize;
-  else
-    SHeader.sh_entsize = IsRela ? sizeof(Elf_Rela) : sizeof(Elf_Rel);
-
   // For relocation section set link to .symtab by default.
   unsigned Link = 0;
   if (!Section.Link && !ExcludedSectionHeaders.count(".symtab") &&
@@ -1200,6 +1195,7 @@ void ELFState<ELFT>::writeSectionContent(
   if (!Section.Relocations)
     return;
 
+  const bool IsRela = Section.Type == llvm::ELF::SHT_RELA;
   for (const ELFYAML::Relocation &Rel : *Section.Relocations) {
     const bool IsDynamic = Section.Link && (*Section.Link == ".dynsym");
     unsigned SymIdx =
@@ -1228,9 +1224,6 @@ template <class ELFT>
 void ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
                                          const ELFYAML::RelrSection &Section,
                                          ContiguousBlobAccumulator &CBA) {
-  SHeader.sh_entsize =
-      Section.EntSize ? uint64_t(*Section.EntSize) : sizeof(Elf_Relr);
-
   if (!Section.Entries)
     return;
 
@@ -1248,8 +1241,6 @@ template <class ELFT>
 void ELFState<ELFT>::writeSectionContent(
     Elf_Shdr &SHeader, const ELFYAML::SymtabShndxSection &Shndx,
     ContiguousBlobAccumulator &CBA) {
-  SHeader.sh_entsize = Shndx.EntSize ? (uint64_t)*Shndx.EntSize : 4;
-
   if (Shndx.Content || Shndx.Size) {
     SHeader.sh_size = writeContent(CBA, Shndx.Content, Shndx.Size);
     return;
@@ -1275,11 +1266,6 @@ void ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
       SN2I.lookup(".symtab", Link))
     SHeader.sh_link = Link;
 
-  if (Section.EntSize)
-    SHeader.sh_entsize = *Section.EntSize;
-  else
-    SHeader.sh_entsize = sizeof(typename ELFT::Word);
-
   if (Section.Signature)
     SHeader.sh_info =
         toSymbolIndex(*Section.Signature, Section.Name, /*IsDynamic=*/false);
@@ -1302,8 +1288,6 @@ template <class ELFT>
 void ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
                                          const ELFYAML::SymverSection &Section,
                                          ContiguousBlobAccumulator &CBA) {
-  SHeader.sh_entsize = Section.EntSize ? (uint64_t)*Section.EntSize : 2;
-
   if (!Section.Entries)
     return;
 
@@ -1409,11 +1393,6 @@ template <class ELFT>
 void ELFState<ELFT>::writeSectionContent(
     Elf_Shdr &SHeader, const ELFYAML::CallGraphProfileSection &Section,
     ContiguousBlobAccumulator &CBA) {
-  if (Section.EntSize)
-    SHeader.sh_entsize = *Section.EntSize;
-  else
-    SHeader.sh_entsize = 16;
-
   unsigned Link = 0;
   if (!Section.Link && !ExcludedSectionHeaders.count(".symtab") &&
       SN2I.lookup(".symtab", Link))
@@ -1442,11 +1421,6 @@ void ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
       SN2I.lookup(".dynsym", Link))
     SHeader.sh_link = Link;
 
-  if (Section.EntSize)
-    SHeader.sh_entsize = *Section.EntSize;
-  else
-    SHeader.sh_entsize = sizeof(typename ELFT::Word);
-
   if (!Section.Bucket)
     return;
 
@@ -1578,7 +1552,6 @@ void ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
 
   object::Elf_Mips_ABIFlags<ELFT> Flags;
   zero(Flags);
-  SHeader.sh_entsize = sizeof(Flags);
   SHeader.sh_size = SHeader.sh_entsize;
 
   Flags.version = Section.Version;
@@ -1602,11 +1575,6 @@ void ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
   assert(Section.Type == llvm::ELF::SHT_DYNAMIC &&
          "Section type is not SHT_DYNAMIC");
 
-  if (Section.EntSize)
-    SHeader.sh_entsize = *Section.EntSize;
-  else
-    SHeader.sh_entsize = 2 * sizeof(uintX_t);
-
   if (!Section.Entries)
     return;
 

llvm/test/Object/obj2yaml.test

@@ -385,6 +385,7 @@
 # ELF-MIPSEL-NEXT:     Type:            SHT_MIPS_ABIFLAGS
 # ELF-MIPSEL-NEXT:     Flags:           [ SHF_ALLOC ]
 # ELF-MIPSEL-NEXT:     AddressAlign:    0x8
+# ELF-MIPSEL-NEXT:     EntSize:         0x0
 # ELF-MIPSEL-NEXT:     ISA:             MIPS32
 # ELF-MIPSEL-NEXT:     ISARevision:     0x1
 # ELF-MIPSEL-NEXT:     FpABI:           FP_DOUBLE

llvm/test/tools/obj2yaml/ELF/call-graph-profile-section.yaml

@@ -15,7 +15,6 @@
 # BASIC-NEXT:   - Name: .llvm.call-graph-profile
 # BASIC-NEXT:     Type: SHT_LLVM_CALL_GRAPH_PROFILE
 # BASIC-NEXT:     Link: .symtab
-# BASIC-NEXT:     EntSize: 0x10
 # BASIC-NEXT:     Entries:
 # BASIC-NEXT:       - From:   foo
 # BASIC-NEXT:         To:     bar
@@ -58,11 +57,9 @@ Symbols:
 # INVALID-NEXT:   - Name: .empty
 # INVALID-NEXT:     Type: SHT_LLVM_CALL_GRAPH_PROFILE
 # INVALID-NEXT:     Link: .symtab
-# INVALID-NEXT:     EntSize: 0x10
 # INVALID-NEXT:   - Name: .multiple.16.valid
 # INVALID-NEXT:     Type: SHT_LLVM_CALL_GRAPH_PROFILE
 # INVALID-NEXT:     Link: .symtab
-# INVALID-NEXT:     EntSize: 0x10
 # INVALID-NEXT:     Entries:
 # INVALID-NEXT:       - From:   foo
 # INVALID-NEXT:         To:     bar
@@ -70,43 +67,37 @@ Symbols:
 # INVALID-NEXT:   - Name: .non.multiple.16
 # INVALID-NEXT:     Type: SHT_LLVM_CALL_GRAPH_PROFILE
 # INVALID-NEXT:     Link: .symtab
-# INVALID-NEXT:     EntSize: 0x10
 # INVALID-NEXT:     Content: '0000000100000002000000000000000300'
 # INVALID-NEXT:   - Name: .multiple.16.invalid
 # INVALID-NEXT:     Type: SHT_LLVM_CALL_GRAPH_PROFILE
 # INVALID-NEXT:     Link: .symtab
-# INVALID-NEXT:     EntSize: 0x10
 # INVALID-NEXT:     Content: 00112233445566778899AABBCCDDEEFF
 # INVALID-NEXT:   - Name: .unknown.symbol.1
 # INVALID-NEXT:     Type: SHT_LLVM_CALL_GRAPH_PROFILE
 # INVALID-NEXT:     Link: .symtab
-# INVALID-NEXT:     EntSize: 0x10
 # INVALID-NEXT:     Content: 000000FF000000020000000000000003
 # INVALID-NEXT:   - Name: .unknown.symbol.2
 # INVALID-NEXT:     Type: SHT_LLVM_CALL_GRAPH_PROFILE
 # INVALID-NEXT:     Link: .symtab
-# INVALID-NEXT:     EntSize: 0x10
 # INVALID-NEXT:     Content: 00000001000000FF0000000000000003
 # INVALID-NEXT:   - Name: .link.to.symtable
 # INVALID-NEXT:     Type: SHT_LLVM_CALL_GRAPH_PROFILE
 # INVALID-NEXT:     Link: .symtab
-# INVALID-NEXT:     EntSize: 0x10
 # INVALID-NEXT:     Entries:
 # INVALID-NEXT:       - From:   foo
 # INVALID-NEXT:         To:     bar
 # INVALID-NEXT:         Weight: 0
 # INVALID-NEXT:   - Name: .link.to.non.symtable.1
 # INVALID-NEXT:     Type: SHT_LLVM_CALL_GRAPH_PROFILE
-# INVALID-NEXT:     EntSize: 0x10
 # INVALID-NEXT:     Content: '00000001000000020000000000000000'
 # INVALID-NEXT:   - Name: .link.to.non.symtable.2
 # INVALID-NEXT:     Type: SHT_LLVM_CALL_GRAPH_PROFILE
 # INVALID-NEXT:     Link: .empty
-# INVALID-NEXT:     EntSize: 0x10
 # INVALID-NEXT:     Content: '00000001000000020000000000000000'
 # INVALID-NEXT:   - Name:    .zero.entry.size
 # INVALID-NEXT:     Type:    SHT_LLVM_CALL_GRAPH_PROFILE
 # INVALID-NEXT:     Link:    .symtab
+# INVALID-NEXT:     EntSize: 0x0
 # INVALID-NEXT:     Entries:
 # INVALID-NEXT:       - From:   foo
 # INVALID-NEXT:         To:     bar
@@ -184,6 +175,7 @@ Sections:
         To:     2
         Weight: 0
 ## Case 5: Check we can dump a section that has a sh_entsize that is not a multiple of 16.
+##         Check that in these cases we print the "EntSize" key.
   - Name: .zero.entry.size
     Type: SHT_LLVM_CALL_GRAPH_PROFILE
     EntSize: 0

llvm/test/tools/obj2yaml/ELF/mips-abi-flags.yaml

@@ -7,7 +7,6 @@
 # CHECK-NEXT:   - Name:         .MIPS.abiflags
 # CHECK-NEXT:     Type:         SHT_MIPS_ABIFLAGS
 # CHECK-NEXT:     AddressAlign: 0x8
-# CHECK-NEXT:     EntSize:      0x18
 # CHECK-NEXT:     ISA:          MIPS64
 # CHECK-NEXT:     ISARevision:  0x5
 # CHECK-NEXT:     ISAExtension: EXT_OCTEON3
@@ -52,11 +51,16 @@ Sections:
 
 # MIPS:      - Name: .MIPS.abiflags
 # MIPS-NEXT:   Type: SHT_MIPS_ABIFLAGS
+## We don't print the "EntSize" key, because 0x18 is the default value
+## for the sh_entsize field of SHT_MIPS_ABIFLAGS.
 # MIPS-NEXT:   ISA:  0x0
 # MIPS-NEXT: ...
 
 # NOT-MIPS:      - Name:    .MIPS.abiflags
 # NOT-MIPS-NEXT:   Type:    0x7000002A
+## We print the "EntSize" key, because 0x7000002A only means
+## "SHT_MIPS_ABIFLAGS" when machine is EM_MIPS, so 0x18 is not a known default value.
+# NOT-MIPS-NEXT:   EntSize: 0x18
 # NOT-MIPS-NEXT:   Content: '000000000000000000000000000000000000000000000000'
 # NOT-MIPS-NEXT: ...
 
@@ -71,3 +75,13 @@ Sections:
     Type:    SHT_PROGBITS
     ShType:  0x7000002a ## SHT_MIPS_ABIFLAGS.
     Size:    0x18
+    EntSize: [[ENTSIZE=0x18]]
+
+## Check we dump the "EntSize" key when the value of the "sh_entsize" field is not equal to 0x18.
+
+# RUN: yaml2obj %s --docnum=2 -DMACHINE=EM_MIPS -DENTSIZE=0x19 -o %t2.mips.entsize
+# RUN: obj2yaml %t2.mips.entsize | FileCheck %s --check-prefix=MIPS-ENTSIZE
+
+# MIPS-ENTSIZE:      - Name:    .MIPS.abiflags
+# MIPS-ENTSIZE-NEXT:   Type:    SHT_MIPS_ABIFLAGS
+# MIPS-ENTSIZE-NEXT:   EntSize: 0x19

llvm/test/tools/obj2yaml/ELF/sht-symtab-shndx.yaml

@@ -16,7 +16,6 @@
 # CASE1-NEXT:   - Name: .symtab_shndx
 # CASE1-NEXT:     Type: SHT_SYMTAB_SHNDX
 # CASE1-NEXT:     Link: .symtab
-# CASE1-NEXT:     EntSize: 0x4
 # CASE1-NEXT:     Entries: [ 0, 1, 2 ]
 # CASE1-NEXT: Symbols:
 # CASE1-NEXT:   - Name:  bar
@@ -186,3 +185,36 @@ Sections:
     Link:    .foo
   - Name:    .foo
     Type:    SHT_PROGBITS
+
+## Check that we are unable to dump the SHT_SYMTAB_SHNDX section when its
+## sh_entsize has an unexpected value (not equal to 4).
+
+# RUN: yaml2obj -DENTSIZE=0xff --docnum=7 %s -o %t7.entsize.bad
+# RUN: not obj2yaml %t7.entsize.bad 2>&1 | \
+# RUN:   FileCheck %s -DFILE=%t7.entsize.bad --check-prefix=CASE7-BAD
+
+# CASE7-BAD: Error reading file: [[FILE]]: unable to read extended section indexes: section [index 1] has invalid sh_entsize: expected 4, but got 255
+
+## Check we don't print the "EntSize" field when the "sh_entsize" field has the default value.
+
+# RUN: yaml2obj -DENTSIZE=4 --docnum=7 %s -o %t7.entsize.ok
+# RUN: obj2yaml %t7.entsize.ok | FileCheck %s -DFILE=%t7.entsize.ok --check-prefix=CASE7
+
+# CASE7:      - Name:    .symtab_shndx
+# CASE7-NEXT:   Type:    SHT_SYMTAB_SHNDX
+# CASE7-NEXT:   Link:    .symtab
+# CASE7-NEXT:   Entries: [ 0 ]
+# CASE7-NEXT: Symbols:
+
+--- !ELF
+FileHeader:
+  Class: ELFCLASS64
+  Data:  ELFDATA2LSB
+  Type:  ET_REL
+Sections:
+  - Name:    .symtab_shndx
+    Type:    SHT_SYMTAB_SHNDX
+    EntSize: [[ENTSIZE]]
+    Entries: [ 0 ]
+    Link:    .symtab
+Symbols: []

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

@@ -16,7 +16,6 @@
 # CHECK-NEXT:     Address:      0x200210
 # CHECK-NEXT:     Link:         .dynsym
 # CHECK-NEXT:     AddressAlign: 0x2
-# CHECK-NEXT:     EntSize:         0x2
 # CHECK-NEXT:     Entries:      [ 0, 3, 4 ]
 # CHECK-NEXT:   - Name:
 # CHECK:      DynamicSymbols:

llvm/test/tools/yaml2obj/ELF/arm-exidx-section.yaml

@@ -56,7 +56,7 @@ Sections:
 # RUN: llvm-readelf --sections %t.props.o | FileCheck %s --check-prefix=PROPERTIES
 
 # PROPERTIES: [Nr] Name       Type      Address          Off    Size   ES Flg Lk    Inf Al
-# PROPERTIES: [ 1] .ARM.exidx ARM_EXIDX 0000000000001122 000055 000000 00 AMS 13124 0   85
+# PROPERTIES: [ 1] .ARM.exidx ARM_EXIDX 0000000000001122 000055 000000 66 AMS 13124 0   85
 
 --- !ELF
 FileHeader:

llvm/tools/obj2yaml/elf2yaml.cpp

@@ -730,29 +730,6 @@ Error ELFDumper<ELFT>::dumpRelocation(const RelT *Rel, const Elf_Shdr *SymTab,
   return Error::success();
 }
 
-template <class ELFT>
-static unsigned getDefaultShEntSize(ELFYAML::ELF_SHT SecType,
-                                    StringRef SecName) {
-  switch (SecType) {
-  case ELF::SHT_GROUP:
-    return sizeof(typename ELFT::Word);
-  case ELF::SHT_REL:
-    return sizeof(typename ELFT::Rel);
-  case ELF::SHT_RELA:
-    return sizeof(typename ELFT::Rela);
-  case ELF::SHT_RELR:
-    return sizeof(typename ELFT::Relr);
-  case ELF::SHT_DYNAMIC:
-    return sizeof(typename ELFT::Dyn);
-  case ELF::SHT_HASH:
-    return sizeof(typename ELFT::Word);
-  default:
-    if (SecName == ".debug_str")
-      return 1;
-    return 0;
-  }
-}
-
 template <class ELFT>
 Error ELFDumper<ELFT>::dumpCommonSection(const Elf_Shdr *Shdr,
                                          ELFYAML::Section &S) {
@@ -772,7 +749,8 @@ Error ELFDumper<ELFT>::dumpCommonSection(const Elf_Shdr *Shdr,
     return NameOrErr.takeError();
   S.Name = NameOrErr.get();
 
-  if (Shdr->sh_entsize != getDefaultShEntSize<ELFT>(S.Type, S.Name))
+  if (Shdr->sh_entsize != ELFYAML::getDefaultShEntSize<ELFT>(
+                              Obj.getHeader().e_machine, S.Type, S.Name))
     S.EntSize = static_cast<llvm::yaml::Hex64>(Shdr->sh_entsize);
 
   if (Shdr->sh_link != ELF::SHN_UNDEF) {