[Lldb-commits] [lldb] [lldb] Handle accelerator plugin breakpoint actions on the client (PR #201489)

[David Spickett via lldb-commits]

================
@@ -4124,6 +4143,181 @@ bool ProcessGDBRemote::NewThreadNotifyBreakpointHit(
   return false;
 }
 
+namespace {
+/// Baton that carries the breakpoint hit arguments to the accelerator plugin
+/// breakpoint callback.
+class AcceleratorBreakpointCallbackBaton
+    : public TypedBaton<AcceleratorBreakpointHitArgs> {
+public:
+  explicit AcceleratorBreakpointCallbackBaton(
+      std::unique_ptr<AcceleratorBreakpointHitArgs> data)
+      : TypedBaton(std::move(data)) {}
+};
+} // namespace
+
+Status
+ProcessGDBRemote::HandleAcceleratorActions(const AcceleratorActions &actions) {
+  Log *log = GetLog(GDBRLog::Process);
+
+  // The same set of actions can be delivered to the client more than once: a
+  // plugin may keep reporting the same actions (with the same identifier) on
+  // subsequent native stops until its state advances. The identifier uniquely
+  // names a set of actions for a plugin, so skip any set we have already
+  // processed to avoid re-running its side effects (e.g. setting the same
+  // breakpoints again).
+  auto it = m_processed_accelerator_actions.find(actions.plugin_name);
+  if (it != m_processed_accelerator_actions.end() &&
+      it->second == actions.identifier) {
+    LLDB_LOG(log,
+             "ProcessGDBRemote::HandleAcceleratorActions skipping already "
+             "processed actions for plugin '{0}' with identifier {1}",
+             actions.plugin_name, actions.identifier);
+    return Status();
+  }
+  m_processed_accelerator_actions[actions.plugin_name] = actions.identifier;
+
+  // Handle each kind of action. More action kinds will be handled here in the
+  // future, so only return early on error; otherwise fall through so the next
+  // kind of action still gets a chance to run.
+  if (!actions.breakpoints.empty()) {
+    if (Status error = HandleAcceleratorBreakpoints(actions); error.Fail())
+      return error;
+  }
+
+  return Status();
+}
+
+Status ProcessGDBRemote::HandleAcceleratorBreakpoints(
+    const AcceleratorActions &actions) {
+  Target &target = GetTarget();
+  Status error;
+  for (const AcceleratorBreakpointInfo &bp : actions.breakpoints) {
+    // Carry data with the breakpoint so the callback can notify the plugin
+    // when the breakpoint is hit.
+    auto args_up = std::make_unique<AcceleratorBreakpointHitArgs>();
+    args_up->plugin_name = actions.plugin_name;
+    args_up->breakpoint = bp;
+
+    // Each breakpoint must specify exactly one of by_name or by_address.
+    BreakpointSP bp_sp;
+    if (bp.by_name && bp.by_address) {
+      // Record the first error but keep setting the remaining breakpoints.
+      if (error.Success())
+        error = Status::FromErrorStringWithFormatv(
+            "accelerator breakpoint {0} specifies both a by_name and a "
+            "by_address specification",
+            bp.identifier);
+      continue;
+    } else if (bp.by_name) {
+      FileSpecList bp_modules;
+      if (bp.by_name->shlib && !bp.by_name->shlib->empty())
+        bp_modules.Append(FileSpec(*bp.by_name->shlib));
+      bp_sp = target.CreateBreakpoint(
+          bp_modules.GetSize() ? &bp_modules : nullptr, // Containing modules.
+          nullptr,                                      // Containing source.
+          bp.by_name->function_name.c_str(),            // Function name.
+          eFunctionNameTypeFull,                        // Function name type.
+          eLanguageTypeUnknown,                         // Language type.
+          0,                                            // Byte offset.
+          false,                                        // Offset is insn count.
+          eLazyBoolNo,                                  // Skip prologue.
+          true,                                         // Internal breakpoint.
+          false);                                       // Request hardware.
+    } else if (bp.by_address) {
+      bp_sp = target.CreateBreakpoint(bp.by_address->load_address,
+                                      /*internal=*/true,
+                                      /*request_hardware=*/false);
+    } else {
+      if (error.Success())
+        error = Status::FromErrorStringWithFormatv(
+            "accelerator breakpoint {0} has neither a by_name nor a by_address "
+            "specification",
+            bp.identifier);
+      continue;
+    }
+
+    if (!bp_sp) {
+      if (error.Success())
+        error = Status::FromErrorStringWithFormatv(
+            "failed to set accelerator breakpoint {0}", bp.identifier);
+      continue;
+    }
+
+    // Give the internal breakpoint a meaningful description for stop reasons.
+    bp_sp->SetBreakpointKind("accelerator-plugin");
----------------
DavidSpickett wrote:

It assume it would be nice to know the name of the plugin too. `accelerator plugin (AMDGPU)` maybe.

If you do add it, first check what `SetBreakpointKind` assumes about the lifetime of the string.

https://github.com/llvm/llvm-project/pull/201489