No functionality changes, mostly cleanup.

Cleaned up the Mutex::Locker and the ReadWriteLock classes a bit.

Also cleaned up the GDBRemoteCommunication class to not have so many packet functions. Used the "NoLock" versions of send/receive packet functions when possible for a bit of performance.

llvm-svn: 154458

7 files changed, 72 insertions, 105 deletions

diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
index caa70258e30..1535fd41260 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -158,21 +158,6 @@ GDBRemoteCommunication::SendNack ()
 }
 
 size_t
-GDBRemoteCommunication::SendPacket (lldb_private::StreamString &payload)
-{
-    Mutex::Locker locker(m_sequence_mutex);
-    const std::string &p (payload.GetString());
-    return SendPacketNoLock (p.c_str(), p.size());
-}
-
-size_t
-GDBRemoteCommunication::SendPacket (const char *payload)
-{
-    Mutex::Locker locker(m_sequence_mutex);
-    return SendPacketNoLock (payload, ::strlen (payload));
-}
-
-size_t
 GDBRemoteCommunication::SendPacket (const char *payload, size_t payload_length)
 {
     Mutex::Locker locker(m_sequence_mutex);
@@ -234,15 +219,20 @@ char
 GDBRemoteCommunication::GetAck ()
 {
     StringExtractorGDBRemote packet;
-    if (WaitForPacketWithTimeoutMicroSeconds (packet, GetPacketTimeoutInMicroSeconds ()) == 1)
+    if (WaitForPacketWithTimeoutMicroSecondsNoLock (packet, GetPacketTimeoutInMicroSeconds ()) == 1)
         return packet.GetChar();
     return 0;
 }
 
 bool
-GDBRemoteCommunication::TryLockSequenceMutex (Mutex::Locker& locker)
+GDBRemoteCommunication::GetSequenceMutex (Mutex::Locker& locker, uint32_t usec_timeout)
 {
-    return locker.TryLock (m_sequence_mutex.GetMutex());
+    if (usec_timeout == 0)
+        return locker.TryLock (m_sequence_mutex.GetMutex());
+    
+    // Wait for the lock
+    locker.Lock (m_sequence_mutex.GetMutex());
+    return true;
 }
 
 
@@ -253,13 +243,6 @@ GDBRemoteCommunication::WaitForNotRunningPrivate (const TimeValue *timeout_ptr)
 }
 
 size_t
-GDBRemoteCommunication::WaitForPacketWithTimeoutMicroSeconds (StringExtractorGDBRemote &packet, uint32_t timeout_usec)
-{
-    Mutex::Locker locker(m_sequence_mutex);
-    return WaitForPacketWithTimeoutMicroSecondsNoLock (packet, timeout_usec);
-}
-
-size_t
 GDBRemoteCommunication::WaitForPacketWithTimeoutMicroSecondsNoLock (StringExtractorGDBRemote &packet, uint32_t timeout_usec)
 {
     uint8_t buffer[8192];
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
index d7986d5b278..ef0c1983898 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
@@ -45,21 +45,6 @@ public:
     virtual
     ~GDBRemoteCommunication();
 
-    size_t
-    SendPacket (const char *payload);
-
-    size_t
-    SendPacket (const char *payload,
-                size_t payload_length);
-
-    size_t
-    SendPacket (lldb_private::StreamString &response);
-
-    // Wait for a packet within 'nsec' seconds
-    size_t
-    WaitForPacketWithTimeoutMicroSeconds (StringExtractorGDBRemote &response,
-                                          uint32_t usec);
-
     char
     GetAck ();
 
@@ -74,7 +59,7 @@ public:
                         size_t payload_length);
 
     bool
-    TryLockSequenceMutex(lldb_private::Mutex::Locker& locker);
+    GetSequenceMutex (lldb_private::Mutex::Locker& locker, uint32_t usec_timeout);
 
     bool
     CheckForPacket (const uint8_t *src, 
@@ -260,6 +245,10 @@ protected:
     };
 
     size_t
+    SendPacket (const char *payload,
+                size_t payload_length);
+
+    size_t
     SendPacketNoLock (const char *payload, 
                       size_t payload_length);
 
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 16a339c86c2..4efacd57eee 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -259,7 +259,7 @@ GDBRemoteCommunicationClient::SendPacketAndWaitForResponse
     Mutex::Locker locker;
     LogSP log (ProcessGDBRemoteLog::GetLogIfAllCategoriesSet (GDBR_LOG_PROCESS));
     size_t response_len = 0;
-    if (TryLockSequenceMutex (locker))
+    if (GetSequenceMutex (locker, 0))
     {
         if (SendPacketNoLock (payload, payload_length))
            response_len = WaitForPacketWithTimeoutMicroSecondsNoLock (response, GetPacketTimeoutInMicroSeconds ());
@@ -361,30 +361,6 @@ GDBRemoteCommunicationClient::SendPacketAndWaitForResponse
     return response_len;
 }
 
