This is an archive of the discontinued LLVM Phabricator instance.

[llvm-readelf/obj][test] - Document what we print in various places for unnamed section symbols.
ClosedPublic

Authored by grimar on Sep 16 2020, 7:40 AM.

Details

Summary

We have an issue with ELFDumper<ELFT>::getSymbolSectionName:

  1. It is used deeply for both LLVM/GNU styles and might return LLVM-style only values to describe symbols: "Undefined", "Processor Specific", "Absolute", etc.
  1. getSymbolSectionName is used by getFullSymbolName and these special values might appear in instead of symbol names in many places. This occurs for unnamed section symbols.

It was not noticed because for most cases I've found it is unexpected to have an
unnamed section symbol. This patch documents the existent behavior, adds tests and FIXMEs.

These FIXMEs are fixed in D87764.

Diff Detail

Event Timeline

grimar created this revision.Sep 16 2020, 7:40 AM
Herald added a project: Restricted Project. · View Herald Transcript
grimar requested review of this revision.Sep 16 2020, 7:40 AM
jhenderson added inline comments.Sep 17 2020, 1:31 AM
llvm/test/tools/llvm-readobj/ELF/dyn-symbols.test
327

I think it would be slightly more obvious to use Name: [[NAME=bar]] and then just set -DNAME='', if that's possible?

352

If you add the check for the first line of GNU output that says how many symbols there are, I think you can probably get away with simplifying the above patterns to single line checks for the names (i.e. without the need for -SAME).

llvm/test/tools/llvm-readobj/ELF/hash-symbols.test
87

Same comment as above.

llvm/test/tools/llvm-readobj/ELF/mips-got.test
707–708

Is there any particular reason you've chosen to have both absolute and common here, but none of the other varieties?

llvm/test/tools/llvm-readobj/ELF/symbol-shndx.test
63

Same comment as above re. naming.

grimar updated this revision to Diff 292463.Sep 17 2020, 4:54 AM
grimar marked 5 inline comments as done.
  • Addressed review comments.
llvm/test/tools/llvm-readobj/ELF/dyn-symbols.test
327

Interesting idea. Done.

llvm/test/tools/llvm-readobj/ELF/mips-got.test
707–708

No. I've just showed 2 arbitrary symbols revealing the issue and an arbitrary normal symbol in between of them.
I do not feel it worth to check all possible values here? They are tested in symbol-shndx.test, we just have the same issue in all other places because of using the same API for getting a symbol name.

This revision is now accepted and ready to land.Sep 17 2020, 5:39 AM