This is an archive of the discontinued LLVM Phabricator instance.

[lldb] Move ScriptedProcess private state update to implementation
ClosedPublic

Authored by mib on Mar 3 2023, 9:39 PM.

Details

Summary

While debugging a Scripted Process, in order to update its state and
work nicely with lldb's execution model, it needs to toggle its private
state from running to stopped, which will result in broadcasting a
process state changed event to the debugger listener.

Originally, this state update was done systematically in the Scripted
Process C++ plugin, however in order to make scripted process
interactive, we need to be able to update their state dynamically.

This patch makes use of the recent addition of the
SBProcess::ForceScriptedState to programatically, and moves the
process private state update to the python implementation of the resume
method instead of doing it in ScriptedProcess::DoResume.

This patch also removes the unused ShouldStop & Stop scripted
process APIs, and adds new ScriptedInterface transform methods for
boolean arguments. This allow the user to programmatically decide if
after running the process, we should stop it (which is the default setting).

Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>

Diff Detail

Event Timeline

mib created this revision.Mar 3 2023, 9:39 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 3 2023, 9:39 PM
mib requested review of this revision.Mar 3 2023, 9:39 PM
This revision was not accepted when it landed; it landed in state Needs Review.Mar 6 2023, 1:14 PM
This revision was automatically updated to reflect the committed changes.
mib reopened this revision.Mar 6 2023, 3:44 PM

Landed this by mistake, please take another look

bulbazord added inline comments.Mar 13 2023, 3:29 PM
lldb/examples/python/scripted_process/scripted_process.py
167–170

This seems incomplete?

lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h
151

Should we do something with the error here?

mib updated this revision to Diff 504877.Mar 13 2023, 3:45 PM
mib marked 2 inline comments as done.

Address @bulbazord comments.

No problem from me. Jonas?

This revision is now accepted and ready to land.Mar 13 2023, 4:41 PM
dyung added a subscriber: dyung.Apr 25 2023, 5:12 PM

Hi @mib, your change is causing a build failure on two build bots, can you take a look and revert if you need time to investigate?

https://lab.llvm.org/buildbot/#/builders/217/builds/20430
https://lab.llvm.org/buildbot/#/builders/243/builds/5438

mib added a comment.Apr 25 2023, 5:20 PM

Hi @mib, your change is causing a build failure on two build bots, can you take a look and revert if you need time to investigate?

https://lab.llvm.org/buildbot/#/builders/217/builds/20430
https://lab.llvm.org/buildbot/#/builders/243/builds/5438

Yep, I have a fix already. I'll push it in a few minutes. Thanks for the follow-up @dyung.

mib added a comment.Apr 25 2023, 5:25 PM

Hi @mib, your change is causing a build failure on two build bots, can you take a look and revert if you need time to investigate?

https://lab.llvm.org/buildbot/#/builders/217/builds/20430
https://lab.llvm.org/buildbot/#/builders/243/builds/5438

Yep, I have a fix already. I'll push it in a few minutes. Thanks for the follow-up @dyung.

@dyung D149218 should fix the build failure.

dyung added a comment.Apr 25 2023, 5:41 PM

Hi @mib, your change is causing a build failure on two build bots, can you take a look and revert if you need time to investigate?

https://lab.llvm.org/buildbot/#/builders/217/builds/20430
https://lab.llvm.org/buildbot/#/builders/243/builds/5438

Yep, I have a fix already. I'll push it in a few minutes. Thanks for the follow-up @dyung.

@dyung D149218 should fix the build failure.

It does, thanks!