[Lldb-commits] TargetList.cpp changes from [PATCH] Merge of Linux and FreeBSD

[Greg Clayton]

On Jan 4, 2012, at 9:25 PM, dawn at burble.org wrote:

> Here's some background for why the changes to TargetList.cpp were needed.  
> 
> On Wed, Jan 04, 2012 at 06:04:17PM -0800, dawn at burble.org wrote:
>>    lldb_svnR147560_POSIX_TargetList.patch - changes to TargetList.cpp separated out.
>> 
>> The changes to TargetList.cpp are needed by both BSD and Linux in order for
>> Attach() to work.  I spent a lot of time debugging this one, and the BSD folks
>> came to the same conclusion.  I'll dig through my notes and try to write up my
>> findings - should this be discussed here or on lldb-dev?
> 
> Perhaps the BSD folks can add more based on their experience, but
> here's what I remember the problem being:
> 
> When attaching, TargetList::CreateTarget() is called with
> file = 0 since it's not known yet.
> So in the nested call to TargetList::CreateTarget() we do
>    target_sp.reset(new Target(debugger, arch, platform_sp));
>    (around line TargetList.cpp:155).
> This causes Target to be created with an invalid arch, and
> things head downhill from there.

The DynamicLoader plug-in is not doing everything that it needs to do on linux is my guess here. It is the job of the dynamic loader to figure out what you have attached to and figure out the architecture and the executable file. On darwin this is done in the "DynamicLoaderMacOSXDYLD::UpdateImageInfosHeaderAndLoadCommands(...)" function, take a look and see how it sets the executable.

The dynamic loader has to be able to set the executable on the target because you might do something like:

/usr/bin/lldb
(lldb) process attach --pid 123

Just setting the target architecture to the current host default is the wrong thing to do, because what if we had done this:

/usr/bin/lldb
(lldb) platform select remote-ios
(lldb) platform connect ....
(lldb) process attach --pid 123

Now you will have a target with "x86_64" as the architecture and you will try to attach to a remote ARM process which is bad.

When a target's executable is set it will grab the architecture from the executable module if it already hasn't bee set (see "Target::SetExecutableModule(...)").


> We do have a default arch set in platform_sp however from:
>    platform_sp = debugger.GetPlatformList().GetSelectedPlatform (); 
>    (around line TargetList.cpp:73).
> giving us m_selected_platform_sp.m_system_arch.m_triple.Data
>            = "i386-pc-linux-gnu".
> But no where is this default copied to the target.
> What's supposed to happen here?

The DynamicLoader needs to do its job as stated above. It should be ok for the target to have an invalid architecture until the dynamic loader has detected the architecture and set the executable.

The other issue that you could run into, and the reason you want your dynamic loader to change the executable is what if you did this:

/usr/bin/lldb
(lldb) target create --arch x86_64-apple-darwin /bin/ls
(lldb) process attach --pid 123

And what if pid "123" is actually "/bin/cat". And even worse, what if the architecture is different? (this won't happen on linux because linux is a single arch OS AFAIK, but on darwin we can run both 32 and 64 bit version of a binary, so we can actually run into this:

/usr/bin/lldb
(lldb) target create --arch i386-apple-darwin /bin/ls
(lldb) process attach --pid 123

Where pid 123 is /bin/ls, but it is actually the x86_64 slice....

> As a work-around, could we add code like
>    if (!arch.IsValid())
>        arch = Host::GetArchitecture(Host::eSystemDefaultArchitecture);

Try the above approach and see how that works for you. The work around above will cause problems for remote debugging.

> before the call to
>    target_sp.reset(new Target(debugger, arch, platform_sp));
>    (around line TargetList.cpp:155)?
> 
> 
> Thanks for any help,
>    -Dawn

Let me know if anything above doesn't make sense. And there is no documentation on this stuff yet, so its not like anyone would just know this information, so this is our fault for not documenting the expected behavior... Sorry about that.

Greg Clayton


> _______________________________________________
> lldb-commits mailing list
> lldb-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits