Page MenuHomePhabricator

[llvm-objdump] Add -R option
ClosedPublic

Authored by paulsemel on May 29 2018, 12:44 PM.

Details

Summary

This option prints dynamic relocation entries of the given file

Diff Detail

Repository
rL LLVM

Event Timeline

paulsemel created this revision.May 29 2018, 12:44 PM

This is absolutely not finished. I kind of works for ELF files (tho it prints every relocations, not only dynamic ones).
I don't know much about other binary file types, I will probably need to dig more on this (and maybe some help if possible).

paulsemel added inline comments.May 29 2018, 1:05 PM
include/llvm/Object/ELFObjectFile.h
795 ↗(On Diff #148966)

I guess this assert was not here for no reason. But I didn't understand why we were forbidding this function for ELF files other than ET_REL.. Can someone explain me those lines ?

paulsemel updated this revision to Diff 149634.Jun 3 2018, 6:42 AM

Added helper function to get dynamic relocation sections.

paulsemel updated this revision to Diff 149635.Jun 3 2018, 6:46 AM

Fix TODO comment.

paulsemel updated this revision to Diff 149774.Jun 4 2018, 8:46 AM

Added long option.

echristo added inline comments.Jun 6 2018, 1:39 AM
include/llvm/Object/ELFObjectFile.h
795 ↗(On Diff #148966)

I think it's that it was looking for a relocatable file and perhaps needs additional checks for any kind of relocatable rather than just relocatable object files.

tools/llvm-objdump/llvm-objdump.cpp
441 ↗(On Diff #149774)

Can you add some explanatory comments here, it's not necessarily obvious what "undef" means in this context as it can be an overloaded term.

paulsemel updated this revision to Diff 150089.Jun 6 2018, 1:55 AM
paulsemel marked an inline comment as done.

Added explanatory comment for the undef variable.

Thanks for reviewing :)

include/llvm/Object/ELFObjectFile.h
795 ↗(On Diff #148966)

Yes, there is probably more check needed. At least, at the ELF format file level, there is no difference between ELF file types

tools/llvm-objdump/llvm-objdump.cpp
441 ↗(On Diff #149774)

Sure !

echristo accepted this revision.Jun 6 2018, 3:15 PM

Some of the logic in the function feels awkward, but that's a cleanup for another time probably.

-eric

This revision is now accepted and ready to land.Jun 6 2018, 3:15 PM

Some of the logic in the function feels awkward, but that's a cleanup for another time probably.

-eric

Yes, I didn't really know how to do it better. I truly think that we want to output the sections and not directly the relocations.
This is why there is this weird logic (there is no pointer to reloc section in the Dynamic Section).
If you find any better way to do it, this might be awesome !
Will land tomorrow :)

Closed by commit rL334196: [llvm-objdump] Add -R option (authored by paulsemel, committed by ). · Explain WhyJun 7 2018, 6:35 AM
This revision was automatically updated to reflect the committed changes.