Author: Michał Górny Date: 2021-07-02T14:36:17+02:00 New Revision: 02ef0f5ab483875b7b6b38e24b245e4fd4053959
URL: https://github.com/llvm/llvm-project/commit/02ef0f5ab483875b7b6b38e24b245e4fd4053959 DIFF: https://github.com/llvm/llvm-project/commit/02ef0f5ab483875b7b6b38e24b245e4fd4053959.diff LOG: [lldb] [gdb-remote client] Refactor SetCurrentThread*() 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. Differential Revision: https://reviews.llvm.org/D100459 Added: Modified: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp Removed: ################################################################################ diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp index 0e529d221495e..ec320543a7828 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp @@ -2639,25 +2639,21 @@ bool GDBRemoteCommunicationClient::KillSpawnedProcess(lldb::pid_t pid) { 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 op) { + lldb_private::StreamString packet; + packet.PutChar('H'); + packet.PutChar(op); 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 @@ -2665,49 +2661,31 @@ bool GDBRemoteCommunicationClient::SetCurrentThread(uint64_t tid) { * 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( diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h index fa67a6c69a535..03704dfdd8cf0 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h @@ -336,6 +336,8 @@ class GDBRemoteCommunicationClient : public GDBRemoteClientBase { // and response times. bool SendSpeedTestPacket(uint32_t send_size, uint32_t recv_size); + llvm::Optional<uint64_t> SendSetCurrentThreadPacket(uint64_t tid, char op); + bool SetCurrentThread(uint64_t tid); bool SetCurrentThreadForRun(uint64_t tid); diff --git a/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp b/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp index b9fc107527a21..45e0356c49486 100644 --- a/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp +++ b/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp @@ -98,7 +98,7 @@ TEST_F(GDBRemoteCommunicationClientTest, WriteRegisterNoSuffix) { }); Handle_QThreadSuffixSupported(server, false); - HandlePacket(server, "Hg47", "OK"); + HandlePacket(server, "Hg0000000000000047", "OK"); HandlePacket(server, "P4=" + one_register_hex, "OK"); ASSERT_TRUE(write_result.get()); @@ -143,7 +143,7 @@ TEST_F(GDBRemoteCommunicationClientTest, SaveRestoreRegistersNoSuffix) { 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); _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
