mgorny created this revision.
mgorny added reviewers: labath, krytarowski, emaste.
Herald added a project: All.
mgorny requested review of this revision.

Implement the support for %Stop asynchronous notification packet format
in LLGS.  This does not implement full support for non-stop mode for
threaded programs -- process plugins continue stopping all threads
on every event.  However, it will be used to implement asynchronous
events in multiprocess debugging.

The non-stop protocol is enabled using QNonStop packet.  When it is
enabled, the server uses notification protocol instead of regular stop
replies.  Since all threads are always stopped, notifications are always
generated for all active threads and copied into stop notification
queue.

If the queue was empty, the initial asynchronous %Stop notification
is sent to the client immediately.  The client needs to (eventually)
acknowledge the notification by sending the vStopped packet, in which
case it is popped from the queue and the stop reason for the next thread
is reported.  This continues until notification queue is empty again,
in which case an OK reply is sent.

The change includes a test case for a program generating a segfault
on 3 threads.  The server is expected to generate a stop notification
for the segfaulting thread, along with the notifications for the other
running threads (with "no stop reason").  This verifies that the stop
reasons are correctly reported for all threads, and that notification
queue works.


https://reviews.llvm.org/D125575

Files:
  lldb/include/lldb/Utility/StringExtractorGDBRemote.h
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
  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/TestLldbGdbServer.py

Index: lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
===================================================================
--- lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
+++ lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
@@ -1410,3 +1410,80 @@
         self.assertEqual(decoded[errno_idx], 0)  # si_errno
         self.assertEqual(decoded[code_idx], SEGV_MAPERR)  # si_code
         self.assertEqual(decoded[addr_idx], 0)  # si_addr
+
+    def test_QNonStop(self):
+        self.build()
+        self.set_inferior_startup_launch()
+        thread_num = 3
+        procs = self.prep_debug_monitor_and_inferior(
+                inferior_args=["thread:segfault"] + thread_num * ["thread:new"])
+        self.test_sequence.add_log_lines(
+            ["read packet: $QNonStop:1#00",
+             "send packet: $OK#00",
+             "read packet: $c#63",
+             ], True)
+        self.expect_gdbremote_sequence()
+
+        segv_signo = lldbutil.get_signal_number('SIGSEGV')
+        all_threads = set()
+        all_segv_threads = []
+
+        # we should get segfaults from all the threads
+        for segv_no in range(thread_num):
+            # first wait for the notification event
+            self.reset_test_sequence()
+            self.test_sequence.add_log_lines(
+                [{"direction": "send",
+                  "regex": r"^%Stop:T([0-9a-fA-F]{2})thread:([0-9a-fA-F]+);",
+                  "capture": {1: "signo", 2: "thread_id"},
+                  },
+                 ], True)
+            threads = [self.expect_gdbremote_sequence()]
+
+            # then we may get events for the remaining threads
+            # (but note that not all threads may have been started yet)
+            while True:
+                self.reset_test_sequence()
+                self.test_sequence.add_log_lines(
+                    ["read packet: $vStopped#00",
+                     {"direction": "send",
+                      "regex": r"^\$(OK|T([0-9a-fA-F]{2})thread:([0-9a-fA-F]+);)",
+                      "capture": {1: "packet", 2: "signo", 3: "thread_id"},
+                      },
+                     ], True)
+                m = self.expect_gdbremote_sequence()
+                if m["packet"] == "OK":
+                    break
+                threads.append(m)
+
+            segv_threads = []
+            other_threads = []
+            for t in threads:
+                signo = int(t["signo"], 16)
+                if signo == segv_signo:
+                    segv_threads.append(t["thread_id"])
+                else:
+                    self.assertEqual(signo, 0)
+                    other_threads.append(t["thread_id"])
+
+            # verify that exactly one thread segfaulted
+            self.assertEqual(len(segv_threads), 1)
+            # we should get only one segv from every thread
+            self.assertNotIn(segv_threads[0], all_segv_threads)
+            all_segv_threads.extend(segv_threads)
+            # segv_threads + other_threads should always be a superset
+            # of all_threads, i.e. we should get states for all threads
+            # already started
+            self.assertFalse(
+                    all_threads.difference(other_threads + segv_threads))
+            all_threads.update(other_threads + segv_threads)
+
+            self.reset_test_sequence()
+            self.test_sequence.add_log_lines(
+                ["read packet: $vCont;C{:02x}:{};c#00"
+                 .format(segv_signo, segv_threads[0]),
+                 ], True)
+            self.expect_gdbremote_sequence()
+
+        # finally, verify that all threads have started
+        self.assertEqual(len(all_threads), thread_num + 1)
Index: lldb/source/Utility/StringExtractorGDBRemote.cpp
===================================================================
--- lldb/source/Utility/StringExtractorGDBRemote.cpp
+++ lldb/source/Utility/StringExtractorGDBRemote.cpp
@@ -148,6 +148,11 @@
         return eServerPacketType_QMemTags;
       break;
 
