JDevlieghere created this revision.
JDevlieghere added reviewers: labath, jingham.
Herald added a subscriber: abidh.
In synchronous mode, the `process connect` command and its aliases should wait
for the stop event before claiming the command is complete. Currently, the stop
event is always handled asynchronously by the debugger. The implementation
takes the same approach as `Process::ResumeSynchronous` which hijacks the event
and handles it on the current thread. Similarly, after this patch, the stop
event is part of the command return object, which is the property used by the
test case.
Most of the discussion for this patch took place in D83446
<https://reviews.llvm.org/D83446>
Repository:
rLLDB LLDB
https://reviews.llvm.org/D83728
Files:
lldb/include/lldb/Target/Platform.h
lldb/source/Commands/CommandObjectProcess.cpp
lldb/source/Target/Platform.cpp
lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
Index: lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
===================================================================
--- /dev/null
+++ lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
@@ -0,0 +1,89 @@
+import lldb
+import binascii
+import os
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from gdbclientutils import *
+
+
+def hexlify(string):
+ return binascii.hexlify(string.encode()).decode()
+
+
+class ProcesConnectResponder(MockGDBServerResponder):
+ def __init__(self):
+ MockGDBServerResponder.__init__(self)
+ self.currentQsProc = 0
+ self.all_users = False
+
+ def qfProcessInfo(self, packet):
+ if "all_users:1" in packet:
+ self.all_users = True
+ name = hexlify("/a/test_process")
+ args = "-".join(
+ map(hexlify,
+ ["/system/bin/sh", "-c", "/data/local/tmp/lldb-server"]))
+ return "pid:10;ppid:1;uid:2;gid:3;euid:4;egid:5;name:" + name + ";args:" + args + ";"
+ else:
+ self.all_users = False
+ return "E04"
+
+ def qsProcessInfo(self):
+ if self.all_users:
+ if self.currentQsProc == 0:
+ self.currentQsProc = 1
+ name = hexlify("/b/another_test_process")
+ # This intentionally has a badly encoded argument
+ args = "X".join(map(hexlify, ["/system/bin/ls", "--help"]))
+ return "pid:11;ppid:2;uid:3;gid:4;euid:5;egid:6;name:" + name + ";args:" + args + ";"
+ elif self.currentQsProc == 1:
+ self.currentQsProc = 0
+ return "E04"
+ else:
+ return "E04"
+
+
+class TestProcesConnect(GDBRemoteTestBase):
+ def test_gdb_remote_sync(self):
+ """Test the gdb-remote command in synchronous mode"""
+ self.server.responder = ProcesConnectResponder()
+ try:
+ self.dbg.SetAsync(False)
+ self.expect("gdb-remote %d" % self.server.port,
+ substrs=['Process', 'stopped'])
+ finally:
+ self.dbg.GetSelectedPlatform().DisconnectRemote()
+
+ def test_gdb_remote_async(self):
+ """Test the gdb-remote command in asynchronous mode"""
+ self.server.responder = ProcesConnectResponder()
+ try:
+ self.dbg.SetAsync(True)
+ self.expect("gdb-remote %d" % self.server.port,
+ matching=False,
+ substrs=['Process', 'stopped'])
+ finally:
+ self.dbg.GetSelectedPlatform().DisconnectRemote()
+
+ def test_process_connect_sync(self):
+ """Test the gdb-remote command in synchronous mode"""
+ self.server.responder = ProcesConnectResponder()
+ try:
+ self.dbg.SetAsync(False)
+ self.expect("process connect connect://localhost:%d" %
+ self.server.port,
+ substrs=['Process', 'stopped'])
+ finally:
+ self.dbg.GetSelectedPlatform().DisconnectRemote()
+
+ def test_process_connect_async(self):
+ """Test the gdb-remote command in asynchronous mode"""
+ self.server.responder = ProcesConnectResponder()
+ try:
+ self.dbg.SetAsync(True)
+ self.expect("process connect connect://localhost:%d" %
+ self.server.port,
+ matching=False,
+ substrs=['Process', 'stopped'])
+ finally:
+ self.dbg.GetSelectedPlatform().DisconnectRemote()
Index: lldb/source/Target/Platform.cpp
===================================================================
--- lldb/source/Target/Platform.cpp
+++ lldb/source/Target/Platform.cpp
@@ -1774,9 +1774,23 @@
lldb::ProcessSP Platform::ConnectProcess(llvm::StringRef connect_url,
llvm::StringRef plugin_name,
- lldb_private::Debugger &debugger,
- lldb_private::Target *target,
- lldb_private::Status &error) {
+ Debugger &debugger, Target *target,
+ Status &error) {
+ return DoConnectProcess(connect_url, plugin_name, debugger, nullptr, target,
+ error);
+}
+
+lldb::ProcessSP Platform::ConnectProcessSynchronous(
+ llvm::StringRef connect_url, llvm::StringRef plugin_name,
+ Debugger &debugger, Stream &stream, Target *target, Status &error) {
+ return DoConnectProcess(connect_url, plugin_name, debugger, &stream, target,
+ error);
+}
+
+lldb::ProcessSP Platform::DoConnectProcess(llvm::StringRef connect_url,
+ llvm::StringRef plugin_name,
+ Debugger &debugger, Stream *stream,
+ Target *target, Status &error) {
error.Clear();
if (!target) {
@@ -1803,13 +1817,32 @@
lldb::ProcessSP process_sp =
target->CreateProcess(debugger.GetListener(), plugin_name, nullptr);
+
if (!process_sp)
return nullptr;
+ // If this private method is called with a stream we are synchronous.
+ const bool synchronous = stream != nullptr;
+
+ ListenerSP listener_sp(
+ Listener::MakeListener("lldb.Process.ConnectProcess.hijack"));
+ if (synchronous)
+ process_sp->HijackProcessEvents(listener_sp);
+
error = process_sp->ConnectRemote(connect_url);
if (error.Fail())
return nullptr;
+ if (synchronous) {
+ EventSP event_sp;
+ process_sp->WaitForProcessToStop(llvm::None, &event_sp, true, listener_sp,
+ nullptr);
+ process_sp->RestoreProcessEvents();
+ bool pop_process_io_handler = false;
+ Process::HandleProcessStateChangedEvent(event_sp, stream,
+ pop_process_io_handler);
+ }
+
return process_sp;
}
Index: lldb/source/Commands/CommandObjectProcess.cpp
===================================================================
--- lldb/source/Commands/CommandObjectProcess.cpp
+++ lldb/source/Commands/CommandObjectProcess.cpp
@@ -820,9 +820,15 @@
Status error;
Debugger &debugger = GetDebugger();
PlatformSP platform_sp = m_interpreter.GetPlatform(true);
- ProcessSP process_sp = platform_sp->ConnectProcess(
- command.GetArgumentAtIndex(0), plugin_name, debugger,
- debugger.GetSelectedTarget().get(), error);
+ ProcessSP process_sp =
+ debugger.GetAsyncExecution()
+ ? platform_sp->ConnectProcess(
+ command.GetArgumentAtIndex(0), plugin_name, debugger,
+ debugger.GetSelectedTarget().get(), error)
+ : platform_sp->ConnectProcessSynchronous(
+ command.GetArgumentAtIndex(0), plugin_name, debugger,
+ result.GetOutputStream(), debugger.GetSelectedTarget().get(),
+ error);
if (error.Fail() || process_sp == nullptr) {
result.AppendError(error.AsCString("Error connecting to the process"));
result.SetStatus(eReturnStatusFailed);
Index: lldb/include/lldb/Target/Platform.h
===================================================================
--- lldb/include/lldb/Target/Platform.h
+++ lldb/include/lldb/Target/Platform.h
@@ -372,9 +372,13 @@
virtual lldb::ProcessSP ConnectProcess(llvm::StringRef connect_url,
llvm::StringRef plugin_name,
- lldb_private::Debugger &debugger,
- lldb_private::Target *target,
- lldb_private::Status &error);
+ Debugger &debugger, Target *target,
+ Status &error);
+
+ virtual lldb::ProcessSP
+ ConnectProcessSynchronous(llvm::StringRef connect_url,
+ llvm::StringRef plugin_name, Debugger &debugger,
+ Stream &stream, Target *target, Status &error);
/// Attach to an existing process using a process ID.
///
@@ -848,6 +852,10 @@
}
protected:
+ lldb::ProcessSP DoConnectProcess(llvm::StringRef connect_url,
+ llvm::StringRef plugin_name,
+ Debugger &debugger, Stream *stream,
+ Target *target, Status &error);
bool m_is_host;
// Set to true when we are able to actually set the OS version while being
// connected. For remote platforms, we might set the version ahead of time
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
