Fix entry point lookup for ObjectFilePECOFF

[zturner]

+greg. I thought I remember Greg saying that that offsets were supposed to be RVAs, not VAs. Am I wrong here?

[sas]

@zturner, you might be right, I might have to change the code I have to load the initial executable's Module and use an offset different than 0 there. I'll wait to see what @clayborg says.

[clayborg]

"offset" here must be a file address, or a virtual address as the file's sections know it. So if you take "offset" and look it up in the sections for the COFF file, it should be a correct address. I don't know what "m_coff_header_opt.image_base" is, but as along as entry + image_base is the address that should be looked up in the section list, then this would be correct.

[zturner]

image base is the address that the process should be loaded to in memory. But this is only a hint anyway, and a process might end up being loaded at a different address. So now that I think about it, this is wrong no matter what because even if you did want a virtual address, you would have to figure out the actual load address at runtime since it might not load at the desired address.

[clayborg]

Let me clarify a few things. A "file address" is an address (lldb::addr_t) that gets translated into a section + offset address (lldb_private::Address which contains a lldb_private::Section + offset. Any addresses coming from ObjectFile parsers must be made into lldb_private::Address objects. As you load shared libraries and executables the dynamic loader plugins will tell the lldb_private::Target where all sections are loaded. So the slide is automatically applied to the section offset address. So you will make "m_entry_point_address" into something that says ".text + 0x123". If we ever want to use this address so set a breakpoint, we will resolve the lldb_private::Address into a "load address" (also a lldb::addr_t), but we will ask the target for foo.exe's ".text" load address, and we will get some slide address like 0x12340000 and then the load address would be "0x12340000 + 0x123", or 0x12340123. So each object file must encode their addresses correctly. Does that make more sense?

[clayborg]

Example code to resolve this would be:

Address entry_addr = objfile->GetEntryPointAddress();

lldb::addr_t entry_load_addr = entry_addr.GetLoadAddress(target);

if (entry_load_addr != LLDB_INVALID_ADDRESS)
{
    // We were able to resolve the address as the target has the section loaded
}
else
{
    // The section is not loaded in the target yet. We might have a target that hasn't
    // been launched yet, the process might have exited, or the address might be
    // from a shared library that will get loaded later in the process' lifetime
}

[clayborg]

Any progress on this? Read the previous comments and see if what I said makes sense.

[clayborg]

Anyone still care about this? It would be nice to move it along or abandon it.

[clayborg]

Does this need some ARM support where we strip bit zero in case the entry point is Thumb?

[sas]

In D19603#908130, @clayborg wrote:

Does this need some ARM support where we strip bit zero in case the entry point is Thumb?

Good question. Somehow we never had any issue with this but I don't remember explicitly checking for difference with thumb entry points either.

That being said, Winphone expects thumb-only for user-space code, so I'd assume m_coff_header_opt.entry does not include the thumb bit, but the kernel still jumps to user code in thumb mode.

source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp

 {
     if (m_entry_point_address.IsValid())
         return m_entry_point_address;
 
     if (!ParseHeader() || !IsExecutable())
         return m_entry_point_address;
 
     SectionList *section_list = GetSectionList();
-    addr_t offset = m_coff_header_opt.entry;
+    addr_t offset = m_coff_header_opt.entry + m_coff_header_opt.image_base;
 
     if (!section_list)
         m_entry_point_address.SetOffset(offset);
     else
         m_entry_point_address.ResolveAddressUsingFileSections(offset, section_list);
     return m_entry_point_address;
 }