[Lldb-commits] [lldb] 706cccb - [lldb] Make `process connect` blocking in synchronous mode.

Jonas Devlieghere via lldb-commits lldb-commits at lists.llvm.org
Tue Jul 14 08:45:41 PDT 2020


Author: Jonas Devlieghere
Date: 2020-07-14T08:45:34-07:00
New Revision: 706cccb889c8d14791c029a7bb69d8eddb6b1728

URL: https://github.com/llvm/llvm-project/commit/706cccb889c8d14791c029a7bb69d8eddb6b1728
DIFF: https://github.com/llvm/llvm-project/commit/706cccb889c8d14791c029a7bb69d8eddb6b1728.diff

LOG: [lldb] Make `process connect` blocking in synchronous mode.

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.

Differential revision: https://reviews.llvm.org/D83728

Added: 
    lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py

Modified: 
    lldb/include/lldb/Target/Platform.h
    lldb/source/Commands/CommandObjectProcess.cpp
    lldb/source/Target/Platform.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Target/Platform.h b/lldb/include/lldb/Target/Platform.h
index 52696f131f82..6234b8244b3f 100644
--- a/lldb/include/lldb/Target/Platform.h
+++ b/lldb/include/lldb/Target/Platform.h
@@ -372,9 +372,13 @@ class Platform : public PluginInterface {
 
   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 @@ class Platform : public PluginInterface {
   }
 
 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

diff  --git a/lldb/source/Commands/CommandObjectProcess.cpp b/lldb/source/Commands/CommandObjectProcess.cpp
index 3659f0db832c..f86779d85b5f 100644
--- a/lldb/source/Commands/CommandObjectProcess.cpp
+++ b/lldb/source/Commands/CommandObjectProcess.cpp
@@ -820,9 +820,15 @@ class CommandObjectProcessConnect : public CommandObjectParsed {
     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);

diff  --git a/lldb/source/Target/Platform.cpp b/lldb/source/Target/Platform.cpp
index 95c35ea826a0..16787141bee0 100644
--- a/lldb/source/Target/Platform.cpp
+++ b/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 @@ Status Platform::UnloadImage(lldb_private::Process *process,
 
 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 Platform::ConnectProcess(llvm::StringRef connect_url,
 
   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;
 }

diff  --git a/lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py b/lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
new file mode 100644
index 000000000000..34ae8d08004d
--- /dev/null
+++ b/lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
@@ -0,0 +1,52 @@
+import lldb
+import binascii
+import os
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from gdbclientutils import *
+
+
+class TestProcessConnect(GDBRemoteTestBase):
+    def test_gdb_remote_sync(self):
+        """Test the gdb-remote command in synchronous mode"""
+        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"""
+        try:
+            self.dbg.SetAsync(True)
+            self.expect("gdb-remote %d" % self.server.port,
+                        matching=False,
+                        substrs=['Process', 'stopped'])
+            lldbutil.expect_state_changes(self, self.dbg.GetListener(),
+                                          self.process(), [lldb.eStateStopped])
+        finally:
+            self.dbg.GetSelectedPlatform().DisconnectRemote()
+
+    def test_process_connect_sync(self):
+        """Test the gdb-remote command in synchronous mode"""
+        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"""
+        try:
+            self.dbg.SetAsync(True)
+            self.expect("process connect connect://localhost:%d" %
+                        self.server.port,
+                        matching=False,
+                        substrs=['Process', 'stopped'])
+            lldbutil.expect_state_changes(self, self.dbg.GetListener(),
+                                          self.process(), [lldb.eStateStopped])
+        finally:
+            self.dbg.GetSelectedPlatform().DisconnectRemote()


        


More information about the lldb-commits mailing list