Author: gclayton
Date: Mon Nov 3 15:02:54 2014
New Revision: 221181
URL: http://llvm.org/viewvc/llvm-project?rev=221181&view=rev
Log:
The change previously committed as 220983 broke large binary memory reads. I
kept the "idx - 1" fix from 220983, but reverted the while loop that was
incorrectly added.
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>
Modified:
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
Modified:
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp?rev=221181&r1=221180&r2=221181&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
(original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp Mon
Nov 3 15:02:54 2014
@@ -411,77 +411,74 @@ GDBRemoteCommunication::CheckForPacket (
// 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())
{
- 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_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;
}
}
- 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)
_______________________________________________
lldb-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
