diff options
6 files changed, 201 insertions, 9 deletions
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp index e61999dbf2a..472c6aae915 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp @@ -623,6 +623,7 @@ GDBRemoteCommunicationClient::GetThreadsInfo() if (m_supports_jThreadsInfo) { StringExtractorGDBRemote response; + response.SetResponseValidatorToJSON(); if (SendPacketAndWaitForResponse("jThreadsInfo", response, false) == PacketResult::Success) { if (response.IsUnsupportedResponse()) @@ -765,9 +766,29 @@ GDBRemoteCommunicationClient::SendPacketAndWaitForResponseNoLock (const char *pa size_t payload_length, StringExtractorGDBRemote &response) { - PacketResult packet_result = SendPacketNoLock (payload, payload_length); + PacketResult packet_result = SendPacketNoLock(payload, payload_length); if (packet_result == PacketResult::Success) - packet_result = ReadPacket (response, GetPacketTimeoutInMicroSeconds (), true); + { + const size_t max_response_retries = 3; + for (size_t i=0; i<max_response_retries; ++i) + { + packet_result = ReadPacket(response, GetPacketTimeoutInMicroSeconds (), 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); + if (log) + log->Printf("error: packet with payload \"%*s\" got invalid response \"%s\": %s", + (int)payload_length, + payload, + response.GetStringRef().c_str(), + (i == (max_response_retries - 1)) ? "using invalid response and giving up" : "ignoring response and waiting for another"); + } + } return packet_result; } @@ -801,6 +822,7 @@ GDBRemoteCommunicationClient::SendPacketAndWaitForResponse { Mutex::Locker async_locker (m_async_mutex); m_async_packet.assign(payload, payload_length); + m_async_response.CopyResponseValidator(response); m_async_packet_predicate.SetValue (true, eBroadcastNever); if (log) @@ -867,6 +889,9 @@ GDBRemoteCommunicationClient::SendPacketAndWaitForResponse if (log) log->Printf ("async: failed to interrupt"); } + + m_async_response.SetResponseValidator(nullptr, nullptr); + } else { @@ -1136,8 +1161,11 @@ GDBRemoteCommunicationClient::SendContinuePacketAndWaitForResponse // which will just re-send a copy of the last stop reply // packet. If we don't do this, then the reply for our // async packet will be the repeat stop reply packet and cause - // a lot of trouble for us! - if (signo != sigint_signo && signo != sigstop_signo) + // a lot of trouble for us! We also have some debugserver + // binaries that would send two stop replies anytime the process + // was interrupted, so we need to also check for an extra + // stop reply packet if we interrupted the process + if (m_interrupt_sent || (signo != sigint_signo && signo != sigstop_signo)) { continue_after_async = false; @@ -3572,6 +3600,8 @@ GDBRemoteCommunicationClient::SendGDBStoppointTypePacket (GDBStoppointType type, // Check we haven't overwritten the end of the packet buffer assert (packet_len + 1 < (int)sizeof(packet)); StringExtractorGDBRemote response; + // Make sure the response is either "OK", "EXX" where XX are two hex digits, or "" (unsupported) + response.SetResponseValidatorToOKErrorNotSupported(); // Try to send the breakpoint packet, and check that it was correctly sent if (SendPacketAndWaitForResponse(packet, packet_len, response, true) == PacketResult::Success) { diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index e666b852ba3..c0db8ebc9de 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -4170,6 +4170,7 @@ ProcessGDBRemote::GetExtendedInfoForThread (lldb::tid_t tid) packet << (char) (0x7d ^ 0x20); StringExtractorGDBRemote response; + response.SetResponseValidatorToJSON(); if (m_gdb_comm.SendPacketAndWaitForResponse(packet.GetData(), packet.GetSize(), response, false) == GDBRemoteCommunication::PacketResult::Success) { StringExtractorGDBRemote::ResponseType response_type = response.GetResponseType(); @@ -4211,6 +4212,7 @@ ProcessGDBRemote::GetLoadedDynamicLibrariesInfos (lldb::addr_t image_list_addres packet << (char) (0x7d ^ 0x20); StringExtractorGDBRemote response; + response.SetResponseValidatorToJSON(); if (m_gdb_comm.SendPacketAndWaitForResponse(packet.GetData(), packet.GetSize(), response, false) == GDBRemoteCommunication::PacketResult::Success) { StringExtractorGDBRemote::ResponseType response_type = response.GetResponseType(); diff --git a/lldb/source/Utility/StringExtractorGDBRemote.cpp b/lldb/source/Utility/StringExtractorGDBRemote.cpp index 44bb1179363..311ce57b83b 100644 --- a/lldb/source/Utility/StringExtractorGDBRemote.cpp +++ b/lldb/source/Utility/StringExtractorGDBRemote.cpp @@ -14,8 +14,7 @@ // Other libraries and framework includes // Project includes #include "Utility/StringExtractorGDBRemote.h" - - +#include "lldb/Utility/LLDBAssert.h" StringExtractorGDBRemote::ResponseType StringExtractorGDBRemote::GetResponseType () const @@ -391,3 +390,136 @@ StringExtractorGDBRemote::GetEscapedBinaryData (std::string &str) return str.size(); } +static bool +OKErrorNotSupportedResponseValidator(void *, const StringExtractorGDBRemote &response) +{ + switch (response.GetResponseType()) + { + case StringExtractorGDBRemote::eOK: + case StringExtractorGDBRemote::eError: + case StringExtractorGDBRemote::eUnsupported: + return true; + + case StringExtractorGDBRemote::eAck: + case StringExtractorGDBRemote::eNack: + case StringExtractorGDBRemote::eResponse: + break; + } + lldbassert(!"Packet validatation failed, check why this is happening"); + return false; +} + +static bool +JSONResponseValidator(void *, const StringExtractorGDBRemote &response) +{ + switch (response.GetResponseType()) + { + case StringExtractorGDBRemote::eUnsupported: + case StringExtractorGDBRemote::eError: + return true; // Accept unsupported or EXX as valid responses + + case StringExtractorGDBRemote::eOK: + case StringExtractorGDBRemote::eAck: + case StringExtractorGDBRemote::eNack: + break; + + case StringExtractorGDBRemote::eResponse: + // JSON that is returned in from JSON query packets is currently always + // either a dictionary which starts with a '{', or an array which + // starts with a '['. This is a quick validator to just make sure the + // response could be valid JSON without having to validate all of the + // JSON content. + switch (response.GetStringRef()[0]) + { + case '{': return true; + case '[': return true; + default: + break; + } + break; + } + lldbassert(!"Packet validatation failed, check why this is happening"); + return false; +} + +static bool +ASCIIHexBytesResponseValidator(void *, const StringExtractorGDBRemote &response) +{ + switch (response.GetResponseType()) + { + case StringExtractorGDBRemote::eUnsupported: + case StringExtractorGDBRemote::eError: + return true; // Accept unsupported or EXX as valid responses + + case StringExtractorGDBRemote::eOK: + case StringExtractorGDBRemote::eAck: + case StringExtractorGDBRemote::eNack: + break; + + case StringExtractorGDBRemote::eResponse: + { + uint32_t valid_count = 0; + for (const char ch : response.GetStringRef()) + { + if (!isxdigit(ch)) + { + lldbassert(!"Packet validatation failed, check why this is happening"); + return false; + } + if (++valid_count >= 16) + break; // Don't validate all the characters in case the packet is very large + } + return true; + } + break; + } + lldbassert(!"Packet validatation failed, check why this is happening"); + return false; +} + +void +StringExtractorGDBRemote::CopyResponseValidator(const StringExtractorGDBRemote& rhs) +{ + m_validator = rhs.m_validator; + m_validator_baton = rhs.m_validator_baton; +} + +void +StringExtractorGDBRemote::SetResponseValidator(ResponseValidatorCallback callback, void *baton) +{ + m_validator = callback; + m_validator_baton = baton; +} + +void +StringExtractorGDBRemote::SetResponseValidatorToOKErrorNotSupported() +{ + m_validator = OKErrorNotSupportedResponseValidator; + m_validator_baton = nullptr; +} + +void +StringExtractorGDBRemote::SetResponseValidatorToASCIIHexBytes() +{ + m_validator = ASCIIHexBytesResponseValidator; + m_validator_baton = nullptr; +} + +void +StringExtractorGDBRemote::SetResponseValidatorToJSON() +{ + m_validator = JSONResponseValidator; + m_validator_baton = nullptr; +} + +bool +StringExtractorGDBRemote::ValidateResponse() const +{ + // If we have a validator callback, try to validate the callback + if (m_validator) + return m_validator(m_validator_baton, *this); + else + return true; // No validator, so response is valid +} + + diff --git a/lldb/source/Utility/StringExtractorGDBRemote.h b/lldb/source/Utility/StringExtractorGDBRemote.h index 7e2f1e7c696..ade0edbbb7a 100644 --- a/lldb/source/Utility/StringExtractorGDBRemote.h +++ b/lldb/source/Utility/StringExtractorGDBRemote.h @@ -20,18 +20,23 @@ class StringExtractorGDBRemote : public StringExtractor { public: + typedef bool (*ResponseValidatorCallback)(void * baton, const StringExtractorGDBRemote &response); StringExtractorGDBRemote() : - StringExtractor () + StringExtractor(), + m_validator(nullptr) { } StringExtractorGDBRemote(const char *cstr) : - StringExtractor (cstr) + StringExtractor(cstr), + m_validator(nullptr) { } + StringExtractorGDBRemote(const StringExtractorGDBRemote& rhs) : - StringExtractor (rhs) + StringExtractor(rhs), + m_validator(rhs.m_validator) { } @@ -39,6 +44,24 @@ public: { } + bool + ValidateResponse() const; + + void + CopyResponseValidator(const StringExtractorGDBRemote& rhs); + + void + SetResponseValidator(ResponseValidatorCallback callback, void *baton); + + void + SetResponseValidatorToOKErrorNotSupported(); + + void + SetResponseValidatorToASCIIHexBytes(); + + void + SetResponseValidatorToJSON(); + enum ServerPacketType { eServerPacketType_nack = 0, @@ -192,6 +215,9 @@ public: size_t GetEscapedBinaryData (std::string &str); +protected: + ResponseValidatorCallback m_validator; + void *m_validator_baton; }; #endif // utility_StringExtractorGDBRemote_h_ diff --git a/lldb/tools/debugserver/source/MacOSX/MachProcess.mm b/lldb/tools/debugserver/source/MacOSX/MachProcess.mm index b9e06307a4a..7caf79c56ce 100644 --- a/lldb/tools/debugserver/source/MacOSX/MachProcess.mm +++ b/lldb/tools/debugserver/source/MacOSX/MachProcess.mm @@ -1048,6 +1048,7 @@ MachProcess::Interrupt() if (Signal (m_sent_interrupt_signo)) { DNBLogThreadedIf(LOG_PROCESS, "MachProcess::Interrupt() - sent %i signal to interrupt process", m_sent_interrupt_signo); + return true; } else { diff --git a/lldb/tools/debugserver/source/RNBRemote.cpp b/lldb/tools/debugserver/source/RNBRemote.cpp index 72c2008c21d..b3256dc6a9d 100644 --- a/lldb/tools/debugserver/source/RNBRemote.cpp +++ b/lldb/tools/debugserver/source/RNBRemote.cpp @@ -4433,6 +4433,7 @@ RNBRemote::HandlePacket_stop_process (const char *p) { // If we failed to interrupt the process, then send a stop // reply packet as the process was probably already stopped + DNBLogThreaded ("RNBRemote::HandlePacket_stop_process() sending extra stop reply because DNBProcessInterrupt returned false"); HandlePacket_last_signal (NULL); } return rnb_success; |
