Thread safety changes in debugserver and also in the process GDB remote plugin.

I added support for asking if the GDB remote server supports thread suffixes
for packets that should be thread specific (register read/write packets) because
the way the GDB remote protocol does it right now is to have a notion of a
current thread for register and memory reads/writes (set via the "$Hg%x" packet)
and a current thread for running ("$Hc%x"). Now we ask the remote GDB server
if it supports adding the thread ID to the register packets and we enable
that feature in LLDB if supported. This stops us from having to send a bunch
of packets that update the current thread ID to some value which is prone to
error, or extra packets.

llvm-svn: 123762

8 files changed, 92 insertions, 14 deletions

diff --git a/lldb/source/Expression/ClangUserExpression.cpp b/lldb/source/Expression/ClangUserExpression.cpp
index 13d5ab9c96b..c435136e2e6 100644
--- a/lldb/source/Expression/ClangUserExpression.cpp
+++ b/lldb/source/Expression/ClangUserExpression.cpp
@@ -20,6 +20,7 @@
 
 #include "lldb/Core/ConstString.h"
 #include "lldb/Core/Log.h"
+#include "lldb/Core/StreamFile.h"
 #include "lldb/Core/StreamString.h"
 #include "lldb/Core/ValueObjectConstResult.h"
 #include "lldb/Expression/ASTResultSynthesizer.h"
@@ -365,6 +366,12 @@ ClangUserExpression::PrepareToExecuteJITExpression (Stream &error_stream,
             error_stream.Printf("Couldn't materialize struct: %s\n", materialize_error.AsCString());
             return false;
         }
