Fix selecting the Platform in TargetList::CreateTargetInternal()

[ted]

Summary

TargetList::CreateTargetInternal() will only select the current Platform. A previous patch always sets platform_sp to the current Platform, so a check later to see if platform_sp was not defined always failed, and the current Platform was used. This patch removes that check, so if the current Platform is not compatible with the target architecture, CreateTargetInternal() will call Platform::GetPlatformForArchitecture() to select a compatible Platform.

Vince, remote linux tests (Ubuntu -> remote Ubuntu) pass the same with and without this patch.

[jingham]

IIRC, the point of the previous patch was to make new targets use the currently selected platform, so that you can do:

(lldb) platform select whatever
(lldb) file some_file

and some_file would use the platform you had specified. That seems a useful behavior.

Can you fix this in a way that preserves that behavior?

[ted]

It should use the current Platform, but only if it is compatible with the target's architecture, right? I believe it does this already, because any change of platform_sp is gated by if(!platform_sp->IsCompatibleArchitecture()).

What I'm trying to solve with this is the case where the Host Platform isn't compatible with the target - for example, LLDB built on Linux, but loading a Windows binary:

(lldb) target list
Current targets:

target #0: /usr2/tedwood/lldb_test/infinite_win_x86 ( arch=x86_64-pc-windows-msvc, platform=host )
target #1: /usr2/tedwood/lldb_test/infinite_x86.elf ( arch=x86_64-unknown-linux, platform=host )

platform=host is incorrect for target 0, but correct for target 1. Since LLDB on Linux comes up with the host (Linux) platform selected, platform_sp is valid and a new platform will never be selected, even though the Linux platform isn't compatible with x86_64-pc-windows.

[ted]

Jim, if I select a platform, then load a target that the platform is not compatible with, should it use the incompatible platform, or find one that is compatible?

[clayborg]

It should find one that is compatible.

[ted]

This will do that - platform_sp is the currently selected platform, and both calls to SetSelectedPlatform() are gated by !platform_sp->IsCompatibleArchitecture(), so it will only search for a new platform if the architecture is not compatible with the currently selected platform.

I took current top-of-tree and applied this patch, and built on Linux. Then I checked to see if lldb behaved as expected.
First, I loaded /bin/ls, and checked the platform - host.
Then I selected platform remote-linux, loaded /bin/ls, and checked the platform - remote-linux.
Finally, I selected platform kalimba, loaded /bin/ls, and checked the platform - host.

(lldb) target create /bin/ls
Current executable set to '/bin/ls' (x86_64).
(lldb) target list
Current targets:

target #0: /bin/ls ( arch=x86_64-unknown-linux, platform=host )

(lldb) platform select remote-linux
Platform: remote-linux
Connected: no
(lldb) target create /bin/ls
Current executable set to '/bin/ls' (x86_64).
(lldb) target list
Current targets:

target #0: /bin/ls ( arch=x86_64-unknown-linux, platform=host )
target #1: /bin/ls ( arch=x86_64-unknown-linux, platform=remote-linux )

(lldb) platform select kalimba
Platform: kalimba
Connected: no
(lldb) target create /bin/ls
Current executable set to '/bin/ls' (x86_64).
(lldb) target list
Current targets:

target #0: /bin/ls ( arch=x86_64-unknown-linux, platform=host )
target #1: /bin/ls ( arch=x86_64-unknown-linux, platform=remote-linux )
target #2: /bin/ls ( arch=x86_64-unknown-linux, platform=host )

[ted]

Friendly ping. This behaves as expected - selecting a compatible platform will use it; selecting an incompatible platform and lldb will find a compatible one.

[clayborg]

Looks fine.

[ovyalov]

With this CL process attaching using remote-android is becoming broken - instead of selected remote-android platform default host platform is used to attach.

Could you revert or change this CL?

[ted]

This change will look for a compatible platform if the current platform is not compatible with the current architecture.

Is the android platform not compatible with the selected architecture at this point?

[ted]

I wonder if it's reporting not compatible because the platform isn't connected yet, and currently the non-host linux platform needs to be connected.

D9683 should help with that.

[ovyalov]

remote-android platform is connected when we attach to a process - now
"process attach" is working for me after this CL
https://github.com/llvm-mirror/lldb/commit/0a5560f6118cb7b10920aa59e4e4665c8d8a9c20
Selected platform is remote-android but when we call process attach no
architecture information is passed
to m_interpreter.GetDebugger().GetTargetList().CreateTarget

source/Target/TargetList.cpp

if (user_exe_path && user_exe_path[0])
                         error.SetErrorString(error_strm.GetString().c_str());
                         return error;
                     }
                 }
             }
         }
     }
 
-    if (!platform_sp)
-    {
-        // Get the current platform and make sure it is compatible with the
-        // current architecture if we have a valid architecture.
-        platform_sp = debugger.GetPlatformList().GetSelectedPlatform ();
-        
-        if (!prefer_platform_arch && arch.IsValid())
-        {
-            if (!platform_sp->IsCompatibleArchitecture(arch, false, &platform_arch))
-            {
-                platform_sp = Platform::GetPlatformForArchitecture(arch, &platform_arch);
-                if (platform_sp)
-                    debugger.GetPlatformList().SetSelectedPlatform(platform_sp);
-            }
-        }
-        else if (platform_arch.IsValid())
-        {
-            // if "arch" isn't valid, yet "platform_arch" is, it means we have an executable file with
-            // a single architecture which should be used
-            ArchSpec fixed_platform_arch;
-            if (!platform_sp->IsCompatibleArchitecture(platform_arch, false, &fixed_platform_arch))
-            {
-                platform_sp = Platform::GetPlatformForArchitecture(platform_arch, &fixed_platform_arch);
-                if (platform_sp)
-                    debugger.GetPlatformList().SetSelectedPlatform(platform_sp);
-            }
-        }
-    }
+    // If we have a valid architecture, make sure the current platform is
+    // compatible with that architecture
     
     if (!platform_arch.IsValid())
         platform_arch = arch;
 
     error = TargetList::CreateTargetInternal (debugger,
                                               user_exe_path,
                                               platform_arch,
                                               get_dependent_files,