Adding Support for Error Strings in Remote Packets

Summary:
This patch adds support for sending strings along with
error codes in the reply packets. The implementation is
based on the feedback recieved in the lldb-dev mailing
list. The patch also adds an extra packet for the client
to query if the server has the capability to provide
strings along with error replys.

Reviewers: labath, jingham, sas, lldb-commits, clayborg

Reviewed By: labath, clayborg

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

llvm-svn: 307768

6 files changed, 63 insertions, 10 deletions

diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 33aed7a43c4..e6fd386b903 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -86,6 +86,7 @@ GDBRemoteCommunicationClient::GDBRemoteCommunicationClient()
       m_supports_jLoadedDynamicLibrariesInfos(eLazyBoolCalculate),
       m_supports_jGetSharedCacheInfo(eLazyBoolCalculate),
       m_supports_QPassSignals(eLazyBoolCalculate),
+      m_supports_error_string_reply(eLazyBoolCalculate),
       m_supports_qProcessInfoPID(true), m_supports_qfProcessInfo(true),
       m_supports_qUserName(true), m_supports_qGroupName(true),
       m_supports_qThreadStopInfo(true), m_supports_z0(true),
@@ -596,6 +597,21 @@ bool GDBRemoteCommunicationClient::GetThreadExtendedInfoSupported() {
   return m_supports_jThreadExtendedInfo;
 }
 
