zturner created this revision.
zturner added a reviewer: clayborg.
Herald added a subscriber: mgorny.
For historical reasons, Windows unfortunately doesn't support using sockets in
standard system calls like read/write, which means that they also can't be
buffered with a `FILE*`. This violates some fundamental assumptions of how
lldb-vscode was written, so fixing it requires some replumbing of the guts. I
introduced an abstraction called `IOStream` which basically knows whether the
descriptor is a socket or a non-socket, and then delegates to the appropriate
underlying APIs accordingly. This means calling `read/write` on `stdin/stdout`
and calling `recv/send` on sockets. Luckily this same strategy will also work
on non-Windows platforms, so the abstractions needed aren't too egregious.
https://reviews.llvm.org/D59104
Files:
lldb/tools/lldb-vscode/CMakeLists.txt
lldb/tools/lldb-vscode/IOStream.cpp
lldb/tools/lldb-vscode/IOStream.h
lldb/tools/lldb-vscode/VSCode.cpp
lldb/tools/lldb-vscode/VSCode.h
lldb/tools/lldb-vscode/lldb-vscode.cpp
lldb/tools/lldb-vscode/package.json
Index: lldb/tools/lldb-vscode/package.json
===================================================================
--- lldb/tools/lldb-vscode/package.json
+++ lldb/tools/lldb-vscode/package.json
@@ -56,6 +56,9 @@
]
},
"program": "./bin/lldb-vscode",
+ "windows": {
+ "program": "./bin/lldb-vscode.exe"
+ },
"configurationAttributes": {
"launch": {
"required": [
Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===================================================================
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -23,6 +23,7 @@
#define NOMINMAX
#include <windows.h>
#undef GetObject
+#include <io.h>
#else
#include <netinet/in.h>
#include <sys/socket.h>
@@ -68,9 +69,9 @@
enum VSCodeBroadcasterBits { eBroadcastBitStopEventThread = 1u << 0 };
-int AcceptConnection(int portno) {
+SOCKET AcceptConnection(int portno) {
// Accept a socket connection from any host on "portno".
- int newsockfd = -1;
+ SOCKET newsockfd = -1;
struct sockaddr_in serv_addr, cli_addr;
SOCKET sockfd = socket(AF_INET, SOCK_STREAM, 0);
if (sockfd < 0) {
@@ -2635,23 +2636,19 @@
#endif
int portno = atoi(arg);
printf("Listening on port %i...\n", portno);
- int socket_fd = AcceptConnection(portno);
+ SOCKET socket_fd = AcceptConnection(portno);
if (socket_fd >= 0) {
- // We must open two FILE objects, one for reading and one for writing
- // the FILE objects have a mutex in them that won't allow reading and
- // writing to the socket stream.
- g_vsc.in = fdopen(socket_fd, "r");
- g_vsc.out = fdopen(socket_fd, "w");
- if (g_vsc.in == nullptr || g_vsc.out == nullptr) {
- if (g_vsc.log)
- *g_vsc.log << "fdopen failed (" << strerror(errno) << ")"
- << std::endl;
- exit(1);
- }
+ g_vsc.input.descriptor = StreamDescriptor::from_socket(socket_fd, true);
+ g_vsc.output.descriptor =
+ StreamDescriptor::from_socket(socket_fd, false);
} else {
exit(1);
}
}
+ } else {
+ g_vsc.input.descriptor = StreamDescriptor::from_file(fileno(stdin), false);
+ g_vsc.output.descriptor =
+ StreamDescriptor::from_file(fileno(stdout), false);
}
auto request_handlers = GetRequestHandlers();
uint32_t packet_idx = 0;
Index: lldb/tools/lldb-vscode/VSCode.h
===================================================================
--- lldb/tools/lldb-vscode/VSCode.h
+++ lldb/tools/lldb-vscode/VSCode.h
@@ -19,6 +19,7 @@
#include "llvm/ADT/DenseSet.h"
#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/raw_ostream.h"
#include "lldb/API/SBAttachInfo.h"
#include "lldb/API/SBBreakpoint.h"
@@ -43,6 +44,7 @@
#include "ExceptionBreakpoint.h"
#include "FunctionBreakpoint.h"
+#include "IOStream.h"
#include "SourceBreakpoint.h"
#include "SourceReference.h"
@@ -62,8 +64,8 @@
enum class OutputType { Console, Stdout, Stderr, Telemetry };
struct VSCode {
- FILE *in;
- FILE *out;
+ InputStream input;
+ OutputStream output;
lldb::SBDebugger debugger;
lldb::SBTarget target;
lldb::SBAttachInfo attach_info;
@@ -94,8 +96,6 @@
~VSCode();
VSCode(const VSCode &rhs) = delete;
void operator=(const VSCode &rhs) = delete;
- void CloseInputStream();
- void CloseOutputStream();
int64_t GetLineForPC(int64_t sourceReference, lldb::addr_t pc) const;
ExceptionBreakpoint *GetExceptionBreakpoint(const std::string &filter);
ExceptionBreakpoint *GetExceptionBreakpoint(const lldb::break_id_t bp_id);
Index: lldb/tools/lldb-vscode/VSCode.cpp
===================================================================
--- lldb/tools/lldb-vscode/VSCode.cpp
+++ lldb/tools/lldb-vscode/VSCode.cpp
@@ -10,12 +10,15 @@
#include <fstream>
#include <mutex>
-#include "VSCode.h"
#include "LLDBUtils.h"
+#include "VSCode.h"
+#include "llvm/Support/FormatVariadic.h"
#if defined(_WIN32)
-#include <io.h>
+#define NOMINMAX
+#include <Windows.h>
#include <fcntl.h>
+#include <io.h>
#endif
using namespace lldb_vscode;
@@ -31,15 +34,15 @@
VSCode g_vsc;
VSCode::VSCode()
- : in(stdin), out(stdout), launch_info(nullptr), variables(),
- broadcaster("lldb-vscode"), num_regs(0), num_locals(0), num_globals(0),
- log(), exception_breakpoints(
- {{"cpp_catch", "C++ Catch", lldb::eLanguageTypeC_plus_plus},
- {"cpp_throw", "C++ Throw", lldb::eLanguageTypeC_plus_plus},
- {"objc_catch", "Objective C Catch", lldb::eLanguageTypeObjC},
- {"objc_throw", "Objective C Throw", lldb::eLanguageTypeObjC},
- {"swift_catch", "Swift Catch", lldb::eLanguageTypeSwift},
- {"swift_throw", "Swift Throw", lldb::eLanguageTypeSwift}}),
+ : launch_info(nullptr), variables(), broadcaster("lldb-vscode"),
+ num_regs(0), num_locals(0), num_globals(0), log(),
+ exception_breakpoints(
+ {{"cpp_catch", "C++ Catch", lldb::eLanguageTypeC_plus_plus},
+ {"cpp_throw", "C++ Throw", lldb::eLanguageTypeC_plus_plus},
+ {"objc_catch", "Objective C Catch", lldb::eLanguageTypeObjC},
+ {"objc_throw", "Objective C Throw", lldb::eLanguageTypeObjC},
+ {"swift_catch", "Swift Catch", lldb::eLanguageTypeSwift},
+ {"swift_throw", "Swift Throw", lldb::eLanguageTypeSwift}}),
focus_tid(LLDB_INVALID_THREAD_ID), sent_terminated_event(false),
stop_at_entry(false) {
const char *log_file_path = getenv("LLDBVSCODE_LOG");
@@ -55,22 +58,6 @@
}
VSCode::~VSCode() {
- CloseInputStream();
- CloseOutputStream();
-}
-
-void VSCode::CloseInputStream() {
- if (in != stdin) {
- fclose(in);
- in = nullptr;
- }
-}
-
-void VSCode::CloseOutputStream() {
- if (out != stdout) {
- fclose(out);
- out = nullptr;
- }
}
int64_t VSCode::GetLineForPC(int64_t sourceReference, lldb::addr_t pc) const {
@@ -103,9 +90,11 @@
// JSON bytes.
//----------------------------------------------------------------------
void VSCode::SendJSON(const std::string &json_str) {
- fprintf(out, "Content-Length: %u\r\n\r\n%s", (uint32_t)json_str.size(),
- json_str.c_str());
- fflush(out);
+ output.write_full("Content-Length: ");
+ output.write_full(llvm::utostr(json_str.size()));
+ output.write_full("\r\n\r\n");
+ output.write_full(json_str);
+
if (log) {
*log << "<-- " << std::endl
<< "Content-Length: " << json_str.size() << "\r\n\r\n"
@@ -130,44 +119,25 @@
// Read a JSON packet from the "in" stream.
//----------------------------------------------------------------------
std::string VSCode::ReadJSON() {
- static std::string header("Content-Length: ");
-
- uint32_t packet_len = 0;
+ std::string length_str;
std::string json_str;
- char line[1024];
-
- while (fgets(line, sizeof(line), in)) {
- if (strncmp(line, header.data(), header.size()) == 0) {
- packet_len = atoi(line + header.size());
- if (fgets(line, sizeof(line), in)) {
- if (!IsEmptyLine(line))
- if (log)
- *log << "warning: expected empty line but got: \"" << line << "\""
- << std::endl;
- break;
- }
- } else {
- if (log)
- *log << "warning: expected \"" << header << "\" but got: \"" << line
- << "\"" << std::endl;
- }
- }
- // This is followed by two windows newline sequences ("\r\n\r\n") so eat
- // two the newline sequences
- if (packet_len > 0) {
- json_str.resize(packet_len);
- auto bytes_read = fread(&json_str[0], 1, packet_len, in);
- if (bytes_read < packet_len) {
- if (log)
- *log << "error: read fewer bytes (" << bytes_read
- << ") than requested (" << packet_len << ")" << std::endl;
- json_str.erase(bytes_read);
- }
- if (log) {
- *log << "--> " << std::endl;
- *log << header << packet_len << "\r\n\r\n" << json_str << std::endl;
- }
- }
+ int length;
+
+ if (!input.read_expected(log.get(), "Content-Length: "))
+ return json_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.get(), "\r\n"))
+ return json_str;
+
+ if (!input.read_full(log.get(), length, json_str))
+ return json_str;
+
return json_str;
}
Index: lldb/tools/lldb-vscode/IOStream.h
===================================================================
--- /dev/null
+++ lldb/tools/lldb-vscode/IOStream.h
@@ -0,0 +1,69 @@
+//===-- IOStream.h ----------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDBVSCODE_IOSTREAM_H_
+#define LLDBVSCODE_IOSTREAM_H_
+
+#if defined(_WIN32)
+// We need to #define NOMINMAX in order to skip `min()` and `max()` macro
+// definitions that conflict with other system headers.
+// We also need to #undef GetObject (which is defined to GetObjectW) because
+// the JSON code we use also has methods named `GetObject()` and we conflict
+// against these.
+#define NOMINMAX
+#include <windows.h>
+#else
+#include <sys/socket.h>
+#endif
+
+#include "llvm/ADT/StringRef.h"
+
+#include <fstream>
+#include <string>
+
+// Windows requires different system calls for dealing with sockets and other
+// types of files, so we can't simply have one code path that just uses read
+// and write everywhere. So we need an abstraction in order to allow us to
+// treat them identically.
+namespace lldb_vscode {
+struct StreamDescriptor {
+ StreamDescriptor();
+ ~StreamDescriptor();
+ StreamDescriptor(StreamDescriptor &&other);
+
+ StreamDescriptor &operator=(StreamDescriptor &&other);
+
+ static StreamDescriptor from_socket(SOCKET s, bool close);
+ static StreamDescriptor from_file(int fd, bool close);
+
+ bool m_is_socket = false;
+ bool m_close = false;
+ union {
+ int m_fd;
+ SOCKET m_socket;
+ };
+};
+
+struct InputStream {
+ StreamDescriptor descriptor;
+
+ bool read_full(std::ofstream *log, size_t length, std::string &text);
+
+ bool read_line(std::ofstream *log, std::string &line);
+
+ bool read_expected(std::ofstream *log, llvm::StringRef expected);
+};
+
+struct OutputStream {
+ StreamDescriptor descriptor;
+
+ bool write_full(llvm::StringRef str);
+};
+} // namespace lldb_vscode
+
+#endif
Index: lldb/tools/lldb-vscode/IOStream.cpp
===================================================================
--- /dev/null
+++ lldb/tools/lldb-vscode/IOStream.cpp
@@ -0,0 +1,153 @@
+//===-- IOStream.cpp --------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "IOStream.h"
+
+#if defined(_WIN32)
+#include <io.h>
+#else
+#include <netinet/in.h>
+#include <sys/socket.h>
+#include <unistd.h>
+#endif
+
+#include <fstream>
+#include <string>
+#include <vector>
+
+using namespace lldb_vscode;
+
+StreamDescriptor::StreamDescriptor() {}
+
+StreamDescriptor::StreamDescriptor(StreamDescriptor &&other) {
+ *this = std::move(other);
+}
+
+StreamDescriptor::~StreamDescriptor() {
+ if (!m_close)
+ return;
+
+ if (m_is_socket)
+#if defined(_WIN32)
+ ::closesocket(m_socket);
+#else
+ ::close(m_socket);
+#endif
+ else
+ ::close(m_fd);
+}
+
+StreamDescriptor &StreamDescriptor::operator=(StreamDescriptor &&other) {
+ m_close = other.m_close;
+ other.m_close = false;
+ m_is_socket = other.m_is_socket;
+ if (m_is_socket)
+ m_socket = other.m_socket;
+ else
+ m_fd = other.m_fd;
+ return *this;
+}
+
+StreamDescriptor StreamDescriptor::from_socket(SOCKET s, bool close) {
+ StreamDescriptor sd;
+ sd.m_is_socket = true;
+ sd.m_socket = s;
+ sd.m_close = close;
+ return sd;
+}
+
+StreamDescriptor StreamDescriptor::from_file(int fd, bool close) {
+ StreamDescriptor sd;
+ sd.m_is_socket = false;
+ sd.m_fd = fd;
+ sd.m_close = close;
+ return sd;
+}
+
+bool OutputStream::write_full(llvm::StringRef str) {
+ while (!str.empty()) {
+ int bytes_written = 0;
+ if (descriptor.m_is_socket)
+ bytes_written = ::send(descriptor.m_socket, str.data(), str.size(), 0);
+ else
+ bytes_written = ::write(descriptor.m_fd, str.data(), str.size());
+
+ if (bytes_written < 0) {
+ if (errno == EINTR || errno == EAGAIN)
+ continue;
+ return false;
+ }
+ str = str.drop_front(bytes_written);
+ }
+
+ return true;
+}
+
+bool InputStream::read_full(std::ofstream *log, size_t length,
+ std::string &text) {
+ std::string data;
+ data.resize(length);
+
+ char *ptr = &data[0];
+ while (length != 0) {
+ int bytes_read = 0;
+ if (descriptor.m_is_socket)
+ bytes_read = ::recv(descriptor.m_socket, ptr, length, 0);
+ else
+ bytes_read = ::read(descriptor.m_fd, ptr, length);
+
+ if (bytes_read < 0) {
+ int reason = 0;
+#if defined(_WIN32)
+ if (descriptor.m_is_socket)
+ reason = WSAGetLastError();
+ else
+ reason = errno;
+#else
+ int reason = errno;
+ if (reason == EINTR || reason == EAGAIN)
+ continue;
+#endif
+
+ if (log)
+ *log << "Error " << reason << " reading from input file.\n";
+ return false;
+ }
+
+ assert(bytes_read <= length);
+ ptr += bytes_read;
+ length -= bytes_read;
+ }
+ text += data;
+ return true;
+}
+
+bool InputStream::read_line(std::ofstream *log, std::string &line) {
+ line.clear();
+ while (true) {
+ if (!read_full(log, 1, line))
+ return false;
+
+ if (llvm::StringRef(line).endswith("\r\n"))
+ break;
+ }
+ line.erase(line.size() - 2);
+ return true;
+}
+
+bool InputStream::read_expected(std::ofstream *log, llvm::StringRef expected) {
+ std::string result;
+ if (!read_full(log, expected.size(), result))
+ return false;
+ if (expected != result) {
+ if (log)
+ *log << "Warning: Expected '" << expected.str() << "', got '" << result
+ << "\n";
+ }
+ return true;
+}
Index: lldb/tools/lldb-vscode/CMakeLists.txt
===================================================================
--- lldb/tools/lldb-vscode/CMakeLists.txt
+++ lldb/tools/lldb-vscode/CMakeLists.txt
@@ -15,6 +15,7 @@
BreakpointBase.cpp
ExceptionBreakpoint.cpp
FunctionBreakpoint.cpp
+ IOStream.cpp
JSONUtils.cpp
LLDBUtils.cpp
SourceBreakpoint.cpp
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
