aadsm updated this revision to Diff 202323.
aadsm added a comment.
- Introduce better error handling by creating 2 new error classes, one for
generic packet errors that contain a number and another for unimplemented
features.
- The logic for reading the xfer object was moved into its own function.
- Removes the define for linux || netbsd for the auxv xfer object.
- Switched from shared pointer to a unique pointer to store the memory buffers
in the map
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62499/new/
https://reviews.llvm.org/D62499
Files:
lldb/include/lldb/Utility/StringExtractorGDBRemote.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
lldb/source/Utility/StringExtractorGDBRemote.cpp
Index: lldb/source/Utility/StringExtractorGDBRemote.cpp
===================================================================
--- lldb/source/Utility/StringExtractorGDBRemote.cpp
+++ lldb/source/Utility/StringExtractorGDBRemote.cpp
@@ -285,8 +285,8 @@
break;
case 'X':
- if (PACKET_STARTS_WITH("qXfer:auxv:read::"))
- return eServerPacketType_qXfer_auxv_read;
+ if (PACKET_STARTS_WITH("qXfer:"))
+ return eServerPacketType_qXfer;
break;
}
break;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -82,7 +82,8 @@
MainLoop::ReadHandleUP m_stdio_handle_up;
lldb::StateType m_inferior_prev_state = lldb::StateType::eStateInvalid;
- std::unique_ptr<llvm::MemoryBuffer> m_active_auxv_buffer_up;
+ std::unordered_map<std::string, std::unique_ptr<llvm::MemoryBuffer>>
+ m_xfer_buffer_map;
std::mutex m_saved_registers_mutex;
std::unordered_map<uint32_t, lldb::DataBufferSP> m_saved_registers_map;
uint32_t m_next_saved_registers_id = 1;
@@ -150,7 +151,7 @@
PacketResult Handle_s(StringExtractorGDBRemote &packet);
- PacketResult Handle_qXfer_auxv_read(StringExtractorGDBRemote &packet);
+ PacketResult Handle_qXfer(StringExtractorGDBRemote &packet);
PacketResult Handle_QSaveRegisterState(StringExtractorGDBRemote &packet);
@@ -193,6 +194,9 @@
FileSpec FindModuleFile(const std::string &module_path,
const ArchSpec &arch) override;
+ llvm::Expected<std::unique_ptr<llvm::MemoryBuffer>>
+ ReadXferObject(llvm::StringRef object, llvm::StringRef annex);
+
private:
void HandleInferiorState_Exited(NativeProcessProtocol *process);
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -144,8 +144,8 @@
StringExtractorGDBRemote::eServerPacketType_qWatchpointSupportInfo,
&GDBRemoteCommunicationServerLLGS::Handle_qWatchpointSupportInfo);
RegisterMemberFunctionHandler(
- StringExtractorGDBRemote::eServerPacketType_qXfer_auxv_read,
- &GDBRemoteCommunicationServerLLGS::Handle_qXfer_auxv_read);
+ StringExtractorGDBRemote::eServerPacketType_qXfer,
+ &GDBRemoteCommunicationServerLLGS::Handle_qXfer);
RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_s,
&GDBRemoteCommunicationServerLLGS::Handle_s);
RegisterMemberFunctionHandler(
@@ -2747,47 +2747,19 @@
return PacketResult::Success;
}
-GDBRemoteCommunication::PacketResult
-GDBRemoteCommunicationServerLLGS::Handle_qXfer_auxv_read(
- StringExtractorGDBRemote &packet) {
-// *BSD impls should be able to do this too.
-#if defined(__linux__) || defined(__NetBSD__)
- Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS));
-
- // Parse out the offset.
- packet.SetFilePos(strlen("qXfer:auxv:read::"));
- if (packet.GetBytesLeft() < 1)
- return SendIllFormedResponse(packet,
- "qXfer:auxv:read:: packet missing offset");
-
- const uint64_t auxv_offset =
- packet.GetHexMaxU64(false, std::numeric_limits<uint64_t>::max());
- if (auxv_offset == std::numeric_limits<uint64_t>::max())
- return SendIllFormedResponse(packet,
- "qXfer:auxv:read:: packet missing offset");
-
- // Parse out comma.
- if (packet.GetBytesLeft() < 1 || packet.GetChar() != ',')
- return SendIllFormedResponse(
- packet, "qXfer:auxv:read:: packet missing comma after offset");
-
- // Parse out the length.
- const uint64_t auxv_length =
- packet.GetHexMaxU64(false, std::numeric_limits<uint64_t>::max());
- if (auxv_length == std::numeric_limits<uint64_t>::max())
- return SendIllFormedResponse(packet,
- "qXfer:auxv:read:: packet missing length");
-
- // Grab the auxv data if we need it.
- if (!m_active_auxv_buffer_up) {
+llvm::Expected<std::unique_ptr<llvm::MemoryBuffer>>
+GDBRemoteCommunicationServerLLGS::ReadXferObject(llvm::StringRef object,
+ llvm::StringRef annex) {
+ if (object == "auxv") {
+ Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS));
// Make sure we have a valid process.
if (!m_debugged_process_up ||
(m_debugged_process_up->GetID() == LLDB_INVALID_PROCESS_ID)) {
if (log)
- log->Printf(
- "GDBRemoteCommunicationServerLLGS::%s failed, no process available",
- __FUNCTION__);
- return SendErrorResponse(0x10);
+ log->Printf("GDBRemoteCommunicationServerLLGS::%s failed, no process "
+ "available",
+ __FUNCTION__);
+ return llvm::make_error<PacketError>(0x10);
}
// Grab the auxv data.
@@ -2795,46 +2767,88 @@
if (!buffer_or_error) {
std::error_code ec = buffer_or_error.getError();
LLDB_LOG(log, "no auxv data retrieved: {0}", ec.message());
- return SendErrorResponse(ec.value());
+ return llvm::make_error<PacketError>(ec.value());
}
- m_active_auxv_buffer_up = std::move(*buffer_or_error);
+ return std::move(*buffer_or_error);
}
+ return llvm::make_error<PacketUnimplementedError>(
+ "Xfer object not supported");
+}
+
+GDBRemoteCommunication::PacketResult
+GDBRemoteCommunicationServerLLGS::Handle_qXfer(
+ StringExtractorGDBRemote &packet) {
+ SmallVector<StringRef, 5> fields;
+ // The packet format is "qXfer:<object>:<action>:<annex>:offset,length"
+ StringRef(packet.GetStringRef()).split(fields, ':', 4);
+ if (fields.size() != 5)
+ return SendIllFormedResponse(packet, "malformed qXfer packet");
+ auto &xfer_object = fields[1];
+ auto &xfer_action = fields[2];
+ auto &xfer_annex = fields[3];
+ StringExtractor offset_data(fields[4]);
+ if (xfer_action != "read")
+ return SendUnimplementedResponse("qXfer action not supported");
+ std::string buffer_key = std::string(xfer_object) + std::string(xfer_action) +
+ std::string(xfer_annex);
+ // Parse offset.
+ const uint64_t xfer_offset =
+ offset_data.GetHexMaxU64(false, std::numeric_limits<uint64_t>::max());
+ if (xfer_offset == std::numeric_limits<uint64_t>::max())
+ return SendIllFormedResponse(packet, "qXfer packet missing offset");
+ // Parse out comma.
+ if (offset_data.GetChar() != ',')
+ return SendIllFormedResponse(packet,
+ "qXfer packet missing comma after offset");
+ // Parse out the length.
+ const uint64_t xfer_length =
+ offset_data.GetHexMaxU64(false, std::numeric_limits<uint64_t>::max());
+ if (xfer_length == std::numeric_limits<uint64_t>::max())
+ return SendIllFormedResponse(packet, "qXfer packet missing length");
+
+ // Get a previously constructed buffer if it exists or create it now.
+ std::shared_ptr<llvm::MemoryBuffer> memory_buffer_up;
+ auto buffer_it = m_xfer_buffer_map.find(buffer_key);
+ if (buffer_it == m_xfer_buffer_map.end()) {
+ auto buffer_up = ReadXferObject(xfer_object, xfer_annex);
+ if (!buffer_up)
+ return SendErrorResponse(buffer_up.takeError());
+ buffer_it = m_xfer_buffer_map
+ .insert(std::make_pair(buffer_key, std::move(*buffer_up)))
+ .first;
+ }
+
+ // Send back the response
StreamGDBRemote response;
bool done_with_buffer = false;
-
- llvm::StringRef buffer = m_active_auxv_buffer_up->getBuffer();
- if (auxv_offset >= buffer.size()) {
+ llvm::StringRef buffer = buffer_it->second->getBuffer();
+ if (xfer_offset >= buffer.size()) {
// We have nothing left to send. Mark the buffer as complete.
response.PutChar('l');
done_with_buffer = true;
} else {
// Figure out how many bytes are available starting at the given offset.
- buffer = buffer.drop_front(auxv_offset);
-
+ buffer = buffer.drop_front(xfer_offset);
// Mark the response type according to whether we're reading the remainder
- // of the auxv data.
- if (auxv_length >= buffer.size()) {
+ // of the data.
+ if (xfer_length >= buffer.size()) {
// There will be nothing left to read after this
response.PutChar('l');
done_with_buffer = true;
} else {
// There will still be bytes to read after this request.
response.PutChar('m');
- buffer = buffer.take_front(auxv_length);
+ buffer = buffer.take_front(xfer_length);
}
-
// Now write the data in encoded binary form.
response.PutEscapedBytes(buffer.data(), buffer.size());
}
if (done_with_buffer)
- m_active_auxv_buffer_up.reset();
+ m_xfer_buffer_map.erase(buffer_key);
return SendPacketNoLock(response.GetString());
-#else
- return SendUnimplementedResponse("not implemented on this platform");
-#endif
}
GDBRemoteCommunication::PacketResult
@@ -3259,8 +3273,8 @@
void GDBRemoteCommunicationServerLLGS::ClearProcessSpecificData() {
Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS));
- LLDB_LOG(log, "clearing auxv buffer: {0}", m_active_auxv_buffer_up.get());
- m_active_auxv_buffer_up.reset();
+ LLDB_LOG(log, "clearing {0} xfer buffers", m_xfer_buffer_map.size());
+ m_xfer_buffer_map.clear();
}
FileSpec
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h
@@ -15,6 +15,9 @@
#include "GDBRemoteCommunication.h"
#include "lldb/lldb-private-forward.h"
+#include "llvm/Support/Errc.h"
+#include "llvm/Support/Error.h"
+
class StringExtractorGDBRemote;
namespace lldb_private {
@@ -59,6 +62,8 @@
PacketResult SendErrorResponse(const Status &error);
+ PacketResult SendErrorResponse(llvm::Error error);
+
PacketResult SendUnimplementedResponse(const char *packet);
PacketResult SendErrorResponse(uint8_t error);
@@ -72,6 +77,34 @@
DISALLOW_COPY_AND_ASSIGN(GDBRemoteCommunicationServer);
};
+class PacketUnimplementedError
+ : public llvm::ErrorInfo<PacketUnimplementedError, llvm::StringError> {
+public:
+ using llvm::ErrorInfo<PacketUnimplementedError,
+ llvm::StringError>::ErrorInfo; // inherit constructors
+ PacketUnimplementedError(const llvm::Twine &S)
+ : ErrorInfo(S, llvm::errc::not_supported) {}
+
+ PacketUnimplementedError() : ErrorInfo(llvm::errc::not_supported) {}
+};
+
+class PacketError : public llvm::ErrorInfo<PacketError> {
+public:
+ static char ID;
+
+ PacketError(uint8_t error_number) : m_error_number(error_number) {}
+ uint8_t GetErrorNumber() { return m_error_number; }
+ void log(llvm::raw_ostream &OS) const override {
+ OS << "Packet error " << m_error_number << ".";
+ }
+ std::error_code convertToErrorCode() const override {
+ return llvm::inconvertibleErrorCode();
+ }
+
+private:
+ uint8_t m_error_number;
+};
+
} // namespace process_gdb_remote
} // namespace lldb_private
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp
@@ -113,6 +113,21 @@
}
GDBRemoteCommunication::PacketResult
+GDBRemoteCommunicationServer::SendErrorResponse(llvm::Error error) {
+ GDBRemoteCommunication::PacketResult result;
+ llvm::handleAllErrors(
+ std::move(error),
+ [&](PacketUnimplementedError &e) {
+ result = SendUnimplementedResponse(e.message().c_str());
+ },
+ [&](PacketError &e) { result = SendErrorResponse(e.GetErrorNumber()); },
+ [&](std::unique_ptr<llvm::ErrorInfoBase> e) {
+ result = SendErrorResponse(std::move(e));
+ });
+ return result;
+}
+
+GDBRemoteCommunication::PacketResult
GDBRemoteCommunicationServer::Handle_QErrorStringEnable(
StringExtractorGDBRemote &packet) {
m_send_error_strings = true;
@@ -138,3 +153,5 @@
bool GDBRemoteCommunicationServer::HandshakeWithClient() {
return GetAck() == PacketResult::Success;
}
+
+char PacketError::ID;
Index: lldb/include/lldb/Utility/StringExtractorGDBRemote.h
===================================================================
--- lldb/include/lldb/Utility/StringExtractorGDBRemote.h
+++ lldb/include/lldb/Utility/StringExtractorGDBRemote.h
@@ -128,7 +128,7 @@
eServerPacketType_qVAttachOrWaitSupported,
eServerPacketType_qWatchpointSupportInfo,
eServerPacketType_qWatchpointSupportInfoSupported,
- eServerPacketType_qXfer_auxv_read,
+ eServerPacketType_qXfer,
eServerPacketType_jSignalsInfo,
eServerPacketType_jModulesInfo,
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
