[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 ↗

(On Diff #250580)

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/GDBRemoteCommunicationReplayServer.cpp

 #include "lldb/Utility/StreamString.h"
 #include "lldb/Utility/StringExtractorGDBRemote.h"
 
 using namespace llvm;
 using namespace lldb;
 using namespace lldb_private;
 using namespace lldb_private::process_gdb_remote;
 
+static std::string expandRLE(std::string packet) {
+  std::string decoded;
+  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;
+}
+
 /// Check if the given expected packet matches the actual packet.
 static bool unexpected(llvm::StringRef expected, llvm::StringRef actual) {
   // The 'expected' string contains the raw data, including the leading $ and
   // trailing checksum. The 'actual' string contains only the packet's content.
   if (expected.contains(actual))
     return false;
   // Contains a PID which might be different.
   if (expected.contains("vAttach"))
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 = 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())