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 <pa...@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
_______________________________________________
lldb-dev mailing list
lldb-dev@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
