[lldb] Add a gdb_remote_client test for connecting to pty

[mgorny]

Summary

Add a minimal mock server utilizing a pty, and add a client test
connecting to that server.

[mgorny]

@labath, this is the part where I ask for help ;-).

An exception happened when receiving the response from the gdb server. Closing the client...
Traceback (most recent call last):
  File "/home/mgorny/git/llvm-project/lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py", line 405, in _run
    data = seven.bitcast_to_string(self._client.recv(4096))
  File "/home/mgorny/git/llvm-project/lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py", line 538, in recv
    return self.read(size)
OSError: [Errno 5] Input/output error
PASS: LLDB (/home/mgorny/git/llvm-project/build/bin/clang-x86_64) :: test_process_connect_sync_dwarf (TestPty.TestPty)

I think it's getting in the 'controlling terminal' magic and getting EIO when lldb finishes or sth like that. Any suggestion how to avoid this exception?

[labath]

In D110878#3034748, @mgorny wrote:

@labath, this is the part where I ask for help ;-).

An exception happened when receiving the response from the gdb server. Closing the client...
Traceback (most recent call last):
  File "/home/mgorny/git/llvm-project/lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py", line 405, in _run
    data = seven.bitcast_to_string(self._client.recv(4096))
  File "/home/mgorny/git/llvm-project/lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py", line 538, in recv
    return self.read(size)
OSError: [Errno 5] Input/output error
PASS: LLDB (/home/mgorny/git/llvm-project/build/bin/clang-x86_64) :: test_process_connect_sync_dwarf (TestPty.TestPty)

I think it's getting in the 'controlling terminal' magic and getting EIO when lldb finishes or sth like that. Any suggestion how to avoid this exception?

