[lldb-dap] Refactoring IO handling for the SBDebugger.

This refactors the IO handling of lldb-dap and its SBDebugger. This adjusts the SBDebugger instance to have a pty associated with in/out/err. Using a pty allows us to handle commands with raw input modes, such as `script` or `breakpoint command add`.

Additionally, to better handle output produced by the debugger and evaluate command I added a print helper. This new print helper will inspect the SBCommandReturnObject and store any associated variables in the variable store. This lets users more easily inspect variables directly in the Debug Console in VSCode.

Author: ashgti

lldb/tools/lldb-dap/DAP.cpp

@@ -20,7 +20,6 @@
 #include "Protocol/ProtocolRequests.h"
 #include "Protocol/ProtocolTypes.h"
 #include "ProtocolUtils.h"
-#include "Transport.h"
 #include "lldb/API/SBBreakpoint.h"
 #include "lldb/API/SBCommandInterpreter.h"
 #include "lldb/API/SBEvent.h"
@@ -29,12 +28,13 @@
 #include "lldb/API/SBMutex.h"
 #include "lldb/API/SBProcess.h"
 #include "lldb/API/SBStream.h"
-#include "lldb/Host/JSONTransport.h"
+#include "lldb/Host/File.h"
 #include "lldb/Host/MainLoop.h"
 #include "lldb/Host/MainLoopBase.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/lldb-defines.h"
 #include "lldb/lldb-enumerations.h"
+#include "lldb/lldb-forward.h"
 #include "lldb/lldb-types.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/STLExtras.h"
@@ -55,7 +55,6 @@
 #include <cstdint>
 #include <cstdio>
 #include <functional>
-#include <future>
 #include <memory>
 #include <mutex>
 #include <optional>
@@ -77,14 +76,6 @@ using namespace lldb_dap;
 using namespace lldb_dap::protocol;
 using namespace lldb_private;
 
