Add a new "qEcho" packet with the following format:

qEcho:%s

where '%s' is any valid string. The response to this packet is the exact packet itself with no changes, just reply with what you received!

This will help us to recover from packets timing out much more gracefully. Currently if a packet times out, LLDB quickly will hose up the debug session. For example, if we send a "abc" packet and we expect "ABC" back in response, but the "abc" command takes longer than the current timeout value this will happen:


--> "abc"
<-- <<<error: timeout>>>

Now we want to send "def" and get "DEF" back:

--> "def"
<-- "ABC"

We got the wrong response for the "def" packet because we didn't sync up with the server to clear any current responses from previously issues commands.

The fix is to modify GDBRemoteCommunication::WaitForPacketWithTimeoutMicroSecondsNoLock() so that when it gets a timeout, it syncs itself up with the client by sending a "qEcho:%u" where %u is an increasing integer, one for each time we timeout. We then wait for 3 timeout periods to sync back up. So the above "abc" session would look like:

--> "abc"
<-- <<<error: timeout>>> 1 second
--> "qEcho:1"
<-- <<<error: timeout>>> 1 second
<-- <<<error: timeout>>> 1 second
<-- "abc"
<-- "qEcho:1"

The first timeout is from trying to get the response, then we know we timed out and we send the "qEcho:1" packet and wait for 3 timeout periods to get back in sync knowing that we might actually get the response for the "abc" packet in the mean time...

In this case we would actually succeed in getting the response for "abc". But lets say the remote GDB server is deadlocked and will never response, it would look like:

--> "abc"
<-- <<<error: timeout>>> 1 second
--> "qEcho:1"
<-- <<<error: timeout>>> 1 second
<-- <<<error: timeout>>> 1 second
<-- <<<error: timeout>>> 1 second

We then disconnect and say we lost connection.

We might also have a bad GDB server that just dropped the "abc" packet on the floor. We can still recover in this case and it would look like:

--> "abc"
<-- <<<error: timeout>>> 1 second
--> "qEcho:1"
<-- "qEcho:1"

Then we know our remote GDB server is still alive and well, and it just dropped the "abc" response on the floor and we can continue to debug.

<rdar://problem/21082939>

llvm-svn: 238530

6 files changed, 137 insertions, 13 deletions

diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
index 38942ec2d17..652af4bdb7a 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -18,6 +18,7 @@
 // C++ Includes
 // Other libraries and framework includes
 #include "lldb/Core/Log.h"
+#include "lldb/Core/RegularExpression.h"
 #include "lldb/Core/StreamFile.h"
 #include "lldb/Core/StreamString.h"
 #include "lldb/Host/ConnectionFileDescriptor.h"