Catch it and turn it into EOF? (btw, I've found this link to be pretty good at explaining the behavior across different OSs.)

lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py

618–623

I think it would be cleaner if we had like a single new interface, which describes what to we expect of the connection, and then a socket- and pty-based implementation of that interface.

648–652

I'm wondering if we should have an overload of SBTarget::ConnectRemote that accepts a file descriptor (or SBFile, SBCommunication, ...).

This would make the socket case easier as well, and it seems like it could be generally useful.

[mgorny]

lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py

648–652

I'm not saying 'no' but I think we'd still want to explicitly test process connect command somewhere.

[mgorny]

Replaced the class hacks with an abstraction over socket & connection sockets.

[labath]

lldb/test/API/functionalities/gdb_remote_client/TestPty.py

22

I think this can be applied at class level

lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py

409

do you actually need to keep the slave open here?

[mgorny]

lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py

409

Yes, it seems this way. If I close it early, the test hangs.

[mgorny]

Make it easier to override socket class at GDBRemoteTestBase level. Add async test, mark the whole class skipIfWindows.

[mgorny]

Add get_connect_url() to simplify combining address with proper protocol/prefix.

lldb/test/API/functionalities/gdb_remote_client/TestPlatformClient.py

def test_process_list_with_all_users(self):
                         return "E04"
                 else:
                     return "E04"
 
         self.server.responder = MyResponder()
 
         try:
             self.runCmd("platform select remote-linux")
-            self.runCmd("platform connect connect://" + self.server.get_connect_address())
+            self.runCmd("platform connect " + self.server.get_connect_url())
             self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
             self.expect("platform process list -x",
                         substrs=["2 matching processes were found", "test_process", "another_test_process"])
             self.expect("platform process list -xv",
                         substrs=[
                             "PID    PARENT USER       GROUP      EFF USER   EFF GROUP  TRIPLE                         ARGUMENTS",
                             "10     1      2          3          4          5                                         /system/bin/sh -c /data/local/tmp/lldb-server",
                             "11     2      3          4          5          6"])
class TestPlatformClient(GDBRemoteTestBase):
     def test_no_timeout(self):
         """Test that we honor the timeout setting. With a large enough timeout,
         we should get the CWD successfully."""
 
         self.server.responder = TestPlatformClient.TimeoutResponder()
         self.runCmd("settings set plugin.process.gdb-remote.packet-timeout 30")
         plat = lldb.SBPlatform("remote-linux")
         try:
-            self.assertSuccess(plat.ConnectRemote(lldb.SBPlatformConnectOptions("connect://"
-                + self.server.get_connect_address())))
+            self.assertSuccess(plat.ConnectRemote(lldb.SBPlatformConnectOptions(
+                self.server.get_connect_url())))
             self.assertEqual(plat.GetWorkingDirectory(), "/foo/bar")
         finally:
             plat.DisconnectRemote()
 
     def test_timeout(self):
         """Test that we honor the timeout setting. With a small timeout, CWD
         retrieval should fail."""
 
         self.server.responder = TestPlatformClient.TimeoutResponder()
         self.runCmd("settings set plugin.process.gdb-remote.packet-timeout 3")
         plat = lldb.SBPlatform("remote-linux")
         try:
-            self.assertSuccess(plat.ConnectRemote(lldb.SBPlatformConnectOptions("connect://"
-                + self.server.get_connect_address())))
+            self.assertSuccess(plat.ConnectRemote(lldb.SBPlatformConnectOptions(
+                self.server.get_connect_url())))
             self.assertIsNone(plat.GetWorkingDirectory())
         finally:
             plat.DisconnectRemote()

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

class TestProcessConnect(GDBRemoteTestBase):
 
     @skipIfWindows
     def test_process_connect_sync(self):
         """Test the gdb-remote command in synchronous mode"""
         try:
             self.dbg.SetAsync(False)
             self.expect("platform select remote-gdb-server",
                         substrs=['Platform: remote-gdb-server', 'Connected: no'])
-            self.expect("process connect connect://" +
-                        self.server.get_connect_address(),
+            self.expect("process connect " + self.server.get_connect_url(),
                         substrs=['Process', 'stopped'])
         finally:
             self.dbg.GetSelectedPlatform().DisconnectRemote()
 
     @skipIfWindows
     def test_process_connect_async(self):
         """Test the gdb-remote command in asynchronous mode"""
         try:
             self.dbg.SetAsync(True)
             self.expect("platform select remote-gdb-server",
                         substrs=['Platform: remote-gdb-server', 'Connected: no'])
-            self.expect("process connect connect://" +
-                        self.server.get_connect_address(),
+            self.expect("process connect " + self.server.get_connect_url(),
                         matching=False,
                         substrs=['Process', 'stopped'])
             lldbutil.expect_state_changes(self, self.dbg.GetListener(),
                                           self.process(), [lldb.eStateStopped])
         finally:
             self.dbg.GetSelectedPlatform().DisconnectRemote()

lldb/test/API/functionalities/gdb_remote_client/TestPty.py

+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from gdbclientutils import *
+
+
+@skipIfWindows
+class TestPty(GDBRemoteTestBase):
+    mydir = TestBase.compute_mydir(__file__)
+    server_socket_class = PtyServerSocket
+
+    def test_process_connect_sync(self):
+        """Test the process connect command in synchronous mode"""
+        try:
+            self.dbg.SetAsync(False)
+            self.expect("platform select remote-gdb-server",
+                        substrs=['Platform: remote-gdb-server', 'Connected: no'])
+            self.expect("process connect " + self.server.get_connect_url(),
+                        substrs=['Process', 'stopped'])
+        finally:
+            self.dbg.GetSelectedPlatform().DisconnectRemote()
+
+    def test_process_connect_async(self):
+        """Test the process connect command in asynchronous mode"""
+        try:
+            self.dbg.SetAsync(True)
+            self.expect("platform select remote-gdb-server",
+                        substrs=['Platform: remote-gdb-server', 'Connected: no'])
+            self.expect("process connect " + self.server.get_connect_url(),
+                        matching=False,
+                        substrs=['Process', 'stopped'])
+            lldbutil.expect_state_changes(self, self.dbg.GetListener(),
+                                          self.process(), [lldb.eStateStopped])
+        finally:
+            self.dbg.GetSelectedPlatform().DisconnectRemote()

lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py

+import ctypes
 import errno
+import io
 import os
 import os.path
+import pty
 import threading
 import socket
+import tty
 import lldb
 import binascii
 import traceback
 from lldbsuite.support import seven
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbtest_config
 
 
class MockGDBServerResponder:
     Raised when we receive a packet for which there is no default action.
     Override the responder class to implement behavior suitable for the test at
     hand.
     """
     class UnexpectedPacketException(Exception):
         pass
 
 
+class ServerSocket:
+    """
+    A wrapper class for TCP or pty-based server.
+    """
+
+    def get_connect_address(self):
+        """Get address for the client to connect to."""
+
+    def get_connect_url(self):
+        """Get URL suitable for process connect command."""
+
+    def close_server(self):
+        """Close all resources used by the server."""
+
+    def accept(self):
+        """Accept a single client connection to the server."""
+
+    def close_connection(self):
+        """Close all resources used by the accepted connection."""
+
+    def recv(self):
+        """Receive a data packet from the connected client."""
+
+    def sendall(self, data):
+        """Send the data to the connected client."""
+
+
+class TCPServerSocket(ServerSocket):
+    def __init__(self):
+        family, type, proto, _, addr = socket.getaddrinfo(
+                "localhost", 0, proto=socket.IPPROTO_TCP)[0]
+        self._server_socket = socket.socket(family, type, proto)
+        self._connection = None
+
+        self._server_socket.bind(addr)
+        self._server_socket.listen(1)
+
+    def get_connect_address(self):
+        return "[{}]:{}".format(*self._server_socket.getsockname())
+
+    def get_connect_url(self):
+        return "connect://" + self.get_connect_address()
+
+    def close_server(self):
+        self._server_socket.close()
+
+    def accept(self):
+        assert self._connection is None
+        # accept() is stubborn and won't fail even when the socket is
+        # shutdown, so we'll use a timeout
+        self._server_socket.settimeout(30.0)
+        client, client_addr = self._server_socket.accept()
+        # The connected client inherits its timeout from self._socket,
+        # but we'll use a blocking socket for the client
+        client.settimeout(None)
+        self._connection = client
+
+    def close_connection(self):
+        assert self._connection is not None
+        self._connection.close()
+        self._connection = None
+
+    def recv(self):
+        assert self._connection is not None
+        return self._connection.recv(4096)
+
+    def sendall(self, data):
+        assert self._connection is not None
+        return self._connection.sendall(data)
+
+
+class PtyServerSocket(ServerSocket):
+    def __init__(self):
+        master, slave = pty.openpty()
+        tty.setraw(master)
+        self._master = io.FileIO(master, 'r+b')
+        self._slave = io.FileIO(slave, 'r+b')
+
+    def get_connect_address(self):
+        libc = ctypes.CDLL(None)
+        libc.ptsname.argtypes = (ctypes.c_int,)
+        libc.ptsname.restype = ctypes.c_char_p
+        return libc.ptsname(self._master.fileno()).decode()
+
+    def get_connect_url(self):
+        return "file://" + self.get_connect_address()
+
+    def close_server(self):
+        self._slave.close()
+        self._master.close()
+
+    def recv(self):
+        try:
+            return self._master.read(4096)
+        except OSError as e:
+            # closing the pty results in EIO on Linux, convert it to EOF
+            if e.errno == errno.EIO:
+                return b''
+            raise
+
+    def sendall(self, data):
+        return self._master.write(data)
+
+
 class MockGDBServer:
     """
     A simple TCP-based GDB server that can test client behavior by receiving
     commands and issuing custom-tailored responses.
 
     Responses are generated via the .responder property, which should be an
     instance of a class based on MockGDBServerResponder.
     """
 
     responder = None
     _socket = None
-    _client = None
     _thread = None
     _receivedData = None
     _receivedDataOffset = None
     _shouldSendAck = True
 
-    def __init__(self):
+    def __init__(self, socket_class):
+        self._socket_class = socket_class
         self.responder = MockGDBServerResponder()
 
     def start(self):
-        family, type, proto, _, addr = socket.getaddrinfo("localhost", 0,
-                proto=socket.IPPROTO_TCP)[0]
-        self._socket = socket.socket(family, type, proto)
-
-
-        self._socket.bind(addr)
-        self._socket.listen(1)
-
+        self._socket = self._socket_class()
         # Start a thread that waits for a client connection.
         self._thread = threading.Thread(target=self._run)
         self._thread.start()
 
     def stop(self):
-        self._socket.close()
+        self._socket.close_server()
         self._thread.join()
         self._thread = None
 
     def get_connect_address(self):
-        return "[{}]:{}".format(*self._socket.getsockname())
+        return self._socket.get_connect_address()
+
+    def get_connect_url(self):
+        return self._socket.get_connect_url()
 
     def _run(self):
         # For testing purposes, we only need to worry about one client
         # connecting just one time.
         try:
-            # accept() is stubborn and won't fail even when the socket is
-            # shutdown, so we'll use a timeout
-            self._socket.settimeout(30.0)
-            client, client_addr = self._socket.accept()
-            self._client = client
-            # The connected client inherits its timeout from self._socket,
-            # but we'll use a blocking socket for the client
-            self._client.settimeout(None)
+            self._socket.accept()
         except:
             return
         self._shouldSendAck = True
         self._receivedData = ""
         self._receivedDataOffset = 0
         data = None
         while True:
             try:
-                data = seven.bitcast_to_string(self._client.recv(4096))
+                data = seven.bitcast_to_string(self._socket.recv())
                 if data is None or len(data) == 0:
                     break
                 self._receive(data)
             except Exception as e:
                 print("An exception happened when receiving the response from the gdb server. Closing the client...")
                 traceback.print_exc()
-                self._client.close()
+                self._socket.close_connection()
                 break
 
     def _receive(self, data):
         """
         Collects data, parses and responds to as many packets as exist.
         Any leftover data is kept for parsing the next time around.
         """
         self._receivedData += data
         try:
             packet = self._parsePacket()
             while packet is not None:
                 self._handlePacket(packet)
                 packet = self._parsePacket()
         except self.InvalidPacketException:
-            self._client.close()
+            self._socket.close_connection()
 
     def _parsePacket(self):
         """
         Reads bytes from self._receivedData, returning:
         - a packet's contents if a valid packet is found
         - the PACKET_ACK unique object if we got an ack
         - None if we only have a partial packet
 
def _handlePacket(self, packet):
             # Ignore ACKs from the client. For the future, we can consider
             # adding validation code to make sure the client only sends ACKs
             # when it's supposed to.
             return
         response = ""
         # We'll handle the ack stuff here since it's not something any of the
         # tests will be concerned about, and it'll get turned off quickly anyway.
         if self._shouldSendAck:
-            self._client.sendall(seven.bitcast_to_bytes('+'))
+            self._socket.sendall(seven.bitcast_to_bytes('+'))
         if packet == "QStartNoAckMode":
             self._shouldSendAck = False
             response = "OK"
         elif self.responder is not None:
             # Delegate everything else to our responder
             response = self.responder.respond(packet)
         # Handle packet framing since we don't want to bother tests with it.
         if response is not None:
             framed = frame_packet(response)
-            self._client.sendall(seven.bitcast_to_bytes(framed))
+            self._socket.sendall(seven.bitcast_to_bytes(framed))
 
     PACKET_ACK = object()
     PACKET_INTERRUPT = object()
 
     class InvalidPacketException(Exception):
         pass
 
+
 class GDBRemoteTestBase(TestBase):
     """
     Base class for GDB client tests.
 
     This class will setup and start a mock GDB server for the test to use.
     It also provides assertPacketLogContains, which simplifies the checking
     of packets sent by the client.
     """
 
     NO_DEBUG_INFO_TESTCASE = True
     mydir = TestBase.compute_mydir(__file__)
     server = None
+    server_socket_class = TCPServerSocket
 
     def setUp(self):
         TestBase.setUp(self)
-        self.server = MockGDBServer()
+        self.server = MockGDBServer(socket_class=self.server_socket_class)
         self.server.start()
 
     def tearDown(self):
         # TestBase.tearDown will kill the process, but we need to kill it early
         # so its client connection closes and we can stop the server before
         # finally calling the base tearDown.
         if self.process() is not None:
             self.process().Kill()
         self.server.stop()
         TestBase.tearDown(self)
 
     def createTarget(self, yaml_path):
         """
         Create a target by auto-generating the object based on the given yaml
         instructions.
 
         This will track the generated object so it can be automatically removed
         during tearDown.
         """
         yaml_base, ext = os.path.splitext(yaml_path)
         obj_path = self.getBuildArtifact(yaml_base)
         self.yaml2obj(yaml_path, obj_path)
         return self.dbg.CreateTarget(obj_path)
 
     def connect(self, target):
         """
         Create a process by connecting to the mock GDB server.
 
         Includes assertions that the process was successfully created.
         """
         listener = self.dbg.GetListener()
         error = lldb.SBError()
         process = target.ConnectRemote(listener,
-                "connect://" + self.server.get_connect_address(), "gdb-remote", error)
+                self.server.get_connect_url(), "gdb-remote", error)
         self.assertTrue(error.Success(), error.description)
         self.assertTrue(process, PROCESS_IS_VALID)
         return process
 
     def assertPacketLogContains(self, packets):
         """
         Assert that the mock server's packet log contains the given packets.
 
class GDBPlatformClientTestBase(GDBRemoteTestBase):
 
     This class extends GDBRemoteTestBase by automatically connecting
     via "platform connect" in the setUp() method.
     """
 
     def setUp(self):
         super().setUp()
         self.runCmd("platform select remote-gdb-server")
-        self.runCmd("platform connect connect://" +
-                    self.server.get_connect_address())
+        self.runCmd("platform connect " + self.server.get_connect_url())
         self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
 
     def tearDown(self):
         self.dbg.GetSelectedPlatform().DisconnectRemote()
         super().tearDown()