[Lldb-commits] [clang] [lldb] [llvm] Patch series to reapply #118734 and substantially improve it (PR #120534)
chandlerc wrote: @dyung - OK, I think the current just-pushed version of this PR is worth another test. I've taught the TableGen string table emission to go back to working around the MSVC issues using a different table form that we used to use in LLVM when MSVC had a reliable error on it. It appears that now it only sometimes gets miscompiled, and so this workaround got accidentally removed without really being in a reliable place. But that still requires that all of the large string tables go through TableGen to get emitted in this form. So I've sent two PRs (separately, they seem independently good) to move NVPTX and Hexagon to TableGen, and this PR contains patches to move more of the ARM builtins to TableGen. I *think* this gets all of the ones that produced too-large string tables, but testing should show. Let me know. If this works and there isn't a really short path to upgrade, I'll start pulling some of the pre-requisite PRs out for independent review, and then update this PR once I'm down to the minimal chain of PRs that do the full conversion of builtins to string tables. Thanks, -Chandler https://github.com/llvm/llvm-project/pull/120534 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Implement `runInTerminal` for Windows (PR #121269)
https://github.com/SuibianP ready_for_review https://github.com/llvm/llvm-project/pull/121269 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Implement `runInTerminal` for Windows (PR #121269)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Hu Jialun (SuibianP)
Changes
Currently, the named pipe is passed by name and a transient `ofstream` is
constructed at each I/O request. This assumes,
- Blocking semantics: FIFO I/O waits for the other side to connect.
- Buffered semantics: Closing one side does not discard existing data.
The former can be replaced by `WaitNamedPipe`/`ConnectNamedPipe` on Win32, but
the second cannot be easily worked around. It is also impossible to have
another "keep-alive" pipe server instance, as server-client pairs are fixed on
connection on Win32 and the client may get connected to it instead of the real
one.
Refactor `FifoFile[IO]` to use an open file handles rather than file name.
---
Full diff: https://github.com/llvm/llvm-project/pull/121269.diff
5 Files Affected:
- (modified) lldb/tools/lldb-dap/FifoFiles.cpp (+102-19)
- (modified) lldb/tools/lldb-dap/FifoFiles.h (+20-15)
- (modified) lldb/tools/lldb-dap/RunInTerminal.cpp (+26-9)
- (modified) lldb/tools/lldb-dap/RunInTerminal.h (+4-2)
- (modified) lldb/tools/lldb-dap/lldb-dap.cpp (+39-10)
``diff
diff --git a/lldb/tools/lldb-dap/FifoFiles.cpp
b/lldb/tools/lldb-dap/FifoFiles.cpp
index 1f1bba80bd3b11..8bca006e63d657 100644
--- a/lldb/tools/lldb-dap/FifoFiles.cpp
+++ b/lldb/tools/lldb-dap/FifoFiles.cpp
@@ -9,7 +9,13 @@
#include "FifoFiles.h"
#include "JSONUtils.h"
-#if !defined(_WIN32)
+#include "llvm/Support/FileSystem.h"
+
+#if defined(_WIN32)
+#include
+#include
+#include
+#else
#include
#include
#include
@@ -24,27 +30,73 @@ using namespace llvm;
namespace lldb_dap {
-FifoFile::FifoFile(StringRef path) : m_path(path) {}
+std::error_code EC;
+FifoFile::FifoFile(StringRef path)
+: m_path(path), m_file(fopen(path.data(), "r+")) {
+ if (m_file == nullptr) {
+EC = std::error_code(errno, std::generic_category());
+llvm::errs() << "Failed to open fifo file: " << path << EC.message()
+ << "\n";
+std::terminate();
+ }
+ if (setvbuf(m_file, NULL, _IONBF, 0))
+llvm::errs() << "Error setting unbuffered mode on C FILE\n";
+}
+FifoFile::FifoFile(StringRef path, FILE *f) : m_path(path), m_file(f) {}
+FifoFile::FifoFile(FifoFile &&other)
+: m_path(other.m_path), m_file(other.m_file) {
+ other.m_file = nullptr;
+}
FifoFile::~FifoFile() {
+ if (m_file)
+fclose(m_file);
#if !defined(_WIN32)
+ // Unreferenced named pipes are deleted automatically on Win32
unlink(m_path.c_str());
#endif
}
-Expected> CreateFifoFile(StringRef path) {
-#if defined(_WIN32)
- return createStringError(inconvertibleErrorCode(), "Unimplemented");
+// This probably belongs to llvm::sys::fs as another FSEntity type
+std::error_code createNamedPipe(const Twine &Prefix, StringRef Suffix,
+int &ResultFd,
+SmallVectorImpl &ResultPath) {
+ const char *Middle = Suffix.empty() ? "-%%" : "-%%.";
+ auto EC = sys::fs::getPotentiallyUniqueFileName(
+#ifdef _WIN32
+ ".\\pipe\\LOCAL\\"
+#else
+ "/tmp/"
+#endif
+ + Prefix + Middle + Suffix,
+ ResultPath);
+ if (EC)
+return EC;
+ ResultPath.push_back(0);
+ const char *path = ResultPath.data();
+#ifdef _WIN32
+ HANDLE h = ::CreateNamedPipeA(
+ path, PIPE_ACCESS_DUPLEX | FILE_FLAG_FIRST_PIPE_INSTANCE,
+ PIPE_TYPE_BYTE | PIPE_READMODE_BYTE | PIPE_WAIT, 1, 1024, 1024, 0, NULL);
+ if (h == INVALID_HANDLE_VALUE)
+return std::error_code(::GetLastError(), std::system_category());
+ ResultFd = _open_osfhandle((intptr_t)h, _O_TEXT | _O_RDWR);
+ if (ResultFd == -1)
+return std::error_code(::GetLastError(), std::system_category());
#else
- if (int err = mkfifo(path.data(), 0600))
-return createStringError(std::error_code(err, std::generic_category()),
- "Couldn't create fifo file: %s", path.data());
- return std::make_shared(path);
+ if (mkfifo(path, 0600) == -1)
+return std::error_code(errno, std::generic_category());
+ EC = openFileForWrite(ResultPath, ResultFd, sys::fs::CD_OpenExisting,
+sys::fs::OF_None, 0600);
+ if (EC)
+return EC;
#endif
+ return std::error_code();
}
-FifoFileIO::FifoFileIO(StringRef fifo_file, StringRef other_endpoint_name)
-: m_fifo_file(fifo_file), m_other_endpoint_name(other_endpoint_name) {}
+FifoFileIO::FifoFileIO(FifoFile &&fifo_file, StringRef other_endpoint_name)
+: m_fifo_file(std::move(fifo_file)),
+ m_other_endpoint_name(other_endpoint_name) {}
Expected FifoFileIO::ReadJSON(std::chrono::milliseconds timeout) {
// We use a pointer for this future, because otherwise its normal destructor
@@ -52,13 +104,20 @@ Expected
FifoFileIO::ReadJSON(std::chrono::milliseconds timeout) {
std::optional line;
std::future *future =
new std::future(std::async(std::launch::async, [&]() {
-std::ifstream reader(m_fifo_file, std::ifstream::in);
-std::string buff
[Lldb-commits] [lldb] [lldb-dap] Implement `runInTerminal` for Windows (PR #121269)
SuibianP wrote: As it turns out that Win32 `execvp` actually [creates a new process instead of replace the current process image](https://github.com/python/cpython/issues/101191#issuecomment-1399013437), I changed the implementation to use `CreateProcess` before reporting the PID of the target. The current implementation works with dape under emacs and should be functional, so I marked it ready for review. On VSCode however, while the extension is able to attach to the process and obtain sort of a stack backtrace, all breakpoints show up as unverified and does not trigger even if re-toggled. I am looking into it now. --- This is not relevant anymore but for the sake of completion: In the old implementation (wait for attach then `execvp`), the launcher process does get resumed after some 3 minutes. I still do not understand how it happened. https://github.com/llvm/llvm-project/pull/121269 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Implement `runInTerminal` for Windows (PR #121269)
https://github.com/SuibianP updated
https://github.com/llvm/llvm-project/pull/121269
>From c23b994141630ef9a1c56760aae889f37334aa69 Mon Sep 17 00:00:00 2001
From: Hu Jialun
Date: Sat, 28 Dec 2024 22:39:33 +0800
Subject: [PATCH] [lldb-dap] Implement runInTerminal for Windows
Currently, the named pipe is passed by name and a transient ofstream is
constructed at each I/O request. This assumes,
- Blocking semantics: FIFO I/O waits for the other side to connect.
- Buffered semantics: Closing one side does not discard existing data.
The former can be replaced by WaitNamedPipe/ConnectNamedPipe on Win32,
but the second cannot be easily worked around. It is also impossible to
have another "keep-alive" pipe server instance, as server-client pairs
are fixed on connection on Win32 and the client may get connected to it
instead of the real one.
Refactor FifoFile[IO] to use an open file handles rather than file name.
Win32 provides no way to replace the process image. Under the hood exec*
actually creates a new process with a new PID. DebugActiveProcess also
cannot get notified of process creations.
Create the new process in a suspended state and resume it after attach.
---
lldb/tools/lldb-dap/FifoFiles.cpp | 121 ++
lldb/tools/lldb-dap/FifoFiles.h | 35
lldb/tools/lldb-dap/RunInTerminal.cpp | 35 ++--
lldb/tools/lldb-dap/RunInTerminal.h | 6 +-
lldb/tools/lldb-dap/lldb-dap.cpp | 49 ---
5 files changed, 191 insertions(+), 55 deletions(-)
diff --git a/lldb/tools/lldb-dap/FifoFiles.cpp
b/lldb/tools/lldb-dap/FifoFiles.cpp
index 1f1bba80bd3b11..8bca006e63d657 100644
--- a/lldb/tools/lldb-dap/FifoFiles.cpp
+++ b/lldb/tools/lldb-dap/FifoFiles.cpp
@@ -9,7 +9,13 @@
#include "FifoFiles.h"
#include "JSONUtils.h"
-#if !defined(_WIN32)
+#include "llvm/Support/FileSystem.h"
+
+#if defined(_WIN32)
+#include
+#include
+#include
+#else
#include
#include
#include
@@ -24,27 +30,73 @@ using namespace llvm;
namespace lldb_dap {
-FifoFile::FifoFile(StringRef path) : m_path(path) {}
+std::error_code EC;
+FifoFile::FifoFile(StringRef path)
+: m_path(path), m_file(fopen(path.data(), "r+")) {
+ if (m_file == nullptr) {
+EC = std::error_code(errno, std::generic_category());
+llvm::errs() << "Failed to open fifo file: " << path << EC.message()
+ << "\n";
+std::terminate();
+ }
+ if (setvbuf(m_file, NULL, _IONBF, 0))
+llvm::errs() << "Error setting unbuffered mode on C FILE\n";
+}
+FifoFile::FifoFile(StringRef path, FILE *f) : m_path(path), m_file(f) {}
+FifoFile::FifoFile(FifoFile &&other)
+: m_path(other.m_path), m_file(other.m_file) {
+ other.m_file = nullptr;
+}
FifoFile::~FifoFile() {
+ if (m_file)
+fclose(m_file);
#if !defined(_WIN32)
+ // Unreferenced named pipes are deleted automatically on Win32
unlink(m_path.c_str());
#endif
}
-Expected> CreateFifoFile(StringRef path) {
-#if defined(_WIN32)
- return createStringError(inconvertibleErrorCode(), "Unimplemented");
+// This probably belongs to llvm::sys::fs as another FSEntity type
+std::error_code createNamedPipe(const Twine &Prefix, StringRef Suffix,
+int &ResultFd,
+SmallVectorImpl &ResultPath) {
+ const char *Middle = Suffix.empty() ? "-%%" : "-%%.";
+ auto EC = sys::fs::getPotentiallyUniqueFileName(
+#ifdef _WIN32
+ ".\\pipe\\LOCAL\\"
+#else
+ "/tmp/"
+#endif
+ + Prefix + Middle + Suffix,
+ ResultPath);
+ if (EC)
+return EC;
+ ResultPath.push_back(0);
+ const char *path = ResultPath.data();
+#ifdef _WIN32
+ HANDLE h = ::CreateNamedPipeA(
+ path, PIPE_ACCESS_DUPLEX | FILE_FLAG_FIRST_PIPE_INSTANCE,
+ PIPE_TYPE_BYTE | PIPE_READMODE_BYTE | PIPE_WAIT, 1, 1024, 1024, 0, NULL);
+ if (h == INVALID_HANDLE_VALUE)
+return std::error_code(::GetLastError(), std::system_category());
+ ResultFd = _open_osfhandle((intptr_t)h, _O_TEXT | _O_RDWR);
+ if (ResultFd == -1)
+return std::error_code(::GetLastError(), std::system_category());
#else
- if (int err = mkfifo(path.data(), 0600))
-return createStringError(std::error_code(err, std::generic_category()),
- "Couldn't create fifo file: %s", path.data());
- return std::make_shared(path);
+ if (mkfifo(path, 0600) == -1)
+return std::error_code(errno, std::generic_category());
+ EC = openFileForWrite(ResultPath, ResultFd, sys::fs::CD_OpenExisting,
+sys::fs::OF_None, 0600);
+ if (EC)
+return EC;
#endif
+ return std::error_code();
}
-FifoFileIO::FifoFileIO(StringRef fifo_file, StringRef other_endpoint_name)
-: m_fifo_file(fifo_file), m_other_endpoint_name(other_endpoint_name) {}
+FifoFileIO::FifoFileIO(FifoFile &&fifo_file, StringRef other_endpoint_name)
+: m_fifo_file(std::move(fifo_file)),
+ m_other_endpoint_name(other_endpoint_name) {}
Expected FifoFileIO::ReadJSON(std::chrono::mi
