From 5c95ee4dd87f34b4564e6eada2e8e4b70834db68 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Tue, 30 Aug 2016 13:56:11 +0000 Subject: [PATCH] Revert "gdb-remote: Make the sequence mutex non-recursive" This reverts commit r279725 as it breaks "dynamic register size" feature of mips. llvm-svn: 280088 --- .../gdb-remote/GDBRemoteClientBase.cpp | 6 +- .../Process/gdb-remote/GDBRemoteClientBase.h | 8 +- .../GDBRemoteCommunicationClient.cpp | 109 +++++++----------- .../gdb-remote/GDBRemoteCommunicationClient.h | 20 ++-- .../gdb-remote/GDBRemoteRegisterContext.cpp | 66 +++++------ .../gdb-remote/GDBRemoteRegisterContext.h | 14 +-- .../Process/gdb-remote/ProcessGDBRemote.cpp | 2 +- .../GDBRemoteCommunicationClientTest.cpp | 42 ++----- 8 files changed, 102 insertions(+), 165 deletions(-) diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp index 4164ea29ddbd..f99352a0a3a0 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp @@ -208,14 +208,12 @@ GDBRemoteClientBase::SendPacketAndWaitForResponse(llvm::StringRef payload, Strin return PacketResult::ErrorSendFailed; } - return SendPacketAndWaitForResponse(payload, response, lock); + return SendPacketAndWaitForResponseNoLock(payload, response); } GDBRemoteCommunication::PacketResult -GDBRemoteClientBase::SendPacketAndWaitForResponse(llvm::StringRef payload, StringExtractorGDBRemote &response, - const Lock &lock) +GDBRemoteClientBase::SendPacketAndWaitForResponseNoLock(llvm::StringRef payload, StringExtractorGDBRemote &response) { - assert(lock); PacketResult packet_result = SendPacketNoLock(payload); if (packet_result != PacketResult::Success) return packet_result; diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h index a3bcc865ac66..684ef3e80d16 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h @@ -73,7 +73,7 @@ public: Lock(GDBRemoteClientBase &comm, bool interrupt); ~Lock(); - explicit operator bool() const { return m_acquired; } + explicit operator bool() { return m_acquired; } // Whether we had to interrupt the continue thread to acquire the connection. bool @@ -83,7 +83,7 @@ public: } private: - std::unique_lock m_async_lock; + std::unique_lock m_async_lock; GDBRemoteClientBase &m_comm; bool m_acquired; bool m_did_interrupt; @@ -94,7 +94,7 @@ public: protected: PacketResult - SendPacketAndWaitForResponse(llvm::StringRef payload, StringExtractorGDBRemote &response, const Lock &lock); + SendPacketAndWaitForResponseNoLock(llvm::StringRef payload, StringExtractorGDBRemote &response); virtual void OnRunPacketSent(bool first); @@ -126,7 +126,7 @@ private: // This handles the synchronization between individual async threads. For now they just use a // simple mutex. - std::mutex m_async_mutex; + std::recursive_mutex m_async_mutex; bool ShouldStop(const UnixSignals &signals, StringExtractorGDBRemote &response); diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp index 8cfe331ee16f..d08b939aa950 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp @@ -518,27 +518,21 @@ GDBRemoteCommunicationClient::GetRemoteQSupported () } } -void -GDBRemoteCommunicationClient::ComputeThreadSuffixSupport() -{ - if (m_supports_thread_suffix != eLazyBoolCalculate) - return; - - StringExtractorGDBRemote response; - m_supports_thread_suffix = eLazyBoolNo; - if (SendPacketAndWaitForResponse("QThreadSuffixSupported", response, false) == PacketResult::Success) - { - if (response.IsOKResponse()) - m_supports_thread_suffix = eLazyBoolYes; - } -} - bool -GDBRemoteCommunicationClient::GetThreadSuffixSupported() +GDBRemoteCommunicationClient::GetThreadSuffixSupported () { - return m_supports_thread_suffix == eLazyBoolYes; + if (m_supports_thread_suffix == eLazyBoolCalculate) + { + StringExtractorGDBRemote response; + m_supports_thread_suffix = eLazyBoolNo; + if (SendPacketAndWaitForResponse("QThreadSuffixSupported", response, false) == PacketResult::Success) + { + if (response.IsOKResponse()) + m_supports_thread_suffix = eLazyBoolYes; + } + } + return m_supports_thread_suffix; } - bool GDBRemoteCommunicationClient::GetVContSupported (char flavor) { @@ -600,17 +594,26 @@ GDBRemoteCommunicationClient::GetVContSupported (char flavor) GDBRemoteCommunication::PacketResult GDBRemoteCommunicationClient::SendThreadSpecificPacketAndWaitForResponse(lldb::tid_t tid, StreamString &&payload, StringExtractorGDBRemote &response, - const Lock &lock) + 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, lock)) + if (!SetCurrentThread(tid)) return PacketResult::ErrorSendFailed; } - return SendPacketAndWaitForResponse(payload.GetString(), response, lock); + return SendPacketAndWaitForResponseNoLock(payload.GetString(), response); } // Check if the target supports 'p' packet. It sends out a 'p' @@ -623,20 +626,11 @@ GDBRemoteCommunicationClient::GetpPacketSupported (lldb::tid_t tid) { if (m_supports_p == eLazyBoolCalculate) { - Lock lock(*this, false); - if (!lock) - { - Log *log(ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet(GDBR_LOG_PROCESS | GDBR_LOG_PACKETS)); - if (log) - log->Printf("GDBRemoteCommunicationClient::%s failed to get sequence mutex", __FUNCTION__); - return false; - } - m_supports_p = eLazyBoolNo; StreamString payload; payload.PutCString("p0"); StringExtractorGDBRemote response; - if (SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), response, lock) == + if (SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), response, false) == PacketResult::Success && response.IsNormalResponse()) { @@ -774,7 +768,7 @@ GDBRemoteCommunicationClient::SendPacketsAndConcatenateResponses // Construct payload char sizeDescriptor[128]; snprintf(sizeDescriptor, sizeof(sizeDescriptor), "%x,%x", offset, response_size); - PacketResult result = SendPacketAndWaitForResponse(payload_prefix_str + sizeDescriptor, this_response, lock); + PacketResult result = SendPacketAndWaitForResponseNoLock(payload_prefix_str + sizeDescriptor, this_response); if (result != PacketResult::Success) return result; @@ -2769,7 +2763,7 @@ GDBRemoteCommunicationClient::KillSpawnedProcess (lldb::pid_t pid) } bool -GDBRemoteCommunicationClient::SetCurrentThread(uint64_t tid, const Lock &lock) +GDBRemoteCommunicationClient::SetCurrentThread (uint64_t tid) { if (m_curr_tid == tid) return true; @@ -2782,7 +2776,7 @@ GDBRemoteCommunicationClient::SetCurrentThread(uint64_t tid, const Lock &lock) packet_len = ::snprintf (packet, sizeof(packet), "Hg%" PRIx64, tid); assert (packet_len + 1 < (int)sizeof(packet)); StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse(llvm::StringRef(packet, packet_len), response, lock) == PacketResult::Success) + if (SendPacketAndWaitForResponse(packet, packet_len, response, false) == PacketResult::Success) { if (response.IsOKResponse()) { @@ -2943,9 +2937,9 @@ GDBRemoteCommunicationClient::GetCurrentThreadIDs (std::vector &thr StringExtractorGDBRemote response; PacketResult packet_result; - for (packet_result = SendPacketAndWaitForResponse("qfThreadInfo", response, lock); + for (packet_result = SendPacketAndWaitForResponseNoLock("qfThreadInfo", response); packet_result == PacketResult::Success && response.IsNormalResponse(); - packet_result = SendPacketAndWaitForResponse("qsThreadInfo", response, lock)) + packet_result = SendPacketAndWaitForResponseNoLock("qsThreadInfo", response)) { char ch = response.GetChar(); if (ch == 'l') @@ -3476,12 +3470,12 @@ GDBRemoteCommunicationClient::AvoidGPackets (ProcessGDBRemote *process) } DataBufferSP -GDBRemoteCommunicationClient::ReadRegister(lldb::tid_t tid, uint32_t reg, const Lock &lock) +GDBRemoteCommunicationClient::ReadRegister(lldb::tid_t tid, uint32_t reg) { StreamString payload; payload.Printf("p%x", reg); StringExtractorGDBRemote response; - if (SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), response, lock) != PacketResult::Success || + if (SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), response, false) != PacketResult::Success || !response.IsNormalResponse()) return nullptr; @@ -3491,12 +3485,12 @@ GDBRemoteCommunicationClient::ReadRegister(lldb::tid_t tid, uint32_t reg, const } DataBufferSP -GDBRemoteCommunicationClient::ReadAllRegisters(lldb::tid_t tid, const Lock &lock) +GDBRemoteCommunicationClient::ReadAllRegisters(lldb::tid_t tid) { StreamString payload; payload.PutChar('g'); StringExtractorGDBRemote response; - if (SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), response, lock) != PacketResult::Success || + if (SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), response, false) != PacketResult::Success || !response.IsNormalResponse()) return nullptr; @@ -3506,26 +3500,25 @@ GDBRemoteCommunicationClient::ReadAllRegisters(lldb::tid_t tid, const Lock &lock } bool -GDBRemoteCommunicationClient::WriteRegister(lldb::tid_t tid, uint32_t reg_num, llvm::ArrayRef data, - const Lock &lock) +GDBRemoteCommunicationClient::WriteRegister(lldb::tid_t tid, uint32_t reg_num, llvm::ArrayRef data) { StreamString payload; payload.Printf("P%x=", reg_num); payload.PutBytesAsRawHex8(data.data(), data.size(), endian::InlHostByteOrder(), endian::InlHostByteOrder()); StringExtractorGDBRemote response; - return SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), response, lock) == + return SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), response, false) == PacketResult::Success && response.IsOKResponse(); } bool -GDBRemoteCommunicationClient::WriteAllRegisters(lldb::tid_t tid, llvm::ArrayRef data, const Lock &lock) +GDBRemoteCommunicationClient::WriteAllRegisters(lldb::tid_t tid, llvm::ArrayRef data) { StreamString payload; payload.PutChar('G'); payload.PutBytesAsRawHex8(data.data(), data.size(), endian::InlHostByteOrder(), endian::InlHostByteOrder()); StringExtractorGDBRemote response; - return SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), response, lock) == + return SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), response, false) == PacketResult::Success && response.IsOKResponse(); } @@ -3536,21 +3529,12 @@ GDBRemoteCommunicationClient::SaveRegisterState (lldb::tid_t tid, uint32_t &save save_id = 0; // Set to invalid save ID if (m_supports_QSaveRegisterState == eLazyBoolNo) return false; - - Lock lock(*this, false); - if (!lock) - { - Log *log(ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet(GDBR_LOG_PROCESS | GDBR_LOG_PACKETS)); - if (log) - log->Printf("GDBRemoteCommunicationClient::%s failed to get sequence mutex", __FUNCTION__); - return false; - } - + m_supports_QSaveRegisterState = eLazyBoolYes; StreamString payload; payload.PutCString("QSaveRegisterState"); StringExtractorGDBRemote response; - if (SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), response, lock) != PacketResult::Success) + if (SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), response, false) != PacketResult::Success) return false; if (response.IsUnsupportedResponse()) @@ -3573,19 +3557,10 @@ GDBRemoteCommunicationClient::RestoreRegisterState (lldb::tid_t tid, uint32_t sa if (m_supports_QSaveRegisterState == eLazyBoolNo) return false; - Lock lock(*this, false); - if (!lock) - { - Log *log(ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet(GDBR_LOG_PROCESS | GDBR_LOG_PACKETS)); - if (log) - log->Printf("GDBRemoteCommunicationClient::%s failed to get sequence mutex", __FUNCTION__); - return false; - } - StreamString payload; payload.Printf("QRestoreRegisterState:%u", save_id); StringExtractorGDBRemote response; - if (SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), response, lock) != PacketResult::Success) + if (SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), response, false) != PacketResult::Success) return false; if (response.IsOKResponse()) @@ -3812,7 +3787,7 @@ GDBRemoteCommunicationClient::ServeSymbolLookups(lldb_private::Process *process) StreamString packet; packet.PutCString ("qSymbol::"); StringExtractorGDBRemote response; - while (SendPacketAndWaitForResponse(packet.GetString(), response, lock) == PacketResult::Success) + while (SendPacketAndWaitForResponseNoLock(packet.GetString(), response) == PacketResult::Success) { if (response.IsOKResponse()) { diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h index a714b403babb..eeeecb53992c 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h @@ -64,9 +64,6 @@ public: SendPacketsAndConcatenateResponses (const char *send_payload_prefix, std::string &response_string); - void - ComputeThreadSuffixSupport(); - bool GetThreadSuffixSupported(); @@ -397,6 +394,9 @@ public: SendSpeedTestPacket (uint32_t send_size, uint32_t recv_size); + bool + SetCurrentThread (uint64_t tid); + bool SetCurrentThreadForRun (uint64_t tid); @@ -488,18 +488,17 @@ public: lldb::DataBufferSP ReadRegister(lldb::tid_t tid, - uint32_t reg_num, // Must be the eRegisterKindProcessPlugin register number - const Lock &lock); + uint32_t reg_num); // Must be the eRegisterKindProcessPlugin register number lldb::DataBufferSP - ReadAllRegisters(lldb::tid_t tid, const Lock &lock); + ReadAllRegisters(lldb::tid_t tid); bool WriteRegister(lldb::tid_t tid, uint32_t reg_num, // eRegisterKindProcessPlugin register number - llvm::ArrayRef data, const Lock &lock); + llvm::ArrayRef data); bool - WriteAllRegisters(lldb::tid_t tid, llvm::ArrayRef data, const Lock &lock); + WriteAllRegisters(lldb::tid_t tid, llvm::ArrayRef data); bool SaveRegisterState(lldb::tid_t tid, uint32_t &save_id); @@ -687,10 +686,7 @@ protected: PacketResult SendThreadSpecificPacketAndWaitForResponse(lldb::tid_t tid, StreamString &&payload, - StringExtractorGDBRemote &response, const Lock &lock); - - bool - SetCurrentThread(uint64_t tid, const Lock &lock); + StringExtractorGDBRemote &response, bool send_async); private: DISALLOW_COPY_AND_ASSIGN (GDBRemoteCommunicationClient); diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp index 0f86bf57e4ac..78e1956c26f1 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp @@ -119,25 +119,8 @@ GDBRemoteRegisterContext::GetRegisterSet (size_t reg_set) bool GDBRemoteRegisterContext::ReadRegister (const RegisterInfo *reg_info, RegisterValue &value) { - ExecutionContext exe_ctx(CalculateThread()); - - Process *process = exe_ctx.GetProcessPtr(); - Thread *thread = exe_ctx.GetThreadPtr(); - if (process == NULL || thread == NULL) - return false; - - GDBRemoteCommunicationClient &gdb_comm(((ProcessGDBRemote *)process)->GetGDBRemote()); - - GDBRemoteClientBase::Lock lock(gdb_comm, false); - if (!lock) - { - if (Log *log = ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet(GDBR_LOG_THREAD | GDBR_LOG_PACKETS)) - log->Printf("GDBRemoteRegisterContext::%s failed to get packet sequence mutex", __FUNCTION__); - return false; - } - // Read the register - if (ReadRegisterBytes(reg_info, m_reg_data, gdb_comm, lock)) + if (ReadRegisterBytes (reg_info, m_reg_data)) { const bool partial_data_ok = false; Error error (value.SetValueFromData(reg_info, m_reg_data, reg_info->byte_offset, partial_data_ok)); @@ -221,23 +204,30 @@ GDBRemoteRegisterContext::PrivateSetRegisterValue (uint32_t reg, uint64_t new_re // Helper function for GDBRemoteRegisterContext::ReadRegisterBytes(). bool -GDBRemoteRegisterContext::GetPrimordialRegister(const RegisterInfo *reg_info, GDBRemoteCommunicationClient &gdb_comm, - const GDBRemoteCommunicationClient::Lock &lock) +GDBRemoteRegisterContext::GetPrimordialRegister(const RegisterInfo *reg_info, + GDBRemoteCommunicationClient &gdb_comm) { const uint32_t lldb_reg = reg_info->kinds[eRegisterKindLLDB]; const uint32_t remote_reg = reg_info->kinds[eRegisterKindProcessPlugin]; StringExtractorGDBRemote response; - if (DataBufferSP buffer_sp = gdb_comm.ReadRegister(m_thread.GetProtocolID(), remote_reg, lock)) + if (DataBufferSP buffer_sp = gdb_comm.ReadRegister(m_thread.GetProtocolID(), remote_reg)) return PrivateSetRegisterValue(lldb_reg, llvm::ArrayRef(buffer_sp->GetBytes(), buffer_sp->GetByteSize())); return false; } bool -GDBRemoteRegisterContext::ReadRegisterBytes(const RegisterInfo *reg_info, DataExtractor &data, - GDBRemoteCommunicationClient &gdb_comm, - const GDBRemoteCommunicationClient::Lock &lock) +GDBRemoteRegisterContext::ReadRegisterBytes (const RegisterInfo *reg_info, DataExtractor &data) { + ExecutionContext exe_ctx (CalculateThread()); + + Process *process = exe_ctx.GetProcessPtr(); + Thread *thread = exe_ctx.GetThreadPtr(); + if (process == NULL || thread == NULL) + return false; + + GDBRemoteCommunicationClient &gdb_comm (((ProcessGDBRemote *)process)->GetGDBRemote()); + InvalidateIfNeeded(false); const uint32_t reg = reg_info->kinds[eRegisterKindLLDB]; @@ -246,7 +236,7 @@ GDBRemoteRegisterContext::ReadRegisterBytes(const RegisterInfo *reg_info, DataEx { if (m_read_all_at_once) { - if (DataBufferSP buffer_sp = gdb_comm.ReadAllRegisters(m_thread.GetProtocolID(), lock)) + if (DataBufferSP buffer_sp = gdb_comm.ReadAllRegisters(m_thread.GetProtocolID())) { memcpy(const_cast(m_reg_data.GetDataStart()), buffer_sp->GetBytes(), std::min(buffer_sp->GetByteSize(), m_reg_data.GetByteSize())); @@ -279,7 +269,7 @@ GDBRemoteRegisterContext::ReadRegisterBytes(const RegisterInfo *reg_info, DataEx { // Read the containing register if it hasn't already been read if (!GetRegisterIsValid(prim_reg)) - success = GetPrimordialRegister(prim_reg_info, gdb_comm, lock); + success = GetPrimordialRegister(prim_reg_info, gdb_comm); } } @@ -293,7 +283,7 @@ GDBRemoteRegisterContext::ReadRegisterBytes(const RegisterInfo *reg_info, DataEx else { // Get each register individually - GetPrimordialRegister(reg_info, gdb_comm, lock); + GetPrimordialRegister(reg_info, gdb_comm); } // Make sure we got a valid register value after reading it @@ -335,8 +325,8 @@ GDBRemoteRegisterContext::WriteRegister (const RegisterInfo *reg_info, // Helper function for GDBRemoteRegisterContext::WriteRegisterBytes(). bool -GDBRemoteRegisterContext::SetPrimordialRegister(const RegisterInfo *reg_info, GDBRemoteCommunicationClient &gdb_comm, - const GDBRemoteCommunicationClient::Lock &lock) +GDBRemoteRegisterContext::SetPrimordialRegister(const RegisterInfo *reg_info, + GDBRemoteCommunicationClient &gdb_comm) { StreamString packet; StringExtractorGDBRemote response; @@ -346,7 +336,7 @@ GDBRemoteRegisterContext::SetPrimordialRegister(const RegisterInfo *reg_info, GD return gdb_comm.WriteRegister( m_thread.GetProtocolID(), reg_info->kinds[eRegisterKindProcessPlugin], - {m_reg_data.PeekData(reg_info->byte_offset, reg_info->byte_size), reg_info->byte_size}, lock); + {m_reg_data.PeekData(reg_info->byte_offset, reg_info->byte_size), reg_info->byte_size}); } bool @@ -393,7 +383,7 @@ GDBRemoteRegisterContext::WriteRegisterBytes (const RegisterInfo *reg_info, Data // Set all registers in one packet if (gdb_comm.WriteAllRegisters(m_thread.GetProtocolID(), - {m_reg_data.GetDataStart(), size_t(m_reg_data.GetByteSize())}, lock)) + {m_reg_data.GetDataStart(), size_t(m_reg_data.GetByteSize())})) { SetAllRegisterValid (false); @@ -423,13 +413,13 @@ GDBRemoteRegisterContext::WriteRegisterBytes (const RegisterInfo *reg_info, Data if (value_reg_info == NULL) success = false; else - success = SetPrimordialRegister(value_reg_info, gdb_comm, lock); + success = SetPrimordialRegister(value_reg_info, gdb_comm); } } else { // This is an actual register, write it - success = SetPrimordialRegister(reg_info, gdb_comm, lock); + success = SetPrimordialRegister(reg_info, gdb_comm); } // Check if writing this register will invalidate any other register values? @@ -535,7 +525,7 @@ GDBRemoteRegisterContext::ReadAllRegisterValues (lldb::DataBufferSP &data_sp) if (gdb_comm.SyncThreadState(m_thread.GetProtocolID())) InvalidateAllRegisters(); - if (use_g_packet && (data_sp = gdb_comm.ReadAllRegisters(m_thread.GetProtocolID(), lock))) + if (use_g_packet && (data_sp = gdb_comm.ReadAllRegisters(m_thread.GetProtocolID()))) return true; // We're going to read each register @@ -546,7 +536,7 @@ GDBRemoteRegisterContext::ReadAllRegisterValues (lldb::DataBufferSP &data_sp) { if (reg_info->value_regs) // skip registers that are slices of real registers continue; - ReadRegisterBytes(reg_info, m_reg_data, gdb_comm, lock); + ReadRegisterBytes(reg_info, m_reg_data); // ReadRegisterBytes saves the contents of the register in to the m_reg_data buffer } data_sp.reset(new DataBufferHeap(m_reg_data.GetDataStart(), m_reg_info.GetRegisterDataByteSize())); @@ -597,7 +587,7 @@ GDBRemoteRegisterContext::WriteAllRegisterValues (const lldb::DataBufferSP &data if (use_g_packet) { if (gdb_comm.WriteAllRegisters(m_thread.GetProtocolID(), - {data_sp->GetBytes(), size_t(data_sp->GetByteSize())}, lock)) + {data_sp->GetBytes(), size_t(data_sp->GetByteSize())})) return true; uint32_t num_restored = 0; @@ -684,7 +674,7 @@ GDBRemoteRegisterContext::WriteAllRegisterValues (const lldb::DataBufferSP &data { SetRegisterIsValid(reg, false); if (gdb_comm.WriteRegister(m_thread.GetProtocolID(), reg_info->kinds[eRegisterKindProcessPlugin], - {restore_src, reg_byte_size}, lock)) + {restore_src, reg_byte_size})) ++num_restored; } } @@ -723,7 +713,7 @@ GDBRemoteRegisterContext::WriteAllRegisterValues (const lldb::DataBufferSP &data SetRegisterIsValid(reg_info, false); if (gdb_comm.WriteRegister(m_thread.GetProtocolID(), reg_info->kinds[eRegisterKindProcessPlugin], - {data_sp->GetBytes() + reg_info->byte_offset, reg_info->byte_size}, lock)) + {data_sp->GetBytes() + reg_info->byte_offset, reg_info->byte_size})) ++num_restored; } return num_restored > 0; diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h index 96a21722a55b..c1d8249e5c42 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h @@ -98,8 +98,8 @@ protected: friend class ThreadGDBRemote; bool - ReadRegisterBytes(const RegisterInfo *reg_info, DataExtractor &data, GDBRemoteCommunicationClient &gdb_comm, - const GDBRemoteCommunicationClient::Lock &lock); + ReadRegisterBytes (const RegisterInfo *reg_info, + DataExtractor &data); bool WriteRegisterBytes (const RegisterInfo *reg_info, @@ -150,13 +150,11 @@ protected: private: // Helper function for ReadRegisterBytes(). - bool - GetPrimordialRegister(const RegisterInfo *reg_info, GDBRemoteCommunicationClient &gdb_comm, - const GDBRemoteCommunicationClient::Lock &lock); + bool GetPrimordialRegister(const RegisterInfo *reg_info, + GDBRemoteCommunicationClient &gdb_comm); // Helper function for WriteRegisterBytes(). - bool - SetPrimordialRegister(const RegisterInfo *reg_info, GDBRemoteCommunicationClient &gdb_comm, - const GDBRemoteCommunicationClient::Lock &lock); + bool SetPrimordialRegister(const RegisterInfo *reg_info, + GDBRemoteCommunicationClient &gdb_comm); DISALLOW_COPY_AND_ASSIGN (GDBRemoteRegisterContext); }; diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index 0e588e2659cf..169d69e1b518 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -1144,7 +1144,7 @@ ProcessGDBRemote::ConnectToDebugserver (const char *connect_url) GetTarget().SetNonStopModeEnabled (m_gdb_comm.SetNonStopMode(true)); m_gdb_comm.GetEchoSupported (); - m_gdb_comm.ComputeThreadSuffixSupport(); + m_gdb_comm.GetThreadSuffixSupported (); m_gdb_comm.GetListThreadsInStopReplySupported (); m_gdb_comm.GetHostInfo (); m_gdb_comm.GetVContSupported ('c'); diff --git a/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp b/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp index 95bd5cd5be0e..0b1f60d33d05 100644 --- a/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp +++ b/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp @@ -76,22 +76,17 @@ TEST_F(GDBRemoteCommunicationClientTest, WriteRegister) if (HasFailure()) return; - std::future suffix_result = std::async(std::launch::async, [&] { client.ComputeThreadSuffixSupport(); }); - Handle_QThreadSuffixSupported(server, true); - suffix_result.get(); - - GDBRemoteCommunicationClient::Lock lock(client, false); - ASSERT_TRUE(bool(lock)); - const lldb::tid_t tid = 0x47; const uint32_t reg_num = 4; std::future write_result = - std::async(std::launch::async, [&] { return client.WriteRegister(tid, reg_num, one_register, lock); }); + std::async(std::launch::async, [&] { return client.WriteRegister(tid, reg_num, one_register); }); + + Handle_QThreadSuffixSupported(server, true); HandlePacket(server, "P4=" + one_register_hex + ";thread:0047;", "OK"); ASSERT_TRUE(write_result.get()); - write_result = std::async(std::launch::async, [&] { return client.WriteAllRegisters(tid, all_registers, lock); }); + write_result = std::async(std::launch::async, [&] { return client.WriteAllRegisters(tid, all_registers); }); HandlePacket(server, "G" + all_registers_hex + ";thread:0047;", "OK"); ASSERT_TRUE(write_result.get()); @@ -105,23 +100,17 @@ TEST_F(GDBRemoteCommunicationClientTest, WriteRegisterNoSuffix) if (HasFailure()) return; - std::future suffix_result = std::async(std::launch::async, [&] { client.ComputeThreadSuffixSupport(); }); - Handle_QThreadSuffixSupported(server, false); - suffix_result.get(); - - GDBRemoteCommunicationClient::Lock lock(client, false); - ASSERT_TRUE(bool(lock)); - const lldb::tid_t tid = 0x47; const uint32_t reg_num = 4; std::future write_result = - std::async(std::launch::async, [&] { return client.WriteRegister(tid, reg_num, one_register, lock); }); + std::async(std::launch::async, [&] { return client.WriteRegister(tid, reg_num, one_register); }); + Handle_QThreadSuffixSupported(server, false); HandlePacket(server, "Hg47", "OK"); HandlePacket(server, "P4=" + one_register_hex, "OK"); ASSERT_TRUE(write_result.get()); - write_result = std::async(std::launch::async, [&] { return client.WriteAllRegisters(tid, all_registers, lock); }); + write_result = std::async(std::launch::async, [&] { return client.WriteAllRegisters(tid, all_registers); }); HandlePacket(server, "G" + all_registers_hex, "OK"); ASSERT_TRUE(write_result.get()); @@ -135,27 +124,21 @@ TEST_F(GDBRemoteCommunicationClientTest, ReadRegister) if (HasFailure()) return; - std::future suffix_result = std::async(std::launch::async, [&] { client.ComputeThreadSuffixSupport(); }); - Handle_QThreadSuffixSupported(server, true); - suffix_result.get(); - const lldb::tid_t tid = 0x47; const uint32_t reg_num = 4; std::future async_result = std::async(std::launch::async, [&] { return client.GetpPacketSupported(tid); }); + Handle_QThreadSuffixSupported(server, true); HandlePacket(server, "p0;thread:0047;", one_register_hex); ASSERT_TRUE(async_result.get()); - GDBRemoteCommunicationClient::Lock lock(client, false); - ASSERT_TRUE(bool(lock)); - std::future read_result = - std::async(std::launch::async, [&] { return client.ReadRegister(tid, reg_num, lock); }); + std::async(std::launch::async, [&] { return client.ReadRegister(tid, reg_num); }); HandlePacket(server, "p4;thread:0047;", "41424344"); auto buffer_sp = read_result.get(); ASSERT_TRUE(bool(buffer_sp)); ASSERT_EQ(0, memcmp(buffer_sp->GetBytes(), one_register, sizeof one_register)); - read_result = std::async(std::launch::async, [&] { return client.ReadAllRegisters(tid, lock); }); + read_result = std::async(std::launch::async, [&] { return client.ReadAllRegisters(tid); }); HandlePacket(server, "g;thread:0047;", all_registers_hex); buffer_sp = read_result.get(); ASSERT_TRUE(bool(buffer_sp)); @@ -170,14 +153,11 @@ TEST_F(GDBRemoteCommunicationClientTest, SaveRestoreRegistersNoSuffix) if (HasFailure()) return; - std::future suffix_result = std::async(std::launch::async, [&] { client.ComputeThreadSuffixSupport(); }); - Handle_QThreadSuffixSupported(server, false); - suffix_result.get(); - const lldb::tid_t tid = 0x47; uint32_t save_id; std::future 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());