+void GDBRemoteCommunicationClient::EnableErrorStringInPacket() {
+  if (m_supports_error_string_reply == eLazyBoolCalculate) {
+    StringExtractorGDBRemote response;
+    // We try to enable error strings in remote packets
+    // but if we fail, we just work in the older way.
+    m_supports_error_string_reply = eLazyBoolNo;
+    if (SendPacketAndWaitForResponse("QEnableErrorStrings", response, false) ==
+        PacketResult::Success) {
+      if (response.IsOKResponse()) {
+        m_supports_error_string_reply = eLazyBoolYes;
+      }
+    }
+  }
+}
+
 bool GDBRemoteCommunicationClient::GetLoadedDynamicLibrariesInfosSupported() {
   if (m_supports_jLoadedDynamicLibrariesInfos == eLazyBoolCalculate) {
     StringExtractorGDBRemote response;
@@ -3181,8 +3197,8 @@ GDBRemoteCommunicationClient::SendStartTracePacket(const TraceOptions &options,
                                    true) ==
       GDBRemoteCommunication::PacketResult::Success) {
     if (!response.IsNormalResponse()) {
-      error.SetError(response.GetError(), eErrorTypeGeneric);
-      LLDB_LOG(log, "Target does not support Tracing");
+      error = response.GetStatus();
+      LLDB_LOG(log, "Target does not support Tracing , error {0}", error);
     } else {
       ret_uid = response.GetHexMaxU64(false, LLDB_INVALID_UID);
     }
@@ -3219,7 +3235,7 @@ GDBRemoteCommunicationClient::SendStopTracePacket(lldb::user_id_t uid,
                                    true) ==
       GDBRemoteCommunication::PacketResult::Success) {
     if (!response.IsOKResponse()) {
-      error.SetError(response.GetError(), eErrorTypeGeneric);
+      error = response.GetStatus();
       LLDB_LOG(log, "stop tracing failed");
     }
   } else {
@@ -3234,6 +3250,7 @@ GDBRemoteCommunicationClient::SendStopTracePacket(lldb::user_id_t uid,
 Status GDBRemoteCommunicationClient::SendGetDataPacket(
     lldb::user_id_t uid, lldb::tid_t thread_id,
     llvm::MutableArrayRef<uint8_t> &buffer, size_t offset) {
+
   StreamGDBRemote escaped_packet;
   escaped_packet.PutCString("jTraceBufferRead:");
   return SendGetTraceDataPacket(escaped_packet, uid, thread_id, buffer, offset);
@@ -3242,6 +3259,7 @@ Status GDBRemoteCommunicationClient::SendGetDataPacket(
 Status GDBRemoteCommunicationClient::SendGetMetaDataPacket(
     lldb::user_id_t uid, lldb::tid_t thread_id,
     llvm::MutableArrayRef<uint8_t> &buffer, size_t offset) {
+
   StreamGDBRemote escaped_packet;
   escaped_packet.PutCString("jTraceMetaRead:");
   return SendGetTraceDataPacket(escaped_packet, uid, thread_id, buffer, offset);
@@ -3308,7 +3326,7 @@ GDBRemoteCommunicationClient::SendGetTraceConfigPacket(lldb::user_id_t uid,
                   custom_params_sp));
       }
     } else {
-      error.SetError(response.GetError(), eErrorTypeGeneric);
+      error = response.GetStatus();
     }
   } else {
     LLDB_LOG(log, "failed to send packet");
@@ -3344,7 +3362,7 @@ Status GDBRemoteCommunicationClient::SendGetTraceDataPacket(
       size_t filled_size = response.GetHexBytesAvail(buffer);
       buffer = llvm::MutableArrayRef<uint8_t>(buffer.data(), filled_size);
     } else {
-      error.SetError(response.GetError(), eErrorTypeGeneric);
+      error = response.GetStatus();
       buffer = buffer.slice(buffer.size());
     }
   } else {
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
index a38110faaec..712d85eed08 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -340,6 +340,8 @@ public:
 
   bool GetQXferAuxvReadSupported();
 
+  void EnableErrorStringInPacket();
+
   bool GetQXferLibrariesReadSupported();
 
   bool GetQXferLibrariesSVR4ReadSupported();
@@ -549,6 +551,7 @@ protected:
   LazyBool m_supports_jLoadedDynamicLibrariesInfos;
   LazyBool m_supports_jGetSharedCacheInfo;
   LazyBool m_supports_QPassSignals;
+  LazyBool m_supports_error_string_reply;
 
   bool m_supports_qProcessInfoPID : 1, m_supports_qfProcessInfo : 1,
       m_supports_qUserName : 1, m_supports_qGroupName : 1,
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp
index dac675ee943..4be92b79fd1 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp
@@ -20,6 +20,7 @@
 // Project includes
 #include "ProcessGDBRemoteLog.h"
 #include "Utility/StringExtractorGDBRemote.h"
+#include "lldb/Utility/StreamString.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -27,7 +28,12 @@ using namespace lldb_private::process_gdb_remote;
 
 GDBRemoteCommunicationServer::GDBRemoteCommunicationServer(
     const char *comm_name, const char *listener_name)
-    : GDBRemoteCommunication(comm_name, listener_name), m_exit_now(false) {}
+    : GDBRemoteCommunication(comm_name, listener_name), m_exit_now(false) {
+  RegisterPacketHandler(
+      StringExtractorGDBRemote::eServerPacketType_QEnableErrorStrings,
+      [this](StringExtractorGDBRemote packet, Status &error, bool &interrupt,
+             bool &quit) { return this->Handle_QErrorStringEnable(packet); });
+}
 
 GDBRemoteCommunicationServer::~GDBRemoteCommunicationServer() {}
 
@@ -100,6 +106,24 @@ GDBRemoteCommunicationServer::SendErrorResponse(uint8_t err) {
 }
 
 GDBRemoteCommunication::PacketResult
+GDBRemoteCommunicationServer::SendErrorResponse(const Status &error) {
+  if (m_send_error_strings) {
+    lldb_private::StreamString packet;
+    packet.Printf("E%2.2x;", static_cast<uint8_t>(error.GetError()));
+    packet.PutCStringAsRawHex8(error.AsCString());
+    return SendPacketNoLock(packet.GetString());
+  } else
+    return SendErrorResponse(error.GetError());
+}
+
+GDBRemoteCommunication::PacketResult
+GDBRemoteCommunicationServer::Handle_QErrorStringEnable(
+    StringExtractorGDBRemote &packet) {
+  m_send_error_strings = true;
+  return SendOKResponse();
+}
+
+GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServer::SendIllFormedResponse(
     const StringExtractorGDBRemote &failed_packet, const char *message) {
   Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PACKETS));
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h
index 6eb25f8b9f9..a3535248004 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h
@@ -57,6 +57,13 @@ protected:
   bool m_exit_now; // use in asynchronous handling to indicate process should
                    // exit.
 
+  bool m_send_error_strings; // If the client enables this then
+                             // we will send error strings as well.
+
+  PacketResult Handle_QErrorStringEnable(StringExtractorGDBRemote &packet);
+
+  PacketResult SendErrorResponse(const Status &error);
+
   PacketResult SendUnimplementedResponse(const char *packet);
 
   PacketResult SendErrorResponse(uint8_t error);
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index c2aa2616a96..a7fe4ee3b14 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -1123,7 +1123,7 @@ GDBRemoteCommunicationServerLLGS::Handle_jTraceStart(
   uid = m_debugged_process_sp->StartTrace(options, error);
   LLDB_LOG(log, "uid is {0} , error is {1}", uid, error.GetError());
   if (error.Fail())
-    return SendErrorResponse(error.GetError());
+    return SendErrorResponse(error);
 
   StreamGDBRemote response;
   response.Printf("%" PRIx64, uid);
@@ -1160,7 +1160,7 @@ GDBRemoteCommunicationServerLLGS::Handle_jTraceStop(
   Status error = m_debugged_process_sp->StopTrace(uid, tid);
 
   if (error.Fail())
-    return SendErrorResponse(error.GetError());
+    return SendErrorResponse(error);
 
   return SendOKResponse();
 }
@@ -1203,7 +1203,7 @@ GDBRemoteCommunicationServerLLGS::Handle_jTraceConfigRead(
   Status error = m_debugged_process_sp->GetTraceConfig(uid, options);
 
   if (error.Fail())
-    return SendErrorResponse(error.GetError());
+    return SendErrorResponse(error);
 
   StreamGDBRemote escaped_response;
   StructuredData::Dictionary json_packet;
@@ -1279,7 +1279,7 @@ GDBRemoteCommunicationServerLLGS::Handle_jTraceRead(
     error = m_debugged_process_sp->GetMetaData(uid, tid, buf, offset);
 
   if (error.Fail())
-    return SendErrorResponse(error.GetError());
+    return SendErrorResponse(error);
 
   for (auto i : buf)
     response.PutHex8(i);
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 042c96111bb..8a66f3865eb 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1031,6 +1031,7 @@ Status ProcessGDBRemote::ConnectToDebugserver(llvm::StringRef connect_url) {
   m_gdb_comm.GetHostInfo();
   m_gdb_comm.GetVContSupported('c');
   m_gdb_comm.GetVAttachOrWaitSupported();
+  m_gdb_comm.EnableErrorStringInPacket();
 
   // Ask the remote server for the default thread id
   if (GetTarget().GetNonStopModeEnabled())