[Lldb-commits] [PATCH] D153513: [lldb] Check that qLaunchGDBServer packet does not return an error
DavidSpickett created this revision.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
While looking at https://github.com/llvm/llvm-project/issues/61955
I noticed that when we send qLaunchGDBServer we check that we got a response
but not what kind of response it was.
I think this was why the bug reporter saw:
(lldb) run
error: invalid host:port specification: '[192.168.64.2]'
The missing port is because we went down a path we only should have
chosen if the operation succeeded. Since we didn't check, we went ahead
with an empty port number.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D153513
Files:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -2590,6 +2590,9 @@
if (SendPacketAndWaitForResponse(stream.GetString(), response) ==
PacketResult::Success) {
+if (response.IsErrorResponse())
+ return false;
+
llvm::StringRef name;
llvm::StringRef value;
while (response.GetNameColonValue(name, value)) {
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -2590,6 +2590,9 @@
if (SendPacketAndWaitForResponse(stream.GetString(), response) ==
PacketResult::Success) {
+if (response.IsErrorResponse())
+ return false;
+
llvm::StringRef name;
llvm::StringRef value;
while (response.GetNameColonValue(name, value)) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D153513: [lldb] Check that qLaunchGDBServer packet does not return an error
DavidSpickett added reviewers: JDevlieghere, jasonmolenda. DavidSpickett added a comment. I was about to say I don't have a way to test this but I think I might have now. Starting a platform, then moving lldb-server causes it to fail on Linux. Perhaps I can do that in an isolated way in a shell test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153513/new/ https://reviews.llvm.org/D153513 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D153513: [lldb] Check that qLaunchGDBServer packet does not return an error
DavidSpickett updated this revision to Diff 533534.
DavidSpickett added a comment.
Added a test from Python, which avoids a bunch of lit and Linux shell specific
issues.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D153513/new/
https://reviews.llvm.org/D153513
Files:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/test/API/commands/platform/launchgdbserver/Makefile
lldb/test/API/commands/platform/launchgdbserver/TestPlatformLaunchGDBServer.py
lldb/test/API/commands/platform/launchgdbserver/main.c
Index: lldb/test/API/commands/platform/launchgdbserver/main.c
===
--- /dev/null
+++ lldb/test/API/commands/platform/launchgdbserver/main.c
@@ -0,0 +1 @@
+int main() { return 0; }
Index:
lldb/test/API/commands/platform/launchgdbserver/TestPlatformLaunchGDBServer.py
===
--- /dev/null
+++
lldb/test/API/commands/platform/launchgdbserver/TestPlatformLaunchGDBServer.py
@@ -0,0 +1,58 @@
+""" Check that errors while handling qLaunchGDBServer are reported to the user.
+Though this isn't a platform command in itself, the best way to test it is
+from Python because we can juggle multiple processes more easily.
+"""
+
+import os
+import socket
+import shutil
+import lldbgdbserverutils
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestPlatformProcessLaunchGDBServer(TestBase):
+NO_DEBUG_INFO_TESTCASE = True
+
+@skipIfRemote
+# Windows doesn't use lldb-server and on Darwin we may be using
debugserver.
+@skipUnlessPlatform(["linux"])
+@add_test_categories(["lldb-server"])
+def test_platform_process_launch_gdb_server(self):
+self.build()
+
+hostname = socket.getaddrinfo("localhost", 0,
proto=socket.IPPROTO_TCP)[0][4][0]
+listen_url = "[%s]:0" % hostname
+
+port_file = self.getBuildArtifact("port")
+commandline_args = [
+"platform",
+"--listen",
+listen_url,
+"--socket-file",
+port_file,
+"--",
+self.getBuildArtifact("a.out"),
+"foo",
+]
+
+# Run lldb-server from a new location.
+new_lldb_server = self.getBuildArtifact("lldb-server")
+shutil.copy(lldbgdbserverutils.get_lldb_server_exe(), new_lldb_server)
+
+self.spawnSubprocess(new_lldb_server, commandline_args)
+socket_id = lldbutil.wait_for_file_on_target(self, port_file)
+
+# Remove our new lldb-server so that when it tries to invoke itself as
a
+# gdbserver, it fails.
+os.remove(new_lldb_server)
+
+new_platform = lldb.SBPlatform("remote-" + self.getPlatform())
+self.dbg.SetSelectedPlatform(new_platform)
+
+connect_url = "connect://[%s]:%s" % (hostname, socket_id)
+self.runCmd("platform connect %s" % connect_url)
+
+self.runCmd("target create {}".format(self.getBuildArtifact("a.out")))
+self.expect("run", substrs=["unable to launch a GDB server on"],
error=True)
Index: lldb/test/API/commands/platform/launchgdbserver/Makefile
===
--- /dev/null
+++ lldb/test/API/commands/platform/launchgdbserver/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -2590,6 +2590,9 @@
if (SendPacketAndWaitForResponse(stream.GetString(), response) ==
PacketResult::Success) {
+if (response.IsErrorResponse())
+ return false;
+
llvm::StringRef name;
llvm::StringRef value;
while (response.GetNameColonValue(name, value)) {
Index: lldb/test/API/commands/platform/launchgdbserver/main.c
===
--- /dev/null
+++ lldb/test/API/commands/platform/launchgdbserver/main.c
@@ -0,0 +1 @@
+int main() { return 0; }
Index: lldb/test/API/commands/platform/launchgdbserver/TestPlatformLaunchGDBServer.py
===
--- /dev/null
+++ lldb/test/API/commands/platform/launchgdbserver/TestPlatformLaunchGDBServer.py
@@ -0,0 +1,58 @@
+""" Check that errors while handling qLaunchGDBServer are reported to the user.
+Though this isn't a platform command in itself, the best way to test it is
+from Python because we can juggle multiple processes more easily.
+"""
+
+import os
+import socket
+import shutil
+import lldbgdbserverutils
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+f
[Lldb-commits] [PATCH] D153513: [lldb] Check that qLaunchGDBServer packet does not return an error
DavidSpickett added a comment. In the bug report the user didn't have a debugserver on the remote, but the handling is the same for lldb-server or debugserver. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153513/new/ https://reviews.llvm.org/D153513 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D153390: [lldb][Windows] Fix ZipFileResolver tests
clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Can we test this? I will ok to unblock windows bots. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153390/new/ https://reviews.llvm.org/D153390 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D153390: [lldb][Windows] Fix ZipFileResolver tests
splhack added a comment. Yes, in fact ZipFileResolverTest is the test what is running on Windows, and failing due to POSIX vs Windows path. This diff will fix the tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153390/new/ https://reviews.llvm.org/D153390 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D153390: [lldb][Windows] Fix ZipFileResolver tests
kusmour accepted this revision. kusmour added a comment. The fix looks straightforward. And @splhack has verified locally. Accepting Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153390/new/ https://reviews.llvm.org/D153390 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 5c4071d - [lldb][Windows] Fix ZipFileResolver tests
Author: Kazuki Sakamoto
Date: 2023-06-22T10:42:40-07:00
New Revision: 5c4071d225169d22387dd27df0f32a4efc8a0caa
URL:
https://github.com/llvm/llvm-project/commit/5c4071d225169d22387dd27df0f32a4efc8a0caa
DIFF:
https://github.com/llvm/llvm-project/commit/5c4071d225169d22387dd27df0f32a4efc8a0caa.diff
LOG: [lldb][Windows] Fix ZipFileResolver tests
D152759 introduced the Android .zip so file support, but it only considered
POSIX path. The code also runs on Windows, so the path could be Windows path.
Support both patterns on Windows.
Differential Revision: https://reviews.llvm.org/D153390
Added:
Modified:
lldb/source/Host/common/ZipFileResolver.cpp
Removed:
diff --git a/lldb/source/Host/common/ZipFileResolver.cpp
b/lldb/source/Host/common/ZipFileResolver.cpp
index abb914cb9ae42..f70ccb79d0896 100644
--- a/lldb/source/Host/common/ZipFileResolver.cpp
+++ b/lldb/source/Host/common/ZipFileResolver.cpp
@@ -25,6 +25,15 @@ bool ZipFileResolver::ResolveSharedLibraryPath(const
FileSpec &file_spec,
static constexpr llvm::StringLiteral k_zip_separator("!/");
std::string path(file_spec.GetPath());
size_t pos = path.find(k_zip_separator);
+
+#if defined(_WIN32)
+ // When the file_spec is resolved as a Windows path, the zip .so path will be
+ // "zip_path!\so_path". Support both patterns on Windows.
+ static constexpr llvm::StringLiteral k_zip_separator_win("!\\");
+ if (pos == std::string::npos)
+pos = path.find(k_zip_separator_win);
+#endif
+
if (pos == std::string::npos) {
// This file_spec does not contain the zip separator.
// Treat this file_spec as a normal file.
@@ -40,6 +49,12 @@ bool ZipFileResolver::ResolveSharedLibraryPath(const
FileSpec &file_spec,
std::string zip_path(path.substr(0, pos));
std::string so_path(path.substr(pos + k_zip_separator.size()));
+#if defined(_WIN32)
+ // Replace the .so path to use POSIX file separator for file searching inside
+ // the zip file.
+ std::replace(so_path.begin(), so_path.end(), '\\', '/');
+#endif
+
// Try to find the .so file from the zip file.
FileSpec zip_file_spec(zip_path);
uint64_t zip_file_size = FileSystem::Instance().GetByteSize(zip_file_spec);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D153390: [lldb][Windows] Fix ZipFileResolver tests
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5c4071d22516: [lldb][Windows] Fix ZipFileResolver tests
(authored by splhack).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D153390/new/
https://reviews.llvm.org/D153390
Files:
lldb/source/Host/common/ZipFileResolver.cpp
Index: lldb/source/Host/common/ZipFileResolver.cpp
===
--- lldb/source/Host/common/ZipFileResolver.cpp
+++ lldb/source/Host/common/ZipFileResolver.cpp
@@ -25,6 +25,15 @@
static constexpr llvm::StringLiteral k_zip_separator("!/");
std::string path(file_spec.GetPath());
size_t pos = path.find(k_zip_separator);
+
+#if defined(_WIN32)
+ // When the file_spec is resolved as a Windows path, the zip .so path will be
+ // "zip_path!\so_path". Support both patterns on Windows.
+ static constexpr llvm::StringLiteral k_zip_separator_win("!\\");
+ if (pos == std::string::npos)
+pos = path.find(k_zip_separator_win);
+#endif
+
if (pos == std::string::npos) {
// This file_spec does not contain the zip separator.
// Treat this file_spec as a normal file.
@@ -40,6 +49,12 @@
std::string zip_path(path.substr(0, pos));
std::string so_path(path.substr(pos + k_zip_separator.size()));
+#if defined(_WIN32)
+ // Replace the .so path to use POSIX file separator for file searching inside
+ // the zip file.
+ std::replace(so_path.begin(), so_path.end(), '\\', '/');
+#endif
+
// Try to find the .so file from the zip file.
FileSpec zip_file_spec(zip_path);
uint64_t zip_file_size = FileSystem::Instance().GetByteSize(zip_file_spec);
Index: lldb/source/Host/common/ZipFileResolver.cpp
===
--- lldb/source/Host/common/ZipFileResolver.cpp
+++ lldb/source/Host/common/ZipFileResolver.cpp
@@ -25,6 +25,15 @@
static constexpr llvm::StringLiteral k_zip_separator("!/");
std::string path(file_spec.GetPath());
size_t pos = path.find(k_zip_separator);
+
+#if defined(_WIN32)
+ // When the file_spec is resolved as a Windows path, the zip .so path will be
+ // "zip_path!\so_path". Support both patterns on Windows.
+ static constexpr llvm::StringLiteral k_zip_separator_win("!\\");
+ if (pos == std::string::npos)
+pos = path.find(k_zip_separator_win);
+#endif
+
if (pos == std::string::npos) {
// This file_spec does not contain the zip separator.
// Treat this file_spec as a normal file.
@@ -40,6 +49,12 @@
std::string zip_path(path.substr(0, pos));
std::string so_path(path.substr(pos + k_zip_separator.size()));
+#if defined(_WIN32)
+ // Replace the .so path to use POSIX file separator for file searching inside
+ // the zip file.
+ std::replace(so_path.begin(), so_path.end(), '\\', '/');
+#endif
+
// Try to find the .so file from the zip file.
FileSpec zip_file_spec(zip_path);
uint64_t zip_file_size = FileSystem::Instance().GetByteSize(zip_file_spec);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D153482: [lldb][NFCI] Remove use of ConstString from StructuredDataPlugin
clayborg added a comment. Looks fine to me. I will let the owner of StructuredDataDarwinLog do the final approval. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153482/new/ https://reviews.llvm.org/D153482 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] f9f279d - [lldb] Fix variable name mismatch between signature and docs (NFC)
Author: Jonas Devlieghere
Date: 2023-06-22T13:50:54-07:00
New Revision: f9f279dc64ac8366fc9095a30d4e0e33827d6bc1
URL:
https://github.com/llvm/llvm-project/commit/f9f279dc64ac8366fc9095a30d4e0e33827d6bc1
DIFF:
https://github.com/llvm/llvm-project/commit/f9f279dc64ac8366fc9095a30d4e0e33827d6bc1.diff
LOG: [lldb] Fix variable name mismatch between signature and docs (NFC)
The variable is named `bundle_dir` but the documentation referenced
`directory` which generated a warning.
Added:
Modified:
lldb/include/lldb/API/SBTrace.h
Removed:
diff --git a/lldb/include/lldb/API/SBTrace.h b/lldb/include/lldb/API/SBTrace.h
index f0c85eac0226d..ab8338b3f4242 100644
--- a/lldb/include/lldb/API/SBTrace.h
+++ b/lldb/include/lldb/API/SBTrace.h
@@ -48,7 +48,7 @@ class LLDB_API SBTrace {
/// \param[out] error
/// This will be set with an error in case of failures.
///
- /// \param[in] directory
+ /// \param[in] bundle_dir
/// The directory where the trace files will be saved.
///
/// \param[in] compact
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D153390: [lldb][Windows] Fix ZipFileResolver tests
splhack added a comment. verified, Windows buildbot is now green. https://lab.llvm.org/buildbot/#/builders/219 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153390/new/ https://reviews.llvm.org/D153390 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D153482: [lldb][NFCI] Remove use of ConstString from StructuredDataPlugin
JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153482/new/ https://reviews.llvm.org/D153482 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D153447: Creating a startDebugging reverse DAP request handler in lldb-vscode.
ashgti updated this revision to Diff 533804.
ashgti added a comment.
Enhancing the test coverage and fixing an issue when a reverse request is
invoked a request from a DAP request handler.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D153447/new/
https://reviews.llvm.org/D153447
Files:
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
lldb/test/API/tools/lldb-vscode/runInTerminal/TestVSCode_runInTerminal.py
lldb/test/API/tools/lldb-vscode/startDebugging/Makefile
lldb/test/API/tools/lldb-vscode/startDebugging/TestVSCode_startDebugging.py
lldb/test/API/tools/lldb-vscode/startDebugging/main.c
lldb/tools/lldb-vscode/JSONUtils.cpp
lldb/tools/lldb-vscode/README.md
lldb/tools/lldb-vscode/VSCode.cpp
lldb/tools/lldb-vscode/VSCode.h
lldb/tools/lldb-vscode/lldb-vscode.cpp
Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -1471,6 +1471,13 @@
g_vsc.debugger =
lldb::SBDebugger::Create(source_init_file, log_cb, nullptr);
+ auto cmd = g_vsc.debugger.GetCommandInterpreter().AddMultiwordCommand(
+ "lldb-vscode", nullptr);
+ cmd.AddCommand(
+ "startDebugging", &g_vsc.start_debugging_request_handler,
+ "Sends a startDebugging request from the debug adapter to the client to "
+ "start a child debug session of the same type as the caller.");
+
g_vsc.progress_event_thread = std::thread(ProgressEventThreadFunction);
// Start our event thread so we can receive events from the debugger, target,
@@ -1564,7 +1571,8 @@
g_vsc.SendJSON(llvm::json::Value(std::move(response)));
}
-llvm::Error request_runInTerminal(const llvm::json::Object &launch_request) {
+llvm::Error request_runInTerminal(const llvm::json::Object &launch_request,
+ const uint64_t timeout_seconds) {
g_vsc.is_attach = true;
lldb::SBAttachInfo attach_info;
@@ -1582,13 +1590,15 @@
#endif
llvm::json::Object reverse_request = CreateRunInTerminalReverseRequest(
launch_request, g_vsc.debug_adaptor_path, comm_file.m_path, debugger_pid);
- llvm::json::Object reverse_response;
- lldb_vscode::PacketStatus status =
- g_vsc.SendReverseRequest(reverse_request, reverse_response);
- if (status != lldb_vscode::PacketStatus::Success)
-return llvm::createStringError(llvm::inconvertibleErrorCode(),
- "Process cannot be launched by the IDE. %s",
- comm_channel.GetLauncherError().c_str());
+ g_vsc.SendReverseRequest("runInTerminal", std::move(reverse_request),
+ [](llvm::Expected value) {
+ if (!value) {
+ llvm::Error err = value.takeError();
+ llvm::errs()
+ << "runInTerminal request failed: "
+ << llvm::toString(std::move(err)) << "\n";
+ }
+ });
if (llvm::Expected pid = comm_channel.GetLauncherPid())
attach_info.SetProcessID(*pid);
@@ -1676,7 +1686,7 @@
const uint64_t timeout_seconds = GetUnsigned(arguments, "timeout", 30);
if (GetBoolean(arguments, "runInTerminal", false)) {
-if (llvm::Error err = request_runInTerminal(request))
+if (llvm::Error err = request_runInTerminal(request, timeout_seconds))
error.SetErrorString(llvm::toString(std::move(err)).c_str());
} else if (launchCommands.empty()) {
// Disable async events so the launch will be successful when we return from
@@ -3464,17 +3474,13 @@
g_vsc.output.descriptor = StreamDescriptor::from_file(new_stdout_fd, false);
}
- while (!g_vsc.sent_terminated_event) {
-llvm::json::Object object;
-lldb_vscode::PacketStatus status = g_vsc.GetNextObject(object);
-if (status == lldb_vscode::PacketStatus::EndOfFile)
- break;
-if (status != lldb_vscode::PacketStatus::Success)
- return 1; // Fatal error
-
-if (!g_vsc.HandleObject(object))
- return 1;
+ bool CleanExit = true;
+ if (auto Err = g_vsc.Loop()) {
+if (g_vsc.log)
+ *g_vsc.log << "Transport Error: " << llvm::toString(std::move(Err))
+ << "\n";
+CleanExit = false;
}
- return EXIT_SUCCESS;
+ return CleanExit ? EXIT_SUCCESS : EXIT_FAILURE;
}
Index: lldb/tools/lldb-vscode/VSCode.h
===
--- lldb/tools/lldb-vscode/VSCode.h
+++ lldb/tools/lldb-vscode/VSCode.h
@@ -11,8 +11,10 @@
#include "llvm/Config/llvm-config.h" // for LLVM_ON_UNIX
+#include
#include
#include
+#include
#include
#include
#include
@@ -73,6 +75,7 @@
};
typedef void (*RequestCallback)(const llvm::json::Object &command);
+typedef void (*ResponseCallback)(llvm::Expected value);
en
[Lldb-commits] [PATCH] D153447: Creating a startDebugging reverse DAP request handler in lldb-vscode.
ashgti updated this revision to Diff 533806.
ashgti added a comment.
Cleaning up the runInTerminal test.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D153447/new/
https://reviews.llvm.org/D153447
Files:
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
lldb/test/API/tools/lldb-vscode/runInTerminal/TestVSCode_runInTerminal.py
lldb/test/API/tools/lldb-vscode/startDebugging/Makefile
lldb/test/API/tools/lldb-vscode/startDebugging/TestVSCode_startDebugging.py
lldb/test/API/tools/lldb-vscode/startDebugging/main.c
lldb/tools/lldb-vscode/JSONUtils.cpp
lldb/tools/lldb-vscode/README.md
lldb/tools/lldb-vscode/VSCode.cpp
lldb/tools/lldb-vscode/VSCode.h
lldb/tools/lldb-vscode/lldb-vscode.cpp
Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -1471,6 +1471,13 @@
g_vsc.debugger =
lldb::SBDebugger::Create(source_init_file, log_cb, nullptr);
+ auto cmd = g_vsc.debugger.GetCommandInterpreter().AddMultiwordCommand(
+ "lldb-vscode", nullptr);
+ cmd.AddCommand(
+ "startDebugging", &g_vsc.start_debugging_request_handler,
+ "Sends a startDebugging request from the debug adapter to the client to "
+ "start a child debug session of the same type as the caller.");
+
g_vsc.progress_event_thread = std::thread(ProgressEventThreadFunction);
// Start our event thread so we can receive events from the debugger, target,
@@ -1564,7 +1571,8 @@
g_vsc.SendJSON(llvm::json::Value(std::move(response)));
}
-llvm::Error request_runInTerminal(const llvm::json::Object &launch_request) {
+llvm::Error request_runInTerminal(const llvm::json::Object &launch_request,
+ const uint64_t timeout_seconds) {
g_vsc.is_attach = true;
lldb::SBAttachInfo attach_info;
@@ -1582,13 +1590,15 @@
#endif
llvm::json::Object reverse_request = CreateRunInTerminalReverseRequest(
launch_request, g_vsc.debug_adaptor_path, comm_file.m_path, debugger_pid);
- llvm::json::Object reverse_response;
- lldb_vscode::PacketStatus status =
- g_vsc.SendReverseRequest(reverse_request, reverse_response);
- if (status != lldb_vscode::PacketStatus::Success)
-return llvm::createStringError(llvm::inconvertibleErrorCode(),
- "Process cannot be launched by the IDE. %s",
- comm_channel.GetLauncherError().c_str());
+ g_vsc.SendReverseRequest("runInTerminal", std::move(reverse_request),
+ [](llvm::Expected value) {
+ if (!value) {
+ llvm::Error err = value.takeError();
+ llvm::errs()
+ << "runInTerminal request failed: "
+ << llvm::toString(std::move(err)) << "\n";
+ }
+ });
if (llvm::Expected pid = comm_channel.GetLauncherPid())
attach_info.SetProcessID(*pid);
@@ -1676,7 +1686,7 @@
const uint64_t timeout_seconds = GetUnsigned(arguments, "timeout", 30);
if (GetBoolean(arguments, "runInTerminal", false)) {
-if (llvm::Error err = request_runInTerminal(request))
+if (llvm::Error err = request_runInTerminal(request, timeout_seconds))
error.SetErrorString(llvm::toString(std::move(err)).c_str());
} else if (launchCommands.empty()) {
// Disable async events so the launch will be successful when we return from
@@ -3464,17 +3474,13 @@
g_vsc.output.descriptor = StreamDescriptor::from_file(new_stdout_fd, false);
}
- while (!g_vsc.sent_terminated_event) {
-llvm::json::Object object;
-lldb_vscode::PacketStatus status = g_vsc.GetNextObject(object);
-if (status == lldb_vscode::PacketStatus::EndOfFile)
- break;
-if (status != lldb_vscode::PacketStatus::Success)
- return 1; // Fatal error
-
-if (!g_vsc.HandleObject(object))
- return 1;
+ bool CleanExit = true;
+ if (auto Err = g_vsc.Loop()) {
+if (g_vsc.log)
+ *g_vsc.log << "Transport Error: " << llvm::toString(std::move(Err))
+ << "\n";
+CleanExit = false;
}
- return EXIT_SUCCESS;
+ return CleanExit ? EXIT_SUCCESS : EXIT_FAILURE;
}
Index: lldb/tools/lldb-vscode/VSCode.h
===
--- lldb/tools/lldb-vscode/VSCode.h
+++ lldb/tools/lldb-vscode/VSCode.h
@@ -11,8 +11,10 @@
#include "llvm/Config/llvm-config.h" // for LLVM_ON_UNIX
+#include
#include
#include
+#include
#include
#include
#include
@@ -73,6 +75,7 @@
};
typedef void (*RequestCallback)(const llvm::json::Object &command);
+typedef void (*ResponseCallback)(llvm::Expected value);
enum class PacketStatus {
Success = 0,
@@ -121,6 +124,11 @@
void Clear();
};
+
[Lldb-commits] [PATCH] D153447: Creating a startDebugging reverse DAP request handler in lldb-vscode.
ashgti requested review of this revision. ashgti added a comment. Hi @wallace I updated the tests and discovered a deadlock with my implementation of the SendReverseRequest. I refactored the function and added some additional tests to cover this more thoroughly. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153447/new/ https://reviews.llvm.org/D153447 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D153597: [lldb] Adjust for changes in objc runtime
bulbazord created this revision.
bulbazord added a reviewer: JDevlieghere.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
The Objective-C runtime and the shared cache has changed slightly.
Given a class_ro_t, the baseMethods ivar is now a pointer union and may
either be a method_list_t pointer or a pointer to a relative list of
lists. The entries of this relative list of lists are indexes that refer
to a specific image in the shared cache in addition to a pointer offset
to find the accompanying method_list_t. We have to go over each of these
entries, parse it, and then if the relevant image is loaded in the
process, we add those methods to the relevant clang Decl.
In order to determine if an image is loaded, the Objective-C runtime
exposes a symbol that lets us determine if a particular image is loaded.
We maintain a data structure SharedCacheImageHeaders to keep track of
that information.
There is a known issue where if an image is loaded after we create a
Decl for a class, the Decl will not have the relevant methods from that
image (i.e. for Categories).
rdar://107957209
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D153597
Files:
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
lldb/test/API/lang/objc/exceptions/TestObjCExceptions.py
Index: lldb/test/API/lang/objc/exceptions/TestObjCExceptions.py
===
--- lldb/test/API/lang/objc/exceptions/TestObjCExceptions.py
+++ lldb/test/API/lang/objc/exceptions/TestObjCExceptions.py
@@ -87,7 +87,6 @@
"userInfo = ",
"1 key/value pair",
"reserved = ",
-"nil",
],
)
@@ -105,7 +104,6 @@
self.assertEqual(
userInfo.GetChildAtIndex(0).GetChildAtIndex(1).description, "some_value"
)
-self.assertEqual(e1.GetChildMemberWithName("reserved").description, "")
self.expect(
"frame variable e2", substrs=["(NSException *) e2 = ", '"SomeReason"']
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
@@ -19,6 +19,8 @@
#include "Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h"
+#include "llvm/ADT/BitVector.h"
+
class RemoteNXMapTable;
namespace lldb_private {
@@ -96,6 +98,12 @@
void GetValuesForGlobalCFBooleans(lldb::addr_t &cf_true,
lldb::addr_t &cf_false) override;
+ void ModulesDidLoad(const ModuleList &module_list) override;
+
+ bool IsSharedCacheImageLoaded(uint16_t image_index);
+
+ std::optional GetSharedCacheImageHeaderVersion();
+
protected:
lldb::BreakpointResolverSP
CreateExceptionResolver(const lldb::BreakpointSP &bkpt, bool catch_bp,
@@ -374,6 +382,35 @@
lldb::addr_t m_args = LLDB_INVALID_ADDRESS;
};
+ class SharedCacheImageHeaders {
+ public:
+static std::unique_ptr
+CreateSharedCacheImageHeaders(AppleObjCRuntimeV2 &runtime);
+
+void SetNeedsUpdate() { m_needs_update = true; }
+
+bool IsImageLoaded(uint16_t image_index);
+
+uint64_t GetVersion();
+
+ private:
+SharedCacheImageHeaders(AppleObjCRuntimeV2 &runtime,
+lldb::addr_t headerInfoRWs_ptr, uint32_t count,
+uint32_t entsize)
+: m_runtime(runtime), m_headerInfoRWs_ptr(headerInfoRWs_ptr),
+ m_loaded_images(count, false), m_version(0), m_count(count),
+ m_entsize(entsize), m_needs_update(true) {}
+llvm::Error UpdateIfNeeded();
+
+AppleObjCRuntimeV2 &m_runtime;
+lldb::addr_t m_headerInfoRWs_ptr;
+llvm::BitVector m_loaded_images;
+uint64_t m_version;
+uint32_t m_count;
+uint32_t m_entsize;
+bool m_needs_update;
+ };
+
AppleObjCRuntimeV2(Process *process, const lldb::ModuleSP &objc_module_sp);
ObjCISA GetPointerISA(ObjCISA isa);
@@ -435,6 +472,7 @@
std::once_flag m_no_expanded_cache_warning;
std::optional> m_CFBoolean_values;
uint64_t m_realized_class_generation_count;
+ std::unique_ptr m_shared_cache_image_headers_up;
};
} // namespace lldb_private
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ lldb/source/
[Lldb-commits] [PATCH] D153597: [lldb] Adjust for changes in objc runtime
JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM. This has been previously reviewed internally. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153597/new/ https://reviews.llvm.org/D153597 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 28fb39f - [lldb] Adjust for changes in objc runtime
Author: Alex Langford
Date: 2023-06-22T16:42:22-07:00
New Revision: 28fb39f16af1003e53008b75c11127b3288742f8
URL:
https://github.com/llvm/llvm-project/commit/28fb39f16af1003e53008b75c11127b3288742f8
DIFF:
https://github.com/llvm/llvm-project/commit/28fb39f16af1003e53008b75c11127b3288742f8.diff
LOG: [lldb] Adjust for changes in objc runtime
The Objective-C runtime and the shared cache has changed slightly.
Given a class_ro_t, the baseMethods ivar is now a pointer union and may
either be a method_list_t pointer or a pointer to a relative list of
lists. The entries of this relative list of lists are indexes that refer
to a specific image in the shared cache in addition to a pointer offset
to find the accompanying method_list_t. We have to go over each of these
entries, parse it, and then if the relevant image is loaded in the
process, we add those methods to the relevant clang Decl.
In order to determine if an image is loaded, the Objective-C runtime
exposes a symbol that lets us determine if a particular image is loaded.
We maintain a data structure SharedCacheImageHeaders to keep track of
that information.
There is a known issue where if an image is loaded after we create a
Decl for a class, the Decl will not have the relevant methods from that
image (i.e. for Categories).
rdar://107957209
Differential Revision: https://reviews.llvm.org/D153597
Added:
Modified:
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
lldb/test/API/lang/objc/exceptions/TestObjCExceptions.py
Removed:
diff --git
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
index 438842db4eec5..80ba352228c54 100644
---
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
+++
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
@@ -370,6 +370,155 @@ bool ClassDescriptorV2::ivar_t::Read(Process *process,
lldb::addr_t addr) {
return !error.Fail();
}
+bool ClassDescriptorV2::relative_list_entry_t::Read(Process *process,
+lldb::addr_t addr) {
+ Log *log = GetLog(LLDBLog::Types);
+ size_t size = sizeof(uint64_t); // m_image_index : 16
+ // m_list_offset : 48
+
+ DataBufferHeap buffer(size, '\0');
+ Status error;
+
+ process->ReadMemory(addr, buffer.GetBytes(), size, error);
+ // FIXME: Propagate this error up
+ if (error.Fail()) {
+LLDB_LOG(log, "Failed to read relative_list_entry_t at address {0:x}",
+ addr);
+return false;
+ }
+
+ DataExtractor extractor(buffer.GetBytes(), size, process->GetByteOrder(),
+ process->GetAddressByteSize());
+ lldb::offset_t cursor = 0;
+ uint64_t raw_entry = extractor.GetU64_unchecked(&cursor);
+ m_image_index = raw_entry & 0x;
+ m_list_offset = (int64_t)(raw_entry >> 16);
+ return true;
+}
+
+bool ClassDescriptorV2::relative_list_list_t::Read(Process *process,
+ lldb::addr_t addr) {
+ Log *log = GetLog(LLDBLog::Types);
+ size_t size = sizeof(uint32_t)// m_entsize
++ sizeof(uint32_t); // m_count
+
+ DataBufferHeap buffer(size, '\0');
+ Status error;
+
+ // FIXME: Propagate this error up
+ process->ReadMemory(addr, buffer.GetBytes(), size, error);
+ if (error.Fail()) {
+LLDB_LOG(log, "Failed to read relative_list_list_t at address 0x" PRIx64,
+ addr);
+return false;
+ }
+
+ DataExtractor extractor(buffer.GetBytes(), size, process->GetByteOrder(),
+ process->GetAddressByteSize());
+ lldb::offset_t cursor = 0;
+ m_entsize = extractor.GetU32_unchecked(&cursor);
+ m_count = extractor.GetU32_unchecked(&cursor);
+ m_first_ptr = addr + cursor;
+ return true;
+}
+
+std::optional
+ClassDescriptorV2::GetMethodList(Process *process,
+ lldb::addr_t method_list_ptr) const {
+ Log *log = GetLog(LLDBLog::Types);
+ ClassDescriptorV2::method_list_t method_list;
+ if (!method_list.Read(process, method_list_ptr))
+return std::nullopt;
+
+ const size_t method_size = method_t::GetSize(process,
method_list.m_is_small);
+ if (method_list.m_entsize != method_size) {
+LLDB_LOG(log,
+ "method_list_t at address 0x" PRIx64 " has an entsize of " PRIu16
+ " but method size should be " PRIu64,
+ method_list_ptr, method_list.m_entsize, method_size);
+return std::nullopt
[Lldb-commits] [PATCH] D153597: [lldb] Adjust for changes in objc runtime
This revision was automatically updated to reflect the committed changes.
Closed by commit rG28fb39f16af1: [lldb] Adjust for changes in objc runtime
(authored by bulbazord).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D153597/new/
https://reviews.llvm.org/D153597
Files:
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
lldb/test/API/lang/objc/exceptions/TestObjCExceptions.py
Index: lldb/test/API/lang/objc/exceptions/TestObjCExceptions.py
===
--- lldb/test/API/lang/objc/exceptions/TestObjCExceptions.py
+++ lldb/test/API/lang/objc/exceptions/TestObjCExceptions.py
@@ -87,7 +87,6 @@
"userInfo = ",
"1 key/value pair",
"reserved = ",
-"nil",
],
)
@@ -105,7 +104,6 @@
self.assertEqual(
userInfo.GetChildAtIndex(0).GetChildAtIndex(1).description, "some_value"
)
-self.assertEqual(e1.GetChildMemberWithName("reserved").description, "")
self.expect(
"frame variable e2", substrs=["(NSException *) e2 = ", '"SomeReason"']
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
@@ -19,6 +19,8 @@
#include "Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h"
+#include "llvm/ADT/BitVector.h"
+
class RemoteNXMapTable;
namespace lldb_private {
@@ -96,6 +98,12 @@
void GetValuesForGlobalCFBooleans(lldb::addr_t &cf_true,
lldb::addr_t &cf_false) override;
+ void ModulesDidLoad(const ModuleList &module_list) override;
+
+ bool IsSharedCacheImageLoaded(uint16_t image_index);
+
+ std::optional GetSharedCacheImageHeaderVersion();
+
protected:
lldb::BreakpointResolverSP
CreateExceptionResolver(const lldb::BreakpointSP &bkpt, bool catch_bp,
@@ -374,6 +382,35 @@
lldb::addr_t m_args = LLDB_INVALID_ADDRESS;
};
+ class SharedCacheImageHeaders {
+ public:
+static std::unique_ptr
+CreateSharedCacheImageHeaders(AppleObjCRuntimeV2 &runtime);
+
+void SetNeedsUpdate() { m_needs_update = true; }
+
+bool IsImageLoaded(uint16_t image_index);
+
+uint64_t GetVersion();
+
+ private:
+SharedCacheImageHeaders(AppleObjCRuntimeV2 &runtime,
+lldb::addr_t headerInfoRWs_ptr, uint32_t count,
+uint32_t entsize)
+: m_runtime(runtime), m_headerInfoRWs_ptr(headerInfoRWs_ptr),
+ m_loaded_images(count, false), m_version(0), m_count(count),
+ m_entsize(entsize), m_needs_update(true) {}
+llvm::Error UpdateIfNeeded();
+
+AppleObjCRuntimeV2 &m_runtime;
+lldb::addr_t m_headerInfoRWs_ptr;
+llvm::BitVector m_loaded_images;
+uint64_t m_version;
+uint32_t m_count;
+uint32_t m_entsize;
+bool m_needs_update;
+ };
+
AppleObjCRuntimeV2(Process *process, const lldb::ModuleSP &objc_module_sp);
ObjCISA GetPointerISA(ObjCISA isa);
@@ -435,6 +472,7 @@
std::once_flag m_no_expanded_cache_warning;
std::optional> m_CFBoolean_values;
uint64_t m_realized_class_generation_count;
+ std::unique_ptr m_shared_cache_image_headers_up;
};
} // namespace lldb_private
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -1619,6 +1619,146 @@
return m_isa_hash_table_ptr;
}
+std::unique_ptr
+AppleObjCRuntimeV2::SharedCacheImageHeaders::CreateSharedCacheImageHeaders(
+AppleObjCRuntimeV2 &runtime) {
+ Log *log = GetLog(LLDBLog::Process | LLDBLog::Types);
+ Process *process = runtime.GetProcess();
+ ModuleSP objc_module_sp(runtime.GetObjCModule());
+ if (!objc_module_sp || !process)
+return nullptr;
+
+ const Symbol *symbol = objc_module_sp->FindFirstSymbolWithNameAndType(
+ ConstString("objc_debug_headerInfoRWs"), lldb::eSymbolTypeAny);
+ if (!symbol) {
+LLDB_LOG(log, "Symbol 'objc_debug_headerInfoRWs' unavailable. Some "
+ "information concerning the shared cache may be unavailable");
+return nullptr;
+ }
+
+ lldb::addr_t objc_debug_headerInfoRWs_addr =
+ symbol->GetLoadAddress(&process->GetTarget());
+ if (objc_debug_headerInfoR
