Page MenuHomePhabricator

[llvm-objdump] Label calls to the PLT
ClosedPublic

Authored by jgalenson on Aug 2 2018, 3:40 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

jgalenson created this revision.Aug 2 2018, 3:40 PM

Is there a better way to handle appending "@plt" to the name? I had to add a vector to keep the original string around so that the StringRefs are still good, but that's a bit ugly.

paulsemel added inline comments.Aug 3 2018, 1:32 AM
tools/llvm-objdump/llvm-objdump.cpp
1244 ↗(On Diff #158848)

Not sure this is the correct way to get the PLT section. Here is how GNU readelf does : first, it searches in the dynamic entries, and then, if there is no dynamic tables, it tries to find by name.

There is a helper for getting dynamic entries in the libObject : dynamicEntries() in include/llvm/Object/ELF.h

Btw, I think it might be cool to have a helper that gets the PLT section in the libObject. And you might want to use it also in https://reviews.llvm.org/D50203.

pcc added inline comments.Aug 3 2018, 10:17 AM
tools/llvm-objdump/llvm-objdump.cpp
1244 ↗(On Diff #158848)

Is there a DT tag for the PLT? At least I can't seem to see one in either an x86_64 or aarch64 binary, which makes sense to me because from the loader's perspective the PLT is just another block of code.

1262 ↗(On Diff #158848)

I think the general way that this is done is with the StringSaver class.

paulsemel added inline comments.Aug 3 2018, 10:30 AM
tools/llvm-objdump/llvm-objdump.cpp
1244 ↗(On Diff #158848)

Exact there is only one for the got I think.. this still look weird to search for the section name (even though 99% of the time the PLT will be called ".plt"

jgalenson added inline comments.Aug 3 2018, 10:34 AM
tools/llvm-objdump/llvm-objdump.cpp
1244 ↗(On Diff #158848)

I also only found tags for the GOT (DT_PLTGOT) and the RelaPLT (DT_JMPREL) and couldn't find a PLT one for x86 or AArch64. Should I only do this for those two and find the PLT as I'm doing now?

Just to be sure I understand how you want me to do this, the flow should be:

Call dynamicEntries().
Walk through the list looking for things with the above tags (or the PLT one if we can find it).
Once we find one we want, use its address to construct a DataRefImpl, which we can use to make a SectionRef.

Is that right? Sorry about the question, but I've never used DynRanges before and there's not much documentation about them.

1262 ↗(On Diff #158848)

Thanks! That looks exactly like what I wanted.

pcc added inline comments.Aug 3 2018, 10:57 AM
tools/llvm-objdump/llvm-objdump.cpp
1244 ↗(On Diff #158848)

I wouldn't worry about the dynamic table to be honest. Presumably the use case is being able to operate on files with stripped section headers, but if we need the section headers for the plt anyway, being able to look up the got/rela.plt via the dynamic table isn't buying us much.

jgalenson updated this revision to Diff 159076.Aug 3 2018, 1:11 PM

Updated to use StringSaver. I'm still waiting on the discussion about whether or not to use dynamicEntries().

paulsemel added inline comments.Aug 3 2018, 1:21 PM
tools/llvm-objdump/llvm-objdump.cpp
1244 ↗(On Diff #158848)

Yes, I couldn't find a way to do it correctly anyway, @pcc is right, just let it like this :)

jgalenson updated this revision to Diff 159933.Aug 9 2018, 8:31 AM

Added newline to end of tests.

pcc added a comment.Aug 14 2018, 2:25 PM

Please add a test with a 32-bit non-PIC executable.

tools/llvm-objdump/llvm-objdump.cpp
1244 ↗(On Diff #159933)

I think this has the same problem with find_if that I mentioned on the other review.

jgalenson updated this revision to Diff 160833.Aug 15 2018, 9:37 AM
jgalenson marked an inline comment as done.
pcc added inline comments.Aug 16 2018, 1:24 PM
test/tools/llvm-objdump/X86/plt.test
3 ↗(On Diff #160833)

Was this line intended to test hello.exe.elf-x86_64?

jgalenson updated this revision to Diff 161102.Aug 16 2018, 1:37 PM
jgalenson added inline comments.
test/tools/llvm-objdump/X86/plt.test
3 ↗(On Diff #160833)

Oops, yes (except that I also used the wrong name, as both my new files are i386). Thanks for catching this.

Friendly ping. Are there any more comments?

Friendly ping.

pcc accepted this revision.Aug 23 2018, 5:12 PM

LGTM

This revision is now accepted and ready to land.Aug 23 2018, 5:12 PM
This revision was automatically updated to reflect the committed changes.
vitalybuka added a subscriber: vitalybuka.EditedAug 24 2018, 12:33 PM

This is broken by the patch http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/22760/steps/check-lld%20asan/logs/stdio

=================================================================
==15093==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 16 byte(s) in 1 object(s) allocated from:
    #0 0x5aa968 in operator new(unsigned long) /b/sanitizer-x86_64-linux-fast/build/llvm/projects/compiler-rt/lib/asan/asan_new_delete.cc:92
    #1 0x957b22 in createX86MCInstrAnalysis(llvm::MCInstrInfo const*) /b/sanitizer-x86_64-linux-fast/build/llvm/lib/Target/X86/MCTargetDesc/X86MCTargetDesc.cpp:579:10
    #2 0xfe4db5 in createMCInstrAnalysis /b/sanitizer-x86_64-linux-fast/build/llvm/include/llvm/Support/TargetRegistry.h:354:12
    #3 0xfe4db5 in llvm::object::ELFObjectFileBase::getPltAddresses() const /b/sanitizer-x86_64-linux-fast/build/llvm/lib/Object/ELFObjectFile.cpp:354
    #4 0x60106c in addPltEntries /b/sanitizer-x86_64-linux-fast/build/llvm/tools/llvm-objdump/llvm-objdump.cpp:1253:34
    #5 0x60106c in DisassembleObject(llvm::object::ObjectFile const*, bool) /b/sanitizer-x86_64-linux-fast/build/llvm/tools/llvm-objdump/llvm-objdump.cpp:1379
    #6 0x5f3bf0 in DumpObject(llvm::object::ObjectFile*, llvm::object::Archive const*, llvm::object::Archive::Child const*) /b/sanitizer-x86_64-linux-fast/build/llvm/tools/llvm-objdump/llvm-objdump.cpp:2308:5
    #7 0x5c298c in DumpInput /b/sanitizer-x86_64-linux-fast/build/llvm/tools/llvm-objdump/llvm-objdump.cpp:2406:5
    #8 0x5c298c in for_each<std::__1::__wrap_iter<std::__1::basic_string<char> *>, void (*)(llvm::StringRef)> /b/sanitizer-x86_64-linux-fast/build/libcxx_build_asan/include/c++/v1/algorithm:832
    #9 0x5c298c in for_each<llvm::cl::list<std::__1::basic_string<char>, bool, llvm::cl::parser<std::string> > &, void (*)(llvm::StringRef)> /b/sanitizer-x86_64-linux-fast/build/llvm/include/llvm/ADT/STLExtras.h:1027
    #10 0x5c298c in main /b/sanitizer-x86_64-linux-fast/build/llvm/tools/llvm-objdump/llvm-objdump.cpp:2473
    #11 0x7ffa4b0832e0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202e0)

Indirect leak of 32 byte(s) in 1 object(s) allocated from:
    #0 0x5aa968 in operator new(unsigned long) /b/sanitizer-x86_64-linux-fast/build/llvm/projects/compiler-rt/lib/asan/asan_new_delete.cc:92
    #1 0x957480 in createX86MCInstrInfo() /b/sanitizer-x86_64-linux-fast/build/llvm/lib/Target/X86/MCTargetDesc/X86MCTargetDesc.cpp:294:20
    #2 0xfe4d88 in createMCInstrInfo /b/sanitizer-x86_64-linux-fast/build/llvm/include/llvm/Support/TargetRegistry.h:346:12
    #3 0xfe4d88 in llvm::object::ELFObjectFileBase::getPltAddresses() const /b/sanitizer-x86_64-linux-fast/build/llvm/lib/Object/ELFObjectFile.cpp:354
    #4 0x60106c in addPltEntries /b/sanitizer-x86_64-linux-fast/build/llvm/tools/llvm-objdump/llvm-objdump.cpp:1253:34
    #5 0x60106c in DisassembleObject(llvm::object::ObjectFile const*, bool) /b/sanitizer-x86_64-linux-fast/build/llvm/tools/llvm-objdump/llvm-objdump.cpp:1379
    #6 0x5f3bf0 in DumpObject(llvm::object::ObjectFile*, llvm::object::Archive const*, llvm::object::Archive::Child const*) /b/sanitizer-x86_64-linux-fast/build/llvm/tools/llvm-objdump/llvm-objdump.cpp:2308:5
    #7 0x5c298c in DumpInput /b/sanitizer-x86_64-linux-fast/build/llvm/tools/llvm-objdump/llvm-objdump.cpp:2406:5
    #8 0x5c298c in for_each<std::__1::__wrap_iter<std::__1::basic_string<char> *>, void (*)(llvm::StringRef)> /b/sanitizer-x86_64-linux-fast/build/libcxx_build_asan/include/c++/v1/algorithm:832
    #9 0x5c298c in for_each<llvm::cl::list<std::__1::basic_string<char>, bool, llvm::cl::parser<std::string> > &, void (*)(llvm::StringRef)> /b/sanitizer-x86_64-linux-fast/build/llvm/include/llvm/ADT/STLExtras.h:1027
    #10 0x5c298c in main /b/sanitizer-x86_64-linux-fast/build/llvm/tools/llvm-objdump/llvm-objdump.cpp:2473
    #11 0x7ffa4b0832e0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202e0)

SUMMARY: AddressSanitizer: 48 byte(s) leaked in 2 allocation(s).

I uploaded https://reviews.llvm.org/D51230 to try to address that. Could you take a look at it?