mgorny created this revision.
mgorny added reviewers: labath, emaste, krytarowski.
mgorny requested review of this revision.
Refactor SetCurrentThread() and SetCurrentThreadForRun() to reduce code
duplication and simplify it. Both methods now call common
SendSetCurrentThreadPacket() that implements the common protocol
exchange part (the only variable is sending `Hg` vs `Hc`) and returns
the selected TID. The logic is rewritten to use a StreamString
instead of snprintf().
A side effect of the change is that thread-id sent is now zero-padded.
However, this should not have practical impact on the server as both
forms are equivalent.
https://reviews.llvm.org/D100459
Files:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
Index: lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
===================================================================
--- lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -99,7 +99,7 @@
});
Handle_QThreadSuffixSupported(server, false);
- HandlePacket(server, "Hg47", "OK");
+ HandlePacket(server, "Hg0000000000000047", "OK");
HandlePacket(server, "P4=" + one_register_hex, "OK");
ASSERT_TRUE(write_result.get());
@@ -144,7 +144,7 @@
return client.SaveRegisterState(tid, save_id);
});
Handle_QThreadSuffixSupported(server, false);
- HandlePacket(server, "Hg47", "OK");
+ HandlePacket(server, "Hg0000000000000047", "OK");
HandlePacket(server, "QSaveRegisterState", "1");
ASSERT_TRUE(async_result.get());
EXPECT_EQ(1u, save_id);
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
@@ -334,6 +334,8 @@
// and response times.
bool SendSpeedTestPacket(uint32_t send_size, uint32_t recv_size);
+ llvm::Optional<uint64_t> SendSetCurrentThreadPacket(uint64_t tid, char type);
+
bool SetCurrentThread(uint64_t tid);
bool SetCurrentThreadForRun(uint64_t tid);
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
@@ -2597,25 +2597,21 @@
return false;
}
-bool GDBRemoteCommunicationClient::SetCurrentThread(uint64_t tid) {
- if (m_curr_tid == tid)
- return true;
-
- char packet[32];
- int packet_len;
+llvm::Optional<uint64_t>
+GDBRemoteCommunicationClient::SendSetCurrentThreadPacket(uint64_t tid,
+ char type) {
+ lldb_private::StreamString packet;
+ packet.PutChar('H');
+ packet.PutChar(type);
if (tid == UINT64_MAX)
- packet_len = ::snprintf(packet, sizeof(packet), "Hg-1");
+ packet.PutCString("-1");
else
- packet_len = ::snprintf(packet, sizeof(packet), "Hg%" PRIx64, tid);
- assert(packet_len + 1 < (int)sizeof(packet));
- UNUSED_IF_ASSERT_DISABLED(packet_len);
+ packet.PutHex64(tid);
StringExtractorGDBRemote response;
- if (SendPacketAndWaitForResponse(packet, response, false) ==
+ if (SendPacketAndWaitForResponse(packet.GetString(), response, false) ==
PacketResult::Success) {
- if (response.IsOKResponse()) {
- m_curr_tid = tid;
- return true;
- }
+ if (response.IsOKResponse())
+ return tid;
/*
* Connected bare-iron target (like YAMON gdb-stub) may not have support for
@@ -2623,49 +2619,31 @@
* The reply from '?' packet could be as simple as 'S05'. There is no packet
* which can
* give us pid and/or tid. Assume pid=tid=1 in such cases.
- */
- if (response.IsUnsupportedResponse() && IsConnected()) {
- m_curr_tid = 1;
- return true;
- }
+ */
+ if (response.IsUnsupportedResponse() && IsConnected())
+ return 1;
}
- return false;
+ return llvm::None;
}
-bool GDBRemoteCommunicationClient::SetCurrentThreadForRun(uint64_t tid) {
- if (m_curr_tid_run == tid)
+bool GDBRemoteCommunicationClient::SetCurrentThread(uint64_t tid) {
+ if (m_curr_tid == tid)
return true;
- char packet[32];
- int packet_len;
- if (tid == UINT64_MAX)
- packet_len = ::snprintf(packet, sizeof(packet), "Hc-1");
- else
- packet_len = ::snprintf(packet, sizeof(packet), "Hc%" PRIx64, tid);
+ llvm::Optional<uint64_t> ret = SendSetCurrentThreadPacket(tid, 'g');
+ if (ret.hasValue())
+ m_curr_tid = ret.getValue();
+ return ret.hasValue();
+}
- assert(packet_len + 1 < (int)sizeof(packet));
- UNUSED_IF_ASSERT_DISABLED(packet_len);
- StringExtractorGDBRemote response;
- if (SendPacketAndWaitForResponse(packet, response, false) ==
- PacketResult::Success) {
- if (response.IsOKResponse()) {
- m_curr_tid_run = tid;
- return true;
- }
+bool GDBRemoteCommunicationClient::SetCurrentThreadForRun(uint64_t tid) {
+ if (m_curr_tid_run == tid)
+ return true;
- /*
- * Connected bare-iron target (like YAMON gdb-stub) may not have support for
- * Hc packet.
- * The reply from '?' packet could be as simple as 'S05'. There is no packet
- * which can
- * give us pid and/or tid. Assume pid=tid=1 in such cases.
- */
- if (response.IsUnsupportedResponse() && IsConnected()) {
- m_curr_tid_run = 1;
- return true;
- }
- }
- return false;
+ llvm::Optional<uint64_t> ret = SendSetCurrentThreadPacket(tid, 'c');
+ if (ret.hasValue())
+ m_curr_tid_run = ret.getValue();
+ return ret.hasValue();
}
bool GDBRemoteCommunicationClient::GetStopReply(
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
