[llvm-readobj] - Report a warning when an unexpected DT_SYMENT tag value is met.

[grimar]

Summary

There was a short discussion about this:
https://reviews.llvm.org/D73484#inline-676942

To summarize:
It is a bit unclear to me why the DT_SYMENT tag exist.
LLD has the code that does:
"addInt(DT_SYMENT, sizeof(Elf_Sym));" and I guess other linkers has the same logic.
It is unclear why it can be possible to have other values rather than values of
a size of platform symbol. Seems it is not possible, and atm for me it looks that
this tag should not be used. This patch starts reporting the warning when the
value it contains differs from a symbol size for a 32/64 bit platform for safety.
It keeps the rest of the logic we have unchanged. Before this patch we did not handle
the tag at all.

[jhenderson]

llvm/tools/llvm-readobj/ELFDumper.cpp

2068

I think this would be better: "DT_SYMENT value of 0x123 is not the size of a symbol (0x456)".

[grimar]

• Addressed review comments.
• Reordered lines in the test case a bit to group them closer to check calls.

[jhenderson]

LGTM.

[MaskRay]

LLD has the code that does: "addInt(DT_SYMENT, sizeof(Elf_Sym));" and I guess other linkers has the same logic.

Confirmed. GNU ld, gold and MCLinker emit DT_SYMENT.

It is a bit unclear to me why the DT_SYMENT tag exist.

Probably for extensibility. Many rtld assert it to be equal to sizeof(Elf*_Sym) though...

musl ignores DT_SYMENT.
FreeBSD (usr.sbin/kldxref/ef.c libexec/rtld-elf/rtld.c sys/kern/link_elf.c), elftools/elf/dynamic.py, and android/bionic_loader/linker.cpp take it as an assertion.
glibc pa-risc reads DT_SYMENT but it seems unnecessary.

An assertion is fine.

[grimar]

In D74479#1874794, @MaskRay wrote:

LLD has the code that does: "addInt(DT_SYMENT, sizeof(Elf_Sym));" and I guess other linkers has the same logic.

Confirmed. GNU ld, gold and MCLinker emit DT_SYMENT.

Yeah, I knew about GNUs, by "the same logic" I meant that there should not be other values emitted rather than "sizeof(Elf_Sym)" generally.
Thanks!

[grimar]

https://reviews.llvm.org/rGb87a0f7416f1a657d808690295b44669849e7bfc

llvm/test/tools/llvm-readobj/ELF/dyn-symbols.test

FileHeader:
   Machine: EM_386
 DynamicSymbols: []
 
 ## Case 9: Check what we print when:
 ## a) The size of the dynamic symbol table is not a multiple of its entry size.
 # RUN: yaml2obj %s --docnum=10 -o %t10
 # RUN: llvm-readobj --dyn-symbols %t10 2>&1 | FileCheck %s -DFILE=%t10 --check-prefix=DYNSYM-SIZE-INVALID1
 # RUN: llvm-readelf --dyn-symbols %t10 2>&1 | FileCheck %s -DFILE=%t10 --check-prefix=DYNSYM-SIZE-INVALID1
+
+# DYNSYM-SIZE-INVALID1: warning: '[[FILE]]': section with index 1 has invalid size (0x1) or entry size (0x10)
+
 ## b) The same, but the DT_SYMTAB tag is present. In this case the dynamic tag has priority over the
 ##    information about a location and an entity size of the dynamic symbol table from the section header.
 ##    The code uses sizeof(Elf_Sym) for an entity size, so it can't be incorrect and
 ##    the message printed is a bit shorter.
 # RUN: yaml2obj %s --docnum=11 -o %t11
 # RUN: llvm-readobj --dyn-symbols %t11 2>&1 | FileCheck %s -DFILE=%t11 --check-prefix=DYNSYM-SIZE-INVALID2
 # RUN: llvm-readelf --dyn-symbols %t11 2>&1 | FileCheck %s -DFILE=%t11 --check-prefix=DYNSYM-SIZE-INVALID2
 
-# DYNSYM-SIZE-INVALID1: warning: '[[FILE]]': section with index 1 has invalid size (0x1) or entry size (0x10)
-
 # DYNSYM-SIZE-INVALID2: warning: '[[FILE]]': section with index 2 has invalid size (0x1){{$}}
 
+## c) In the case when the DT_SYMENT tag is present, we report when it's value does not match the
+#     value of the symbol size for the platform.
+# RUN: yaml2obj %s -D BITS=32 --docnum=12 -o %t12
+# RUN: llvm-readobj --dyn-symbols %t12 2>&1 | FileCheck %s -DFILE=%t12 --check-prefix=DYNSYM-SIZE-INVALID3
+# RUN: llvm-readelf --dyn-symbols %t12 2>&1 | FileCheck %s -DFILE=%t12 --check-prefix=DYNSYM-SIZE-INVALID3
+# RUN: yaml2obj %s -D BITS=64 --docnum=12 -o %t13
+# RUN: llvm-readobj --dyn-symbols %t13 2>&1 | FileCheck %s -DFILE=%t13 --check-prefix=DYNSYM-SIZE-INVALID4
+# RUN: llvm-readelf --dyn-symbols %t13 2>&1 | FileCheck %s -DFILE=%t13 --check-prefix=DYNSYM-SIZE-INVALID4
+
+# DYNSYM-SIZE-INVALID3: warning: '[[FILE]]': DT_SYMENT value of 0x123 is not the size of a symbol (0x10){{$}}
+# DYNSYM-SIZE-INVALID4: warning: '[[FILE]]': DT_SYMENT value of 0x123 is not the size of a symbol (0x18){{$}}
+
 --- !ELF
 FileHeader:
   Class:   ELFCLASS32
   Data:    ELFDATA2LSB
   Type:    ET_DYN
   Machine: EM_386
 Sections:
   - Name: .dynsym
- Name:    .dynsym
     Type:    SHT_DYNSYM
     Address: 0x100
     Size:    0x1
 ProgramHeaders:
   - Type: PT_LOAD
     VAddr: 0x100
     Sections:
       - Section: .dynsym
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS[[BITS]]
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_X86_64
+Sections:
+  - Name:    .dynamic
+    Type:    SHT_DYNAMIC
+    Entries:
+      - Tag:   DT_SYMENT
+        Value: 0x123
+      - Tag:   DT_NULL
+        Value: 0

llvm/tools/llvm-readobj/ELFDumper.cpp

case ELF::DT_SYMTAB: {
               ObjF->getFileName());
 
         DynSymRegion.Addr = VA;
         DynSymRegion.EntSize = sizeof(Elf_Sym);
         DynSymRegion.EntSizePrintName = "";
       }
       break;
     }
+    case ELF::DT_SYMENT: {
+      uint64_t Val = Dyn.getVal();
+      if (Val != sizeof(Elf_Sym))
+        reportWarning(createError("DT_SYMENT value of 0x" +
+                                  Twine::utohexstr(Val) +
+                                  " is not the size of a symbol (0x" +
+                                  Twine::utohexstr(sizeof(Elf_Sym)) + ")"),
+                      ObjF->getFileName());
+      break;
+    }
     case ELF::DT_RELA:
       DynRelaRegion.Addr = toMappedAddr(Dyn.getTag(), Dyn.getPtr());
       break;
     case ELF::DT_RELASZ:
       DynRelaRegion.Size = Dyn.getVal();
       DynRelaRegion.SizePrintName = "DT_RELASZ value";
       break;
     case ELF::DT_RELAENT: