This is an archive of the discontinued LLVM Phabricator instance.

[llvm-readobj] Add GNU style dumper for .gnu.version section
ClosedPublic

Authored by Higuoxing on Mar 27 2019, 6:52 AM.

Details

Summary

Currently, llvm-readobj do not support GNU style dumper for symbol versioning sections. In this patch, I would like to implement dumper for .gnu.version section

Event Timeline

Higuoxing created this revision.Mar 27 2019, 6:52 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 27 2019, 6:52 AM
Higuoxing updated this revision to Diff 192442.Mar 27 2019, 7:19 AM
  • Add test file
rupprecht accepted this revision.Mar 27 2019, 3:29 PM

Very cool. It looks like -V also prints a definition/needed section that this patch doesn't support yet. Is that coming in a later patch?

i.e.,

$ readelf -V ~/src/llvm-project/llvm/test/tools/llvm-readobj/Inputs/verdef.elf-x86-64

Version symbols section '.gnu.version' contains 8 entries:
 Addr: 000000000000024c  Offset: 0x00024c  Link: 1 (.dynsym)
  000:   0 (*local*)       1 (*global*)      1 (*global*)      3 (VERSION2)
  004:   1 (*global*)      2 (VERSION1)      2 (VERSION1)      3 (VERSION2)

# This block is missing from this patch
Version definition section '.gnu.version_d' contains 3 entries:
  Addr: 0x000000000000025c  Offset: 0x00025c  Link: 2 (.dynstr)
  000000: Rev: 1  Flags: BASE  Index: 1  Cnt: 1  Name: blah
  0x001c: Rev: 1  Flags: none  Index: 2  Cnt: 1  Name: VERSION1
  0x0038: Rev: 1  Flags: none  Index: 3  Cnt: 2  Name: VERSION2
  0x0054: Parent 1: VERSION1

$ readelf -V ~/src/llvm-project/llvm/test/tools/llvm-readobj/Inputs/verneed.elf-x86-64 

Version symbols section '.gnu.version' contains 4 entries:
 Addr: 0000000000010228  Offset: 0x000228  Link: 1 (.dynsym)
  000:   0 (*local*)       2 (v3)            3 (v2)            4 (v1)         

# This block is missing from this patch
Version needs section '.gnu.version_r' contains 2 entries:
 Addr: 0x0000000000010230  Offset: 0x000230  Link: 5 (.dynstr)
  000000: Version: 1  File: verneed1.so.0  Cnt: 2
  0x0020:   Name: v2  Flags: none  Version: 3
  0x0030:   Name: v3  Flags: none  Version: 2
  0x0010: Version: 1  File: verneed2.so.0  Cnt: 1
  0x0040:   Name: v1  Flags: none  Version: 4
This revision is now accepted and ready to land.Mar 27 2019, 3:29 PM
Higuoxing added a comment.EditedMar 27 2019, 6:25 PM

Very cool. It looks like -V also prints a definition/needed section that this patch doesn't support yet. Is that coming in a later patch?

Thanks for reviewing! I have implemented prototype dumpers for them, and I would like to introduce them in next series of patches.

Higuoxing retitled this revision from [llvm-readobj][WIP] Add GNU style dumper for .gnu.version section to [llvm-readobj] Add GNU style dumper for .gnu.version section.Mar 27 2019, 6:26 PM
jhenderson requested changes to this revision.Mar 28 2019, 3:24 AM
jhenderson added inline comments.
llvm/tools/llvm-readobj/ELFDumper.cpp
3358–3408

Use uint64_t (or possibly size_t) here. ELF64 sections have section sizes of 64-bits.

Various variables below will similarly need adjusting.

3373

Does this behaviour hold if using --wide in GNU readelf?

3391

Test case for hidden symbols?

3401

What happens if the version name is longer than this? (Is that even possible?)

This revision now requires changes to proceed.Mar 28 2019, 3:24 AM
Higuoxing updated this revision to Diff 193417.Apr 2 2019, 7:21 PM
Higuoxing marked 7 inline comments as done.

Addressed @jhenderson 's comments

  • Add test case for invalid versioning symbol.
Higuoxing added inline comments.Apr 2 2019, 7:21 PM
llvm/tools/llvm-readobj/ELFDumper.cpp
3373

No, in GNU readelf, --wide option has no effect on -V option.

3391

Done.

3401

Yes, it's possible. But GNU readelf does not do any improvement here, shall we?

Higuoxing updated this revision to Diff 193420.Apr 2 2019, 7:42 PM
Higuoxing marked 2 inline comments as not done.

.

jhenderson added inline comments.Apr 3 2019, 6:11 AM
llvm/test/tools/llvm-readobj/elf-hidden-versym.test
32–41 ↗(On Diff #193420)

Could you add less padding between the Name and other fields, and the values for those fields, please? Ditto in the other new test.

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

Entries is now a uin64_t, so VersymRow and VersymIndex below should be too.

3401

I think it's okay to leave for now, but you might want to consider improving it in a future version, by using a column width that varies depending on name widths.

Higuoxing updated this revision to Diff 193483.Apr 3 2019, 6:24 AM
  • Using uint64_t instead of uint32_t
Higuoxing added inline comments.Apr 3 2019, 6:26 AM
llvm/tools/llvm-readobj/ELFDumper.cpp
3401

Thanks, I will consider improve this behavior

This revision is now accepted and ready to land.Apr 3 2019, 6:26 AM

LGTM.

Thanks for reviewing!

This revision was automatically updated to reflect the committed changes.

Seems that this patch breaks some tests, I will take a look

grimar added a comment.Apr 3 2019, 8:21 AM

Seems that this patch breaks some tests, I will take a look

It was me. Fixed in https://reviews.llvm.org/rL357598

Seems that this patch breaks some tests, I will take a look

It was me. Fixed in https://reviews.llvm.org/rL357598

Thank you very much :)