mgorny created this revision.
mgorny added reviewers: labath, jingham, krytarowski, emaste.
Herald added a subscriber: arichardson.
Herald added a project: All.
mgorny requested review of this revision.
Introduce an isolation layer between GDBRemoteClientBase
and GDBRemoteCommunication. This makes it possible to replace
the public packet reading/writing API with wrappers that will make
threaded access possible in the future.
Sponsored by: The FreeBSD Foundation
https://reviews.llvm.org/D135029
Files:
lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
lldb/unittests/tools/lldb-server/tests/TestClient.cpp
Index: lldb/unittests/tools/lldb-server/tests/TestClient.cpp
===================================================================
--- lldb/unittests/tools/lldb-server/tests/TestClient.cpp
+++ lldb/unittests/tools/lldb-server/tests/TestClient.cpp
@@ -31,8 +31,8 @@
#endif
TestClient::TestClient(std::unique_ptr<Connection> Conn) {
- SetConnection(std::move(Conn));
- SetPacketTimeout(std::chrono::seconds(10));
+ GetCommunication().SetConnection(std::move(Conn));
+ GetCommunication().SetPacketTimeout(std::chrono::seconds(10));
}
TestClient::~TestClient() {
@@ -50,7 +50,7 @@
if (Error E = SendMessage("QStartNoAckMode"))
return E;
- m_send_acks = false;
+ GetCommunication().DisableSendingAcks();
return Error::success();
}
@@ -268,7 +268,8 @@
m_stop_reply = std::move(*StopReplyOr);
if (!isa<StopReplyStop>(m_stop_reply)) {
StringExtractorGDBRemote R;
- PacketResult result = ReadPacket(R, GetPacketTimeout(), false);
+ PacketResult result =
+ ReadPacket(R, GetCommunication().GetPacketTimeout(), false);
if (result != PacketResult::ErrorDisconnected) {
return make_error<StringError>(
formatv("Expected connection close after sending {0}. Got {1}/{2} "
Index: lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
===================================================================
--- lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -29,7 +29,9 @@
typedef GDBRemoteCommunication::PacketResult PacketResult;
struct TestClient : public GDBRemoteCommunicationClient {
- TestClient() { m_send_acks = false; }
+ TestClient() {
+ GetCommunication().DisableSendingAcks();
+ }
};
void Handle_QThreadSuffixSupported(MockServer &server, bool supported) {
@@ -62,7 +64,8 @@
class GDBRemoteCommunicationClientTest : public GDBRemoteTest {
public:
void SetUp() override {
- ASSERT_THAT_ERROR(GDBRemoteCommunication::ConnectLocally(client, server),
+ ASSERT_THAT_ERROR(GDBRemoteCommunication::ConnectLocally(
+ client.GetCommunication(), server),
llvm::Succeeded());
}
Index: lldb/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
===================================================================
--- lldb/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
+++ lldb/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
@@ -40,14 +40,15 @@
struct TestClient : public GDBRemoteClientBase {
TestClient() : GDBRemoteClientBase("test.client") {
- m_send_acks = false;
+ GetCommunication().DisableSendingAcks();
}
};
class GDBRemoteClientBaseTest : public GDBRemoteTest {
public:
void SetUp() override {
- ASSERT_THAT_ERROR(GDBRemoteCommunication::ConnectLocally(client, server),
+ ASSERT_THAT_ERROR(GDBRemoteCommunication::ConnectLocally(
+ client.GetCommunication(), server),
llvm::Succeeded());
ASSERT_EQ(TestClient::eBroadcastBitRunPacketSent,
listener_sp->StartListeningForEvents(
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -109,7 +109,10 @@
return;
}
StreamFile stream(std::move(file.get()));
- ((ProcessGDBRemote *)p)->GetGDBRemote().DumpHistory(stream);
+ ((ProcessGDBRemote *)p)
+ ->GetGDBRemote()
+ .GetCommunication()
+ .DumpHistory(stream);
}
} // namespace lldb
@@ -281,7 +284,8 @@
const uint64_t timeout_seconds =
GetGlobalPluginProperties().GetPacketTimeout();
if (timeout_seconds > 0)
- m_gdb_comm.SetPacketTimeout(std::chrono::seconds(timeout_seconds));
+ m_gdb_comm.GetCommunication().SetPacketTimeout(
+ std::chrono::seconds(timeout_seconds));
m_use_g_packet_for_reading =
GetGlobalPluginProperties().GetUseGPacketForReading();
@@ -796,8 +800,8 @@
{
// Scope for the scoped timeout object
- GDBRemoteCommunication::ScopedTimeout timeout(m_gdb_comm,
- std::chrono::seconds(10));
+ GDBRemoteCommunication::ScopedTimeout timeout(
+ m_gdb_comm.GetCommunication(), std::chrono::seconds(10));
int arg_packet_err = m_gdb_comm.SendArgumentsPacket(launch_info);
if (arg_packet_err == 0) {
@@ -862,7 +866,7 @@
uint32_t retry_count = 0;
while (!m_gdb_comm.IsConnected()) {
if (conn_up->Connect(connect_url, &error) == eConnectionStatusSuccess) {
- m_gdb_comm.SetConnection(std::move(conn_up));
+ m_gdb_comm.GetCommunication().SetConnection(std::move(conn_up));
break;
}
@@ -3262,7 +3266,7 @@
communication_fd = gdb_socket;
#endif
- error = m_gdb_comm.StartDebugserverProcess(
+ error = m_gdb_comm.GetCommunication().StartDebugserverProcess(
nullptr, GetTarget().GetPlatform().get(), debugserver_launch_info,
nullptr, nullptr, communication_fd);
@@ -3276,7 +3280,7 @@
// Our process spawned correctly, we can now set our connection to use
// our end of the socket pair
cleanup_our.release();
- m_gdb_comm.SetConnection(
+ m_gdb_comm.GetCommunication().SetConnection(
std::make_unique<ConnectionFileDescriptor>(our_socket, true));
#endif
StartAsyncThread();
@@ -3810,7 +3814,7 @@
if (m_gdb_comm.GetLoadedDynamicLibrariesInfosSupported()) {
// Scope for the scoped timeout object
- GDBRemoteCommunication::ScopedTimeout timeout(m_gdb_comm,
+ GDBRemoteCommunication::ScopedTimeout timeout(m_gdb_comm.GetCommunication(),
std::chrono::seconds(10));
StreamString packet;
@@ -4945,7 +4949,8 @@
ProcessGDBRemote *process =
(ProcessGDBRemote *)m_interpreter.GetExecutionContext().GetProcessPtr();
if (process) {
- process->GetGDBRemote().DumpHistory(result.GetOutputStream());
+ process->GetGDBRemote().GetCommunication().DumpHistory(
+ result.GetOutputStream());
result.SetStatus(eReturnStatusSuccessFinishResult);
return true;
}
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
@@ -457,7 +457,7 @@
if (log) {
if (log->GetVerbose()) {
StreamString strm;
- gdb_comm.DumpHistory(strm);
+ gdb_comm.GetCommunication().DumpHistory(strm);
LLDB_LOGF(log,
"error: failed to get packet sequence mutex, not sending "
"write register for \"%s\":\n%s",
@@ -566,7 +566,7 @@
if (log) {
if (log->GetVerbose()) {
StreamString strm;
- gdb_comm.DumpHistory(strm);
+ gdb_comm.GetCommunication().DumpHistory(strm);
LLDB_LOGF(log,
"error: failed to get packet sequence mutex, not sending "
"read all registers:\n%s",
@@ -741,7 +741,7 @@
if (log) {
if (log->GetVerbose()) {
StreamString strm;
- gdb_comm.DumpHistory(strm);
+ gdb_comm.GetCommunication().DumpHistory(strm);
LLDB_LOGF(log,
"error: failed to get packet sequence mutex, not sending "
"write all registers:\n%s",
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -569,6 +569,7 @@
LazyBool m_supports_memory_tagging = eLazyBoolCalculate;
LazyBool m_supports_qSaveCore = eLazyBoolCalculate;
LazyBool m_uses_native_signals = eLazyBoolCalculate;
+ LazyBool m_supports_qEcho = eLazyBoolCalculate;
bool m_supports_qProcessInfoPID : 1, m_supports_qfProcessInfo : 1,
m_supports_qUserName : 1, m_supports_qGroupName : 1,
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -196,20 +196,20 @@
bool GDBRemoteCommunicationClient::QueryNoAckModeSupported() {
if (m_supports_not_sending_acks == eLazyBoolCalculate) {
- m_send_acks = true;
m_supports_not_sending_acks = eLazyBoolNo;
// This is the first real packet that we'll send in a debug session and it
// may take a little longer than normal to receive a reply. Wait at least
// 6 seconds for a reply to this packet.
- ScopedTimeout timeout(*this, std::max(GetPacketTimeout(), seconds(6)));
+ GDBRemoteCommunication::ScopedTimeout timeout(
+ m_comm, std::max(m_comm.GetPacketTimeout(), seconds(6)));
StringExtractorGDBRemote response;
if (SendPacketAndWaitForResponse("QStartNoAckMode", response) ==
PacketResult::Success) {
if (response.IsOKResponse()) {
- m_send_acks = false;
+ m_comm.DisableSendingAcks();
m_supports_not_sending_acks = eLazyBoolYes;
}
return true;
@@ -380,9 +380,10 @@
m_supports_qXfer_memory_map_read = eLazyBoolYes;
else if (x == "qXfer:siginfo:read+")
m_supports_qXfer_siginfo_read = eLazyBoolYes;
- else if (x == "qEcho")
+ else if (x == "qEcho") {
m_supports_qEcho = eLazyBoolYes;
- else if (x == "QPassSignals+")
+ m_comm.EnableqEcho();
+ } else if (x == "QPassSignals+")
m_supports_QPassSignals = eLazyBoolYes;
else if (x == "multiprocess+")
m_supports_multiprocess = eLazyBoolYes;
@@ -1148,9 +1149,8 @@
if (SendPacketAndWaitForResponse(packet, response) != PacketResult::Success)
return;
- if (response.IsOKResponse()) {
- m_compression_type = avail_type;
- }
+ if (response.IsOKResponse())
+ m_comm.EnableCompression(avail_type);
}
}
@@ -1217,7 +1217,7 @@
if (force || m_qHostInfo_is_valid == eLazyBoolCalculate) {
// host info computation can require DNS traffic and shelling out to external processes.
// Increase the timeout to account for that.
- ScopedTimeout timeout(*this, seconds(10));
+ GDBRemoteCommunication::ScopedTimeout timeout(m_comm, seconds(10));
m_qHostInfo_is_valid = eLazyBoolNo;
StringExtractorGDBRemote response;
if (SendPacketAndWaitForResponse("qHostInfo", response) ==
@@ -1310,7 +1310,7 @@
uint32_t timeout_seconds;
if (!value.getAsInteger(0, timeout_seconds)) {
m_default_packet_timeout = seconds(timeout_seconds);
- SetPacketTimeout(m_default_packet_timeout);
+ m_comm.SetPacketTimeout(m_default_packet_timeout);
++num_keys_decoded;
}
} else if (name.equals("vm-page-size")) {
@@ -2356,7 +2356,7 @@
StringExtractorGDBRemote response;
// Increase timeout as the first qfProcessInfo packet takes a long time on
// Android. The value of 1min was arrived at empirically.
- ScopedTimeout timeout(*this, minutes(1));
+ GDBRemoteCommunication::ScopedTimeout timeout(m_comm, minutes(1));
if (SendPacketAndWaitForResponse(packet.GetString(), response) ==
PacketResult::Success) {
do {
@@ -2634,7 +2634,7 @@
}
}
// give the process a few seconds to startup
- ScopedTimeout timeout(*this, seconds(10));
+ GDBRemoteCommunication::ScopedTimeout timeout(m_comm, seconds(10));
if (SendPacketAndWaitForResponse(stream.GetString(), response) ==
PacketResult::Success) {
@@ -3901,7 +3901,8 @@
unescaped_payload.GetSize());
// Increase the timeout for jModulesInfo since this packet can take longer.
- ScopedTimeout timeout(*this, std::chrono::seconds(10));
+ GDBRemoteCommunication::ScopedTimeout timeout(m_comm,
+ std::chrono::seconds(10));
StringExtractorGDBRemote response;
if (SendPacketAndWaitForResponse(payload.GetString(), response) !=
@@ -4295,9 +4296,9 @@
llvm::Expected<int> GDBRemoteCommunicationClient::KillProcess(lldb::pid_t pid) {
StringExtractorGDBRemote response;
- GDBRemoteCommunication::ScopedTimeout(*this, seconds(3));
+ GDBRemoteCommunication::ScopedTimeout(m_comm, seconds(3));
- if (SendPacketAndWaitForResponse("k", response, GetPacketTimeout()) !=
+ if (SendPacketAndWaitForResponse("k", response, m_comm.GetPacketTimeout()) !=
PacketResult::Success)
return llvm::createStringError(llvm::inconvertibleErrorCode(),
"failed to send k packet");
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
@@ -166,10 +166,26 @@
/// Expand GDB run-length encoding.
static std::string ExpandRLE(std::string);
+ void EnableqEcho() { m_supports_qEcho = true; }
+
+ void DisableSendingAcks() { m_send_acks = false; }
+
+ void EnableCompression(CompressionType type) { m_compression_type = type; }
+
+ PacketResult ReadPacket(StringExtractorGDBRemote &response,
+ Timeout<std::micro> timeout, bool sync_on_timeout);
+
+ PacketResult ReadPacketWithOutputSupport(
+ StringExtractorGDBRemote &response, Timeout<std::micro> timeout,
+ bool sync_on_timeout,
+ llvm::function_ref<void(llvm::StringRef)> output_callback);
+
+ PacketResult SendPacketNoLock(llvm::StringRef payload);
+
protected:
std::chrono::seconds m_packet_timeout;
uint32_t m_echo_number;
- LazyBool m_supports_qEcho;
+ bool m_supports_qEcho;
GDBRemoteCommunicationHistory m_history;
bool m_send_acks;
bool m_is_platform; // Set to true if this class represents a platform,
@@ -180,16 +196,12 @@
std::recursive_mutex m_bytes_mutex;
CompressionType m_compression_type;
- PacketResult SendPacketNoLock(llvm::StringRef payload);
PacketResult SendNotificationPacketNoLock(llvm::StringRef notify_type,
std::deque<std::string>& queue,
llvm::StringRef payload);
PacketResult SendRawPacketNoLock(llvm::StringRef payload,
bool skip_ack = false);
- PacketResult ReadPacket(StringExtractorGDBRemote &response,
- Timeout<std::micro> timeout, bool sync_on_timeout);
-
PacketResult WaitForPacketNoLock(StringExtractorGDBRemote &response,
Timeout<std::micro> timeout,
bool sync_on_timeout);
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -64,7 +64,7 @@
#else
m_packet_timeout(1),
#endif
- m_echo_number(0), m_supports_qEcho(eLazyBoolCalculate), m_history(512),
+ m_echo_number(0), m_supports_qEcho(false), m_history(512),
m_send_acks(true), m_is_platform(false),
m_compression_type(CompressionType::None), m_listen_url() {
}
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
@@ -16,8 +16,10 @@
namespace lldb_private {
namespace process_gdb_remote {
-class GDBRemoteClientBase : public GDBRemoteCommunication, public Broadcaster {
+class GDBRemoteClientBase : public Broadcaster {
public:
+ typedef GDBRemoteCommunication::PacketResult PacketResult;
+
enum {
eBroadcastBitRunPacketSent = (1u << 0),
};
@@ -96,7 +98,28 @@
void SyncWithContinueThread();
};
+ // Wrappers for GDBRemoteCommunication API.
+ PacketResult ReadPacket(StringExtractorGDBRemote &response,
+ Timeout<std::micro> timeout, bool sync_on_timeout);
+
+ PacketResult SendPacketNoLock(llvm::StringRef payload);
+
+ size_t SendAck();
+
+ size_t Write(const void *src, size_t src_len, lldb::ConnectionStatus &status,
+ Status *error_ptr);
+
+ bool IsConnected() const;
+
+ lldb::ConnectionStatus Disconnect(Status *error_ptr = nullptr);
+
+ GDBRemoteCommunication &GetCommunication() {
+ return m_comm;
+ }
+
protected:
+ GDBRemoteCommunication m_comm;
+
PacketResult
SendPacketAndWaitForResponseNoLock(llvm::StringRef payload,
StringExtractorGDBRemote &response);
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
@@ -32,8 +32,8 @@
GDBRemoteClientBase::ContinueDelegate::~ContinueDelegate() = default;
GDBRemoteClientBase::GDBRemoteClientBase(const char *comm_name)
- : GDBRemoteCommunication(), Broadcaster(nullptr, comm_name),
- m_async_count(0), m_is_running(false), m_should_stop(false) {}
+ : Broadcaster(nullptr, comm_name), m_async_count(0), m_is_running(false),
+ m_should_stop(false) {}
StateType GDBRemoteClientBase::SendContinuePacketAndWaitForResponse(
ContinueDelegate &delegate, const UnixSignals &signals,
@@ -230,7 +230,7 @@
if (packet_result != PacketResult::Success)
return packet_result;
- return ReadPacketWithOutputSupport(response, GetPacketTimeout(), true,
+ return ReadPacketWithOutputSupport(response, m_comm.GetPacketTimeout(), true,
output_callback);
}
@@ -243,7 +243,7 @@
const size_t max_response_retries = 3;
for (size_t i = 0; i < max_response_retries; ++i) {
- packet_result = ReadPacket(response, GetPacketTimeout(), true);
+ packet_result = ReadPacket(response, m_comm.GetPacketTimeout(), true);
// Make sure we received a response
if (packet_result != PacketResult::Success)
return packet_result;
@@ -402,3 +402,28 @@
}
m_comm.m_cv.notify_one();
}
+
+GDBRemoteClientBase::PacketResult
+GDBRemoteClientBase::ReadPacket(StringExtractorGDBRemote &response,
+ Timeout<std::micro> timeout,
+ bool sync_on_timeout) {
+ return m_comm.ReadPacket(response, timeout, sync_on_timeout);
+}
+
+GDBRemoteClientBase::PacketResult
+GDBRemoteClientBase::SendPacketNoLock(llvm::StringRef payload) {
+ return m_comm.SendPacketNoLock(payload);
+}
+
+size_t GDBRemoteClientBase::SendAck() { return m_comm.SendAck(); }
+
+size_t GDBRemoteClientBase::Write(const void *src, size_t src_len,
+ ConnectionStatus &status, Status *error_ptr) {
+ return m_comm.Write(src, src_len, status, error_ptr);
+}
+
+bool GDBRemoteClientBase::IsConnected() const { return m_comm.IsConnected(); }
+
+ConnectionStatus GDBRemoteClientBase::Disconnect(Status *error_ptr) {
+ return m_comm.Disconnect(error_ptr);
+}
Index: lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
===================================================================
--- lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
+++ lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
@@ -235,10 +235,11 @@
auto client_up =
std::make_unique<process_gdb_remote::GDBRemoteCommunicationClient>();
- client_up->SetPacketTimeout(
+ client_up->GetCommunication().SetPacketTimeout(
process_gdb_remote::ProcessGDBRemote::GetPacketTimeout());
- client_up->SetConnection(std::make_unique<ConnectionFileDescriptor>());
- client_up->Connect(url, &error);
+ client_up->GetCommunication().SetConnection(
+ std::make_unique<ConnectionFileDescriptor>());
+ client_up->GetCommunication().Connect(url, &error);
if (error.Fail())
return error;
@@ -365,7 +366,7 @@
{
// Scope for the scoped timeout object
process_gdb_remote::GDBRemoteCommunication::ScopedTimeout timeout(
- *m_gdb_client_up, std::chrono::seconds(5));
+ m_gdb_client_up->GetCommunication(), std::chrono::seconds(5));
arg_packet_err = m_gdb_client_up->SendArgumentsPacket(launch_info);
}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
