This is an archive of the discontinued LLVM Phabricator instance.

[llvm-readobj][llvm-readelf] - Refactor parsing of the SHT_GNU_versym section.
ClosedPublic

Authored by grimar on Dec 5 2019, 3:02 AM.

Details

Summary

This introduce a new helper which is used to parse the SHT_GNU_versym section.
LLVM/GNU styles implementations now use it to share the logic.

Diff Detail

Event Timeline

grimar created this revision.Dec 5 2019, 3:02 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 5 2019, 3:02 AM
jhenderson added inline comments.Dec 6 2019, 1:52 AM
llvm/test/tools/llvm-readobj/elf-versym-invalid.test
17

25 -> 255

32

referenc -> reference

33

Delete the extra "table"

73

I'm not asking for a change, but I'm noting that this warning message is getting a bit silly in length! I don't have any better suggestion though without losing context.

96

Nit: missing full stop.

124

Here and elsewhere "... when a SHT_GNU_versym..."

131

What's the trailing "()" for here?

152

in the associated symbol table.

184

dynamic symbols -> dynamic symbol

llvm/tools/llvm-readobj/ELFDumper.cpp
374–375

// Returns the linked symbol table and associated string table for a given section.

379

ExpectedType should be an Elf_Word, I think.

452

symbols table -> symbol table

5936

Should this be for (size_t I = 0, S = Syms.size(); I < S; ++I)?

5940

true /* IsDynamic */ -> /*IsDynamic=*/true

grimar updated this revision to Diff 232520.Dec 6 2019, 3:01 AM
grimar marked 18 inline comments as done.
  • Addressed review comments.
llvm/test/tools/llvm-readobj/elf-versym-invalid.test
73

Yeah, but this is a result of a multiple messages concatenations. It is kind of by design.

131

It usually contains a section name. E.g.:

Addr: 0000000000000000 Offset: 0x000040 Link: 4 (.dynsym)

Here the sh_link==0 hence we see no name. Doesn't seem to be a bug problem? (since having sh_link==0 is ubnormal).

llvm/tools/llvm-readobj/ELFDumper.cpp
379

Probably not. It is expected to accept an enum value which is unsigned:
(https://github.com/llvm-mirror/llvm/blob/master/include/llvm/BinaryFormat/ELF.h#L815)

enum : unsigned {
...
  SHT_DYNSYM = 11,                      // Symbol table.
5936

Done.

This revision is now accepted and ready to land.Dec 6 2019, 3:40 AM
This revision was automatically updated to reflect the committed changes.