Page MenuHomePhabricator

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

Authored by grimar on Wed, Feb 12, 5:16 AM.

Details

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.

Diff Detail

Event Timeline

grimar created this revision.Wed, Feb 12, 5:16 AM
Herald added a project: Restricted Project. · View Herald Transcript
jhenderson added inline comments.Wed, Feb 12, 7:31 AM
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 updated this revision to Diff 244349.Thu, Feb 13, 12:42 AM
  • Addressed review comments.
  • Reordered lines in the test case a bit to group them closer to check calls.
This revision is now accepted and ready to land.Thu, Feb 13, 1:36 AM
MaskRay accepted this revision.Thu, Feb 13, 10:28 AM

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.

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!