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

[mib]

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 <[e-mail]>

[mib]

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

[mib]

ping @JDevlieghere @jingham

[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.

[JDevlieghere]

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?

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

public:
   bool IsAlive() override;
 
   size_t DoReadMemory(lldb::addr_t addr, void *buf, size_t size,
                       Status &error) override;
 
   size_t DoWriteMemory(lldb::addr_t vm_addr, const void *buf, size_t size,
                        Status &error) override;
 
+  Status EnableBreakpointSite(BreakpointSite *bp_site) override;
+
   ArchSpec GetArchitecture();
 
   Status
   GetMemoryRegions(lldb_private::MemoryRegionInfos &region_list) override;
 
   bool GetProcessInfo(ProcessInstanceInfo &info) override;
 
   lldb_private::StructuredData::ObjectSP

lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp

return ScriptedInterface::ErrorWithMessage<size_t>(
         LLVM_PRETTY_FUNCTION, "Failed to copy write buffer to memory.", error);
 
   // FIXME: We should use the diagnostic system to report a warning if the
   // `bytes_written` is different from `size`.
 
   return bytes_written;
 }
 
+Status ScriptedProcess::EnableBreakpointSite(BreakpointSite *bp_site) {
+  assert(bp_site != nullptr);
+
+  if (bp_site->IsEnabled()) {
+    return {};
+  }
+
+  if (bp_site->HardwareRequired()) {
+    return Status("Scripted Processes don't support hardware breakpoints");
+  }
+
+  return EnableSoftwareBreakpoint(bp_site);
+}
+
 ArchSpec ScriptedProcess::GetArchitecture() {
   return GetTarget().GetArchitecture();
 }
 
 Status ScriptedProcess::DoGetMemoryRegionInfo(lldb::addr_t load_addr,
                                               MemoryRegionInfo &region) {
   Status error;
   if (auto region_or_err =