@@ -150,6 +151,8 @@ GDBRemoteCommunication::GDBRemoteCommunication(const char *comm_name,
 #else
     m_packet_timeout (1),
 #endif
+    m_echo_number(0),
+    m_supports_qEcho (eLazyBoolCalculate),
     m_sequence_mutex (Mutex::eMutexTypeRecursive),
     m_public_is_running (false),
     m_private_is_running (false),
@@ -292,7 +295,7 @@ GDBRemoteCommunication::PacketResult
 GDBRemoteCommunication::GetAck ()
 {
     StringExtractorGDBRemote packet;
-    PacketResult result = WaitForPacketWithTimeoutMicroSecondsNoLock (packet, GetPacketTimeoutInMicroSeconds ());
+    PacketResult result = WaitForPacketWithTimeoutMicroSecondsNoLock (packet, GetPacketTimeoutInMicroSeconds (), false);
     if (result == PacketResult::Success)
     {
         if (packet.GetResponseType() == StringExtractorGDBRemote::ResponseType::eAck)
@@ -321,7 +324,7 @@ GDBRemoteCommunication::WaitForNotRunningPrivate (const TimeValue *timeout_ptr)
 }
 
 GDBRemoteCommunication::PacketResult
-GDBRemoteCommunication::WaitForPacketWithTimeoutMicroSecondsNoLock (StringExtractorGDBRemote &packet, uint32_t timeout_usec)
+GDBRemoteCommunication::WaitForPacketWithTimeoutMicroSecondsNoLock (StringExtractorGDBRemote &packet, uint32_t timeout_usec, bool sync_on_timeout)
 {
     uint8_t buffer[8192];
     Error error;
@@ -358,6 +361,104 @@ GDBRemoteCommunication::WaitForPacketWithTimeoutMicroSecondsNoLock (StringExtrac
             {
             case eConnectionStatusTimedOut:
             case eConnectionStatusInterrupted:
+                if (sync_on_timeout)
+                {
+                    //------------------------------------------------------------------
+                    /// Sync the remote GDB server and make sure we get a response that
+                    /// corresponds to what we send.
+                    ///
+                    /// Sends a "qEcho" packet and makes sure it gets the exact packet
+                    /// echoed back. If the qEcho packet isn't supported, we send a qC
+                    /// packet and make sure we get a valid thread ID back. We use the
+                    /// "qC" packet since its response if very unique: is responds with
+                    /// "QC%x" where %x is the thread ID of the current thread. This
+                    /// makes the response unique enough from other packet responses to
+                    /// ensure we are back on track.
+                    ///
+                    /// This packet is needed after we time out sending a packet so we
+                    /// can ensure that we are getting the response for the packet we
+                    /// are sending. There are no sequence IDs in the GDB remote
+                    /// protocol (there used to be, but they are not supported anymore)
+                    /// so if you timeout sending packet "abc", you might then send
+                    /// packet "cde" and get the response for the previous "abc" packet.
+                    /// Many responses are "OK" or "" (unsupported) or "EXX" (error) so
+                    /// many responses for packets can look like responses for other
+                    /// packets. So if we timeout, we need to ensure that we can get
+                    /// back on track. If we can't get back on track, we must
+                    /// disconnect.
+                    //------------------------------------------------------------------
+                    bool sync_success = false;
+                    bool got_actual_response = false;
+                    // We timed out, we need to sync back up with the
+                    char echo_packet[32];
+                    int echo_packet_len = 0;
+                    RegularExpression response_regex;
+
+                    if (m_supports_qEcho == eLazyBoolYes)
+                    {
+                        echo_packet_len = ::snprintf (echo_packet, sizeof(echo_packet), "qEcho:%u", ++m_echo_number);
+                        std::string regex_str = "^";
+                        regex_str += echo_packet;
+                        regex_str += "$";
+                        response_regex.Compile(regex_str.c_str());
+                    }
+                    else
+                    {
+                        echo_packet_len = ::snprintf (echo_packet, sizeof(echo_packet), "qC");
+                        response_regex.Compile("^QC[0-9A-Fa-f]+$");
+                    }
+
+                    PacketResult echo_packet_result = SendPacketNoLock (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);
+                            if (echo_packet_result == PacketResult::Success)
+                            {
+                                ++successful_responses;
+                                if (response_regex.Execute(echo_response.GetStringRef().c_str()))
+                                {
+                                    sync_success = true;
+                                    break;
+                                }
+                                else if (successful_responses == 1)
+                                {
+                                    // We got something else back as the first successful response, it probably is
+                                    // the  response to the packet we actually wanted, so copy it over if this
+                                    // is the first success and continue to try to get the qEcho response
+                                    packet = echo_response;
+                                    got_actual_response = true;
+                                }
+                            }
+                            else if (echo_packet_result == PacketResult::ErrorReplyTimeout)
+                                continue;   // Packet timed out, continue waiting for a response
+                            else
+                                break;      // Something else went wrong getting the packet back, we failed and are done trying
+                        }
+                    }
+
+                    // We weren't able to sync back up with the server, we must abort otherwise
+                    // all responses might not be from the right packets...
+                    if (sync_success)
+                    {
+                        // We timed out, but were able to recover
+                        if (got_actual_response)
+                        {
+                            // We initially timed out, but we did get a response that came in before the successful
+                            // reply to our qEcho packet, so lets say everything is fine...
+                            return PacketResult::Success;
+                        }
+                    }
+                    else
+                    {
+                        disconnected = true;
+                        Disconnect();
+                    }
+                }
                 timed_out = true;
                 break;
             case eConnectionStatusSuccess:
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
index 6fd1b854d8a..011adec9e4e 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
@@ -281,7 +281,8 @@ protected:
 
     PacketResult
     WaitForPacketWithTimeoutMicroSecondsNoLock (StringExtractorGDBRemote &response, 
-                                                uint32_t timeout_usec);
+                                                uint32_t timeout_usec,
+                                                bool sync_on_timeout);
 
     bool
     WaitForNotRunningPrivate (const TimeValue *timeout_ptr);
@@ -290,6 +291,8 @@ protected:
     // Classes that inherit from GDBRemoteCommunication can see and modify these
     //------------------------------------------------------------------
     uint32_t m_packet_timeout;
+    uint32_t m_echo_number;
+    LazyBool m_supports_qEcho;
 #ifdef ENABLE_MUTEX_ERROR_CHECKING
     TrackingMutex m_sequence_mutex;
 #else
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index a452ae95b07..3e9278853a7 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -145,7 +145,7 @@ GDBRemoteCommunicationClient::HandshakeWithServer (Error *error_ptr)
         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 = WaitForPacketWithTimeoutMicroSecondsNoLock (response, timeout_usec);
+            packet_result = WaitForPacketWithTimeoutMicroSecondsNoLock (response, timeout_usec, false);
 
         // The return value from QueryNoAckModeSupported() is true if the packet
         // was sent and _any_ response (including UNIMPLEMENTED) was received),
@@ -170,13 +170,24 @@ GDBRemoteCommunicationClient::HandshakeWithServer (Error *error_ptr)
 }
 
 bool
+GDBRemoteCommunicationClient::GetEchoSupported ()
+{
+    if (m_supports_qEcho == eLazyBoolCalculate)
+    {
+        GetRemoteQSupported();
+    }
+    return m_supports_qEcho == eLazyBoolYes;
+}
+
+
+bool
 GDBRemoteCommunicationClient::GetAugmentedLibrariesSVR4ReadSupported ()
 {
     if (m_supports_augmented_libraries_svr4_read == eLazyBoolCalculate)
     {
         GetRemoteQSupported();
     }
-    return (m_supports_augmented_libraries_svr4_read == eLazyBoolYes);
+    return m_supports_augmented_libraries_svr4_read == eLazyBoolYes;
 }
 
 bool
@@ -186,7 +197,7 @@ GDBRemoteCommunicationClient::GetQXferLibrariesSVR4ReadSupported ()
     {
         GetRemoteQSupported();
     }
-    return (m_supports_qXfer_libraries_svr4_read == eLazyBoolYes);
+    return m_supports_qXfer_libraries_svr4_read == eLazyBoolYes;
 }
 
 bool
