Author: labath Date: Thu Aug 25 03:34:57 2016 New Revision: 279725 URL: http://llvm.org/viewvc/llvm-project?rev=279725&view=rev Log: gdb-remote: Make the sequence mutex non-recursive
Summary: This is a preparatory commit for D22914, where I'd like to replace this mutex by an R/W lock (which is also not recursive). This required a couple of changes: - The only caller of Read/WriteRegister, GDBRemoteRegisterContext class, was already acquiring the mutex, so these functions do not need to. All functions which now do not take a lock, take an lock argument instead, to remind the caller of this fact. - GetThreadSuffixSupported() was being called from locked and unlocked contexts (including contexts where the process was running, and the call would fail if it did not have the result cached). I have split this into two functions, one which computes the thread suffix support and caches it (this one always takes the lock), and another, which returns the cached value (and never needs to take the lock). This feels quite natural as ProcessGdbRemote was already pre-caching this value at the start. Reviewers: clayborg Subscribers: lldb-commits Differential Revision: https://reviews.llvm.org/D23802 Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp lldb/trunk/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp?rev=279725&r1=279724&r2=279725&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp (original) +++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp Thu Aug 25 03:34:57 2016 @@ -208,12 +208,14 @@ GDBRemoteClientBase::SendPacketAndWaitFo return PacketResult::ErrorSendFailed; } - return SendPacketAndWaitForResponseNoLock(payload, response); + return SendPacketAndWaitForResponse(payload, response, lock); } GDBRemoteCommunication::PacketResult -GDBRemoteClientBase::SendPacketAndWaitForResponseNoLock(llvm::StringRef payload, StringExtractorGDBRemote &response) +GDBRemoteClientBase::SendPacketAndWaitForResponse(llvm::StringRef payload, StringExtractorGDBRemote &response, + const Lock &lock) { + assert(lock); PacketResult packet_result = SendPacketNoLock(payload.data(), payload.size()); if (packet_result != PacketResult::Success) return packet_result; Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h?rev=279725&r1=279724&r2=279725&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h (original) +++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h Thu Aug 25 03:34:57 2016 @@ -73,7 +73,7 @@ public: Lock(GDBRemoteClientBase &comm, bool interrupt); ~Lock(); - explicit operator bool() { return m_acquired; } + explicit operator bool() const { 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<std::recursive_mutex> m_async_lock; + std::unique_lock<std::mutex> m_async_lock; GDBRemoteClientBase &m_comm; bool m_acquired; bool m_did_interrupt; @@ -94,7 +94,7 @@ public: protected: PacketResult - SendPacketAndWaitForResponseNoLock(llvm::StringRef payload, StringExtractorGDBRemote &response); + SendPacketAndWaitForResponse(llvm::StringRef payload, StringExtractorGDBRemote &response, const Lock &lock); 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::recursive_mutex m_async_mutex; + std::mutex m_async_mutex; bool ShouldStop(const UnixSignals &signals, StringExtractorGDBRemote &response); Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp?rev=279725&r1=279724&r2=279725&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp (original) +++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp Thu Aug 25 03:34:57 2016 @@ -516,21 +516,27 @@ GDBRemoteCommunicationClient::GetRemoteQ } } -bool -GDBRemoteCommunicationClient::GetThreadSuffixSupported () +void +GDBRemoteCommunicationClient::ComputeThreadSuffixSupport() { - if (m_supports_thread_suffix == eLazyBoolCalculate) + if (m_supports_thread_suffix != eLazyBoolCalculate) + return; + + StringExtractorGDBRemote response; + m_supports_thread_suffix = eLazyBoolNo; + if (SendPacketAndWaitForResponse("QThreadSuffixSupported", response, false) == PacketResult::Success) { - StringExtractorGDBRemote response; - m_supports_thread_suffix = eLazyBoolNo; - if (SendPacketAndWaitForResponse("QThreadSuffixSupported", response, false) == PacketResult::Success) - { - if (response.IsOKResponse()) - m_supports_thread_suffix = eLazyBoolYes; - } + if (response.IsOKResponse()) + m_supports_thread_suffix = eLazyBoolYes; } - return m_supports_thread_suffix; } + +bool +GDBRemoteCommunicationClient::GetThreadSuffixSupported() +{ + return m_supports_thread_suffix == eLazyBoolYes; +} + bool GDBRemoteCommunicationClient::GetVContSupported (char flavor) { @@ -592,26 +598,17 @@ GDBRemoteCommunicationClient::GetVContSu GDBRemoteCommunication::PacketResult GDBRemoteCommunicationClient::SendThreadSpecificPacketAndWaitForResponse(lldb::tid_t tid, StreamString &&payload, StringExtractorGDBRemote &response, - bool send_async) + const Lock &lock) { - 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)) + if (!SetCurrentThread(tid, lock)) return PacketResult::ErrorSendFailed; } - return SendPacketAndWaitForResponseNoLock(payload.GetString(), response); + return SendPacketAndWaitForResponse(payload.GetString(), response, lock); } // Check if the target supports 'p' packet. It sends out a 'p' @@ -624,11 +621,20 @@ GDBRemoteCommunicationClient::GetpPacket { 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, false) == + if (SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), response, lock) == PacketResult::Success && response.IsNormalResponse()) { @@ -766,7 +772,7 @@ GDBRemoteCommunicationClient::SendPacket // Construct payload char sizeDescriptor[128]; snprintf(sizeDescriptor, sizeof(sizeDescriptor), "%x,%x", offset, response_size); - PacketResult result = SendPacketAndWaitForResponseNoLock(payload_prefix_str + sizeDescriptor, this_response); + PacketResult result = SendPacketAndWaitForResponse(payload_prefix_str + sizeDescriptor, this_response, lock); if (result != PacketResult::Success) return result; @@ -2789,7 +2795,7 @@ GDBRemoteCommunicationClient::KillSpawne } bool -GDBRemoteCommunicationClient::SetCurrentThread (uint64_t tid) +GDBRemoteCommunicationClient::SetCurrentThread(uint64_t tid, const Lock &lock) { if (m_curr_tid == tid) return true; @@ -2802,7 +2808,7 @@ GDBRemoteCommunicationClient::SetCurrent packet_len = ::snprintf (packet, sizeof(packet), "Hg%" PRIx64, tid); assert (packet_len + 1 < (int)sizeof(packet)); StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse(packet, packet_len, response, false) == PacketResult::Success) + if (SendPacketAndWaitForResponse(llvm::StringRef(packet, packet_len), response, lock) == PacketResult::Success) { if (response.IsOKResponse()) { @@ -2963,9 +2969,9 @@ GDBRemoteCommunicationClient::GetCurrent StringExtractorGDBRemote response; PacketResult packet_result; - for (packet_result = SendPacketAndWaitForResponseNoLock("qfThreadInfo", response); + for (packet_result = SendPacketAndWaitForResponse("qfThreadInfo", response, lock); packet_result == PacketResult::Success && response.IsNormalResponse(); - packet_result = SendPacketAndWaitForResponseNoLock("qsThreadInfo", response)) + packet_result = SendPacketAndWaitForResponse("qsThreadInfo", response, lock)) { char ch = response.GetChar(); if (ch == 'l') @@ -3496,12 +3502,12 @@ GDBRemoteCommunicationClient::AvoidGPack } DataBufferSP -GDBRemoteCommunicationClient::ReadRegister(lldb::tid_t tid, uint32_t reg) +GDBRemoteCommunicationClient::ReadRegister(lldb::tid_t tid, uint32_t reg, const Lock &lock) { StreamString payload; payload.Printf("p%x", reg); StringExtractorGDBRemote response; - if (SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), response, false) != PacketResult::Success || + if (SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), response, lock) != PacketResult::Success || !response.IsNormalResponse()) return nullptr; @@ -3511,12 +3517,12 @@ GDBRemoteCommunicationClient::ReadRegist } DataBufferSP -GDBRemoteCommunicationClient::ReadAllRegisters(lldb::tid_t tid) +GDBRemoteCommunicationClient::ReadAllRegisters(lldb::tid_t tid, const Lock &lock) { StreamString payload; payload.PutChar('g'); StringExtractorGDBRemote response; - if (SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), response, false) != PacketResult::Success || + if (SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), response, lock) != PacketResult::Success || !response.IsNormalResponse()) return nullptr; @@ -3526,25 +3532,26 @@ GDBRemoteCommunicationClient::ReadAllReg } bool -GDBRemoteCommunicationClient::WriteRegister(lldb::tid_t tid, uint32_t reg_num, llvm::ArrayRef<uint8_t> data) +GDBRemoteCommunicationClient::WriteRegister(lldb::tid_t tid, uint32_t reg_num, llvm::ArrayRef<uint8_t> data, + const Lock &lock) { 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, false) == + return SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), response, lock) == PacketResult::Success && response.IsOKResponse(); } bool -GDBRemoteCommunicationClient::WriteAllRegisters(lldb::tid_t tid, llvm::ArrayRef<uint8_t> data) +GDBRemoteCommunicationClient::WriteAllRegisters(lldb::tid_t tid, llvm::ArrayRef<uint8_t> data, const Lock &lock) { StreamString payload; payload.PutChar('G'); payload.PutBytesAsRawHex8(data.data(), data.size(), endian::InlHostByteOrder(), endian::InlHostByteOrder()); StringExtractorGDBRemote response; - return SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), response, false) == + return SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), response, lock) == PacketResult::Success && response.IsOKResponse(); } @@ -3555,12 +3562,21 @@ GDBRemoteCommunicationClient::SaveRegist 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, false) != PacketResult::Success) + if (SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), response, lock) != PacketResult::Success) return false; if (response.IsUnsupportedResponse()) @@ -3583,10 +3599,19 @@ GDBRemoteCommunicationClient::RestoreReg 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, false) != PacketResult::Success) + if (SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), response, lock) != PacketResult::Success) return false; if (response.IsOKResponse()) @@ -3815,7 +3840,7 @@ GDBRemoteCommunicationClient::ServeSymbo StreamString packet; packet.PutCString ("qSymbol::"); StringExtractorGDBRemote response; - while (SendPacketAndWaitForResponseNoLock(packet.GetString(), response) == PacketResult::Success) + while (SendPacketAndWaitForResponse(packet.GetString(), response, lock) == PacketResult::Success) { if (response.IsOKResponse()) { Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h?rev=279725&r1=279724&r2=279725&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h (original) +++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h Thu Aug 25 03:34:57 2016 @@ -64,6 +64,9 @@ public: SendPacketsAndConcatenateResponses (const char *send_payload_prefix, std::string &response_string); + void + ComputeThreadSuffixSupport(); + bool GetThreadSuffixSupported(); @@ -395,9 +398,6 @@ public: uint32_t recv_size); bool - SetCurrentThread (uint64_t tid); - - bool SetCurrentThreadForRun (uint64_t tid); bool @@ -488,17 +488,18 @@ public: lldb::DataBufferSP ReadRegister(lldb::tid_t tid, - uint32_t reg_num); // Must be the eRegisterKindProcessPlugin register number + uint32_t reg_num, // Must be the eRegisterKindProcessPlugin register number + const Lock &lock); lldb::DataBufferSP - ReadAllRegisters(lldb::tid_t tid); + ReadAllRegisters(lldb::tid_t tid, const Lock &lock); bool WriteRegister(lldb::tid_t tid, uint32_t reg_num, // eRegisterKindProcessPlugin register number - llvm::ArrayRef<uint8_t> data); + llvm::ArrayRef<uint8_t> data, const Lock &lock); bool - WriteAllRegisters(lldb::tid_t tid, llvm::ArrayRef<uint8_t> data); + WriteAllRegisters(lldb::tid_t tid, llvm::ArrayRef<uint8_t> data, const Lock &lock); bool SaveRegisterState(lldb::tid_t tid, uint32_t &save_id); @@ -686,7 +687,10 @@ protected: PacketResult SendThreadSpecificPacketAndWaitForResponse(lldb::tid_t tid, StreamString &&payload, - StringExtractorGDBRemote &response, bool send_async); + StringExtractorGDBRemote &response, const Lock &lock); + + bool + SetCurrentThread(uint64_t tid, const Lock &lock); private: DISALLOW_COPY_AND_ASSIGN (GDBRemoteCommunicationClient); Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp?rev=279725&r1=279724&r2=279725&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp (original) +++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp Thu Aug 25 03:34:57 2016 @@ -119,8 +119,25 @@ GDBRemoteRegisterContext::GetRegisterSet 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)) + if (ReadRegisterBytes(reg_info, m_reg_data, gdb_comm, lock)) { const bool partial_data_ok = false; Error error (value.SetValueFromData(reg_info, m_reg_data, reg_info->byte_offset, partial_data_ok)); @@ -204,30 +221,23 @@ GDBRemoteRegisterContext::PrivateSetRegi // Helper function for GDBRemoteRegisterContext::ReadRegisterBytes(). bool -GDBRemoteRegisterContext::GetPrimordialRegister(const RegisterInfo *reg_info, - GDBRemoteCommunicationClient &gdb_comm) +GDBRemoteRegisterContext::GetPrimordialRegister(const RegisterInfo *reg_info, GDBRemoteCommunicationClient &gdb_comm, + const GDBRemoteCommunicationClient::Lock &lock) { 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)) + if (DataBufferSP buffer_sp = gdb_comm.ReadRegister(m_thread.GetProtocolID(), remote_reg, lock)) return PrivateSetRegisterValue(lldb_reg, llvm::ArrayRef<uint8_t>(buffer_sp->GetBytes(), buffer_sp->GetByteSize())); return false; } bool -GDBRemoteRegisterContext::ReadRegisterBytes (const RegisterInfo *reg_info, DataExtractor &data) +GDBRemoteRegisterContext::ReadRegisterBytes(const RegisterInfo *reg_info, DataExtractor &data, + GDBRemoteCommunicationClient &gdb_comm, + const GDBRemoteCommunicationClient::Lock &lock) { - 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]; @@ -236,7 +246,7 @@ GDBRemoteRegisterContext::ReadRegisterBy { if (m_read_all_at_once) { - if (DataBufferSP buffer_sp = gdb_comm.ReadAllRegisters(m_thread.GetProtocolID())) + if (DataBufferSP buffer_sp = gdb_comm.ReadAllRegisters(m_thread.GetProtocolID(), lock)) { memcpy(const_cast<uint8_t *>(m_reg_data.GetDataStart()), buffer_sp->GetBytes(), std::min(buffer_sp->GetByteSize(), m_reg_data.GetByteSize())); @@ -269,7 +279,7 @@ GDBRemoteRegisterContext::ReadRegisterBy { // Read the containing register if it hasn't already been read if (!GetRegisterIsValid(prim_reg)) - success = GetPrimordialRegister(prim_reg_info, gdb_comm); + success = GetPrimordialRegister(prim_reg_info, gdb_comm, lock); } } @@ -283,7 +293,7 @@ GDBRemoteRegisterContext::ReadRegisterBy else { // Get each register individually - GetPrimordialRegister(reg_info, gdb_comm); + GetPrimordialRegister(reg_info, gdb_comm, lock); } // Make sure we got a valid register value after reading it @@ -325,8 +335,8 @@ GDBRemoteRegisterContext::WriteRegister // Helper function for GDBRemoteRegisterContext::WriteRegisterBytes(). bool -GDBRemoteRegisterContext::SetPrimordialRegister(const RegisterInfo *reg_info, - GDBRemoteCommunicationClient &gdb_comm) +GDBRemoteRegisterContext::SetPrimordialRegister(const RegisterInfo *reg_info, GDBRemoteCommunicationClient &gdb_comm, + const GDBRemoteCommunicationClient::Lock &lock) { StreamString packet; StringExtractorGDBRemote response; @@ -336,7 +346,7 @@ GDBRemoteRegisterContext::SetPrimordialR 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}); + {m_reg_data.PeekData(reg_info->byte_offset, reg_info->byte_size), reg_info->byte_size}, lock); } bool @@ -383,7 +393,7 @@ GDBRemoteRegisterContext::WriteRegisterB // Set all registers in one packet if (gdb_comm.WriteAllRegisters(m_thread.GetProtocolID(), - {m_reg_data.GetDataStart(), size_t(m_reg_data.GetByteSize())})) + {m_reg_data.GetDataStart(), size_t(m_reg_data.GetByteSize())}, lock)) { SetAllRegisterValid (false); @@ -413,13 +423,13 @@ GDBRemoteRegisterContext::WriteRegisterB if (value_reg_info == NULL) success = false; else - success = SetPrimordialRegister(value_reg_info, gdb_comm); + success = SetPrimordialRegister(value_reg_info, gdb_comm, lock); } } else { // This is an actual register, write it - success = SetPrimordialRegister(reg_info, gdb_comm); + success = SetPrimordialRegister(reg_info, gdb_comm, lock); } // Check if writing this register will invalidate any other register values? @@ -525,7 +535,7 @@ GDBRemoteRegisterContext::ReadAllRegiste if (gdb_comm.SyncThreadState(m_thread.GetProtocolID())) InvalidateAllRegisters(); - if (use_g_packet && (data_sp = gdb_comm.ReadAllRegisters(m_thread.GetProtocolID()))) + if (use_g_packet && (data_sp = gdb_comm.ReadAllRegisters(m_thread.GetProtocolID(), lock))) return true; // We're going to read each register @@ -536,7 +546,7 @@ GDBRemoteRegisterContext::ReadAllRegiste { if (reg_info->value_regs) // skip registers that are slices of real registers continue; - ReadRegisterBytes(reg_info, m_reg_data); + ReadRegisterBytes(reg_info, m_reg_data, gdb_comm, lock); // 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())); @@ -587,7 +597,7 @@ GDBRemoteRegisterContext::WriteAllRegist if (use_g_packet) { if (gdb_comm.WriteAllRegisters(m_thread.GetProtocolID(), - {data_sp->GetBytes(), size_t(data_sp->GetByteSize())})) + {data_sp->GetBytes(), size_t(data_sp->GetByteSize())}, lock)) return true; uint32_t num_restored = 0; @@ -674,7 +684,7 @@ GDBRemoteRegisterContext::WriteAllRegist { SetRegisterIsValid(reg, false); if (gdb_comm.WriteRegister(m_thread.GetProtocolID(), reg_info->kinds[eRegisterKindProcessPlugin], - {restore_src, reg_byte_size})) + {restore_src, reg_byte_size}, lock)) ++num_restored; } } @@ -713,7 +723,7 @@ GDBRemoteRegisterContext::WriteAllRegist 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})) + {data_sp->GetBytes() + reg_info->byte_offset, reg_info->byte_size}, lock)) ++num_restored; } return num_restored > 0; Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h?rev=279725&r1=279724&r2=279725&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h (original) +++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h Thu Aug 25 03:34:57 2016 @@ -98,8 +98,8 @@ protected: friend class ThreadGDBRemote; bool - ReadRegisterBytes (const RegisterInfo *reg_info, - DataExtractor &data); + ReadRegisterBytes(const RegisterInfo *reg_info, DataExtractor &data, GDBRemoteCommunicationClient &gdb_comm, + const GDBRemoteCommunicationClient::Lock &lock); bool WriteRegisterBytes (const RegisterInfo *reg_info, @@ -150,11 +150,13 @@ protected: private: // Helper function for ReadRegisterBytes(). - bool GetPrimordialRegister(const RegisterInfo *reg_info, - GDBRemoteCommunicationClient &gdb_comm); + bool + GetPrimordialRegister(const RegisterInfo *reg_info, GDBRemoteCommunicationClient &gdb_comm, + const GDBRemoteCommunicationClient::Lock &lock); // Helper function for WriteRegisterBytes(). - bool SetPrimordialRegister(const RegisterInfo *reg_info, - GDBRemoteCommunicationClient &gdb_comm); + bool + SetPrimordialRegister(const RegisterInfo *reg_info, GDBRemoteCommunicationClient &gdb_comm, + const GDBRemoteCommunicationClient::Lock &lock); DISALLOW_COPY_AND_ASSIGN (GDBRemoteRegisterContext); }; Modified: lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp?rev=279725&r1=279724&r2=279725&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp (original) +++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp Thu Aug 25 03:34:57 2016 @@ -1150,7 +1150,7 @@ ProcessGDBRemote::ConnectToDebugserver ( GetTarget().SetNonStopModeEnabled (m_gdb_comm.SetNonStopMode(true)); m_gdb_comm.GetEchoSupported (); - m_gdb_comm.GetThreadSuffixSupported (); + m_gdb_comm.ComputeThreadSuffixSupport(); m_gdb_comm.GetListThreadsInStopReplySupported (); m_gdb_comm.GetHostInfo (); m_gdb_comm.GetVContSupported ('c'); Modified: lldb/trunk/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp?rev=279725&r1=279724&r2=279725&view=diff ============================================================================== --- lldb/trunk/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp (original) +++ lldb/trunk/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp Thu Aug 25 03:34:57 2016 @@ -76,17 +76,22 @@ TEST_F(GDBRemoteCommunicationClientTest, if (HasFailure()) return; + std::future<void> 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<bool> write_result = - std::async(std::launch::async, [&] { return client.WriteRegister(tid, reg_num, one_register); }); - - Handle_QThreadSuffixSupported(server, true); + std::async(std::launch::async, [&] { return client.WriteRegister(tid, reg_num, one_register, lock); }); 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); }); + write_result = std::async(std::launch::async, [&] { return client.WriteAllRegisters(tid, all_registers, lock); }); HandlePacket(server, "G" + all_registers_hex + ";thread:0047;", "OK"); ASSERT_TRUE(write_result.get()); @@ -100,17 +105,23 @@ TEST_F(GDBRemoteCommunicationClientTest, if (HasFailure()) return; + std::future<void> 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<bool> write_result = - std::async(std::launch::async, [&] { return client.WriteRegister(tid, reg_num, one_register); }); + std::async(std::launch::async, [&] { return client.WriteRegister(tid, reg_num, one_register, lock); }); - 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); }); + write_result = std::async(std::launch::async, [&] { return client.WriteAllRegisters(tid, all_registers, lock); }); HandlePacket(server, "G" + all_registers_hex, "OK"); ASSERT_TRUE(write_result.get()); @@ -124,21 +135,27 @@ TEST_F(GDBRemoteCommunicationClientTest, if (HasFailure()) return; + std::future<void> 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<bool> 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<DataBufferSP> read_result = - std::async(std::launch::async, [&] { return client.ReadRegister(tid, reg_num); }); + std::async(std::launch::async, [&] { return client.ReadRegister(tid, reg_num, lock); }); 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); }); + read_result = std::async(std::launch::async, [&] { return client.ReadAllRegisters(tid, lock); }); HandlePacket(server, "g;thread:0047;", all_registers_hex); buffer_sp = read_result.get(); ASSERT_TRUE(bool(buffer_sp)); @@ -153,11 +170,14 @@ TEST_F(GDBRemoteCommunicationClientTest, if (HasFailure()) return; + std::future<void> 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<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()); _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
