JDevlieghere updated this revision to Diff 277609.
JDevlieghere added a comment.

Address Jim's feedback


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83728/new/

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
@@ -12,9 +12,6 @@
 #include <memory>
 #include <vector>
 
-#include "llvm/Support/FileSystem.h"
-#include "llvm/Support/Path.h"
-
 #include "lldb/Breakpoint/BreakpointIDList.h"
 #include "lldb/Breakpoint/BreakpointLocation.h"
 #include "lldb/Core/Debugger.h"
@@ -40,8 +37,8 @@
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/Utility/StructuredData.h"
-
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Path.h"
 
 // Define these constants from POSIX mman.h rather than include the file so
 // that they will be correct even when compiled on Linux.
@@ -1774,9 +1771,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,12 +1814,34 @@
 
   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())
+  if (error.Fail()) {
+    if (synchronous)
+      process_sp->RestoreProcessEvents();
     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,12 @@
   }
 
 protected:
+  /// Private implementation of connecting to a process. If the stream is set
+  /// we connect synchronously.
+  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

Reply via email to