@@ -196,7 +207,7 @@ GDBRemoteCommunicationClient::GetQXferLibrariesReadSupported ()
     {
         GetRemoteQSupported();
     }
-    return (m_supports_qXfer_libraries_read == eLazyBoolYes);
+    return m_supports_qXfer_libraries_read == eLazyBoolYes;
 }
 
 bool
@@ -206,7 +217,7 @@ GDBRemoteCommunicationClient::GetQXferAuxvReadSupported ()
     {
         GetRemoteQSupported();
     }
-    return (m_supports_qXfer_auxv_read == eLazyBoolYes);
+    return m_supports_qXfer_auxv_read == eLazyBoolYes;
 }
 
 bool
@@ -216,7 +227,7 @@ GDBRemoteCommunicationClient::GetQXferFeaturesReadSupported ()
     {
         GetRemoteQSupported();
     }
-    return (m_supports_qXfer_features_read == eLazyBoolYes);
+    return m_supports_qXfer_features_read == eLazyBoolYes;
 }
 
 uint64_t
@@ -412,6 +423,11 @@ GDBRemoteCommunicationClient::GetRemoteQSupported ()
         if (::strstr (response_cstr, "qXfer:features:read+"))
             m_supports_qXfer_features_read = eLazyBoolYes;
 
+        if (::strstr (response_cstr, "qEcho"))
+            m_supports_qEcho = eLazyBoolYes;
+        else
+            m_supports_qEcho = eLazyBoolNo;
+
         const char *packet_size_str = ::strstr (response_cstr, "PacketSize=");
         if (packet_size_str)
         {
@@ -638,7 +654,7 @@ GDBRemoteCommunicationClient::SendPacketAndWaitForResponseNoLock (const char *pa
 {
     PacketResult packet_result = SendPacketNoLock (payload, payload_length);
     if (packet_result == PacketResult::Success)
-        packet_result = WaitForPacketWithTimeoutMicroSecondsNoLock (response, GetPacketTimeoutInMicroSeconds ());
+        packet_result = WaitForPacketWithTimeoutMicroSecondsNoLock (response, GetPacketTimeoutInMicroSeconds (), true);
     return packet_result;
 }
 
@@ -888,7 +904,7 @@ GDBRemoteCommunicationClient::SendContinuePacketAndWaitForResponse
         if (log)
             log->Printf ("GDBRemoteCommunicationClient::%s () WaitForPacket(%s)", __FUNCTION__, continue_packet.c_str());
 
-        if (WaitForPacketWithTimeoutMicroSecondsNoLock(response, UINT32_MAX) == PacketResult::Success)
+        if (WaitForPacketWithTimeoutMicroSecondsNoLock(response, UINT32_MAX, false) == PacketResult::Success)
         {
             if (response.Empty())
                 state = eStateInvalid;
@@ -945,7 +961,7 @@ GDBRemoteCommunicationClient::SendContinuePacketAndWaitForResponse
                                 // packet to make sure it doesn't get in the way
                                 StringExtractorGDBRemote extra_stop_reply_packet;
                                 uint32_t timeout_usec = 1000;
-                                if (WaitForPacketWithTimeoutMicroSecondsNoLock (extra_stop_reply_packet, timeout_usec) == PacketResult::Success)
+                                if (WaitForPacketWithTimeoutMicroSecondsNoLock (extra_stop_reply_packet, timeout_usec, false) == PacketResult::Success)
                                 {
                                     switch (extra_stop_reply_packet.GetChar())
                                     {
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
index 517eb3d0f17..23b5d382a0c 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -430,6 +430,9 @@ public:
     GetRemoteMaxPacketSize();
 
     bool
+    GetEchoSupported ();
+
+    bool
     GetAugmentedLibrariesSVR4ReadSupported ();
 
     bool
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp
index 409efdfa2f7..13ce9b2dc5c 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp
@@ -51,7 +51,7 @@ GDBRemoteCommunicationServer::GetPacketAndSendResponse (uint32_t timeout_usec,
 {
     StringExtractorGDBRemote packet;
 
-    PacketResult packet_result = WaitForPacketWithTimeoutMicroSecondsNoLock (packet, timeout_usec);
+    PacketResult packet_result = WaitForPacketWithTimeoutMicroSecondsNoLock (packet, timeout_usec, false);
     if (packet_result == PacketResult::Success)
     {
         const StringExtractorGDBRemote::ServerPacketType packet_type = packet.GetServerPacketType ();
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 8f2efa2cf73..61cc89dc71b 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1170,6 +1170,7 @@ ProcessGDBRemote::ConnectToDebugserver (const char *connect_url)
     if (GetTarget().GetNonStopModeEnabled())
         m_gdb_comm.SetNonStopMode(true);
 
+    m_gdb_comm.GetEchoSupported ();
     m_gdb_comm.GetThreadSuffixSupported ();
     m_gdb_comm.GetListThreadsInStopReplySupported ();
     m_gdb_comm.GetHostInfo ();