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@owenpshaw.net>

llvm-svn: 322190

5 files changed, 55 insertions, 3 deletions

diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
index 4d4a4f8c5c7..62b88456b9d 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
@@ -177,6 +177,30 @@ GDBRemoteClientBase::SendPacketAndWaitForResponse(
 }
 
 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);
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
index 2646405c9b9..3d84ce0ebe1 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
@@ -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);
 
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
index 18d9631f6c3..dd57dd13833 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -275,6 +275,23 @@ GDBRemoteCommunication::PacketResult GDBRemoteCommunication::GetAck() {
 }
 
 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) {
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
index ecc9386e49c..ddd22127d26 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
@@ -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);
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 44efcbe20ae..78399b733a7 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -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();