[lldb-dap] Refactor lldb-dap.cpp to not use global DAP variable.

[ashgti]

This removes the global DAP variable and instead allocates a DAP instance in main. This should allow us to refactor lldb-dap to enable a server mode that accepts multiple connections.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: John Harrison (ashgti)

Changes

This removes the global DAP variable and instead allocates a DAP instance in main. This should allow us to refactor lldb-dap to enable a server mode that accepts multiple connections.

---

Patch is 117.71 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/116272.diff

3 Files Affected:

• (modified) lldb/tools/lldb-dap/DAP.cpp (+4-6)
• (modified) lldb/tools/lldb-dap/DAP.h (+1-3)
• (modified) lldb/tools/lldb-dap/lldb-dap.cpp (+543-543)

diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index bdb24fc78cfb64..aee6214857bd0d 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -32,8 +32,6 @@ using namespace lldb_dap;
 
 namespace lldb_dap {
 
-DAP g_dap;
-
 DAP::DAP()
     : broadcaster("lldb-dap"), exception_breakpoints(),
       focus_tid(LLDB_INVALID_THREAD_ID), stop_at_entry(false), is_attach(false),
@@ -693,15 +691,15 @@ bool DAP::HandleObject(const llvm::json::Object &object) {
   if (packet_type == "request") {
     const auto command = GetString(object, "command");
     auto handler_pos = request_handlers.find(command);
-    if (handler_pos != request_handlers.end()) {
-      handler_pos->second(object);
-      return true; // Success
-    } else {
+    if (handler_pos == request_handlers.end()) {
       if (log)
         *log << "error: unhandled command \"" << command.data() << "\""
              << std::endl;
       return false; // Fail
     }
+
+    handler_pos->second(*this, object);
+    return true; // Success
   }
 
   if (packet_type == "response") {
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index 5381b3271ba96f..c6e86a59875ea2 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -63,7 +63,7 @@ enum DAPBroadcasterBits {
   eBroadcastBitStopProgressThread = 1u << 1
 };
 
-typedef void (*RequestCallback)(const llvm::json::Object &command);
+typedef void (*RequestCallback)(DAP &dap, const llvm::json::Object &command);
 typedef void (*ResponseCallback)(llvm::Expected<llvm::json::Value> value);
 
 enum class PacketStatus {
@@ -353,8 +353,6 @@ struct DAP {
   void SendJSON(const std::string &json_str);
 };
 
-extern DAP g_dap;
-
 } // namespace lldb_dap
 
 #endif
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index fc22ec03b672e6..055a012ed51c6a 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -24,7 +24,6 @@
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/ScopeExit.h"
-#include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Option/Arg.h"
 #include "llvm/Option/ArgList.h"
@@ -119,34 +118,34 @@ constexpr int StackPageSize = 20;
 
 /// Prints a welcome message on the editor if the preprocessor variable
 /// LLDB_DAP_WELCOME_MESSAGE is defined.
-static void PrintWelcomeMessage() {
+static void PrintWelcomeMessage(DAP &dap) {
 #ifdef LLDB_DAP_WELCOME_MESSAGE
-  g_dap.SendOutput(OutputType::Console, LLDB_DAP_WELCOME_MESSAGE);
+  dap.SendOutput(OutputType::Console, LLDB_DAP_WELCOME_MESSAGE);
 #endif
 }
 
-lldb::SBValueList *GetTopLevelScope(int64_t variablesReference) {
+lldb::SBValueList *GetTopLevelScope(DAP &dap, int64_t variablesReference) {
   switch (variablesReference) {
   case VARREF_LOCALS:
-    return &g_dap.variables.locals;
+    return &dap.variables.locals;
   case VARREF_GLOBALS:
-    return &g_dap.variables.globals;
+    return &dap.variables.globals;
   case VARREF_REGS:
-    return &g_dap.variables.registers;
+    return &dap.variables.registers;
   default:
     return nullptr;
   }
 }
 
-SOCKET AcceptConnection(int portno) {
+SOCKET AcceptConnection(DAP &dap, int portno) {
   // Accept a socket connection from any host on "portno".
   SOCKET newsockfd = -1;
   struct sockaddr_in serv_addr, cli_addr;
   SOCKET sockfd = socket(AF_INET, SOCK_STREAM, 0);
   if (sockfd < 0) {
-    if (g_dap.log)
-      *g_dap.log << "error: opening socket (" << strerror(errno) << ")"
-                 << std::endl;
+    if (dap.log)
+      *dap.log << "error: opening socket (" << strerror(errno) << ")"
+               << std::endl;
   } else {
     memset((char *)&serv_addr, 0, sizeof(serv_addr));
     serv_addr.sin_family = AF_INET;
@@ -154,9 +153,9 @@ SOCKET AcceptConnection(int portno) {
     serv_addr.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
     serv_addr.sin_port = htons(portno);
     if (bind(sockfd, (struct sockaddr *)&serv_addr, sizeof(serv_addr)) < 0) {
-      if (g_dap.log)
-        *g_dap.log << "error: binding socket (" << strerror(errno) << ")"
-                   << std::endl;
+      if (dap.log)
+        *dap.log << "error: binding socket (" << strerror(errno) << ")"
+                 << std::endl;
     } else {
       listen(sockfd, 5);
       socklen_t clilen = sizeof(cli_addr);
@@ -164,9 +163,8 @@ SOCKET AcceptConnection(int portno) {
           llvm::sys::RetryAfterSignal(static_cast<SOCKET>(-1), accept, sockfd,
                                       (struct sockaddr *)&cli_addr, &clilen);
       if (newsockfd < 0)
-        if (g_dap.log)
-          *g_dap.log << "error: accept (" << strerror(errno) << ")"
-                     << std::endl;
+        if (dap.log)
+          *dap.log << "error: accept (" << strerror(errno) << ")" << std::endl;
     }
 #if defined(_WIN32)
     closesocket(sockfd);
@@ -190,66 +188,65 @@ std::vector<const char *> MakeArgv(const llvm::ArrayRef<std::string> &strs) {
 }
 
 // Send a "exited" event to indicate the process has exited.
-void SendProcessExitedEvent(lldb::SBProcess &process) {
+void SendProcessExitedEvent(DAP &dap, lldb::SBProcess &process) {
   llvm::json::Object event(CreateEventObject("exited"));
   llvm::json::Object body;
   body.try_emplace("exitCode", (int64_t)process.GetExitStatus());
   event.try_emplace("body", std::move(body));
-  g_dap.SendJSON(llvm::json::Value(std::move(event)));
+  dap.SendJSON(llvm::json::Value(std::move(event)));
 }
 
-void SendThreadExitedEvent(lldb::tid_t tid) {
+void SendThreadExitedEvent(DAP &dap, lldb::tid_t tid) {
   llvm::json::Object event(CreateEventObject("thread"));
   llvm::json::Object body;
   body.try_emplace("reason", "exited");
   body.try_emplace("threadId", (int64_t)tid);
   event.try_emplace("body", std::move(body));
-  g_dap.SendJSON(llvm::json::Value(std::move(event)));
+  dap.SendJSON(llvm::json::Value(std::move(event)));
 }
 
 // Send a "continued" event to indicate the process is in the running state.
-void SendContinuedEvent() {
-  lldb::SBProcess process = g_dap.target.GetProcess();
+void SendContinuedEvent(DAP &dap) {
+  lldb::SBProcess process = dap.target.GetProcess();
   if (!process.IsValid()) {
     return;
   }
 
   // If the focus thread is not set then we haven't reported any thread status
   // to the client, so nothing to report.
-  if (!g_dap.configuration_done_sent ||
-      g_dap.focus_tid == LLDB_INVALID_THREAD_ID) {
+  if (!dap.configuration_done_sent || dap.focus_tid == LLDB_INVALID_THREAD_ID) {
     return;
   }
 
   llvm::json::Object event(CreateEventObject("continued"));
   llvm::json::Object body;
-  body.try_emplace("threadId", (int64_t)g_dap.focus_tid);
+  body.try_emplace("threadId", (int64_t)dap.focus_tid);
   body.try_emplace("allThreadsContinued", true);
   event.try_emplace("body", std::move(body));
-  g_dap.SendJSON(llvm::json::Value(std::move(event)));
+  dap.SendJSON(llvm::json::Value(std::move(event)));
 }
 
 // Send a "terminated" event to indicate the process is done being
 // debugged.
-void SendTerminatedEvent() {
+void SendTerminatedEvent(DAP &dap) {
   // Prevent races if the process exits while we're being asked to disconnect.
-  llvm::call_once(g_dap.terminated_event_flag, [&] {
-    g_dap.RunTerminateCommands();
+  llvm::call_once(dap.terminated_event_flag, [&] {
+    dap.RunTerminateCommands();
     // Send a "terminated" event
-    llvm::json::Object event(CreateTerminatedEventObject(g_dap.target));
-    g_dap.SendJSON(llvm::json::Value(std::move(event)));
+    llvm::json::Object event(CreateTerminatedEventObject(dap.target));
+    dap.SendJSON(llvm::json::Value(std::move(event)));
   });
 }
 
 // Send a thread stopped event for all threads as long as the process
 // is stopped.
-void SendThreadStoppedEvent() {
-  lldb::SBProcess process = g_dap.target.GetProcess();
+void SendThreadStoppedEvent(DAP &dap) {
+  lldb::SBProcess process = dap.target.GetProcess();
   if (process.IsValid()) {
     auto state = process.GetState();
     if (state == lldb::eStateStopped) {
       llvm::DenseSet<lldb::tid_t> old_thread_ids;
-      old_thread_ids.swap(g_dap.thread_ids);
+      old_thread_ids.swap(dap.thread_ids);
       uint32_t stop_id = process.GetStopID();
       const uint32_t num_threads = process.GetNumThreads();
 
@@ -265,10 +262,10 @@ void SendThreadStoppedEvent() {
         const lldb::tid_t tid = thread.GetThreadID();
         const bool has_reason = ThreadHasStopReason(thread);
         // If the focus thread doesn't have a stop reason, clear the thread ID
-        if (tid == g_dap.focus_tid) {
+        if (tid == dap.focus_tid) {
           focus_thread_exists = true;
           if (!has_reason)
-            g_dap.focus_tid = LLDB_INVALID_THREAD_ID;
+            dap.focus_tid = LLDB_INVALID_THREAD_ID;
         }
         if (has_reason) {
           ++num_threads_with_reason;
@@ -277,47 +274,46 @@ void SendThreadStoppedEvent() {
         }
       }
 
-      // We will have cleared g_dap.focus_tid if the focus thread doesn't have
+      // We will have cleared dap.focus_tid if the focus thread doesn't have
       // a stop reason, so if it was cleared, or wasn't set, or doesn't exist,
       // then set the focus thread to the first thread with a stop reason.
-      if (!focus_thread_exists || g_dap.focus_tid == LLDB_INVALID_THREAD_ID)
-        g_dap.focus_tid = first_tid_with_reason;
+      if (!focus_thread_exists || dap.focus_tid == LLDB_INVALID_THREAD_ID)
+        dap.focus_tid = first_tid_with_reason;
 
       // If no threads stopped with a reason, then report the first one so
       // we at least let the UI know we stopped.
       if (num_threads_with_reason == 0) {
         lldb::SBThread thread = process.GetThreadAtIndex(0);
-        g_dap.focus_tid = thread.GetThreadID();
-        g_dap.SendJSON(CreateThreadStopped(g_dap, thread, stop_id));
+        dap.focus_tid = thread.GetThreadID();
+        dap.SendJSON(CreateThreadStopped(dap, thread, stop_id));
       } else {
         for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
           lldb::SBThread thread = process.GetThreadAtIndex(thread_idx);
-          g_dap.thread_ids.insert(thread.GetThreadID());
+          dap.thread_ids.insert(thread.GetThreadID());
           if (ThreadHasStopReason(thread)) {
-            g_dap.SendJSON(CreateThreadStopped(g_dap, thread, stop_id));
+            dap.SendJSON(CreateThreadStopped(dap, thread, stop_id));
           }
         }
       }
 
       for (auto tid : old_thread_ids) {
-        auto end = g_dap.thread_ids.end();
-        auto pos = g_dap.thread_ids.find(tid);
+        auto end = dap.thread_ids.end();
+        auto pos = dap.thread_ids.find(tid);
         if (pos == end)
-          SendThreadExitedEvent(tid);
+          SendThreadExitedEvent(dap, tid);
       }
     } else {
-      if (g_dap.log)
-        *g_dap.log << "error: SendThreadStoppedEvent() when process"
-                      " isn't stopped ("
-                   << lldb::SBDebugger::StateAsCString(state) << ')'
-                   << std::endl;
+      if (dap.log)
+        *dap.log << "error: SendThreadStoppedEvent() when process"
+                    " isn't stopped ("
+                 << lldb::SBDebugger::StateAsCString(state) << ')' << std::endl;
     }
   } else {
-    if (g_dap.log)
-      *g_dap.log << "error: SendThreadStoppedEvent() invalid process"
-                 << std::endl;
+    if (dap.log)
+      *dap.log << "error: SendThreadStoppedEvent() invalid process"
+               << std::endl;
   }
-  g_dap.RunStopCommands();
+  dap.RunStopCommands();
 }
 
 // "ProcessEvent": {
@@ -374,14 +370,14 @@ void SendThreadStoppedEvent() {
 //     }
 //   ]
 // }
-void SendProcessEvent(LaunchMethod launch_method) {
-  lldb::SBFileSpec exe_fspec = g_dap.target.GetExecutable();
+void SendProcessEvent(DAP &dap, LaunchMethod launch_method) {
+  lldb::SBFileSpec exe_fspec = dap.target.GetExecutable();
   char exe_path[PATH_MAX];
   exe_fspec.GetPath(exe_path, sizeof(exe_path));
   llvm::json::Object event(CreateEventObject("process"));
   llvm::json::Object body;
   EmplaceSafeString(body, "name", std::string(exe_path));
-  const auto pid = g_dap.target.GetProcess().GetProcessID();
+  const auto pid = dap.target.GetProcess().GetProcessID();
   body.try_emplace("systemProcessId", (int64_t)pid);
   body.try_emplace("isLocalProcess", true);
   const char *startMethod = nullptr;
@@ -398,31 +394,31 @@ void SendProcessEvent(LaunchMethod launch_method) {
   }
   body.try_emplace("startMethod", startMethod);
   event.try_emplace("body", std::move(body));
-  g_dap.SendJSON(llvm::json::Value(std::move(event)));
+  dap.SendJSON(llvm::json::Value(std::move(event)));
 }
 
 // Grab any STDOUT and STDERR from the process and send it up to VS Code
 // via an "output" event to the "stdout" and "stderr" categories.
-void SendStdOutStdErr(lldb::SBProcess &process) {
+void SendStdOutStdErr(DAP &dap, lldb::SBProcess &process) {
   char buffer[OutputBufferSize];
   size_t count;
   while ((count = process.GetSTDOUT(buffer, sizeof(buffer))) > 0)
-    g_dap.SendOutput(OutputType::Stdout, llvm::StringRef(buffer, count));
+    dap.SendOutput(OutputType::Stdout, llvm::StringRef(buffer, count));
   while ((count = process.GetSTDERR(buffer, sizeof(buffer))) > 0)
-    g_dap.SendOutput(OutputType::Stderr, llvm::StringRef(buffer, count));
+    dap.SendOutput(OutputType::Stderr, llvm::StringRef(buffer, count));
 }
 
-void ProgressEventThreadFunction() {
+void ProgressEventThreadFunction(DAP &dap) {
   lldb::SBListener listener("lldb-dap.progress.listener");
-  g_dap.debugger.GetBroadcaster().AddListener(
+  dap.debugger.GetBroadcaster().AddListener(
       listener, lldb::SBDebugger::eBroadcastBitProgress);
-  g_dap.broadcaster.AddListener(listener, eBroadcastBitStopProgressThread);
+  dap.broadcaster.AddListener(listener, eBroadcastBitStopProgressThread);
   lldb::SBEvent event;
   bool done = false;
   while (!done) {
     if (listener.WaitForEvent(1, event)) {
       const auto event_mask = event.GetType();
-      if (event.BroadcasterMatchesRef(g_dap.broadcaster)) {
+      if (event.BroadcasterMatchesRef(dap.broadcaster)) {
         if (event_mask & eBroadcastBitStopProgressThread) {
           done = true;
         }
@@ -434,7 +430,7 @@ void ProgressEventThreadFunction() {
         const char *message = lldb::SBDebugger::GetProgressFromEvent(
             event, progress_id, completed, total, is_debugger_specific);
         if (message)
-          g_dap.SendProgressEvent(progress_id, message, completed, total);
+          dap.SendProgressEvent(progress_id, message, completed, total);
       }
     }
   }
@@ -445,9 +441,9 @@ void ProgressEventThreadFunction() {
 // "FILE *" to output packets back to VS Code and they have mutexes in them
 // them prevent multiple threads from writing simultaneously so no locking
 // is required.
-void EventThreadFunction() {
+void EventThreadFunction(DAP &dap) {
   lldb::SBEvent event;
-  lldb::SBListener listener = g_dap.debugger.GetListener();
+  lldb::SBListener listener = dap.debugger.GetListener();
   bool done = false;
   while (!done) {
     if (listener.WaitForEvent(1, event)) {
@@ -483,50 +479,50 @@ void EventThreadFunction() {
             // stop events which we do not want to send an event for. We will
             // manually send a stopped event in request_configurationDone(...)
             // so don't send any before then.
-            if (g_dap.configuration_done_sent) {
+            if (dap.configuration_done_sent) {
               // Only report a stopped event if the process was not
               // automatically restarted.
               if (!lldb::SBProcess::GetRestartedFromEvent(event)) {
-                SendStdOutStdErr(process);
-                SendThreadStoppedEvent();
+                SendStdOutStdErr(dap, process);
+                SendThreadStoppedEvent(dap);
               }
             }
             break;
           case lldb::eStateRunning:
-            g_dap.WillContinue();
-            SendContinuedEvent();
+            dap.WillContinue();
+            SendContinuedEvent(dap);
             break;
           case lldb::eStateExited:
             lldb::SBStream stream;
             process.GetStatus(stream);
-            g_dap.SendOutput(OutputType::Console, stream.GetData());
+            dap.SendOutput(OutputType::Console, stream.GetData());
 
             // When restarting, we can get an "exited" event for the process we
             // just killed with the old PID, or even with no PID. In that case
             // we don't have to terminate the session.
             if (process.GetProcessID() == LLDB_INVALID_PROCESS_ID ||
-                process.GetProcessID() == g_dap.restarting_process_id) {
-              g_dap.restarting_process_id = LLDB_INVALID_PROCESS_ID;
+                process.GetProcessID() == dap.restarting_process_id) {
+              dap.restarting_process_id = LLDB_INVALID_PROCESS_ID;
             } else {
               // Run any exit LLDB commands the user specified in the
               // launch.json
-              g_dap.RunExitCommands();
-              SendProcessExitedEvent(process);
-              SendTerminatedEvent();
+              dap.RunExitCommands();
+              SendProcessExitedEvent(dap, process);
+              SendTerminatedEvent(dap);
               done = true;
             }
             break;
           }
         } else if ((event_mask & lldb::SBProcess::eBroadcastBitSTDOUT) ||
                    (event_mask & lldb::SBProcess::eBroadcastBitSTDERR)) {
-          SendStdOutStdErr(process);
+          SendStdOutStdErr(dap, process);
         }
       } else if (lldb::SBBreakpoint::EventIsBreakpointEvent(event)) {
         if (event_mask & lldb::SBTarget::eBroadcastBitBreakpointChanged) {
           auto event_type =
               lldb::SBBreakpoint::GetBreakpointEventTypeFromEvent(event);
           auto bp = Breakpoint(
-              g_dap, lldb::SBBreakpoint::GetBreakpointFromEvent(event));
+              dap, lldb::SBBreakpoint::GetBreakpointFromEvent(event));
           // If the breakpoint was originated from the IDE, it will have the
           // BreakpointBase::GetBreakpointLabel() label attached. Regardless
           // of wether the locations were added or removed, the breakpoint
@@ -548,10 +544,10 @@ void EventThreadFunction() {
             body.try_emplace("breakpoint", source_bp);
             body.try_emplace("reason", "changed");
             bp_event.try_emplace("body", std::move(body));
-            g_dap.SendJSON(llvm::json::Value(std::move(bp_event)));
+            dap.SendJSON(llvm::json::Value(std::move(bp_event)));
           }
         }
-      } else if (event.BroadcasterMatchesRef(g_dap.broadcaster)) {
+      } else if (event.BroadcasterMatchesRef(dap.broadcaster)) {
         if (event_mask & eBroadcastBitStopEventThread) {
           done = true;
         }
@@ -560,9 +556,11 @@ void EventThreadFunction() {
   }
 }
 
-lldb::SBValue FindVariable(uint64_t variablesReference, llvm::StringRef name) {
+lldb::SBValue FindVariable(DAP &dap, uint64_t variablesReference,
+                           llvm::StringRef name) {
   lldb::SBValue variable;
-  if (lldb::SBValueList *top_scope = GetTopLevelScope(variablesReference)) {
+  if (lldb::SBValueList *top_scope =
+          GetTopLevelScope(dap, variablesReference)) {
     bool is_duplicated_variable_name = name.contains(" @");
     // variablesReference is one of our scopes, not an actual variable it is
     // asking for a variable in locals or globals or registers
@@ -584,7 +582,7 @@ lldb::SBValue FindVariable(uint64_t variablesReference, llvm::StringRef name) {
 
     // We have a named item within an actual variable so we need to find it
     // withing the container variable by name.
-    lldb::SBValue container = g_dap.variables.GetVariable(variablesReference);
+    lldb::SBValue container = dap.variables.GetVariable(variablesReference);
     variable = container.GetChildMemberWithName(name.data());
     if (!variable.IsValid()) {
       if (name.starts_with("[")) {
@@ -602,7 +600,7 @@ lldb::SBValue FindVariable(uint64_t variablesReference, llvm::StringRef name) {
 
 // Both attach and launch take a either a sourcePath or sourceMap
 // argument (or neither), from which we need to set the target.source-map.
-void SetSourceMapFromArguments(const llvm::json::Object &arguments)...
[truncated]

[JDevlieghere]

🥳

[labath]

Should the dap object be copyable? Given that things hold references to it, maybe the copy operations should be deleted (and this object constructed in place) ?

[ashgti]

I can follow up with a delete of the copy constructor