[obj2yaml] - SHT_DYNAMIC and SHT_REL* sections: stop dumping sh_entsize field when it has the default value.

[grimar]

Summary

Currently obj2yaml always emits the EntSize property when sh_entsize != 0.
It is not correct. For example, for SHT_DYNAMIC section, EntSize == 0
is abnormal, while sizeof(ELFT::Dyn) is the expected default.

To reduce the output produces we should not dump default values.

yaml2obj tests that shows sh_entsize values produced are:

1. For SHT_REL* sections: yaml2obj\ELF\reloc-sec-entry-size.yaml
2. For SHT_DYNAMIC: yaml2obj\ELF\dynamic-section.yaml (See D76226)

[MaskRay]

Quote the ELF spec.

Some sections hold a table of fixed-size entries, such as a symbol table. For such a section, this member gives the size in bytes of each entry. The member contains 0 if the section does not hold a table of fixed-size entries.

For SHT_DYNAMIC and SHT_REL*, I think omitting sh_entsize is fine because they can be followed by entries. The following entries make it clear that they are tables of fixed-size entries.

If they could't be followed by entries, I would be nervous as this change added hidden knowledge.

Please hold off and wait for @jhenderson

[jhenderson]

Looks good to me. I think it would be expected that an unspecified EntSize means "the default" for a section.

I assume there are test cases that show that we do dump it when it is not the default for these kinds of sections?

llvm/test/tools/obj2yaml/dynamic-section.test

0–7

If you reflow this comment, you should be able to fit it onto 3 lines, I think.

llvm/test/tools/obj2yaml/relr-section.yaml

-4–1

Also, check we do not...

[grimar]

In D76227#1926119, @jhenderson wrote:

I assume there are test cases that show that we do dump it when it is not the default for these kinds of sections?

