[lldb] [gdb-remote client] Refactor handling qSupported
authorMichał Górny <mgorny@moritz.systems>
Thu, 8 Apr 2021 22:18:09 +0000 (00:18 +0200)
committerMichał Górny <mgorny@moritz.systems>
Mon, 12 Apr 2021 22:23:07 +0000 (00:23 +0200)
Refactor the qSupported handler to split the reply into an array,
and identify features within the array rather than searching the string
for partial matches.  While at it, use StringRef.split() to process
the compression list instead of reinventing the wheel.

Switch the arguments to MaybeEnableCompression() to use an ArrayRef
of StringRefs to simplify parameter passing from GetRemoteQSupported().

Differential Revision: https://reviews.llvm.org/D100146

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h

index c032fc2..2db985c 100644 (file)
@@ -345,6 +345,9 @@ void GDBRemoteCommunicationClient::GetRemoteQSupported() {
   m_supports_qXfer_features_read = eLazyBoolNo;
   m_supports_qXfer_memory_map_read = eLazyBoolNo;
   m_supports_multiprocess = eLazyBoolNo;
+  m_supports_qEcho = eLazyBoolNo;
+  m_supports_QPassSignals = eLazyBoolNo;
+
   m_max_packet_size = UINT64_MAX; // It's supposed to always be there, but if
                                   // not, we assume no limit
 
@@ -362,97 +365,51 @@ void GDBRemoteCommunicationClient::GetRemoteQSupported() {
   if (SendPacketAndWaitForResponse(packet.GetString(), response,
                                    /*send_async=*/false) ==
       PacketResult::Success) {
-    const char *response_cstr = response.GetStringRef().data();
-
     // Hang on to the qSupported packet, so that platforms can do custom
     // configuration of the transport before attaching/launching the process.
-    m_qSupported_response = response_cstr;
-
-    if (::strstr(response_cstr, "qXfer:auxv:read+"))
-      m_supports_qXfer_auxv_read = eLazyBoolYes;
-    if (::strstr(response_cstr, "qXfer:libraries-svr4:read+"))
-      m_supports_qXfer_libraries_svr4_read = eLazyBoolYes;
-    if (::strstr(response_cstr, "augmented-libraries-svr4-read")) {
-      m_supports_qXfer_libraries_svr4_read = eLazyBoolYes; // implied
-      m_supports_augmented_libraries_svr4_read = eLazyBoolYes;
-    }
-    if (::strstr(response_cstr, "qXfer:libraries:read+"))
-      m_supports_qXfer_libraries_read = eLazyBoolYes;
-    if (::strstr(response_cstr, "qXfer:features:read+"))
-      m_supports_qXfer_features_read = eLazyBoolYes;
-    if (::strstr(response_cstr, "qXfer:memory-map:read+"))
-      m_supports_qXfer_memory_map_read = eLazyBoolYes;
-
-    // Look for a list of compressions in the features list e.g.
-    // qXfer:features:read+;PacketSize=20000;qEcho+;SupportedCompressions=zlib-
-    // deflate,lzma
-    const char *features_list = ::strstr(response_cstr, "qXfer:features:");
-    if (features_list) {
-      const char *compressions =
-          ::strstr(features_list, "SupportedCompressions=");
-      if (compressions) {
-        std::vector<std::string> supported_compressions;
-        compressions += sizeof("SupportedCompressions=") - 1;
-        const char *end_of_compressions = strchr(compressions, ';');
-        if (end_of_compressions == nullptr) {
-          end_of_compressions = strchr(compressions, '\0');
-        }
-        const char *current_compression = compressions;
-        while (current_compression < end_of_compressions) {
-          const char *next_compression_name = strchr(current_compression, ',');
-          const char *end_of_this_word = next_compression_name;
-          if (next_compression_name == nullptr ||
-              end_of_compressions < next_compression_name) {
-            end_of_this_word = end_of_compressions;
-          }
-
-          if (end_of_this_word) {
-            if (end_of_this_word == current_compression) {
-              current_compression++;
-            } else {
-              std::string this_compression(
-                  current_compression, end_of_this_word - current_compression);
-              supported_compressions.push_back(this_compression);
-              current_compression = end_of_this_word + 1;
-            }
-          } else {
-            supported_compressions.push_back(current_compression);
-            current_compression = end_of_compressions;
-          }
+    m_qSupported_response = response.GetStringRef().str();
+
+    llvm::SmallVector<llvm::StringRef, 16> server_features;
+    response.GetStringRef().split(server_features, ';');
+
+    for (auto x : server_features) {
+      if (x == "qXfer:auxv:read+")
+        m_supports_qXfer_auxv_read = eLazyBoolYes;
+      else if (x == "qXfer:libraries-svr4:read+")
+        m_supports_qXfer_libraries_svr4_read = eLazyBoolYes;
+      else if (x == "augmented-libraries-svr4-read") {
+        m_supports_qXfer_libraries_svr4_read = eLazyBoolYes; // implied
+        m_supports_augmented_libraries_svr4_read = eLazyBoolYes;
+      } else if (x == "qXfer:libraries:read+")
+        m_supports_qXfer_libraries_read = eLazyBoolYes;
+      else if (x == "qXfer:features:read+")
+        m_supports_qXfer_features_read = eLazyBoolYes;
+      else if (x == "qXfer:memory-map:read+")
+        m_supports_qXfer_memory_map_read = eLazyBoolYes;
+      else if (x == "qEcho")
+        m_supports_qEcho = eLazyBoolYes;
+      else if (x == "QPassSignals+")
+        m_supports_QPassSignals = eLazyBoolYes;
+      else if (x == "multiprocess+")
+        m_supports_multiprocess = eLazyBoolYes;
+      // Look for a list of compressions in the features list e.g.
+      // qXfer:features:read+;PacketSize=20000;qEcho+;SupportedCompressions=zlib-
+      // deflate,lzma
+      else if (x.consume_front("SupportedCompressions=")) {
+        llvm::SmallVector<llvm::StringRef, 4> compressions;
+        x.split(compressions, ',');
+        if (!compressions.empty())
+          MaybeEnableCompression(compressions);
+      } else if (x.consume_front("PacketSize=")) {
+        StringExtractorGDBRemote packet_response(x);
+        m_max_packet_size =
+            packet_response.GetHexMaxU64(/*little_endian=*/false, UINT64_MAX);
+        if (m_max_packet_size == 0) {
+          m_max_packet_size = UINT64_MAX; // Must have been a garbled response
+          Log *log(
+              ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS));
+          LLDB_LOGF(log, "Garbled PacketSize spec in qSupported response");
         }
-
-        if (supported_compressions.size() > 0) {
-          MaybeEnableCompression(supported_compressions);
-        }
-      }
-    }
-
-    if (::strstr(response_cstr, "qEcho"))
-      m_supports_qEcho = eLazyBoolYes;
-    else
-      m_supports_qEcho = eLazyBoolNo;
-
-    if (::strstr(response_cstr, "QPassSignals+"))
-      m_supports_QPassSignals = eLazyBoolYes;
-    else
-      m_supports_QPassSignals = eLazyBoolNo;
-
-    if (::strstr(response_cstr, "multiprocess+"))
-      m_supports_multiprocess = eLazyBoolYes;
-    else
-      m_supports_multiprocess = eLazyBoolNo;
-
-    const char *packet_size_str = ::strstr(response_cstr, "PacketSize=");
-    if (packet_size_str) {
-      StringExtractorGDBRemote packet_response(packet_size_str +
-                                               strlen("PacketSize="));
-      m_max_packet_size =
-          packet_response.GetHexMaxU64(/*little_endian=*/false, UINT64_MAX);
-      if (m_max_packet_size == 0) {
-        m_max_packet_size = UINT64_MAX; // Must have been a garbled response
-        Log *log(
-            ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS));
-        LLDB_LOGF(log, "Garbled PacketSize spec in qSupported response");
       }
     }
   }
@@ -1036,7 +993,7 @@ bool GDBRemoteCommunicationClient::GetGDBServerVersion() {
 }
 
 void GDBRemoteCommunicationClient::MaybeEnableCompression(
-    std::vector<std::string> supported_compressions) {
+    llvm::ArrayRef<llvm::StringRef> supported_compressions) {
   CompressionType avail_type = CompressionType::None;
   std::string avail_name;
 
index 96519d3..d559882 100644 (file)
@@ -601,7 +601,8 @@ protected:
 
   // Given the list of compression types that the remote debug stub can support,
   // possibly enable compression if we find an encoding we can handle.
-  void MaybeEnableCompression(std::vector<std::string> supported_compressions);
+  void MaybeEnableCompression(
+      llvm::ArrayRef<llvm::StringRef> supported_compressions);
 
   bool DecodeProcessInfoResponse(StringExtractorGDBRemote &response,
                                  ProcessInstanceInfo &process_info);