+    case 'N':
+      if (PACKET_STARTS_WITH("QNonStop:"))
+        return eServerPacketType_QNonStop;
+      break;
+
     case 'R':
       if (PACKET_STARTS_WITH("QRestoreRegisterState:"))
         return eServerPacketType_QRestoreRegisterState;
@@ -367,6 +372,10 @@
         return eServerPacketType_vCont_actions;
       if (PACKET_STARTS_WITH("vRun;"))
         return eServerPacketType_vRun;
+      if (PACKET_MATCHES("vStopped"))
+        return eServerPacketType_vStopped;
+      break;
+
     }
     break;
   case '_':
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -106,6 +106,8 @@
   uint32_t m_next_saved_registers_id = 1;
   bool m_thread_suffix_supported = false;
   bool m_list_threads_in_stop_reply = false;
+  bool m_non_stop = false;
+  std::deque<std::string> m_stop_notification_queue;
 
   NativeProcessProtocol::Extension m_extensions_supported = {};
 
@@ -115,6 +117,9 @@
 
   PacketResult SendStopReplyPacketForThread(lldb::tid_t tid);
 
+  PacketResult SendStopReplyPacketForThread(NativeThreadProtocol &thread,
+                                            bool send_notify = true);
+
   PacketResult SendStopReasonForState(lldb::StateType process_state);
 
   PacketResult Handle_k(StringExtractorGDBRemote &packet);
@@ -217,6 +222,10 @@
 
   PacketResult Handle_qSaveCore(StringExtractorGDBRemote &packet);
 
+  PacketResult Handle_QNonStop(StringExtractorGDBRemote &packet);
+
+  PacketResult Handle_vStopped(StringExtractorGDBRemote &packet);
+
   PacketResult Handle_g(StringExtractorGDBRemote &packet);
 
   PacketResult Handle_qMemTags(StringExtractorGDBRemote &packet);
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -235,6 +235,13 @@
   RegisterMemberFunctionHandler(
       StringExtractorGDBRemote::eServerPacketType_qLLDBSaveCore,
       &GDBRemoteCommunicationServerLLGS::Handle_qSaveCore);
