[Lldb-commits] [lldb] r322190 - Handle O reply packets during qRcmd

Pavel Labath via lldb-commits lldb-commits at lists.llvm.org
Wed Jan 10 06:39:08 PST 2018


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 <llvm at 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());
+}




More information about the lldb-commits mailing list