Page MenuHomePhabricator

[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

Diff Detail

Repository
rL LLVM

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
3370 ↗(On Diff #192442)

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

Various variables below will similarly need adjusting.

3385 ↗(On Diff #192442)

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

3403 ↗(On Diff #192442)

Test case for hidden symbols?

3413 ↗(On Diff #192442)

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
3385 ↗(On Diff #192442)

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

3403 ↗(On Diff #192442)

Done.

3413 ↗(On Diff #192442)

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 ↗(On Diff #193420)

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

3413 ↗(On Diff #192442)

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
3413 ↗(On Diff #192442)

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 :)