-//template<typename _Tp>
-//class ScopedValueChanger
-//{
-//public:
-//    // Take a value reference and the value to assign it to when this class
-//    // instance goes out of scope.
-//    ScopedValueChanger (_Tp &value_ref, _Tp value) :
-//        m_value_ref (value_ref),
-//        m_value (value)
-//    {
-//    }
-//
-//    // This object is going out of scope, change the value pointed to by
-//    // m_value_ref to the value we got during construction which was stored in
-//    // m_value;
-//    ~ScopedValueChanger ()
-//    {
-//        m_value_ref = m_value;
-//    }
-//protected:
-//    _Tp &m_value_ref;   // A reference to the value we will change when this object destructs
-//    _Tp m_value;        // The value to assign to m_value_ref when this goes out of scope.
-//};
-
 StateType
 GDBRemoteCommunicationClient::SendContinuePacketAndWaitForResponse
 (
@@ -415,7 +391,7 @@ GDBRemoteCommunicationClient::SendContinuePacketAndWaitForResponse
         {
             if (log)
                 log->Printf ("GDBRemoteCommunicationClient::%s () sending continue packet: %s", __FUNCTION__, continue_packet.c_str());
-            if (SendPacket(continue_packet.c_str(), continue_packet.size()) == 0)
+            if (SendPacketNoLock(continue_packet.c_str(), continue_packet.size()) == 0)
                 state = eStateInvalid;
         
             m_private_is_running.SetValue (true, eBroadcastAlways);
@@ -426,7 +402,7 @@ GDBRemoteCommunicationClient::SendContinuePacketAndWaitForResponse
         if (log)
             log->Printf ("GDBRemoteCommunicationClient::%s () WaitForPacket(%s)", __FUNCTION__, continue_packet.c_str());
 
-        if (WaitForPacketWithTimeoutMicroSeconds (response, UINT32_MAX))
+        if (WaitForPacketWithTimeoutMicroSecondsNoLock(response, UINT32_MAX))
         {
             if (response.Empty())
                 state = eStateInvalid;
@@ -647,7 +623,7 @@ GDBRemoteCommunicationClient::SendAsyncSignal (int signo)
     return false;
 }
 
-// This function takes a mutex locker as a parameter in case the TryLockSequenceMutex
+// This function takes a mutex locker as a parameter in case the GetSequenceMutex
 // actually succeeds. If it doesn't succeed in acquiring the sequence mutex 
 // (the expected result), then it will send the halt packet. If it does succeed
 // then the caller that requested the interrupt will want to keep the sequence
@@ -672,7 +648,12 @@ GDBRemoteCommunicationClient::SendInterrupt
     if (IsRunning())
     {
         // Only send an interrupt if our debugserver is running...
-        if (TryLockSequenceMutex (locker) == false)
+        if (GetSequenceMutex (locker, 0))
+        {
+            if (log)
+                log->Printf ("SendInterrupt () - got sequence mutex without having to interrupt");
+        }
+        else
         {
             // Someone has the mutex locked waiting for a response or for the
             // inferior to stop, so send the interrupt on the down low...
@@ -718,11 +699,6 @@ GDBRemoteCommunicationClient::SendInterrupt
             }
             return false;
         }
-        else
-        {
-            if (log)
-                log->Printf ("SendInterrupt () - got sequence mutex without having to interrupt");
-        }
     }
     else
     {
@@ -1174,6 +1150,12 @@ GDBRemoteCommunicationClient::DeallocateMemory (addr_t addr)
     return false;
 }
 
+bool
+GDBRemoteCommunicationClient::Detach ()
+{
+    return SendPacket ("D", 1) > 0;
+}
+
 Error
 GDBRemoteCommunicationClient::GetMemoryRegionInfo (lldb::addr_t addr, 
                                                   lldb_private::MemoryRegionInfo &region_info)
@@ -1861,7 +1843,7 @@ GDBRemoteCommunicationClient::GetCurrentThreadIDs (std::vector<lldb::tid_t> &thr
     Mutex::Locker locker;
     thread_ids.clear();
     
-    if (TryLockSequenceMutex (locker))
+    if (GetSequenceMutex (locker, 0))
     {
         sequence_mutex_unavailable = false;
         StringExtractorGDBRemote response;
@@ -1894,3 +1876,19 @@ GDBRemoteCommunicationClient::GetCurrentThreadIDs (std::vector<lldb::tid_t> &thr
     }
     return thread_ids.size();
 }
+
+lldb::addr_t
+GDBRemoteCommunicationClient::GetShlibInfoAddr()
+{
+    if (!IsRunning())
+    {
+        StringExtractorGDBRemote response;
+        if (SendPacketAndWaitForResponse("qShlibInfoAddr", ::strlen ("qShlibInfoAddr"), response, false))
+        {
+            if (response.IsNormalResponse())
+                return response.GetHexMaxU64(false, LLDB_INVALID_ADDRESS);
+        }
+    }
+    return LLDB_INVALID_ADDRESS;
+}
+
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
index c7ac21d83ad..4794021908c 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -199,6 +199,9 @@ public:
     bool
     DeallocateMemory (lldb::addr_t addr);
 
+    bool
+    Detach ();
+
     lldb_private::Error
     GetMemoryRegionInfo (lldb::addr_t addr, 
                         lldb_private::MemoryRegionInfo &range_info); 
@@ -232,6 +235,9 @@ public:
     bool
     GetHostname (std::string &s);
 
+    lldb::addr_t
+    GetShlibInfoAddr();
+
     bool
     GetSupportsThreadSuffix ();
 
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp
index 3fc97295a80..3f8677d1c7e 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp
@@ -85,7 +85,7 @@ GDBRemoteCommunicationServer::GetPacketAndSendResponse (uint32_t timeout_usec,
                                                         bool &quit)
 {
     StringExtractorGDBRemote packet;
-    if (WaitForPacketWithTimeoutMicroSeconds(packet, timeout_usec))
+    if (WaitForPacketWithTimeoutMicroSecondsNoLock (packet, timeout_usec))
     {
         const StringExtractorGDBRemote::ServerPacketType packet_type = packet.GetServerPacketType ();
         switch (packet_type)
@@ -178,7 +178,7 @@ size_t
 GDBRemoteCommunicationServer::SendUnimplementedResponse (const char *)
 {
     // TODO: Log the packet we aren't handling...
-    return SendPacket ("");
+    return SendPacketNoLock ("", 0);
 }
 
 size_t
@@ -187,14 +187,14 @@ GDBRemoteCommunicationServer::SendErrorResponse (uint8_t err)
     char packet[16];
     int packet_len = ::snprintf (packet, sizeof(packet), "E%2.2x", err);
     assert (packet_len < sizeof(packet));
-    return SendPacket (packet, packet_len);
+    return SendPacketNoLock (packet, packet_len);
 }
 
 
 size_t
 GDBRemoteCommunicationServer::SendOKResponse ()
 {
-    return SendPacket ("OK");
+    return SendPacketNoLock ("OK", 2);
 }
 
 bool
@@ -269,7 +269,7 @@ GDBRemoteCommunicationServer::Handle_qHostInfo (StringExtractorGDBRemote &packet
         response.PutChar(';');
     }
     
-    return SendPacket (response) > 0;
+    return SendPacketNoLock (response.GetData(), response.GetSize()) > 0;
 }
 
 static void
@@ -308,7 +308,7 @@ GDBRemoteCommunicationServer::Handle_qProcessInfoPID (StringExtractorGDBRemote &
         {
             StreamString response;
             CreateProcessInfoResponse (proc_info, response);
-            return SendPacket (response);
+            return SendPacketNoLock (response.GetData(), response.GetSize());
         }
     }
     return SendErrorResponse (1);
@@ -423,7 +423,7 @@ GDBRemoteCommunicationServer::Handle_qsProcessInfo (StringExtractorGDBRemote &pa
         StreamString response;
         CreateProcessInfoResponse (m_proc_infos.GetProcessInfoAtIndex(m_proc_infos_index), response);
         ++m_proc_infos_index;
-        return SendPacket (response);
+        return SendPacketNoLock (response.GetData(), response.GetSize());
     }
     return SendErrorResponse (4);
 }
@@ -441,7 +441,7 @@ GDBRemoteCommunicationServer::Handle_qUserName (StringExtractorGDBRemote &packet
         {
             StreamString response;
             response.PutCStringAsRawHex8 (name.c_str());
-            return SendPacket (response);
+            return SendPacketNoLock (response.GetData(), response.GetSize());
         }
     }
     return SendErrorResponse (5);
@@ -461,7 +461,7 @@ GDBRemoteCommunicationServer::Handle_qGroupName (StringExtractorGDBRemote &packe
         {
             StreamString response;
             response.PutCStringAsRawHex8 (name.c_str());
-            return SendPacket (response);
+            return SendPacketNoLock (response.GetData(), response.GetSize());
         }
     }
     return SendErrorResponse (6);
@@ -498,7 +498,7 @@ GDBRemoteCommunicationServer::Handle_qSpeedTest (StringExtractorGDBRemote &packe
                     bytes_left = 0;
                 }
             }
-            return SendPacket (response);
+            return SendPacketNoLock (response.GetData(), response.GetSize());
         }
     }
     return SendErrorResponse (7);
@@ -657,7 +657,7 @@ GDBRemoteCommunicationServer::Handle_qC (StringExtractorGDBRemote &packet)
             m_process_launch_info.Clear();
         }
     }
-    return SendPacket (response);
+    return SendPacketNoLock (response.GetData(), response.GetSize());
 }
 
 bool
@@ -710,7 +710,7 @@ GDBRemoteCommunicationServer::Handle_qLaunchGDBServer (StringExtractorGDBRemote
                             const int response_len = ::snprintf (response, sizeof(response), "pid:%llu;port:%u;", debugserver_pid, port);
                             assert (response_len < sizeof(response));
                             //m_port_to_pid_map[port] = debugserver_launch_info.GetProcessID();
-                            success = SendPacket (response, response_len) > 0;
+                            success = SendPacketNoLock (response, response_len) > 0;
                         }
                     }
                     ::unlink (unix_socket_name);
@@ -736,7 +736,7 @@ GDBRemoteCommunicationServer::Handle_qLaunchSuccess (StringExtractorGDBRemote &p
     StreamString response;    
     response.PutChar('E');
     response.PutCString(m_process_launch_error.AsCString("<unknown error>"));
-    return SendPacket (response);
+    return SendPacketNoLock (response.GetData(), response.GetSize());
 }
 
 bool
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
index 120caf3d7a7..33b74b42085 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
@@ -181,7 +181,7 @@ GDBRemoteRegisterContext::ReadRegisterBytes (const RegisterInfo *reg_info, DataE
     if (!m_reg_valid[reg])
     {
         Mutex::Locker locker;
-        if (gdb_comm.TryLockSequenceMutex (locker))
+        if (gdb_comm.GetSequenceMutex (locker, 0))
         {
             const bool thread_suffix_supported = gdb_comm.GetThreadSuffixSupported();
             ProcessSP process_sp (m_thread.GetProcess());
@@ -331,7 +331,7 @@ GDBRemoteRegisterContext::WriteRegisterBytes (const lldb_private::RegisterInfo *
                                   m_reg_data.GetByteOrder()))   // dst byte order
     {
         Mutex::Locker locker;
-        if (gdb_comm.TryLockSequenceMutex (locker))
+        if (gdb_comm.GetSequenceMutex (locker, 0))
         {
             const bool thread_suffix_supported = gdb_comm.GetThreadSuffixSupported();
             ProcessSP process_sp (m_thread.GetProcess());
@@ -440,7 +440,7 @@ GDBRemoteRegisterContext::ReadAllRegisterValues (lldb::DataBufferSP &data_sp)
     StringExtractorGDBRemote response;
 
     Mutex::Locker locker;
-    if (gdb_comm.TryLockSequenceMutex (locker))
+    if (gdb_comm.GetSequenceMutex (locker, 0))
     {
         char packet[32];
         const bool thread_suffix_supported = gdb_comm.GetThreadSuffixSupported();
@@ -496,7 +496,7 @@ GDBRemoteRegisterContext::WriteAllRegisterValues (const lldb::DataBufferSP &data
 
     StringExtractorGDBRemote response;
     Mutex::Locker locker;
-    if (gdb_comm.TryLockSequenceMutex (locker))
+    if (gdb_comm.GetSequenceMutex (locker, 0))
     {
         const bool thread_suffix_supported = gdb_comm.GetThreadSuffixSupported();
         ProcessSP process_sp (m_thread.GetProcess());
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 035c6128db9..9b875012a6a 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1621,10 +1621,10 @@ ProcessGDBRemote::DoDetach()
 
     m_thread_list.DiscardThreadPlans();
 
-    size_t response_size = m_gdb_comm.SendPacket ("D", 1);
+    bool success = m_gdb_comm.Detach ();
     if (log)
     {
-        if (response_size)
+        if (success)
             log->PutCString ("ProcessGDBRemote::DoDetach() detach packet sent successfully");
         else
             log->PutCString ("ProcessGDBRemote::DoDetach() detach packet send failed");
@@ -1691,16 +1691,7 @@ ProcessGDBRemote::IsAlive ()
 addr_t
 ProcessGDBRemote::GetImageInfoAddress()
 {
-    if (!m_gdb_comm.IsRunning())
-    {
-        StringExtractorGDBRemote response;
-        if (m_gdb_comm.SendPacketAndWaitForResponse("qShlibInfoAddr", ::strlen ("qShlibInfoAddr"), response, false))
-        {
-            if (response.IsNormalResponse())
-                return response.GetHexMaxU64(false, LLDB_INVALID_ADDRESS);
-        }
-    }
-    return LLDB_INVALID_ADDRESS;
+    return m_gdb_comm.GetShlibInfoAddr();
 }
 
 //------------------------------------------------------------------