[Lldb-commits] [lldb] 1903f35 - [lldb] [llgs] Send process output asynchronously in non-stop mode

Michał Górny via lldb-commits lldb-commits at lists.llvm.org
Fri Jul 15 08:27:13 PDT 2022


Author: Michał Górny
Date: 2022-07-15T17:20:39+02:00
New Revision: 1903f358bcc74fd96c65f0d1deba577c63238c86

URL: https://github.com/llvm/llvm-project/commit/1903f358bcc74fd96c65f0d1deba577c63238c86
DIFF: https://github.com/llvm/llvm-project/commit/1903f358bcc74fd96c65f0d1deba577c63238c86.diff

LOG: [lldb] [llgs] Send process output asynchronously in non-stop mode

Introduce a new %Stdio notification category and use it to send process
output asynchronously when running in non-stop mode.  This is an LLDB
extension since GDB does not use the 'O' packet for process output,
just for replies to 'qRcmd' packets.

Using the async notification mechanism implies that only the first
output packet is sent immediately to the client.  The client needs
to request subsequent notifications (if any) using the new vStdio packet
(that works pretty much like vStopped for the Stop notification queue).

The packet handler in lldb-server tests is updated to handle the async
stdio packets in addition to the regular O packets.  However, due
to the implications noted above, it can only handle the first output
packet sent by the server.  Subsequent packets need to be explicitly
requested via vStdio.

Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D128849

Added: 
    

Modified: 
    lldb/include/lldb/Utility/StringExtractorGDBRemote.h
    lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py
    lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
    lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
    lldb/source/Utility/StringExtractorGDBRemote.cpp
    lldb/test/API/tools/lldb-server/TestNonStop.py

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Utility/StringExtractorGDBRemote.h b/lldb/include/lldb/Utility/StringExtractorGDBRemote.h
index c32ce0389116e..d869950ab6dd1 100644
--- a/lldb/include/lldb/Utility/StringExtractorGDBRemote.h
+++ b/lldb/include/lldb/Utility/StringExtractorGDBRemote.h
@@ -179,6 +179,7 @@ class StringExtractorGDBRemote : public StringExtractor {
     eServerPacketType_QNonStop,
     eServerPacketType_vStopped,
     eServerPacketType_vCtrlC,
+    eServerPacketType_vStdio,
   };
 
   ServerPacketType GetServerPacketType() const;

diff  --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py
index eff6dd6d53007..82e1685666bc0 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py
@@ -79,26 +79,6 @@ def _is_packet_lldb_gdbserver_input(packet_type, llgs_input_is_read):
         raise "Unknown packet type: {}".format(packet_type)
 
 
-def handle_O_packet(context, packet_contents, logger):
-    """Handle O packets."""
-    if (not packet_contents) or (len(packet_contents) < 1):
-        return False
-    elif packet_contents[0] != "O":
-        return False
-    elif packet_contents == "OK":
-        return False
-
-    new_text = gdbremote_hex_decode_string(packet_contents[1:])
-    context["O_content"] += new_text
-    context["O_count"] += 1
-
-    if logger:
-        logger.debug(
-            "text: new \"{}\", cumulative: \"{}\"".format(
-                new_text, context["O_content"]))
-
-    return True
-
 _STRIP_CHECKSUM_REGEX = re.compile(r'#[0-9a-fA-F]{2}$')
 _STRIP_COMMAND_PREFIX_REGEX = re.compile(r"^\$")
 _STRIP_COMMAND_PREFIX_M_REGEX = re.compile(r"^\$m")
@@ -859,19 +839,21 @@ def process_is_running(pid, unknown_value=True):
     # Check if the pid is in the process_ids
     return pid in process_ids
 
+
 def _handle_output_packet_string(packet_contents):
-    if (not packet_contents) or (len(packet_contents) < 1):
+    # Warning: in non-stop mode, we currently handle only the first output
+    # packet since we'd need to inject vStdio packets
+    if not packet_contents.startswith((b"$O", b"%Stdio:O")):
         return None
-    elif packet_contents[0:1] != b"O":
-        return None
-    elif packet_contents == b"OK":
+    elif packet_contents == b"$OK":
         return None
     else:
-        return binascii.unhexlify(packet_contents[1:])
+        return binascii.unhexlify(packet_contents.partition(b"O")[2])
+
 
 class Server(object):
 
-    _GDB_REMOTE_PACKET_REGEX = re.compile(br'^[\$%]([^\#]*)#[0-9a-fA-F]{2}')
+    _GDB_REMOTE_PACKET_REGEX = re.compile(br'^([\$%][^\#]*)#[0-9a-fA-F]{2}')
 
     class ChecksumMismatch(Exception):
         pass

diff  --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index 8edb49f06a334..49f37ba7219f0 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -245,6 +245,9 @@ void GDBRemoteCommunicationServerLLGS::RegisterPacketHandlers() {
   RegisterMemberFunctionHandler(
       StringExtractorGDBRemote::eServerPacketType_QNonStop,
       &GDBRemoteCommunicationServerLLGS::Handle_QNonStop);
+  RegisterMemberFunctionHandler(
+      StringExtractorGDBRemote::eServerPacketType_vStdio,
+      &GDBRemoteCommunicationServerLLGS::Handle_vStdio);
   RegisterMemberFunctionHandler(
       StringExtractorGDBRemote::eServerPacketType_vStopped,
       &GDBRemoteCommunicationServerLLGS::Handle_vStopped);
@@ -1192,6 +1195,9 @@ GDBRemoteCommunicationServerLLGS::SendONotification(const char *buffer,
   response.PutChar('O');
   response.PutBytesAsRawHex8(buffer, len);
 
+  if (m_non_stop)
+    return SendNotificationPacketNoLock("Stdio", m_stdio_notification_queue,
+                                        response.GetString());
   return SendPacketNoLock(response.GetString());
 }
 
@@ -3907,26 +3913,38 @@ GDBRemoteCommunicationServerLLGS::Handle_QNonStop(
 }
 
 GDBRemoteCommunication::PacketResult
-GDBRemoteCommunicationServerLLGS::Handle_vStopped(
-    StringExtractorGDBRemote &packet) {
+GDBRemoteCommunicationServerLLGS::HandleNotificationAck(
+    std::deque<std::string> &queue) {
   // Per the protocol, the first message put into the queue is sent
-  // immediately.  However, it remains the queue until the client ACKs
-  // it via vStopped -- then we pop it and send the next message.
-  // The process repeats until the last message in the queue is ACK-ed,
-  // in which case the vStopped packet sends an OK response.
-
-  if (m_stop_notification_queue.empty())
+  // immediately.  However, it remains the queue until the client ACKs it --
+  // then we pop it and send the next message.  The process repeats until
+  // the last message in the queue is ACK-ed, in which case the packet sends
+  // an OK response.
+  if (queue.empty())
     return SendErrorResponse(Status("No pending notification to ack"));
-  m_stop_notification_queue.pop_front();
-  if (!m_stop_notification_queue.empty())
-    return SendPacketNoLock(m_stop_notification_queue.front());
+  queue.pop_front();
+  if (!queue.empty())
+    return SendPacketNoLock(queue.front());
+  return SendOKResponse();
+}
+
+GDBRemoteCommunication::PacketResult
+GDBRemoteCommunicationServerLLGS::Handle_vStdio(
+    StringExtractorGDBRemote &packet) {
+  return HandleNotificationAck(m_stdio_notification_queue);
+}
+
+GDBRemoteCommunication::PacketResult
+GDBRemoteCommunicationServerLLGS::Handle_vStopped(
+    StringExtractorGDBRemote &packet) {
+  PacketResult ret = HandleNotificationAck(m_stop_notification_queue);
   // If this was the last notification and all the processes exited,
   // terminate the server.
-  if (m_debugged_processes.empty()) {
+  if (m_stop_notification_queue.empty() && m_debugged_processes.empty()) {
     m_exit_now = true;
     m_mainloop.RequestTermination();
   }
-  return SendOKResponse();
+  return ret;
 }
 
 GDBRemoteCommunication::PacketResult

diff  --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
index 47e8a783603de..3d67032fc62d6 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -117,6 +117,7 @@ class GDBRemoteCommunicationServerLLGS
   bool m_thread_suffix_supported = false;
   bool m_list_threads_in_stop_reply = false;
   bool m_non_stop = false;
+  std::deque<std::string> m_stdio_notification_queue;
   std::deque<std::string> m_stop_notification_queue;
 
   NativeProcessProtocol::Extension m_extensions_supported = {};
@@ -247,6 +248,10 @@ class GDBRemoteCommunicationServerLLGS
 
   PacketResult Handle_QNonStop(StringExtractorGDBRemote &packet);
 
+  PacketResult HandleNotificationAck(std::deque<std::string> &queue);
+
+  PacketResult Handle_vStdio(StringExtractorGDBRemote &packet);
+
   PacketResult Handle_vStopped(StringExtractorGDBRemote &packet);
 
   PacketResult Handle_vCtrlC(StringExtractorGDBRemote &packet);

diff  --git a/lldb/source/Utility/StringExtractorGDBRemote.cpp b/lldb/source/Utility/StringExtractorGDBRemote.cpp
index 07954408f6d02..fc740615dd05a 100644
--- a/lldb/source/Utility/StringExtractorGDBRemote.cpp
+++ b/lldb/source/Utility/StringExtractorGDBRemote.cpp
@@ -380,6 +380,8 @@ StringExtractorGDBRemote::GetServerPacketType() const {
         return eServerPacketType_vStopped;
       if (PACKET_MATCHES("vCtrlC"))
         return eServerPacketType_vCtrlC;
+      if (PACKET_MATCHES("vStdio"))
+        return eServerPacketType_vStdio;
       break;
 
     }

diff  --git a/lldb/test/API/tools/lldb-server/TestNonStop.py b/lldb/test/API/tools/lldb-server/TestNonStop.py
index e4367f6ec9ff8..e55ae0fa7c7d2 100644
--- a/lldb/test/API/tools/lldb-server/TestNonStop.py
+++ b/lldb/test/API/tools/lldb-server/TestNonStop.py
@@ -296,3 +296,47 @@ def test_vCont_then_partial_stop(self):
     @add_test_categories(["llgs"])
     def test_vCont_then_partial_stop_run_both(self):
         self.vCont_then_partial_stop_test(True)
+
+    @add_test_categories(["llgs"])
+    def test_stdio(self):
+        self.build()
+        self.set_inferior_startup_launch()
+        # Since we can't easily ensure that lldb will send output in two parts,
+        # just put a stop in the middle.  Since we don't clear vStdio,
+        # the second message won't be delivered immediately.
+        self.prep_debug_monitor_and_inferior(
+            inferior_args=["message 1", "stop", "message 2"])
+        self.test_sequence.add_log_lines(
+            ["read packet: $QNonStop:1#00",
+             "send packet: $OK#00",
+             "read packet: $c#63",
+             "send packet: $OK#00",
+             {"direction": "send", "regex": r"^%Stop:T.*"},
+             "read packet: $vStopped#00",
+             "send packet: $OK#00",
+             "read packet: $c#63",
+             "send packet: $OK#00",
+             "send packet: %Stop:W00#00",
+             ], True)
+        ret = self.expect_gdbremote_sequence()
+        self.assertIn(ret["O_content"], b"message 1\r\n")
+
+        # Now, this is somewhat messy.  expect_gdbremote_sequence() will
+        # automatically consume output packets, so we just send vStdio,
+        # assume the first reply was consumed, send another one and expect
+        # a non-consumable "OK" reply.
+        self.reset_test_sequence()
+        self.test_sequence.add_log_lines(
+            ["read packet: $vStdio#00",
+             "read packet: $vStdio#00",
+             "send packet: $OK#00",
+             ], True)
+        ret = self.expect_gdbremote_sequence()
+        self.assertIn(ret["O_content"], b"message 2\r\n")
+
+        self.reset_test_sequence()
+        self.test_sequence.add_log_lines(
+            ["read packet: $vStopped#00",
+             "send packet: $OK#00",
+             ], True)
+        self.expect_gdbremote_sequence()


        


More information about the lldb-commits mailing list