[lldb/Reproducers] Decode run-length encoding in GDB replay server.

[JDevlieghere]

Summary

The GDB replay server sanity-checks that every packet it receives matches what it expects from the serialized packet log. This mechanism tripped for TestReproducerAttach.py on Linux, because one of the packets (jModulesInfo) uses run-length encoding. The replay server was comparing the expanded incoming packet with the unexpanded packet in the log. As a result, it claimed to have received an unexpected packet, which caused the test to fail.

This patch addresses that issue by expanding the run-length encoding before comparing the packets.

[JDevlieghere]

Share RLE-decoding logic.

[jasonmolenda]

lgtm.

[shafik]

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp

1374

This line feels cryptic. Why 3 and ' '. Maybe two lines would be better, still have to think carefully about order of operations too.

The code is using three representations for characters: char literals, raw integers and hex. Maybe use hex everywhere would be more consistent.

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h

public:
 
   void DumpHistory(Stream &strm);
 
   void SetPacketRecorder(repro::PacketRecorder *recorder);
 
   static llvm::Error ConnectLocally(GDBRemoteCommunication &client,
                                     GDBRemoteCommunication &server);
 
+  /// Expand GDB run-length encoding.
+  static std::string ExpandRLE(std::string);
+
 protected:
   std::chrono::seconds m_packet_timeout;
   uint32_t m_echo_number;
   LazyBool m_supports_qEcho;
   GDBRemoteCommunicationHistory m_history;
   bool m_send_acks;
   bool m_is_platform; // Set to true if this class represents a platform,
                       // false if this class represents a debug session for

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp

if (content_length == std::string::npos) {
                       m_bytes.c_str());
         }
       }
 
       m_history.AddPacket(m_bytes, total_length,
                           GDBRemotePacket::ePacketTypeRecv, total_length);
 
       // Copy the packet from m_bytes to packet_str expanding the run-length
-      // encoding in the process. Reserve enough byte for the most common case
-      // (no RLE used)
-      std ::string packet_str;
-      packet_str.reserve(m_bytes.length());
-      for (std::string::const_iterator c = m_bytes.begin() + content_start;
-           c != m_bytes.begin() + content_end; ++c) {
-        if (*c == '*') {
-          // '*' indicates RLE. Next character will give us the repeat count
-          // and previous character is what is to be repeated.
-          char char_to_repeat = packet_str.back();
-          // Number of time the previous character is repeated
-          int repeat_count = *++c + 3 - ' ';
-          // We have the char_to_repeat and repeat_count. Now push it in the
-          // packet.
-          for (int i = 0; i < repeat_count; ++i)
-            packet_str.push_back(char_to_repeat);
-        } else if (*c == 0x7d) {
-          // 0x7d is the escape character.  The next character is to be XOR'd
-          // with 0x20.
-          char escapee = *++c ^ 0x20;
-          packet_str.push_back(escapee);
-        } else {
-          packet_str.push_back(*c);
-        }
-      }
+      // encoding in the process.
+      std ::string packet_str =
+          ExpandRLE(m_bytes.substr(content_start, content_end - content_start));
       packet = StringExtractorGDBRemote(packet_str);
 
       if (m_bytes[0] == '$' || m_bytes[0] == '%') {
         assert(checksum_idx < m_bytes.size());
         if (::isxdigit(m_bytes[checksum_idx + 0]) ||
             ::isxdigit(m_bytes[checksum_idx + 1])) {
           if (GetSendAcks()) {
             const char *packet_checksum_cstr = &m_bytes[checksum_idx];
void llvm::format_provider<GDBRemoteCommunication::PacketResult>::format(
   case PacketResult::ErrorDisconnected:
     Stream << "ErrorDisconnected";
     break;
   case PacketResult::ErrorNoSequenceLock:
     Stream << "ErrorNoSequenceLock";
     break;
   }
 }
+
+std::string GDBRemoteCommunication::ExpandRLE(std::string packet) {
+  // Reserve enough byte for the most common case (no RLE used).
+  std::string decoded;
+  decoded.reserve(packet.size());
+  for (std::string::const_iterator c = packet.begin(); c != packet.end(); ++c) {
+    if (*c == '*') {
+      // '*' indicates RLE. Next character will give us the repeat count and
+      // previous character is what is to be repeated.
+      char char_to_repeat = decoded.back();
+      // Number of time the previous character is repeated.
+      int repeat_count = *++c + 3 - ' ';
+      // We have the char_to_repeat and repeat_count. Now push it in the
+      // packet.
+      for (int i = 0; i < repeat_count; ++i)
+        decoded.push_back(char_to_repeat);
+    } else if (*c == 0x7d) {
+      // 0x7d is the escape character.  The next character is to be XOR'd with
+      // 0x20.
+      char escapee = *++c ^ 0x20;
+      decoded.push_back(escapee);
+    } else {
+      decoded.push_back(*c);
+    }
+  }
+  return decoded;
+}

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp

if (packet.GetStringRef().find("QEnvironment") == 0)
     return SendRawPacketNoLock("$OK#9a");
 
   Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS));
   while (!m_packet_history.empty()) {
     // Pop last packet from the history.
     GDBRemotePacket entry = m_packet_history.back();
     m_packet_history.pop_back();
 
+    // Decode run-length encoding.
+    const std::string expanded_data =
+        GDBRemoteCommunication::ExpandRLE(entry.packet.data);
+
     // We've handled the handshake implicitly before. Skip the packet and move
     // on.
     if (entry.packet.data == "+")
       continue;
 
     if (entry.type == GDBRemotePacket::ePacketTypeSend) {
-      if (unexpected(entry.packet.data, packet.GetStringRef())) {
+      if (unexpected(expanded_data, packet.GetStringRef())) {
         LLDB_LOG(log,
                  "GDBRemoteCommunicationReplayServer expected packet: '{0}'",
-                 entry.packet.data);
+                 expanded_data);
         LLDB_LOG(log, "GDBRemoteCommunicationReplayServer actual packet: '{0}'",
                  packet.GetStringRef());
 #ifndef NDEBUG
         // This behaves like a regular assert, but prints the expected and
         // received packet before aborting.
-        printf("Reproducer expected packet: '%s'\n", entry.packet.data.c_str());
+        printf("Reproducer expected packet: '%s'\n", expanded_data.c_str());
         printf("Reproducer received packet: '%s'\n",
                packet.GetStringRef().data());
         llvm::report_fatal_error("Encountered unexpected packet during replay");
 #endif
         return PacketResult::ErrorSendFailed;
       }
 
       // Ignore QEnvironment packets as they're handled earlier.
-      if (entry.packet.data.find("QEnvironment") == 1) {
+      if (expanded_data.find("QEnvironment") == 1) {
         assert(m_packet_history.back().type ==
                GDBRemotePacket::ePacketTypeRecv);
         m_packet_history.pop_back();
       }
 
       continue;
     }
 

lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py

 from lldbsuite.test import lldbutil
 
 
 class ReproducerAttachTestCase(TestBase):
 
     mydir = TestBase.compute_mydir(__file__)
     NO_DEBUG_INFO_TESTCASE = True
 
-    @skipIfLinux # Reproducer received unexpected packet.
     @skipIfFreeBSD
     @skipIfNetBSD
     @skipIfWindows
     @skipIfRemote
     @skipIfiOSSimulator
     def test_reproducer_attach(self):
         """Test thread creation after process attach."""
         exe = '%s_%d' % (self.testMethodName, os.getpid())