[Lldb-commits] [lldb] r279725 - gdb-remote: Make the sequence mutex non-recursive

Pavel Labath via lldb-commits lldb-commits at lists.llvm.org
Thu Aug 25 01:34:57 PDT 2016


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());




More information about the lldb-commits mailing list