Merge commit '907947e499b1' from swift/release/5.5 into swift/main

Author: git apple-llvm automerger

lldb/include/lldb/Target/Process.h

@@ -93,6 +93,7 @@ class ProcessProperties : public Properties {
   bool GetWarningsUnsupportedLanguage() const;
   bool GetStopOnExec() const;
   std::chrono::seconds GetUtilityExpressionTimeout() const;
+  std::chrono::seconds GetInterruptTimeout() const;
   bool GetOSPluginReportsAllThreads() const;
   void SetOSPluginReportsAllThreads(bool does_report);
   bool GetSteppingRunsAllThreads() const;

lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp

@@ -736,8 +736,8 @@ const UnixSignalsSP &PlatformRemoteGDBServer::GetRemoteUnixSignals() {
   m_remote_signals_sp = UnixSignals::Create(GetRemoteSystemArchitecture());
 
   StringExtractorGDBRemote response;
-  auto result = m_gdb_client.SendPacketAndWaitForResponse("jSignalsInfo",
-                                                          response, false);
+  auto result =
+      m_gdb_client.SendPacketAndWaitForResponse("jSignalsInfo", response);
 
   if (result != decltype(result)::Success ||
       response.GetResponseType() != response.eResponse)

lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp

@@ -20,7 +20,10 @@ using namespace lldb_private;
 using namespace lldb_private::process_gdb_remote;
 using namespace std::chrono;
 
-static const seconds kInterruptTimeout(5);
+// When we've sent a continue packet and are waiting for the target to stop,
+// we wake up the wait with this interval to make sure the stub hasn't gone
+// away while we were waiting.
+static const seconds kWakeupInterval(5);
 
 /////////////////////////
 // GDBRemoteClientBase //
@@ -35,7 +38,8 @@ GDBRemoteClientBase::GDBRemoteClientBase(const char *comm_name,
 
 StateType GDBRemoteClientBase::SendContinuePacketAndWaitForResponse(
     ContinueDelegate &delegate, const UnixSignals &signals,
-    llvm::StringRef payload, StringExtractorGDBRemote &response) {
+    llvm::StringRef payload, std::chrono::seconds interrupt_timeout,
+    StringExtractorGDBRemote &response) {
   Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS));
   response.Clear();
 
@@ -48,16 +52,34 @@ StateType GDBRemoteClientBase::SendContinuePacketAndWaitForResponse(
   if (!cont_lock)
     return eStateInvalid;
   OnRunPacketSent(true);
-
+  // The main ReadPacket loop wakes up at computed_timeout intervals, just to 
+  // check that the connection hasn't dropped.  When we wake up we also check
+  // whether there is an interrupt request that has reached its endpoint.
+  // If we want a shorter interrupt timeout that kWakeupInterval, we need to 
+  // choose the shorter interval for the wake up as well.
+  std::chrono::seconds computed_timeout = std::min(interrupt_timeout, 
+                                                   kWakeupInterval);
   for (;;) {
-    PacketResult read_result = ReadPacket(response, kInterruptTimeout, false);
+    PacketResult read_result = ReadPacket(response, computed_timeout, false);
     switch (read_result) {
     case PacketResult::ErrorReplyTimeout: {
       std::lock_guard<std::mutex> lock(m_mutex);
-      if (m_async_count == 0)
+      if (m_async_count == 0) {
         continue;
-      if (steady_clock::now() >= m_interrupt_time + kInterruptTimeout)
+      }
+      auto cur_time = steady_clock::now();
+      if (cur_time >= m_interrupt_endpoint)
         return eStateInvalid;
+      else {
+        // We woke up and found an interrupt is in flight, but we haven't
+        // exceeded the interrupt wait time.  So reset the wait time to the
+        // time left till the interrupt timeout.  But don't wait longer
+        // than our wakeup timeout.
+        auto new_wait = m_interrupt_endpoint - cur_time;
+        computed_timeout = std::min(kWakeupInterval,
+            std::chrono::duration_cast<std::chrono::seconds>(new_wait));
+        continue;
+      }
       break;
     }
     case PacketResult::Success:
@@ -133,8 +155,9 @@ StateType GDBRemoteClientBase::SendContinuePacketAndWaitForResponse(
   }
 }
 
-bool GDBRemoteClientBase::SendAsyncSignal(int signo) {
-  Lock lock(*this, true);
+bool GDBRemoteClientBase::SendAsyncSignal(
+    int signo, std::chrono::seconds interrupt_timeout) {
+  Lock lock(*this, interrupt_timeout);
   if (!lock || !lock.DidInterrupt())
     return false;
 
@@ -144,25 +167,26 @@ bool GDBRemoteClientBase::SendAsyncSignal(int signo) {
   return true;
 }
 
-bool GDBRemoteClientBase::Interrupt() {
-  Lock lock(*this, true);
+bool GDBRemoteClientBase::Interrupt(std::chrono::seconds interrupt_timeout) {
+  Lock lock(*this, interrupt_timeout);
   if (!lock.DidInterrupt())
     return false;
   m_should_stop = true;
   return true;
 }
+
 GDBRemoteCommunication::PacketResult
 GDBRemoteClientBase::SendPacketAndWaitForResponse(
     llvm::StringRef payload, StringExtractorGDBRemote &response,
-    bool send_async) {
-  Lock lock(*this, send_async);
+    std::chrono::seconds interrupt_timeout) {
+  Lock lock(*this, interrupt_timeout);
   if (!lock) {
     if (Log *log =
             ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS))
       LLDB_LOGF(log,
                 "GDBRemoteClientBase::%s failed to get mutex, not sending "
-                "packet '%.*s' (send_async=%d)",
-                __FUNCTION__, int(payload.size()), payload.data(), send_async);
+                "packet '%.*s'",
+                __FUNCTION__, int(payload.size()), payload.data());
     return PacketResult::ErrorSendFailed;
   }
 
@@ -172,16 +196,16 @@ GDBRemoteClientBase::SendPacketAndWaitForResponse(
 GDBRemoteCommunication::PacketResult
 GDBRemoteClientBase::SendPacketAndReceiveResponseWithOutputSupport(
     llvm::StringRef payload, StringExtractorGDBRemote &response,
-    bool send_async,
+    std::chrono::seconds interrupt_timeout,
     llvm::function_ref<void(llvm::StringRef)> output_callback) {
-  Lock lock(*this, send_async);
+  Lock lock(*this, interrupt_timeout);
   if (!lock) {
     if (Log *log =
             ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS))
       LLDB_LOGF(log,
                 "GDBRemoteClientBase::%s failed to get mutex, not sending "
-                "packet '%.*s' (send_async=%d)",
-                __FUNCTION__, int(payload.size()), payload.data(), send_async);
+                "packet '%.*s'",
+                __FUNCTION__, int(payload.size()), payload.data());
     return PacketResult::ErrorSendFailed;
   }
 
@@ -222,13 +246,14 @@ GDBRemoteClientBase::SendPacketAndWaitForResponseNoLock(
   return packet_result;
 }
 
-bool GDBRemoteClientBase::SendvContPacket(llvm::StringRef payload,
-                                          StringExtractorGDBRemote &response) {
+bool GDBRemoteClientBase::SendvContPacket(
+    llvm::StringRef payload, std::chrono::seconds interrupt_timeout,
+    StringExtractorGDBRemote &response) {
   Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS));
   LLDB_LOGF(log, "GDBRemoteCommunicationClient::%s ()", __FUNCTION__);
 
   // we want to lock down packet sending while we continue
-  Lock lock(*this, true);
+  Lock lock(*this, interrupt_timeout);
 
   LLDB_LOGF(log,
             "GDBRemoteCommunicationClient::%s () sending vCont packet: %.*s",
@@ -336,18 +361,20 @@ GDBRemoteClientBase::ContinueLock::lock() {
 // GDBRemoteClientBase::Lock //
 ///////////////////////////////
 
-GDBRemoteClientBase::Lock::Lock(GDBRemoteClientBase &comm, bool interrupt)
+GDBRemoteClientBase::Lock::Lock(GDBRemoteClientBase &comm,
+                                std::chrono::seconds interrupt_timeout)
     : m_async_lock(comm.m_async_mutex, std::defer_lock), m_comm(comm),
-      m_acquired(false), m_did_interrupt(false) {
-  SyncWithContinueThread(interrupt);
+      m_interrupt_timeout(interrupt_timeout), m_acquired(false),
+      m_did_interrupt(false) {
+  SyncWithContinueThread();
   if (m_acquired)
     m_async_lock.lock();
 }
 
-void GDBRemoteClientBase::Lock::SyncWithContinueThread(bool interrupt) {
+void GDBRemoteClientBase::Lock::SyncWithContinueThread() {
   Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS));
   std::unique_lock<std::mutex> lock(m_comm.m_mutex);
-  if (m_comm.m_is_running && !interrupt)
+  if (m_comm.m_is_running && m_interrupt_timeout == std::chrono::seconds(0))
     return; // We were asked to avoid interrupting the sender. Lock is not
             // acquired.
 
@@ -365,9 +392,9 @@ void GDBRemoteClientBase::Lock::SyncWithContinueThread(bool interrupt) {
                        "interrupt packet");
         return;
       }
+      m_comm.m_interrupt_endpoint = steady_clock::now() + m_interrupt_timeout;
       if (log)
         log->PutCString("GDBRemoteClientBase::Lock::Lock sent packet: \\x03");
-      m_comm.m_interrupt_time = steady_clock::now();
     }
     m_comm.m_cv.wait(lock, [this] { return !m_comm.m_is_running; });
     m_did_interrupt = true;

lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h

@@ -33,29 +33,46 @@ class GDBRemoteClientBase : public GDBRemoteCommunication {
 
   GDBRemoteClientBase(const char *comm_name, const char *listener_name);
 
-  bool SendAsyncSignal(int signo);
+  bool SendAsyncSignal(int signo, std::chrono::seconds interrupt_timeout);
 
-  bool Interrupt();
+  bool Interrupt(std::chrono::seconds interrupt_timeout);
 
   lldb::StateType SendContinuePacketAndWaitForResponse(
       ContinueDelegate &delegate, const UnixSignals &signals,
-      llvm::StringRef payload, StringExtractorGDBRemote &response);
-
-  PacketResult SendPacketAndWaitForResponse(llvm::StringRef payload,
-                                            StringExtractorGDBRemote &response,
-                                            bool send_async);
+      llvm::StringRef payload, std::chrono::seconds interrupt_timeout,
+      StringExtractorGDBRemote &response);
+
+  // If interrupt_timeout == 0 seconds, don't interrupt the target.
+  // Only send the packet if the target is stopped.
+  // If you want to use this mode, use the fact that the timeout is defaulted
+  // so it's clear from the call-site that you are using no-interrupt.
+  // If it is non-zero, interrupt the target if it is running, and
+  // send the packet.
+  // It the target doesn't respond within the given timeout, it returns
+  // ErrorReplyTimeout.
+  PacketResult SendPacketAndWaitForResponse(
+      llvm::StringRef payload, StringExtractorGDBRemote &response,
+      std::chrono::seconds interrupt_timeout = std::chrono::seconds(0));
 
   PacketResult SendPacketAndReceiveResponseWithOutputSupport(
       llvm::StringRef payload, StringExtractorGDBRemote &response,
-      bool send_async,
+      std::chrono::seconds interrupt_timeout,
       llvm::function_ref<void(llvm::StringRef)> output_callback);
 
   bool SendvContPacket(llvm::StringRef payload,
+                       std::chrono::seconds interrupt_timeout,
                        StringExtractorGDBRemote &response);
 
   class Lock {
   public:
-    Lock(GDBRemoteClientBase &comm, bool interrupt);
+    // If interrupt_timeout == 0 seconds, only take the lock if the target is
+    // not running. If using this option, use the fact that the
+    // interrupt_timeout is defaulted so it will be obvious at the call site
+    // that you are choosing this mode. If it is non-zero, interrupt the target
+    // if it is running, waiting for the given timeout for the interrupt to
+    // succeed.
+    Lock(GDBRemoteClientBase &comm,
+         std::chrono::seconds interrupt_timeout = std::chrono::seconds(0));
     ~Lock();
 
     explicit operator bool() { return m_acquired; }
@@ -67,10 +84,11 @@ class GDBRemoteClientBase : public GDBRemoteCommunication {
   private:
     std::unique_lock<std::recursive_mutex> m_async_lock;
     GDBRemoteClientBase &m_comm;
+    std::chrono::seconds m_interrupt_timeout;
     bool m_acquired;
     bool m_did_interrupt;
 
-    void SyncWithContinueThread(bool interrupt);
+    void SyncWithContinueThread();
   };
 
 protected:
@@ -109,7 +127,7 @@ class GDBRemoteClientBase : public GDBRemoteCommunication {
 
   /// When was the interrupt packet sent. Used to make sure we time out if the
   /// stub does not respond to interrupt requests.
-  std::chrono::time_point<std::chrono::steady_clock> m_interrupt_time;
+  std::chrono::time_point<std::chrono::steady_clock> m_interrupt_endpoint;
 
   /// Number of threads interested in sending.
   uint32_t m_async_count;