This is an archive of the discontinued LLVM Phabricator instance.

Implement ObjectFilePECOFF::GetEntryPointAddress.
ClosedPublic

Authored by sas on Mar 8 2016, 2:55 PM.

Diff Detail

Event Timeline

sas updated this revision to Diff 50075.Mar 8 2016, 2:55 PM
sas retitled this revision from to Implement ObjectFilePECOFF::GetEntryPointAddress..
sas updated this object.
sas added reviewers: zturner, clayborg.
sas added a subscriber: lldb-commits.
sas added a subscriber: fjricci.Mar 8 2016, 2:55 PM
zturner edited edge metadata.Mar 8 2016, 3:11 PM

Looks pretty non-controversial. Does this fix anything? Seems like there should be a test that breaks somehow if this doesn't work. I don't know off the top of my head what the debugger command is "get the address of the entry point", but presumably it was broken before this patch?

clayborg requested changes to this revision.Mar 8 2016, 3:22 PM
clayborg edited edge metadata.

ObjectFilePECOFF::GetEntryPointAddress() is not implemented correctly. See inlined comments.

source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
824

You will need to convert the file address "m_coff_header_opt.entry" into a section offset address before returning this. See ObjectFileELF::GetEntryPointAddress() for info on how to do this correctly and also how to cache the entry address in case it is asked for multiple time. ObjectFileELF has an instance variable "Address m_entry_point_address;" that it will populate once and lazily and return it is we have resolved the address, otherwise it will resolve the file address using the section list.

This revision now requires changes to proceed.Mar 8 2016, 3:22 PM

lldb_private::Address is a section offset address class. We use section offset addresses everywhere. We also have the notion of three types of addresses: file address, load address and host address.

File addresses are virtual addresses as they are found in the object files (ELF, PECOFF, mach-o). They don't mean anything to the debugger because you will load your object file at various addresses as you are debugging. So we like to store the address as ".text + 0x123". Then when you load your shared library during runtime, the ".text" section will get slid by some offset. The target tracks where each shared library loads each of its sections and can turn a load address, or an address from a live process, into a section offset address. So load addresses can be resolved back into section + offset when running. A host address is when we stored constant data in the LLDB process itself. So in order to be able to slide your shared library around when running, we store all addresses as lldb_private::Address objects to facilite this. Targets can resolve a load address into a lldb_private::Address. When parsing things in object files, you must always translate any file addresses into lldb_private::Addresses (in symbols, debug info for functions and line entries, entry point addresses and much more).

Your function was causing an implicit constructor to be called on Address:

Address (lldb::addr_t abs_addr);

This will always result in a Address that has no section with an m_offset set to "abs_addr", meaning this is an absolute address that never slides.

jingham added a subscriber: jingham.Mar 8 2016, 4:07 PM

We use the entry point address as the place to stop when we call functions by hand in the target. That's one place which is legit TEXT, but we are pretty sure the code you are calling will never call. So if the entry point address isn't set correctly we probably won't be able to call any functions by hand.

Jim

zturner added a subscriber: zturner.Mar 8 2016, 4:19 PM

I was hoping there would be something like image headers that would just
display various information about the image, but it looks like we don't
have such a command that I can find. Seems like being able to find out the
entry point of a certain module would be useful.

sas added a comment.Mar 17 2016, 12:01 PM

Yep, @jingham is right, I need this for expression evaluation. I'll do the change @clayborg requested.

sas added a comment.Mar 17 2016, 12:04 PM

FWIW, I didn't cache this value because in the current form it is just returning some data that is already cached by ParseHeader(). If I split it in section/offset address, it might be required.

sas updated this revision to Diff 50978.Mar 17 2016, 2:40 PM
sas edited edge metadata.

Change implementation according to @clayborg's comments.

clayborg accepted this revision.Mar 17 2016, 4:57 PM
clayborg edited edge metadata.

Looks good.

This revision is now accepted and ready to land.Mar 17 2016, 4:57 PM
This revision was automatically updated to reflect the committed changes.