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

llvm-svn: 279725

diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
index fde3803..c3991a3 100644 (file)
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
@@ -208,12 +208,14 @@ GDBRemoteClientBase::SendPacketAndWaitForResponse(llvm::StringRef payload, Strin
         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;

diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
index 684ef3e..a3bcc86 100644 (file)
--- 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() { 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);

diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 038ba30..5f766a7 100644 (file)
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -516,21 +516,27 @@ GDBRemoteCommunicationClient::GetRemoteQSupported ()
     }
 }
 
-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::GetVContSupported (char flavor)
 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::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, false) ==
+        if (SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), response, lock) ==
                 PacketResult::Success &&
             response.IsNormalResponse())
         {
@@ -766,7 +772,7 @@ GDBRemoteCommunicationClient::SendPacketsAndConcatenateResponses
         // 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::KillSpawnedProcess (lldb::pid_t pid)
 }
 
 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::SetCurrentThread (uint64_t tid)
         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::GetCurrentThreadIDs (std::vector<lldb::tid_t> &thr
         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::AvoidGPackets (ProcessGDBRemote *process)
 }
 
 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::ReadRegister(lldb::tid_t tid, uint32_t reg)
 }
 
 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::ReadAllRegisters(lldb::tid_t tid)
 }
 
 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::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, false) != PacketResult::Success)
+    if (SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), response, lock) != PacketResult::Success)
         return false;
 
     if (response.IsUnsupportedResponse())
@@ -3583,10 +3599,19 @@ 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, false) != PacketResult::Success)
+    if (SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), response, lock) != PacketResult::Success)
         return false;
 
     if (response.IsOKResponse())
@@ -3815,7 +3840,7 @@ GDBRemoteCommunicationClient::ServeSymbolLookups(lldb_private::Process *process)
             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())
                 {

diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
index eeeecb5..a714b40 100644 (file)
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -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);

diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
index 78e1956..0f86bf5 100644 (file)
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
@@ -119,8 +119,25 @@ 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))
+    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::PrivateSetRegisterValue (uint32_t reg, uint64_t new_re
 
 // 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::ReadRegisterBytes (const RegisterInfo *reg_info, DataE
     {
         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::ReadRegisterBytes (const RegisterInfo *reg_info, DataE
                 {
                     // 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::ReadRegisterBytes (const RegisterInfo *reg_info, DataE
         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 (const RegisterInfo *reg_info,
 
 // 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::SetPrimordialRegister(const RegisterInfo *reg_info,
 
     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::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())}))
+                                                   {m_reg_data.GetDataStart(), size_t(m_reg_data.GetByteSize())}, lock))
 
                     {
                         SetAllRegisterValid (false);
@@ -413,13 +423,13 @@ GDBRemoteRegisterContext::WriteRegisterBytes (const RegisterInfo *reg_info, Data
                             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::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())))
+        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::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);
+            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::WriteAllRegisterValues (const lldb::DataBufferSP &data
         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::WriteAllRegisterValues (const lldb::DataBufferSP &data
                 {
                     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::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}))
+                                           {data_sp->GetBytes() + reg_info->byte_offset, reg_info->byte_size}, lock))
                     ++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 c1d8249..96a2172 100644 (file)
--- 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);
+    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);
 };

diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 67fa0ee..dacc5ba 100644 (file)
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1150,7 +1150,7 @@ ProcessGDBRemote::ConnectToDebugserver (const char *connect_url)
         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');

diff --git a/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp b/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
index 0b1f60d..95bd5cd 100644 (file)
--- a/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ b/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -76,17 +76,22 @@ TEST_F(GDBRemoteCommunicationClientTest, WriteRegister)
     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, WriteRegisterNoSuffix)
     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, ReadRegister)
     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, SaveRestoreRegistersNoSuffix)
     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());