[Lldb-commits] [PATCH] Fix for PR25300

[Tim Northover via lldb-commits]

Hi,

I decided to take a quick look at PR25300 (roughly, an '*' at the end
of an environment variable crashes lldb-server). The issue is that
during packet decompression (apparently even for uncompressed packets)
there is basic RLE, triggered by '*', so it needs to be quoted in some
way before being sent.

I came up with patches doing it in two different places (both attached here):

1. Switch to QEnvironmentHexEncoded if the string has these chars.
2. Safely escape these chars in SendPackedNoLock.

The second seems more robust and semantically correct (viewing only
'$' and '#' as characters that are actually special in a non
transport-dependent way). But it does have a performance overhead for
each packet sent (a likely not very optimised extra strcpy).

Which would be preferred, and does anyone have any hints on writing a
test for it? I can't see any existing QEnvironment tests to base mine
on.

Cheers.

Tim.
-------------- next part --------------
diff --git a/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp b/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
index ea95298..9392af7 100644
--- a/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ b/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -238,12 +238,29 @@ GDBRemoteCommunication::SendPacketNoLock (const char *payload, size_t payload_le
 {
     if (IsConnected())
     {
-        StreamString packet(0, 4, eByteOrderBig);
+        std::string safe_payload;
+        safe_payload.reserve(payload_length);
+        for (const char *c = payload; c != payload + payload_length; ++c)
+        {
+            if (*c == '*' || *c == '}')
+            {
+                // These have special meaning even for nominally uncompressed
+                // strings, so we must escape them. Fortunately, the special
+                // meaning of '}' is precisely an escape of the next char (^
+                // 0x20).
+                safe_payload.push_back('}');
+                safe_payload.push_back(*c ^ 0x20);
+            }
+            else
+                safe_payload.push_back(*c);
+        }
+        payload_length = safe_payload.size();
 
+        StreamString packet(0, 4, eByteOrderBig);
         packet.PutChar('$');
-        packet.Write (payload, payload_length);
+        packet.Write (safe_payload.data(), payload_length);
         packet.PutChar('#');
-        packet.PutHex8(CalculcateChecksum (payload, payload_length));
+        packet.PutHex8(CalculcateChecksum (safe_payload.data(), payload_length));
 
         Log *log (ProcessGDBRemoteLog::GetLogIfAllCategoriesSet (GDBR_LOG_PACKETS));
         ConnectionStatus status = eConnectionStatusSuccess;
-------------- next part --------------
diff --git a/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index a6fcd14..5e0964b 100644
--- a/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -1577,6 +1577,8 @@ GDBRemoteCommunicationClient::SendEnvironmentPacket (char const *name_equal_valu
                 {
                     case '$':
                     case '#':
+                    case '*':
+                    case '}':
                         send_hex_encoding = true;
                         break;
                     default: