Introduce chrono to more gdb-remote functions

[labath]

Summary

This replaces the usage of raw integers with duration classes in the gdb-remote
packet management functions. The values are still converted back to integers once
they go into the generic Communication class -- that I am leaving to a separate
change.

The changes are mostly straight-forward (*). The thing I'd most like feedback on
is the representation of timeouts.

Currently, we use UINT32_MAX to denote infinite timeout. This is not well suited
for duration classes, as they tend to do arithmetic on the values, and the
identity of the MAX value can easily get lost (e.g.
microseconds(seconds(UINT32_MAX)).count() != UINT32_MAX). We cannot use zero to
represent infinity (as Listener classes do) because we already use it to do
non-blocking polling reads. For this reason, I believe it is better to have an
explicit value for infinity.

The way I achieved that is via llvm::Optional, and I think it reads quite
natural. Passing llvm::None as "timeout" means "no timeout", while passing zero
means "poll". The only tricky part is this breaks implicit conversions (seconds
are implicitly convertible to microseconds, but Optional<seconds> cannot be
easily converted into Optional<microseconds>). To make this less annoying I have
switched a couple of places from using seconds to microseconds to make the code
flow better.

I think other places could also benefit from the same pattern (e.g., if Listener
functions accepted an llvm::Optional timeout, then we wouldn't need to have
separate Peek* and Get* versions of the functions).

(*) The other tricky part was GDBRemoteCommunication::PopPacketFromQueue, which
was needlessly complicated. I've simplified it, but that one is only used in
non-stop mode, and so is untested.

[clayborg]

It would be nice if we can find a way to be able to use any time units we want when specifying the timeout. I would rather not have everyone have to convert their times to microseconds. Maybe we just add some overloads to the public facing functions and have just one internal function that uses llvm::Optional.

As a side note I would also like us to fix llvm::Optional so it can be debugged. Right now if you expand an llvm::Optional<T> you see the an array of characters instead of your type T which means when debugging you can't see your type's value. Boost correctly uses C++11 unions so that we can see the type when expanded in the debugger.

source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp

136 ↗

(On Diff #78865)

Can me make this clearer with something like:

#ifdef LLDB_CONFIGURATION_DEBUG
    m_packet_timeout(std::chrono::seconds(1000));
#else
    m_packet_timeout(std::chrono::seconds(1));
#endif

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

125 ↗

(On Diff #78865)

Can't we do:

const std::chrono::milliseconds timeout(10);

Or is this what you mentioned with the llvm::Optional stuff in your description?

206 ↗

(On Diff #78865)

It would be nice if we can just use std::chrono::seconds. I would hope this line doesn't need to change?

[zturner]

In D26971#602738, @clayborg wrote:

It would be nice if we can find a way to be able to use any time units we want when specifying the timeout. I would rather not have everyone have to convert their times to microseconds. Maybe we just add some overloads to the public facing functions and have just one internal function that uses llvm::Optional.

As a side note I would also like us to fix llvm::Optional so it can be debugged. Right now if you expand an llvm::Optional<T> you see the an array of characters instead of your type T which means when debugging you can't see your type's value. Boost correctly uses C++11 unions so that we can see the type when expanded in the debugger.

Couldn't someone just add a data formatter for llvm::Optional<> to lldb?

[zturner]

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

56 ↗

(On Diff #78865)

I think we should all be willing to agree that using namespace std::chrono is fine in CPP files. The namespace is not particularly large, and it makes code much nicer to read.

249 ↗

(On Diff #78865)

You can also write using namespace std::chrono::literals at the top of the file, and then you can write:

ReadPacket(extra_stop_reply_packet, 100ms, false);

source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp

136 ↗

(On Diff #78865)

Or if using literals, 1000s or 1s

source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h

83 ↗

(On Diff #78865)

Could this class be templated on duration type? So we could use us, ms, or whatever pleased us?

232 ↗

(On Diff #78865)

One idea might be to have

template <typename T>
using GdbTimeout = llvm::Optional<std::chrono::duration<int, T>

then you could write:

PacketResult ReadPacket(StringExtractorGDBRemote &response, GdbTimeout<std::micro> timeout);

[labath]

I suppose we could create a new class (I am not sure what a good name for that would be... OptionalDuration, which inherits from llvm:Optional, but provides the additional conversion operator to make things work seamlessly. Something like:

template<typename Ratio>
class OptionalDur<Ratio>: public llvm::Optional<std::chrono::duration<Ratio>> {
  template<typename Ratio2, typename = std::enable_if<std::is_convertible<duration<Ratio2>, duration<Ratio>>>::type>
  OptionalDur(OptionalDur<Ratio2> other) { ...}
};

It will be a bunch of template-fu, but it can be made to work. What do you think? (I am also open to other ideas on how to implement this....)

For the formatting, I am not sure what it would take to make Optional use a union, but I can certainly make a data formatter for it. I've been planning to dig into that area soon anyway...

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

125 ↗

(On Diff #78865)

Yes, that's exactly what I meant.

[labath]

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

56 ↗

(On Diff #78865)

I am fine with that. Should we make that an official policy somewhere?

249 ↗

(On Diff #78865)

I like literals as well, but I wasn't sure what is the general opinion about them.

source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h

83 ↗

(On Diff #78865)

You don't need templates for this. Thanks to implicit conversions it can be used with any type with a coarser granularity than microseconds. It's only when Optional comes into the picture that things start to break down.

232 ↗

(On Diff #78865)

That sounds fine, although I would put the class in some place more general, as I plan to use it outside the gdb plugin as well.

[labath]

In D26971#602738, @clayborg wrote:

It would be nice if we can find a way to be able to use any time units we want when specifying the timeout. I would rather not have everyone have to convert their times to microseconds. Maybe we just add some overloads to the public facing functions and have just one internal function that uses llvm::Optional.

I am not sure what would the public-facing functions be in this case. All of the code here is internal to the plugin, but I was definitely annoyed when I had to convert everything to microsecs. I am afraid we will have to make overloads of many functions to make the usages here clean.

I am going to try to see how hairy the new optional class ends up looking. I like that more as it concentrates the uglyness into one place.

[labath]

A new version, which uses a helper class which enables all the implicit
conversions that one would normally expect from the duration classes.

Things left TBD:

• name
• where to put it
• whether inheriting from Optional is fine (it is slightly dodgy, but it reduces boilerplate).
• using namespace std::chrono, where it makes sense.

[labath]

Changes since last version:

• Rename the class to Timeout<>, to reflect the fact that I'd like to use it as a general method for describing timeouts.
• add "using namespace std::chrono" to files with significat chrono usage.

I did not use chrono literals, as those are a c++14 feature. I don't think that
matters much as thanks to the previous import, something like seconds(X) is quite
short already.

Let me know what you think.

[clayborg]

Much better, looks good.

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

 #include "lldb/Target/UnixSignals.h"
 #include "lldb/Utility/LLDBAssert.h"
 
 #include "ProcessGDBRemoteLog.h"
 
 using namespace lldb;
 using namespace lldb_private;
 using namespace lldb_private::process_gdb_remote;
+using namespace std::chrono;
 
-static const std::chrono::seconds kInterruptTimeout(5);
+static const seconds kInterruptTimeout(5);
 
 /////////////////////////
 // GDBRemoteClientBase //
 /////////////////////////
 
 GDBRemoteClientBase::ContinueDelegate::~ContinueDelegate() = default;
 
 GDBRemoteClientBase::GDBRemoteClientBase(const char *comm_name,
response.Clear();
     m_should_stop = false;
   }
   ContinueLock cont_lock(*this);
   if (!cont_lock)
     return eStateInvalid;
   OnRunPacketSent(true);
 
   for (;;) {
-    PacketResult read_result = ReadPacket(
-        response,
-        std::chrono::duration_cast<std::chrono::microseconds>(kInterruptTimeout)
-            .count(),
-        false);
+    PacketResult read_result = ReadPacket(response, kInterruptTimeout, false);
     switch (read_result) {
     case PacketResult::ErrorReplyTimeout: {
       std::lock_guard<std::mutex> lock(m_mutex);
       if (m_async_count == 0)
         continue;
-      if (std::chrono::steady_clock::now() >=
-          m_interrupt_time + kInterruptTimeout)
+      if (steady_clock::now() >= m_interrupt_time + kInterruptTimeout)
         return eStateInvalid;
     }
     case PacketResult::Success:
       break;
     default:
       if (log)
         log->Printf("GDBRemoteClientBase::%s () ReadPacket(...) => false",
                     __FUNCTION__);

 GDBRemoteClientBase::SendPacketAndWaitForResponseNoLock(
     llvm::StringRef payload, StringExtractorGDBRemote &response) {
   PacketResult packet_result = SendPacketNoLock(payload);
   if (packet_result != PacketResult::Success)
     return packet_result;
 
   const size_t max_response_retries = 3;
   for (size_t i = 0; i < max_response_retries; ++i) {
-    packet_result = ReadPacket(
-        response, std::chrono::duration_cast<std::chrono::microseconds>(
-                      GetPacketTimeout())
-                      .count(),
-        true);
+    packet_result = ReadPacket(response, GetPacketTimeout(), true);
     // Make sure we received a response
     if (packet_result != PacketResult::Success)
       return packet_result;
     // Make sure our response is valid for the payload that was sent
     if (response.ValidateResponse())
       return packet_result;
     // Response says it wasn't valid
     Log *log = ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PACKETS);
log->Printf(
         __FUNCTION__, int(payload.size()), payload.data());
 
   if (SendPacketNoLock(payload) != PacketResult::Success)
     return false;
 
   OnRunPacketSent(true);
 
   // wait for the response to the vCont
-  if (ReadPacket(response, UINT32_MAX, false) == PacketResult::Success) {
+  if (ReadPacket(response, llvm::None, false) == PacketResult::Success) {
     if (response.IsOKResponse())
       return true;
   }
 
   return false;
 }
 bool GDBRemoteClientBase::ShouldStop(const UnixSignals &signals,
                                      StringExtractorGDBRemote &response) {
   std::lock_guard<std::mutex> lock(m_mutex);
 
   if (m_async_count == 0)
     return true; // We were not interrupted. The process stopped on its own.
 
   // Older debugserver stubs (before April 2016) can return two
   // stop-reply packets in response to a ^C packet.
   // Additionally, all debugservers still return two stop replies if
   // the inferior stops due to some other reason before the remote
   // stub manages to interrupt it. We need to wait for this
   // additional packet to make sure the packet sequence does not get
   // skewed.
   StringExtractorGDBRemote extra_stop_reply_packet;
-  uint32_t timeout_usec = 100000; // 100ms
-  ReadPacket(extra_stop_reply_packet, timeout_usec, false);
+  ReadPacket(extra_stop_reply_packet, milliseconds(100), false);
 
   // Interrupting is typically done using SIGSTOP or SIGINT, so if
   // the process stops with some other signal, we definitely want to
   // stop.
   const uint8_t signo = response.GetHexU8(UINT8_MAX);
   if (signo != signals.GetSignalNumberFromName("SIGSTOP") &&
       signo != signals.GetSignalNumberFromName("SIGINT"))
     return true;
 
   // We probably only stopped to perform some async processing, so continue
   // after that is done.
   // TODO: This is not 100% correct, as the process may have been stopped with
-  // SIGINT or SIGSTOP
-  // that was not caused by us (e.g. raise(SIGINT)). This will normally cause a
-  // stop, but if it's
-  // done concurrently with a async interrupt, that stop will get eaten
-  // (llvm.org/pr20231).
+  // SIGINT or SIGSTOP that was not caused by us (e.g. raise(SIGINT)). This will
+  // normally cause a stop, but if it's done concurrently with a async
+  // interrupt, that stop will get eaten (llvm.org/pr20231).
   return false;
 }
 
 void GDBRemoteClientBase::OnRunPacketSent(bool first) {
   if (first)
     BroadcastEvent(eBroadcastBitRunPacketSent, NULL);
 }
 
if (m_comm.m_async_count == 1) {
         --m_comm.m_async_count;
         if (log)
           log->Printf("GDBRemoteClientBase::Lock::Lock failed to send "
                       "interrupt packet");
         return;
       }
       if (log)
         log->PutCString("GDBRemoteClientBase::Lock::Lock sent packet: \\x03");
-      m_comm.m_interrupt_time = std::chrono::steady_clock::now();
+      m_comm.m_interrupt_time = steady_clock::now();
     }
     m_comm.m_cv.wait(lock, [this] { return m_comm.m_is_running == false; });
     m_did_interrupt = true;
   }
   m_acquired = true;
 }
 
 GDBRemoteClientBase::Lock::~Lock() {
   if (!m_acquired)
     return;
   {
     std::unique_lock<std::mutex> lock(m_comm.m_mutex);
     --m_comm.m_async_count;
   }
   m_comm.m_cv.notify_one();
 }

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h

enum class CompressionType {
   LZFSE,       // an Apple compression scheme, requires Apple's libcompression
   LZ4, // lz compression - called "lz4 raw" in libcompression terms, compat with
        // https://code.google.com/p/lz4/
   LZMA, // Lempel–Ziv–Markov chain algorithm
 };
 
 class ProcessGDBRemote;
 
+template <typename Ratio>
+class Timeout : public llvm::Optional<std::chrono::duration<int64_t, Ratio>> {
+private:
+  template <typename Ratio2> using Dur = std::chrono::duration<int64_t, Ratio2>;
+  template <typename Ratio2>
+  using EnableIf = std::enable_if<
+      std::is_convertible<std::chrono::duration<int64_t, Ratio2>,
+                          std::chrono::duration<int64_t, Ratio>>::value>;
+
+  using Base = llvm::Optional<Dur<Ratio>>;
+
+public:
+  Timeout(llvm::NoneType none) : Base(none) {}
+  Timeout(const Timeout &other) = default;
+
+  template <typename Ratio2, typename = typename EnableIf<Ratio2>::type>
+  Timeout(const Timeout<Ratio2> &other)
+      : Base(other ? Base(Dur<Ratio>(*other)) : llvm::None) {}
+
+  template <typename Ratio2, typename = typename EnableIf<Ratio2>::type>
+  Timeout(const Dur<Ratio2> &other) : Base(Dur<Ratio>(other)) {}
+};
+
 class GDBRemoteCommunication : public Communication {
 public:
   enum {
     eBroadcastBitRunPacketSent = kLoUserBroadcastBit,
     eBroadcastBitGdbReadThreadGotNotify =
         kLoUserBroadcastBit << 1 // Sent when we received a notify packet.
   };
 
bool m_is_platform; // Set to true if this class represents a platform,
                       // false if this class represents a debug session for
                       // a single process
 
   CompressionType m_compression_type;
 
   PacketResult SendPacketNoLock(llvm::StringRef payload);
 
   PacketResult ReadPacket(StringExtractorGDBRemote &response,
-                          uint32_t timeout_usec, bool sync_on_timeout);
+                          Timeout<std::micro> timeout, bool sync_on_timeout);
 
   // Pop a packet from the queue in a thread safe manner
   PacketResult PopPacketFromQueue(StringExtractorGDBRemote &response,
-                                  uint32_t timeout_usec);
+                                  Timeout<std::micro> timeout);
 
-  PacketResult
-  WaitForPacketWithTimeoutMicroSecondsNoLock(StringExtractorGDBRemote &response,
-                                             uint32_t timeout_usec,
-                                             bool sync_on_timeout);
+  PacketResult WaitForPacketNoLock(StringExtractorGDBRemote &response,
+                                   Timeout<std::micro> timeout,
 
   bool CompressionIsEnabled() {
     return m_compression_type != CompressionType::None;
   }
 
   // If compression is enabled, decompress the packet in m_bytes and update
   // m_bytes with the uncompressed version.
   // Returns 'true' packet was decompressed and m_bytes is the now-decompressed

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp

if (bytes_written == packet_length) {
                     packet_data);
     }
   }
   return PacketResult::ErrorSendFailed;
 }
 
 GDBRemoteCommunication::PacketResult GDBRemoteCommunication::GetAck() {
   StringExtractorGDBRemote packet;
-  PacketResult result = ReadPacket(
-      packet,
-      std::chrono::duration_cast<std::chrono::microseconds>(GetPacketTimeout())
-          .count(),
-      false);
+  PacketResult result = ReadPacket(packet, GetPacketTimeout(), false);
   if (result == PacketResult::Success) {
     if (packet.GetResponseType() ==
         StringExtractorGDBRemote::ResponseType::eAck)
       return PacketResult::Success;
     else
       return PacketResult::ErrorSendAck;
   }
   return result;
 }
 
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunication::ReadPacket(StringExtractorGDBRemote &response,
-                                   uint32_t timeout_usec,
+                                   Timeout<std::micro> timeout,
                                    bool sync_on_timeout) {
   if (m_read_thread_enabled)
-    return PopPacketFromQueue(response, timeout_usec);
+    return PopPacketFromQueue(response, timeout);
   else
-    return WaitForPacketWithTimeoutMicroSecondsNoLock(response, timeout_usec,
-                                                      sync_on_timeout);
+    return WaitForPacketNoLock(response, timeout, sync_on_timeout);
 }
 
 // This function is called when a packet is requested.
 // A whole packet is popped from the packet queue and returned to the caller.
 // Packets are placed into this queue from the communication read thread.
 // See GDBRemoteCommunication::AppendBytesToCache.
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunication::PopPacketFromQueue(StringExtractorGDBRemote &response,
-                                           uint32_t timeout_usec) {
-  auto until = std::chrono::system_clock::now() +
-               std::chrono::microseconds(timeout_usec);
-
-  while (true) {
-    // scope for the mutex
-    {
-      // lock down the packet queue
-      std::unique_lock<std::mutex> lock(m_packet_queue_mutex);
+                                           Timeout<std::micro> timeout) {
+  auto pred = [&] { return !m_packet_queue.empty() && IsConnected(); };
 
-      // Wait on condition variable.
-      if (m_packet_queue.size() == 0) {
-        std::cv_status result =
-            m_condition_queue_not_empty.wait_until(lock, until);
-        if (result == std::cv_status::timeout)
-          break;
-      }
+  if (!timeout)
+    m_condition_queue_not_empty.wait(lock, pred);
+  else {
+    if (!m_condition_queue_not_empty.wait_for(lock, *timeout, pred))
+      return PacketResult::ErrorReplyTimeout;
+    if (!IsConnected())
+      return PacketResult::ErrorDisconnected;
 
-      if (m_packet_queue.size() > 0) {
-        // get the front element of the queue
-        response = m_packet_queue.front();
 
-        // remove the front element
-        m_packet_queue.pop();
 
-        // we got a packet
-        return PacketResult::Success;
-      }
-    }
-
-    // Disconnected
-    if (!IsConnected())
-      return PacketResult::ErrorDisconnected;
-
-    // Loop while not timed out
-  }
-
-  return PacketResult::ErrorReplyTimeout;
-}
 
 GDBRemoteCommunication::PacketResult
-GDBRemoteCommunication::WaitForPacketWithTimeoutMicroSecondsNoLock(
-    StringExtractorGDBRemote &packet, uint32_t timeout_usec,
+GDBRemoteCommunication::WaitForPacketNoLock(StringExtractorGDBRemote &packet,
+                                            Timeout<std::micro> timeout,
                                             bool sync_on_timeout) {
   uint8_t buffer[8192];
   Error error;
 
   Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PACKETS |
                                                          GDBR_LOG_VERBOSE));
 
   // Check for a packet from our cache first without trying any reading...
   if (CheckForPacket(NULL, 0, packet) != PacketType::Invalid)
     return PacketResult::Success;
 
   bool timed_out = false;
   bool disconnected = false;
   while (IsConnected() && !timed_out) {
     lldb::ConnectionStatus status = eConnectionStatusNoConnection;
     size_t bytes_read =
-        Read(buffer, sizeof(buffer), timeout_usec, status, &error);
+        Read(buffer, sizeof(buffer), timeout ? timeout->count() : UINT32_MAX,
+             status, &error);
 
     if (log)
-      log->Printf("%s: Read (buffer, (sizeof(buffer), timeout_usec = 0x%x, "
+      log->Printf("%s: Read (buffer, (sizeof(buffer), timeout = %ld us, "
                   "status = %s, error = %s) => bytes_read = %" PRIu64,
-                  LLVM_PRETTY_FUNCTION, timeout_usec,
+                  LLVM_PRETTY_FUNCTION, long(timeout ? timeout->count() : -1),
                   Communication::ConnectionStatusAsCString(status),
                   error.AsCString(), (uint64_t)bytes_read);
 
     if (bytes_read > 0) {
       if (CheckForPacket(buffer, bytes_read, packet) != PacketType::Invalid)
         return PacketResult::Success;
     } else {
       switch (status) {
if (bytes_read > 0) {
 
           PacketResult echo_packet_result =
               SendPacketNoLock(llvm::StringRef(echo_packet, echo_packet_len));
           if (echo_packet_result == PacketResult::Success) {
             const uint32_t max_retries = 3;
             uint32_t successful_responses = 0;
             for (uint32_t i = 0; i < max_retries; ++i) {
               StringExtractorGDBRemote echo_response;
-              echo_packet_result = WaitForPacketWithTimeoutMicroSecondsNoLock(
-                  echo_response, timeout_usec, false);
+              echo_packet_result =
+                  WaitForPacketNoLock(echo_response, timeout, false);
               if (echo_packet_result == PacketResult::Success) {
                 ++successful_responses;
                 if (response_regex.Execute(echo_response.GetStringRef())) {
                   sync_success = true;
                   break;
                 } else if (successful_responses == 1) {
                   // We got something else back as the first successful
                   // response, it probably is

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

 
 #if defined(HAVE_LIBCOMPRESSION)
 #include <compression.h>
 #endif
 
 using namespace lldb;
 using namespace lldb_private;
 using namespace lldb_private::process_gdb_remote;
+using namespace std::chrono;
 
 //----------------------------------------------------------------------
 // GDBRemoteCommunicationClient constructor
 //----------------------------------------------------------------------
 GDBRemoteCommunicationClient::GDBRemoteCommunicationClient()
     : GDBRemoteClientBase("gdb-remote.client", "gdb-remote.client.rx_packet"),
       m_supports_not_sending_acks(eLazyBoolCalculate),
       m_supports_thread_suffix(eLazyBoolCalculate),
bool GDBRemoteCommunicationClient::HandshakeWithServer(Error *error_ptr) {
 
   // Start the read thread after we send the handshake ack since if we
   // fail to send the handshake ack, there is no reason to continue...
   if (SendAck()) {
     // Wait for any responses that might have been queued up in the remote
     // GDB server and flush them all
     StringExtractorGDBRemote response;
     PacketResult packet_result = PacketResult::Success;
-    const uint32_t timeout_usec = 10 * 1000; // Wait for 10 ms for a response
     while (packet_result == PacketResult::Success)
-      packet_result = ReadPacket(response, timeout_usec, false);
+      packet_result = ReadPacket(response, milliseconds(10), false);
 
     // The return value from QueryNoAckModeSupported() is true if the packet
     // was sent and _any_ response (including UNIMPLEMENTED) was received),
     // or false if no response was received. This quickly tells us if we have
     // a live connection to a remote GDB server...
     if (QueryNoAckModeSupported()) {
       return true;
     } else {
if (m_supports_not_sending_acks == eLazyBoolCalculate) {
     m_send_acks = true;
     m_supports_not_sending_acks = eLazyBoolNo;
 
     // This is the first real packet that we'll send in a debug session and it
     // may take a little
     // longer than normal to receive a reply.  Wait at least 6 seconds for a
     // reply to this packet.
 
-    ScopedTimeout timeout(
-        *this, std::max(GetPacketTimeout(), std::chrono::seconds(6)));
+    ScopedTimeout timeout(*this, std::max(GetPacketTimeout(), seconds(6)));
 
     StringExtractorGDBRemote response;
     if (SendPacketAndWaitForResponse("QStartNoAckMode", response, false) ==
         PacketResult::Success) {
       if (response.IsOKResponse()) {
         m_send_acks = false;
         m_supports_not_sending_acks = eLazyBoolYes;
       }
if (did_exec == false) {
     m_os_version_major = UINT32_MAX;
     m_os_version_minor = UINT32_MAX;
     m_os_version_update = UINT32_MAX;
     m_os_build.clear();
     m_os_kernel.clear();
     m_hostname.clear();
     m_gdb_server_name.clear();
     m_gdb_server_version = UINT32_MAX;
-    m_default_packet_timeout = std::chrono::seconds(0);
+    m_default_packet_timeout = seconds(0);
     m_max_packet_size = 0;
     m_qSupported_response.clear();
     m_supported_async_json_packets_is_valid = false;
     m_supported_async_json_packets_sp.reset();
     m_supports_jModulesInfo = true;
   }
 
   // These flags should be reset when we first connect to a GDB server
if (SendPacketAndWaitForResponse("qHostInfo", response, false) ==
                     .Case("before", eLazyBoolNo)
                     .Case("after", eLazyBoolYes)
                     .Default(eLazyBoolCalculate);
             if (m_watchpoints_trigger_after_instruction != eLazyBoolCalculate)
               ++num_keys_decoded;
           } else if (name.equals("default_packet_timeout")) {
             uint32_t timeout_seconds;
             if (!value.getAsInteger(0, timeout_seconds)) {
-              m_default_packet_timeout = std::chrono::seconds(timeout_seconds);
+              m_default_packet_timeout = seconds(timeout_seconds);
               SetPacketTimeout(m_default_packet_timeout);
               ++num_keys_decoded;
             }
           }
         }
 
         if (num_keys_decoded > 0)
           m_qHostInfo_is_valid = eLazyBoolYes;

 
 const lldb_private::ArchSpec &
 GDBRemoteCommunicationClient::GetHostArchitecture() {
   if (m_qHostInfo_is_valid == eLazyBoolCalculate)
     GetHostInfo();
   return m_host_arch;
 }
 
-std::chrono::seconds
-GDBRemoteCommunicationClient::GetHostDefaultPacketTimeout() {
+seconds GDBRemoteCommunicationClient::GetHostDefaultPacketTimeout() {
   if (m_qHostInfo_is_valid == eLazyBoolCalculate)
     GetHostInfo();
   return m_default_packet_timeout;
 }
 
 addr_t GDBRemoteCommunicationClient::AllocateMemory(size_t size,
                                                     uint32_t permissions) {
   if (m_supports_alloc_dealloc_memory != eLazyBoolNo) {
if (!match_info.MatchAllProcesses()) {
         packet.PutCString("triple:");
         packet.PutCString(triple.getTriple());
         packet.PutChar(';');
       }
     }
     StringExtractorGDBRemote response;
     // Increase timeout as the first qfProcessInfo packet takes a long time
     // on Android. The value of 1min was arrived at empirically.
-    ScopedTimeout timeout(*this, std::chrono::seconds(60));
+    ScopedTimeout timeout(*this, minutes(1));
     if (SendPacketAndWaitForResponse(packet.GetString(), response, false) ==
         PacketResult::Success) {
       do {
         ProcessInstanceInfo process_info;
         if (!DecodeProcessInfoResponse(response, process_info))
           break;
         process_infos.Append(process_info);
         response.GetStringRef().clear();
while (bytes_left > 0) {
     } else {
       packet.Printf("%*.*s;", bytes_left, bytes_left,
                     "abcdefghijklmnopqrstuvwxyz");
       bytes_left = 0;
     }
   }
 }
 
-std::chrono::duration<float> calculate_standard_deviation(
-    const std::vector<std::chrono::duration<float>> &v) {
-  using Dur = std::chrono::duration<float>;
+duration<float>
+calculate_standard_deviation(const std::vector<duration<float>> &v) {
+  using Dur = duration<float>;
   Dur sum = std::accumulate(std::begin(v), std::end(v), Dur());
   Dur mean = sum / v.size();
   float accum = 0;
   for (auto d : v) {
     float delta = (d - mean).count();
     accum += delta * delta;
   };
 
   return Dur(sqrtf(accum / (v.size() - 1)));
 }
 
 void GDBRemoteCommunicationClient::TestPacketSpeed(const uint32_t num_packets,
                                                    uint32_t max_send,
                                                    uint32_t max_recv,
                                                    uint64_t recv_amount,
                                                    bool json, Stream &strm) {
-  using namespace std::chrono;
-
   uint32_t i;
   if (SendSpeedTestPacket(0, 0)) {
     StreamString packet;
     if (json)
       strm.Printf("{ \"packet_speeds\" : {\n    \"num_packets\" : %u,\n    "
                   "\"results\" : [",
                   num_packets);
     else
if (HostInfo::GetHostname(hostname)) {
       stream.Printf("host:%s;", hostname.c_str());
     } else {
       // Make the GDB server we launch accept connections from any host since we
       // can't figure out the hostname
       stream.Printf("host:*;");
     }
   }
   // give the process a few seconds to startup
-  ScopedTimeout timeout(*this, std::chrono::seconds(10));
+  ScopedTimeout timeout(*this, seconds(10));
 
   if (SendPacketAndWaitForResponse(stream.GetString(), response, false) ==
       PacketResult::Success) {
     llvm::StringRef name;
     llvm::StringRef value;
     while (response.GetNameColonValue(name, value)) {
       if (name.equals("port"))
         value.getAsInteger(0, port);

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h

GDBRemoteCommunicationServer(const char *comm_name,
                                const char *listener_name);
 
   ~GDBRemoteCommunicationServer() override;
 
   void
   RegisterPacketHandler(StringExtractorGDBRemote::ServerPacketType packet_type,
                         PacketHandler handler);
 
-  PacketResult GetPacketAndSendResponse(uint32_t timeout_usec, Error &error,
-                                        bool &interrupt, bool &quit);
+  PacketResult GetPacketAndSendResponse(Timeout<std::micro> timeout,
+                                        Error &error, bool &interrupt,
+                                        bool &quit);
 
   // After connecting, do a little handshake with the client to make sure
   // we are at least communicating
   bool HandshakeWithClient();
 
 protected:
   std::map<StringExtractorGDBRemote::ServerPacketType, PacketHandler>
       m_packet_handlers;

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp

 
 void GDBRemoteCommunicationServer::RegisterPacketHandler(
     StringExtractorGDBRemote::ServerPacketType packet_type,
     PacketHandler handler) {
   m_packet_handlers[packet_type] = std::move(handler);
 }
 
 GDBRemoteCommunication::PacketResult
-GDBRemoteCommunicationServer::GetPacketAndSendResponse(uint32_t timeout_usec,
-                                                       Error &error,
-                                                       bool &interrupt,
-                                                       bool &quit) {
+GDBRemoteCommunicationServer::GetPacketAndSendResponse(
+    Timeout<std::micro> timeout, Error &error, bool &interrupt, bool &quit) {
   StringExtractorGDBRemote packet;
 
-  PacketResult packet_result =
-      WaitForPacketWithTimeoutMicroSecondsNoLock(packet, timeout_usec, false);
+  PacketResult packet_result = WaitForPacketNoLock(packet, timeout, false);
   if (packet_result == PacketResult::Success) {
     const StringExtractorGDBRemote::ServerPacketType packet_type =
         packet.GetServerPacketType();
     switch (packet_type) {
     case StringExtractorGDBRemote::eServerPacketType_nack:
     case StringExtractorGDBRemote::eServerPacketType_ack:
       break;
 

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

if (!m_handshake_completed) {
     }
     m_handshake_completed = true;
   }
 
   bool interrupt = false;
   bool done = false;
   Error error;
   while (true) {
-    const PacketResult result =
-        GetPacketAndSendResponse(0, error, interrupt, done);
+    const PacketResult result = GetPacketAndSendResponse(
+        std::chrono::microseconds(0), error, interrupt, done);
     if (result == PacketResult::ErrorReplyTimeout)
       break; // No more packets in the queue
 
     if ((result != PacketResult::Success)) {
       if (log)
         log->Printf("GDBRemoteCommunicationServerLLGS::%s processing a packet "
                     "failed: %s",
                     __FUNCTION__, error.AsCString());

lldb/trunk/tools/lldb-server/lldb-platform.cpp

if (platform.IsConnected()) {
           fprintf(stderr, "failed to start gdbserver: %s\n", error.AsCString());
       }
 
       // After we connected, we need to get an initial ack from...
       if (platform.HandshakeWithClient()) {
         bool interrupt = false;
         bool done = false;
         while (!interrupt && !done) {
-          if (platform.GetPacketAndSendResponse(UINT32_MAX, error, interrupt,
+          if (platform.GetPacketAndSendResponse(llvm::None, error, interrupt,
                                                 done) !=
               GDBRemoteCommunication::PacketResult::Success)
             break;
         }
 
         if (error.Fail()) {
           fprintf(stderr, "error: %s\n", error.AsCString());
         }