Adding Support for Error Strings in Remote Packets

[ravitheja]

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.

[labath]

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

3204

AsCString unnecessary

3329

I don't think every packet function should need to write these four lines of code. This should be abstracted away. Ideally I'd like to see this as a method on the response class (i.e., simply error = response.GetStatus()), but if it's hard to plumb that information into the class cleanly, I can imagine it being a method on the communication class as well (error = this->GetStatus(response))

update: It looks like the GetStatus function already supports that: Why not just write this as error = response.GetStatus() ???

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp

110

Condensed version without dodgy C functions:
return SendPacketNoLock(llvm::formatv("E{0:x-2};{1}", error.GetError(), error).str());

116

Aren't you supposed to send these only if the client enabled the error response?

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h

71

Why the reference?

source/Utility/StringExtractorGDBRemote.cpp

473

replace 47 with the actual error code

[ravitheja]

changes from feedback.

[ravitheja]

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp

116

From the discussions on the dev-list I thought the client needs to query if the server will send the error string or not and from the
server side its always enabled ?

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h

71

how about const reference ?

[ravitheja]

With this patch, I see the extra packet to query from server is probably unnecessary. I mean the error code is still there and it should be sufficient for the client to detect an error. As long as it does not mistake it for something else it should not be a problem ? .

[labath]

In D34945#798808, @ravitheja wrote:

With this patch, I see the extra packet to query from server is probably unnecessary. I mean the error code is still there and it should be sufficient for the client to detect an error. As long as it does not mistake it for something else it should not be a problem ? .

Well, if we don't care about newer lldb-server being able to communicate with older clients than it's theoretically superfluous. I'm not sure whether we care about this scenario, but I was kinda expecting we do it this way, because that's how it's done elsewhere (e.g. QListThreadsInStopReply). Let's see if others have anything to say about this (might take a bit of time as it's holiday in the US now).

docs/lldb-gdb-remote.txt

137

The quotes here are misleading -- after reading this I would expect that the string is sent quoted. Best be explicit about the encoding of the error string.

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h

71

much better

[ravitheja]

Yeah that would be broken, as long as we make the error packet more than 3 bytes, then it won't work with the older lldb clients.

[clayborg]

I would think we would try to enable this using something like:

QEnableErrorStrings

And if the the server responds with "OK" then we know it is enabled. By default the server should not enable any fancy features without being asked. We would like lldb-server to stay compatible with other GDB remote clients.

The error string should be hex ASCII encoded so the string can contain any characters. Any string that can contain any content should be hex ASCII encoded.

docs/lldb-gdb-remote.txt

137

Maybe:

EXX;AAAAAA

Where XX is a hex byte error number, followed by a semicolon and where AAAAAA is a hex ascii encoded error message.

284

Just make similar to whatever we decide on in the docs for QErrorStringInPacketSupported

318

Just make similar to whatever we decide on in the docs for QErrorStringInPacketSupported

351

Just make similar to whatever we decide on in the docs for QErrorStringInPacketSupported

393

Just make similar to whatever we decide on in the docs for QErrorStringInPacketSupported

[ravitheja]

Updating Doc and changing feature to be enabled by client.

[ravitheja]

Forgot this in the doc.

[labath]

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

3176

I don't like how every packet function needs to enable this manually. Every function can benefit from this. Can we just set this once at startup, when we probe other server features?

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp

111

Doesn't look like this is hex-encoding.

source/Utility/StringExtractorGDBRemote.cpp

477

This doesn't look like hex-encoding.

[clayborg]

See inlined comments.

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

3176

I would move this into ProcessGDBRemote::ConnectToDebugserver() where many one time things happen. We should enable this once at the beginning and then deal with the error messages always possibly being present. The code in ProcessGDBRemote::ConnectToDebugserver() where you would insert:

m_gdb_comm.GetEchoSupported();
m_gdb_comm.GetThreadSuffixSupported();
m_gdb_comm.GetListThreadsInStopReplySupported();
m_gdb_comm.GetHostInfo();
m_gdb_comm.GetVContSupported('c');
m_gdb_comm.GetVAttachOrWaitSupported();
m_gdb_comm.EnableErrorStringInPacket(); /// <<< HERE

3220

remove

3253

remove

3262

remove

3275

remove

3345

remove

source/Utility/StringExtractorGDBRemote.cpp

21–22

We should probably be a bit more specific with the check to check for the ';' or NULL terminator at index 3:

if (isxdigit(m_packet[1]) && isxdigit(m_packet[2])) {
  if (m_packet.size() == 3)
    return eError; 
  if (m_packet[3] == ';') {
    if (verify all remaining bytes are valid HEX ASCII bytes)
      return eError; 
  }
}

We don't want some random string packet response like "Each time I do" to make this the response claim to be an error response.

477

hex encode

[clayborg]

source/Utility/StringExtractorGDBRemote.cpp

477

There is also a hex decode that will need to be added somewhere...

[ravitheja]

Support for Hex encoded strings and more error checking.

[labath]

I am generally happy with this, just a couple of things I noticed below:

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

3329

The indenting look wrong. Please run the patch through clang-format before submission.

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h

158 ↗

(On Diff #105380)

It doesn't look like you're implementing this anywhere. Please remove.

source/Utility/StringExtractorGDBRemote.cpp

26

Please use StringRef here. No need to copy the string just to examine it's contents.

29

This will break out of the inner for loop, which is probably not what you intended.

[ravitheja]

Correcting mistakes.

[clayborg]

Looks good. Just one question on why we register a packet handler.

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp

32–35

Why is this done here and not where all of the other packets are registered?

[ravitheja]

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp

32–35

I did it here coz this class has the member SendErrorResponse which I wanted to overload. Also I think the remote packets are a bit spreadout among inherited classes, so it will be available for all these classes and then these won't be spread out like packet handling code will be in one place.

[clayborg]

Wait for an OK from Pavel as well.

[clayborg]

Note that if you put "Differential Revision: " followed by the URL for this:

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

in your source control commit message it will automatically close this for you and add the SVN revision number into this as a message. Please note the SVN revision in here if possible since you did it manually.

[ravitheja]

Uploaded SVN Revision Number - 307773 to fix Android Builder.

docs/lldb-gdb-remote.txt

 respond with a "$#00" (unimplemented):
 
 send packet: $QEnvironment:41434b5f434f4c4f525f46494c454e414d453d626f6c642379656c6c6f77#00
 read packet: $OK#00
 
 This packet can be sent one or more times _prior_ to sending a "A" packet.
 
 //----------------------------------------------------------------------
+// "QEnableErrorStrings"
+//
+// BRIEF
+//  This packet enables reporting of Error strings in remote packet
+//  replies from the server to client. If the server supports this
+//  feature, it should send an OK response. The client can expect the
+//  following error replies if this feature is enabled in the server ->
+//
+//  EXX;AAAAAAAAA
+//
+//  where AAAAAAAAA will be a hex encoded ASCII string.
+//  XX is hex encoded byte number.
+//
+//  It must be noted that even if the client has enabled reporting
+//  strings in error replies, it must not expect error strings to all
+//  error replies.
+// 
+// PRIORITY TO IMPLEMENT
+//  Low. Only needed if the remote target wants to provide strings that
+//  are human readable along with an error code.
+//----------------------------------------------------------------------
+
+send packet: $QErrorStringInPacketSupported
+read packet: $OK#00
+
+//----------------------------------------------------------------------
 // "QSetSTDIN:<ascii-hex-path>"
 // "QSetSTDOUT:<ascii-hex-path>"
 // "QSetSTDERR:<ascii-hex-path>"
 //
 // BRIEF
 //  Setup where STDIN, STDOUT, and STDERR go prior to sending an "A" 
 //  packet.
 //

 //  params          Any parameters that are specific to      O
 //                  certain trace technologies should be
 //                  collectively specified as a JSON
 //                  dictionary
 //  ==========      ====================================================
 //
 //  Each tracing instance is identified by a trace id which is returned
 //  as the reply to this packet. In case the tracing failed to begin an
-//  error code is returned instead.
+//  error code along with a hex encoded ASCII message is returned
+//  instead.
 //----------------------------------------------------------------------
 
 send packet: jTraceStart:{"type":<type>,"buffersize":<buffersize>}]
-read packet: <trace id>/E<error code>
+read packet: <trace id>/E<error code>;AAAAAAAAA
 
 //----------------------------------------------------------------------
 // jTraceStop:
 //
 // BRIEF
 //  Stop tracing instance with trace id <trace id>, of course trace
 //  needs to be started before. The following parameters should be
 //  formatted as a JSON dictionary to the packet. Since sending

 //                  on. Since <trace id> could map to
 //                  multiple trace instances (in case it
 //                  maps to the complete process), the
 //                  threadid of a particular thread could
 //                  be appended as "threadid:<thread id>;"
 //                  to stop tracing on that thread.
 //  ==========      ====================================================
 //
-//  An OK response is sent in case of success else an error code is
-//  returned.
+//  An OK response is sent in case of success else an error code along
+//  with a hex encoded ASCII message is returned.
 //----------------------------------------------------------------------
 
 send packet: jTraceStop:{"traceid":<trace id>}]
-read packet: <OK response>/E<error code>
+read packet: <OK response>/E<error code>;AAAAAAAAA
 
 //----------------------------------------------------------------------
 // jTraceBufferRead:
 //
 // BRIEF
 //  Packet for reading the trace for tracing instance <trace id>, i.e the
 //  id obtained from StartTrace API. The following parameters should be
 //  formatted as a JSON dictionary to the packet. Since sending

 //
 //  buffersize      The size of the data intended to read.  M
 //
 //  threadid        The id of the thread to retrieve data   O
 //                  from.
 //  ==========      ====================================================
 //
 //  The trace data is sent as raw binary data if the read was successful
-//  else an error code is sent.
+//  else an error code along with a hex encoded ASCII message is sent.
 //----------------------------------------------------------------------
 
 send packet: jTraceBufferRead:{"traceid":<trace id>,"offset":<byteoffset>,"buffersize":<byte_count>}]
-read packet: <binary trace data>/E<error code>
+read packet: <binary trace data>/E<error code>;AAAAAAAAA
 
 //----------------------------------------------------------------------
 // jTraceMetaRead:
 //
 // BRIEF
 //  Similar Packet as above except it reads meta data.
 //----------------------------------------------------------------------
 

 //                  trace configuration of that thread.
 //  ==========      ====================================================
 //
 //  In the response packet the trace configuration is sent as text,
 //  formatted as a JSON dictionary. Since sending JSON data over
 //  gdb-remote protocol has certain limitations, binary escaping
 //  convention is used.
 //  In case the trace instance with the <trace id> was not found, an
-//  error code is returned.
+//  error code along with a hex encoded ASCII message is returned.
 //----------------------------------------------------------------------
 
 send packet: jTraceConfigRead:{"traceid":<trace id>}
-read packet: {"conf1":<conf1>,"conf2":<conf2>,"params":{"paramName":paramValue}]}];/E<error code>
+read packet: {"conf1":<conf1>,"conf2":<conf2>,"params":{"paramName":paramValue}]}];/E<error code>;AAAAAAAAA
 
 //----------------------------------------------------------------------
 // "qRegisterInfo<hex-reg-id>"
 //
 // BRIEF
 //  Discover register information from the remote GDB server.
 //
 // PRIORITY TO IMPLEMENT

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

public:
   bool SendSpeedTestPacket(uint32_t send_size, uint32_t recv_size);
 
   bool SetCurrentThread(uint64_t tid);
 
   bool SetCurrentThreadForRun(uint64_t tid);
 
   bool GetQXferAuxvReadSupported();
 
+  void EnableErrorStringInPacket();
+
   bool GetQXferLibrariesReadSupported();
 
   bool GetQXferLibrariesSVR4ReadSupported();
 
   uint64_t GetRemoteMaxPacketSize();
 
   bool GetEchoSupported();
 
protected:
   LazyBool m_supports_qXfer_libraries_read;
   LazyBool m_supports_qXfer_libraries_svr4_read;
   LazyBool m_supports_qXfer_features_read;
   LazyBool m_supports_augmented_libraries_svr4_read;
   LazyBool m_supports_jThreadExtendedInfo;
   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,
       m_supports_qThreadStopInfo : 1, m_supports_z0 : 1, m_supports_z1 : 1,
       m_supports_z2 : 1, m_supports_z3 : 1, m_supports_z4 : 1,
       m_supports_QEnvironment : 1, m_supports_QEnvironmentHexEncoded : 1,
       m_supports_qSymbol : 1, m_qSymbol_requests_done : 1,
       m_supports_qModuleInfo : 1, m_supports_jThreadsInfo : 1,

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

: GDBRemoteClientBase("gdb-remote.client", "gdb-remote.client.rx_packet"),
       m_supports_qXfer_libraries_read(eLazyBoolCalculate),
       m_supports_qXfer_libraries_svr4_read(eLazyBoolCalculate),
       m_supports_qXfer_features_read(eLazyBoolCalculate),
       m_supports_augmented_libraries_svr4_read(eLazyBoolCalculate),
       m_supports_jThreadExtendedInfo(eLazyBoolCalculate),
       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),
       m_supports_z1(true), m_supports_z2(true), m_supports_z3(true),
       m_supports_z4(true), m_supports_QEnvironment(true),
       m_supports_QEnvironmentHexEncoded(true), m_supports_qSymbol(true),
       m_qSymbol_requests_done(false), m_supports_qModuleInfo(true),
       m_supports_jThreadsInfo(true), m_supports_jModulesInfo(true),
if (SendPacketAndWaitForResponse("jThreadExtendedInfo:", response, false) ==
       if (response.IsOKResponse()) {
         m_supports_jThreadExtendedInfo = eLazyBoolYes;
       }
     }
   }
   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;
     m_supports_jLoadedDynamicLibrariesInfos = eLazyBoolNo;
     if (SendPacketAndWaitForResponse("jGetLoadedDynamicLibrariesInfos:",
                                      response,
                                      false) == PacketResult::Success) {
       if (response.IsOKResponse()) {

 }
 
 lldb::user_id_t
 GDBRemoteCommunicationClient::SendStartTracePacket(const TraceOptions &options,
                                                    Status &error) {
   Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS));
   lldb::user_id_t ret_uid = LLDB_INVALID_UID;
 
   StreamGDBRemote escaped_packet;
   escaped_packet.PutCString("jTraceStart:");
 
   StructuredData::Dictionary json_packet;
   json_packet.AddIntegerItem("type", options.getType());
   json_packet.AddIntegerItem("buffersize", options.getTraceBufferSize());
   json_packet.AddIntegerItem("metabuffersize", options.getMetaDataBufferSize());
 
   if (options.getThreadID() != LLDB_INVALID_THREAD_ID)
     json_packet.AddIntegerItem("threadid", options.getThreadID());
 
   StructuredData::DictionarySP custom_params = options.getTraceParams();
   if (custom_params)
     json_packet.AddItem("params", custom_params);
 
   StreamString json_string;
   json_packet.Dump(json_string, false);
   escaped_packet.PutEscapedBytes(json_string.GetData(), json_string.GetSize());
 
   StringExtractorGDBRemote response;
   if (SendPacketAndWaitForResponse(escaped_packet.GetString(), response,
                                    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);
     }
   } else {
     LLDB_LOG(log, "failed to send packet");
     error.SetErrorStringWithFormat("failed to send packet: '%s'",
                                    escaped_packet.GetData());
   }
   return ret_uid;
 }
 
 Status
 GDBRemoteCommunicationClient::SendStopTracePacket(lldb::user_id_t uid,
                                                   lldb::tid_t thread_id) {
   Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS));
   StringExtractorGDBRemote response;
   Status error;
 
   StructuredData::Dictionary json_packet;
   StreamGDBRemote escaped_packet;
   StreamString json_string;
   escaped_packet.PutCString("jTraceStop:");
 
   json_packet.AddIntegerItem("traceid", uid);
 
   if (thread_id != LLDB_INVALID_THREAD_ID)
     json_packet.AddIntegerItem("threadid", thread_id);
 
   json_packet.Dump(json_string, false);
 
   escaped_packet.PutEscapedBytes(json_string.GetData(), json_string.GetSize());
 
   if (SendPacketAndWaitForResponse(escaped_packet.GetString(), response,
                                    true) ==
       GDBRemoteCommunication::PacketResult::Success) {
     if (!response.IsOKResponse()) {
-      error.SetError(response.GetError(), eErrorTypeGeneric);
+      error = response.GetStatus();
       LLDB_LOG(log, "stop tracing failed");
     }
   } else {
     LLDB_LOG(log, "failed to send packet");
     error.SetErrorStringWithFormat(
         "failed to send packet: '%s' with error '%d'", escaped_packet.GetData(),
         response.GetError());
   }
   return error;
 }
 
 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);
 }
 
 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);
 }
 
 Status
 GDBRemoteCommunicationClient::SendGetTraceConfigPacket(lldb::user_id_t uid,
                                                        TraceOptions &options) {
   Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS));
   StringExtractorGDBRemote response;
   Status error;
 
   StreamString json_string;
   StreamGDBRemote escaped_packet;
   escaped_packet.PutCString("jTraceConfigRead:");
 
   StructuredData::Dictionary json_packet;
   json_packet.AddIntegerItem("traceid", uid);
 
   if (options.getThreadID() != LLDB_INVALID_THREAD_ID)
     json_packet.AddIntegerItem("threadid", options.getThreadID());
if (response.IsNormalResponse()) {
           error.SetErrorString("Invalid Configuration obtained");
           return error;
         } else
           options.setTraceParams(
               static_pointer_cast<StructuredData::Dictionary>(
                   custom_params_sp));
       }
     } else {
-      error.SetError(response.GetError(), eErrorTypeGeneric);
+      error = response.GetStatus();
     }
   } else {
     LLDB_LOG(log, "failed to send packet");
     error.SetErrorStringWithFormat("failed to send packet: '%s'",
                                    escaped_packet.GetData());
   }
   return error;
 }
 
 Status GDBRemoteCommunicationClient::SendGetTraceDataPacket(
     StreamGDBRemote &packet, lldb::user_id_t uid, lldb::tid_t thread_id,
     llvm::MutableArrayRef<uint8_t> &buffer, size_t offset) {
   Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS));
   Status error;
 
   StructuredData::Dictionary json_packet;
 
   json_packet.AddIntegerItem("traceid", uid);
   json_packet.AddIntegerItem("offset", offset);
   json_packet.AddIntegerItem("buffersize", buffer.size());
 
   if (thread_id != LLDB_INVALID_THREAD_ID)
     json_packet.AddIntegerItem("threadid", thread_id);
 
   StreamString json_string;
   json_packet.Dump(json_string, false);
 
   packet.PutEscapedBytes(json_string.GetData(), json_string.GetSize());
   StringExtractorGDBRemote response;
   if (SendPacketAndWaitForResponse(packet.GetString(), response, true) ==
       GDBRemoteCommunication::PacketResult::Success) {
     if (response.IsNormalResponse()) {
       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 {
     LLDB_LOG(log, "failed to send packet");
     error.SetErrorStringWithFormat("failed to send packet: '%s'",
                                    packet.GetData());
     buffer = buffer.slice(buffer.size());
   }

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h

public:
   bool HandshakeWithClient();
 
 protected:
   std::map<StringExtractorGDBRemote::ServerPacketType, PacketHandler>
       m_packet_handlers;
   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);
 
   PacketResult SendIllFormedResponse(const StringExtractorGDBRemote &packet,
                                      const char *error_message);
 
   PacketResult SendOKResponse();
 
 private:
   DISALLOW_COPY_AND_ASSIGN(GDBRemoteCommunicationServer);
 };
 
 } // namespace process_gdb_remote
 } // namespace lldb_private
 
 #endif // liblldb_GDBRemoteCommunicationServer_h_

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp

 
 // C Includes
 // C++ Includes
 #include <cstring>
 
 // Project includes
 #include "ProcessGDBRemoteLog.h"
 #include "Utility/StringExtractorGDBRemote.h"
+#include "lldb/Utility/StreamString.h"
 
 using namespace lldb;
 using namespace lldb_private;
 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() {}
 
 void GDBRemoteCommunicationServer::RegisterPacketHandler(
     StringExtractorGDBRemote::ServerPacketType packet_type,
     PacketHandler handler) {
   m_packet_handlers[packet_type] = std::move(handler);
 }

 GDBRemoteCommunicationServer::SendErrorResponse(uint8_t err) {
   char packet[16];
   int packet_len = ::snprintf(packet, sizeof(packet), "E%2.2x", err);
   assert(packet_len < (int)sizeof(packet));
   return SendPacketNoLock(llvm::StringRef(packet, packet_len));
 }
 
 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));
   if (log)
     log->Printf("GDBRemoteCommunicationServer::%s: ILLFORMED: '%s' (%s)",
                 __FUNCTION__, failed_packet.GetStringRef().c_str(),
                 message ? message : "");
   return SendErrorResponse(0x03);

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

if (buffersize == std::numeric_limits<uint64_t>::max() ||
     return SendIllFormedResponse(packet, "JTrace:start: Ill formed packet ");
   }
 
   Status error;
   lldb::user_id_t uid = LLDB_INVALID_UID;
   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);
   return SendPacketNoLock(response.GetString());
 }
 
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerLLGS::Handle_jTraceStop(
GDBRemoteCommunicationServerLLGS::Handle_jTraceStop(
   if (!json_dict->GetValueForKeyAsInteger("traceid", uid))
     return SendIllFormedResponse(packet, "jTraceStop: Ill formed packet ");
 
   json_dict->GetValueForKeyAsInteger("threadid", tid);
 
   Status error = m_debugged_process_sp->StopTrace(uid, tid);
 
   if (error.Fail())
-    return SendErrorResponse(error.GetError());
+    return SendErrorResponse(error);
 
   return SendOKResponse();
 }
 
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerLLGS::Handle_jTraceConfigRead(
     StringExtractorGDBRemote &packet) {
 
GDBRemoteCommunicationServerLLGS::Handle_jTraceConfigRead(
 
   TraceOptions options;
   StreamGDBRemote response;
 
   options.setThreadID(threadid);
   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;
 
   json_packet.AddIntegerItem("type", options.getType());
   json_packet.AddIntegerItem("buffersize", options.getTraceBufferSize());
   json_packet.AddIntegerItem("metabuffersize", options.getMetaDataBufferSize());
 
GDBRemoteCommunicationServerLLGS::Handle_jTraceRead(
   llvm::MutableArrayRef<uint8_t> buf(buffer.get(), byte_count);
 
   if (tracetype == BufferData)
     error = m_debugged_process_sp->GetData(uid, tid, buf, offset);
   else if (tracetype == MetaData)
     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);
 
   StreamGDBRemote escaped_response;
   escaped_response.PutEscapedBytes(response.GetData(), response.GetSize());
   return SendPacketNoLock(escaped_response.GetString());
 }

source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

if (GetTarget().GetNonStopModeEnabled())
     GetTarget().SetNonStopModeEnabled(m_gdb_comm.SetNonStopMode(true));
 
   m_gdb_comm.GetEchoSupported();
   m_gdb_comm.GetThreadSuffixSupported();
   m_gdb_comm.GetListThreadsInStopReplySupported();
   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())
     m_gdb_comm.GetDefaultThreadId(m_initial_tid);
 
   size_t num_cmds = GetExtraStartupCommands().GetArgumentCount();
   for (size_t idx = 0; idx < num_cmds; idx++) {
     StringExtractorGDBRemote response;

source/Utility/StringExtractorGDBRemote.h

 //===-- StringExtractorGDBRemote.h ------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef utility_StringExtractorGDBRemote_h_
 #define utility_StringExtractorGDBRemote_h_
 
+#include "lldb/Utility/Status.h"
 #include "lldb/Utility/StringExtractor.h"
 #include "llvm/ADT/StringRef.h" // for StringRef
 
 #include <string>
 
 #include <stddef.h> // for size_t
 #include <stdint.h> // for uint8_t
 
enum ServerPacketType {
     eServerPacketType_qLaunchSuccess,
     eServerPacketType_qModuleInfo,
     eServerPacketType_qProcessInfoPID,
     eServerPacketType_qSpeedTest,
     eServerPacketType_qUserName,
     eServerPacketType_qGetWorkingDir,
     eServerPacketType_qFileLoadAddress,
     eServerPacketType_QEnvironment,
+    eServerPacketType_QEnableErrorStrings,
     eServerPacketType_QLaunchArch,
     eServerPacketType_QSetDisableASLR,
     eServerPacketType_QSetDetachOnError,
     eServerPacketType_QSetSTDIN,
     eServerPacketType_QSetSTDOUT,
     eServerPacketType_QSetSTDERR,
     eServerPacketType_QSetWorkingDir,
     eServerPacketType_QStartNoAckMode,
public:
   bool IsNormalResponse() const;
 
   bool IsErrorResponse() const;
 
   // Returns zero if the packet isn't a EXX packet where XX are two hex
   // digits. Otherwise the error encoded in XX is returned.
   uint8_t GetError();
 
+  lldb_private::Status GetStatus();
+
   size_t GetEscapedBinaryData(std::string &str);
 
 protected:
   ResponseValidatorCallback m_validator;
   void *m_validator_baton;
 };
 
 #endif // utility_StringExtractorGDBRemote_h_

source/Utility/StringExtractorGDBRemote.cpp

 #include <string.h>
 
 StringExtractorGDBRemote::ResponseType
 StringExtractorGDBRemote::GetResponseType() const {
   if (m_packet.empty())
     return eUnsupported;
 
   switch (m_packet[0]) {
   case 'E':
-    if (m_packet.size() == 3 && isxdigit(m_packet[1]) && isxdigit(m_packet[2]))
+    if (isxdigit(m_packet[1]) && isxdigit(m_packet[2])) {
+      if (m_packet.size() == 3)
         return eError;
+      llvm::StringRef packet_ref(m_packet);
+      if (packet_ref[3] == ';') {
+        auto err_string = packet_ref.substr(4);
+        for (auto e : err_string)
+          if (!isxdigit(e))
+            return eResponse;
+        return eError;
+      }
+    }
     break;
 
   case 'O':
     if (m_packet.size() == 2 && m_packet[1] == 'K')
       return eOK;
     break;
 
   case '+':
#define PACKET_STARTS_WITH(s)                                                  \
   case 'Q':
 
     switch (packet_cstr[1]) {
     case 'E':
       if (PACKET_STARTS_WITH("QEnvironment:"))
         return eServerPacketType_QEnvironment;
       if (PACKET_STARTS_WITH("QEnvironmentHexEncoded:"))
         return eServerPacketType_QEnvironmentHexEncoded;
+      if (PACKET_STARTS_WITH("QEnableErrorStrings"))
+        return eServerPacketType_QEnableErrorStrings;
       break;
 
     case 'P':
       if (PACKET_STARTS_WITH("QPassSignals:"))
         return eServerPacketType_QPassSignals;
 
     case 'S':
       if (PACKET_MATCHES("QStartNoAckMode"))
bool StringExtractorGDBRemote::IsUnsupportedResponse() const {
   return GetResponseType() == eUnsupported;
 }
 
 bool StringExtractorGDBRemote::IsNormalResponse() const {
   return GetResponseType() == eResponse;
 }
 
 bool StringExtractorGDBRemote::IsErrorResponse() const {
-  return GetResponseType() == eError && m_packet.size() == 3 &&
-         isxdigit(m_packet[1]) && isxdigit(m_packet[2]);
+  return GetResponseType() == eError && isxdigit(m_packet[1]) &&
+         isxdigit(m_packet[2]);
 }
 
 uint8_t StringExtractorGDBRemote::GetError() {
   if (GetResponseType() == eError) {
     SetFilePos(1);
     return GetHexU8(255);
   }
   return 0;
 }
 
+lldb_private::Status StringExtractorGDBRemote::GetStatus() {
+  lldb_private::Status error;
+  if (GetResponseType() == eError) {
+    SetFilePos(1);
+    uint8_t errc = GetHexU8(255);
+    error.SetError(errc, lldb::eErrorTypeGeneric);
+
+    std::string error_messg("Error ");
+    error_messg += std::to_string(errc);
+    if (GetChar() == ';')
+      GetHexByteString(error_messg);
+
+    error.SetErrorString(error_messg);
+  }
+  return error;
+}
+
 size_t StringExtractorGDBRemote::GetEscapedBinaryData(std::string &str) {
   // Just get the data bytes in the string as
   // GDBRemoteCommunication::CheckForPacket()
   // already removes any 0x7d escaped characters. If any 0x7d characters are
   // left in
   // the packet, then they are supposed to be there...
   str.clear();
   const size_t bytes_left = GetBytesLeft();