+
+  RegisterMemberFunctionHandler(
+      StringExtractorGDBRemote::eServerPacketType_QNonStop,
+      &GDBRemoteCommunicationServerLLGS::Handle_QNonStop);
+  RegisterMemberFunctionHandler(
+      StringExtractorGDBRemote::eServerPacketType_vStopped,
+      &GDBRemoteCommunicationServerLLGS::Handle_vStopped);
 }
 
 void GDBRemoteCommunicationServerLLGS::SetLaunchInfo(const ProcessLaunchInfo &info) {
@@ -767,25 +774,31 @@
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerLLGS::SendStopReplyPacketForThread(
     lldb::tid_t tid) {
-  Log *log = GetLog(LLDBLog::Process | LLDBLog::Thread);
-
   // Ensure we have a debugged process.
   if (!m_current_process ||
       (m_current_process->GetID() == LLDB_INVALID_PROCESS_ID))
     return SendErrorResponse(50);
 
-  LLDB_LOG(log, "preparing packet for pid {0} tid {1}",
-           m_current_process->GetID(), tid);
-
   // Ensure we can get info on the given thread.
   NativeThreadProtocol *thread = m_current_process->GetThreadByID(tid);
   if (!thread)
     return SendErrorResponse(51);
 
+  return SendStopReplyPacketForThread(*thread);
+}
+
+GDBRemoteCommunication::PacketResult
+GDBRemoteCommunicationServerLLGS::SendStopReplyPacketForThread(
+    NativeThreadProtocol &thread, bool send_notify) {
+  Log *log = GetLog(LLDBLog::Process | LLDBLog::Thread);
+
+  LLDB_LOG(log, "preparing packet for pid {0} tid {1}",
+           m_current_process->GetID(), thread.GetID());
+
   // Grab the reason this thread stopped.
   struct ThreadStopInfo tid_stop_info;
   std::string description;
-  if (!thread->GetStopReason(tid_stop_info, description))
+  if (!thread.GetStopReason(tid_stop_info, description))
     return SendErrorResponse(52);
 
   // FIXME implement register handling for exec'd inferiors.
@@ -801,17 +814,17 @@
   LLDB_LOG(
       log,
       "pid {0}, tid {1}, got signal signo = {2}, reason = {3}, exc_type = {4}",
-      m_current_process->GetID(), tid, signum, int(tid_stop_info.reason),
-      tid_stop_info.details.exception.type);
+      m_current_process->GetID(), thread.GetID(), signum,
+      int(tid_stop_info.reason), tid_stop_info.details.exception.type);
 
   // Print the signal number.
   response.PutHex8(signum & 0xff);
 
   // Include the tid.
-  response.Printf("thread:%" PRIx64 ";", tid);
+  response.Printf("thread:%" PRIx64 ";", thread.GetID());
 
   // Include the thread name if there is one.
-  const std::string thread_name = thread->GetName();
+  const std::string thread_name = thread.GetName();
   if (!thread_name.empty()) {
     size_t thread_name_len = thread_name.length();
 
@@ -875,7 +888,7 @@
     char delimiter = ':';
     for (NativeThreadProtocol *thread;
          (thread = m_current_process->GetThreadAtIndex(i)) != nullptr; ++i) {
-      NativeRegisterContext& reg_ctx = thread->GetRegisterContext();
+      NativeRegisterContext &reg_ctx = thread->GetRegisterContext();
 
       uint32_t reg_to_read = reg_ctx.ConvertRegisterKindToRegisterNumber(
           eRegisterKindGeneric, LLDB_REGNUM_GENERIC_PC);
@@ -906,7 +919,7 @@
   //
 
   // Grab the register context.
-  NativeRegisterContext& reg_ctx = thread->GetRegisterContext();
+  NativeRegisterContext &reg_ctx = thread.GetRegisterContext();
   const auto expedited_regs =
       reg_ctx.GetExpeditedRegisters(ExpeditedRegs::Full);
 
@@ -923,8 +936,9 @@
                                           &reg_value, lldb::eByteOrderBig);
         response.PutChar(';');
       } else {
-        LLDB_LOGF(log, "GDBRemoteCommunicationServerLLGS::%s failed to read "
-                       "register '%s' index %" PRIu32 ": %s",
+        LLDB_LOGF(log,
+                  "GDBRemoteCommunicationServerLLGS::%s failed to read "
+                  "register '%s' index %" PRIu32 ": %s",
                   __FUNCTION__,
                   reg_info_p->name ? reg_info_p->name : "<unnamed-register>",
                   reg_num, error.AsCString());
@@ -973,7 +987,21 @@
                     tid_stop_info.details.fork.child_tid);
   }
 
-  return SendPacketNoLock(response.GetString());
+  if (m_non_stop) {
+    PacketResult ret = SendNotificationPacketNoLock(
+        "Stop", m_stop_notification_queue, response.GetString());
+    if (send_notify) {
+      // Queue notification events for the remaining threads.
+      uint32_t thread_index = 0;
+      while (NativeThreadProtocol *listed_thread =
+                 m_current_process->GetThreadAtIndex(thread_index++)) {
+        if (listed_thread->GetID() != thread.GetID())
+          SendStopReplyPacketForThread(*listed_thread, false);
+      }
+    }
+    return ret;
+  } else
+    return SendPacketNoLock(response.GetString());
 }
 
 void GDBRemoteCommunicationServerLLGS::HandleInferiorState_Exited(
@@ -3657,6 +3685,33 @@
   return SendPacketNoLock(response.GetString());
 }
 
+GDBRemoteCommunication::PacketResult
+GDBRemoteCommunicationServerLLGS::Handle_QNonStop(
+    StringExtractorGDBRemote &packet) {
+  StringRef packet_str{packet.GetStringRef()};
+  assert(packet_str.startswith("QNonStop:"));
+  packet_str.consume_front("QNonStop:");
+  if (packet_str == "0") {
+    m_non_stop = false;
+    // TODO: stop all threads
+  } else if (packet_str == "1") {
+    m_non_stop = true;
+  } else
+    return SendErrorResponse(Status("Invalid QNonStop packet"));
+  return SendOKResponse();
+}
+
+GDBRemoteCommunication::PacketResult
+GDBRemoteCommunicationServerLLGS::Handle_vStopped(
+    StringExtractorGDBRemote &packet) {
+  if (m_stop_notification_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());
+  return SendOKResponse();
+}
+
 void GDBRemoteCommunicationServerLLGS::MaybeCloseInferiorTerminalConnection() {
   Log *log = GetLog(LLDBLog::Process);
 
@@ -3840,6 +3895,7 @@
                             "QThreadSuffixSupported+",
                             "QListThreadsInStopReply+",
                             "qXfer:features:read+",
+                            "QNonStop+",
                         });
 
   // report server-only features
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
@@ -183,6 +183,9 @@
   CompressionType m_compression_type;
 
   PacketResult SendPacketNoLock(llvm::StringRef payload);
+  PacketResult SendNotificationPacketNoLock(llvm::StringRef notify_type,
+                                            std::deque<std::string>& queue,
+                                            llvm::StringRef payload);
   PacketResult SendRawPacketNoLock(llvm::StringRef payload,
                                    bool skip_ack = false);
 
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -124,6 +124,29 @@
   return SendRawPacketNoLock(packet_str);
 }
 
+GDBRemoteCommunication::PacketResult
+GDBRemoteCommunication::SendNotificationPacketNoLock(
+    llvm::StringRef notify_type, std::deque<std::string> &queue,
+    llvm::StringRef payload) {
+  PacketResult ret = PacketResult::Success;
+
+  // If there are no notification in the queue, send the notification
+  // packet.
+  if (queue.empty()) {
+    StreamString packet(0, 4, eByteOrderBig);
+    packet.PutChar('%');
+    packet.Write(notify_type.data(), notify_type.size());
+    packet.PutChar(':');
+    packet.Write(payload.data(), payload.size());
+    packet.PutChar('#');
+    packet.PutHex8(CalculcateChecksum(payload));
+    ret = SendRawPacketNoLock(packet.GetString(), true);
+  }
+
+  queue.push_back(payload.str());
+  return ret;
+}
+
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunication::SendRawPacketNoLock(llvm::StringRef packet,
                                             bool skip_ack) {
Index: lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py
@@ -866,7 +866,7 @@
 
 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
Index: lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
@@ -851,6 +851,7 @@
         "memory-tagging",
         "qSaveCore",
         "native-signals",
+        "QNonStop",
     ]
 
     def parse_qSupported_response(self, context):
Index: lldb/include/lldb/Utility/StringExtractorGDBRemote.h
===================================================================
--- lldb/include/lldb/Utility/StringExtractorGDBRemote.h
+++ lldb/include/lldb/Utility/StringExtractorGDBRemote.h
@@ -174,6 +174,8 @@
     eServerPacketType_QMemTags, // write memory tags
 
     eServerPacketType_qLLDBSaveCore,
+    eServerPacketType_QNonStop,
+    eServerPacketType_vStopped,
   };
 
   ServerPacketType GetServerPacketType() const;
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to