This is an archive of the discontinued LLVM Phabricator instance.

[lldb/Plugin] Add breakpoint setting support to ScriptedProcesses.
ClosedPublic

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

Details

Summary

This patch adds support for breakpoint setting to Scripted Processes.

For now, Scripted Processes only support setting software breakpoints.

When doing interactive scripted process debugging, it makes use of the
memory writing capability to write the trap opcodes in the memory of the
driving process. However the real process' target doesn't keep track of
the breakpoints that got added by the scripted process. This is a design
that we might need to change in the future, since we'll probably need to
do some book keeping to handle breakpoints that were set by different
scripted processes.

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

Diff Detail

Event Timeline

mib created this revision.Mar 3 2023, 9:40 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 3 2023, 9:40 PM
mib requested review of this revision.Mar 3 2023, 9:40 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:45 PM

I've landed this by mistake, please take another look

bulbazord accepted this revision.Apr 17 2023, 1:13 PM
bulbazord added a subscriber: bulbazord.

Seems ok to me, one small thought. Shouldn't hold this patch up though.

lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
270–271

I like the assert, but I can't help but wonder if we should change the signature of this method? Something like EnableBreakpointSite(BreakpointSite &bp_site) or something like this... Seems like the other overrides assume that it is not nullptr before using it and all the callsites verify it before calling it too.

This revision is now accepted and ready to land.Apr 17 2023, 1:13 PM
JDevlieghere added inline comments.Apr 17 2023, 1:49 PM
lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
270–271

+1. I wondered the same thing when looking at the header.

281

The same applies to Process::EnableSoftwareBreakpoint as well. It also has an assert and seems like it should just be checked in the caller and enforced by using a reference.

lldb/source/Plugins/Process/scripted/ScriptedProcess.h
75

Should this take a reference instead?