Nope :( There are two more patches requred for that. I am going to post them a bit later today and will hold this one until we land them.

llvm/test/Object/AArch64/yaml2obj-elf-aarch64-rel.yaml

 # RUN: yaml2obj %s -o %t
 # RUN: obj2yaml  %t | FileCheck %s
 
 # CHECK:      - Name:            .rela.text
 # CHECK-NEXT:   Type:            SHT_RELA
 # CHECK-NEXT:    Link:            .symtab
 # CHECK-NEXT:    AddressAlign:    0x0000000000000008
-# CHECK-NEXT:    EntSize:         0x0000000000000018
 # CHECK-NEXT:    Info:            .text
 # CHECK-NEXT:    Relocations:     
 # CHECK-NEXT:      - Symbol:          main
 # CHECK-NEXT:        Type:            R_AARCH64_ABS64
 # CHECK-NEXT:      - Offset:          0x0000000000000008
 # CHECK-NEXT:        Symbol:          main
 # CHECK-NEXT:        Type:            R_AARCH64_TLSGD_ADR_PREL21
 

llvm/test/Object/obj2yaml.test

 # ELF-MIPSEL-NEXT:     Type:            SHT_PROGBITS
 # ELF-MIPSEL-NEXT:     Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
 # ELF-MIPSEL-NEXT:     AddressAlign:    0x0000000000000004
 # ELF-MIPSEL-NEXT:     Content:         0000023C00004224E8FFBD271400BFAF1000B0AF218059000000018E000024240000198E09F8200321E000020000198E09F8200321E00002000002241000B08F1400BF8F0800E0031800BD27
 # ELF-MIPSEL-NEXT:   - Name:            .rel.text
 # ELF-MIPSEL-NEXT:     Type:            SHT_REL
 # ELF-MIPSEL-NEXT:     Link:            .symtab
 # ELF-MIPSEL-NEXT:     AddressAlign:    0x0000000000000004
-# ELF-MIPSEL-NEXT:     EntSize:         0x0000000000000008
 # ELF-MIPSEL-NEXT:     Info:            .text
 # ELF-MIPSEL-NEXT:     Relocations:
 # ELF-MIPSEL-NEXT:       - Symbol:          _gp_disp
 # ELF-MIPSEL-NEXT:         Type:            R_MIPS_HI16
 # ELF-MIPSEL-NEXT:       - Offset:          0x0000000000000004
 # ELF-MIPSEL-NEXT:         Symbol:          _gp_disp
 # ELF-MIPSEL-NEXT:         Type:            R_MIPS_LO16
 # ELF-MIPSEL-NEXT:       - Offset:          0x0000000000000018

 # ELF-MIPS64EL-NEXT:     Type:            SHT_PROGBITS
 # ELF-MIPS64EL-NEXT:     Flags:           [ SHF_WRITE, SHF_ALLOC ]
 # ELF-MIPS64EL-NEXT:     AddressAlign:    0x0000000000000010
 # ELF-MIPS64EL-NEXT:     Content:         '00000000000000000000000000000000'
 # ELF-MIPS64EL-NEXT:   - Name:            .rela.data
 # ELF-MIPS64EL-NEXT:     Type:            SHT_RELA
 # ELF-MIPS64EL-NEXT:     Link:            .symtab
 # ELF-MIPS64EL-NEXT:     AddressAlign:    0x0000000000000008
-# ELF-MIPS64EL-NEXT:     EntSize:         0x0000000000000018
 # ELF-MIPS64EL-NEXT:     Info:            .data
 # ELF-MIPS64EL-NEXT:     Relocations:
 # ELF-MIPS64EL-NEXT:       - Symbol:          zed
 # ELF-MIPS64EL-NEXT:         Type:            R_MIPS_64
 # ELF-MIPS64EL-NEXT:   - Name:            .bss
 # ELF-MIPS64EL-NEXT:     Type:            SHT_NOBITS
 # ELF-MIPS64EL-NEXT:     Flags:           [ SHF_WRITE, SHF_ALLOC ]
 # ELF-MIPS64EL-NEXT:     AddressAlign:    0x0000000000000010

 # ELF-X86-64-NEXT:     Type:            SHT_PROGBITS
 # ELF-X86-64-NEXT:     Address:         0x0000000000000033
 # ELF-X86-64-NEXT:     AddressAlign:    0x0000000000000001
 # ELF-X86-64-NEXT:   - Name:            .rela.text
 # ELF-X86-64-NEXT:     Type:            SHT_RELA
 # ELF-X86-64-NEXT:     Address:         0x0000000000000038
 # ELF-X86-64-NEXT:     Link:            .symtab
 # ELF-X86-64-NEXT:     AddressAlign:    0x0000000000000008
-# ELF-X86-64-NEXT:     EntSize:         0x0000000000000018
 # ELF-X86-64-NEXT:     Info:            .text
 # ELF-X86-64-NEXT:     Relocations:
 # ELF-X86-64-NEXT:       - Offset:          0x000000000000000D
 # ELF-X86-64-NEXT:         Symbol:          .rodata.str1.1
 # ELF-X86-64-NEXT:         Type:            R_X86_64_32S
 # ELF-X86-64-NEXT:       - Offset:          0x0000000000000012
 # ELF-X86-64-NEXT:         Symbol:          puts
 # ELF-X86-64-NEXT:         Type:            R_X86_64_PC32

llvm/test/tools/obj2yaml/dynamic-section.test

-## Check we can use obj2yaml to yamalize the object containing
-## .dynamic section. Check that resulting section has the
-## proper attributes and dynamic tags.
+## Check we can use obj2yaml to yamalize the object containing .dynamic
+## section. Check that resulting section has the proper attributes and
+## dynamic tags and that we do not dump the default sh_entsize.
 
 # RUN: yaml2obj -DENTSIZE=0x10 %s -o %t1
 # RUN: obj2yaml %t1 | FileCheck %s
 
 # CHECK:       Sections:
 # CHECK-NEXT:  - Name:            .dynamic
 # CHECK-NEXT:    Type:            SHT_DYNAMIC
 # CHECK-NEXT:    Address:         0x0000000000001000
 # CHECK-NEXT:    AddressAlign:    0x0000000000002000
-# CHECK-NEXT:    EntSize:         0x0000000000000010
 # CHECK-NEXT:    Entries:
 # CHECK-NEXT:      - Tag:             DT_NULL
 # CHECK-NEXT:        Value:           0x0000000000000000
 # CHECK-NEXT:      - Tag:             DT_NEEDED
 # CHECK-NEXT:        Value:           0x0000000000000001
 # CHECK-NEXT:      - Tag:             DT_PLTRELSZ
 # CHECK-NEXT:        Value:           0x0000000000000002
 # CHECK-NEXT:      - Tag:             DT_PLTGOT

llvm/test/tools/obj2yaml/elf-ppc64-relocations.yaml

 ## Check obj2yaml is able to decode all PPC64 relocations.
 
 # RUN: yaml2obj %s -o %t
 # RUN: obj2yaml %t | FileCheck %s
 
 # CHECK:      --- !ELF
 # CHECK-NEXT: FileHeader:
 # CHECK-NEXT:   Class:   ELFCLASS64
 # CHECK-NEXT:   Data:    ELFDATA2MSB
 # CHECK-NEXT:   Type:    ET_REL
 # CHECK-NEXT:   Machine: EM_PPC64
 # CHECK-NEXT: Sections:
-# CHECK-NEXT:   - Name:    .rela.text
-# CHECK-NEXT:     Type:    SHT_RELA
-# CHECK-NEXT:     EntSize: 0x0000000000000018
+# CHECK-NEXT:   - Name: .rela.text
+# CHECK-NEXT:     Type: SHT_RELA
 # CHECK-NEXT:     Relocations:
 # CHECK-NEXT:       - Type: R_PPC64_NONE
 # CHECK-NEXT:       - Type: R_PPC64_ADDR32
 # CHECK-NEXT:       - Type: R_PPC64_ADDR24
 # CHECK-NEXT:       - Type: R_PPC64_ADDR16
 # CHECK-NEXT:       - Type: R_PPC64_ADDR16_LO
 # CHECK-NEXT:       - Type: R_PPC64_ADDR16_HI
 # CHECK-NEXT:       - Type: R_PPC64_ADDR16_HA

llvm/test/tools/obj2yaml/elf-reladyn-section-shinfo.yaml

+# RUN: yaml2obj %s -o %t
+# RUN: llvm-readobj --sections %t | FileCheck %s
+# RUN: obj2yaml %t | FileCheck %s --check-prefix=YAML
+
+## .rela.dyn is a dynamic relocation section that normally has
+## no value in sh_info field. Check we are able to use
+## yaml2obj/obj2yaml without needing to explicitly set it.
+
+# CHECK:      Name: .rela.dyn
+# CHECK-NEXT: Type: SHT_RELA
+# CHECK-NEXT: Flags [
+# CHECK-NEXT:   SHF_ALLOC
+# CHECK-NEXT: ]
+# CHECK-NEXT: Address:
+# CHECK-NEXT: Offset:
+# CHECK-NEXT: Size:
+# CHECK-NEXT: Link:
+# CHECK-NEXT: Info: 0
+# CHECK-NEXT: AddressAlignment:
+# CHECK-NEXT: EntrySize:
+
+# YAML:      - Name:  .rela.dyn
+# YAML-NEXT:   Type:  SHT_RELA
+# YAML-NEXT:   Flags: [ SHF_ALLOC ]
+# YAML-NEXT:   Link:  .dynsym
+# YAML-NEXT: - Name:
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_X86_64
+  Entry:   0x0000000000001000
+Sections:
+  - Name:    .rela.dyn
+    Type:    SHT_RELA
+    Flags:   [ SHF_ALLOC ]
+    Link:    .dynsym
+# Add at least one symbol to trigger the .dynsym emission.
+DynamicSymbols:
+  - Name:    bar
+    Binding: STB_GLOBAL

llvm/test/tools/obj2yaml/no-symbol-reloc.test

 # CHECK-NEXT:   Type:    ET_REL
 # CHECK-NEXT:   Machine: EM_X86_64
 # CHECK-NEXT: Sections:
 # CHECK-NEXT:   - Name:    .text
 # CHECK-NEXT:     Type:    SHT_PROGBITS
 # CHECK-NEXT:     Flags:   [ SHF_ALLOC, SHF_EXECINSTR ]
 # CHECK-NEXT:   - Name:    .rela.text
 # CHECK-NEXT:     Type:    SHT_RELA
-# CHECK-NEXT:     EntSize: 0x0000000000000018
 # CHECK-NEXT:     Info:    .text
 # CHECK-NEXT:     Relocations:
 # CHECK-NEXT:       - Type:   R_X86_64_NONE
 # CHECK-NEXT:       - Offset: 0x0000000000000004
 # CHECK-NEXT:         Type:   R_X86_64_NONE
 # CHECK-NEXT: ...
 
 --- !ELF

llvm/test/tools/obj2yaml/relocation-type.yaml

 ## Show how obj2yaml dumps relocation types.
 
 ## Show that obj2yaml is able to dump relocation types when `e_machine` kind is unknown.
 # RUN: yaml2obj %s -o %t1 -DMACHINE=0x1234
 # RUN: obj2yaml %t1 | FileCheck %s -DMACHINE=0x1234 -DFIRST=0x00000000 -DSECOND=0x00000001
 
 # CHECK:      --- !ELF
 # CHECK-NEXT: FileHeader:
 # CHECK-NEXT:   Class:   ELFCLASS64
 # CHECK-NEXT:   Data:    ELFDATA2MSB
 # CHECK-NEXT:   Type:    ET_REL
 # CHECK-NEXT:   Machine: [[MACHINE]]
 # CHECK-NEXT: Sections:
 # CHECK-NEXT:   - Name:    .rela.text
 # CHECK-NEXT:     Type:    SHT_RELA
-# CHECK-NEXT:     EntSize: 0x0000000000000018
 # CHECK-NEXT:     Relocations:
 # CHECK-NEXT:       - Offset: 0x0000000000000009
 # CHECK-NEXT:         Type:   [[FIRST]]
 # CHECK-NEXT:       - Offset: 0x0000000000000008
 # CHECK-NEXT:         Type:   [[SECOND]]
 # CHECK-NEXT:       - Offset: 0x0000000000000007
 # CHECK-NEXT:         Type:   0x0000007F
 # CHECK-NEXT:       - Offset: 0x0000000000000006

llvm/test/tools/obj2yaml/relr-section.yaml

-## Test how we dump SHT_RELR sections for 32 and 64-bit targets.
-
-## Test we use the "Entries" property when it is possible do
-## dump values correctly.
-
-# RUN: yaml2obj --docnum=1 -D BITS=32 -D ENCODE=LSB %s -o %t.32le
-# RUN: obj2yaml %t.32le | FileCheck %s --check-prefix=ELF32LE
-# RUN: yaml2obj --docnum=1 -D BITS=32 -D ENCODE=MSB %s -o %t.32be
-# RUN: obj2yaml %t.32be | FileCheck %s --check-prefix=ELF32BE
-# RUN: yaml2obj --docnum=1 -D BITS=64 -D ENCODE=LSB %s -o %t.64le
-# RUN: obj2yaml %t.64le | FileCheck %s --check-prefix=ELF64LE
-# RUN: yaml2obj --docnum=1 -D BITS=64 -D ENCODE=MSB %s -o %t.64be
-# RUN: obj2yaml %t.64be | FileCheck %s --check-prefix=ELF64BE
-
-# ELF64LE:      Sections:
-# ELF64LE-NEXT:   - Name:    .relr.dyn
-# ELF64LE-NEXT:     Type:    SHT_RELR
-# ELF64LE-NEXT:     EntSize: 0x0000000000000008
-# ELF64LE-NEXT:     Entries: [ 0x8877665544332211 ]
-
-# ELF32LE:      Sections:
-# ELF32LE-NEXT:   - Name:    .relr.dyn
-# ELF32LE-NEXT:     Type:    SHT_RELR
-# ELF32LE-NEXT:     EntSize: 0x0000000000000004
-# ELF32LE-NEXT:     Entries: [ 0x0000000044332211, 0x0000000088776655 ]
-
-# ELF64BE:      Sections:
-# ELF64BE-NEXT:   - Name:    .relr.dyn
-# ELF64BE-NEXT:     Type:    SHT_RELR
-# ELF64BE-NEXT:     EntSize: 0x0000000000000008
-# ELF64BE-NEXT:     Entries: [ 0x1122334455667788 ]
-
-# ELF32BE:      Sections:
-# ELF32BE-NEXT:  - Name:    .relr.dyn
-# ELF32BE-NEXT:    Type:    SHT_RELR
-# ELF32BE-NEXT:    EntSize: 0x0000000000000004
-# ELF32BE-NEXT:    Entries: [ 0x0000000011223344, 0x0000000055667788 ]
-
---- !ELF
-FileHeader:
-  Class:   ELFCLASS[[BITS]]
-  Data:    ELFDATA2[[ENCODE]]
-  Type:    ET_DYN
-  Machine: EM_X86_64
-Sections:
-  - Name: .relr.dyn
-    Type: SHT_RELR
-    Content: "1122334455667788"
-
-## Test we use the "Content" property when a SHT_RELR section is truncated.
-
-# RUN: yaml2obj --docnum=2 %s -o %t.content
-# RUN: obj2yaml %t.content | FileCheck %s --check-prefix=CONTENT
-
-# CONTENT:      - Name:    .relr.dyn
-# CONTENT-NEXT:   Type:    SHT_RELR
-# CONTENT-NEXT:   EntSize: 0x0000000000000008
-# CONTENT-NEXT:   Content: '11223344556677'
-
---- !ELF
-FileHeader:
-  Class:   ELFCLASS64
-  Data:    ELFDATA2MSB
-  Type:    ET_DYN
-  Machine: EM_X86_64
-Sections:
-  - Name: .relr.dyn
-    Type: SHT_RELR
-    Content: "11223344556677"
+## Test how we dump SHT_RELR sections for 32 and 64-bit targets.
+
+## Test we use the "Entries" property when it is possible to
+## dump values correctly. Also, check we do not dump sh_entsize when
+## it has the default value.
+
+# RUN: yaml2obj --docnum=1 -D BITS=32 -D ENCODE=LSB %s -o %t.32le
+# RUN: obj2yaml %t.32le | FileCheck %s --check-prefix=ELF32LE
+# RUN: yaml2obj --docnum=1 -D BITS=32 -D ENCODE=MSB %s -o %t.32be
+# RUN: obj2yaml %t.32be | FileCheck %s --check-prefix=ELF32BE
+# RUN: yaml2obj --docnum=1 -D BITS=64 -D ENCODE=LSB %s -o %t.64le
+# RUN: obj2yaml %t.64le | FileCheck %s --check-prefix=ELF64LE
+# RUN: yaml2obj --docnum=1 -D BITS=64 -D ENCODE=MSB %s -o %t.64be
+# RUN: obj2yaml %t.64be | FileCheck %s --check-prefix=ELF64BE
+
+# ELF64LE:      Sections:
+# ELF64LE-NEXT:   - Name:    .relr.dyn
+# ELF64LE-NEXT:     Type:    SHT_RELR
+# ELF64LE-NEXT:     Entries: [ 0x8877665544332211 ]
+
+# ELF32LE:      Sections:
+# ELF32LE-NEXT:   - Name:    .relr.dyn
+# ELF32LE-NEXT:     Type:    SHT_RELR
+# ELF32LE-NEXT:     Entries: [ 0x0000000044332211, 0x0000000088776655 ]
+
+# ELF64BE:      Sections:
+# ELF64BE-NEXT:   - Name:    .relr.dyn
+# ELF64BE-NEXT:     Type:    SHT_RELR
+# ELF64BE-NEXT:     Entries: [ 0x1122334455667788 ]
+
+# ELF32BE:      Sections:
+# ELF32BE-NEXT:  - Name:    .relr.dyn
+# ELF32BE-NEXT:    Type:    SHT_RELR
+# ELF32BE-NEXT:    Entries: [ 0x0000000011223344, 0x0000000055667788 ]
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS[[BITS]]
+  Data:    ELFDATA2[[ENCODE]]
+  Type:    ET_DYN
+  Machine: EM_X86_64
+Sections:
+  - Name: .relr.dyn
+    Type: SHT_RELR
+    Content: "1122334455667788"
+
+## Test we use the "Content" property when a SHT_RELR section is truncated.
+
+# RUN: yaml2obj --docnum=2 %s -o %t.content
+# RUN: obj2yaml %t.content | FileCheck %s --check-prefix=CONTENT
+
+# CONTENT:      - Name:    .relr.dyn
+# CONTENT-NEXT:   Type:    SHT_RELR
+# CONTENT-NEXT:   Content: '11223344556677'
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2MSB
+  Type:    ET_DYN
+  Machine: EM_X86_64
+Sections:
+  - Name: .relr.dyn
+    Type: SHT_RELR
+    Content: "11223344556677"
 
 ## Test we are able to dump a SHT_RELR section when sh_entsize is invalid.
 ## Here we use 0xFE as a value instead of expected 0x8.
 
 # RUN: yaml2obj --docnum=3 %s -o %t.entsize
 # RUN: obj2yaml %t.entsize | FileCheck %s --check-prefix=ENTSIZE
 
 # ENTSIZE:        - Name:    .relr.dyn

llvm/tools/obj2yaml/elf2yaml.cpp

Error ELFDumper<ELFT>::dumpRelocation(const RelT *Rel, const Elf_Shdr *SymTab,
   if (!NameOrErr)
     return NameOrErr.takeError();
   R.Symbol = NameOrErr.get();
 
   return Error::success();
 }
 
 template <class ELFT>
+static unsigned getDefaultShEntSize(ELFYAML::ELF_SHT SecType) {
+  switch (SecType) {
+  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);
+  default:
+    return 0;
+  }
+}
+
+template <class ELFT>
 Error ELFDumper<ELFT>::dumpCommonSection(const Elf_Shdr *Shdr,
                                          ELFYAML::Section &S) {
   // Dump fields. We do not dump the ShOffset field. When not explicitly
   // set, the value is set by yaml2obj automatically.
   S.Type = Shdr->sh_type;
   if (Shdr->sh_flags)
     S.Flags = static_cast<ELFYAML::ELF_SHF>(Shdr->sh_flags);
   if (Shdr->sh_addr)
     S.Address = static_cast<uint64_t>(Shdr->sh_addr);
   S.AddressAlign = Shdr->sh_addralign;
-  if (Shdr->sh_entsize)
+
+  if (Shdr->sh_entsize != getDefaultShEntSize<ELFT>(S.Type))
     S.EntSize = static_cast<llvm::yaml::Hex64>(Shdr->sh_entsize);
 
   auto NameOrErr = getUniquedSectionName(Shdr);
   if (!NameOrErr)
     return NameOrErr.takeError();
   S.Name = NameOrErr.get();
 
   if (Shdr->sh_link != ELF::SHN_UNDEF) {