[Lldb-commits] [lldb] [LLDB][Part 3] Support enabling/disabling InstrumentationRuntime plugins in an debug session (PR #193334)

[Jonas Devlieghere via lldb-commits]

================
@@ -2503,13 +2547,110 @@ llvm::StringRef PluginManager::PluginDomainKindToStr(PluginDomainKind kind) {
 llvm::Error PluginManager::SetInstrumentationRuntimePluginEnabled(
     llvm::StringRef name, bool enable, Debugger &requesting_debugger,
     PluginDomainKind domain) {
-  if (domain != lldb::ePluginDomainKindGlobal)
-    return llvm::createStringErrorV("{} domain is not supported",
-                                    PluginDomainKindToStr(domain));
-  if (!GetInstrumentationRuntimeInstances().SetInstanceEnabled(name, enable))
-    return llvm::createStringError("plugin could not be found");
 
-  return llvm::Error::success();
+  auto GetInstrumentationRuntimeTy =
+      [&]() -> llvm::Expected<lldb::InstrumentationRuntimeType> {
+    auto type_cb = GetInstrumentationRuntimeInstances().GetTypeCallbackForName(
+        name, /*enabled_only=*/false);
+    if (!type_cb)
+      return llvm::createStringErrorV(
+          "Could not get InstrumentationRuntimeType for plugin {}", name);
+    return type_cb();
+  };
+
+  switch (domain) {
+  case lldb::ePluginDomainKindGlobal:
+    // Update the global enablement flag
+    if (!GetInstrumentationRuntimeInstances().SetInstanceEnabled(name, enable))
+      return llvm::createStringErrorV("could not find plugin {}", name);
+    // We should in principle iterate over all debuggers and enable/disable
+    // their runtimes. However, this doesn't work because we need to hold
+    // `GetDebuggerListMutex` to safely iterate and this can cause deadlock when
+    // trying to activate a plugin (creating a breakpoint might call
+    // `Debugger::ReportProgress` which also tries to lock the mutex). For now
+    // just don't update live processes.
+    // FIXME: We should probably emit a warning about this.
+    return llvm::Error::success();
+  case lldb::ePluginDomainKindDebugger: {
+    // Deliberately don't update the global enablement flag here.
+    auto instrumentation_runtime_ty = GetInstrumentationRuntimeTy();
+    if (auto E = instrumentation_runtime_ty.takeError())
+      return E;
+
+    // Loop over all targets in requesting debugger and enable the plugin in
+    // each of them.
+    llvm::Error errors = llvm::Error::success();
+    bool found_targets = false;
+    for (const auto &target_sp :
+         requesting_debugger.GetTargetList().Targets()) {
+      found_targets = true;
+      ProcessSP process_sp = target_sp->GetProcessSP();
+      if (!process_sp || !process_sp->IsAlive())
+        continue;
----------------
JDevlieghere wrote:

If we found no processes, should we report an error too? Something like "debugger has targets but no processes" or something?

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