mib marked 3 inline comments as done. mib added a subscriber: jingham. mib added inline comments.
================ Comment at: lldb/examples/python/scripted_process/scripted_platform.py:31 + def list_processes(self): + """ Get a list of processes that can be ran on the platform. + ---------------- labath wrote: > mib wrote: > > mib wrote: > > > mib wrote: > > > > labath wrote: > > > > > I am surprised that you want to go down the "run" path for this > > > > > functionality. I think most of the launch functionality does not make > > > > > sense for this use case (e.g., you can't provide arguments to these > > > > > processes, when you "run" them, can you?), and it is not consistent > > > > > with what the "process listing" functionality does for regular > > > > > platforms. > > > > > > > > > > OTOH, the "attach" flow makes perfect sense here -- you take the pid > > > > > of an existing process, attach to it, and stop it at a random point > > > > > in its execution. You can't customize anything about how that process > > > > > is run (because it's already running) -- all you can do is choose how > > > > > you want to select the target process. > > > > For now, there is no support for attaching to a scripted process, > > > > because we didn't have any use for it quite yet: cripted processes were > > > > mostly used for doing post-mortem debugging, so we "ran" them > > > > artificially in lldb by providing some launch options (the name of the > > > > class managing the process and an optional user-provided dictionary) > > > > through the command line or using an `SBLaunchInfo` object. > > > > > > > > I guess I'll need to extend the `platform process launch/attach` > > > > commands and `SBAttachInfo` object to also support these options since > > > > they're required for the scripted process instantiation. > > > > > > > > Note that we aren't really attaching to the real running process, we're > > > > creating a scripted process that knows how to read memory to mock the > > > > real process. > > > @labath, I'll do that work on a follow-up patch > > @labath here D139945 :) > Thanks. However, are you still planning to use the launch path for your > feature? Because if you're not, then I think this comment should say "Get a > list of processes that **are running**" (or that **can be attached to**). > > And if you are, then I'd like to hear your thoughts on the discrepancy > between what "launching" means for scripted and non-scripted platforms. > The way I see it is that the scripted platform will create a process with the right process plugin. In the case of scripted processes, the `ProcessLaunchInfo` argument should have the script class name set (which automatically sets the process plugin name to "ScriptedProcess" in the launch info). Once the process is instantiated (before the launch), the scripted platform will need to redirect to process stop events through its event multiplexer. So the way I see it essentially, running a regular process with the scripted platform should be totally transparent. Something that is also worth discussing IMO, is the discrepancy between launching and attaching from the scripted platform: One possibility could be that `platform process launch` would launch all the scripted processes listed by the scripted platform and set them up with the multiplexer, whereas `platform process attach` would just create a scripted process individually. I know this doesn't match the current behavior of the platform commands so if you guys think we should preserve the expected behavior, I guess. May be @jingham has some opinion about this ? ================ Comment at: lldb/test/API/functionalities/scripted_platform/my_scripted_platform.py:33 +def __lldb_init_module(debugger, dict): + if not 'SKIP_SCRIPTED_PLATFORM_SELECT' in os.environ: + debugger.HandleCommand( ---------------- labath wrote: > ?? This is so the user can decide if the `platform select` command should be ran when the script is imported in lldb or not. If the user sets this env variable, the command will only be printed to the user, so they can copy it and run it whenever they want. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139250/new/ https://reviews.llvm.org/D139250 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
