Index: source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h =================================================================== --- source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h +++ source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h @@ -83,7 +83,7 @@ } private: - std::unique_lock m_async_lock; + std::unique_lock m_async_lock; GDBRemoteClientBase &m_comm; bool m_acquired; bool m_did_interrupt; @@ -126,7 +126,7 @@ // 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); Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h =================================================================== --- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h +++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h @@ -64,6 +64,9 @@ SendPacketsAndConcatenateResponses (const char *send_payload_prefix, std::string &response_string); + void + ComputeThreadSuffixSupport(); + bool GetThreadSuffixSupported(); @@ -395,9 +398,6 @@ uint32_t recv_size); bool - SetCurrentThread (uint64_t tid); - - bool SetCurrentThreadForRun (uint64_t tid); bool @@ -487,18 +487,18 @@ CalculateMD5 (const FileSpec& file_spec, uint64_t &high, uint64_t &low); lldb::DataBufferSP - ReadRegister(lldb::tid_t tid, + ReadRegisterNoLock(lldb::tid_t tid, uint32_t reg_num); // Must be the eRegisterKindProcessPlugin register number lldb::DataBufferSP - ReadAllRegisters(lldb::tid_t tid); + ReadAllRegistersNoLock(lldb::tid_t tid); bool - WriteRegister(lldb::tid_t tid, uint32_t reg_num, // eRegisterKindProcessPlugin register number + WriteRegisterNoLock(lldb::tid_t tid, uint32_t reg_num, // eRegisterKindProcessPlugin register number llvm::ArrayRef data); bool - WriteAllRegisters(lldb::tid_t tid, llvm::ArrayRef data); + WriteAllRegistersNoLock(lldb::tid_t tid, llvm::ArrayRef data); bool SaveRegisterState(lldb::tid_t tid, uint32_t &save_id); @@ -685,8 +685,11 @@ OnRunPacketSent(bool first) override; PacketResult - SendThreadSpecificPacketAndWaitForResponse(lldb::tid_t tid, StreamString &&payload, - StringExtractorGDBRemote &response, bool send_async); + SendThreadSpecificPacketAndWaitForResponseNoLock(lldb::tid_t tid, StreamString &&payload, + StringExtractorGDBRemote &response); + + bool + SetCurrentThreadNoLock(uint64_t tid); private: DISALLOW_COPY_AND_ASSIGN (GDBRemoteCommunicationClient); Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp =================================================================== --- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp +++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp @@ -516,21 +516,27 @@ } } -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) { @@ -590,24 +596,14 @@ } GDBRemoteCommunication::PacketResult -GDBRemoteCommunicationClient::SendThreadSpecificPacketAndWaitForResponse(lldb::tid_t tid, StreamString &&payload, - StringExtractorGDBRemote &response, - bool send_async) +GDBRemoteCommunicationClient::SendThreadSpecificPacketAndWaitForResponseNoLock(lldb::tid_t tid, StreamString &&payload, + StringExtractorGDBRemote &response) { - 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 (!SetCurrentThreadNoLock(tid)) return PacketResult::ErrorSendFailed; } @@ -624,11 +620,20 @@ { 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 (SendThreadSpecificPacketAndWaitForResponseNoLock(tid, std::move(payload), response) == PacketResult::Success && response.IsNormalResponse()) { @@ -2789,7 +2794,7 @@ } bool -GDBRemoteCommunicationClient::SetCurrentThread (uint64_t tid) +GDBRemoteCommunicationClient::SetCurrentThreadNoLock(uint64_t tid) { if (m_curr_tid == tid) return true; @@ -2802,7 +2807,7 @@ 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 (SendPacketAndWaitForResponseNoLock(llvm::StringRef(packet, packet_len), response) == PacketResult::Success) { if (response.IsOKResponse()) { @@ -3496,12 +3501,12 @@ } DataBufferSP -GDBRemoteCommunicationClient::ReadRegister(lldb::tid_t tid, uint32_t reg) +GDBRemoteCommunicationClient::ReadRegisterNoLock(lldb::tid_t tid, uint32_t reg) { StreamString payload; payload.Printf("p%x", reg); StringExtractorGDBRemote response; - if (SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), response, false) != PacketResult::Success || + if (SendThreadSpecificPacketAndWaitForResponseNoLock(tid, std::move(payload), response) != PacketResult::Success || !response.IsNormalResponse()) return nullptr; @@ -3511,12 +3516,12 @@ } DataBufferSP -GDBRemoteCommunicationClient::ReadAllRegisters(lldb::tid_t tid) +GDBRemoteCommunicationClient::ReadAllRegistersNoLock(lldb::tid_t tid) { StreamString payload; payload.PutChar('g'); StringExtractorGDBRemote response; - if (SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), response, false) != PacketResult::Success || + if (SendThreadSpecificPacketAndWaitForResponseNoLock(tid, std::move(payload), response) != PacketResult::Success || !response.IsNormalResponse()) return nullptr; @@ -3526,25 +3531,25 @@ } bool -GDBRemoteCommunicationClient::WriteRegister(lldb::tid_t tid, uint32_t reg_num, llvm::ArrayRef data) +GDBRemoteCommunicationClient::WriteRegisterNoLock(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, false) == + return SendThreadSpecificPacketAndWaitForResponseNoLock(tid, std::move(payload), response) == PacketResult::Success && response.IsOKResponse(); } bool -GDBRemoteCommunicationClient::WriteAllRegisters(lldb::tid_t tid, llvm::ArrayRef data) +GDBRemoteCommunicationClient::WriteAllRegistersNoLock(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, false) == + return SendThreadSpecificPacketAndWaitForResponseNoLock(tid, std::move(payload), response) == PacketResult::Success && response.IsOKResponse(); } @@ -3555,12 +3560,21 @@ 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 (SendThreadSpecificPacketAndWaitForResponseNoLock(tid, std::move(payload), response) != PacketResult::Success) return false; if (response.IsUnsupportedResponse()) @@ -3583,10 +3597,19 @@ 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 (SendThreadSpecificPacketAndWaitForResponseNoLock(tid, std::move(payload), response) != PacketResult::Success) return false; if (response.IsOKResponse()) Index: source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h =================================================================== --- source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h +++ source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h @@ -98,8 +98,7 @@ friend class ThreadGDBRemote; bool - ReadRegisterBytes (const RegisterInfo *reg_info, - DataExtractor &data); + ReadRegisterBytes(const RegisterInfo *reg_info, DataExtractor &data, GDBRemoteCommunicationClient &gdb_comm); bool WriteRegisterBytes (const RegisterInfo *reg_info, Index: source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp =================================================================== --- source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp +++ source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp @@ -119,8 +119,25 @@ 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)) { const bool partial_data_ok = false; Error error (value.SetValueFromData(reg_info, m_reg_data, reg_info->byte_offset, partial_data_ok)); @@ -210,24 +227,15 @@ 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.ReadRegisterNoLock(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) +GDBRemoteRegisterContext::ReadRegisterBytes (const RegisterInfo *reg_info, DataExtractor &data, GDBRemoteCommunicationClient &gdb_comm) { - 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 +244,7 @@ { if (m_read_all_at_once) { - if (DataBufferSP buffer_sp = gdb_comm.ReadAllRegisters(m_thread.GetProtocolID())) + if (DataBufferSP buffer_sp = gdb_comm.ReadAllRegistersNoLock(m_thread.GetProtocolID())) { memcpy(const_cast(m_reg_data.GetDataStart()), buffer_sp->GetBytes(), std::min(buffer_sp->GetByteSize(), m_reg_data.GetByteSize())); @@ -334,7 +342,7 @@ // Invalidate just this register SetRegisterIsValid(reg, false); - return gdb_comm.WriteRegister( + return gdb_comm.WriteRegisterNoLock( m_thread.GetProtocolID(), reg_info->kinds[eRegisterKindProcessPlugin], {m_reg_data.PeekData(reg_info->byte_offset, reg_info->byte_size), reg_info->byte_size}); } @@ -382,7 +390,7 @@ InvalidateIfNeeded (true); // Set all registers in one packet - if (gdb_comm.WriteAllRegisters(m_thread.GetProtocolID(), + if (gdb_comm.WriteAllRegistersNoLock(m_thread.GetProtocolID(), {m_reg_data.GetDataStart(), size_t(m_reg_data.GetByteSize())})) { @@ -525,7 +533,7 @@ 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.ReadAllRegistersNoLock(m_thread.GetProtocolID()))) return true; // We're going to read each register @@ -536,7 +544,7 @@ { 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); // 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())); @@ -586,8 +594,8 @@ // The data_sp contains the G response packet. if (use_g_packet) { - if (gdb_comm.WriteAllRegisters(m_thread.GetProtocolID(), - {data_sp->GetBytes(), size_t(data_sp->GetByteSize())})) + if (gdb_comm.WriteAllRegistersNoLock(m_thread.GetProtocolID(), + {data_sp->GetBytes(), size_t(data_sp->GetByteSize())})) return true; uint32_t num_restored = 0; @@ -673,7 +681,7 @@ if (restore_src) { SetRegisterIsValid(reg, false); - if (gdb_comm.WriteRegister(m_thread.GetProtocolID(), reg_info->kinds[eRegisterKindProcessPlugin], + if (gdb_comm.WriteRegisterNoLock(m_thread.GetProtocolID(), reg_info->kinds[eRegisterKindProcessPlugin], {restore_src, reg_byte_size})) ++num_restored; } @@ -712,7 +720,7 @@ } SetRegisterIsValid(reg_info, false); - if (gdb_comm.WriteRegister(m_thread.GetProtocolID(), reg_info->kinds[eRegisterKindProcessPlugin], + if (gdb_comm.WriteRegisterNoLock(m_thread.GetProtocolID(), reg_info->kinds[eRegisterKindProcessPlugin], {data_sp->GetBytes() + reg_info->byte_offset, reg_info->byte_size})) ++num_restored; } Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp =================================================================== --- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -1150,7 +1150,7 @@ 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'); Index: unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp =================================================================== --- unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp +++ unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp @@ -76,17 +76,23 @@ 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); }); + std::async(std::launch::async, [&] { return client.WriteRegisterNoLock(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); }); + write_result = std::async(std::launch::async, [&] { return client.WriteAllRegistersNoLock(tid, all_registers); }); HandlePacket(server, "G" + all_registers_hex + ";thread:0047;", "OK"); ASSERT_TRUE(write_result.get()); @@ -100,17 +106,23 @@ 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); }); + std::async(std::launch::async, [&] { return client.WriteRegisterNoLock(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); }); + write_result = std::async(std::launch::async, [&] { return client.WriteAllRegistersNoLock(tid, all_registers); }); HandlePacket(server, "G" + all_registers_hex, "OK"); ASSERT_TRUE(write_result.get()); @@ -124,21 +136,24 @@ 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()); std::future read_result = - std::async(std::launch::async, [&] { return client.ReadRegister(tid, reg_num); }); + std::async(std::launch::async, [&] { return client.ReadRegisterNoLock(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); }); + read_result = std::async(std::launch::async, [&] { return client.ReadAllRegistersNoLock(tid); }); HandlePacket(server, "g;thread:0047;", all_registers_hex); buffer_sp = read_result.get(); ASSERT_TRUE(bool(buffer_sp)); @@ -153,11 +168,14 @@ 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());