This is an archive of the discontinued LLVM Phabricator instance.

[lldb] Improve breakpoint management for interactive scripted process
ClosedPublic

Authored by mib on Apr 17 2023, 11:11 AM.

Details

Summary

This patch improves breakpoint management when doing interactive
scripted process debugging.

In other to know which process set a breakpoint, we need to do some book
keeping on the multiplexer scripted process. When initializing the
multiplexer, we will first copy breakpoints that are already set on the
driving target.

Everytime we launch or resume, we should copy breakpoints from the
multiplexer to the driving process.

When creating a breakpoint from a child process, it needs to be set both
on the multiplexer and on the driving process. We also tag the created
breakpoint with the name and pid of the originator process.

This patch also implements all the requirement to achieve proper
breakpoint management. That involves:

  • Adding python interator for breakpoints and watchpoints in SBTarget
  • Add a new ScriptedProcess.create_breakpoint python method

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

Diff Detail

Event Timeline

mib created this revision.Apr 17 2023, 11:11 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 17 2023, 11:11 AM
mib requested review of this revision.Apr 17 2023, 11:11 AM
mib updated this revision to Diff 514326.Apr 17 2023, 11:13 AM

Re-format

Seems okay to me, but it's a little messy that we're having to manage breakpoints like this.

lldb/bindings/interface/SBTargetExtensions.i
144–171

Are these used at all?

lldb/test/API/functionalities/interactive_scripted_process/interactive_scripted_process.py
310–319

It's interesting that we dump to a file. It'd be cool if we could dump it to a StructuredData or something instead of a file.

323

Why do we delete all of the breakpoints just to re-set them afterwards? Is there a difference between what we set and what was there before?

mib added a comment.Apr 17 2023, 2:17 PM

Seems okay to me, but it's a little messy that we're having to manage breakpoints like this.

I don't think there is a better way to do this, as breakpoint are managed per target but we're actually trying to copy them from one target to another. Let me know if you can think of another of doing it

mib marked 3 inline comments as done.Apr 17 2023, 2:25 PM
mib added inline comments.
lldb/bindings/interface/SBTargetExtensions.i
144–171

Not in this patch but I feel like it should be implemented already and this goes along with the breakpoint accessors and iterator.

lldb/test/API/functionalities/interactive_scripted_process/interactive_scripted_process.py
310–319

100% agree, I'll do that as a followup.

323

There could be: Let's say the user set a breakpoint on the driving target (A) and a different one on the multiplexer target (B). We need first to copy all the driving target breakpoints to the multiplexer target, delete them and copy back all the multiplexer target breakpoints (A + B) to the driving target, to keep them in sync.

mib marked 3 inline comments as done.Apr 20 2023, 3:27 PM

LGTM but I'll hold off on accepting to give Alex a change to take another look as he had more significant comments.

lldb/source/Interpreter/ScriptInterpreter.cpp
78–91

NTBF for this patch but we should really be using templates here to avoid code duplication.

lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
124

return false

bulbazord accepted this revision.Apr 21 2023, 9:45 AM

one nit but otherwise LGTM.

lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
124

+1

This revision is now accepted and ready to land.Apr 21 2023, 9:45 AM
mib updated this revision to Diff 515902.Apr 21 2023, 1:38 PM
mib marked 2 inline comments as done.

Re-format python code with black