Page MenuHomePhabricator

Implement GetFileLoadAddress for the Windows process plugin
Needs RevisionPublic

Authored by asmith on Wed, Jan 2, 5:08 PM.

Details

Summary

When a process is loaded, update its sections with the load address to resolve any created breakpoints. For the remote debugging case, the debugged process is launched remotely so GetFileLoadAddress is intended to pass the load address from remote to LLDB (client).

Diff Detail

Event Timeline

asmith created this revision.Wed, Jan 2, 5:08 PM

We should probably have a test for this. It sounds like all you need to do to test this is create a target, set a breakpoint, run the process, then verify that you see some text that says the breakpoint was resolved. With the new build.py script this should be able to work in a cross-platform manner, so maybe it can go under lit/ExecControl and use compiler=any (or don't specify a compiler at all, since that's the default).

labath added a subscriber: labath.Thu, Jan 3, 1:59 AM

I think there is something wrong here. Normally, it should be the job of the dynamic loader plugin to figure out where a module got loaded (and populate the target section load list). Process::GetFileLoadAddress is there to help it with this job. So, the fact that ProcessWindows::GetFileLoadAddress goes back to the target section load list to get the data creates a weird loop. The only way I can see this working is if this implementation happens to return the module base address, as specified in the object file, and that base address happens to be correct (no ASLR). The kind of code I would expect to see here is a call to some windows API to determine the load address straight from the OS (if such a thing is possible). For example, on Linux, we parse the /proc/<pid>/maps file to determine this information.

source/Plugins/Process/Windows/Common/ProcessWindows.cpp
878

I think FileOffset will always be zero here. The only case where this is not zero is for fat macho binaries. And even in that case, I don't think adding that offset to the load address is the right thing to do.

886

typo

Seems like DynamicLoaderWindowsDYLD isn't doing its job correctly here and DynamicLoaderWindowsDYLD should be fixed. We shouldn't have to go to the process at all in order to set the section load addresses correctly. Why? As you might have seen lldb-server.exe is being worked on now and then we will have two clients for DynamicLoaderWindowsDYLD: the native ProcessWindows plug-in and ProcessGDBRemote. The

source/Plugins/Process/Windows/Common/ProcessWindows.cpp
858

This entire function is doing the job of the dynamic loader and should be moved into the dynamic loader.

clayborg requested changes to this revision.Thu, Jan 3, 11:20 AM

So it seems like DynamicLoaderWindowsDYLD is not doings its job correctly and it needs to be fixed. DynamicLoaderWindowsDYLD is responsible for setting all of the section load addresses correctly using an abstract lldb_private::Process plug-in. No special calls should be added if that is possible. We have someone working on lldb-server.exe and then we will have two clients for DynamicLoaderWindowsDYLD: ProcessWindows and ProcessGDBRemote. DynamicLoaderWindowsDYLD should work for any lldb_private::Process subclass without the need to call back into a specific lldb_private::Process subclass method.

This revision now requires changes to proceed.Thu, Jan 3, 11:20 AM
asmith marked an inline comment as done.Mon, Jan 14, 10:25 PM
source/Plugins/Process/Windows/Common/ProcessWindows.cpp
858

when do you think the changes you mentioned for the loader and lldb-server.exe would be available for review? should we abandon this and wait for those?

clayborg added inline comments.Tue, Jan 15, 7:12 AM
source/Plugins/Process/Windows/Common/ProcessWindows.cpp
858

I am saying the DynamicLoaderWindowsDYLD should be fixed, so that no matter how we connect to a windows process, we get correct DYLD functionality. The way this patch is currently coded, you require every process plug-in that connects to windows processes to have this work around.