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

We already set the `sh_entsize` field in a single place
for all non-implicit sections.

This patch reorders the logic slightly and with it
we finally have the only one place where the `sh_entsize` is set.

obj2yaml will not dump the `EntSize` key for `SHT_DYNSYM/SHT_SYMTAB` sections anymore,
when the value of `sh_entsize` is equal to `sizeof(Elf_Sym)`

Note that this also seems revealed an issue in llvm-objcopy:
Previously yaml2obj set the `sh_entsize` for the `.symtab` section to 0x18,
now we it sets it for `SHT_SYMTAB` sections, i.e. by type.
But the `llvm-objcopy/ELF/only-keep-debug.test` has a `.symtab` section of type `SHT_STRTAB`,
and now yaml2obj sets the `sh_entsize` to 0 for it.
I had to update the corresponding check lines for `ES`, but the behavior of
`llvm-objcopy` should be fixed instead I think.
I've added a TODO and a comment.

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

llvm/include/llvm/ObjectYAML/ELFYAML.h

@@ -78,6 +78,9 @@ unsigned getDefaultShEntSize(unsigned EMachine, ELF_SHT SecType,
     return sizeof(object::Elf_Mips_ABIFlags<ELFT>);
 
   switch (SecType) {
+  case ELF::SHT_SYMTAB:
+  case ELF::SHT_DYNSYM:
+    return sizeof(typename ELFT::Sym);
   case ELF::SHT_GROUP:
     return sizeof(typename ELFT::Word);
   case ELF::SHT_REL:

llvm/lib/ObjectYAML/ELFEmitter.cpp

@@ -762,6 +762,12 @@ void ELFState<ELFT>::initSectionHeaders(std::vector<Elf_Shdr> &SHeaders,
         SHeader.sh_link = Link;
     }
 
+    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);
+
     // We have a few sections like string or symbol tables that are usually
     // added implicitly to the end. However, if they are explicitly specified
     // in the YAML, we need to write them here. This ensures the file offset
@@ -787,12 +793,6 @@ void ELFState<ELFT>::initSectionHeaders(std::vector<Elf_Shdr> &SHeaders,
 
     assignSectionAddress(SHeader, Sec);
 
-    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.
@@ -970,9 +970,6 @@ void ELFState<ELFT>::initSymtabSectionHeader(Elf_Shdr &SHeader,
   // then we should set the fields requested.
   SHeader.sh_info = (RawSec && RawSec->Info) ? (unsigned)(*RawSec->Info)
                                              : findFirstNonGlobal(Symbols) + 1;
-  SHeader.sh_entsize = (YAMLSec && YAMLSec->EntSize)
-                           ? (uint64_t)(*YAMLSec->EntSize)
-                           : sizeof(Elf_Sym);
   SHeader.sh_addralign = YAMLSec ? (uint64_t)YAMLSec->AddressAlign : 8;
 
   assignSectionAddress(SHeader, YAMLSec);
@@ -1015,9 +1012,6 @@ void ELFState<ELFT>::initStrtabSectionHeader(Elf_Shdr &SHeader, StringRef Name,
     SHeader.sh_size = STB.getSize();
   }
 
-  if (YAMLSec && YAMLSec->EntSize)
-    SHeader.sh_entsize = *YAMLSec->EntSize;
-
   if (RawSec && RawSec->Info)
     SHeader.sh_info = *RawSec->Info;
 
@@ -1084,11 +1078,6 @@ void ELFState<ELFT>::initDWARFSectionHeader(Elf_Shdr &SHeader, StringRef Name,
     llvm_unreachable("debug sections can only be initialized via the 'DWARF' "
                      "entry or a RawContentSection");
 
-  if (YAMLSec && YAMLSec->EntSize)
-    SHeader.sh_entsize = *YAMLSec->EntSize;
-  else if (Name == ".debug_str")
-    SHeader.sh_entsize = 1;
-
   if (RawSec && RawSec->Info)
     SHeader.sh_info = *RawSec->Info;
 

llvm/test/tools/llvm-objcopy/ELF/only-keep-debug.test

@@ -188,7 +188,7 @@ ProgramHeaders:
 # CHECK3:      [Nr] Name          Type     Address          Off    Size   ES Flg Lk Inf Al
 # CHECK3:      [ 1] .dynsym       NOBITS   0000000000000000 000040 000018 18   A  2   1 1024
 # CHECK3-NEXT: [ 2] .dynstr       NOBITS   0000000000000018 000040 000001 00   A  0   0  0
-# CHECK3-NEXT: [ 3] .symtab       NOBITS   0000000000000019 000040 000018 18   A  4   1  0
+# CHECK3-NEXT: [ 3] .symtab       NOBITS   0000000000000019 000040 000018 00   A  4   1  0
 # CHECK3-NEXT: [ 4] .strtab       NOBITS   0000000000000031 000040 000001 00   A  0   0  0
 # CHECK3-NEXT: [ 5] .shstrtab     STRTAB   0000000000000000 000040 00002b 00      0   0  1
 
@@ -208,6 +208,8 @@ Sections:
     Type:  SHT_STRTAB
     Flags: [ SHF_ALLOC ]
   - Name:  .symtab
+## TODO: this should be SHT_SYMTAB, but currently llvm-objcopy reports an error:
+##       error: Symbol table has link index of 4 which is not a string table
     Type:  SHT_STRTAB
     Flags: [ SHF_ALLOC ]
     Link:  .strtab

llvm/test/tools/obj2yaml/ELF/entsize.yaml

@@ -1,12 +1,22 @@
+## Check how obj2yaml dumps the sh_entsize field.
+
+## Check we don't dump the "EntSize" key for SHT_SYMTAB/SHT_DYNSYM sections
+## when the value of sh_entsize is equal to sizeof(ELF_Sym) == 0x18.
+
 # RUN: yaml2obj %s -o %t
 # RUN: obj2yaml %t | FileCheck %s
 
-## Check obj2yaml is able to dump sh_entsize field of a section.
-
-# CHECK:      - Name:            .rodata.cst4
-# CHECK-NEXT:   Type:            SHT_PROGBITS
-# CHECK-NEXT:   Flags:           [ SHF_ALLOC, SHF_MERGE ]
-# CHECK-NEXT:   EntSize:         0x4
+# CHECK:       - Name:    .rodata.cst4
+# CHECK-NEXT:    Type:    SHT_PROGBITS
+# CHECK-NEXT:    EntSize: 0x4
+# CHECK-NEXT:  - Name: .mysymtab
+# CHECK-NEXT:    Type: SHT_SYMTAB
+# CHECK-NEXT:    Link: .strtab
+# CHECK-NEXT:    Size: 0x0
+# CHECK-NEXT:  - Name: .mydynsym
+# CHECK-NEXT:    Type: SHT_DYNSYM
+# CHECK-NEXT:    Size: 0x0
+# CHECK-NEXT: ...
 
 --- !ELF
 FileHeader:
@@ -14,7 +24,28 @@ FileHeader:
   Data:  ELFDATA2LSB
   Type:  ET_REL
 Sections:
-  - Name:            .rodata.cst4
-    Type:            SHT_PROGBITS
-    Flags:           [ SHF_ALLOC, SHF_MERGE ]
-    EntSize:         0x0000000000000004
+  - Name:    .rodata.cst4
+    Type:    SHT_PROGBITS
+    EntSize: 0x4
+  - Name:    .mysymtab
+    Type:    SHT_SYMTAB
+    EntSize: [[SYMTABES=0x18]]
+  - Name:    .mydynsym
+    Type:    SHT_DYNSYM
+    EntSize: [[DYNSYMES=0x18]]
+
+## Document that we are unable to dump a SHT_SYMTAB section when its entry size
+## is not equal to sizeof(ELF_Sym).
+
+# RUN: yaml2obj %s -DSYMTABES=0x19 -o %t2
+# RUN: not obj2yaml %t2 2>&1 | FileCheck %s -DFILE=%t2 --check-prefix=ERR1
+
+# ERR1: Error reading file: [[FILE]]: section [index 2] has invalid sh_entsize: expected 24, but got 25
+
+## Document that we are unable to dump a SHT_DYNSYM section when its entry size
+## is not equal to sizeof(ELF_Sym).
+
+# RUN: yaml2obj %s -DDYNSYMES=0x19 -o %t3
+# RUN: not obj2yaml %t3 2>&1 | FileCheck %s -DFILE=%t3 --check-prefix=ERR2
+
+# ERR2: Error reading file: [[FILE]]: section [index 3] has invalid sh_entsize: expected 24, but got 25

llvm/test/tools/obj2yaml/ELF/implicit-sections-order.yaml

@@ -45,7 +45,6 @@
 # OUTPUT-NEXT:     Flags:   [ SHF_ALLOC ]
 # OUTPUT-NEXT:     Address: 0x1000
 # OUTPUT-NEXT:     Link:    .dynstr
-# OUTPUT-NEXT:     EntSize: 0x18
 # OUTPUT-NEXT:   - Name: .foo.2
 # OUTPUT-NEXT:     Type: SHT_PROGBITS
 # OUTPUT-NEXT:   - Name:    .dynstr
@@ -138,7 +137,6 @@ DynamicSymbols:
 # OUTPUT2-NEXT:     Flags:   [ SHF_ALLOC ]
 # OUTPUT2-NEXT:     Address: 0x3000
 # OUTPUT2-NEXT:     Link:    .strtab
-# OUTPUT2-NEXT:     EntSize: 0x18
 # OUTPUT2-NEXT:   - Name: .foo.4
 # OUTPUT2-NEXT:     Type: SHT_PROGBITS
 # OUTPUT2-NEXT:   - Name:    .strtab

llvm/test/tools/obj2yaml/ELF/no-symtab.yaml

@@ -45,18 +45,14 @@ Symbols: []
 # RUN: obj2yaml %t3 | FileCheck %s --check-prefix=EMPTY
 
 # EMPTY:      Sections:
-# EMPTY-NEXT:   - Name:    .symtab
-# EMPTY-NEXT:     Type:    SHT_SYMTAB
-# EMPTY-NEXT:     Link:    .strtab
-## TODO: we shouldn't dump the default "EntSize" value.
-# EMPTY-NEXT:     EntSize: 0x18
-# EMPTY-NEXT:     Size:    0x0
-# EMPTY-NEXT:   - Name:    .dynsym
-# EMPTY-NEXT:     Type:    SHT_DYNSYM
-# EMPTY-NEXT:     Flags:   [ SHF_ALLOC ]
-## TODO: we shouldn't dump the default "EntSize" value.
-# EMPTY-NEXT:     EntSize: 0x18
-# EMPTY-NEXT:     Size:    0x0
+# EMPTY-NEXT:   - Name:  .symtab
+# EMPTY-NEXT:     Type:  SHT_SYMTAB
+# EMPTY-NEXT:     Link:  .strtab
+# EMPTY-NEXT:     Size:  0x0
+# EMPTY-NEXT:   - Name:  .dynsym
+# EMPTY-NEXT:     Type:  SHT_DYNSYM
+# EMPTY-NEXT:     Flags: [ SHF_ALLOC ]
+# EMPTY-NEXT:     Size:  0x0
 # EMPTY-NEXT: ...
 
 --- !ELF