Re: [lldb-dev] Multiple platforms with the same name
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 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
Re: [lldb-dev] Multiple platforms with the same name
> On Jan 19, 2022, at 4:28 AM, Pavel Labath 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. > > 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? > 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? > 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. > 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
Re: [lldb-dev] Multiple platforms with the same name
> On Jan 19, 2022, at 4:28 AM, Pavel Labath 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. We made a choice early on in lldb that it would be a one-to-many debugger (as opposed to gdb where you use one gdb process to debug one inferior). The idea was to allow people who have more complex inter-app communications to use the scripting features of lldb to make the process of debugging IPC and such-like more natural (like a “step-in” that steps across process boundaries when you step into a message dispatch). Or to run two instances that are slightly different and compare the paths through some bit of code. Or other cool uses we hadn’t thought of. I don’t do this kind of debugging much either, but then I just debug lldb all the time which is a fairly simple process, and it’s communication with the stub is pretty simple. So I don’t think that’s dispositive for how useful this design actually is... Since the Platform class holds details about the current debug sessions on that platform, it has to take part in this design, which means either allowing one Platform to connect to all instances of it’s kind that lldb might want to debug, or making one Platform per instance. The latter design was what we had always intended, it is certainly how we’ve talked about it for as long as I can remember. OTOH, the whole Platform class is a bit of a mashup, since it holds both “things you need to know about a class of systems in order to debug on them” and “the connection you make to a particular instance”. I think the intention would be clearer if we separated the “PlatformExpert” part of Platform and the “the Remote machine I’m talking to” part of Platform. > > 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). > b) "platform list" outputs two block -- the list of available plugins and the > list of plugin instances > c) a new "platform create" command to create a platform > - e.g. "platform create my-arm-test-machine --plugin remote-linux" > 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) > e) SBPlatform gets a static factory function taking two string arguments > f) existing SBPlatform constructor (taking one string) creates a new platform > instance with a name selected by us (remote-linux, remote-linux-2, etc.), but > its use is discouraged/deprecated. > g) all other existing APIs (command line and SB) remain unchanged but any > "platform name" argument is taken to mean the platform instance name, and it > has the "platform select" semantics (select if it exists, create if it > doesn't) > > I think this would strike a good balance between
Re: [lldb-dev] Multiple platforms with the same name
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. 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). b) "platform list" outputs two block -- the list of available plugins and the list of plugin instances c) a new "platform create" command to create a platform - e.g. "platform create my-arm-test-machine --plugin remote-linux" 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) e) SBPlatform gets a static factory function taking two string arguments f) existing SBPlatform constructor (taking one string) creates a new platform instance with a name selected by us (remote-linux, remote-linux-2, etc.), but its use is discouraged/deprecated. g) all other existing APIs (command line and SB) remain unchanged but any "platform name" argument is taken to mean the platform instance name, and it has the "platform select" semantics (select if it exists, create if it doesn't) I think this would strike a good balance between a consistent interface and preserving existing semantics. The open questions are: - is it worth it? While nice in theory, personally I have never actually needed to connect to more than one machine at the same time. - what to do about platform-specific settings. The functionality has existed for a long time, but there was only one plugin (PlatformDarwinKernel) using it. I've now added a bunch of settings to the qemu-user platform on the assumption that there will only be one instance of the class. These are global, but they would really make more sense on a per-instance basis. We could either leave it be (I don't need multiple instances now), or come up with a way to have per-platform settings, similar like we do for targets. We could also do something with the "platform settings" command, which currently only sets the working directory. Let me know what you think, Pavel ___ lldb-dev mailing list lldb-dev@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
Re: [lldb-dev] Multiple platforms with the same name
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
> On Jan 17, 2022, at 8:18 AM, Pavel Labath wrote:
>
> Hello all,
>
> currently our code treats platform name more-or-less as a unique identifier
> (e.g. Platform::Find returns at most one platform instance --the first one it
> finds).
>
> This is why I was surprised that the "platform select" CLI command always
> creates a new instance of the given platform, even if the platform of a given
> name already exists. This is because Platform::Create does not search the
> existing platform list before creating a new one. This might sound reasonable
> at first, but for example the Platform::Create overload which takes an
> ArchSpec first tries to look for a compatible platforms among the existing
> ones before creating a new one.
>
> For this reason, I am tempted to call this a bug and fix the name-taking
> Create overload. This change passes the test suite, except for a single test,
> which now gets confused because some information gets leaked from one test to
> another. (although our coverage of the Platform class in the tests is fairly
> weak)
>
> However, this test got me thinking. It happens to use the the SB way of
> manipulating platforms, and "creates" a new instance as
> lldb.SBPlatform("remote-linux"). For this kind of a command, it would be
> reasonable/expected to create a new instance, were it not for the fact that
> this platform would be very tricky to access from the command line, and even
> through some APIs -- SBDebugger::CreateTarget takes a platform _name_.
>
> So, which one is it? Should we always have at most one instance of each
> platform, or are multiple instances ok?
> cheers,
> pl
>
> PS: In case you're wondering about how I run into this, I was trying to
> create a pre-configured platform instance in (e.g.) an lldbinit file, without
> making it the default. That way it would get automatically selected when the
> user opens an executable of the appropriate type. This actually works,
> *except* for the case when the user selects the platform manually. That's
> because in that case, we would create an empty/unpopulated platform, and it
> would be the one being selected because it was the /current/ platform.
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
