The change previously committed as 220983 broke large binary memory reads. I kept...

The details are: large packets (like large memory reads (m packets) or large binary memory reads (x packet)) can get responses that come in across multiple read() calls. The while loop that was added meant that if only a partial packet came in (like only "$abc" coming for a response) GDBRemoteCommunication::CheckForPacket() was called, it would deadlock in the while loop because no more data is going to come in as this function needs to be called again with more data from another read. So the original fix will need to be corrected and resubmitted.

<rdar://problem/18853744>

llvm-svn: 221181

diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
index 50e1d4c..422096b 100644 (file)
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -411,77 +411,74 @@ GDBRemoteCommunication::CheckForPacket (const uint8_t *src, size_t src_len, Stri
         // it off with an invalid value that is the same as the current
         // index.
         size_t content_start = 0;
-        size_t content_length = std::string::npos;
+        size_t content_length = 0;
         size_t total_length = 0;
         size_t checksum_idx = std::string::npos;
 
-        while (!m_bytes.empty() && content_length == std::string::npos)
+        switch (m_bytes[0])
         {
-            switch (m_bytes[0])
-            {
-                case '+':       // Look for ack
-                case '-':       // Look for cancel
-                case '\x03':    // ^C to halt target
-                    content_length = total_length = 1;  // The command is one byte long...
-                    break;
-
-                case '$':
-                    // Look for a standard gdb packet?
+            case '+':       // Look for ack
+            case '-':       // Look for cancel
+            case '\x03':    // ^C to halt target
+                content_length = total_length = 1;  // The command is one byte long...
+                break;
+
+            case '$':
+                // Look for a standard gdb packet?
+                {
+                    size_t hash_pos = m_bytes.find('#');
+                    if (hash_pos != std::string::npos)
                     {
-                        size_t hash_pos = m_bytes.find('#');
-                        if (hash_pos != std::string::npos)
+                        if (hash_pos + 2 < m_bytes.size())
+                        {
+                            checksum_idx = hash_pos + 1;
+                            // Skip the dollar sign
+                            content_start = 1; 
+                            // Don't include the # in the content or the $ in the content length
+                            content_length = hash_pos - 1;  
+                            
+                            total_length = hash_pos + 3; // Skip the # and the two hex checksum bytes
+                        }
+                        else
                         {
-                            if (hash_pos + 2 < m_bytes.size())
-                            {
-                                checksum_idx = hash_pos + 1;
-                                // Skip the dollar sign
-                                content_start = 1;
-                                // Don't include the # in the content or the $ in the content length
-                                content_length = hash_pos - 1;
-
-                                total_length = hash_pos + 3; // Skip the # and the two hex checksum bytes
-                            }
-                            else
-                            {
-                                // Checksum bytes aren't all here yet
-                                content_length = std::string::npos;
-                            }
+                            // Checksum bytes aren't all here yet
+                            content_length = std::string::npos;
                         }
                     }
-                    break;
+                }
+                break;
 
-                default:
+            default:
+                {
+                    // We have an unexpected byte and we need to flush all bad 
+                    // data that is in m_bytes, so we need to find the first
+                    // byte that is a '+' (ACK), '-' (NACK), \x03 (CTRL+C interrupt),
+                    // or '$' character (start of packet header) or of course,
+                    // the end of the data in m_bytes...
+                    const size_t bytes_len = m_bytes.size();
+                    bool done = false;
+                    uint32_t idx;
+                    for (idx = 1; !done && idx < bytes_len; ++idx)
                     {
-                        // We have an unexpected byte and we need to flush all bad
-                        // data that is in m_bytes, so we need to find the first
-                        // byte that is a '+' (ACK), '-' (NACK), \x03 (CTRL+C interrupt),
-                        // or '$' character (start of packet header) or of course,
-                        // the end of the data in m_bytes...
-                        const size_t bytes_len = m_bytes.size();
-                        bool done = false;
-                        uint32_t idx;
-                        for (idx = 1; !done && idx < bytes_len; ++idx)
+                        switch (m_bytes[idx])
                         {
-                            switch (m_bytes[idx])
-                            {
-                            case '+':
-                            case '-':
-                            case '\x03':
-                            case '$':
-                                done = true;
-                                break;
-
-                            default:
-                                break;
-                            }
+                        case '+':
+                        case '-':
+                        case '\x03':
+                        case '$':
+                            done = true;
+                            break;
+                                
+                        default:
+                            break;
                         }
-                        if (log)
-                            log->Printf ("GDBRemoteCommunication::%s tossing %u junk bytes: '%.*s'",
-                                         __FUNCTION__, idx - 1, idx - 1, m_bytes.c_str());
-                        m_bytes.erase(0, idx - 1);
                     }
-                    break;
-            }
+                    if (log)
+                        log->Printf ("GDBRemoteCommunication::%s tossing %u junk bytes: '%.*s'",
+                                     __FUNCTION__, idx - 1, idx - 1, m_bytes.c_str());
+                    m_bytes.erase(0, idx - 1);
+                }
+                break;
         }
 
         if (content_length == std::string::npos)