-namespace {
-#ifdef _WIN32
-const char DEV_NULL[] = "nul";
-#else
-const char DEV_NULL[] = "/dev/null";
-#endif
-} // namespace
-
 namespace lldb_dap {
 
 static std::string GetStringFromStructuredData(lldb::SBStructuredData &data,
@@ -226,8 +217,6 @@ ExceptionBreakpoint *DAP::GetExceptionBreakpoint(const lldb::break_id_t bp_id) {
 }
 
 llvm::Error DAP::ConfigureIO(std::FILE *overrideOut, std::FILE *overrideErr) {
-  in = lldb::SBFile(std::fopen(DEV_NULL, "r"), /*transfer_ownership=*/true);
-
   if (auto Error = out.RedirectTo(overrideOut, [this](llvm::StringRef output) {
         SendOutput(OutputType::Console, output);
       }))
@@ -375,7 +364,8 @@ Id DAP::Send(const Message &message) {
 //     "required": [ "event", "body" ]
 //   }]
 // }
-void DAP::SendOutput(OutputType o, const llvm::StringRef output) {
+void DAP::SendOutput(OutputType o, const llvm::StringRef output,
+                     int64_t varref) {
   if (output.empty())
     return;
 
@@ -409,6 +399,10 @@ void DAP::SendOutput(OutputType o, const llvm::StringRef output) {
     llvm::json::Object body;
     body.try_emplace("category", category);
     EmplaceSafeString(body, "output", output.slice(idx, end + 1).str());
+    if (varref != 0) {
+      body["variablesReference"] = varref;
+      varref = 0; // In case there are multiple lines.
+    }
     event.try_emplace("body", std::move(body));
     SendJSON(llvm::json::Value(std::move(event)));
     idx = end + 1;
@@ -1075,9 +1069,7 @@ llvm::Error DAP::Loop() {
   auto thread = std::thread(std::bind(&DAP::TransportHandler, this));
 
   auto cleanup = llvm::make_scope_exit([this]() {
-    // FIXME: Merge these into the MainLoop handler.
-    out.Stop();
-    err.Stop();
+    pty.ClosePrimaryFileDescriptor();
     StopEventHandlers();
 
     // Destroy the debugger when the session ends. This will trigger the
@@ -1304,6 +1296,33 @@ void DAP::StartEventThread() {
   event_thread = std::thread(&DAP::EventThread, this);
 }
 
+void DAP::StartIOThread() {
+  lldb::FileSP file = std::make_shared<lldb_private::NativeFile>(
+      pty.GetPrimaryFileDescriptor(),
+      lldb_private::NativeFile::eOpenOptionReadWrite |
+          lldb_private::NativeFile::eOpenOptionNonBlocking,
+      false);
+
+  Status error;
+  handle = m_loop.RegisterReadObject(
+      file,
+      [this, file](MainLoopBase &loop) {
+        char buf[4096];
+        size_t len = sizeof(buf);
+        if (auto err = file->Read(buf, len).takeError())
+          DAP_LOG_ERROR(log, std::move(err), "Reading from pty failed {0}");
+        if (len == 0) { // EOF
+          loop.RequestTermination();
+          return;
+        }
+        llvm::StringRef output = {buf, len};
+        output = output.trim();
+        if (!output.empty())
+          SendOutput(OutputType::Console, output);
+      },
+      error);
+}
+
 void DAP::StartProgressEventThread() {
   progress_event_thread = std::thread(&DAP::ProgressEventThread, this);
 }

lldb/tools/lldb-dap/DAP.h

@@ -22,21 +22,19 @@
 #include "Transport.h"
 #include "Variables.h"
 #include "lldb/API/SBBroadcaster.h"
-#include "lldb/API/SBCommandInterpreter.h"
+#include "lldb/API/SBCommandReturnObject.h"
 #include "lldb/API/SBDebugger.h"
 #include "lldb/API/SBError.h"
-#include "lldb/API/SBFile.h"
 #include "lldb/API/SBFormat.h"
 #include "lldb/API/SBFrame.h"
 #include "lldb/API/SBMutex.h"
 #include "lldb/API/SBTarget.h"
 #include "lldb/API/SBThread.h"
 #include "lldb/Host/MainLoop.h"
-#include "lldb/Utility/Status.h"
+#include "lldb/Host/PseudoTerminal.h"
 #include "lldb/lldb-types.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
-#include "llvm/ADT/FunctionExtras.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
@@ -47,6 +45,7 @@
 #include <condition_variable>
 #include <cstdint>
 #include <deque>
+#include <future>
 #include <memory>
 #include <mutex>
 #include <optional>
@@ -80,13 +79,18 @@ enum class ReplMode { Variable = 0, Command, Auto };
 
 using DAPTransport = lldb_private::transport::JSONTransport<ProtocolDescriptor>;
 
+struct EvaluateContext {
+  lldb::SBCommandReturnObject result;
+  std::promise<void> done;
+};
+
 struct DAP final : public DAPTransport::MessageHandler {
   /// Path to the lldb-dap binary itself.
   static llvm::StringRef debug_adapter_path;
 
   Log *log;
   DAPTransport &transport;
-  lldb::SBFile in;
+  lldb_private::PseudoTerminal pty;
   OutputRedirector out;
   OutputRedirector err;
 
@@ -99,6 +103,8 @@ struct DAP final : public DAPTransport::MessageHandler {
   /// The target instance for this DAP session.
   lldb::SBTarget target;
 
+  std::optional<EvaluateContext> context;
+
   Variables variables;
   lldb::SBBroadcaster broadcaster;
   FunctionBreakpointMap function_breakpoints;
@@ -223,7 +229,8 @@ struct DAP final : public DAPTransport::MessageHandler {
   /// Send the given message to the client.
   protocol::Id Send(const protocol::Message &message);
 
-  void SendOutput(OutputType o, const llvm::StringRef output);
+  void SendOutput(OutputType o, const llvm::StringRef output,
+                  int64_t varref = 0);
 
   void SendProgressEvent(uint64_t progress_id, const char *message,
                          uint64_t completed, uint64_t total);
@@ -409,6 +416,7 @@ struct DAP final : public DAPTransport::MessageHandler {
   lldb::SBMutex GetAPIMutex() const { return target.GetAPIMutex(); }
 
   void StartEventThread();
+  void StartIOThread();
   void StartProgressEventThread();
 
   /// Sets the given protocol `breakpoints` in the given `source`, while
@@ -477,6 +485,8 @@ struct DAP final : public DAPTransport::MessageHandler {
   // Loop for managing reading from the client.
   lldb_private::MainLoop &m_loop;
 
+  lldb_private::MainLoop::ReadHandleUP handle;
+
   std::mutex m_cancelled_requests_mutex;
   llvm::SmallSet<int64_t, 4> m_cancelled_requests;
 

lldb/tools/lldb-dap/EventHelper.cpp

@@ -243,7 +243,7 @@ void SendTerminatedEvent(DAP &dap) { dap.SendTerminatedEvent(); }
 // 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(DAP &dap, lldb::SBProcess &process) {
-  char buffer[OutputBufferSize];
+  char buffer[OutputBufferSize] = {0};
   size_t count;
   while ((count = process.GetSTDOUT(buffer, sizeof(buffer))) > 0)
     dap.SendOutput(OutputType::Stdout, llvm::StringRef(buffer, count));

lldb/tools/lldb-dap/Handler/CompletionsHandler.cpp

@@ -7,10 +7,11 @@
 //===----------------------------------------------------------------------===//
 
 #include "DAP.h"
-#include "JSONUtils.h"
 #include "Protocol/ProtocolRequests.h"
 #include "Protocol/ProtocolTypes.h"
 #include "RequestHandler.h"
+#include "lldb/API/SBCommandInterpreter.h"
+#include "lldb/API/SBDebugger.h"
 #include "lldb/API/SBStringList.h"
 
 using namespace llvm;

lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp

@@ -9,8 +9,14 @@
 #include "DAP.h"
 #include "EventHelper.h"
 #include "JSONUtils.h"
-#include "LLDBUtils.h"
 #include "RequestHandler.h"
+#include "lldb/API/SBCommandInterpreter.h"
+#include "lldb/API/SBCommandReturnObject.h"
+#include "lldb/lldb-enumerations.h"
+#include "llvm/ADT/StringRef.h"
+#include <future>
+#include <optional>
+#include <unistd.h>
 
 namespace lldb_dap {
 
@@ -164,13 +170,36 @@ void EvaluateRequestHandler::operator()(
       dap.focus_tid = frame.GetThread().GetThreadID();
     }
 
-    bool required_command_failed = false;
-    std::string result = RunLLDBCommands(
-        dap.debugger, llvm::StringRef(), {expression}, required_command_failed,
-        /*parse_command_directives=*/false, /*echo_commands=*/false);
+    std::string result = "";
+    int64_t varref = 0;
+    int fd = dap.pty.GetPrimaryFileDescriptor();
+    for (auto expr : llvm::split(expression, "\n")) {
+      if (!dap.debugger.GetCommandInterpreter().IsActive()) {
+        ::write(fd, expr.data(), expr.size());
+        std::string rcnl = "\r\n";
+        ::write(fd, rcnl.data(), rcnl.size());
+        continue;
+      }
+
+      dap.context = EvaluateContext{};
+      std::future<void> done = dap.context->done.get_future();
+      ::write(fd, expr.data(), expr.size());
+      std::string rcnl = "\r\n";
+      ::write(fd, rcnl.data(), rcnl.size());
+      dap.GetAPIMutex().unlock();
+      done.wait();
+      dap.GetAPIMutex().lock();
+      lldb::SBCommandReturnObject ret = dap.context->result;
+      dap.context = std::nullopt;
+      if (ret)
+        result += ret.GetOutput();
+      auto variables = ret.GetValues(lldb::eNoDynamicValues);
+      if (variables)
+        varref = dap.variables.InsertVariables(variables, true);
+    }
 
     EmplaceSafeString(body, "result", result);
-    body.try_emplace("variablesReference", (int64_t)0);
+    body.try_emplace("variablesReference", (int64_t)varref);
   } else {
     if (context == "repl") {
       // If the expression is empty and the last expression was for a