+
+#if 0
+		// jingham: look here
+        StreamFile logfile ("/tmp/exprs.txt", "a");
+        logfile.Printf("0x%16.16llx: thread = 0x%4.4x, expr = '%s'\n", m_jit_addr, exe_ctx.thread ? exe_ctx.thread->GetID() : -1, m_expr_text.c_str());
+#endif
         
         if (log)
         {
diff --git a/lldb/source/Expression/ClangUtilityFunction.cpp b/lldb/source/Expression/ClangUtilityFunction.cpp
index f682e360778..d3ede6b4a34 100644
--- a/lldb/source/Expression/ClangUtilityFunction.cpp
+++ b/lldb/source/Expression/ClangUtilityFunction.cpp
@@ -17,6 +17,7 @@
 
 #include "lldb/Core/ConstString.h"
 #include "lldb/Core/Stream.h"
+#include "lldb/Core/StreamFile.h"
 #include "lldb/Expression/ClangExpressionDeclMap.h"
 #include "lldb/Expression/ClangExpressionParser.h"
 #include "lldb/Expression/ClangUtilityFunction.h"
@@ -124,6 +125,15 @@ ClangUtilityFunction::Install (Stream &error_stream,
         
     Error jit_error = parser.MakeJIT (m_jit_begin, m_jit_end, exe_ctx);
     
+#if 0
+	// jingham: look here
+    StreamFile logfile ("/tmp/exprs.txt", "a");
+    logfile.Printf ("0x%16.16llx: func = %s, source =\n%s\n", 
+                    m_jit_begin, 
+                    m_function_name.c_str(), 
+                    m_function_text.c_str());
+#endif
+
     m_expr_decl_map->DidParse();
     
     m_expr_decl_map.reset();
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
index 5925812f7f3..1a7b27b40f9 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -34,6 +34,7 @@ using namespace lldb_private;
 GDBRemoteCommunication::GDBRemoteCommunication() :
     Communication("gdb-remote.packets"),
     m_send_acks (true),
+    m_thread_suffix_supported (false),
     m_rx_packet_listener ("gdbremote.rx_packet"),
     m_sequence_mutex (Mutex::eMutexTypeRecursive),
     m_is_running (false),
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
index 7bc795dcf7a..c0d8685db39 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
@@ -100,6 +100,18 @@ public:
     }
 
     bool
+    GetThreadSuffixSupported () const
+    {
+        return m_thread_suffix_supported;
+    }
+    
+    void
+    SetThreadSuffixSupported (bool enabled)
+    {
+        m_thread_suffix_supported = enabled;
+    }
+
+    bool
     SendAsyncSignal (int signo);
 
     bool
@@ -244,7 +256,8 @@ protected:
     //------------------------------------------------------------------
     // Classes that inherit from GDBRemoteCommunication can see and modify these
     //------------------------------------------------------------------
-    bool m_send_acks;
+    bool m_send_acks:1,
+         m_thread_suffix_supported:1;
     lldb_private::Listener m_rx_packet_listener;
     lldb_private::Mutex m_sequence_mutex;    // Restrict access to sending/receiving packets to a single thread at a time
     lldb_private::Predicate<bool> m_is_running;
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
index eaf91eb98b1..2c00b9a8431 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
@@ -217,15 +217,19 @@ GDBRemoteRegisterContext::ReadRegisterBytes (uint32_t reg, DataExtractor &data)
         Mutex::Locker locker;
         if (gdb_comm.GetSequenceMutex (locker))
         {
-            if (GetGDBProcess().SetCurrentGDBRemoteThread(m_thread.GetID()))
+            const bool thread_suffix_supported = gdb_comm.GetThreadSuffixSupported();
+            if (thread_suffix_supported || GetGDBProcess().SetCurrentGDBRemoteThread(m_thread.GetID()))
             {
-                char packet[32];
+                char packet[64];
                 StringExtractorGDBRemote response;
-                int packet_len;
+                int packet_len = 0;
                 if (m_read_all_at_once)
                 {
                     // Get all registers in one packet
-                    packet_len = ::snprintf (packet, sizeof(packet), "g");
+                    if (thread_suffix_supported)
+                        packet_len = ::snprintf (packet, sizeof(packet), "g;thread:%4.4x;", m_thread.GetID());
+                    else
+                        packet_len = ::snprintf (packet, sizeof(packet), "g");
                     assert (packet_len < (sizeof(packet) - 1));
                     if (gdb_comm.SendPacketAndWaitForResponse(packet, response, 1, false))
                     {
@@ -237,7 +241,10 @@ GDBRemoteRegisterContext::ReadRegisterBytes (uint32_t reg, DataExtractor &data)
                 else
                 {
                     // Get each register individually
-                    packet_len = ::snprintf (packet, sizeof(packet), "p%x", reg);
+                    if (thread_suffix_supported)
+                        packet_len = ::snprintf (packet, sizeof(packet), "p%x;thread:%4.4x;", reg, m_thread.GetID());
+                    else
+                        packet_len = ::snprintf (packet, sizeof(packet), "p%x", reg);
                     assert (packet_len < (sizeof(packet) - 1));
                     if (gdb_comm.SendPacketAndWaitForResponse(packet, response, 1, false))
                         PrivateSetRegisterValue (reg, response);
@@ -319,7 +326,8 @@ GDBRemoteRegisterContext::WriteRegisterBytes (uint32_t reg, DataExtractor &data,
         Mutex::Locker locker;
         if (gdb_comm.GetSequenceMutex (locker))
         {
-            if (GetGDBProcess().SetCurrentGDBRemoteThread(m_thread.GetID()))
+            const bool thread_suffix_supported = gdb_comm.GetThreadSuffixSupported();
+            if (thread_suffix_supported || GetGDBProcess().SetCurrentGDBRemoteThread(m_thread.GetID()))
             {
                 uint32_t offset, end_offset;
                 StreamString packet;
@@ -336,6 +344,9 @@ GDBRemoteRegisterContext::WriteRegisterBytes (uint32_t reg, DataExtractor &data,
                                               eByteOrderHost,
                                               eByteOrderHost);
                     
+                    if (thread_suffix_supported)
+                        packet.Printf (";thread:%4.4x;", m_thread.GetID());
+
                     // Invalidate all register values
                     InvalidateIfNeeded (true);
 
@@ -361,6 +372,9 @@ GDBRemoteRegisterContext::WriteRegisterBytes (uint32_t reg, DataExtractor &data,
                                               eByteOrderHost,
                                               eByteOrderHost);
 
+                    if (thread_suffix_supported)
+                        packet.Printf (";thread:%4.4x;", m_thread.GetID());
+
                     // Invalidate just this register
                     m_reg_valid[reg] = false;
                     if (gdb_comm.SendPacketAndWaitForResponse(packet.GetString().c_str(),
@@ -391,16 +405,31 @@ GDBRemoteRegisterContext::ReadAllRegisterValues (lldb::DataBufferSP &data_sp)
     Mutex::Locker locker;
     if (gdb_comm.GetSequenceMutex (locker))
     {
-        if (GetGDBProcess().SetCurrentGDBRemoteThread(m_thread.GetID()))
+        char packet[32];
+        const bool thread_suffix_supported = gdb_comm.GetThreadSuffixSupported();
+        if (thread_suffix_supported || GetGDBProcess().SetCurrentGDBRemoteThread(m_thread.GetID()))
         {
-            if (gdb_comm.SendPacketAndWaitForResponse("g", response, 1, false))
+            int packet_len = 0;
+            if (thread_suffix_supported)
+                packet_len = ::snprintf (packet, sizeof(packet), "g;thread:%4.4x", m_thread.GetID());
+            else
+                packet_len = ::snprintf (packet, sizeof(packet), "g");
+            assert (packet_len < (sizeof(packet) - 1));
+
+            if (gdb_comm.SendPacketAndWaitForResponse(packet, packet_len, response, 1, false))
             {
                 if (response.IsErrorPacket())
                     return false;
-                    
+                
                 response.GetStringRef().insert(0, 1, 'G');
-                data_sp.reset (new DataBufferHeap(response.GetStringRef().c_str(), 
-                                                  response.GetStringRef().size()));
+                if (thread_suffix_supported)
+                {
+                    char thread_id_cstr[64];
+                    ::snprintf (thread_id_cstr, sizeof(thread_id_cstr), ";thread:%4.4x;", m_thread.GetID());
+                    response.GetStringRef().append (thread_id_cstr);
+                }
+                data_sp.reset (new DataBufferHeap (response.GetStringRef().c_str(), 
+                                                   response.GetStringRef().size()));
                 return true;
             }
         }
@@ -419,7 +448,8 @@ GDBRemoteRegisterContext::WriteAllRegisterValues (const lldb::DataBufferSP &data
     Mutex::Locker locker;
     if (gdb_comm.GetSequenceMutex (locker))
     {
-        if (GetGDBProcess().SetCurrentGDBRemoteThread(m_thread.GetID()))
+        const bool thread_suffix_supported = gdb_comm.GetThreadSuffixSupported();
+        if (thread_suffix_supported || GetGDBProcess().SetCurrentGDBRemoteThread(m_thread.GetID()))
         {
             if (gdb_comm.SendPacketAndWaitForResponse((const char *)data_sp->GetBytes(), 
                                                       data_sp->GetByteSize(), 
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 80580f8dddc..26aa4564281 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -554,6 +554,13 @@ ProcessGDBRemote::ConnectToDebugserver (const char *host_port)
             if (response.IsOKPacket())
                 m_gdb_comm.SetAckMode (false);
         }
+
+        if (m_gdb_comm.SendPacketAndWaitForResponse("QThreadSuffixSupported", response, 1, false))
+        {
+            if (response.IsOKPacket())
+                m_gdb_comm.SetThreadSuffixSupported (true);
+        }
+
     }
     return error;
 }
@@ -2021,7 +2028,7 @@ ProcessGDBRemote::SetCurrentGDBRemoteThreadForRun (int tid)
         return true;
 
     char packet[32];
-    const int packet_len = ::snprintf (packet, sizeof(packet), "Hg%x", tid);
+    const int packet_len = ::snprintf (packet, sizeof(packet), "Hc%x", tid);
     assert (packet_len + 1 < sizeof(packet));
     StringExtractorGDBRemote response;
     if (m_gdb_comm.SendPacketAndWaitForResponse(packet, packet_len, response, 2, false))
diff --git a/lldb/source/Target/ThreadList.cpp b/lldb/source/Target/ThreadList.cpp
index a4004ea9ee2..e5056326b1c 100644
--- a/lldb/source/Target/ThreadList.cpp
+++ b/lldb/source/Target/ThreadList.cpp
@@ -340,6 +340,7 @@ ThreadList::ShouldReportRun (Event *event_ptr)
 void
 ThreadList::Clear()
 {
+    Mutex::Locker locker(m_threads_mutex);
     m_stop_id = 0;
     m_threads.clear();
     m_selected_tid = LLDB_INVALID_THREAD_ID;
@@ -504,6 +505,7 @@ ThreadList::WillResume ()
 void
 ThreadList::DidResume ()
 {
+    Mutex::Locker locker(m_threads_mutex);
     collection::iterator pos, end = m_threads.end();
     for (pos = m_threads.begin(); pos != end; ++pos)
     {
diff --git a/lldb/source/Utility/StringExtractor.h b/lldb/source/Utility/StringExtractor.h
index ed67b926c2a..bb65f707f7d 100644
--- a/lldb/source/Utility/StringExtractor.h
+++ b/lldb/source/Utility/StringExtractor.h
@@ -113,6 +113,14 @@ public:
     size_t
     GetHexByteString (std::string &str);
 
+    const char *
+    Peek ()
+    {
+        if (m_index < m_packet.size())
+            return m_packet.c_str() + m_index;
+        return NULL;
+    }
+
 protected:
     //------------------------------------------------------------------
     // For StringExtractor only