This revision was automatically updated to reflect the committed changes.
Closed by commit rL279040: gdb-remote: Centralize thread specific packet
handling (authored by labath).
Changed prior to commit:
https://reviews.llvm.org/D23604?vs=68337&id=68495#toc
Repository:
rL LLVM
https://reviews.llvm.org/D23604
Files:
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
lldb/trunk/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
Index: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
===================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -633,6 +633,10 @@
void
OnRunPacketSent(bool first) override;
+ PacketResult
+ SendThreadSpecificPacketAndWaitForResponse(lldb::tid_t tid, StreamString &&payload,
+ StringExtractorGDBRemote &response, bool send_async);
+
private:
DISALLOW_COPY_AND_ASSIGN (GDBRemoteCommunicationClient);
};
Index: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -574,6 +574,31 @@
return false;
}
+GDBRemoteCommunication::PacketResult
+GDBRemoteCommunicationClient::SendThreadSpecificPacketAndWaitForResponse(lldb::tid_t tid, StreamString &&payload,
+ StringExtractorGDBRemote &response,
+ bool send_async)
+{
+ Lock lock(*this, send_async);
+ if (!lock)
+ {
+ if (Log *log = ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet(GDBR_LOG_PROCESS | GDBR_LOG_PACKETS))
+ log->Printf("GDBRemoteCommunicationClient::%s: Didn't get sequence mutex for %s packet.", __FUNCTION__,
+ payload.GetString().c_str());
+ return PacketResult::ErrorNoSequenceLock;
+ }
+
+ if (GetThreadSuffixSupported())
+ payload.Printf(";thread:%4.4" PRIx64 ";", tid);
+ else
+ {
+ if (!SetCurrentThread(tid))
+ return PacketResult::ErrorSendFailed;
+ }
+
+ return SendPacketAndWaitForResponseNoLock(payload.GetString(), response);
+}
+
// Check if the target supports 'p' packet. It sends out a 'p'
// packet and checks the response. A normal packet will tell us
// that support is available.
@@ -584,18 +609,15 @@
{
if (m_supports_p == eLazyBoolCalculate)
{
- StringExtractorGDBRemote response;
m_supports_p = eLazyBoolNo;
- char packet[256];
- if (GetThreadSuffixSupported())
- snprintf(packet, sizeof(packet), "p0;thread:%" PRIx64 ";", tid);
- else
- snprintf(packet, sizeof(packet), "p0");
-
- if (SendPacketAndWaitForResponse(packet, response, false) == PacketResult::Success)
+ StreamString payload;
+ payload.PutCString("p0");
+ StringExtractorGDBRemote response;
+ if (SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), response, false) ==
+ PacketResult::Success &&
+ response.IsNormalResponse())
{
- if (response.IsNormalResponse())
- m_supports_p = eLazyBoolYes;
+ m_supports_p = eLazyBoolYes;
}
}
return m_supports_p;
@@ -3473,108 +3495,42 @@
bool
GDBRemoteCommunicationClient::ReadRegister(lldb::tid_t tid, uint32_t reg, StringExtractorGDBRemote &response)
{
- Lock lock(*this, false);
- if (lock)
- {
- const bool thread_suffix_supported = GetThreadSuffixSupported();
-
- if (thread_suffix_supported || SetCurrentThread(tid))
- {
- char packet[64];
- int packet_len = 0;
- if (thread_suffix_supported)
- packet_len = ::snprintf (packet, sizeof(packet), "p%x;thread:%4.4" PRIx64 ";", reg, tid);
- else
- packet_len = ::snprintf (packet, sizeof(packet), "p%x", reg);
- assert (packet_len < ((int)sizeof(packet) - 1));
- return SendPacketAndWaitForResponse(packet, response, false) == PacketResult::Success;
- }
- }
- else if (Log *log = ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet(GDBR_LOG_PROCESS | GDBR_LOG_PACKETS))
- {
- log->Printf("GDBRemoteCommunicationClient::%s: Didn't get sequence mutex for p packet.", __FUNCTION__);
- }
- return false;
-
+ StreamString payload;
+ payload.Printf("p%x", reg);
+ return SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), response, false) ==
+ PacketResult::Success;
}
bool
GDBRemoteCommunicationClient::ReadAllRegisters (lldb::tid_t tid, StringExtractorGDBRemote &response)
{
- Lock lock(*this, false);
- if (lock)
- {
- const bool thread_suffix_supported = GetThreadSuffixSupported();
-
- if (thread_suffix_supported || SetCurrentThread(tid))
- {
- char packet[64];
- int packet_len = 0;
- // Get all registers in one packet
- if (thread_suffix_supported)
- packet_len = ::snprintf (packet, sizeof(packet), "g;thread:%4.4" PRIx64 ";", tid);
- else
- packet_len = ::snprintf (packet, sizeof(packet), "g");
- assert (packet_len < ((int)sizeof(packet) - 1));
- return SendPacketAndWaitForResponse(packet, response, false) == PacketResult::Success;
- }
- }
- else if (Log *log = ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet(GDBR_LOG_PROCESS | GDBR_LOG_PACKETS))
- {
- log->Printf("GDBRemoteCommunicationClient::%s: Didn't get sequence mutex for g packet.", __FUNCTION__);
- }
- return false;
+ StreamString payload;
+ payload.PutChar('g');
+ return SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), response, false) ==
+ PacketResult::Success;
}
bool
GDBRemoteCommunicationClient::WriteRegister(lldb::tid_t tid, uint32_t reg_num, llvm::StringRef data)
{
- Lock lock(*this, false);
- if (!lock)
- {
- if (Log *log = ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet(GDBR_LOG_PROCESS | GDBR_LOG_PACKETS))
- log->Printf("GDBRemoteCommunicationClient::%s: Didn't get sequence mutex for P packet.", __FUNCTION__);
- return false;
- }
-
- const bool thread_suffix_supported = GetThreadSuffixSupported();
- if (!thread_suffix_supported && !SetCurrentThread(tid))
- return false;
-
- StreamString packet;
- packet.Printf("P%x=", reg_num);
- packet.PutBytesAsRawHex8(data.data(), data.size(), endian::InlHostByteOrder(), endian::InlHostByteOrder());
- if (thread_suffix_supported)
- packet.Printf(";thread:%4.4" PRIx64 ";", tid);
+ StreamString payload;
+ payload.Printf("P%x=", reg_num);
+ payload.PutBytesAsRawHex8(data.data(), data.size(), endian::InlHostByteOrder(), endian::InlHostByteOrder());
StringExtractorGDBRemote response;
- return SendPacketAndWaitForResponse(packet.GetData(), packet.GetSize(), response, false) == PacketResult::Success &&
- response.IsOKResponse();
+ return SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), response, false) ==
+ PacketResult::Success;
}
bool
GDBRemoteCommunicationClient::WriteAllRegisters(lldb::tid_t tid, llvm::StringRef data)
{
- Lock lock(*this, false);
- if (!lock)
- {
- if (Log *log = ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet(GDBR_LOG_PROCESS | GDBR_LOG_PACKETS))
- log->Printf("GDBRemoteCommunicationClient::%s: Didn't get sequence mutex for G packet.", __FUNCTION__);
- return false;
- }
-
- const bool thread_suffix_supported = GetThreadSuffixSupported();
- if (!thread_suffix_supported && !SetCurrentThread(tid))
- return false;
-
- StreamString packet;
- packet.PutChar('G');
- packet.Write(data.data(), data.size());
- if (thread_suffix_supported)
- packet.Printf(";thread:%4.4" PRIx64 ";", tid);
+ StreamString payload;
+ payload.PutChar('G');
+ payload.Write(data.data(), data.size());
StringExtractorGDBRemote response;
- return SendPacketAndWaitForResponse(packet.GetData(), packet.GetSize(), response, false) == PacketResult::Success &&
- response.IsOKResponse();
+ return SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), response, false) ==
+ PacketResult::Success;
}
bool
@@ -3585,43 +3541,21 @@
return false;
m_supports_QSaveRegisterState = eLazyBoolYes;
- Lock lock(*this, false);
- if (lock)
- {
- const bool thread_suffix_supported = GetThreadSuffixSupported();
- if (thread_suffix_supported || SetCurrentThread(tid))
- {
- char packet[256];
- if (thread_suffix_supported)
- ::snprintf (packet, sizeof(packet), "QSaveRegisterState;thread:%4.4" PRIx64 ";", tid);
- else
- ::snprintf(packet, sizeof(packet), "QSaveRegisterState");
-
- StringExtractorGDBRemote response;
+ StreamString payload;
+ payload.PutCString("QSaveRegisterState");
+ StringExtractorGDBRemote response;
+ if (SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), response, false) != PacketResult::Success)
+ return false;
- if (SendPacketAndWaitForResponse(packet, response, false) == PacketResult::Success)
- {
- if (response.IsUnsupportedResponse())
- {
- // This packet isn't supported, don't try calling it again
- m_supports_QSaveRegisterState = eLazyBoolNo;
- }
-
- const uint32_t response_save_id = response.GetU32(0);
- if (response_save_id != 0)
- {
- save_id = response_save_id;
- return true;
- }
- }
- }
- }
- else if (Log *log = ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet(GDBR_LOG_PROCESS | GDBR_LOG_PACKETS))
- {
- log->Printf("GDBRemoteCommunicationClient::%s: Didn't get sequence mutex for QSaveRegisterState packet.",
- __FUNCTION__);
- }
- return false;
+ if (response.IsUnsupportedResponse())
+ m_supports_QSaveRegisterState = eLazyBoolNo;
+
+ const uint32_t response_save_id = response.GetU32(0);
+ if (response_save_id == 0)
+ return false;
+
+ save_id = response_save_id;
+ return true;
}
bool
@@ -3633,40 +3567,17 @@
if (m_supports_QSaveRegisterState == eLazyBoolNo)
return false;
- Lock lock(*this, false);
- if (lock)
- {
- const bool thread_suffix_supported = GetThreadSuffixSupported();
- if (thread_suffix_supported || SetCurrentThread(tid))
- {
- char packet[256];
- if (thread_suffix_supported)
- ::snprintf (packet, sizeof(packet), "QRestoreRegisterState:%u;thread:%4.4" PRIx64 ";", save_id, tid);
- else
- ::snprintf (packet, sizeof(packet), "QRestoreRegisterState:%u" PRIx64 ";", save_id);
-
- StringExtractorGDBRemote response;
-
- if (SendPacketAndWaitForResponse(packet, response, false) == PacketResult::Success)
- {
- if (response.IsOKResponse())
- {
- return true;
- }
- else if (response.IsUnsupportedResponse())
- {
- // This packet isn't supported, don't try calling this packet or
- // QSaveRegisterState again...
- m_supports_QSaveRegisterState = eLazyBoolNo;
- }
- }
- }
- }
- else if (Log *log = ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet(GDBR_LOG_PROCESS | GDBR_LOG_PACKETS))
- {
- log->Printf("GDBRemoteCommunicationClient::%s: Didn't get sequence mutex for QRestoreRegisterState packet.",
- __FUNCTION__);
- }
+ StreamString payload;
+ payload.Printf("QRestoreRegisterState:%u", save_id);
+ StringExtractorGDBRemote response;
+ if (SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), response, false) != PacketResult::Success)
+ return false;
+
+ if (response.IsOKResponse())
+ return true;
+
+ if (response.IsUnsupportedResponse())
+ m_supports_QSaveRegisterState = eLazyBoolNo;
return false;
}
Index: lldb/trunk/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
===================================================================
--- lldb/trunk/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ lldb/trunk/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -54,6 +54,8 @@
ASSERT_EQ(PacketResult::Success, server.SendPacket(response));
}
+const char all_registers[] = "404142434445464748494a4b4c4d4e4f";
+
} // end anonymous namespace
class GDBRemoteCommunicationClientTest : public GDBRemoteTest
@@ -78,10 +80,9 @@
HandlePacket(server, "P4=41424344;thread:0047;", "OK");
ASSERT_TRUE(write_result.get());
- write_result = std::async(std::launch::async,
- [&] { return client.WriteAllRegisters(tid, "404142434445464748494a4b4c4d4e4f"); });
+ write_result = std::async(std::launch::async, [&] { return client.WriteAllRegisters(tid, all_registers); });
- HandlePacket(server, "G404142434445464748494a4b4c4d4e4f;thread:0047;", "OK");
+ HandlePacket(server, std::string("G") + all_registers + ";thread:0047;", "OK");
ASSERT_TRUE(write_result.get());
}
@@ -103,9 +104,58 @@
HandlePacket(server, "P4=41424344", "OK");
ASSERT_TRUE(write_result.get());
- write_result = std::async(std::launch::async,
- [&] { return client.WriteAllRegisters(tid, "404142434445464748494a4b4c4d4e4f"); });
+ write_result = std::async(std::launch::async, [&] { return client.WriteAllRegisters(tid, all_registers); });
- HandlePacket(server, "G404142434445464748494a4b4c4d4e4f", "OK");
+ HandlePacket(server, std::string("G") + all_registers, "OK");
ASSERT_TRUE(write_result.get());
}
+
+TEST_F(GDBRemoteCommunicationClientTest, ReadRegister)
+{
+ TestClient client;
+ MockServer server;
+ Connect(client, server);
+ if (HasFailure())
+ return;
+
+ const lldb::tid_t tid = 0x47;
+ const uint32_t reg_num = 4;
+ std::future<bool> async_result = std::async(std::launch::async, [&] { return client.GetpPacketSupported(tid); });
+ Handle_QThreadSuffixSupported(server, true);
+ HandlePacket(server, "p0;thread:0047;", "41424344");
+ ASSERT_TRUE(async_result.get());
+
+ StringExtractorGDBRemote response;
+ async_result = std::async(std::launch::async, [&] { return client.ReadRegister(tid, reg_num, response); });
+ HandlePacket(server, "p4;thread:0047;", "41424344");
+ ASSERT_TRUE(async_result.get());
+ ASSERT_EQ("41424344", response.GetStringRef());
+
+ async_result = std::async(std::launch::async, [&] { return client.ReadAllRegisters(tid, response); });
+ HandlePacket(server, "g;thread:0047;", all_registers);
+ ASSERT_TRUE(async_result.get());
+ ASSERT_EQ(all_registers, response.GetStringRef());
+}
+
+TEST_F(GDBRemoteCommunicationClientTest, SaveRestoreRegistersNoSuffix)
+{
+ TestClient client;
+ MockServer server;
+ Connect(client, server);
+ if (HasFailure())
+ return;
+
+ const lldb::tid_t tid = 0x47;
+ uint32_t save_id;
+ std::future<bool> async_result =
+ std::async(std::launch::async, [&] { return client.SaveRegisterState(tid, save_id); });
+ Handle_QThreadSuffixSupported(server, false);
+ HandlePacket(server, "Hg47", "OK");
+ HandlePacket(server, "QSaveRegisterState", "1");
+ ASSERT_TRUE(async_result.get());
+ EXPECT_EQ(1u, save_id);
+
+ async_result = std::async(std::launch::async, [&] { return client.RestoreRegisterState(tid, save_id); });
+ HandlePacket(server, "QRestoreRegisterState:1", "OK");
+ ASSERT_TRUE(async_result.get());
+}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
