Author: labath Date: Wed Jan 10 06:39:08 2018 New Revision: 322190 URL: http://llvm.org/viewvc/llvm-project?rev=322190&view=rev Log: Handle O reply packets during qRcmd
Summary: Gdb servers like openocd may send many $O reply packets for the client to output during a qRcmd command sequence. Currently, lldb interprets the first O packet as an unexpected response. Besides generating no output, this causes lldb to get out of sync with future commands because it continues reading O packets from the first command as response to subsequent commands. This patch handles any O packets during an qRcmd, treating the first non-O packet as the true response. Preliminary discussion at http://lists.llvm.org/pipermail/lldb-dev/2018-January/013078.html Reviewers: clayborg Reviewed By: clayborg Subscribers: labath, lldb-commits Differential Revision: https://reviews.llvm.org/D41745 Patch by Owen Shaw <l...@owenpshaw.net> Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h 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/ProcessGDBRemote.cpp lldb/trunk/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp?rev=322190&r1=322189&r2=322190&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp (original) +++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp Wed Jan 10 06:39:08 2018 @@ -177,6 +177,30 @@ GDBRemoteClientBase::SendPacketAndWaitFo } GDBRemoteCommunication::PacketResult +GDBRemoteClientBase::SendPacketAndReceiveResponseWithOutputSupport( + llvm::StringRef payload, StringExtractorGDBRemote &response, + bool send_async, + llvm::function_ref<void(llvm::StringRef)> output_callback) { + Lock lock(*this, send_async); + if (!lock) { + if (Log *log = + ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS)) + log->Printf("GDBRemoteClientBase::%s failed to get mutex, not sending " + "packet '%.*s' (send_async=%d)", + __FUNCTION__, int(payload.size()), payload.data(), + send_async); + return PacketResult::ErrorSendFailed; + } + + PacketResult packet_result = SendPacketNoLock(payload); + if (packet_result != PacketResult::Success) + return packet_result; + + return ReadPacketWithOutputSupport(response, GetPacketTimeout(), true, + output_callback); +} + +GDBRemoteCommunication::PacketResult GDBRemoteClientBase::SendPacketAndWaitForResponseNoLock( llvm::StringRef payload, StringExtractorGDBRemote &response) { PacketResult packet_result = SendPacketNoLock(payload); Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h?rev=322190&r1=322189&r2=322190&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h (original) +++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h Wed Jan 10 06:39:08 2018 @@ -48,6 +48,11 @@ public: StringExtractorGDBRemote &response, bool send_async); + PacketResult SendPacketAndReceiveResponseWithOutputSupport( + llvm::StringRef payload, StringExtractorGDBRemote &response, + bool send_async, + llvm::function_ref<void(llvm::StringRef)> output_callback); + bool SendvContPacket(llvm::StringRef payload, StringExtractorGDBRemote &response); 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=322190&r1=322189&r2=322190&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp (original) +++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp Wed Jan 10 06:39:08 2018 @@ -275,6 +275,23 @@ GDBRemoteCommunication::PacketResult GDB } GDBRemoteCommunication::PacketResult +GDBRemoteCommunication::ReadPacketWithOutputSupport( + StringExtractorGDBRemote &response, Timeout<std::micro> timeout, + bool sync_on_timeout, + llvm::function_ref<void(llvm::StringRef)> output_callback) { + auto result = ReadPacket(response, timeout, sync_on_timeout); + while (result == PacketResult::Success && response.IsNormalResponse() && + response.PeekChar() == 'O') { + response.GetChar(); + std::string output; + if (response.GetHexByteString(output)) + output_callback(output); + result = ReadPacket(response, timeout, sync_on_timeout); + } + return result; +} + +GDBRemoteCommunication::PacketResult GDBRemoteCommunication::ReadPacket(StringExtractorGDBRemote &response, Timeout<std::micro> timeout, bool sync_on_timeout) { 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=322190&r1=322189&r2=322190&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h (original) +++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h Wed Jan 10 06:39:08 2018 @@ -232,6 +232,11 @@ protected: PacketResult ReadPacket(StringExtractorGDBRemote &response, Timeout<std::micro> timeout, bool sync_on_timeout); + PacketResult ReadPacketWithOutputSupport( + StringExtractorGDBRemote &response, Timeout<std::micro> timeout, + bool sync_on_timeout, + llvm::function_ref<void(llvm::StringRef)> output_callback); + // Pop a packet from the queue in a thread safe manner PacketResult PopPacketFromQueue(StringExtractorGDBRemote &response, Timeout<std::micro> timeout); 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=322190&r1=322189&r2=322190&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp (original) +++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp Wed Jan 10 06:39:08 2018 @@ -5145,10 +5145,11 @@ public: bool send_async = true; StringExtractorGDBRemote response; - process->GetGDBRemote().SendPacketAndWaitForResponse( - packet.GetString(), response, send_async); - result.SetStatus(eReturnStatusSuccessFinishResult); Stream &output_strm = result.GetOutputStream(); + process->GetGDBRemote().SendPacketAndReceiveResponseWithOutputSupport( + packet.GetString(), response, send_async, + [&output_strm](llvm::StringRef output) { output_strm << output; }); + result.SetStatus(eReturnStatusSuccessFinishResult); output_strm.Printf(" packet: %s\n", packet.GetData()); const std::string &response_str = response.GetStringRef(); Modified: lldb/trunk/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp?rev=322190&r1=322189&r2=322190&view=diff ============================================================================== --- lldb/trunk/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp (original) +++ lldb/trunk/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp Wed Jan 10 06:39:08 2018 @@ -339,3 +339,23 @@ TEST_F(GDBRemoteClientBaseTest, Interrup ASSERT_TRUE(async_result.get()); ASSERT_EQ(eStateInvalid, continue_state.get()); } + +TEST_F(GDBRemoteClientBaseTest, SendPacketAndReceiveResponseWithOutputSupport) { + StringExtractorGDBRemote response; + StreamString command_output; + + ASSERT_EQ(PacketResult::Success, server.SendPacket("O")); + ASSERT_EQ(PacketResult::Success, server.SendPacket("O48656c6c6f2c")); + ASSERT_EQ(PacketResult::Success, server.SendPacket("O20")); + ASSERT_EQ(PacketResult::Success, server.SendPacket("O")); + ASSERT_EQ(PacketResult::Success, server.SendPacket("O776f726c64")); + ASSERT_EQ(PacketResult::Success, server.SendPacket("OK")); + + PacketResult result = client.SendPacketAndReceiveResponseWithOutputSupport( + "qRcmd,test", response, true, + [&command_output](llvm::StringRef output) { command_output << output; }); + + ASSERT_EQ(PacketResult::Success, result); + ASSERT_EQ("OK", response.GetStringRef()); + ASSERT_EQ("Hello, world", command_output.GetString().str()); +} _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits