JDevlieghere created this revision.
JDevlieghere added reviewers: labath, jasonmolenda.
Herald added a subscriber: abidh.
JDevlieghere updated this revision to Diff 250305.
JDevlieghere added a comment.

Share RLE-decoding logic.


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.


https://reviews.llvm.org/D76163

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp
  lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py

Index: lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py
===================================================================
--- lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py
+++ lldb/test/API/functionalities/reproducers/attach/TestReproducerAttach.py
@@ -15,7 +15,6 @@
     mydir = TestBase.compute_mydir(__file__)
     NO_DEBUG_INFO_TESTCASE = True
 
-    @skipIfLinux # Reproducer received unexpected packet.
     @skipIfFreeBSD
     @skipIfNetBSD
     @skipIfWindows
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationReplayServer.cpp
@@ -131,22 +131,26 @@
     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");
@@ -155,7 +159,7 @@
       }
 
       // 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();
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
@@ -142,6 +142,9 @@
   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;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -810,31 +810,9 @@
                           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] == '%') {
@@ -1382,3 +1360,30 @@
     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-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to