[Lldb-commits] [lldb] [lldb] [debugserver] fix qLaunchSuccess error, add QErrorStringInPack… (PR #82593)

Jason Molenda via lldb-commits lldb-commits at lists.llvm.org
Wed Feb 21 22:58:46 PST 2024


https://github.com/jasonmolenda created https://github.com/llvm/llvm-project/pull/82593

…etSupported

Pavel added an extension to lldb's gdb remote serial protocol that allows the debug stub to append an error message (ascii hex encoded) after an error response packet Exx.  This was added in 2017 in https://reviews.llvm.org/D34945 .  lldb sends the
QErrorStringInPacketSupported packet and then the remote stub may add these error strings.

debugserver has added these strings to the vAttach family of packets to explain why an attach failed, without waiting to see the QErrorStringInPacketSupported packet to enable the mode.

debugserver also has a bad implementation of this for the qLaunchSuccess packet, where "E" is sent followed by the literal characters of the error, instead of Exx;<asciihex string>, which lldb can have problems parsing.

This patch moves three utility functions earlier in RNBRemote.cpp, it accepts the QErrorStringInPacketSupported packet and only appends the expanded error messages when that has been sent (lldb always sends it at the beginning of a connection), fixes the error string returned by qLaunchSuccess and changes the vAttach error returns to only add the error string when the mode is enabled.

>From 0ba4e6402969028fa6152c366a56063a56acded1 Mon Sep 17 00:00:00 2001
From: Jason Molenda <jason at molenda.com>
Date: Wed, 21 Feb 2024 22:48:36 -0800
Subject: [PATCH] [lldb] [debugserver] fix qLaunchSuccess error, add
 QErrorStringInPacketSupported

Pavel added an extension to lldb's gdb remote serial protocol that
allows the debug stub to append an error message (ascii hex encoded)
after an error response packet Exx.  This was added in 2017 in
https://reviews.llvm.org/D34945 .  lldb sends the
QErrorStringInPacketSupported packet and then the remote stub may
add these error strings.

debugserver has added these strings to the vAttach family of packets
to explain why an attach failed, without waiting to see the
QErrorStringInPacketSupported packet to enable the mode.

debugserver also has a bad implementation of this for the qLaunchSuccess
packet, where "E" is sent followed by the literal characters of the
error, instead of Exx;<asciihex string>, which lldb can have problems
parsing.

This patch moves three utility functions earlier in RNBRemote.cpp,
it accepts the QErrorStringInPacketSupported packet and only appends
the expanded error messages when that has been sent (lldb always sends
it at the beginning of a connection), fixes the error string returned
by qLaunchSuccess and changes the vAttach error returns to only add
the error string when the mode is enabled.
---
 lldb/tools/debugserver/source/RNBRemote.cpp | 157 ++++++++++++--------
 lldb/tools/debugserver/source/RNBRemote.h   |   5 +
 2 files changed, 103 insertions(+), 59 deletions(-)

diff --git a/lldb/tools/debugserver/source/RNBRemote.cpp b/lldb/tools/debugserver/source/RNBRemote.cpp
index feea4c914ec536..8338e4d0032870 100644
--- a/lldb/tools/debugserver/source/RNBRemote.cpp
+++ b/lldb/tools/debugserver/source/RNBRemote.cpp
@@ -143,6 +143,39 @@ uint64_t decode_uint64(const char *p, int base, char **end = nullptr,
   return addr;
 }
 
+void append_hex_value(std::ostream &ostrm, const void *buf, size_t buf_size,
+                      bool swap) {
+  int i;
+  const uint8_t *p = (const uint8_t *)buf;
+  if (swap) {
+    for (i = static_cast<int>(buf_size) - 1; i >= 0; i--)
+      ostrm << RAWHEX8(p[i]);
+  } else {
+    for (size_t i = 0; i < buf_size; i++)
+      ostrm << RAWHEX8(p[i]);
+  }
+}
+
+std::string cstring_to_asciihex_string(const char *str) {
+  std::string hex_str;
+  hex_str.reserve(strlen(str) * 2);
+  while (str && *str) {
+    uint8_t c = *str++;
+    char hexbuf[5];
+    snprintf(hexbuf, sizeof(hexbuf), "%02x", c);
+    hex_str += hexbuf;
+  }
+  return hex_str;
+}
+
+void append_hexified_string(std::ostream &ostrm, const std::string &string) {
+  size_t string_size = string.size();
+  const char *string_buf = string.c_str();
+  for (size_t i = 0; i < string_size; i++) {
+    ostrm << RAWHEX8(*(string_buf + i));
+  }
+}
+
 extern void ASLLogCallback(void *baton, uint32_t flags, const char *format,
                            va_list args);
 
@@ -171,7 +204,8 @@ RNBRemote::RNBRemote()
       m_extended_mode(false), m_noack_mode(false),
       m_thread_suffix_supported(false), m_list_threads_in_stop_reply(false),
       m_compression_minsize(384), m_enable_compression_next_send_packet(false),
-      m_compression_mode(compression_types::none) {
+      m_compression_mode(compression_types::none),
+      m_enable_error_strings(false) {
   DNBLogThreadedIf(LOG_RNB_REMOTE, "%s", __PRETTY_FUNCTION__);
   CreatePacketTable();
 }
@@ -365,6 +399,11 @@ void RNBRemote::CreatePacketTable() {
   t.push_back(Packet(
       query_symbol_lookup, &RNBRemote::HandlePacket_qSymbol, NULL, "qSymbol:",
       "Notify that host debugger is ready to do symbol lookups"));
+  t.push_back(Packet(enable_error_strings,
+                     &RNBRemote::HandlePacket_QEnableErrorStrings, NULL,
+                     "QEnableErrorStrings",
+                     "Tell " DEBUGSERVER_PROGRAM_NAME
+                     " it can append descriptive error messages in replies."));
   t.push_back(Packet(json_query_thread_extended_info,
                      &RNBRemote::HandlePacket_jThreadExtendedInfo, NULL,
                      "jThreadExtendedInfo",
@@ -1566,11 +1605,13 @@ rnb_err_t RNBRemote::HandlePacket_qLaunchSuccess(const char *p) {
   if (m_ctx.HasValidProcessID() || m_ctx.LaunchStatus().Status() == 0)
     return SendPacket("OK");
   std::ostringstream ret_str;
-  std::string status_str;
-  std::string error_quoted = binary_encode_string
-               (m_ctx.LaunchStatusAsString(status_str));
-  ret_str << "E" << error_quoted;
-
+  ret_str << "E89";
+  if (m_enable_error_strings) {
+    std::string status_str;
+    std::string error_quoted =
+        cstring_to_asciihex_string(m_ctx.LaunchStatusAsString(status_str));
+    ret_str << ";" << error_quoted;
+  }
   return SendPacket(ret_str.str());
 }
 
@@ -2534,39 +2575,6 @@ rnb_err_t RNBRemote::HandlePacket_QSetProcessEvent(const char *p) {
   return SendPacket("OK");
 }
 
-void append_hex_value(std::ostream &ostrm, const void *buf, size_t buf_size,
-                      bool swap) {
-  int i;
-  const uint8_t *p = (const uint8_t *)buf;
-  if (swap) {
-    for (i = static_cast<int>(buf_size) - 1; i >= 0; i--)
-      ostrm << RAWHEX8(p[i]);
-  } else {
-    for (size_t i = 0; i < buf_size; i++)
-      ostrm << RAWHEX8(p[i]);
-  }
-}
-
-std::string cstring_to_asciihex_string(const char *str) {
-  std::string hex_str;
-  hex_str.reserve (strlen (str) * 2);
-  while (str && *str) {
-    uint8_t c = *str++;
-    char hexbuf[5];
-    snprintf (hexbuf, sizeof(hexbuf), "%02x", c);
-    hex_str += hexbuf;
-  }
-  return hex_str;
-}
-
-void append_hexified_string(std::ostream &ostrm, const std::string &string) {
-  size_t string_size = string.size();
-  const char *string_buf = string.c_str();
-  for (size_t i = 0; i < string_size; i++) {
-    ostrm << RAWHEX8(*(string_buf + i));
-  }
-}
-
 void register_value_in_hex_fixed_width(std::ostream &ostrm, nub_process_t pid,
                                        nub_thread_t tid,
                                        const register_map_entry_t *reg,
@@ -3908,10 +3916,13 @@ rnb_err_t RNBRemote::HandlePacket_v(const char *p) {
     if (attach_pid == INVALID_NUB_PROCESS_ARCH) {
       DNBLogError("debugserver is x86_64 binary running in translation, attach "
                   "failed.");
-      std::string return_message = "E96;";
-      return_message +=
-          cstring_to_asciihex_string("debugserver is x86_64 binary running in "
-                                     "translation, attach failed.");
+      std::string return_message = "E96";
+      if (m_enable_error_strings) {
+        return_message += ";";
+        return_message += cstring_to_asciihex_string(
+            "debugserver is x86_64 binary running in "
+            "translation, attach failed.");
+      }
       SendPacket(return_message.c_str());
       return rnb_err;
     }
@@ -3944,15 +3955,22 @@ rnb_err_t RNBRemote::HandlePacket_v(const char *p) {
         // The order of these checks is important.  
         if (process_does_not_exist (pid_attaching_to)) {
           DNBLogError("Tried to attach to pid that doesn't exist");
-          std::string return_message = "E96;";
-          return_message += cstring_to_asciihex_string("no such process.");
+          std::string return_message = "E96";
+          if (m_enable_error_strings) {
+            return_message += ";";
+            return_message += cstring_to_asciihex_string("no such process.");
+          }
           return SendPacket(return_message);
         }
         if (process_is_already_being_debugged (pid_attaching_to)) {
           DNBLogError("Tried to attach to process already being debugged");
-          std::string return_message = "E96;";
-          return_message += cstring_to_asciihex_string("tried to attach to "
+          std::string return_message = "E96";
+          if (m_enable_error_strings) {
+            return_message += ";";
+            return_message +=
+                cstring_to_asciihex_string("tried to attach to "
                                            "process already being debugged");
+          }
           return SendPacket(return_message);
         }
         uid_t my_uid, process_uid;
@@ -3969,30 +3987,43 @@ rnb_err_t RNBRemote::HandlePacket_v(const char *p) {
             process_username = pw->pw_name;
           }
           DNBLogError("Tried to attach to process with uid mismatch");
-          std::string return_message = "E96;";
-          std::string msg = "tried to attach to process as user '" 
-                            + my_username + "' and process is running "
-                            "as user '" + process_username + "'";
-          return_message += cstring_to_asciihex_string(msg.c_str());
+          std::string return_message = "E96";
+          if (m_enable_error_strings) {
+            return_message += ";";
+            std::string msg = "tried to attach to process as user '" +
+                              my_username +
+                              "' and process is running "
+                              "as user '" +
+                              process_username + "'";
+            return_message += cstring_to_asciihex_string(msg.c_str());
+          }
           return SendPacket(return_message);
         }
         if (!login_session_has_gui_access() && !developer_mode_enabled()) {
           DNBLogError("Developer mode is not enabled and this is a "
                       "non-interactive session");
-          std::string return_message = "E96;";
-          return_message += cstring_to_asciihex_string("developer mode is "
+          std::string return_message = "E96";
+          if (m_enable_error_strings) {
+            return_message += ";";
+            return_message +=
+                cstring_to_asciihex_string("developer mode is "
                                            "not enabled on this machine "
                                            "and this is a non-interactive "
                                            "debug session.");
+          }
           return SendPacket(return_message);
         }
         if (!login_session_has_gui_access()) {
           DNBLogError("This is a non-interactive session");
-          std::string return_message = "E96;";
-          return_message += cstring_to_asciihex_string("this is a "
+          std::string return_message = "E96";
+          if (m_enable_error_strings) {
+            return_message += ";";
+            return_message +=
+                cstring_to_asciihex_string("this is a "
                                            "non-interactive debug session, "
                                            "cannot get permission to debug "
                                            "processes.");
+          }
           return SendPacket(return_message);
         }
       }
@@ -4013,9 +4044,12 @@ rnb_err_t RNBRemote::HandlePacket_v(const char *p) {
         error_explainer += err_str;
         error_explainer += ")";
       }
-      std::string default_return_msg = "E96;";
-      default_return_msg += cstring_to_asciihex_string 
-                              (error_explainer.c_str());
+      std::string default_return_msg = "E96";
+      if (m_enable_error_strings) {
+        default_return_msg += ";";
+        default_return_msg +=
+            cstring_to_asciihex_string(error_explainer.c_str());
+      }
       SendPacket (default_return_msg);
       DNBLogError("Attach failed: \"%s\".", err_str);
       return rnb_err;
@@ -6195,6 +6229,11 @@ rnb_err_t RNBRemote::HandlePacket_qSymbol(const char *command) {
   }
 }
 
+rnb_err_t RNBRemote::HandlePacket_QEnableErrorStrings(const char *p) {
+  m_enable_error_strings = true;
+  return SendPacket("OK");
+}
+
 // Note that all numeric values returned by qProcessInfo are hex encoded,
 // including the pid and the cpu type.
 
diff --git a/lldb/tools/debugserver/source/RNBRemote.h b/lldb/tools/debugserver/source/RNBRemote.h
index dad390ae0b6320..91f04fd6b40e11 100644
--- a/lldb/tools/debugserver/source/RNBRemote.h
+++ b/lldb/tools/debugserver/source/RNBRemote.h
@@ -137,6 +137,7 @@ class RNBRemote {
     set_detach_on_error,                // 'QSetDetachOnError:'
     query_transfer,                     // 'qXfer:'
     json_query_dyld_process_state,      // 'jGetDyldProcessState'
+    enable_error_strings,               // 'QEnableErrorStrings'
     unknown_type
   };
 
@@ -196,6 +197,7 @@ class RNBRemote {
   rnb_err_t HandlePacket_qGDBServerVersion(const char *p);
   rnb_err_t HandlePacket_qProcessInfo(const char *p);
   rnb_err_t HandlePacket_qSymbol(const char *p);
+  rnb_err_t HandlePacket_QEnableErrorStrings(const char *p);
   rnb_err_t HandlePacket_QStartNoAckMode(const char *p);
   rnb_err_t HandlePacket_QThreadSuffixSupported(const char *p);
   rnb_err_t HandlePacket_QSetLogging(const char *p);
@@ -405,6 +407,9 @@ class RNBRemote {
   bool m_enable_compression_next_send_packet;
 
   compression_types m_compression_mode;
+
+  bool m_enable_error_strings; // Whether we can append asciihex error strings
+                               // after Exx error replies
 };
 
 /* We translate the /usr/include/mach/exception_types.h exception types



More information about the lldb-commits mailing list