[lldb-dev] Multiple platforms with the same name

[Pavel Labath via lldb-dev]

I'm sorry for the slow response. I had to attend to some other things 
first. It sounds like there's agreement to support multiple platform 
instances, so I'll try to move things in that direction.

Further responses inline

On 20/01/2022 01:19, Greg Clayton wrote:
> 
> 
>> On Jan 19, 2022, at 4:28 AM, Pavel Labath <pavel at labath.sk> wrote:
>>
>> On 19/01/2022 00:38, Greg Clayton wrote:
>>> Platforms can contain connection specific setting and data. You might want to create two different "remote-linux" platforms and connect each one to a different remote linux machine. Each target which uses this platform would each be able to fetch files, resolve symbol files, get OS version/build string/kernel info, get set working directory from the remote server they are attached. Since each platform tends to belong to a target and since you might want to create two different targets and have each one connected to a different remote machine, I believe it is fine to have multiple instances.
>>> I would vote to almost always create a new instance unless it is the host platform. Though it should be possible to create to targets and possibly set the platform on one target using the platform from another that might already be connected.
>>> I am open to suggestions if anyone has any objections.
>>> Greg
>>
>> I agree that permitting multiple platforms would be a more principled position, but it was not clear to me if that was ever planned to be the case.
> 
> This code definitely evolved as time went on. Then we added the remote capabilities. As Jim said, there are two parts for the platform that _could_ be separated: PlatformLocal and PlatformRemote. Horrible names that can be improved upon, I am sure, but just names I quickly came up with.
> 
> PlatformLocal would be "what can I do for a platform that only involves finding things on this machine for supporting debugging on a remote platform". This would involve things like:
> - where are remote files cached on the local machine for easy access
> - where can I locate SDK/NDK stuff that might help me for this platform
> - what architectures/triples are supported by this platform so it can be selected
> - how to start a debug session for a given binary (which might use parts of PlatformRemote) as platforms like "iOS-simulator" do not require any remote connections to be able to start a process. Same could happen for VM based debugging on a local machine.
> 
> PlatformRemote
> - get/put files
> - get/set working directory
> - install executable so OS can see/launch it
> - create/delete directories
> 
> So as things evolved, everything got thrown into the Platform case and we just made things work as we went. I am sure this can be improved.
I actually have a branch where I've tried to separate the local and 
remote cases, and remove the if(IsHost()) checks everywhere, but I 
haven't found yet found the time to clean it up and send an rfc.

> 
>>
>> If it was (or if we want it to be), then I think we need to start making bigger distinctions between the platform plugins (classes), and the actual instantiations of those classes. Currently there is no way to refer to "older" instances of the platforms as they all share the same name (the name of the plugin). Like, you can enumerate them through SBDebugger.GetPlatformAtIndex(), but that's about the only thing you can do as all the interfaces (including the SB ones) take a platform _name_ as an argument. This gets particularly confusing as in some circumstances we end up choosing the newer one (e.g. if its the "current" platform) and sometimes the older.
>>
>> If we want to do that, then this is what I'd propose:
>> a) Each platform plugin and each platform instance gets a name. We enforce the uniqueness of these names (within their category).
> 
> Maybe it would be better to maintain the name, but implement an instance identifier for each platform instance?
I'm not sure what you mean by that. Or, if you mean what I think you 
mean, then we're actually in agreement. Each platform plugin (class) 
gets a name (or identifier, or whatever we want to call it), and each 
instance of that class gets a name as well.

Practically speaking, I think we could reuse the existing GetPluginName 
and GetName (currently hardwired to return GetPluginName()). The former 
would return the plugin name, and the latter would give the "instance 
identifier".

> 
>> b) "platform list" outputs two block -- the list of available plugins and the list of plugin instances
> 
> If we added a instance identifier, then we could just show the available plug-in names followed by their instances?
Yes, that would just be a different (and probably better) way of 
displaying the same information. We can definitely do that.

> 
>> c) a new "platform create" command to create a platform
>>   - e.g. "platform create my-arm-test-machine --plugin remote-linux"
> 
> Now we are assuming you want to connect to a remote machine when we create platform? "platform connect" can be used currently if we want to actually connect to a remote platform, but there is a lot of stuff in the iOS platforms that really only deals with finding stuff on the local machine. Each platform plugin in "platform connect" has the ability to create its own unique connection arguments and options which is nice for different platforms.
> 
> The creation and connecting should still be done separately. Seeing the arguments you added above leads me to believe this is like a "select" and a "connect" all in one. And each "platform connect" has unique and different arguments and options that are tailored to each plug-in currently.

I'm not assuming anything here. "my-arm-test-machine" is just an 
instance identifier for this particular platform. It does not have to 
correspond to any existing domain name or anything else.

The command should be read as "create a platform instance with the name 
'my-arm-test-machine' using the plugin 'remote-linux'. It would create 
the instance in the default (for remote platforms, that means 
unconnected) state. To connect, one would still need to issue a platform 
connect command.

> 
>> d) "platform select" selects the platform with the given /instance/ name
>>   - for convenience and compatibility if the name does not refer to any existing platform instance, but it *does* refer to a platform plugin, it would create a platform instance with the same name as the class. (So the first "platform select remote-linux" would create a new instance (also called remote-linux) and all subsequent selects would switch to that one -- a change to existing behavior)
> 
> that is fine. We just have to make sure that this still works:
> 
> (lldb) platform select remote-linux
> (lldb) file /path/to/ios/binary/Foo.app
> 
> The target created with "/path/to/ios/binary/Foo.app" should select the "remote-ios" platform. It is fine to always check if "remote-linux" platform would work for this binary first, but when it clearly doesn't, it should fall back to the mechanism that we currently have where we auto select a compatible platform.
Yes, I have no intention of changing that.

> 
> The other option is to let "platform select <name>" create a new instance of the platform if an instance already exists? If we are in interactive mode, like in the command interpreter, we can query the user to say "There is already an instance of platform "<name>" that exists, do you want to create a new one?".
> 
> Might also be nice to add a new option that allows us to create a new instance with "--always-create" or something like that in case we do want to have two instances with the same plug-in. Then users of course can select the platform with the unique instance name.

Personally, I have found it very confusing that something called 
"select" actually creates an instance. Having an explicit "create" 
command would be more consistent with "target create" for instance.

That said, the syntax I proposed above is somewhat verbose, and I am 
definitely open to changing that. One possibility would be to turn the 
names around and have "platform create remote-linux --name foo", where 
if you don't specify the name argument, then the lldb automatically 
generates a new name.

Theoretically we could even drop instance names completely, and refer to 
platforms by their index (like we do for targets, etc.), but then we'd 
have to do something about the APIs which already expect a name.

> 
>> e) SBPlatform gets a static factory function taking two string arguments
> 
> What are the two arguments? We can't change existing LLDB API, but we can add more. Keep that in mind as we ponder changes.
The plugin name and the instance name. So,

my_new_platform = lldb.SBPlatform.Create("remote-linux", "foo")

would create an instance of the remote-linux plugin, and call it "foo".


> Each setting has the option of saying "I am a global setting" or "I am an instance setting". It should be easy to set the right settings up as global or instance based right?

That might be possible, though I'm not sure what it would take to do 
that. I don't think we have any instance-specific plugin settings at the 
moment.

It would be slightly weird though, because the plugin settings include 
plugin names in their paths. Unlike say target.run-args, where that name 
always refers to the currently selected target, plugin specific settings 
have names like platform.plugin.qemu-user.emulator-path. That raises 
questions like what should that name refer to if the currently selected 
platform is not of the "qemu-user" class.

I think the only reasonable answer is "the global version of that 
setting", but that still feels a bit strange. In a way, it might be 
better if we had a "platform" setting, which always referred to settings 
for the current platform, but I am not totally happy with that idea 
either (then we'd have settings appearing and disappearing based on the 
current selection).

But I don't think we need to solve this right now. I think we can first 
properly support multiple platform instances, and then figure out what 
to do about settings.

pl