[Lldb-commits] [lldb] r238530 - Add a new "qEcho" packet with the following format:
Greg Clayton
gclayton at apple.com
Thu May 28 17:01:56 PDT 2015
Author: gclayton
Date: Thu May 28 19:01:55 2015
New Revision: 238530
URL: http://llvm.org/viewvc/llvm-project?rev=238530&view=rev
Log:
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>
Modified:
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp
lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/trunk/test/tools/lldb-server/gdbremote_testcase.py
lldb/trunk/tools/debugserver/source/RNBRemote.cpp
lldb/trunk/tools/debugserver/source/RNBRemote.h
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=238530&r1=238529&r2=238530&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp Thu May 28 19:01:55 2015
@@ -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::GDBRemoteCommuni
#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::WaitForNotRunnin
}
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::WaitForPacketWit
{
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:
Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h?rev=238530&r1=238529&r2=238530&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h Thu May 28 19:01:55 2015
@@ -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
Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp?rev=238530&r1=238529&r2=238530&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp Thu May 28 19:01:55 2015
@@ -145,7 +145,7 @@ GDBRemoteCommunicationClient::HandshakeW
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::HandshakeW
}
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::GetQXferLi
{
GetRemoteQSupported();
}
- return (m_supports_qXfer_libraries_svr4_read == eLazyBoolYes);
+ return m_supports_qXfer_libraries_svr4_read == eLazyBoolYes;
}
bool
@@ -196,7 +207,7 @@ GDBRemoteCommunicationClient::GetQXferLi
{
GetRemoteQSupported();
}
- return (m_supports_qXfer_libraries_read == eLazyBoolYes);
+ return m_supports_qXfer_libraries_read == eLazyBoolYes;
}
bool
@@ -206,7 +217,7 @@ GDBRemoteCommunicationClient::GetQXferAu
{
GetRemoteQSupported();
}
- return (m_supports_qXfer_auxv_read == eLazyBoolYes);
+ return m_supports_qXfer_auxv_read == eLazyBoolYes;
}
bool
@@ -216,7 +227,7 @@ GDBRemoteCommunicationClient::GetQXferFe
{
GetRemoteQSupported();
}
- return (m_supports_qXfer_features_read == eLazyBoolYes);
+ return m_supports_qXfer_features_read == eLazyBoolYes;
}
uint64_t
@@ -412,6 +423,11 @@ GDBRemoteCommunicationClient::GetRemoteQ
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::SendPacket
{
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::SendContin
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::SendContin
// 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())
{
Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h?rev=238530&r1=238529&r2=238530&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h Thu May 28 19:01:55 2015
@@ -430,6 +430,9 @@ public:
GetRemoteMaxPacketSize();
bool
+ GetEchoSupported ();
+
+ bool
GetAugmentedLibrariesSVR4ReadSupported ();
bool
Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp?rev=238530&r1=238529&r2=238530&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp Thu May 28 19:01:55 2015
@@ -51,7 +51,7 @@ GDBRemoteCommunicationServer::GetPacketA
{
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 ();
Modified: lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp?rev=238530&r1=238529&r2=238530&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp Thu May 28 19:01:55 2015
@@ -1170,6 +1170,7 @@ ProcessGDBRemote::ConnectToDebugserver (
if (GetTarget().GetNonStopModeEnabled())
m_gdb_comm.SetNonStopMode(true);
+ m_gdb_comm.GetEchoSupported ();
m_gdb_comm.GetThreadSuffixSupported ();
m_gdb_comm.GetListThreadsInStopReplySupported ();
m_gdb_comm.GetHostInfo ();
Modified: lldb/trunk/test/tools/lldb-server/gdbremote_testcase.py
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/test/tools/lldb-server/gdbremote_testcase.py?rev=238530&r1=238529&r2=238530&view=diff
==============================================================================
--- lldb/trunk/test/tools/lldb-server/gdbremote_testcase.py (original)
+++ lldb/trunk/test/tools/lldb-server/gdbremote_testcase.py Thu May 28 19:01:55 2015
@@ -717,6 +717,7 @@ class GdbRemoteTestCaseBase(TestBase):
"qXfer:libraries:read",
"qXfer:libraries-svr4:read",
"qXfer:features:read",
+ "qEcho"
]
def parse_qSupported_response(self, context):
Modified: lldb/trunk/tools/debugserver/source/RNBRemote.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/RNBRemote.cpp?rev=238530&r1=238529&r2=238530&view=diff
==============================================================================
--- lldb/trunk/tools/debugserver/source/RNBRemote.cpp (original)
+++ lldb/trunk/tools/debugserver/source/RNBRemote.cpp Thu May 28 19:01:55 2015
@@ -163,6 +163,7 @@ RNBRemote::CreatePacketTable ()
t.push_back (Packet (remove_access_watch_bp, &RNBRemote::HandlePacket_z, NULL, "z4", "Remove access watchpoint"));
t.push_back (Packet (query_monitor, &RNBRemote::HandlePacket_qRcmd, NULL, "qRcmd", "Monitor command"));
t.push_back (Packet (query_current_thread_id, &RNBRemote::HandlePacket_qC, NULL, "qC", "Query current thread ID"));
+ t.push_back (Packet (query_echo, &RNBRemote::HandlePacket_qEcho, NULL, "qEcho:", "Echo the packet back to allow the debugger to sync up with this server"));
t.push_back (Packet (query_get_pid, &RNBRemote::HandlePacket_qGetPid, NULL, "qGetPid", "Query process id"));
t.push_back (Packet (query_thread_ids_first, &RNBRemote::HandlePacket_qThreadInfo, NULL, "qfThreadInfo", "Get list of active threads (first req)"));
t.push_back (Packet (query_thread_ids_subsequent, &RNBRemote::HandlePacket_qThreadInfo, NULL, "qsThreadInfo", "Get list of active threads (subsequent req)"));
@@ -1499,6 +1500,16 @@ RNBRemote::HandlePacket_qC (const char *
}
rnb_err_t
+RNBRemote::HandlePacket_qEcho (const char *p)
+{
+ // Just send the exact same packet back that we received to
+ // synchronize the response packets after a previous packet
+ // timed out. This allows the debugger to get back on track
+ // with responses after a packet timeout.
+ return SendPacket (p);
+}
+
+rnb_err_t
RNBRemote::HandlePacket_qGetPid (const char *p)
{
nub_process_t pid;
@@ -3086,7 +3097,7 @@ RNBRemote::HandlePacket_qSupported (cons
{
uint32_t max_packet_size = 128 * 1024; // 128KBytes is a reasonable max packet size--debugger can always use less
char buf[64];
- snprintf (buf, sizeof(buf), "qXfer:features:read+;PacketSize=%x", max_packet_size);
+ snprintf (buf, sizeof(buf), "qXfer:features:read+;PacketSize=%x;qEcho", max_packet_size);
return SendPacket (buf);
}
Modified: lldb/trunk/tools/debugserver/source/RNBRemote.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/RNBRemote.h?rev=238530&r1=238529&r2=238530&view=diff
==============================================================================
--- lldb/trunk/tools/debugserver/source/RNBRemote.h (original)
+++ lldb/trunk/tools/debugserver/source/RNBRemote.h Thu May 28 19:01:55 2015
@@ -83,6 +83,7 @@ public:
query_monitor, // 'qRcmd'
query_current_thread_id, // 'qC'
query_get_pid, // 'qGetPid'
+ query_echo, // 'qEcho'
query_thread_ids_first, // 'qfThreadInfo'
query_thread_ids_subsequent, // 'qsThreadInfo'
query_thread_extra_info, // 'qThreadExtraInfo'
@@ -177,6 +178,7 @@ public:
rnb_err_t HandlePacket_qC (const char *p);
rnb_err_t HandlePacket_qRcmd (const char *p);
rnb_err_t HandlePacket_qGetPid (const char *p);
+ rnb_err_t HandlePacket_qEcho (const char *p);
rnb_err_t HandlePacket_qLaunchSuccess (const char *p);
rnb_err_t HandlePacket_qRegisterInfo (const char *p);
rnb_err_t HandlePacket_qShlibInfoAddr (const char *p);
More information about the lldb-commits
mailing list