[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
slydiman wrote: I have moved all Pipe related changes to #101383. https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
slydiman wrote: @labath > one of the issues (ETXTBSY) you refer to as "system bugs" I meant https://bugzilla.kernel.org/show_bug.cgi?id=546 and [this discussion](https://github.com/llvm/llvm-project/pull/100670/files#r1693201568). I'm sad that you don't believe. I concluded that no one uses the real multithreading on Linux. Someone tried and faced unsolvable problems. Therefore, #100670 is doomed. https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
https://github.com/slydiman edited https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
https://github.com/slydiman updated https://github.com/llvm/llvm-project/pull/101283 >From 6b2a41ba3d71270e81e24a42d3b4f5dc2f96b939 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Wed, 31 Jul 2024 05:41:21 +0400 Subject: [PATCH] [lldb] Updated lldb-server to spawn the child process and share socket on Windows `lldb-server platform --server` works on Windows now w/o multithreading. The rest functionality remains unchanged. Depends on #101383. Fixes #90923, fixes #56346. This is the part 1 of the replacement of #100670. In the part 2 I plan to switch `lldb-server gdbserver` to use `--fd` and listen a common gdb port for all gdbserver connections. Then we can remove gdb port mapping to fix #97537. --- lldb/tools/lldb-server/lldb-platform.cpp | 347 --- 1 file changed, 306 insertions(+), 41 deletions(-) diff --git a/lldb/tools/lldb-server/lldb-platform.cpp b/lldb/tools/lldb-server/lldb-platform.cpp index 7148a1d2a3094..e23237ef574c3 100644 --- a/lldb/tools/lldb-server/lldb-platform.cpp +++ b/lldb/tools/lldb-server/lldb-platform.cpp @@ -22,6 +22,7 @@ #include #include "llvm/Support/FileSystem.h" +#include "llvm/Support/ScopedPrinter.h" #include "llvm/Support/WithColor.h" #include "llvm/Support/raw_ostream.h" @@ -32,8 +33,10 @@ #include "lldb/Host/ConnectionFileDescriptor.h" #include "lldb/Host/HostGetOpt.h" #include "lldb/Host/OptionParser.h" +#include "lldb/Host/Socket.h" #include "lldb/Host/common/TCPSocket.h" #include "lldb/Utility/FileSpec.h" +#include "lldb/Utility/LLDBLog.h" #include "lldb/Utility/Status.h" using namespace lldb; @@ -60,6 +63,9 @@ static struct option g_long_options[] = { {"max-gdbserver-port", required_argument, nullptr, 'M'}, {"socket-file", required_argument, nullptr, 'f'}, {"server", no_argument, _server, 1}, +#if defined(_WIN32) +{"accept", required_argument, nullptr, 'a'}, +#endif {nullptr, 0, nullptr, 0}}; #if defined(__APPLE__) @@ -114,6 +120,249 @@ static Status save_socket_id_to_file(const std::string _id, return status; } +static void client_handle(GDBRemoteCommunicationServerPlatform , + const lldb_private::Args ) { + if (!platform.IsConnected()) +return; + + if (args.GetArgumentCount() > 0) { +lldb::pid_t pid = LLDB_INVALID_PROCESS_ID; +std::optional port; +std::string socket_name; +Status error = platform.LaunchGDBServer(args, +"", // hostname +pid, port, socket_name); +if (error.Success()) + platform.SetPendingGdbServer(pid, *port, socket_name); +else + fprintf(stderr, "failed to start gdbserver: %s\n", error.AsCString()); + } + + bool interrupt = false; + bool done = false; + Status error; + while (!interrupt && !done) { +if (platform.GetPacketAndSendResponse(std::nullopt, error, interrupt, + done) != +GDBRemoteCommunication::PacketResult::Success) + break; + } + + if (error.Fail()) +WithColor::error() << error.AsCString() << '\n'; +} + +static GDBRemoteCommunicationServerPlatform::PortMap gdbserver_portmap; +static std::mutex gdbserver_portmap_mutex; + +#if defined(_WIN32) +static void spawn_process_reaped(lldb::pid_t pid, int signal, int status) { + std::lock_guard guard(gdbserver_portmap_mutex); + gdbserver_portmap.FreePortForProcess(pid); +} + +static bool spawn_process_parent(const char *progname, Connection *conn, + uint16_t gdb_port, uint16_t port_offset, + const lldb_private::Args , + const std::string _file, + const StringRef log_channels) { + Log *log = GetLog(LLDBLog::Platform); + Pipe socket_pipe; + Status error = socket_pipe.CreateNew(true); + if (error.Fail()) { +LLDB_LOGF(log, + "lldb-platform parent: " + "cannot create pipe: %s", + error.AsCString()); +return false; + } + + ProcessLaunchInfo launch_info; + FileSpec self_spec(progname, FileSpec::Style::native); + launch_info.SetExecutableFile(self_spec, true); + Args _args = launch_info.GetArguments(); + self_args.AppendArgument(llvm::StringRef("platform")); + self_args.AppendArgument(llvm::StringRef("--accept")); + self_args.AppendArgument(llvm::to_string(socket_pipe.GetReadPipe())); + if (gdb_port) { +self_args.AppendArgument(llvm::StringRef("--gdbserver-port")); +self_args.AppendArgument(llvm::to_string(gdb_port)); + } + if (port_offset > 0) { +self_args.AppendArgument(llvm::StringRef("--port-offset")); +self_args.AppendArgument(llvm::to_string(port_offset)); + } + if (!log_file.empty()) { +self_args.AppendArgument(llvm::StringRef("--log-file")); +self_args.AppendArgument(log_file); + } + if (!log_channels.empty()) { +
[Lldb-commits] [lldb] [lldb] Added Pipe::WriteWithTimeout() (PR #101383)
https://github.com/slydiman created https://github.com/llvm/llvm-project/pull/101383 Fixed few bugs in PipeWindows. Added the test for async read/write. >From 1d6edb89e242f17908518e9d1b5da431d0d0908b Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Wed, 31 Jul 2024 22:02:53 +0400 Subject: [PATCH] [lldb] Added Pipe::WriteWithTimeout() Fixed few bugs in PipeWindows. Added the test for async read/write. --- lldb/include/lldb/Host/PipeBase.h| 5 +- lldb/include/lldb/Host/posix/PipePosix.h | 4 +- lldb/include/lldb/Host/windows/PipeWindows.h | 5 +- lldb/source/Host/common/PipeBase.cpp | 5 + lldb/source/Host/posix/PipePosix.cpp | 6 +- lldb/source/Host/windows/PipeWindows.cpp | 124 --- lldb/unittests/Host/PipeTest.cpp | 61 + 7 files changed, 161 insertions(+), 49 deletions(-) diff --git a/lldb/include/lldb/Host/PipeBase.h b/lldb/include/lldb/Host/PipeBase.h index 48c19b899cef6..d51d0cd54e036 100644 --- a/lldb/include/lldb/Host/PipeBase.h +++ b/lldb/include/lldb/Host/PipeBase.h @@ -56,7 +56,10 @@ class PipeBase { // Delete named pipe. virtual Status Delete(llvm::StringRef name) = 0; - virtual Status Write(const void *buf, size_t size, size_t _written) = 0; + virtual Status WriteWithTimeout(const void *buf, size_t size, + const std::chrono::microseconds , + size_t _written) = 0; + Status Write(const void *buf, size_t size, size_t _written); virtual Status ReadWithTimeout(void *buf, size_t size, const std::chrono::microseconds , size_t _read) = 0; diff --git a/lldb/include/lldb/Host/posix/PipePosix.h b/lldb/include/lldb/Host/posix/PipePosix.h index ec4c752a24e94..2e291160817c4 100644 --- a/lldb/include/lldb/Host/posix/PipePosix.h +++ b/lldb/include/lldb/Host/posix/PipePosix.h @@ -64,7 +64,9 @@ class PipePosix : public PipeBase { Status Delete(llvm::StringRef name) override; - Status Write(const void *buf, size_t size, size_t _written) override; + Status WriteWithTimeout(const void *buf, size_t size, + const std::chrono::microseconds , + size_t _written) override; Status ReadWithTimeout(void *buf, size_t size, const std::chrono::microseconds , size_t _read) override; diff --git a/lldb/include/lldb/Host/windows/PipeWindows.h b/lldb/include/lldb/Host/windows/PipeWindows.h index 4b5be28d7ae6c..e28d104cc60ec 100644 --- a/lldb/include/lldb/Host/windows/PipeWindows.h +++ b/lldb/include/lldb/Host/windows/PipeWindows.h @@ -32,7 +32,6 @@ class PipeWindows : public PipeBase { Status CreateNew(bool child_process_inherit) override; // Create a named pipe. - Status CreateNewNamed(bool child_process_inherit); Status CreateNew(llvm::StringRef name, bool child_process_inherit) override; Status CreateWithUniqueName(llvm::StringRef prefix, bool child_process_inherit, @@ -60,7 +59,9 @@ class PipeWindows : public PipeBase { Status Delete(llvm::StringRef name) override; - Status Write(const void *buf, size_t size, size_t _written) override; + Status WriteWithTimeout(const void *buf, size_t size, + const std::chrono::microseconds , + size_t _written) override; Status ReadWithTimeout(void *buf, size_t size, const std::chrono::microseconds , size_t _read) override; diff --git a/lldb/source/Host/common/PipeBase.cpp b/lldb/source/Host/common/PipeBase.cpp index b3e0ab34a58df..904a2df12392d 100644 --- a/lldb/source/Host/common/PipeBase.cpp +++ b/lldb/source/Host/common/PipeBase.cpp @@ -18,6 +18,11 @@ Status PipeBase::OpenAsWriter(llvm::StringRef name, std::chrono::microseconds::zero()); } +Status PipeBase::Write(const void *buf, size_t size, size_t _written) { + return WriteWithTimeout(buf, size, std::chrono::microseconds::zero(), + bytes_written); +} + Status PipeBase::Read(void *buf, size_t size, size_t _read) { return ReadWithTimeout(buf, size, std::chrono::microseconds::zero(), bytes_read); diff --git a/lldb/source/Host/posix/PipePosix.cpp b/lldb/source/Host/posix/PipePosix.cpp index f35c348990df6..00c6242f3f2e8 100644 --- a/lldb/source/Host/posix/PipePosix.cpp +++ b/lldb/source/Host/posix/PipePosix.cpp @@ -335,7 +335,9 @@ Status PipePosix::ReadWithTimeout(void *buf, size_t size, return error; } -Status PipePosix::Write(const void *buf, size_t size, size_t _written) { +Status PipePosix::WriteWithTimeout(const void *buf, size_t size, + const std::chrono::microseconds , + size_t _written) { std::lock_guard guard(m_write_mutex); bytes_written = 0; if
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
https://github.com/slydiman edited https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed lldb-server crash (TestLogHandler was not thread safe) (PR #101326)
https://github.com/slydiman closed https://github.com/llvm/llvm-project/pull/101326 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
@@ -114,6 +120,218 @@ static Status save_socket_id_to_file(const std::string _id, return status; } +static GDBRemoteCommunicationServerPlatform::PortMap gdbserver_portmap; +static std::mutex gdbserver_portmap_mutex; + +#if defined(_WIN32) +static void SpawnProcessReaped(lldb::pid_t pid, int signal, int status) { + std::lock_guard guard(gdbserver_portmap_mutex); + gdbserver_portmap.FreePortForProcess(pid); +} + +static bool SpawnProcessParent(const char *progname, Connection *conn, + uint16_t gdb_port, uint16_t port_offset, + const lldb_private::Args , + const std::string _file, + const StringRef log_channels) { + Log *log = GetLog(LLDBLog::Platform); + Pipe socket_pipe; + Status error = socket_pipe.CreateNew(true); + if (error.Fail()) { +LLDB_LOGF(log, + "lldb-platform parent: " + "cannot create pipe: %s", + error.AsCString()); +return false; + } + + ProcessLaunchInfo launch_info; + FileSpec self_spec(progname, FileSpec::Style::native); + launch_info.SetExecutableFile(self_spec, true); + Args _args = launch_info.GetArguments(); + self_args.AppendArgument(llvm::StringRef("platform")); + self_args.AppendArgument(llvm::StringRef("--accept")); + self_args.AppendArgument(llvm::to_string(socket_pipe.GetReadPipe())); + if (gdb_port) { +self_args.AppendArgument(llvm::StringRef("--gdbserver-port")); +self_args.AppendArgument(llvm::to_string(gdb_port)); + } + if (port_offset > 0) { +self_args.AppendArgument(llvm::StringRef("--port-offset")); +self_args.AppendArgument(llvm::to_string(port_offset)); + } + if (!log_file.empty()) { +self_args.AppendArgument(llvm::StringRef("--log-file")); +self_args.AppendArgument(log_file); + } + if (!log_channels.empty()) { +self_args.AppendArgument(llvm::StringRef("--log-channels")); +self_args.AppendArgument(log_channels); + } + if (args.GetArgumentCount() > 0) { +self_args.AppendArgument("--"); +self_args.AppendArguments(args); + } + + launch_info.SetLaunchInSeparateProcessGroup(false); + launch_info.SetMonitorProcessCallback(); + + // Copy the current environment. + // WSASocket(FROM_PROTOCOL_INFO) will fail in the child process + // with the error WSAEPROVIDERFAILEDINIT if the SystemRoot is missing + // in the environment. + launch_info.GetEnvironment() = Host::GetEnvironment(); + + launch_info.GetFlags().Set(eLaunchFlagDisableSTDIO); + + launch_info.AppendCloseFileAction(socket_pipe.GetWriteFileDescriptor()); + + // Close STDIN, STDOUT and STDERR. + launch_info.AppendCloseFileAction(STDIN_FILENO); + launch_info.AppendCloseFileAction(STDOUT_FILENO); + launch_info.AppendCloseFileAction(STDERR_FILENO); + + // Redirect STDIN, STDOUT and STDERR to "/dev/null". + launch_info.AppendSuppressFileAction(STDIN_FILENO, true, false); + launch_info.AppendSuppressFileAction(STDOUT_FILENO, false, true); + launch_info.AppendSuppressFileAction(STDERR_FILENO, false, true); + + std::string cmd; + self_args.GetCommandString(cmd); + + error = Host::LaunchProcess(launch_info); + if (error.Fail()) { +LLDB_LOGF(log, + "lldb-platform parent: " + "cannot launch child process for connection: %s", + error.AsCString()); +return false; + } + + lldb::pid_t childPid = launch_info.GetProcessID(); + if (childPid == LLDB_INVALID_PROCESS_ID) { +LLDB_LOGF(log, "lldb-platform parent: " + "cannot launch child process for connection: invalid pid"); +return false; + } + LLDB_LOGF(log, +"lldb-platform parent: " +"launched '%s', pid=0x%x", +cmd.c_str(), childPid); + + { +std::lock_guard guard(gdbserver_portmap_mutex); +gdbserver_portmap.AssociatePortWithProcess(gdb_port, childPid); + } + + if (socket_pipe.CanRead()) +socket_pipe.CloseReadFileDescriptor(); + if (!socket_pipe.CanWrite()) { +LLDB_LOGF(log, "lldb-platform parent: " + "cannot write to socket_pipe"); +Host::Kill(childPid, SIGTERM); +return false; + } + + const TCPSocket = + static_cast(*conn->GetReadObject()); + NativeSocket nativeSocket = socket.GetNativeSocket(); + + WSAPROTOCOL_INFO protocol_info; + if (::WSADuplicateSocket(nativeSocket, childPid, _info) == + SOCKET_ERROR) { +LLDB_LOGF(log, + "lldb-platform parent: " + "WSADuplicateSocket() failed, error: %d", + ::WSAGetLastError()); +Host::Kill(childPid, SIGTERM); +return false; + } + + size_t num_bytes; + error = socket_pipe.WriteWithTimeout(_info, sizeof(protocol_info), + std::chrono::seconds(2), num_bytes); + if (error.Fail()) { +LLDB_LOGF(log, + "lldb-platform parent: " + "socket_pipe.WriteWithTimeout(WSAPROTOCOL_INFO) failed: %s", +
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
https://github.com/slydiman edited https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
https://github.com/slydiman updated https://github.com/llvm/llvm-project/pull/101283 >From add315c8db91c4d51f9b298cc64478c0497857a5 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Wed, 31 Jul 2024 05:41:21 +0400 Subject: [PATCH] [lldb] Updated lldb-server to spawn the child process and share socket on Windows `lldb-server platform --server` works on Windows now w/o multithreading. The rest functionality remains unchanged. Added also PipeWindows::WriteWithTimeout(), fixed PipeWindows::ReadWithTimeout() and missing initialization of m_read_overlapped.hEvent in the constructor PipeWindows(lldb::pipe_t read, lldb::pipe_t write). Fixes #90923, fixes #56346. Depends on #101326. This is the part 1 of the replacement of #100670. In the part 2 I plan to switch `lldb-server gdbserver` to use `--fd` and listen a common gdb port for all gdbserver connections. Then we can remove gdb port mapping to fix #97537. --- lldb/include/lldb/Host/windows/PipeWindows.h | 3 + lldb/source/Host/windows/PipeWindows.cpp | 69 +++- lldb/tools/lldb-server/lldb-platform.cpp | 347 --- 3 files changed, 370 insertions(+), 49 deletions(-) diff --git a/lldb/include/lldb/Host/windows/PipeWindows.h b/lldb/include/lldb/Host/windows/PipeWindows.h index 4b5be28d7ae6c..c99cf52be46c1 100644 --- a/lldb/include/lldb/Host/windows/PipeWindows.h +++ b/lldb/include/lldb/Host/windows/PipeWindows.h @@ -61,6 +61,9 @@ class PipeWindows : public PipeBase { Status Delete(llvm::StringRef name) override; Status Write(const void *buf, size_t size, size_t _written) override; + Status WriteWithTimeout(const void *buf, size_t size, + const std::chrono::microseconds , + size_t _written); Status ReadWithTimeout(void *buf, size_t size, const std::chrono::microseconds , size_t _read) override; diff --git a/lldb/source/Host/windows/PipeWindows.cpp b/lldb/source/Host/windows/PipeWindows.cpp index c82c919607b5b..8f86c7e28046e 100644 --- a/lldb/source/Host/windows/PipeWindows.cpp +++ b/lldb/source/Host/windows/PipeWindows.cpp @@ -58,7 +58,10 @@ PipeWindows::PipeWindows(pipe_t read, pipe_t write) } ZeroMemory(_read_overlapped, sizeof(m_read_overlapped)); + m_read_overlapped.hEvent = ::CreateEventA(nullptr, TRUE, FALSE, nullptr); + ZeroMemory(_write_overlapped, sizeof(m_write_overlapped)); + m_write_overlapped.hEvent = ::CreateEventA(nullptr, TRUE, FALSE, nullptr); } PipeWindows::~PipeWindows() { Close(); } @@ -77,6 +80,7 @@ Status PipeWindows::CreateNew(bool child_process_inherit) { m_write_fd = _open_osfhandle((intptr_t)m_write, _O_WRONLY); ZeroMemory(_write_overlapped, sizeof(m_write_overlapped)); + m_write_overlapped.hEvent = ::CreateEventA(nullptr, TRUE, FALSE, nullptr); return Status(); } @@ -202,6 +206,7 @@ Status PipeWindows::OpenNamedPipe(llvm::StringRef name, m_write_fd = _open_osfhandle((intptr_t)m_write, _O_WRONLY); ZeroMemory(_write_overlapped, sizeof(m_write_overlapped)); +m_write_overlapped.hEvent = ::CreateEventA(nullptr, TRUE, FALSE, nullptr); } return Status(); @@ -228,6 +233,8 @@ int PipeWindows::ReleaseWriteFileDescriptor() { return PipeWindows::kInvalidDescriptor; int result = m_write_fd; m_write_fd = PipeWindows::kInvalidDescriptor; + if (m_write_overlapped.hEvent) +::CloseHandle(m_write_overlapped.hEvent); m_write = INVALID_HANDLE_VALUE; ZeroMemory(_write_overlapped, sizeof(m_write_overlapped)); return result; @@ -250,6 +257,9 @@ void PipeWindows::CloseWriteFileDescriptor() { if (!CanWrite()) return; + if (m_write_overlapped.hEvent) +::CloseHandle(m_write_overlapped.hEvent); + _close(m_write_fd); m_write = INVALID_HANDLE_VALUE; m_write_fd = PipeWindows::kInvalidDescriptor; @@ -283,7 +293,12 @@ Status PipeWindows::ReadWithTimeout(void *buf, size_t size, DWORD sys_bytes_read = size; BOOL result = ::ReadFile(m_read, buf, sys_bytes_read, _bytes_read, _read_overlapped); - if (!result && GetLastError() != ERROR_IO_PENDING) + if (result) { +bytes_read = sys_bytes_read; +return Status(); + } + + if (GetLastError() != ERROR_IO_PENDING) return Status(::GetLastError(), eErrorTypeWin32); DWORD timeout = (duration == std::chrono::microseconds::zero()) @@ -319,18 +334,56 @@ Status PipeWindows::ReadWithTimeout(void *buf, size_t size, Status PipeWindows::Write(const void *buf, size_t num_bytes, size_t _written) { + return WriteWithTimeout(buf, num_bytes, std::chrono::microseconds::zero(), + bytes_written); +} + +Status PipeWindows::WriteWithTimeout(const void *buf, size_t size, + const std::chrono::microseconds , + size_t _written) { if (!CanWrite()) return Status(ERROR_INVALID_HANDLE, eErrorTypeWin32); -
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
https://github.com/slydiman edited https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
https://github.com/slydiman updated https://github.com/llvm/llvm-project/pull/101283 >From c785238cf366746a383e74a89be7c7431cd41d3c Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Wed, 31 Jul 2024 05:41:21 +0400 Subject: [PATCH] [lldb] Updated lldb-server to spawn the child process and share socket on Windows `lldb-server platform --server` works on Windows now w/o multithreading. The rest functionality remains unchanged. Added also PipeWindows::WriteWithTimeout(), fixed PipeWindows::ReadWithTimeout() and missing initialization of m_read_overlapped.hEvent in the constructor PipeWindows(lldb::pipe_t read, lldb::pipe_t write). Fixes #90923, fixes #56346. Depends on #101326. This is the part 1 of the replacement of #100670. In the part 2 I plan to switch `lldb-server gdbserver` to use `--fd` and listen a common gdb port for all gdbserver connections. Then we can remove gdb port mapping to fix #97537. --- lldb/include/lldb/Host/windows/PipeWindows.h | 3 + lldb/source/Host/windows/PipeWindows.cpp | 69 +++- lldb/tools/lldb-server/lldb-platform.cpp | 347 --- 3 files changed, 370 insertions(+), 49 deletions(-) diff --git a/lldb/include/lldb/Host/windows/PipeWindows.h b/lldb/include/lldb/Host/windows/PipeWindows.h index 4b5be28d7ae6c..c99cf52be46c1 100644 --- a/lldb/include/lldb/Host/windows/PipeWindows.h +++ b/lldb/include/lldb/Host/windows/PipeWindows.h @@ -61,6 +61,9 @@ class PipeWindows : public PipeBase { Status Delete(llvm::StringRef name) override; Status Write(const void *buf, size_t size, size_t _written) override; + Status WriteWithTimeout(const void *buf, size_t size, + const std::chrono::microseconds , + size_t _written); Status ReadWithTimeout(void *buf, size_t size, const std::chrono::microseconds , size_t _read) override; diff --git a/lldb/source/Host/windows/PipeWindows.cpp b/lldb/source/Host/windows/PipeWindows.cpp index c82c919607b5b..8f86c7e28046e 100644 --- a/lldb/source/Host/windows/PipeWindows.cpp +++ b/lldb/source/Host/windows/PipeWindows.cpp @@ -58,7 +58,10 @@ PipeWindows::PipeWindows(pipe_t read, pipe_t write) } ZeroMemory(_read_overlapped, sizeof(m_read_overlapped)); + m_read_overlapped.hEvent = ::CreateEventA(nullptr, TRUE, FALSE, nullptr); + ZeroMemory(_write_overlapped, sizeof(m_write_overlapped)); + m_write_overlapped.hEvent = ::CreateEventA(nullptr, TRUE, FALSE, nullptr); } PipeWindows::~PipeWindows() { Close(); } @@ -77,6 +80,7 @@ Status PipeWindows::CreateNew(bool child_process_inherit) { m_write_fd = _open_osfhandle((intptr_t)m_write, _O_WRONLY); ZeroMemory(_write_overlapped, sizeof(m_write_overlapped)); + m_write_overlapped.hEvent = ::CreateEventA(nullptr, TRUE, FALSE, nullptr); return Status(); } @@ -202,6 +206,7 @@ Status PipeWindows::OpenNamedPipe(llvm::StringRef name, m_write_fd = _open_osfhandle((intptr_t)m_write, _O_WRONLY); ZeroMemory(_write_overlapped, sizeof(m_write_overlapped)); +m_write_overlapped.hEvent = ::CreateEventA(nullptr, TRUE, FALSE, nullptr); } return Status(); @@ -228,6 +233,8 @@ int PipeWindows::ReleaseWriteFileDescriptor() { return PipeWindows::kInvalidDescriptor; int result = m_write_fd; m_write_fd = PipeWindows::kInvalidDescriptor; + if (m_write_overlapped.hEvent) +::CloseHandle(m_write_overlapped.hEvent); m_write = INVALID_HANDLE_VALUE; ZeroMemory(_write_overlapped, sizeof(m_write_overlapped)); return result; @@ -250,6 +257,9 @@ void PipeWindows::CloseWriteFileDescriptor() { if (!CanWrite()) return; + if (m_write_overlapped.hEvent) +::CloseHandle(m_write_overlapped.hEvent); + _close(m_write_fd); m_write = INVALID_HANDLE_VALUE; m_write_fd = PipeWindows::kInvalidDescriptor; @@ -283,7 +293,12 @@ Status PipeWindows::ReadWithTimeout(void *buf, size_t size, DWORD sys_bytes_read = size; BOOL result = ::ReadFile(m_read, buf, sys_bytes_read, _bytes_read, _read_overlapped); - if (!result && GetLastError() != ERROR_IO_PENDING) + if (result) { +bytes_read = sys_bytes_read; +return Status(); + } + + if (GetLastError() != ERROR_IO_PENDING) return Status(::GetLastError(), eErrorTypeWin32); DWORD timeout = (duration == std::chrono::microseconds::zero()) @@ -319,18 +334,56 @@ Status PipeWindows::ReadWithTimeout(void *buf, size_t size, Status PipeWindows::Write(const void *buf, size_t num_bytes, size_t _written) { + return WriteWithTimeout(buf, num_bytes, std::chrono::microseconds::zero(), + bytes_written); +} + +Status PipeWindows::WriteWithTimeout(const void *buf, size_t size, + const std::chrono::microseconds , + size_t _written) { if (!CanWrite()) return Status(ERROR_INVALID_HANDLE, eErrorTypeWin32); -
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
https://github.com/slydiman edited https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed lldb-server crash (TestLogHandler was not thread safe) (PR #101326)
https://github.com/slydiman created https://github.com/llvm/llvm-project/pull/101326 Host::LaunchProcess() requires to SetMonitorProcessCallback. This callback is called from the child process monitor thread. We cannot control this thread anyway. lldb-server may crash if there is a logging around this callback because TestLogHandler is not thread safe. I faced this issue debugging 100 simultaneous child processes. Note StreamLogHandler::Emit() in lldb/source/Utility/Log.cpp already contains the similar mutex. >From 113340676cfd90867a272db0ba78f1385f00cb4c Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Wed, 31 Jul 2024 16:33:13 +0400 Subject: [PATCH] [lldb] Fixed lldb-server crash (TestLogHandler was not thread safe) Host::LaunchProcess() requires to SetMonitorProcessCallback. This callback is called from the child process monitor thread. We cannot control this thread anyway. lldb-server may crash if there is a logging around this callback because TestLogHandler is not thread safe. I faced this issue debugging 100 simultaneous child processes. Note StreamLogHandler::Emit() in lldb/source/Utility/Log.cpp already contains the similar mutex. --- lldb/tools/lldb-server/LLDBServerUtilities.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lldb/tools/lldb-server/LLDBServerUtilities.cpp b/lldb/tools/lldb-server/LLDBServerUtilities.cpp index c3a8df19e969e..5facfbf3105e9 100644 --- a/lldb/tools/lldb-server/LLDBServerUtilities.cpp +++ b/lldb/tools/lldb-server/LLDBServerUtilities.cpp @@ -27,11 +27,13 @@ class TestLogHandler : public LogHandler { : m_stream_sp(stream_sp) {} void Emit(llvm::StringRef message) override { +std::lock_guard guard(m_mutex); (*m_stream_sp) << message; m_stream_sp->flush(); } private: + std::mutex m_mutex; std::shared_ptr m_stream_sp; }; ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
https://github.com/slydiman edited https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
slydiman wrote: @labath > For consistency, and to minimize the number of ifdefs, I believe windows and > non-windows paths should use as similar approaches as possible. That means I > think the end state should be that the posix path also uses a fork+exec > approach. fork+exec on non-Windows OS is redundant. It simply does not make sence. We can use multithreading or CreateProcess on Windows. It seems [the multithreading solution](https://github.com/llvm/llvm-project/pull/100670) faced a lot of issues and system's bugs on Linux. So CreateProcess is the only way. But it is the fork() workaround for Windows. > For me, it also seems natural that this should be the first step. While doing > that, we can make sure to create abstractions that can be used to implement > the windows behavior as well. I'm imagining some sort of a function or a > class, which takes the target binary, its arguments, and a socket, and > launches that binary with that socket. (we'll probably also need a similar > function/class on the other end to receive that socket). Look at GDBRemoteCommunication::StartDebugserverProcess(). I planed to extend it to use `pass_comm_fd` of Windows in the part 2. It is hard to create abstractions because of a lot of parameters. We can move most of SpawnProcessChild() code to `ConnectionFileDescriptor::ConnectFD` in ConnectionFileDescriptorPosix.cpp BTW, I don't like #ifdef _WIN32 in *Posix.cpp files. But currently the common ConnectionFileDescriptor.h contains `#include "lldb/Host/posix/ConnectionFileDescriptorPosix.h"` > I'm not sure if we really need the WriteWithTimeout functionality. The reason > we don't have it so far is that pipes have a buffer, and unless we're writing > a huge chunk of data, the write will never block anyway. There's nothing > wrong with it in principle though, and if you do want to have it, it'd be > best if it comes with in its own patch and with a test.) Before this patch the pipe read/write 1 byte in ConnectionFileDescriptor::CommandPipe. The maximal size was 6 bytes (the port value) in GDBRemoteCommunication::StartDebugserverProcess() and it is only the place where ReadWithTimeout() is used. But WSAPROTOCOL_INFO is big enough (628 bytes). Usually the pipe buffer is 4K. But we need an ability to avoid hangs anyway. I agree that all pipe related changes may be moved to another patch. And we need to add WriteWithTimeout() to PipePosix too. Note there is not any test for Pipe now. And I have no idea how to test it, especially async version. Running `lldb-server platform --server` on Windows is the best test. It must be tested with a high load with multiple connections. Otherwise any pipe tests are useles. https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
slydiman wrote: > What, if any, are the changes to how you would run lldb-server on Windows? I > see a new argument here but not sure if that's only meant for lldb-server to > pass to itself when it starts a child process. The explanation [is here](https://github.com/llvm/llvm-project/pull/101283#discussion_r1698289698). This new argument is the fork() workaround for Windows. We can keep this new argument for non-Windows OS, but it is redundant because fork() is enough. https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
@@ -283,54 +293,94 @@ Status PipeWindows::ReadWithTimeout(void *buf, size_t size, DWORD sys_bytes_read = size; BOOL result = ::ReadFile(m_read, buf, sys_bytes_read, _bytes_read, _read_overlapped); - if (!result && GetLastError() != ERROR_IO_PENDING) -return Status(::GetLastError(), eErrorTypeWin32); - - DWORD timeout = (duration == std::chrono::microseconds::zero()) - ? INFINITE - : duration.count() * 1000; - DWORD wait_result = ::WaitForSingleObject(m_read_overlapped.hEvent, timeout); - if (wait_result != WAIT_OBJECT_0) { -// The operation probably failed. However, if it timed out, we need to -// cancel the I/O. Between the time we returned from WaitForSingleObject -// and the time we call CancelIoEx, the operation may complete. If that -// hapens, CancelIoEx will fail and return ERROR_NOT_FOUND. If that -// happens, the original operation should be considered to have been -// successful. -bool failed = true; -DWORD failure_error = ::GetLastError(); -if (wait_result == WAIT_TIMEOUT) { - BOOL cancel_result = CancelIoEx(m_read, _read_overlapped); - if (!cancel_result && GetLastError() == ERROR_NOT_FOUND) -failed = false; + if (!result) { slydiman wrote: Thanks. Updated. https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
@@ -283,54 +293,94 @@ Status PipeWindows::ReadWithTimeout(void *buf, size_t size, DWORD sys_bytes_read = size; BOOL result = ::ReadFile(m_read, buf, sys_bytes_read, _bytes_read, _read_overlapped); - if (!result && GetLastError() != ERROR_IO_PENDING) -return Status(::GetLastError(), eErrorTypeWin32); - - DWORD timeout = (duration == std::chrono::microseconds::zero()) - ? INFINITE - : duration.count() * 1000; - DWORD wait_result = ::WaitForSingleObject(m_read_overlapped.hEvent, timeout); - if (wait_result != WAIT_OBJECT_0) { -// The operation probably failed. However, if it timed out, we need to -// cancel the I/O. Between the time we returned from WaitForSingleObject -// and the time we call CancelIoEx, the operation may complete. If that -// hapens, CancelIoEx will fail and return ERROR_NOT_FOUND. If that -// happens, the original operation should be considered to have been -// successful. -bool failed = true; -DWORD failure_error = ::GetLastError(); -if (wait_result == WAIT_TIMEOUT) { - BOOL cancel_result = CancelIoEx(m_read, _read_overlapped); - if (!cancel_result && GetLastError() == ERROR_NOT_FOUND) -failed = false; + if (!result) { +if (GetLastError() != ERROR_IO_PENDING) + return Status(::GetLastError(), eErrorTypeWin32); +else { slydiman wrote: Thanks. Updated. https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
https://github.com/slydiman updated https://github.com/llvm/llvm-project/pull/101283 >From 526708c8ac09275049c9afc8e7673fc5f3d889ed Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Wed, 31 Jul 2024 05:41:21 +0400 Subject: [PATCH] [lldb] Updated lldb-server to spawn the child process and share socket on Windows `lldb-server platform --server` works on Windows now w/o multithreading. The rest functionality remains unchanged. Added also PipeWindows::WriteWithTimeout(), fixed PipeWindows::ReadWithTimeout() and missing initialization of m_read_overlapped.hEvent in the constructor PipeWindows(lldb::pipe_t read, lldb::pipe_t write). Fixes #90923, fixes #56346. This is the part 1 of the replacement of #100670. In the part 2 I plan to switch `lldb-server gdbserver` to use `--fd` and listen a common gdb port for all gdbserver connections. Then we can remove gdb port mapping to fix #97537. --- lldb/include/lldb/Host/windows/PipeWindows.h | 3 + lldb/source/Host/windows/PipeWindows.cpp | 69 +++- .../GDBRemoteCommunicationServerPlatform.cpp | 34 ++ .../GDBRemoteCommunicationServerPlatform.h| 2 +- .../tools/lldb-server/LLDBServerUtilities.cpp | 2 + lldb/tools/lldb-server/lldb-platform.cpp | 315 +++--- 6 files changed, 375 insertions(+), 50 deletions(-) diff --git a/lldb/include/lldb/Host/windows/PipeWindows.h b/lldb/include/lldb/Host/windows/PipeWindows.h index 4b5be28d7ae6c..c99cf52be46c1 100644 --- a/lldb/include/lldb/Host/windows/PipeWindows.h +++ b/lldb/include/lldb/Host/windows/PipeWindows.h @@ -61,6 +61,9 @@ class PipeWindows : public PipeBase { Status Delete(llvm::StringRef name) override; Status Write(const void *buf, size_t size, size_t _written) override; + Status WriteWithTimeout(const void *buf, size_t size, + const std::chrono::microseconds , + size_t _written); Status ReadWithTimeout(void *buf, size_t size, const std::chrono::microseconds , size_t _read) override; diff --git a/lldb/source/Host/windows/PipeWindows.cpp b/lldb/source/Host/windows/PipeWindows.cpp index c82c919607b5b..8f86c7e28046e 100644 --- a/lldb/source/Host/windows/PipeWindows.cpp +++ b/lldb/source/Host/windows/PipeWindows.cpp @@ -58,7 +58,10 @@ PipeWindows::PipeWindows(pipe_t read, pipe_t write) } ZeroMemory(_read_overlapped, sizeof(m_read_overlapped)); + m_read_overlapped.hEvent = ::CreateEventA(nullptr, TRUE, FALSE, nullptr); + ZeroMemory(_write_overlapped, sizeof(m_write_overlapped)); + m_write_overlapped.hEvent = ::CreateEventA(nullptr, TRUE, FALSE, nullptr); } PipeWindows::~PipeWindows() { Close(); } @@ -77,6 +80,7 @@ Status PipeWindows::CreateNew(bool child_process_inherit) { m_write_fd = _open_osfhandle((intptr_t)m_write, _O_WRONLY); ZeroMemory(_write_overlapped, sizeof(m_write_overlapped)); + m_write_overlapped.hEvent = ::CreateEventA(nullptr, TRUE, FALSE, nullptr); return Status(); } @@ -202,6 +206,7 @@ Status PipeWindows::OpenNamedPipe(llvm::StringRef name, m_write_fd = _open_osfhandle((intptr_t)m_write, _O_WRONLY); ZeroMemory(_write_overlapped, sizeof(m_write_overlapped)); +m_write_overlapped.hEvent = ::CreateEventA(nullptr, TRUE, FALSE, nullptr); } return Status(); @@ -228,6 +233,8 @@ int PipeWindows::ReleaseWriteFileDescriptor() { return PipeWindows::kInvalidDescriptor; int result = m_write_fd; m_write_fd = PipeWindows::kInvalidDescriptor; + if (m_write_overlapped.hEvent) +::CloseHandle(m_write_overlapped.hEvent); m_write = INVALID_HANDLE_VALUE; ZeroMemory(_write_overlapped, sizeof(m_write_overlapped)); return result; @@ -250,6 +257,9 @@ void PipeWindows::CloseWriteFileDescriptor() { if (!CanWrite()) return; + if (m_write_overlapped.hEvent) +::CloseHandle(m_write_overlapped.hEvent); + _close(m_write_fd); m_write = INVALID_HANDLE_VALUE; m_write_fd = PipeWindows::kInvalidDescriptor; @@ -283,7 +293,12 @@ Status PipeWindows::ReadWithTimeout(void *buf, size_t size, DWORD sys_bytes_read = size; BOOL result = ::ReadFile(m_read, buf, sys_bytes_read, _bytes_read, _read_overlapped); - if (!result && GetLastError() != ERROR_IO_PENDING) + if (result) { +bytes_read = sys_bytes_read; +return Status(); + } + + if (GetLastError() != ERROR_IO_PENDING) return Status(::GetLastError(), eErrorTypeWin32); DWORD timeout = (duration == std::chrono::microseconds::zero()) @@ -319,18 +334,56 @@ Status PipeWindows::ReadWithTimeout(void *buf, size_t size, Status PipeWindows::Write(const void *buf, size_t num_bytes, size_t _written) { + return WriteWithTimeout(buf, num_bytes, std::chrono::microseconds::zero(), + bytes_written); +} + +Status PipeWindows::WriteWithTimeout(const void *buf, size_t size, + const
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
@@ -159,6 +159,40 @@ GDBRemoteCommunicationServerPlatform::GDBRemoteCommunicationServerPlatform( GDBRemoteCommunicationServerPlatform::~GDBRemoteCommunicationServerPlatform() = default; +void GDBRemoteCommunicationServerPlatform::Proc( +const lldb_private::Args ) { + if (!IsConnected()) +return; + + if (args.GetArgumentCount() > 0) { +lldb::pid_t pid = LLDB_INVALID_PROCESS_ID; +std::optional port; +std::string socket_name; +Status error = LaunchGDBServer(args, + "", // hostname + pid, port, socket_name); +if (error.Success()) + SetPendingGdbServer(pid, *port, socket_name); slydiman wrote: This code is just moved from lldb-platform.cpp GetPacketAndSendResponse() ignores `error` value. GetPacketAndSendResponse() must set the flag `quit` or `interrupt` and update `error`. https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
@@ -159,6 +159,40 @@ GDBRemoteCommunicationServerPlatform::GDBRemoteCommunicationServerPlatform( GDBRemoteCommunicationServerPlatform::~GDBRemoteCommunicationServerPlatform() = default; +void GDBRemoteCommunicationServerPlatform::Proc( +const lldb_private::Args ) { + if (!IsConnected()) +return; + + if (args.GetArgumentCount() > 0) { +lldb::pid_t pid = LLDB_INVALID_PROCESS_ID; +std::optional port; +std::string socket_name; +Status error = LaunchGDBServer(args, + "", // hostname + pid, port, socket_name); +if (error.Success()) + SetPendingGdbServer(pid, *port, socket_name); + } + + bool interrupt = false; + bool done = false; + Status error; + while (!interrupt && !done) { +if (GetPacketAndSendResponse(std::nullopt, error, interrupt, done) != +GDBRemoteCommunication::PacketResult::Success) + break; + } + + if (error.Fail()) { +Log *log = GetLog(LLDBLog::Platform); +LLDB_LOGF(log, + "GDBRemoteCommunicationServerPlatform::%s() " + "GetPacketAndSendResponse: %s", + __FUNCTION__, error.AsCString()); slydiman wrote: Look at other LLDB_LOGF() usage in GDBRemoteCommunicationServerPlatform.cpp I just used the similar code and format. This will print `GDBRemoteCommunicationServerPlatform::Proc() GetPacketAndSendResponse: lost connection` to the log on Linux and `GDBRemoteCommunicationServerPlatform::lldb_private::process_gdb_remote::GDBRemoteCommunicationServerPlatform::Proc() GetPacketAndSendResponse: lost connection` on Windows. https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
@@ -159,6 +159,40 @@ GDBRemoteCommunicationServerPlatform::GDBRemoteCommunicationServerPlatform( GDBRemoteCommunicationServerPlatform::~GDBRemoteCommunicationServerPlatform() = default; +void GDBRemoteCommunicationServerPlatform::Proc( slydiman wrote: Note `void SetInferiorArguments(const lldb_private::Args );` is a missing function. It is a bug. I have just removed it and added Proc(). There is no any connection between these functions. I have just moved a common code from lldb-platform to GDBRemoteCommunicationServerPlatform::Proc(). We can rename it to ClientHandle() or such and even kept inside lldb-platform.cpp https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
@@ -27,11 +27,13 @@ class TestLogHandler : public LogHandler { : m_stream_sp(stream_sp) {} void Emit(llvm::StringRef message) override { +std::lock_guard guard(m_mutex); slydiman wrote: Probably I can separate this change to another patch. It is not related to the socket sharing. It is the fix of an old bug. Host::LaunchProcess() requires to SetMonitorProcessCallback. This callback is called from the child process monitor thread. We cannot control this thread. lldb-server may crash if there is a logging around this callback because TestLogHandler is not thread safe. I've kept this code here for now because I faced this issue only debugging this patch with 100 simultaneous child processes. It is hard to reproduce it other way. Note also [this comment](https://github.com/llvm/llvm-project/pull/101283#issuecomment-2259628620). https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
@@ -114,6 +120,218 @@ static Status save_socket_id_to_file(const std::string _id, return status; } +static GDBRemoteCommunicationServerPlatform::PortMap gdbserver_portmap; +static std::mutex gdbserver_portmap_mutex; + +#if defined(_WIN32) slydiman wrote: We can move this code to some windows specific directory. But it is windows specific anyway. Note it contains a lot of `lldb-server platform` specific parameters. We can just pass the socket FD via the command line on non-Windows OS. `lldb-server gdbserver` already supports it with the parameter `--fd`. But it does not work on Windows. acceprt_fd here is a pipe FD which is used to transfer the structure WSAPROTOCOL_INFO. We need to know the child pid to initialize WSAPROTOCOL_INFO. Finally the child process is able to initialize the accepted socket FROM_PROTOCOL_INFO. https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
slydiman wrote: > What exactly does `lldb-server --server` mean? Sorry. I have updated the description with `lldb-server platform --server`. https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
https://github.com/slydiman edited https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
https://github.com/slydiman edited https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
slydiman wrote: Note GDBRemoteCommunicationServerPlatform::DebugserverProcessReaped() may cause a crash if this callback will be called from the child monitor thread after destroing the instance of GDBRemoteCommunicationServerPlatform. It is impossible to control this thread anyway. Hope I will fix it in the part 2 removing m_port_map. m_spawned_pids may be static since we have one GDBRemoteCommunicationServerPlatform instance per process. https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
https://github.com/slydiman updated https://github.com/llvm/llvm-project/pull/101283 >From 396b9a0595c04a8ec2e01c06788ddf9498ee3c4c Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Wed, 31 Jul 2024 05:41:21 +0400 Subject: [PATCH] [lldb] Updated lldb-server to spawn the child process and share socket on Windows `lldb-server --server` works on Windows now w/o multithreading. The rest functionality remains unchanged. Added also PipeWindows::WriteWithTimeout(), fixed PipeWindows::ReadWithTimeout() and missing initialization of m_read_overlapped.hEvent in the constructor PipeWindows(lldb::pipe_t read, lldb::pipe_t write). Fixes #90923, fixes #56346. This is the part 1 of the replacement of #100670. In the part 2 I plan to switch `lldb-server gdbserver` to use --fd and listen a common gdb port for all gdbserver connections. Then we can remove gdb port mapping to fix #97537. --- lldb/include/lldb/Host/windows/PipeWindows.h | 3 + lldb/source/Host/windows/PipeWindows.cpp | 128 --- .../GDBRemoteCommunicationServerPlatform.cpp | 34 ++ .../GDBRemoteCommunicationServerPlatform.h| 2 +- .../tools/lldb-server/LLDBServerUtilities.cpp | 2 + lldb/tools/lldb-server/lldb-platform.cpp | 316 +++--- 6 files changed, 404 insertions(+), 81 deletions(-) diff --git a/lldb/include/lldb/Host/windows/PipeWindows.h b/lldb/include/lldb/Host/windows/PipeWindows.h index 4b5be28d7ae6c..c99cf52be46c1 100644 --- a/lldb/include/lldb/Host/windows/PipeWindows.h +++ b/lldb/include/lldb/Host/windows/PipeWindows.h @@ -61,6 +61,9 @@ class PipeWindows : public PipeBase { Status Delete(llvm::StringRef name) override; Status Write(const void *buf, size_t size, size_t _written) override; + Status WriteWithTimeout(const void *buf, size_t size, + const std::chrono::microseconds , + size_t _written); Status ReadWithTimeout(void *buf, size_t size, const std::chrono::microseconds , size_t _read) override; diff --git a/lldb/source/Host/windows/PipeWindows.cpp b/lldb/source/Host/windows/PipeWindows.cpp index c82c919607b5b..318d5abbed74d 100644 --- a/lldb/source/Host/windows/PipeWindows.cpp +++ b/lldb/source/Host/windows/PipeWindows.cpp @@ -58,7 +58,10 @@ PipeWindows::PipeWindows(pipe_t read, pipe_t write) } ZeroMemory(_read_overlapped, sizeof(m_read_overlapped)); + m_read_overlapped.hEvent = ::CreateEventA(nullptr, TRUE, FALSE, nullptr); + ZeroMemory(_write_overlapped, sizeof(m_write_overlapped)); + m_write_overlapped.hEvent = ::CreateEventA(nullptr, TRUE, FALSE, nullptr); } PipeWindows::~PipeWindows() { Close(); } @@ -77,6 +80,7 @@ Status PipeWindows::CreateNew(bool child_process_inherit) { m_write_fd = _open_osfhandle((intptr_t)m_write, _O_WRONLY); ZeroMemory(_write_overlapped, sizeof(m_write_overlapped)); + m_write_overlapped.hEvent = ::CreateEventA(nullptr, TRUE, FALSE, nullptr); return Status(); } @@ -202,6 +206,7 @@ Status PipeWindows::OpenNamedPipe(llvm::StringRef name, m_write_fd = _open_osfhandle((intptr_t)m_write, _O_WRONLY); ZeroMemory(_write_overlapped, sizeof(m_write_overlapped)); +m_write_overlapped.hEvent = ::CreateEventA(nullptr, TRUE, FALSE, nullptr); } return Status(); @@ -228,6 +233,8 @@ int PipeWindows::ReleaseWriteFileDescriptor() { return PipeWindows::kInvalidDescriptor; int result = m_write_fd; m_write_fd = PipeWindows::kInvalidDescriptor; + if (m_write_overlapped.hEvent) +::CloseHandle(m_write_overlapped.hEvent); m_write = INVALID_HANDLE_VALUE; ZeroMemory(_write_overlapped, sizeof(m_write_overlapped)); return result; @@ -250,6 +257,9 @@ void PipeWindows::CloseWriteFileDescriptor() { if (!CanWrite()) return; + if (m_write_overlapped.hEvent) +::CloseHandle(m_write_overlapped.hEvent); + _close(m_write_fd); m_write = INVALID_HANDLE_VALUE; m_write_fd = PipeWindows::kInvalidDescriptor; @@ -283,54 +293,94 @@ Status PipeWindows::ReadWithTimeout(void *buf, size_t size, DWORD sys_bytes_read = size; BOOL result = ::ReadFile(m_read, buf, sys_bytes_read, _bytes_read, _read_overlapped); - if (!result && GetLastError() != ERROR_IO_PENDING) -return Status(::GetLastError(), eErrorTypeWin32); - - DWORD timeout = (duration == std::chrono::microseconds::zero()) - ? INFINITE - : duration.count() * 1000; - DWORD wait_result = ::WaitForSingleObject(m_read_overlapped.hEvent, timeout); - if (wait_result != WAIT_OBJECT_0) { -// The operation probably failed. However, if it timed out, we need to -// cancel the I/O. Between the time we returned from WaitForSingleObject -// and the time we call CancelIoEx, the operation may complete. If that -// hapens, CancelIoEx will fail and return ERROR_NOT_FOUND. If that -// happens, the original operation should be considered to have
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
https://github.com/slydiman created https://github.com/llvm/llvm-project/pull/101283 `lldb-server --server` works on Windows now w/o multithreading. The rest functionality remains unchanged. Added also PipeWindows::WriteWithTimeout(), fixed PipeWindows::ReadWithTimeout() and missing initialization of m_read_overlapped.hEvent in the constructor PipeWindows(lldb::pipe_t read, lldb::pipe_t write). Fixes #90923, fixes #56346. This is the part 1 of the replacement of #100670. In the part 2 I plan to switch `lldb-server gdbserver` to use `--fd` and listen a common gdb port for all gdbserver connections. Then we can remove gdb port mapping to fix #97537. >From 28ee2d294000852cc8b0a40db55b0619e9d257c9 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Wed, 31 Jul 2024 05:41:21 +0400 Subject: [PATCH] [lldb] Updated lldb-server to spawn the child process and share socket on Windows `lldb-server --server` works on Windows now w/o multithreading. The rest functionality remains unchanged. Added also PipeWindows::WriteWithTimeout(), fixed PipeWindows::ReadWithTimeout() and missing initialization of m_read_overlapped.hEvent in the constructor PipeWindows(lldb::pipe_t read, lldb::pipe_t write). Fixes #90923, fixes #56346. This is the part 1 of the replacement of #100670. In the part 2 I plan to switch `lldb-server gdbserver` to use --fd and listen a common gdb port for all gdbserver connections. Then we can remove gdb port mapping to fix #97537. --- lldb/include/lldb/Host/windows/PipeWindows.h | 3 + lldb/source/Host/windows/PipeWindows.cpp | 128 --- .../GDBRemoteCommunicationServerPlatform.cpp | 34 ++ .../GDBRemoteCommunicationServerPlatform.h| 2 +- .../tools/lldb-server/LLDBServerUtilities.cpp | 2 + lldb/tools/lldb-server/lldb-platform.cpp | 316 +++--- 6 files changed, 404 insertions(+), 81 deletions(-) diff --git a/lldb/include/lldb/Host/windows/PipeWindows.h b/lldb/include/lldb/Host/windows/PipeWindows.h index 4b5be28d7ae6c..c99cf52be46c1 100644 --- a/lldb/include/lldb/Host/windows/PipeWindows.h +++ b/lldb/include/lldb/Host/windows/PipeWindows.h @@ -61,6 +61,9 @@ class PipeWindows : public PipeBase { Status Delete(llvm::StringRef name) override; Status Write(const void *buf, size_t size, size_t _written) override; + Status WriteWithTimeout(const void *buf, size_t size, + const std::chrono::microseconds , + size_t _written); Status ReadWithTimeout(void *buf, size_t size, const std::chrono::microseconds , size_t _read) override; diff --git a/lldb/source/Host/windows/PipeWindows.cpp b/lldb/source/Host/windows/PipeWindows.cpp index c82c919607b5b..318d5abbed74d 100644 --- a/lldb/source/Host/windows/PipeWindows.cpp +++ b/lldb/source/Host/windows/PipeWindows.cpp @@ -58,7 +58,10 @@ PipeWindows::PipeWindows(pipe_t read, pipe_t write) } ZeroMemory(_read_overlapped, sizeof(m_read_overlapped)); + m_read_overlapped.hEvent = ::CreateEventA(nullptr, TRUE, FALSE, nullptr); + ZeroMemory(_write_overlapped, sizeof(m_write_overlapped)); + m_write_overlapped.hEvent = ::CreateEventA(nullptr, TRUE, FALSE, nullptr); } PipeWindows::~PipeWindows() { Close(); } @@ -77,6 +80,7 @@ Status PipeWindows::CreateNew(bool child_process_inherit) { m_write_fd = _open_osfhandle((intptr_t)m_write, _O_WRONLY); ZeroMemory(_write_overlapped, sizeof(m_write_overlapped)); + m_write_overlapped.hEvent = ::CreateEventA(nullptr, TRUE, FALSE, nullptr); return Status(); } @@ -202,6 +206,7 @@ Status PipeWindows::OpenNamedPipe(llvm::StringRef name, m_write_fd = _open_osfhandle((intptr_t)m_write, _O_WRONLY); ZeroMemory(_write_overlapped, sizeof(m_write_overlapped)); +m_write_overlapped.hEvent = ::CreateEventA(nullptr, TRUE, FALSE, nullptr); } return Status(); @@ -228,6 +233,8 @@ int PipeWindows::ReleaseWriteFileDescriptor() { return PipeWindows::kInvalidDescriptor; int result = m_write_fd; m_write_fd = PipeWindows::kInvalidDescriptor; + if (m_write_overlapped.hEvent) +::CloseHandle(m_write_overlapped.hEvent); m_write = INVALID_HANDLE_VALUE; ZeroMemory(_write_overlapped, sizeof(m_write_overlapped)); return result; @@ -250,6 +257,9 @@ void PipeWindows::CloseWriteFileDescriptor() { if (!CanWrite()) return; + if (m_write_overlapped.hEvent) +::CloseHandle(m_write_overlapped.hEvent); + _close(m_write_fd); m_write = INVALID_HANDLE_VALUE; m_write_fd = PipeWindows::kInvalidDescriptor; @@ -283,54 +293,94 @@ Status PipeWindows::ReadWithTimeout(void *buf, size_t size, DWORD sys_bytes_read = size; BOOL result = ::ReadFile(m_read, buf, sys_bytes_read, _bytes_read, _read_overlapped); - if (!result && GetLastError() != ERROR_IO_PENDING) -return Status(::GetLastError(), eErrorTypeWin32); - - DWORD timeout = (duration == std::chrono::microseconds::zero())
[Lldb-commits] [lldb] [lldb] Multithreading lldb-server works on Windows now; fixed gdb port mapping (PR #100670)
slydiman wrote: @labath It seems we do not need qSupported and qUpgradeToGdbConnection. We can run `lldb-server platform --server --listen 1234 --gdbserver-port 1235` Option 1: On receiving qLaunchGDBServer we can - fork the child process to know the new pid - send the response with the pid and port 1235 - wait for the connection on the port 1235 for 10 seconds - kill the child process if no connection accepted in 10 seconds - execve `lldb-server gdbserver --fd x` where x is fd of the accepted connection We need a pipe for communication between the main `lldb-server platform` process and the forked child process within 10 secconds. But unfortunately this scenario is impossible on Windows because of fork() is missing. Option 2: It seems PlatformRemoteGDBServer uses `debugserver_pid` only to kill the gdbserver process if connection failed. It seems we can just do nothing and respond `pid:0;port:1235;` to `qLaunchGDBServer`. Then listen the port 1235 and launch `lldb-server gdbserver --fd x` when a connection is accepted. `lldb-server gdbserver` must exit if the connection x is closed. https://github.com/llvm/llvm-project/pull/100670 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Improved lldb-server stability for remote launching (PR #100659)
https://github.com/slydiman closed https://github.com/llvm/llvm-project/pull/100659 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Improved lldb-server stability for remote launching (PR #100659)
slydiman wrote: I have moved this patch to #100670. https://github.com/llvm/llvm-project/pull/100659 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Multithreading lldb-server works on Windows now; fixed gdb port mapping (PR #100670)
https://github.com/slydiman updated https://github.com/llvm/llvm-project/pull/100670 >From 0870140c8b8d465570a696b35b59fbf55610919a Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Thu, 25 Jul 2024 00:34:34 +0400 Subject: [PATCH 1/2] [lldb] Multithreading lldb-server works on Windows now; fixed gdb port mapping Removed fork(). Used threads and the common thread-safe port map for all platform connections. Updated lldb::FileSystem to use llvm::vfs::createPhysicalFileSystem() with an own virtual working directory per thread. This patch depends on #100659, #100666. This patch fixes #97537, #90923, #56346. lldb-server has been tested on Windows with 50 connections and 100 processes launched simultaneously. Tested also the cross build with Linux x86_64 host and Linux Aarch64 target. --- lldb/include/lldb/Host/FileSystem.h | 7 + lldb/source/Host/common/FileSystem.cpp| 8 + lldb/source/Host/posix/PipePosix.cpp | 12 + .../GDBRemoteCommunicationServerCommon.cpp| 15 +- .../GDBRemoteCommunicationServerPlatform.cpp | 309 -- .../GDBRemoteCommunicationServerPlatform.h| 31 +- .../tools/lldb-server/LLDBServerUtilities.cpp | 2 + lldb/tools/lldb-server/lldb-platform.cpp | 99 ++ 8 files changed, 300 insertions(+), 183 deletions(-) diff --git a/lldb/include/lldb/Host/FileSystem.h b/lldb/include/lldb/Host/FileSystem.h index 640f3846e448c..5e25414a894d3 100644 --- a/lldb/include/lldb/Host/FileSystem.h +++ b/lldb/include/lldb/Host/FileSystem.h @@ -47,6 +47,12 @@ class FileSystem { static FileSystem (); + static void InitializePerThread() { +lldbassert(!InstancePerThread() && "Already initialized."); + InstancePerThread().emplace(llvm::IntrusiveRefCntPtr( +llvm::vfs::createPhysicalFileSystem().release())); + } + template static void Initialize(T &&...t) { lldbassert(!InstanceImpl() && "Already initialized."); InstanceImpl().emplace(std::forward(t)...); @@ -206,6 +212,7 @@ class FileSystem { private: static std::optional (); + static std::optional (); llvm::IntrusiveRefCntPtr m_fs; std::unique_ptr m_tilde_resolver; std::string m_home_directory; diff --git a/lldb/source/Host/common/FileSystem.cpp b/lldb/source/Host/common/FileSystem.cpp index 5153a0a9ec513..cb76086616d6b 100644 --- a/lldb/source/Host/common/FileSystem.cpp +++ b/lldb/source/Host/common/FileSystem.cpp @@ -49,7 +49,15 @@ void FileSystem::Terminate() { InstanceImpl().reset(); } +std::optional ::InstancePerThread() { + static thread_local std::optional t_fs; + return t_fs; +} + std::optional ::InstanceImpl() { + std::optional = InstancePerThread(); + if (fs) +return fs; static std::optional g_fs; return g_fs; } diff --git a/lldb/source/Host/posix/PipePosix.cpp b/lldb/source/Host/posix/PipePosix.cpp index f35c348990df6..1aa02efe86610 100644 --- a/lldb/source/Host/posix/PipePosix.cpp +++ b/lldb/source/Host/posix/PipePosix.cpp @@ -324,6 +324,18 @@ Status PipePosix::ReadWithTimeout(void *buf, size_t size, bytes_read += result; if (bytes_read == size || result == 0) break; + +// This is the workaround for the following bug in Linux multithreading +// select() https://bugzilla.kernel.org/show_bug.cgi?id=546 +// ReadWithTimeout() with a non-zero timeout is used only to +// read the port number from the gdbserver pipe +// in GDBRemoteCommunication::StartDebugserverProcess(). +// The port number may be "1024\0".."65535\0". +if (timeout.count() > 0 && size == 6 && bytes_read == 5 && +static_cast(buf)[4] == '\0') { + break; +} + } else if (errno == EINTR) { continue; } else { diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp index f9d37490e16ae..cef836e001adf 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp @@ -646,7 +646,9 @@ GDBRemoteCommunicationServerCommon::Handle_vFile_Size( packet.GetHexByteString(path); if (!path.empty()) { uint64_t Size; -if (llvm::sys::fs::file_size(path, Size)) +FileSpec file_spec(path); +FileSystem::Instance().Resolve(file_spec); +if (llvm::sys::fs::file_size(file_spec.GetPath(), Size)) return SendErrorResponse(5); StreamString response; response.PutChar('F'); @@ -725,7 +727,9 @@ GDBRemoteCommunicationServerCommon::Handle_vFile_unlink( packet.SetFilePos(::strlen("vFile:unlink:")); std::string path; packet.GetHexByteString(path); - Status error(llvm::sys::fs::remove(path)); + FileSpec file_spec(path); + FileSystem::Instance().Resolve(file_spec); + Status error(llvm::sys::fs::remove(file_spec.GetPath())); StreamString response;
[Lldb-commits] [lldb] [lldb] Improved lldb-server stability for remote launching (PR #100659)
slydiman wrote: > I'd like to hear your opinion on my port mapping alternative. https://github.com/llvm/llvm-project/pull/100670#issuecomment-2255991921 https://github.com/llvm/llvm-project/pull/100659 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Multithreading lldb-server works on Windows now; fixed gdb port mapping (PR #100670)
slydiman wrote: > The way this would work is by letting the platform instance > delegate/upgrate/convert the platform connection into a gdbserver one. The > way this would work would be something like this: > > 1. `lldb-server platform` would advertise (say in `qSupported`) its support > for this new mode. > 2. Before asking the platform to launch a new gdb server, lldb would query > this feature. If present, instead of the usual action (`qLaunchGDBServer`), > it would create _another_ platform connection, using the same port as the > original one. As we're using the same port, we'd go through all the nats just > like the original connection. > 3. On this new connection it would send a new special packet (let's call it > `qUpgradeToGdbConnection`) > 4. `lldb server platform` would launch an gdbserver instance and everything > else would proceed as before. > > On non-darwin platform (darwin uses `debugserver`) we could optimize to avoid > spawning a new process, and just call the relevant gdb-server code directly. But debugserver on darwin will not support this feauture. It will still require the port map or disabling firewall. https://github.com/llvm/llvm-project/pull/100670 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Improved lldb-server stability for remote launching (PR #100659)
slydiman wrote: > Ok, so if I'm reading this right you're saying you saw no ETXTBSY errors with > the current implementation --server flag. Is that correct ? Right. Initially I have marked #100670 as dependent on this. Ok, agreed. So, we can try to use O_CLOFORK. And simple solution is to call execve() as fast as possible and wait some time in case of ETXTBSY (this PR). https://github.com/llvm/llvm-project/pull/100659 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Improved lldb-server stability for remote launching (PR #100659)
slydiman wrote: See also https://github.com/golang/go/issues/22315 https://github.com/llvm/llvm-project/pull/100659 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Improved lldb-server stability for remote launching (PR #100659)
slydiman wrote: > that could mean that something like `$CC hello.cc && ./a.out` could fail > (what we're doing here isn't fundamentally different than that). The difference is that cc creates a.out and exits itself. But `lldb-server platform` is still running after creating the executable. Something must be flushed? https://github.com/llvm/llvm-project/pull/100659 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Improved lldb-server stability for remote launching (PR #100659)
slydiman wrote: > Does this refer the the forking implementation you get with the --server > flag, or the serial implementation which only handles one connection at a > time (without the flag)? I mean the `--server` flag. It is very hard to reproduce this issue with the serial implementation because it is much slower. > - thread 1 opens a file for writing (file descriptor A) and starts writing it > - thread 2 starts launching a gdb server. It calls fork(), which creates > another process with a copy of fd A (the fd has the CLOEXEC flag set, but not > CLOFORK (the flag doesn't exist)) > - thread 1 finishes writing, closes fd A (in its/parent process) > - thread 1launches gdb-server, gdb-server tries to launch the file it has > just written, gets ETXTBSY because the fd is still open in the child process > In this scenario, waiting does make the exec succeed, because the FD in the > forked process will not stay open very long. It will get closed as soon as > the process runs execve (due to CLOEXEC). Your scenario is impossible. How FD will be closed by execve() if execve() failed with ETXTBSY? - `lldb-server platform` (a `thread 1` or just a process) creates, writes and closes the file. The FD is closed and may be reused by the system. The client lldb received the response OK for `vFile:close` request. - The same `thread 1` launched `gdb server` (fork+execve). The FD of a created and closed file cannot be copied any way. - The client lldb connects to `gdb server` and send the request `vRun` - `gdb server` did not create or write the executable. It never had the FD of this file and did not inherit it. `gdb server` just fork a child process and try to call execve(), but it failed with ETXTBSY. > `$CC hello.cc && ./a.out` could fail May it work? How is it possible? https://github.com/llvm/llvm-project/pull/100659 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Multithreading lldb-server works on Windows now; fixed gdb port mapping (PR #100670)
slydiman wrote: > If this is only really used for a "only for few requests via the platform > protocol", then why not make the CWD a property of the platform object? > (Either through a virtual filesystem, or just by having it as a string, and > resolving things explicitly) It is possible to store an own FileSystem object in the platform handler, but it requires to update 80% of GDBRemoteCommunicationServerCommon.cpp and implement some behavior switch in inherited classes. I tried to minimize changes. I have added the new FileSystem::InitializePerThread() which is used only in GDBRemoteCommunicationServerPlatform and its base clases in case of multithreading. All other code uses the same FileSystem, nothing changed. FileSystem::InitializePerThread() uses the CWD of the app. So the behavior for the thread is the same as for a forked child process. I don't see any other threads where FileSystem is used. `lldb-server platform` creates only one additional thread to monitor a child process. But it does not use any file system operations. Anyway if FileSystem::InitializePerThread() was not called, any new thread uses the common app's FileSystem. It is safe. https://github.com/llvm/llvm-project/pull/100670 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Multithreading lldb-server works on Windows now; fixed gdb port mapping (PR #100670)
slydiman wrote: > One way I can imagine this happening is if the FileSystem instance was local > to a GDBRemoteCommunicationServerPlatform instance -- rather than the thread > it happens to be (mostly) running on. This will require more changes to, > basically, plumb the filesystem instance to every place that needs to be used > from the platform object, but I think that's actually a good thing. It will > give us a record of which code is prepared to deal with virtualized > filesystems and which one isn't. I just don't think we can assume that all > our code (present and future) will handle the per-thread filesystem situation > correctly. GDBRemoteCommunicationServerPlatform extends GDBRemoteCommunicationServerCommon, GDBRemoteCommunication, etc. The working directory may be used for own tasks (for example load/save settings) and to handle request with a relative path. Most such requests are handled in GDBRemoteCommunicationServerCommon. It seems everything works correctly since all tests passed. > (That said, it still may be better to just spawn a new process instead. I > don't think this is a particularly hot code path (I think the test suite is > basically the only user of multithreaded platforms), and it will make sure we > don't hit the ETXTBSY issue). How will the spawned process help? I think the only way to fix ETXTBSY issue is to copy the executable to the target and launch it from the same process. It seems MacOS uses system's gdbserver instead of `lldb-server gdbserver`. Please correct me if I'm wrong. So ETXTBSY issue is the design issue. Probably it is necessary to move vFile:open, vFile:pwrite, vFile:close to gdbserver somehow to fix ETXTBSY issue. https://github.com/llvm/llvm-project/pull/100670 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Multithreading lldb-server works on Windows now; fixed gdb port mapping (PR #100670)
https://github.com/slydiman edited https://github.com/llvm/llvm-project/pull/100670 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Multithreading lldb-server works on Windows now; fixed gdb port mapping (PR #100670)
https://github.com/slydiman edited https://github.com/llvm/llvm-project/pull/100670 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Multithreading lldb-server works on Windows now; fixed gdb port mapping (PR #100670)
https://github.com/slydiman edited https://github.com/llvm/llvm-project/pull/100670 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Improved lldb-server stability for remote launching (PR #100659)
slydiman wrote: Of course lldb waits for the vFile:close response before sending the vRun packet and lldb-server actually closes the file handle (all of them). No any leaks. Otherwise this workaround wouldn't work. The behavior is the same on Linux and Windows targets. I launched 100 connections and 200 processes simultaneously on Windows (lldb-server gdbserver + a test app). I got 3..10 fails because of the error ERROR_SHARING_VIOLATION. After this patch I got 0..3 fails for 100 connections and 0 fails for 50 connections. After closing the copied file probably the system may cache it some time per process. The file may be blocked by the built-in antivirus for some time. It is hard to figure out the exact reason. We have a buildbot to run cross API tests in 8 threads with Linux Aarch64 target. All tests are green with the current (single thread) lldb-server. But we got randomly failed 50..60 tests with the multithreading version of lldb-server. Probably it is just little bit faster and system did not unlock the executable. We noticed that usually failed tests use simple and tiny executables. But this fact does not help to explain the reason of the problem. We got 100% green tests after this patch. https://github.com/llvm/llvm-project/pull/100659 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Optimized lldb-server memory usage (PR #100666)
https://github.com/slydiman closed https://github.com/llvm/llvm-project/pull/100666 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Optimized lldb-server memory usage (PR #100666)
https://github.com/slydiman updated https://github.com/llvm/llvm-project/pull/100666 >From 0e451f16b91bab2bc2cd1375eb02e4fe71998790 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Fri, 26 Jul 2024 02:40:49 +0400 Subject: [PATCH 1/3] [lldb] Optimized lldb-server memory usage MAX_PATH is definitely larger than 6 bytes we are expecting for this message, and could be rather large depending on the target OS (4K for some Linux OSs). Since the buffer gets allocated on the stack we better be conservative and allocate what we actually need. --- .../Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp index 187370eb36cae..b961c0e42469a 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp @@ -1145,7 +1145,8 @@ Status GDBRemoteCommunication::StartDebugserverProcess( if (socket_pipe.CanWrite()) socket_pipe.CloseWriteFileDescriptor(); if (socket_pipe.CanRead()) { -char port_cstr[PATH_MAX] = {0}; +// The port number may be "1024\0".."65535\0". +char port_cstr[6] = {0}; port_cstr[0] = '\0'; size_t num_bytes = sizeof(port_cstr); // Read port from pipe with 10 second timeout. >From 4562af94a7417575fec100e5bbe745b21870e6fc Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Fri, 26 Jul 2024 11:55:08 +0400 Subject: [PATCH 2/3] Removed the redundant line. --- .../source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp index b961c0e42469a..07aa8209ca4ba 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp @@ -1147,7 +1147,6 @@ Status GDBRemoteCommunication::StartDebugserverProcess( if (socket_pipe.CanRead()) { // The port number may be "1024\0".."65535\0". char port_cstr[6] = {0}; -port_cstr[0] = '\0'; size_t num_bytes = sizeof(port_cstr); // Read port from pipe with 10 second timeout. error = socket_pipe.ReadWithTimeout( >From ac300300b7e5c7a0cf1882d0a81efc24e587125c Mon Sep 17 00:00:00 2001 From: Dmitry Vassiliev Date: Fri, 26 Jul 2024 18:53:20 +0400 Subject: [PATCH 3/3] Updated the comment. --- .../Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp index 07aa8209ca4ba..5d0a3e31d0437 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp @@ -1145,7 +1145,7 @@ Status GDBRemoteCommunication::StartDebugserverProcess( if (socket_pipe.CanWrite()) socket_pipe.CloseWriteFileDescriptor(); if (socket_pipe.CanRead()) { -// The port number may be "1024\0".."65535\0". +// The port number may be up to "65535\0". char port_cstr[6] = {0}; size_t num_bytes = sizeof(port_cstr); // Read port from pipe with 10 second timeout. ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Multithreading lldb-server works on Windows now; fixed gdb port mapping (PR #100670)
@@ -324,6 +324,18 @@ Status PipePosix::ReadWithTimeout(void *buf, size_t size, bytes_read += result; if (bytes_read == size || result == 0) break; + +// This is the workaround for the following bug in Linux multithreading +// select() https://bugzilla.kernel.org/show_bug.cgi?id=546 +// ReadWithTimeout() with a non-zero timeout is used only to +// read the port number from the gdbserver pipe +// in GDBRemoteCommunication::StartDebugserverProcess(). +// The port number may be "1024\0".."65535\0". +if (timeout.count() > 0 && size == 6 && bytes_read == 5 && +static_cast(buf)[4] == '\0') { + break; +} slydiman wrote: I don't think so. Note select() works fine with the pipe closed on other side in the single thread. select() hangs in case of simultaneously calls from few threads. I tried to connect to `lldb-server platform` and launch `lldb-server gdbserver` in few threads. It works 50/50 in case of 2 threads and 100% failed in case of 3+ threads. Instead of using select() I tried - use poll() - use read(size = 0) - use non blocked pipe and call read() w/o select() or poll() - change pipe buffer size Nothing helped. It is the bug in the kernel. read() will hang too if the pipe is closed on the other side. Non blocking read() will return EAGAIN instead of 0. The system just does not recognize the closed pipe in case of multithreading. So, I don't see a way to fix this on Linux. The only way is a workaround. https://github.com/llvm/llvm-project/pull/100670 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Multithreading lldb-server works on Windows now; fixed gdb port mapping (PR #100670)
slydiman wrote: > to replace the fork with spawning a new process (fork+execve) instead. Have > you considered that? To fix all gdb port mapping issues we need a common port mapping available to all platform connections. It is possible only with the multithreading. https://github.com/llvm/llvm-project/pull/100670 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Optimized lldb-server memory usage (PR #100666)
@@ -1145,7 +1145,8 @@ Status GDBRemoteCommunication::StartDebugserverProcess( if (socket_pipe.CanWrite()) socket_pipe.CloseWriteFileDescriptor(); if (socket_pipe.CanRead()) { -char port_cstr[PATH_MAX] = {0}; +// The port number may be "1024\0".."65535\0". +char port_cstr[6] = {0}; port_cstr[0] = '\0'; slydiman wrote: Updated. Thanks. https://github.com/llvm/llvm-project/pull/100666 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Optimized lldb-server memory usage (PR #100666)
https://github.com/slydiman updated https://github.com/llvm/llvm-project/pull/100666 >From 0e451f16b91bab2bc2cd1375eb02e4fe71998790 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Fri, 26 Jul 2024 02:40:49 +0400 Subject: [PATCH 1/2] [lldb] Optimized lldb-server memory usage MAX_PATH is definitely larger than 6 bytes we are expecting for this message, and could be rather large depending on the target OS (4K for some Linux OSs). Since the buffer gets allocated on the stack we better be conservative and allocate what we actually need. --- .../Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp index 187370eb36cae..b961c0e42469a 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp @@ -1145,7 +1145,8 @@ Status GDBRemoteCommunication::StartDebugserverProcess( if (socket_pipe.CanWrite()) socket_pipe.CloseWriteFileDescriptor(); if (socket_pipe.CanRead()) { -char port_cstr[PATH_MAX] = {0}; +// The port number may be "1024\0".."65535\0". +char port_cstr[6] = {0}; port_cstr[0] = '\0'; size_t num_bytes = sizeof(port_cstr); // Read port from pipe with 10 second timeout. >From 4562af94a7417575fec100e5bbe745b21870e6fc Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Fri, 26 Jul 2024 11:55:08 +0400 Subject: [PATCH 2/2] Removed the redundant line. --- .../source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp index b961c0e42469a..07aa8209ca4ba 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp @@ -1147,7 +1147,6 @@ Status GDBRemoteCommunication::StartDebugserverProcess( if (socket_pipe.CanRead()) { // The port number may be "1024\0".."65535\0". char port_cstr[6] = {0}; -port_cstr[0] = '\0'; size_t num_bytes = sizeof(port_cstr); // Read port from pipe with 10 second timeout. error = socket_pipe.ReadWithTimeout( ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Multithreading lldb-server works on Windows now; fixed gdb port mapping (PR #100670)
https://github.com/slydiman edited https://github.com/llvm/llvm-project/pull/100670 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Multithreading lldb-server works on Windows now; fixed gdb port mapping (PR #100670)
https://github.com/slydiman created https://github.com/llvm/llvm-project/pull/100670 Removed fork(). Used threads and the common thread-safe port map for all platform connections. Updated lldb::FileSystem to use llvm::vfs::createPhysicalFileSystem() with an own virtual working directory per thread. This patch depends on #100659, #100666. This patch fixes #97537, #90923, #56346. lldb-server has been tested on Windows with 50 connections and 100 processes launched simultaneously. Tested also the cross build with Linux x86_64 host and Linux Aarch64 target. >From 0870140c8b8d465570a696b35b59fbf55610919a Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Thu, 25 Jul 2024 00:34:34 +0400 Subject: [PATCH] [lldb] Multithreading lldb-server works on Windows now; fixed gdb port mapping Removed fork(). Used threads and the common thread-safe port map for all platform connections. Updated lldb::FileSystem to use llvm::vfs::createPhysicalFileSystem() with an own virtual working directory per thread. This patch depends on #100659, #100666. This patch fixes #97537, #90923, #56346. lldb-server has been tested on Windows with 50 connections and 100 processes launched simultaneously. Tested also the cross build with Linux x86_64 host and Linux Aarch64 target. --- lldb/include/lldb/Host/FileSystem.h | 7 + lldb/source/Host/common/FileSystem.cpp| 8 + lldb/source/Host/posix/PipePosix.cpp | 12 + .../GDBRemoteCommunicationServerCommon.cpp| 15 +- .../GDBRemoteCommunicationServerPlatform.cpp | 309 -- .../GDBRemoteCommunicationServerPlatform.h| 31 +- .../tools/lldb-server/LLDBServerUtilities.cpp | 2 + lldb/tools/lldb-server/lldb-platform.cpp | 99 ++ 8 files changed, 300 insertions(+), 183 deletions(-) diff --git a/lldb/include/lldb/Host/FileSystem.h b/lldb/include/lldb/Host/FileSystem.h index 640f3846e448c..5e25414a894d3 100644 --- a/lldb/include/lldb/Host/FileSystem.h +++ b/lldb/include/lldb/Host/FileSystem.h @@ -47,6 +47,12 @@ class FileSystem { static FileSystem (); + static void InitializePerThread() { +lldbassert(!InstancePerThread() && "Already initialized."); + InstancePerThread().emplace(llvm::IntrusiveRefCntPtr( +llvm::vfs::createPhysicalFileSystem().release())); + } + template static void Initialize(T &&...t) { lldbassert(!InstanceImpl() && "Already initialized."); InstanceImpl().emplace(std::forward(t)...); @@ -206,6 +212,7 @@ class FileSystem { private: static std::optional (); + static std::optional (); llvm::IntrusiveRefCntPtr m_fs; std::unique_ptr m_tilde_resolver; std::string m_home_directory; diff --git a/lldb/source/Host/common/FileSystem.cpp b/lldb/source/Host/common/FileSystem.cpp index 5153a0a9ec513..cb76086616d6b 100644 --- a/lldb/source/Host/common/FileSystem.cpp +++ b/lldb/source/Host/common/FileSystem.cpp @@ -49,7 +49,15 @@ void FileSystem::Terminate() { InstanceImpl().reset(); } +std::optional ::InstancePerThread() { + static thread_local std::optional t_fs; + return t_fs; +} + std::optional ::InstanceImpl() { + std::optional = InstancePerThread(); + if (fs) +return fs; static std::optional g_fs; return g_fs; } diff --git a/lldb/source/Host/posix/PipePosix.cpp b/lldb/source/Host/posix/PipePosix.cpp index f35c348990df6..1aa02efe86610 100644 --- a/lldb/source/Host/posix/PipePosix.cpp +++ b/lldb/source/Host/posix/PipePosix.cpp @@ -324,6 +324,18 @@ Status PipePosix::ReadWithTimeout(void *buf, size_t size, bytes_read += result; if (bytes_read == size || result == 0) break; + +// This is the workaround for the following bug in Linux multithreading +// select() https://bugzilla.kernel.org/show_bug.cgi?id=546 +// ReadWithTimeout() with a non-zero timeout is used only to +// read the port number from the gdbserver pipe +// in GDBRemoteCommunication::StartDebugserverProcess(). +// The port number may be "1024\0".."65535\0". +if (timeout.count() > 0 && size == 6 && bytes_read == 5 && +static_cast(buf)[4] == '\0') { + break; +} + } else if (errno == EINTR) { continue; } else { diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp index f9d37490e16ae..cef836e001adf 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp @@ -646,7 +646,9 @@ GDBRemoteCommunicationServerCommon::Handle_vFile_Size( packet.GetHexByteString(path); if (!path.empty()) { uint64_t Size; -if (llvm::sys::fs::file_size(path, Size)) +FileSpec file_spec(path); +FileSystem::Instance().Resolve(file_spec); +if (llvm::sys::fs::file_size(file_spec.GetPath(), Size))
[Lldb-commits] [lldb] [lldb] Improved lldb-server stability for remote launching (PR #100659)
@@ -201,7 +201,7 @@ struct ForkLaunchInfo { execve(info.argv[0], const_cast(info.argv), info.envp); #if defined(__linux__) - if (errno == ETXTBSY) { + for (int i = 0; i < 50; ++i) { slydiman wrote: I think it is redundant. Currently lldb-server contains a hardcoded timeout in many places. Note the total timeout 10 seconds will cause the error "Sending vRun packet failed". So 5 seconds is good enough for the stable connection and do the trick on slow machines. https://github.com/llvm/llvm-project/pull/100659 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Improved lldb-server stability for remote launching (PR #100659)
https://github.com/slydiman updated https://github.com/llvm/llvm-project/pull/100659 >From 21fd03faa1224d44bd132a0b53d66d896210a044 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Fri, 26 Jul 2024 01:48:35 +0400 Subject: [PATCH] [lldb] Improved lldb-server stability for remote launching We faced the issue running cross api tests in 8 threads. The executable is installed to the target by the process `lldb-server platform`, but launched by the another process `lldb-server gdbserver`. We got the error ETXTBSY on Linux and ERROR_SHARING_VIOLATION on Windows. It seems the known issue and ProcessLauncherPosixFork.cpp already contains the workaround, but it is not enough. Updated the workaround with the total timeout 5 seconds and added the same workaround to ProcessLauncherWindows.cpp too. --- .../Host/posix/ProcessLauncherPosixFork.cpp | 6 +++-- .../Host/windows/ProcessLauncherWindows.cpp | 26 +++ 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/lldb/source/Host/posix/ProcessLauncherPosixFork.cpp b/lldb/source/Host/posix/ProcessLauncherPosixFork.cpp index 0a832ebad13a7..637c2846e6bb2 100644 --- a/lldb/source/Host/posix/ProcessLauncherPosixFork.cpp +++ b/lldb/source/Host/posix/ProcessLauncherPosixFork.cpp @@ -201,7 +201,7 @@ struct ForkLaunchInfo { execve(info.argv[0], const_cast(info.argv), info.envp); #if defined(__linux__) - if (errno == ETXTBSY) { + for (int i = 0; i < 50; ++i) { // On android M and earlier we can get this error because the adb daemon // can hold a write handle on the executable even after it has finished // uploading it. This state lasts only a short time and happens only when @@ -210,7 +210,9 @@ struct ForkLaunchInfo { // shell" command in the fork() child before it has had a chance to exec.) // Since this state should clear up quickly, wait a while and then give it // one more go. -usleep(5); +if (errno != ETXTBSY) + break; +usleep(10); execve(info.argv[0], const_cast(info.argv), info.envp); } #endif diff --git a/lldb/source/Host/windows/ProcessLauncherWindows.cpp b/lldb/source/Host/windows/ProcessLauncherWindows.cpp index baa422c15cae2..ee5f8fda1d492 100644 --- a/lldb/source/Host/windows/ProcessLauncherWindows.cpp +++ b/lldb/source/Host/windows/ProcessLauncherWindows.cpp @@ -113,14 +113,30 @@ ProcessLauncherWindows::LaunchProcess(const ProcessLaunchInfo _info, // command line is not empty, its contents may be modified by CreateProcessW. WCHAR *pwcommandLine = wcommandLine.empty() ? nullptr : [0]; - BOOL result = ::CreateProcessW( - wexecutable.c_str(), pwcommandLine, NULL, NULL, TRUE, flags, env_block, - wworkingDirectory.size() == 0 ? NULL : wworkingDirectory.c_str(), - , ); + BOOL result; + DWORD last_error = 0; + // This is the workaround for the error "The process cannot access the file + // because it is being used by another process". Note the executable file is + // installed to the target by the process `lldb-server platform`, but launched + // by the process `lldb-server gdbserver`. Sometimes system may block the file + // for some time after copying. + for (int i = 0; i < 50; ++i) { +result = ::CreateProcessW( +wexecutable.c_str(), pwcommandLine, NULL, NULL, TRUE, flags, env_block, +wworkingDirectory.size() == 0 ? NULL : wworkingDirectory.c_str(), +, ); +if (!result) { + last_error = ::GetLastError(); + if (last_error != ERROR_SHARING_VIOLATION) +break; + ::Sleep(100); +} else + break; + } if (!result) { // Call GetLastError before we make any other system calls. -error.SetError(::GetLastError(), eErrorTypeWin32); +error.SetError(last_error, eErrorTypeWin32); // Note that error 50 ("The request is not supported") will occur if you // try debug a 64-bit inferior from a 32-bit LLDB. } ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Optimized lldb-server memory usage (PR #100666)
https://github.com/slydiman created https://github.com/llvm/llvm-project/pull/100666 MAX_PATH is definitely larger than 6 bytes we are expecting for this message, and could be rather large depending on the target OS (4K for some Linux OSs). Since the buffer gets allocated on the stack we better be conservative and allocate what we actually need. >From 0e451f16b91bab2bc2cd1375eb02e4fe71998790 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Fri, 26 Jul 2024 02:40:49 +0400 Subject: [PATCH] [lldb] Optimized lldb-server memory usage MAX_PATH is definitely larger than 6 bytes we are expecting for this message, and could be rather large depending on the target OS (4K for some Linux OSs). Since the buffer gets allocated on the stack we better be conservative and allocate what we actually need. --- .../Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp index 187370eb36cae..b961c0e42469a 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp @@ -1145,7 +1145,8 @@ Status GDBRemoteCommunication::StartDebugserverProcess( if (socket_pipe.CanWrite()) socket_pipe.CloseWriteFileDescriptor(); if (socket_pipe.CanRead()) { -char port_cstr[PATH_MAX] = {0}; +// The port number may be "1024\0".."65535\0". +char port_cstr[6] = {0}; port_cstr[0] = '\0'; size_t num_bytes = sizeof(port_cstr); // Read port from pipe with 10 second timeout. ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Improved lldb-server stability for remote launching (PR #100659)
https://github.com/slydiman created https://github.com/llvm/llvm-project/pull/100659 We faced the issue running cross api tests in 8 threads. The executable is installed to the target by the process `lldb-server platform`, but launched by the another process `lldb-server gdbserver`. We got the error ETXTBSY on Linux and ERROR_SHARING_VIOLATION on Windows. It seems the known issue and ProcessLauncherPosixFork.cpp already contains the workaround, but it is not enough. Updated the workaround with the total timeout 5 seconds and added the same workaround to ProcessLauncherWindows.cpp too. >From 6e24f75be4c779ed336a3894ac5312b0fc0e845c Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Fri, 26 Jul 2024 01:48:35 +0400 Subject: [PATCH] [lldb] Improved lldb-server stability for remote launching We faced the issue running cross api tests in 8 threads. The executable is installed to the target by the process `lldb-server platform`, but launched by the another process `lldb-server gdbserver`. We got the error ETXTBSY on Linux and ERROR_SHARING_VIOLATION on Windows. It seems the known issue and ProcessLauncherPosixFork.cpp already contains the workaround, but it is not enough. Updated the workaround with the total timeout 5 seconds and added the same workaround to ProcessLauncherWindows.cpp too. --- .../Host/posix/ProcessLauncherPosixFork.cpp | 6 -- .../Host/windows/ProcessLauncherWindows.cpp | 20 +-- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/lldb/source/Host/posix/ProcessLauncherPosixFork.cpp b/lldb/source/Host/posix/ProcessLauncherPosixFork.cpp index 0a832ebad13a7..637c2846e6bb2 100644 --- a/lldb/source/Host/posix/ProcessLauncherPosixFork.cpp +++ b/lldb/source/Host/posix/ProcessLauncherPosixFork.cpp @@ -201,7 +201,7 @@ struct ForkLaunchInfo { execve(info.argv[0], const_cast(info.argv), info.envp); #if defined(__linux__) - if (errno == ETXTBSY) { + for (int i = 0; i < 50; ++i) { // On android M and earlier we can get this error because the adb daemon // can hold a write handle on the executable even after it has finished // uploading it. This state lasts only a short time and happens only when @@ -210,7 +210,9 @@ struct ForkLaunchInfo { // shell" command in the fork() child before it has had a chance to exec.) // Since this state should clear up quickly, wait a while and then give it // one more go. -usleep(5); +if (errno != ETXTBSY) + break; +usleep(10); execve(info.argv[0], const_cast(info.argv), info.envp); } #endif diff --git a/lldb/source/Host/windows/ProcessLauncherWindows.cpp b/lldb/source/Host/windows/ProcessLauncherWindows.cpp index baa422c15cae2..143f20cb3f510 100644 --- a/lldb/source/Host/windows/ProcessLauncherWindows.cpp +++ b/lldb/source/Host/windows/ProcessLauncherWindows.cpp @@ -113,14 +113,30 @@ ProcessLauncherWindows::LaunchProcess(const ProcessLaunchInfo _info, // command line is not empty, its contents may be modified by CreateProcessW. WCHAR *pwcommandLine = wcommandLine.empty() ? nullptr : [0]; - BOOL result = ::CreateProcessW( + BOOL result; + DWORD last_error = 0; + // This is the workaround for the error "The process cannot access the file + // because it is being used by another process". Note the executable file is + // installed to the target by the process `lldb-server platform`, but launched + // by the process `lldb-server gdbserver`. Sometimes system may block the file + // for some time after copying. + for (int i = 0; i < 50; ++i) { +result = ::CreateProcessW( wexecutable.c_str(), pwcommandLine, NULL, NULL, TRUE, flags, env_block, wworkingDirectory.size() == 0 ? NULL : wworkingDirectory.c_str(), , ); +if (!result) { + last_error = ::GetLastError(); + if (last_error != ERROR_SHARING_VIOLATION) +break; + ::Sleep(100); +} else + break; + } if (!result) { // Call GetLastError before we make any other system calls. -error.SetError(::GetLastError(), eErrorTypeWin32); +error.SetError(last_error, eErrorTypeWin32); // Note that error 50 ("The request is not supported") will occur if you // try debug a 64-bit inferior from a 32-bit LLDB. } ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Windows] Fixed Host::Kill() (PR #99721)
https://github.com/slydiman closed https://github.com/llvm/llvm-project/pull/99721 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Windows] Fixed Host::Kill() (PR #99721)
https://github.com/slydiman edited https://github.com/llvm/llvm-project/pull/99721 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Windows] Fixed Host::Kill() (PR #99721)
https://github.com/slydiman updated https://github.com/llvm/llvm-project/pull/99721 >From c033ba3832141409c2dac7e7363c0c85caa2274b Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Sat, 20 Jul 2024 03:48:12 +0400 Subject: [PATCH] [lldb][Windows] Fixed Host::Kill() HostProcessWindows::Terminate() correctly uses m_process which type is process_t (HANDLE) to call ::TerminateProcess(). But Host::Kill() uses a cast from pid, which is wrong. This patch fixes #51793 --- lldb/source/Host/windows/Host.cpp | 4 +++- .../API/functionalities/gdb_remote_client/TestPlatformKill.py | 1 - 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/lldb/source/Host/windows/Host.cpp b/lldb/source/Host/windows/Host.cpp index 6908f0003eaf7..642092f61d924 100644 --- a/lldb/source/Host/windows/Host.cpp +++ b/lldb/source/Host/windows/Host.cpp @@ -103,7 +103,9 @@ lldb::thread_t Host::GetCurrentThread() { } void Host::Kill(lldb::pid_t pid, int signo) { - TerminateProcess((HANDLE)pid, 1); + AutoHandle handle(::OpenProcess(PROCESS_TERMINATE, FALSE, pid), nullptr); + if (handle.IsValid()) +::TerminateProcess(handle.get(), 1); } const char *Host::GetSignalAsCString(int signo) { return NULL; } diff --git a/lldb/test/API/functionalities/gdb_remote_client/TestPlatformKill.py b/lldb/test/API/functionalities/gdb_remote_client/TestPlatformKill.py index 46cda4d66b488..430f1871d3b30 100644 --- a/lldb/test/API/functionalities/gdb_remote_client/TestPlatformKill.py +++ b/lldb/test/API/functionalities/gdb_remote_client/TestPlatformKill.py @@ -8,7 +8,6 @@ class TestPlatformKill(GDBRemoteTestBase): @skipIfRemote -@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr52451") def test_kill_different_platform(self): """Test connecting to a remote linux platform""" ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Windows] Fixed Host::Kill() (PR #99721)
https://github.com/slydiman edited https://github.com/llvm/llvm-project/pull/99721 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Windows] Fixed Host::Kill() (PR #99721)
https://github.com/slydiman created https://github.com/llvm/llvm-project/pull/99721 HostProcessWindows::Terminate() correctly uses m_process which type is process_t (HANDLE) to call ::TerminateProcess(). But Host::Kill() uses a cast from pid, which is wrong. >From 0712380a0ef60fae0a3035ac1e572bf9b1ea3bae Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Sat, 20 Jul 2024 03:48:12 +0400 Subject: [PATCH] [lldb][Windows] Fixed Host::Kill() HostProcessWindows::Terminate() correctly uses m_process which type is process_t (HANDLE) to call ::TerminateProcess(). But Host::Kill() uses a cast from pid, which is wrong. --- lldb/source/Host/windows/Host.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lldb/source/Host/windows/Host.cpp b/lldb/source/Host/windows/Host.cpp index 6908f0003eaf7..642092f61d924 100644 --- a/lldb/source/Host/windows/Host.cpp +++ b/lldb/source/Host/windows/Host.cpp @@ -103,7 +103,9 @@ lldb::thread_t Host::GetCurrentThread() { } void Host::Kill(lldb::pid_t pid, int signo) { - TerminateProcess((HANDLE)pid, 1); + AutoHandle handle(::OpenProcess(PROCESS_TERMINATE, FALSE, pid), nullptr); + if (handle.IsValid()) +::TerminateProcess(handle.get(), 1); } const char *Host::GetSignalAsCString(int signo) { return NULL; } ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed the error `unable to launch a GDB server` in API tests (PR #98833)
slydiman wrote: > Agreed, so silent UB would just be making things worse. So, this patch fixes that issue, right? > Perhaps I am spelling the port option differently, or you mean a lldb-server > gdbserver launched by a platform ignores the port. It seems it depends on the system. Note I'm working on a patch to fix #97537 and all port mapping related issues. The idea is to use threads for platform mode and use a common port map for all connections instead of making a new `portmap_for_child` with 1 port. https://github.com/llvm/llvm-project/pull/98833 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed the error `unable to launch a GDB server` in API tests (PR #98833)
slydiman wrote: The best solution is simply to remove all code related to gdb port mapping, because it does not work at all. Try to run `netstat` and you will be surprised. 1. lldb-server in gdbserver mode completely ignores the port from the url `tcp://[hostname]:port`. It binds to port 0 and it is hardcoded. 2. Currently lldb-server in platform mode always runs `lldb-server gdbserver tcp://[hostname]:0` because of the bug added 4 years ago and no one noticed this issue. If gdb port parameters specified, now it causes issues like #97537, but a random port will be used for connection anyway. If no gdb port parameters specified, the port map must be empty and must stay empty. AllowPort(0) makes the port map not empty and causes an unexpected behavior. This patch fixes this issue. https://github.com/llvm/llvm-project/pull/98833 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed the error `unable to launch a GDB server` in API tests (PR #98833)
slydiman wrote: > Did you consider making `AllowPort(0)` do nothing? GetNextAvailablePort() returns 0 in case of the empty map. LaunchGDBServer() does not update the map with the pid if the port is 0. So 0 is a special value which means `any`. Adding 0 to a map causes an unexpected behavior. https://github.com/llvm/llvm-project/pull/98833 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed the error `unable to launch a GDB server` in API tests (PR #98833)
https://github.com/slydiman edited https://github.com/llvm/llvm-project/pull/98833 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed the error `unable to launch a GDB server` in API tests (PR #98833)
https://github.com/slydiman edited https://github.com/llvm/llvm-project/pull/98833 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed the error `unable to launch a GDB server` in API tests (PR #98833)
slydiman wrote: It seems `StartDebugserverProcess()` ignores the port anyway and parameters `--min-gdbserver-port`, `--max-gdbserver-port`, `--gdbserver-port` are useless at all, but it is out of scope of this patch. https://github.com/llvm/llvm-project/pull/98833 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed the error `unable to launch a GDB server` in API tests (PR #98833)
https://github.com/slydiman edited https://github.com/llvm/llvm-project/pull/98833 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed the error `unable to launch a GDB server` in API tests (PR #98833)
https://github.com/slydiman edited https://github.com/llvm/llvm-project/pull/98833 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed the error `unable to launch a GDB server` in API tests (PR #98833)
https://github.com/slydiman created https://github.com/llvm/llvm-project/pull/98833 TestPlatformLaunchGDBServer.py runs ldb-server w/o parameters `--min-gdbserver-port`, `--max-gdbserver-port` or `--gdbserver-port`. So `gdbserver_portmap` is empty and `gdbserver_portmap.GetNextAvailablePort()` will return 0. Do not pass 0 to `portmap_for_child` in this case. Added few asserts in `GDBRemoteCommunicationServerPlatform::PortMap` to avoid such issue in the future. This patch fixes #97537. It seems StartDebugserverProcess() ignores the port anyway and parameters `--min-gdbserver-port`, `--max-gdbserver-port` and `--gdbserver-port` are useless at all, but it is out of scope of this patch. >From e061a85a54e3493a2ba4be90612fd66252f06ba2 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Sun, 14 Jul 2024 23:36:46 +0400 Subject: [PATCH] [lldb] Fixed the error `unable to launch a GDB server` in API tests TestPlatformLaunchGDBServer.py runs ldb-server w/o --min-gdbserver-port, --max-gdbserver-port or --gdbserver-port, so gdbserver_portmap is empty and gdbserver_portmap.GetNextAvailablePort() will return 0. Do not pass 0 to portmap_for_child in this case. Added few asserts in GDBRemoteCommunicationServerPlatform::PortMap to avoid this in the future. This patch fixes #97537. It seems StartDebugserverProcess() ignores the port anyway and parameters --min-gdbserver-port, --max-gdbserver-port and --gdbserver-port are useless at all, but it is out of scope of this patch. --- .../GDBRemoteCommunicationServerPlatform.cpp | 2 ++ lldb/tools/lldb-server/lldb-platform.cpp | 10 ++ 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp index 5285ec1d3db4e..65f1cc12ba307 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp @@ -46,11 +46,13 @@ using namespace lldb_private; GDBRemoteCommunicationServerPlatform::PortMap::PortMap(uint16_t min_port, uint16_t max_port) { + assert(min_port); for (; min_port < max_port; ++min_port) m_port_map[min_port] = LLDB_INVALID_PROCESS_ID; } void GDBRemoteCommunicationServerPlatform::PortMap::AllowPort(uint16_t port) { + assert(port); // Do not modify existing mappings m_port_map.insert({port, LLDB_INVALID_PROCESS_ID}); } diff --git a/lldb/tools/lldb-server/lldb-platform.cpp b/lldb/tools/lldb-server/lldb-platform.cpp index cfd0a3797d810..7148a1d2a3094 100644 --- a/lldb/tools/lldb-server/lldb-platform.cpp +++ b/lldb/tools/lldb-server/lldb-platform.cpp @@ -313,9 +313,11 @@ int main_platform(int argc, char *argv[]) { GDBRemoteCommunicationServerPlatform::PortMap portmap_for_child; llvm::Expected available_port = gdbserver_portmap.GetNextAvailablePort(); - if (available_port) -portmap_for_child.AllowPort(*available_port); - else { + if (available_port) { +// GetNextAvailablePort() may return 0 if gdbserver_portmap is empty. +if (*available_port) + portmap_for_child.AllowPort(*available_port); + } else { llvm::consumeError(available_port.takeError()); fprintf(stderr, "no available gdbserver port for connection - dropping...\n"); @@ -352,7 +354,7 @@ int main_platform(int argc, char *argv[]) { if (platform.IsConnected()) { if (inferior_arguments.GetArgumentCount() > 0) { lldb::pid_t pid = LLDB_INVALID_PROCESS_ID; -std::optional port = 0; +std::optional port; std::string socket_name; Status error = platform.LaunchGDBServer(inferior_arguments, "", // hostname ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Have lldb-server assign ports to children in platform mode (PR #88845)
slydiman wrote: Here is the log of lldb-server processes (parent and child) on remote Linux ``` /home/ubuntu/lldb-server p --log-channels lldb all --listen *:1234 --server --min-gdbserver-port 1236 --max-gdbserver-port 1240 /home/ubuntu/lldb-server gdbserver tcp://[10.1.1.170]:0 --native-regs --pipe 6 ``` Note the port `0` in the url `tcp://[10.1.1.170]:0` is a bug now. but any port in this url will be ignored. I don't see where `--min-gdbserver-port`, `--max-gdbserver-port` and `--gdbserver-port` values are really used. Do we still need them? `--port-offset` is not used too. Probably it is better to revert #88845 since the port mapping does not work as expected anyway. But #88845 caused test failures on cross builds. See https://github.com/llvm/llvm-project/issues/97537#issuecomment-2226588550 for more details. https://github.com/llvm/llvm-project/pull/88845 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Unify Platform::ResolveExecutable (PR #96256)
slydiman wrote: It seems this patch breaks the following tests on cross setups: ``` Failed Tests (6): lldb-api :: commands/process/attach/TestProcessAttach.py lldb-api :: commands/process/detach-resumes/TestDetachResumes.py lldb-api :: functionalities/dyld-exec-linux/TestDyldExecLinux.py lldb-api :: functionalities/dyld-launch-linux/TestDyldLaunchLinux.py lldb-api :: functionalities/exec/TestExec.py lldb-api :: functionalities/thread/create_after_attach/TestCreateAfterAttach.py ``` Note the behavior is the same for Windows and Linux hosts with Linux AArch64 target. Probably removing `RemoteAwarePlatform::ResolveExecutable` causes the issue. For example here is the log for lldb-api :: commands/process/attach/TestProcessAttach.py ``` Command Output (stdout): -- Setting up remote platform 'remote-linux' Connecting to remote platform 'remote-linux' at 'connect://test1:1234'... Connected. Setting remote platform working directory to '/home/ubuntu/lldb-tests'... Skipping the following test categories: ['dsym', 'gmodules', 'debugserver', 'objc', 'lldb-dap'] -- Command Output (stderr): -- PASS: LLDB (C:\\lldb-test-scripts\build-lldb\bin\clang.exe-aarch64) :: test_attach_to_process_by_id (TestProcessAttach.ProcessAttachTestCase.test_attach_to_process_by_id) PASS: LLDB (C:\\lldb-test-scripts\build-lldb\bin\clang.exe-aarch64) :: test_attach_to_process_by_id_autocontinue (TestProcessAttach.ProcessAttachTestCase.test_attach_to_process_by_id_autocontinue) FAIL: LLDB (C:\\lldb-test-scripts\build-lldb\bin\clang.exe-aarch64) :: test_attach_to_process_by_id_correct_executable_offset (TestProcessAttach.ProcessAttachTestCase.test_attach_to_process_by_id_correct_executable_offset) PASS: LLDB (C:\lldb-test-scripts\build-lldb\bin\clang.exe-aarch64) :: test_attach_to_process_by_name (TestProcessAttach.ProcessAttachTestCase.test_attach_to_process_by_name) PASS: LLDB (C:\lldb-test-scripts\build-lldb\bin\clang.exe-aarch64) :: test_attach_to_process_from_different_dir_by_id (TestProcessAttach.ProcessAttachTestCase.test_attach_to_process_from_different_dir_by_id) == FAIL: test_attach_to_process_by_id_correct_executable_offset (TestProcessAttach.ProcessAttachTestCase.test_attach_to_process_by_id_correct_executable_offset) Test that after attaching to a process the executable offset -- Traceback (most recent call last): File "C:\lldb-test-scripts\lldb\test\API\commands\process\attach\TestProcessAttach.py", line 119, in test_attach_to_process_by_id_correct_executable_offset lldbutil.run_break_set_by_file_and_line( File "C:\\lldb-test-scripts\lldb\packages\Python\lldbsuite\test\lldbutil.py", line 378, in run_break_set_by_file_and_line check_breakpoint_result( File "C:\\lldb-test-scripts\lldb\packages\Python\lldbsuite\test\lldbutil.py", line 592, in check_breakpoint_result test.assertTrue( AssertionError: False is not true : Expecting 1 locations, got 0. Ran 5 tests in 71.809s FAILED (failures=1) ``` https://github.com/llvm/llvm-project/pull/96256 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix TestModuleLoadedNotifys API test to work correctly on most of Linux targets (PR #94672)
https://github.com/slydiman closed https://github.com/llvm/llvm-project/pull/94672 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix TestModuleLoadedNotifys API test to work correctly on most of Linux targets (PR #94672)
@@ -118,6 +118,6 @@ def test_launch_notifications(self): # On Linux we get events for ld.so, [vdso], the binary and then all libraries. avg_solibs_added_per_event = round( -float(total_solibs_added) / float(total_modules_added_events) +10.0 * float(total_solibs_added) / float(total_modules_added_events) slydiman wrote: I just checked the documentation for assertGreater() trying to figure out the purpose of round() usage in the original code. But it does not matter anymore, since the patch has been significantly refactored. https://github.com/llvm/llvm-project/pull/94672 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix TestModuleLoadedNotifys API test to work correctly on most of Linux targets (PR #94672)
@@ -115,9 +151,10 @@ def test_launch_notifications(self): # binaries in batches. Check that we got back more than 1 solib per event. # In practice on Darwin today, we get back two events for a do-nothing c # program: a.out and dyld, and then all the rest of the system libraries. -# On Linux we get events for ld.so, [vdso], the binary and then all libraries. - -avg_solibs_added_per_event = round( -float(total_solibs_added) / float(total_modules_added_events) +# On Linux we get events for ld.so, [vdso], the binary and then all libraries, +# but the different configurations could load a different number of .so modules +# per event. +self.assertGreaterEqual( +len(set(max_solib_chunk_per_event).intersection(expected_solibs)), +len(expected_solibs), slydiman wrote: Done. https://github.com/llvm/llvm-project/pull/94672 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix TestModuleLoadedNotifys API test to work correctly on most of Linux targets (PR #94672)
@@ -1,3 +1,23 @@ -CXX_SOURCES := main.cpp - -include Makefile.rules +CXX_SOURCES := main.cpp +LD_EXTRAS := -L. -lloadunload_d -lloadunload_c -lloadunload_a -lloadunload_b slydiman wrote: Done. https://github.com/llvm/llvm-project/pull/94672 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix TestModuleLoadedNotifys API test to work correctly on most of Linux targets (PR #94672)
@@ -0,0 +1,7 @@ +extern "C" int b_function(); + +int a_init() { return 234; } + +int a_global = a_init(); slydiman wrote: Done. https://github.com/llvm/llvm-project/pull/94672 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix TestModuleLoadedNotifys API test to work correctly on most of Linux targets (PR #94672)
@@ -9,22 +9,51 @@ from lldbsuite.test import lldbutil +@skipUnlessPlatform(["linux"] + lldbplatformutil.getDarwinOSTriples()) class ModuleLoadedNotifysTestCase(TestBase): NO_DEBUG_INFO_TESTCASE = True # At least DynamicLoaderDarwin and DynamicLoaderPOSIXDYLD should batch up # notifications about newly added/removed libraries. Other DynamicLoaders may # not be written this way. -@skipUnlessPlatform(["linux"] + lldbplatformutil.getDarwinOSTriples()) def setUp(self): # Call super's setUp(). TestBase.setUp(self) # Find the line number to break inside main(). self.line = line_number("main.cpp", "// breakpoint") +def setup_test(self, solibs): +if lldb.remote_platform: +path = lldb.remote_platform.GetWorkingDirectory() +for f in solibs: +lldbutil.install_to_target(self, self.getBuildArtifact(f)) +else: +path = self.getBuildDir() +if self.dylibPath in os.environ: +sep = self.platformContext.shlib_path_separator +path = os.environ[self.dylibPath] + sep + path +self.runCmd( +"settings append target.env-vars '{}={}'".format(self.dylibPath, path) +) +self.default_path = path + def test_launch_notifications(self): """Test that lldb broadcasts newly loaded libraries in batches.""" + +ext = "so" +if self.platformIsDarwin(): +ext = "dylib" slydiman wrote: Done. https://github.com/llvm/llvm-project/pull/94672 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix TestModuleLoadedNotifys API test to work correctly on most of Linux targets (PR #94672)
https://github.com/slydiman updated https://github.com/llvm/llvm-project/pull/94672 >From 3f91ecacdcf1eedc95b72e8a85591e60a863431e Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Thu, 6 Jun 2024 23:38:03 +0400 Subject: [PATCH 1/4] [lldb] Fix TestModuleLoadedNotifys API test to work correctly on most of Linux targets. The different build configuration and target Linux system can load a different number of .so libraries (2 and more). As example, Linux x86_64 host shows the following loaded modules: ``` Loaded files: ld-linux-x86-64.so.2 Loaded files: [vdso] Loaded files: a.out Loaded files: libstdc++.so.6, libm.so.6, libgcc_s.so.1, libc.so.6 ``` avg_solibs_added_per_event = 1.75 But Linux Aarch64 (remote target) with statically linked C++ library (clang) shows: ``` Loaded files: ld-2.31.so Loaded files: [vdso] Loaded files: a.out Loaded files: libm.so.6, libc.so.6 ``` avg_solibs_added_per_event = 1.25 Increase precision by 1 digit to fix the test. --- .../target-new-solib-notifications/TestModuleLoadedNotifys.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py b/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py index abf761fb3420b..28a1026ae4fcc 100644 --- a/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py +++ b/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py @@ -118,6 +118,6 @@ def test_launch_notifications(self): # On Linux we get events for ld.so, [vdso], the binary and then all libraries. avg_solibs_added_per_event = round( -float(total_solibs_added) / float(total_modules_added_events) +10.0 * float(total_solibs_added) / float(total_modules_added_events) ) -self.assertGreater(avg_solibs_added_per_event, 1) +self.assertGreater(avg_solibs_added_per_event, 10) >From 0518bff0710a8f4404847e1cd28c6bcdbbb00093 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Fri, 7 Jun 2024 12:36:44 +0400 Subject: [PATCH 2/4] Keep the `avg_solibs_added_per_event` name correct. --- .../TestModuleLoadedNotifys.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py b/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py index 28a1026ae4fcc..ef407abce59fc 100644 --- a/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py +++ b/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py @@ -117,7 +117,7 @@ def test_launch_notifications(self): # program: a.out and dyld, and then all the rest of the system libraries. # On Linux we get events for ld.so, [vdso], the binary and then all libraries. -avg_solibs_added_per_event = round( -10.0 * float(total_solibs_added) / float(total_modules_added_events) +avg_solibs_added_per_event = float(total_solibs_added) / float( +total_modules_added_events ) -self.assertGreater(avg_solibs_added_per_event, 10) +self.assertGreater(round(10.0 * avg_solibs_added_per_event), 10) >From 60671ecc918692bba6d937bc7590eae55fd20514 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Sat, 8 Jun 2024 12:53:06 +0400 Subject: [PATCH 3/4] Added own so modules. --- .../target-new-solib-notifications/Makefile | 26 +++-- .../TestModuleLoadedNotifys.py| 53 --- .../target-new-solib-notifications/a.cpp | 7 +++ .../target-new-solib-notifications/b.cpp | 5 ++ .../target-new-solib-notifications/c.cpp | 1 + .../target-new-solib-notifications/d.cpp | 5 ++ .../target-new-solib-notifications/main.cpp | 22 +--- 7 files changed, 102 insertions(+), 17 deletions(-) create mode 100644 lldb/test/API/functionalities/target-new-solib-notifications/a.cpp create mode 100644 lldb/test/API/functionalities/target-new-solib-notifications/b.cpp create mode 100644 lldb/test/API/functionalities/target-new-solib-notifications/c.cpp create mode 100644 lldb/test/API/functionalities/target-new-solib-notifications/d.cpp diff --git a/lldb/test/API/functionalities/target-new-solib-notifications/Makefile b/lldb/test/API/functionalities/target-new-solib-notifications/Makefile index 8b20bcb05..50169c0ae1b80 100644 --- a/lldb/test/API/functionalities/target-new-solib-notifications/Makefile +++ b/lldb/test/API/functionalities/target-new-solib-notifications/Makefile @@ -1,3 +1,23 @@ -CXX_SOURCES := main.cpp - -include Makefile.rules +CXX_SOURCES := main.cpp +LD_EXTRAS := -L. -lloadunload_d -lloadunload_c -lloadunload_a -lloadunload_b + +a.out: lib_b lib_a lib_c lib_d + +include Makefile.rules + +lib_a: lib_b +
[Lldb-commits] [lldb] [lldb] Gracefully down TestCoroutineHandle test in case the 'coroutine' feature is missing (PR #94903)
https://github.com/slydiman closed https://github.com/llvm/llvm-project/pull/94903 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Gracefully down TestCoroutineHandle test in case the 'coroutine' feature is missing (PR #94903)
https://github.com/slydiman updated https://github.com/llvm/llvm-project/pull/94903 >From 6e988bcbbd4310a09c6baa553b4cf55998e92034 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Sun, 9 Jun 2024 18:33:03 +0400 Subject: [PATCH] [lldb] Gracefully down TestCoroutineHandle test in case the 'coroutine' feature is missing Do not let the compiler gets failed in case the target platform does not support the 'coroutine' C++ features. Just compile without it and let lldb know about missed/unsupported feature. --- .../generic/coroutine_handle/Makefile | 8 ++ .../generic/coroutine_handle/main.cpp | 27 --- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/Makefile b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/Makefile index 6914024392cfd..3e5da0ecae669 100644 --- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/Makefile +++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/Makefile @@ -1,4 +1,12 @@ CXX_SOURCES := main.cpp CFLAGS_EXTRAS := -std=c++20 +ifeq "1" "$(USE_LIBSTDCPP)" + CFLAGS_EXTRAS += -DUSE_LIBSTDCPP +endif + +ifeq "1" "$(USE_LIBCPP)" + CFLAGS_EXTRAS += -DUSE_LIBCPP +endif + include Makefile.rules diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp index 8cb81c3bc9f4c..aaf76e0d8d99a 100644 --- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp +++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp @@ -1,13 +1,27 @@ +#if defined(USE_LIBSTDCPP) +#include +// glibc++ >= 11 and c++20 +#if defined(_GLIBCXX_RELEASE) && _GLIBCXX_RELEASE >= 11 #include +#define HAS_CPP_COROUTINES 1 +#endif +#endif + +// libc++ always has 'coroutine' feature. +#if defined(USE_LIBCPP) +#include +#define HAS_CPP_COROUTINES 1 +#endif bool is_implementation_supported() { -#ifdef _GLIBCXX_RELEASE - return _GLIBCXX_RELEASE >= 11; -#else +#ifdef HAS_CPP_COROUTINES return true; +#else + return false; #endif } +#ifdef HAS_CPP_COROUTINES // `int_generator` is a stripped down, minimal coroutine generator // type. struct int_generator { @@ -39,8 +53,11 @@ int_generator my_generator_func() { co_yield 42; } // a place to reliably set a breakpoint on. void empty_function_so_we_can_set_a_breakpoint() {} +#endif // HAS_CPP_COROUTINES + int main() { bool is_supported = is_implementation_supported(); +#ifdef HAS_CPP_COROUTINES int_generator gen = my_generator_func(); std::coroutine_handle<> type_erased_hdl = gen.hdl; std::coroutine_handle incorrectly_typed_hdl = @@ -48,4 +65,8 @@ int main() { gen.hdl.resume();// Break at initial_suspend gen.hdl.resume();// Break after co_yield empty_function_so_we_can_set_a_breakpoint(); // Break at final_suspend + return 0; +#else + return 0; // Break at initial_suspend +#endif // HAS_CPP_COROUTINES } ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Gracefully down TestCoroutineHandle test in case the 'coroutine' feature is missing (PR #94903)
https://github.com/slydiman created https://github.com/llvm/llvm-project/pull/94903 Do not let the compiler gets failed in case the target platform does not support the 'coroutine' C++ features. Just compile without it and let lldb know about missed/unsupported feature. >From b0a1c4152ebbca95f5560562c9f5605b505d353c Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Sun, 9 Jun 2024 18:33:03 +0400 Subject: [PATCH] [lldb] Gracefully down TestCoroutineHandle test in case the 'coroutine' feature is missing Do not let the compiler gets failed in case the target platform does not support the 'coroutine' C++ features. Just compile without it and let lldb know about missed/unsupported feature. --- .../generic/coroutine_handle/Makefile | 8 +++ .../generic/coroutine_handle/main.cpp | 24 --- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/Makefile b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/Makefile index 6914024392cfd..3e5da0ecae669 100644 --- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/Makefile +++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/Makefile @@ -1,4 +1,12 @@ CXX_SOURCES := main.cpp CFLAGS_EXTRAS := -std=c++20 +ifeq "1" "$(USE_LIBSTDCPP)" + CFLAGS_EXTRAS += -DUSE_LIBSTDCPP +endif + +ifeq "1" "$(USE_LIBCPP)" + CFLAGS_EXTRAS += -DUSE_LIBCPP +endif + include Makefile.rules diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp index 8cb81c3bc9f4c..ab1bea7597cfe 100644 --- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp +++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/main.cpp @@ -1,13 +1,27 @@ +#if defined(USE_LIBSTDCPP) +#include +// glibc++ >= 11 and c++20 +#if defined(_GLIBCXX_RELEASE) && _GLIBCXX_RELEASE >= 11 #include +#define HAS_CPP_COROUTINES 1 +#endif +#endif + +// libc++ always has 'coroutine' feature. +#if defined(USE_LIBCPP) +#include +#define HAS_CPP_COROUTINES 1 +#endif bool is_implementation_supported() { -#ifdef _GLIBCXX_RELEASE - return _GLIBCXX_RELEASE >= 11; -#else +#ifdef HAS_CPP_COROUTINES return true; +#else + return false; #endif } +#ifdef HAS_CPP_COROUTINES // `int_generator` is a stripped down, minimal coroutine generator // type. struct int_generator { @@ -39,8 +53,11 @@ int_generator my_generator_func() { co_yield 42; } // a place to reliably set a breakpoint on. void empty_function_so_we_can_set_a_breakpoint() {} +#endif // HAS_CPP_COROUTINES + int main() { bool is_supported = is_implementation_supported(); +#ifdef HAS_CPP_COROUTINES int_generator gen = my_generator_func(); std::coroutine_handle<> type_erased_hdl = gen.hdl; std::coroutine_handle incorrectly_typed_hdl = @@ -48,4 +65,5 @@ int main() { gen.hdl.resume();// Break at initial_suspend gen.hdl.resume();// Break after co_yield empty_function_so_we_can_set_a_breakpoint(); // Break at final_suspend +#endif // HAS_CPP_COROUTINES } ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix TestModuleLoadedNotifys API test to work correctly on most of Linux targets (PR #94672)
https://github.com/slydiman updated https://github.com/llvm/llvm-project/pull/94672 >From 3f91ecacdcf1eedc95b72e8a85591e60a863431e Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Thu, 6 Jun 2024 23:38:03 +0400 Subject: [PATCH 1/3] [lldb] Fix TestModuleLoadedNotifys API test to work correctly on most of Linux targets. The different build configuration and target Linux system can load a different number of .so libraries (2 and more). As example, Linux x86_64 host shows the following loaded modules: ``` Loaded files: ld-linux-x86-64.so.2 Loaded files: [vdso] Loaded files: a.out Loaded files: libstdc++.so.6, libm.so.6, libgcc_s.so.1, libc.so.6 ``` avg_solibs_added_per_event = 1.75 But Linux Aarch64 (remote target) with statically linked C++ library (clang) shows: ``` Loaded files: ld-2.31.so Loaded files: [vdso] Loaded files: a.out Loaded files: libm.so.6, libc.so.6 ``` avg_solibs_added_per_event = 1.25 Increase precision by 1 digit to fix the test. --- .../target-new-solib-notifications/TestModuleLoadedNotifys.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py b/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py index abf761fb3420b..28a1026ae4fcc 100644 --- a/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py +++ b/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py @@ -118,6 +118,6 @@ def test_launch_notifications(self): # On Linux we get events for ld.so, [vdso], the binary and then all libraries. avg_solibs_added_per_event = round( -float(total_solibs_added) / float(total_modules_added_events) +10.0 * float(total_solibs_added) / float(total_modules_added_events) ) -self.assertGreater(avg_solibs_added_per_event, 1) +self.assertGreater(avg_solibs_added_per_event, 10) >From 0518bff0710a8f4404847e1cd28c6bcdbbb00093 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Fri, 7 Jun 2024 12:36:44 +0400 Subject: [PATCH 2/3] Keep the `avg_solibs_added_per_event` name correct. --- .../TestModuleLoadedNotifys.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py b/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py index 28a1026ae4fcc..ef407abce59fc 100644 --- a/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py +++ b/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py @@ -117,7 +117,7 @@ def test_launch_notifications(self): # program: a.out and dyld, and then all the rest of the system libraries. # On Linux we get events for ld.so, [vdso], the binary and then all libraries. -avg_solibs_added_per_event = round( -10.0 * float(total_solibs_added) / float(total_modules_added_events) +avg_solibs_added_per_event = float(total_solibs_added) / float( +total_modules_added_events ) -self.assertGreater(avg_solibs_added_per_event, 10) +self.assertGreater(round(10.0 * avg_solibs_added_per_event), 10) >From 60671ecc918692bba6d937bc7590eae55fd20514 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Sat, 8 Jun 2024 12:53:06 +0400 Subject: [PATCH 3/3] Added own so modules. --- .../target-new-solib-notifications/Makefile | 26 +++-- .../TestModuleLoadedNotifys.py| 53 --- .../target-new-solib-notifications/a.cpp | 7 +++ .../target-new-solib-notifications/b.cpp | 5 ++ .../target-new-solib-notifications/c.cpp | 1 + .../target-new-solib-notifications/d.cpp | 5 ++ .../target-new-solib-notifications/main.cpp | 22 +--- 7 files changed, 102 insertions(+), 17 deletions(-) create mode 100644 lldb/test/API/functionalities/target-new-solib-notifications/a.cpp create mode 100644 lldb/test/API/functionalities/target-new-solib-notifications/b.cpp create mode 100644 lldb/test/API/functionalities/target-new-solib-notifications/c.cpp create mode 100644 lldb/test/API/functionalities/target-new-solib-notifications/d.cpp diff --git a/lldb/test/API/functionalities/target-new-solib-notifications/Makefile b/lldb/test/API/functionalities/target-new-solib-notifications/Makefile index 8b20bcb05..50169c0ae1b80 100644 --- a/lldb/test/API/functionalities/target-new-solib-notifications/Makefile +++ b/lldb/test/API/functionalities/target-new-solib-notifications/Makefile @@ -1,3 +1,23 @@ -CXX_SOURCES := main.cpp - -include Makefile.rules +CXX_SOURCES := main.cpp +LD_EXTRAS := -L. -lloadunload_d -lloadunload_c -lloadunload_a -lloadunload_b + +a.out: lib_b lib_a lib_c lib_d + +include Makefile.rules + +lib_a: lib_b +
[Lldb-commits] [lldb] [lldb] Fix TestModuleLoadedNotifys API test to work correctly on most of Linux targets (PR #94672)
https://github.com/slydiman updated https://github.com/llvm/llvm-project/pull/94672 >From 3f91ecacdcf1eedc95b72e8a85591e60a863431e Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Thu, 6 Jun 2024 23:38:03 +0400 Subject: [PATCH 1/3] [lldb] Fix TestModuleLoadedNotifys API test to work correctly on most of Linux targets. The different build configuration and target Linux system can load a different number of .so libraries (2 and more). As example, Linux x86_64 host shows the following loaded modules: ``` Loaded files: ld-linux-x86-64.so.2 Loaded files: [vdso] Loaded files: a.out Loaded files: libstdc++.so.6, libm.so.6, libgcc_s.so.1, libc.so.6 ``` avg_solibs_added_per_event = 1.75 But Linux Aarch64 (remote target) with statically linked C++ library (clang) shows: ``` Loaded files: ld-2.31.so Loaded files: [vdso] Loaded files: a.out Loaded files: libm.so.6, libc.so.6 ``` avg_solibs_added_per_event = 1.25 Increase precision by 1 digit to fix the test. --- .../target-new-solib-notifications/TestModuleLoadedNotifys.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py b/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py index abf761fb3420b..28a1026ae4fcc 100644 --- a/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py +++ b/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py @@ -118,6 +118,6 @@ def test_launch_notifications(self): # On Linux we get events for ld.so, [vdso], the binary and then all libraries. avg_solibs_added_per_event = round( -float(total_solibs_added) / float(total_modules_added_events) +10.0 * float(total_solibs_added) / float(total_modules_added_events) ) -self.assertGreater(avg_solibs_added_per_event, 1) +self.assertGreater(avg_solibs_added_per_event, 10) >From 0518bff0710a8f4404847e1cd28c6bcdbbb00093 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Fri, 7 Jun 2024 12:36:44 +0400 Subject: [PATCH 2/3] Keep the `avg_solibs_added_per_event` name correct. --- .../TestModuleLoadedNotifys.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py b/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py index 28a1026ae4fcc..ef407abce59fc 100644 --- a/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py +++ b/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py @@ -117,7 +117,7 @@ def test_launch_notifications(self): # program: a.out and dyld, and then all the rest of the system libraries. # On Linux we get events for ld.so, [vdso], the binary and then all libraries. -avg_solibs_added_per_event = round( -10.0 * float(total_solibs_added) / float(total_modules_added_events) +avg_solibs_added_per_event = float(total_solibs_added) / float( +total_modules_added_events ) -self.assertGreater(avg_solibs_added_per_event, 10) +self.assertGreater(round(10.0 * avg_solibs_added_per_event), 10) >From d7832a1181613caeb68c597db17680c3c6008de8 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Sat, 8 Jun 2024 12:53:06 +0400 Subject: [PATCH 3/3] Added own so modules. --- .../target-new-solib-notifications/Makefile | 26 +++-- .../TestModuleLoadedNotifys.py| 53 --- .../target-new-solib-notifications/a.cpp | 7 +++ .../target-new-solib-notifications/b.cpp | 5 ++ .../target-new-solib-notifications/c.cpp | 1 + .../target-new-solib-notifications/d.cpp | 5 ++ .../target-new-solib-notifications/main.cpp | 22 +--- 7 files changed, 102 insertions(+), 17 deletions(-) create mode 100644 lldb/test/API/functionalities/target-new-solib-notifications/a.cpp create mode 100644 lldb/test/API/functionalities/target-new-solib-notifications/b.cpp create mode 100644 lldb/test/API/functionalities/target-new-solib-notifications/c.cpp create mode 100644 lldb/test/API/functionalities/target-new-solib-notifications/d.cpp diff --git a/lldb/test/API/functionalities/target-new-solib-notifications/Makefile b/lldb/test/API/functionalities/target-new-solib-notifications/Makefile index 8b20bcb05..50169c0ae1b80 100644 --- a/lldb/test/API/functionalities/target-new-solib-notifications/Makefile +++ b/lldb/test/API/functionalities/target-new-solib-notifications/Makefile @@ -1,3 +1,23 @@ -CXX_SOURCES := main.cpp - -include Makefile.rules +CXX_SOURCES := main.cpp +LD_EXTRAS := -L. -lloadunload_d -lloadunload_c -lloadunload_a -lloadunload_b + +a.out: lib_b lib_a lib_c lib_d + +include Makefile.rules + +lib_a: lib_b +
[Lldb-commits] [lldb] [lldb] Fix TestModuleLoadedNotifys API test to work correctly on most of Linux targets (PR #94672)
https://github.com/slydiman updated https://github.com/llvm/llvm-project/pull/94672 >From 3f91ecacdcf1eedc95b72e8a85591e60a863431e Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Thu, 6 Jun 2024 23:38:03 +0400 Subject: [PATCH 1/3] [lldb] Fix TestModuleLoadedNotifys API test to work correctly on most of Linux targets. The different build configuration and target Linux system can load a different number of .so libraries (2 and more). As example, Linux x86_64 host shows the following loaded modules: ``` Loaded files: ld-linux-x86-64.so.2 Loaded files: [vdso] Loaded files: a.out Loaded files: libstdc++.so.6, libm.so.6, libgcc_s.so.1, libc.so.6 ``` avg_solibs_added_per_event = 1.75 But Linux Aarch64 (remote target) with statically linked C++ library (clang) shows: ``` Loaded files: ld-2.31.so Loaded files: [vdso] Loaded files: a.out Loaded files: libm.so.6, libc.so.6 ``` avg_solibs_added_per_event = 1.25 Increase precision by 1 digit to fix the test. --- .../target-new-solib-notifications/TestModuleLoadedNotifys.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py b/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py index abf761fb3420b..28a1026ae4fcc 100644 --- a/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py +++ b/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py @@ -118,6 +118,6 @@ def test_launch_notifications(self): # On Linux we get events for ld.so, [vdso], the binary and then all libraries. avg_solibs_added_per_event = round( -float(total_solibs_added) / float(total_modules_added_events) +10.0 * float(total_solibs_added) / float(total_modules_added_events) ) -self.assertGreater(avg_solibs_added_per_event, 1) +self.assertGreater(avg_solibs_added_per_event, 10) >From 0518bff0710a8f4404847e1cd28c6bcdbbb00093 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Fri, 7 Jun 2024 12:36:44 +0400 Subject: [PATCH 2/3] Keep the `avg_solibs_added_per_event` name correct. --- .../TestModuleLoadedNotifys.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py b/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py index 28a1026ae4fcc..ef407abce59fc 100644 --- a/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py +++ b/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py @@ -117,7 +117,7 @@ def test_launch_notifications(self): # program: a.out and dyld, and then all the rest of the system libraries. # On Linux we get events for ld.so, [vdso], the binary and then all libraries. -avg_solibs_added_per_event = round( -10.0 * float(total_solibs_added) / float(total_modules_added_events) +avg_solibs_added_per_event = float(total_solibs_added) / float( +total_modules_added_events ) -self.assertGreater(avg_solibs_added_per_event, 10) +self.assertGreater(round(10.0 * avg_solibs_added_per_event), 10) >From 0aec72ad777be192d0dd0fc0aba5a52fd0767ada Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Sat, 8 Jun 2024 12:53:06 +0400 Subject: [PATCH 3/3] Added own so modules. --- .../target-new-solib-notifications/Makefile | 26 +++-- .../TestModuleLoadedNotifys.py| 53 --- .../target-new-solib-notifications/a.cpp | 14 + .../target-new-solib-notifications/b.cpp | 9 .../target-new-solib-notifications/c.cpp | 5 ++ .../target-new-solib-notifications/d.cpp | 10 .../target-new-solib-notifications/main.cpp | 23 +--- 7 files changed, 123 insertions(+), 17 deletions(-) create mode 100644 lldb/test/API/functionalities/target-new-solib-notifications/a.cpp create mode 100644 lldb/test/API/functionalities/target-new-solib-notifications/b.cpp create mode 100644 lldb/test/API/functionalities/target-new-solib-notifications/c.cpp create mode 100644 lldb/test/API/functionalities/target-new-solib-notifications/d.cpp diff --git a/lldb/test/API/functionalities/target-new-solib-notifications/Makefile b/lldb/test/API/functionalities/target-new-solib-notifications/Makefile index 8b20bcb05..50169c0ae1b80 100644 --- a/lldb/test/API/functionalities/target-new-solib-notifications/Makefile +++ b/lldb/test/API/functionalities/target-new-solib-notifications/Makefile @@ -1,3 +1,23 @@ -CXX_SOURCES := main.cpp - -include Makefile.rules +CXX_SOURCES := main.cpp +LD_EXTRAS := -L. -lloadunload_d -lloadunload_c -lloadunload_a -lloadunload_b + +a.out: lib_b lib_a lib_c lib_d + +include Makefile.rules + +lib_a: lib_b +
[Lldb-commits] [lldb] [lldb] Fix TestModuleLoadedNotifys API test to work correctly on most of Linux targets (PR #94672)
https://github.com/slydiman updated https://github.com/llvm/llvm-project/pull/94672 >From 3f91ecacdcf1eedc95b72e8a85591e60a863431e Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Thu, 6 Jun 2024 23:38:03 +0400 Subject: [PATCH 1/3] [lldb] Fix TestModuleLoadedNotifys API test to work correctly on most of Linux targets. The different build configuration and target Linux system can load a different number of .so libraries (2 and more). As example, Linux x86_64 host shows the following loaded modules: ``` Loaded files: ld-linux-x86-64.so.2 Loaded files: [vdso] Loaded files: a.out Loaded files: libstdc++.so.6, libm.so.6, libgcc_s.so.1, libc.so.6 ``` avg_solibs_added_per_event = 1.75 But Linux Aarch64 (remote target) with statically linked C++ library (clang) shows: ``` Loaded files: ld-2.31.so Loaded files: [vdso] Loaded files: a.out Loaded files: libm.so.6, libc.so.6 ``` avg_solibs_added_per_event = 1.25 Increase precision by 1 digit to fix the test. --- .../target-new-solib-notifications/TestModuleLoadedNotifys.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py b/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py index abf761fb3420b..28a1026ae4fcc 100644 --- a/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py +++ b/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py @@ -118,6 +118,6 @@ def test_launch_notifications(self): # On Linux we get events for ld.so, [vdso], the binary and then all libraries. avg_solibs_added_per_event = round( -float(total_solibs_added) / float(total_modules_added_events) +10.0 * float(total_solibs_added) / float(total_modules_added_events) ) -self.assertGreater(avg_solibs_added_per_event, 1) +self.assertGreater(avg_solibs_added_per_event, 10) >From 0518bff0710a8f4404847e1cd28c6bcdbbb00093 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Fri, 7 Jun 2024 12:36:44 +0400 Subject: [PATCH 2/3] Keep the `avg_solibs_added_per_event` name correct. --- .../TestModuleLoadedNotifys.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py b/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py index 28a1026ae4fcc..ef407abce59fc 100644 --- a/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py +++ b/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py @@ -117,7 +117,7 @@ def test_launch_notifications(self): # program: a.out and dyld, and then all the rest of the system libraries. # On Linux we get events for ld.so, [vdso], the binary and then all libraries. -avg_solibs_added_per_event = round( -10.0 * float(total_solibs_added) / float(total_modules_added_events) +avg_solibs_added_per_event = float(total_solibs_added) / float( +total_modules_added_events ) -self.assertGreater(avg_solibs_added_per_event, 10) +self.assertGreater(round(10.0 * avg_solibs_added_per_event), 10) >From 2e2b3087be5763e73136b391f6b666067dea036c Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Sat, 8 Jun 2024 12:53:06 +0400 Subject: [PATCH 3/3] Added own so modules. --- .../target-new-solib-notifications/Makefile | 26 +++-- .../TestModuleLoadedNotifys.py| 53 --- .../target-new-solib-notifications/a.cpp | 14 + .../target-new-solib-notifications/b.cpp | 9 .../target-new-solib-notifications/c.cpp | 5 ++ .../target-new-solib-notifications/d.cpp | 10 .../target-new-solib-notifications/main.cpp | 23 +--- 7 files changed, 123 insertions(+), 17 deletions(-) create mode 100644 lldb/test/API/functionalities/target-new-solib-notifications/a.cpp create mode 100644 lldb/test/API/functionalities/target-new-solib-notifications/b.cpp create mode 100644 lldb/test/API/functionalities/target-new-solib-notifications/c.cpp create mode 100644 lldb/test/API/functionalities/target-new-solib-notifications/d.cpp diff --git a/lldb/test/API/functionalities/target-new-solib-notifications/Makefile b/lldb/test/API/functionalities/target-new-solib-notifications/Makefile index 8b20bcb05..50169c0ae1b80 100644 --- a/lldb/test/API/functionalities/target-new-solib-notifications/Makefile +++ b/lldb/test/API/functionalities/target-new-solib-notifications/Makefile @@ -1,3 +1,23 @@ -CXX_SOURCES := main.cpp - -include Makefile.rules +CXX_SOURCES := main.cpp +LD_EXTRAS := -L. -lloadunload_d -lloadunload_c -lloadunload_a -lloadunload_b + +a.out: lib_b lib_a lib_c lib_d + +include Makefile.rules + +lib_a: lib_b +
[Lldb-commits] [lldb] [lldb] Fix TestModuleLoadedNotifys API test to work correctly on most of Linux targets (PR #94672)
@@ -118,6 +118,6 @@ def test_launch_notifications(self): # On Linux we get events for ld.so, [vdso], the binary and then all libraries. avg_solibs_added_per_event = round( -float(total_solibs_added) / float(total_modules_added_events) +10.0 * float(total_solibs_added) / float(total_modules_added_events) slydiman wrote: @labath > So, another way to make this test be more resilient is to introduce a couple > of shared libraries of our own The patch is updated this way. Please review. Thanks. https://github.com/llvm/llvm-project/pull/94672 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix TestModuleLoadedNotifys API test to work correctly on most of Linux targets (PR #94672)
https://github.com/slydiman updated https://github.com/llvm/llvm-project/pull/94672 >From 3f91ecacdcf1eedc95b72e8a85591e60a863431e Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Thu, 6 Jun 2024 23:38:03 +0400 Subject: [PATCH 1/3] [lldb] Fix TestModuleLoadedNotifys API test to work correctly on most of Linux targets. The different build configuration and target Linux system can load a different number of .so libraries (2 and more). As example, Linux x86_64 host shows the following loaded modules: ``` Loaded files: ld-linux-x86-64.so.2 Loaded files: [vdso] Loaded files: a.out Loaded files: libstdc++.so.6, libm.so.6, libgcc_s.so.1, libc.so.6 ``` avg_solibs_added_per_event = 1.75 But Linux Aarch64 (remote target) with statically linked C++ library (clang) shows: ``` Loaded files: ld-2.31.so Loaded files: [vdso] Loaded files: a.out Loaded files: libm.so.6, libc.so.6 ``` avg_solibs_added_per_event = 1.25 Increase precision by 1 digit to fix the test. --- .../target-new-solib-notifications/TestModuleLoadedNotifys.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py b/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py index abf761fb3420b..28a1026ae4fcc 100644 --- a/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py +++ b/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py @@ -118,6 +118,6 @@ def test_launch_notifications(self): # On Linux we get events for ld.so, [vdso], the binary and then all libraries. avg_solibs_added_per_event = round( -float(total_solibs_added) / float(total_modules_added_events) +10.0 * float(total_solibs_added) / float(total_modules_added_events) ) -self.assertGreater(avg_solibs_added_per_event, 1) +self.assertGreater(avg_solibs_added_per_event, 10) >From 0518bff0710a8f4404847e1cd28c6bcdbbb00093 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Fri, 7 Jun 2024 12:36:44 +0400 Subject: [PATCH 2/3] Keep the `avg_solibs_added_per_event` name correct. --- .../TestModuleLoadedNotifys.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py b/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py index 28a1026ae4fcc..ef407abce59fc 100644 --- a/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py +++ b/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py @@ -117,7 +117,7 @@ def test_launch_notifications(self): # program: a.out and dyld, and then all the rest of the system libraries. # On Linux we get events for ld.so, [vdso], the binary and then all libraries. -avg_solibs_added_per_event = round( -10.0 * float(total_solibs_added) / float(total_modules_added_events) +avg_solibs_added_per_event = float(total_solibs_added) / float( +total_modules_added_events ) -self.assertGreater(avg_solibs_added_per_event, 10) +self.assertGreater(round(10.0 * avg_solibs_added_per_event), 10) >From 6fb19a05a6a4fb4463035bcd35a53fc51e446a05 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Sat, 8 Jun 2024 12:53:06 +0400 Subject: [PATCH 3/3] Added own so modules. --- .../target-new-solib-notifications/Makefile | 26 +++-- .../TestModuleLoadedNotifys.py| 53 --- .../target-new-solib-notifications/a.cpp | 14 + .../target-new-solib-notifications/b.cpp | 9 .../target-new-solib-notifications/c.cpp | 5 ++ .../target-new-solib-notifications/d.cpp | 10 .../target-new-solib-notifications/main.cpp | 24 ++--- 7 files changed, 124 insertions(+), 17 deletions(-) create mode 100644 lldb/test/API/functionalities/target-new-solib-notifications/a.cpp create mode 100644 lldb/test/API/functionalities/target-new-solib-notifications/b.cpp create mode 100644 lldb/test/API/functionalities/target-new-solib-notifications/c.cpp create mode 100644 lldb/test/API/functionalities/target-new-solib-notifications/d.cpp diff --git a/lldb/test/API/functionalities/target-new-solib-notifications/Makefile b/lldb/test/API/functionalities/target-new-solib-notifications/Makefile index 8b20bcb05..50169c0ae1b80 100644 --- a/lldb/test/API/functionalities/target-new-solib-notifications/Makefile +++ b/lldb/test/API/functionalities/target-new-solib-notifications/Makefile @@ -1,3 +1,23 @@ -CXX_SOURCES := main.cpp - -include Makefile.rules +CXX_SOURCES := main.cpp +LD_EXTRAS := -L. -lloadunload_d -lloadunload_c -lloadunload_a -lloadunload_b + +a.out: lib_b lib_a lib_c lib_d + +include Makefile.rules + +lib_a: lib_b +
[Lldb-commits] [lldb] [lldb] Fix TestModuleLoadedNotifys API test to work correctly on most of Linux targets (PR #94672)
https://github.com/slydiman edited https://github.com/llvm/llvm-project/pull/94672 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix TestModuleLoadedNotifys API test to work correctly on most of Linux targets (PR #94672)
https://github.com/slydiman updated https://github.com/llvm/llvm-project/pull/94672 >From 3f91ecacdcf1eedc95b72e8a85591e60a863431e Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Thu, 6 Jun 2024 23:38:03 +0400 Subject: [PATCH 1/2] [lldb] Fix TestModuleLoadedNotifys API test to work correctly on most of Linux targets. The different build configuration and target Linux system can load a different number of .so libraries (2 and more). As example, Linux x86_64 host shows the following loaded modules: ``` Loaded files: ld-linux-x86-64.so.2 Loaded files: [vdso] Loaded files: a.out Loaded files: libstdc++.so.6, libm.so.6, libgcc_s.so.1, libc.so.6 ``` avg_solibs_added_per_event = 1.75 But Linux Aarch64 (remote target) with statically linked C++ library (clang) shows: ``` Loaded files: ld-2.31.so Loaded files: [vdso] Loaded files: a.out Loaded files: libm.so.6, libc.so.6 ``` avg_solibs_added_per_event = 1.25 Increase precision by 1 digit to fix the test. --- .../target-new-solib-notifications/TestModuleLoadedNotifys.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py b/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py index abf761fb3420b..28a1026ae4fcc 100644 --- a/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py +++ b/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py @@ -118,6 +118,6 @@ def test_launch_notifications(self): # On Linux we get events for ld.so, [vdso], the binary and then all libraries. avg_solibs_added_per_event = round( -float(total_solibs_added) / float(total_modules_added_events) +10.0 * float(total_solibs_added) / float(total_modules_added_events) ) -self.assertGreater(avg_solibs_added_per_event, 1) +self.assertGreater(avg_solibs_added_per_event, 10) >From 0518bff0710a8f4404847e1cd28c6bcdbbb00093 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Fri, 7 Jun 2024 12:36:44 +0400 Subject: [PATCH 2/2] Keep the `avg_solibs_added_per_event` name correct. --- .../TestModuleLoadedNotifys.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py b/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py index 28a1026ae4fcc..ef407abce59fc 100644 --- a/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py +++ b/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py @@ -117,7 +117,7 @@ def test_launch_notifications(self): # program: a.out and dyld, and then all the rest of the system libraries. # On Linux we get events for ld.so, [vdso], the binary and then all libraries. -avg_solibs_added_per_event = round( -10.0 * float(total_solibs_added) / float(total_modules_added_events) +avg_solibs_added_per_event = float(total_solibs_added) / float( +total_modules_added_events ) -self.assertGreater(avg_solibs_added_per_event, 10) +self.assertGreater(round(10.0 * avg_solibs_added_per_event), 10) ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix TestModuleLoadedNotifys API test to work correctly on most of Linux targets (PR #94672)
@@ -118,6 +118,6 @@ def test_launch_notifications(self): # On Linux we get events for ld.so, [vdso], the binary and then all libraries. avg_solibs_added_per_event = round( -float(total_solibs_added) / float(total_modules_added_events) +10.0 * float(total_solibs_added) / float(total_modules_added_events) slydiman wrote: > You can't really call it "number of libraries per event" anymore if you > multiply by ten. avg10_solibs_added_per_event ? > Maybe you could just remove the round call, and compare to 1.0 No, because assertGreater() expects int parameters. > Also, this test is very unhermetic Right, and it seems that the behavior on Darwin is very different. I'd just check `min_modules_per_event >= 1`. https://github.com/llvm/llvm-project/pull/94672 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix TestModuleLoadedNotifys API test to work correctly on most of Linux targets (PR #94672)
https://github.com/slydiman created https://github.com/llvm/llvm-project/pull/94672 The different build configuration and target Linux system can load a different number of .so libraries (2 and more). As example, Linux x86_64 host shows the following loaded modules: ``` Loaded files: ld-linux-x86-64.so.2 Loaded files: [vdso] Loaded files: a.out Loaded files: libstdc++.so.6, libm.so.6, libgcc_s.so.1, libc.so.6 ``` avg_solibs_added_per_event = 1.75 But Linux Aarch64 (remote target) with statically linked C++ library (clang) shows: ``` Loaded files: ld-2.31.so Loaded files: [vdso] Loaded files: a.out Loaded files: libm.so.6, libc.so.6 ``` avg_solibs_added_per_event = 1.25 Increase precision by 1 digit to fix the test. >From 3f91ecacdcf1eedc95b72e8a85591e60a863431e Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Thu, 6 Jun 2024 23:38:03 +0400 Subject: [PATCH] [lldb] Fix TestModuleLoadedNotifys API test to work correctly on most of Linux targets. The different build configuration and target Linux system can load a different number of .so libraries (2 and more). As example, Linux x86_64 host shows the following loaded modules: ``` Loaded files: ld-linux-x86-64.so.2 Loaded files: [vdso] Loaded files: a.out Loaded files: libstdc++.so.6, libm.so.6, libgcc_s.so.1, libc.so.6 ``` avg_solibs_added_per_event = 1.75 But Linux Aarch64 (remote target) with statically linked C++ library (clang) shows: ``` Loaded files: ld-2.31.so Loaded files: [vdso] Loaded files: a.out Loaded files: libm.so.6, libc.so.6 ``` avg_solibs_added_per_event = 1.25 Increase precision by 1 digit to fix the test. --- .../target-new-solib-notifications/TestModuleLoadedNotifys.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py b/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py index abf761fb3420b..28a1026ae4fcc 100644 --- a/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py +++ b/lldb/test/API/functionalities/target-new-solib-notifications/TestModuleLoadedNotifys.py @@ -118,6 +118,6 @@ def test_launch_notifications(self): # On Linux we get events for ld.so, [vdso], the binary and then all libraries. avg_solibs_added_per_event = round( -float(total_solibs_added) / float(total_modules_added_events) +10.0 * float(total_solibs_added) / float(total_modules_added_events) ) -self.assertGreater(avg_solibs_added_per_event, 1) +self.assertGreater(avg_solibs_added_per_event, 10) ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Disable TestPtyServer API test when remote testing (PR #94587)
https://github.com/slydiman created https://github.com/llvm/llvm-project/pull/94587 The local PTY is not available for the remotely executed lldb-server to pass the test. Also, in general, we cannot execute the local lldb-server instance because it could be compiled for the different system/cpu target. >From 9288b9a59290a4ded37928f2f553f4c334ebacad Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev Date: Thu, 6 Jun 2024 11:32:13 +0400 Subject: [PATCH] [lldb] Disable TestPtyServer API test when remote testing The local PTY is not available for the remotely executed lldb-server to pass the test. Also, in general, we cannot execute the local lldb-server instance because it could be compiled for the different system/cpu target. --- lldb/test/API/tools/lldb-server/TestPtyServer.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lldb/test/API/tools/lldb-server/TestPtyServer.py b/lldb/test/API/tools/lldb-server/TestPtyServer.py index 4bfcf70bfa01b..345f68f6d87d3 100644 --- a/lldb/test/API/tools/lldb-server/TestPtyServer.py +++ b/lldb/test/API/tools/lldb-server/TestPtyServer.py @@ -7,6 +7,7 @@ import xml.etree.ElementTree as ET +@skipIfRemote @skipIf(hostoslist=["windows"]) class PtyServerTestCase(gdbremote_testcase.GdbRemoteTestCaseBase): def setUp(self): ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed TestEchoCommands.test running on a remote target (PR #94127)
https://github.com/slydiman converted_to_draft https://github.com/llvm/llvm-project/pull/94127 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Disable find-module.test in case of a remote target (PR #94165)
https://github.com/slydiman converted_to_draft https://github.com/llvm/llvm-project/pull/94165 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed TestEchoCommands.test running on a remote target (PR #94127)
slydiman wrote: Note `EchoCommandsQuiet.out` expects that `TestEchoCommands.test.tmp.file` will be ``` start done ``` But in case of a remote target it will be something like ``` start Platform: remote-linux Connected: no Platform: remote-linux Triple: aarch64-unknown-linux-gnu OS Version: 5.10.192 (5.10.192-tegra) Hostname: jetson-orin-1795 Connected: yes WorkingDir: /home/ubuntu Kernel: #1 SMP PREEMPT Mon Feb 19 20:19:53 PST 2024 done ``` https://github.com/llvm/llvm-project/pull/94127 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed TestEchoCommands.test running on a remote target (PR #94127)
slydiman wrote: > How can I configure my build to run this sort of thing? The simplest way is to use the same arch target, e. g. x86_64 Linux host and x86_64 Linux target. You do not need a cross compiler in this case. To run Shell tests with a remote target it is necesasry - define LLVM_TARGET_TRIPLE=x86_64-unknown-linux-gnu - define LLDB_PLATFORM_URL=connect://:1234 - run lldv-server on the target `sudo //lldb-server p --listen '*:1234' --server --log-channels 'lldb all' --min-gdbserver-port 1240 --max-gdbserver-port 1250` - run `cmake --build . --target check-lldb-shell` from a build dir https://github.com/llvm/llvm-project/pull/94127 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits