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
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 33aed7a..e6fd386 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -86,6 +86,7 @@
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 @@
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 @@
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 @@
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 @@
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::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 @@
custom_params_sp));
}
} else {
- error.SetError(response.GetError(), eErrorTypeGeneric);
+ error = response.GetStatus();
}
} else {
LLDB_LOG(log, "failed to send packet");
@@ -3344,7 +3362,7 @@
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 a38110f..712d85e 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -340,6 +340,8 @@
bool GetQXferAuxvReadSupported();
+ void EnableErrorStringInPacket();
+
bool GetQXferLibrariesReadSupported();
bool GetQXferLibrariesSVR4ReadSupported();
@@ -549,6 +551,7 @@
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 dac675e..4be92b7 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 @@
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 @@
}
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 6eb25f8..a353524 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h
@@ -57,6 +57,13 @@
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 c2aa261..a7fe4ee 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -1123,7 +1123,7 @@
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 @@
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 @@
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 @@
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 042c961..8a66f38 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1031,6 +1031,7 @@
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())
diff --git a/lldb/source/Utility/StringExtractorGDBRemote.cpp b/lldb/source/Utility/StringExtractorGDBRemote.cpp
index 3473a9e..8174c15 100644
--- a/lldb/source/Utility/StringExtractorGDBRemote.cpp
+++ b/lldb/source/Utility/StringExtractorGDBRemote.cpp
@@ -19,8 +19,18 @@
switch (m_packet[0]) {
case 'E':
- if (m_packet.size() == 3 && isxdigit(m_packet[1]) && isxdigit(m_packet[2]))
- return eError;
+ 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':
@@ -86,6 +96,8 @@
return eServerPacketType_QEnvironment;
if (PACKET_STARTS_WITH("QEnvironmentHexEncoded:"))
return eServerPacketType_QEnvironmentHexEncoded;
+ if (PACKET_STARTS_WITH("QEnableErrorStrings"))
+ return eServerPacketType_QEnableErrorStrings;
break;
case 'P':
@@ -438,8 +450,8 @@
}
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() {
@@ -450,6 +462,23 @@
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()
diff --git a/lldb/source/Utility/StringExtractorGDBRemote.h b/lldb/source/Utility/StringExtractorGDBRemote.h
index 473cab0..f4ed642 100644
--- a/lldb/source/Utility/StringExtractorGDBRemote.h
+++ b/lldb/source/Utility/StringExtractorGDBRemote.h
@@ -10,6 +10,7 @@
#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
@@ -72,6 +73,7 @@
eServerPacketType_qGetWorkingDir,
eServerPacketType_qFileLoadAddress,
eServerPacketType_QEnvironment,
+ eServerPacketType_QEnableErrorStrings,
eServerPacketType_QLaunchArch,
eServerPacketType_QSetDisableASLR,
eServerPacketType_QSetDetachOnError,
@@ -190,6 +192,8 @@
// digits. Otherwise the error encoded in XX is returned.
uint8_t GetError();
+ lldb_private::Status GetStatus();
+
size_t GetEscapedBinaryData(std::string &str);
protected: