Revert "[lldb-dap] Ensure the IO forwarding threads are managed by the DAP object lifecycle. (#120457)"

This reverts commit 0d9cf26. Breaks the
lldb-aarch64-windows buildbot.

Author: d0k

lldb/tools/lldb-dap/CMakeLists.txt

@@ -1,3 +1,7 @@
+if ( CMAKE_SYSTEM_NAME MATCHES "Windows" OR CMAKE_SYSTEM_NAME MATCHES "NetBSD" )
+  list(APPEND extra_libs lldbHost)
+endif ()
+
 if (HAVE_LIBPTHREAD)
   list(APPEND extra_libs pthread)
 endif ()
@@ -22,23 +26,22 @@ add_lldb_tool(lldb-dap
   lldb-dap.cpp
   Breakpoint.cpp
   BreakpointBase.cpp
-  DAP.cpp
   ExceptionBreakpoint.cpp
   FifoFiles.cpp
   FunctionBreakpoint.cpp
-  InstructionBreakpoint.cpp
   IOStream.cpp
   JSONUtils.cpp
   LLDBUtils.cpp
   OutputRedirector.cpp
   ProgressEvent.cpp
   RunInTerminal.cpp
   SourceBreakpoint.cpp
+  DAP.cpp
   Watchpoint.cpp
+  InstructionBreakpoint.cpp
 
   LINK_LIBS
     liblldb
-    lldbHost
     ${extra_libs}
 
   LINK_COMPONENTS

lldb/tools/lldb-dap/DAP.cpp

@@ -6,62 +6,34 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include <chrono>
+#include <cstdarg>
+#include <fstream>
+#include <mutex>
+
 #include "DAP.h"
 #include "JSONUtils.h"
 #include "LLDBUtils.h"
-#include "OutputRedirector.h"
-#include "lldb/API/SBBreakpoint.h"
 #include "lldb/API/SBCommandInterpreter.h"
-#include "lldb/API/SBCommandReturnObject.h"
 #include "lldb/API/SBLanguageRuntime.h"
 #include "lldb/API/SBListener.h"
-#include "lldb/API/SBProcess.h"
 #include "lldb/API/SBStream.h"
-#include "lldb/Host/FileSystem.h"
-#include "lldb/Utility/Status.h"
-#include "lldb/lldb-defines.h"
-#include "lldb/lldb-enumerations.h"
-#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringExtras.h"
-#include "llvm/ADT/Twine.h"
-#include "llvm/Support/Error.h"
-#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FormatVariadic.h"
-#include "llvm/Support/raw_ostream.h"
-#include <algorithm>
-#include <cassert>
-#include <chrono>
-#include <cstdarg>
-#include <cstdio>
-#include <fstream>
-#include <mutex>
-#include <utility>
 
 #if defined(_WIN32)
 #define NOMINMAX
 #include <fcntl.h>
 #include <io.h>
 #include <windows.h>
-#else
-#include <unistd.h>
 #endif
 
 using namespace lldb_dap;
 
-namespace {
-#ifdef _WIN32
-const char DEV_NULL[] = "nul";
-#else
-const char DEV_NULL[] = "/dev/null";
-#endif
-} // namespace
-
 namespace lldb_dap {
 
-DAP::DAP(llvm::StringRef path, std::ofstream *log, ReplMode repl_mode,
-         StreamDescriptor input, StreamDescriptor output)
-    : debug_adaptor_path(path), log(log), input(std::move(input)),
-      output(std::move(output)), broadcaster("lldb-dap"),
+DAP::DAP(llvm::StringRef path, ReplMode repl_mode)
+    : debug_adaptor_path(path), broadcaster("lldb-dap"),
       exception_breakpoints(), focus_tid(LLDB_INVALID_THREAD_ID),
       stop_at_entry(false), is_attach(false),
       enable_auto_variable_summaries(false),
@@ -71,7 +43,21 @@ DAP::DAP(llvm::StringRef path, std::ofstream *log, ReplMode repl_mode,
       configuration_done_sent(false), waiting_for_run_in_terminal(false),
       progress_event_reporter(
           [&](const ProgressEvent &event) { SendJSON(event.ToJSON()); }),
-      reverse_request_seq(0), repl_mode(repl_mode) {}
+      reverse_request_seq(0), repl_mode(repl_mode) {
+  const char *log_file_path = getenv("LLDBDAP_LOG");
+#if defined(_WIN32)
+  // Windows opens stdout and stdin in text mode which converts \n to 13,10
+  // while the value is just 10 on Darwin/Linux. Setting the file mode to binary
+  // fixes this.
+  int result = _setmode(fileno(stdout), _O_BINARY);
+  assert(result);
+  result = _setmode(fileno(stdin), _O_BINARY);
+  UNUSED_IF_ASSERT_DISABLED(result);
+  assert(result);
+#endif
+  if (log_file_path)
+    log.reset(new std::ofstream(log_file_path));
+}
 
 DAP::~DAP() = default;
 
@@ -187,45 +173,6 @@ ExceptionBreakpoint *DAP::GetExceptionBreakpoint(const lldb::break_id_t bp_id) {
   return nullptr;
 }
 
-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([this](llvm::StringRef output) {
-        SendOutput(OutputType::Stdout, output);
-      }))
-    return Error;
-
-  if (overrideOut) {
-    auto fd = out.GetWriteFileDescriptor();
-    if (auto Error = fd.takeError())
-      return Error;
-
-    if (dup2(*fd, fileno(overrideOut)) == -1)
-      return llvm::errorCodeToError(llvm::errnoAsErrorCode());
-  }
-
-  if (auto Error = err.RedirectTo([this](llvm::StringRef output) {
-        SendOutput(OutputType::Stderr, output);
-      }))
-    return Error;
-
-  if (overrideErr) {
-    auto fd = err.GetWriteFileDescriptor();
-    if (auto Error = fd.takeError())
-      return Error;
-
-    if (dup2(*fd, fileno(overrideErr)) == -1)
-      return llvm::errorCodeToError(llvm::errnoAsErrorCode());
-  }
-
-  return llvm::Error::success();
-}
-
-void DAP::StopIO() {
-  out.Stop();
-  err.Stop();
-}
-
 // Send the JSON in "json_str" to the "out" stream. Correctly send the
 // "Content-Length:" field followed by the length, followed by the raw
 // JSON bytes.
@@ -261,19 +208,19 @@ std::string DAP::ReadJSON() {
   std::string json_str;
   int length;
 
-  if (!input.read_expected(log, "Content-Length: "))
+  if (!input.read_expected(log.get(), "Content-Length: "))
     return json_str;
 
-  if (!input.read_line(log, length_str))
+  if (!input.read_line(log.get(), length_str))
     return json_str;
 
   if (!llvm::to_integer(length_str, length))
     return json_str;
 
-  if (!input.read_expected(log, "\r\n"))
+  if (!input.read_expected(log.get(), "\r\n"))
     return json_str;
 
-  if (!input.read_full(log, length, json_str))
+  if (!input.read_full(log.get(), length, json_str))
     return json_str;
 
   if (log) {

lldb/tools/lldb-dap/DAP.h

@@ -9,38 +9,36 @@
 #ifndef LLDB_TOOLS_LLDB_DAP_DAP_H
 #define LLDB_TOOLS_LLDB_DAP_DAP_H
 
-#include "DAPForward.h"
-#include "ExceptionBreakpoint.h"
-#include "FunctionBreakpoint.h"
-#include "IOStream.h"
-#include "InstructionBreakpoint.h"
-#include "OutputRedirector.h"
-#include "ProgressEvent.h"
-#include "SourceBreakpoint.h"
-#include "lldb/API/SBBroadcaster.h"
-#include "lldb/API/SBCommandInterpreter.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/SBTarget.h"
-#include "lldb/API/SBThread.h"
-#include "lldb/API/SBValue.h"
-#include "lldb/API/SBValueList.h"
-#include "lldb/lldb-types.h"
+#include <cstdio>
+#include <iosfwd>
+#include <map>
+#include <optional>
+#include <thread>
+
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
-#include "llvm/Support/Error.h"
 #include "llvm/Support/JSON.h"
 #include "llvm/Support/Threading.h"
-#include <map>
-#include <mutex>
-#include <optional>
-#include <thread>
-#include <vector>
+#include "llvm/Support/raw_ostream.h"
+
+#include "lldb/API/SBAttachInfo.h"
+#include "lldb/API/SBCommandInterpreter.h"
+#include "lldb/API/SBCommandReturnObject.h"
+#include "lldb/API/SBDebugger.h"
+#include "lldb/API/SBEvent.h"
+#include "lldb/API/SBFormat.h"
+#include "lldb/API/SBLaunchInfo.h"
+#include "lldb/API/SBTarget.h"
+#include "lldb/API/SBThread.h"
+
+#include "ExceptionBreakpoint.h"
+#include "FunctionBreakpoint.h"
+#include "IOStream.h"
+#include "InstructionBreakpoint.h"
+#include "ProgressEvent.h"
+#include "SourceBreakpoint.h"
 
 #define VARREF_LOCALS (int64_t)1
 #define VARREF_GLOBALS (int64_t)2
@@ -140,18 +138,15 @@ struct SendEventRequestHandler : public lldb::SBCommandPluginInterface {
 
 struct DAP {
   llvm::StringRef debug_adaptor_path;
-  std::ofstream *log;
   InputStream input;
   OutputStream output;
-  lldb::SBFile in;
-  OutputRedirector out;
-  OutputRedirector err;
   lldb::SBDebugger debugger;
   lldb::SBTarget target;
   Variables variables;
   lldb::SBBroadcaster broadcaster;
   std::thread event_thread;
   std::thread progress_event_thread;
+  std::unique_ptr<std::ofstream> log;
   llvm::StringMap<SourceBreakpointMap> source_breakpoints;
   FunctionBreakpointMap function_breakpoints;
   InstructionBreakpointMap instruction_breakpoints;
@@ -203,23 +198,13 @@ struct DAP {
   // will contain that expression.
   std::string last_nonempty_var_expression;
 
-  DAP(llvm::StringRef path, std::ofstream *log, ReplMode repl_mode,
-      StreamDescriptor input, StreamDescriptor output);
+  DAP(llvm::StringRef path, ReplMode repl_mode);
   ~DAP();
   DAP(const DAP &rhs) = delete;
   void operator=(const DAP &rhs) = delete;
   ExceptionBreakpoint *GetExceptionBreakpoint(const std::string &filter);
   ExceptionBreakpoint *GetExceptionBreakpoint(const lldb::break_id_t bp_id);
 
-  /// Redirect stdout and stderr fo the IDE's console output.
-  ///
-  /// Errors in this operation will be printed to the log file and the IDE's
-  /// console output as well.
-  llvm::Error ConfigureIO(std::FILE *overrideOut, std::FILE *overrideErr);
-
-  /// Stop the redirected IO threads and associated pipes.
-  void StopIO();
-
   // Serialize the JSON value into a string and send the JSON packet to
   // the "out" stream.
   void SendJSON(const llvm::json::Value &json);

lldb/tools/lldb-dap/IOStream.h

@@ -52,9 +52,6 @@ struct StreamDescriptor {
 struct InputStream {
   StreamDescriptor descriptor;
 
-  explicit InputStream(StreamDescriptor descriptor)
-      : descriptor(std::move(descriptor)) {}
-
   bool read_full(std::ofstream *log, size_t length, std::string &text);
 
   bool read_line(std::ofstream *log, std::string &line);
@@ -65,9 +62,6 @@ struct InputStream {
 struct OutputStream {
   StreamDescriptor descriptor;
 
-  explicit OutputStream(StreamDescriptor descriptor)
-      : descriptor(std::move(descriptor)) {}
-
   bool write_full(llvm::StringRef str);
 };
 } // namespace lldb_dap