Add a call to "sync" a thread state before checkpointing registers in preparation for

calling functions.  This is necessary on Mac OS X, since bad things can happen if you set
the registers of a thread that's sitting in a kernel trap.

<rdar://problem/11145013>

llvm-svn: 160756

12 files changed, 130 insertions, 4 deletions

diff --git a/lldb/include/lldb/Target/RegisterContext.h b/lldb/include/lldb/Target/RegisterContext.h
index 7c77146f492..aa2e1e76d2b 100644
--- a/lldb/include/lldb/Target/RegisterContext.h
+++ b/lldb/include/lldb/Target/RegisterContext.h
@@ -59,6 +59,13 @@ public:
     virtual bool
     WriteRegister (const RegisterInfo *reg_info, const RegisterValue &reg_value) = 0;
     
+    // These two functions are used to implement "push" and "pop" of register states.  They are used primarily
+    // for expression evaluation, where we need to push a new state (storing the old one in data_sp) and then
+    // restoring the original state by passing the data_sp we got from ReadAllRegisters to WriteAllRegisterValues.
+    // ReadAllRegisters will do what is necessary to return a coherent set of register values for this thread, which
+    // may mean e.g. interrupting a thread that is sitting in a kernel trap.  That is a somewhat disruptive operation,
+    // so these API's should only be used when this behavior is needed.
+    
     virtual bool
     ReadAllRegisterValues (lldb::DataBufferSP &data_sp) = 0;
 
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index b4263d18ca6..8225e69f373 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -51,6 +51,7 @@ GDBRemoteCommunicationClient::GDBRemoteCommunicationClient(bool is_platform) :
     m_supports_watchpoint_support_info  (eLazyBoolCalculate),
     m_watchpoints_trigger_after_instruction(eLazyBoolCalculate),
     m_attach_or_wait_reply(eLazyBoolCalculate),
+    m_prepare_for_reg_writing_reply (eLazyBoolCalculate),
     m_supports_qProcessInfoPID (true),
     m_supports_qfProcessInfo (true),
     m_supports_qUserName (true),
@@ -154,6 +155,26 @@ GDBRemoteCommunicationClient::GetVAttachOrWaitSupported ()
         return false;
 }
 
+bool
+GDBRemoteCommunicationClient::GetSyncThreadStateSupported ()
+{
+    if (m_prepare_for_reg_writing_reply == eLazyBoolCalculate)
+    {
+        m_prepare_for_reg_writing_reply = eLazyBoolNo;
+        
+        StringExtractorGDBRemote response;
+        if (SendPacketAndWaitForResponse("qSyncThreadStateSupported", response, false))
+        {
+            if (response.IsOKResponse())
+                m_prepare_for_reg_writing_reply = eLazyBoolYes;
+        }
+    }
+    if (m_prepare_for_reg_writing_reply == eLazyBoolYes)
+        return true;
+    else
+        return false;
+}
+
 
 void
 GDBRemoteCommunicationClient::ResetDiscoverableSettings()
@@ -168,6 +189,8 @@ GDBRemoteCommunicationClient::ResetDiscoverableSettings()
     m_qHostInfo_is_valid = eLazyBoolCalculate;
     m_supports_alloc_dealloc_memory = eLazyBoolCalculate;
     m_supports_memory_region_info = eLazyBoolCalculate;
+    m_prepare_for_reg_writing_reply = eLazyBoolCalculate;
+    m_attach_or_wait_reply = eLazyBoolCalculate;
 
     m_supports_qProcessInfoPID = true;
     m_supports_qfProcessInfo = true;
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
index eee3aa50ed1..adfec5b6b04 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -224,6 +224,9 @@ public:
     bool
     GetVAttachOrWaitSupported ();
     
+    bool
+    GetSyncThreadStateSupported();
+    
     void
     ResetDiscoverableSettings();
 
@@ -369,6 +372,7 @@ protected:
     lldb_private::LazyBool m_supports_watchpoint_support_info;
     lldb_private::LazyBool m_watchpoints_trigger_after_instruction;
     lldb_private::LazyBool m_attach_or_wait_reply;
+    lldb_private::LazyBool m_prepare_for_reg_writing_reply;
     
     bool
         m_supports_qProcessInfoPID:1,
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
index 4247add8554..a1cac219051 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
@@ -286,7 +286,6 @@ GDBRemoteRegisterContext::ReadRegisterBytes (const RegisterInfo *reg_info, DataE
     return true;
 }
 
-
 bool
 GDBRemoteRegisterContext::WriteRegister (const RegisterInfo *reg_info,
                                          const RegisterValue &value)
@@ -326,6 +325,29 @@ GDBRemoteRegisterContext::SetPrimordialRegister(const lldb_private::RegisterInfo
     }
     return false;
 }
+
+void
+GDBRemoteRegisterContext::SyncThreadState(Process *process)
+{
+    // NB.  We assume our caller has locked the sequence mutex.
+    
+    GDBRemoteCommunicationClient &gdb_comm (((ProcessGDBRemote *) process)->GetGDBRemote());
+    if (!gdb_comm.GetSyncThreadStateSupported())
+        return;
+
+    StreamString packet;
+    StringExtractorGDBRemote response;
+    packet.Printf ("QSyncThreadState:%4.4llx;", m_thread.GetID());
+    if (gdb_comm.SendPacketAndWaitForResponse(packet.GetString().c_str(),
+                                              packet.GetString().size(),
+                                              response,
+                                              false))
+    {
+        if (response.IsOKResponse())
+            InvalidateAllRegisters();
+    }
+}
+
 bool
 GDBRemoteRegisterContext::WriteRegisterBytes (const lldb_private::RegisterInfo *reg_info, DataExtractor &data, uint32_t data_offset)
 {
@@ -479,6 +501,8 @@ GDBRemoteRegisterContext::ReadAllRegisterValues (lldb::DataBufferSP &data_sp)
     Mutex::Locker locker;
     if (gdb_comm.GetSequenceMutex (locker, "Didn't get sequence mutex for read all registers."))
     {
+        SyncThreadState(process);
+        
         char packet[32];
         const bool thread_suffix_supported = gdb_comm.GetThreadSuffixSupported();
         ProcessSP process_sp (m_thread.GetProcess());
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
index e39815a527a..e3141761c9e 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
@@ -243,6 +243,9 @@ protected:
     void
     SetAllRegisterValid (bool b);
 
+    void
+    SyncThreadState(lldb_private::Process *process);  // Assumes the sequence mutex has already been acquired.
+    
     GDBRemoteDynamicRegisterInfo &m_reg_info;
     std::vector<bool> m_reg_valid;
     lldb_private::DataExtractor m_reg_data;
diff --git a/lldb/tools/debugserver/source/DNB.cpp b/lldb/tools/debugserver/source/DNB.cpp
index 6de893d0396..35ebda146ae 100644
--- a/lldb/tools/debugserver/source/DNB.cpp
+++ b/lldb/tools/debugserver/source/DNB.cpp
@@ -1797,6 +1797,19 @@ DNBProcessGetThreadAtIndex (nub_process_t pid, size_t thread_idx)
     return INVALID_NUB_THREAD;
 }
 
+//----------------------------------------------------------------------
+// Do whatever is needed to sync the thread's register state with it's kernel values.
+//----------------------------------------------------------------------
+nub_bool_t
+DNBProcessSyncThreadState (nub_process_t pid, nub_thread_t tid)
+{
+    MachProcessSP procSP;
+    if (GetProcessSP (pid, procSP))
+        return procSP->SyncThreadState (tid);
+    return false;
+
+}
+
 nub_addr_t
 DNBProcessGetSharedLibraryInfoAddress (nub_process_t pid)
 {
diff --git a/lldb/tools/debugserver/source/DNB.h b/lldb/tools/debugserver/source/DNB.h
index d2068260d46..d3943977b37 100644
--- a/lldb/tools/debugserver/source/DNB.h
+++ b/lldb/tools/debugserver/source/DNB.h
@@ -79,6 +79,7 @@ nub_size_t      DNBProcessGetNumThreads                 (nub_process_t pid) DNB_
 nub_thread_t    DNBProcessGetCurrentThread              (nub_process_t pid) DNB_EXPORT;
 nub_thread_t    DNBProcessSetCurrentThread              (nub_process_t pid, nub_thread_t tid) DNB_EXPORT;
 nub_thread_t    DNBProcessGetThreadAtIndex              (nub_process_t pid, nub_size_t thread_idx) DNB_EXPORT;
+nub_bool_t      DNBProcessSyncThreadState               (nub_process_t pid, nub_thread_t tid) DNB_EXPORT;
 nub_addr_t      DNBProcessGetSharedLibraryInfoAddress   (nub_process_t pid) DNB_EXPORT;
 nub_bool_t      DNBProcessSharedLibrariesUpdated        (nub_process_t pid) DNB_EXPORT;
 nub_size_t      DNBProcessGetSharedLibraryInfo          (nub_process_t pid, nub_bool_t only_changed, DNBExecutableImageInfo **image_infos) DNB_EXPORT;
diff --git a/lldb/tools/debugserver/source/MacOSX/MachProcess.cpp b/lldb/tools/debugserver/source/MacOSX/MachProcess.cpp
index cac5143f283..05a3774c94f 100644
--- a/lldb/tools/debugserver/source/MacOSX/MachProcess.cpp
+++ b/lldb/tools/debugserver/source/MacOSX/MachProcess.cpp
@@ -151,6 +151,22 @@ MachProcess::GetThreadAtIndex (nub_size_t thread_idx) const
     return m_thread_list.ThreadIDAtIndex(thread_idx);
 }
 
+nub_bool_t
+MachProcess::SyncThreadState (nub_thread_t tid)
+{
+    MachThreadSP thread_sp(m_thread_list.GetThreadByID(tid));
+    if (!thread_sp)
+        return false;
+    kern_return_t kret = ::thread_abort_safely(thread_sp->ThreadID());
+    DNBLogThreadedIf (LOG_THREAD, "thread = 0x%4.4x calling thread_abort_safely (tid) => %u (GetGPRState() for stop_count = %u)", thread_sp->ThreadID(), kret, thread_sp->Process()->StopCount());
+
+    if (kret == KERN_SUCCESS)
+        return true;
+    else
+        return false;
+    
+}
+
 nub_thread_t
 MachProcess::GetCurrentThread ()
 {
diff --git a/lldb/tools/debugserver/source/MacOSX/MachProcess.h b/lldb/tools/debugserver/source/MacOSX/MachProcess.h
index 899109d1b7d..77553151025 100644
--- a/lldb/tools/debugserver/source/MacOSX/MachProcess.h
+++ b/lldb/tools/debugserver/source/MacOSX/MachProcess.h
@@ -159,6 +159,7 @@ public:
                             GetRegisterSetInfo (nub_thread_t tid, nub_size_t *num_reg_sets) const;
     bool                    GetRegisterValue (nub_thread_t tid, uint32_t set, uint32_t reg, DNBRegisterValue *reg_value) const;
     bool                    SetRegisterValue (nub_thread_t tid, uint32_t set, uint32_t reg, const DNBRegisterValue *value) const;
+    nub_bool_t              SyncThreadState (nub_thread_t tid);
     const char *            ThreadGetName (nub_thread_t tid);
     nub_state_t             ThreadGetState (nub_thread_t tid);
     nub_size_t              GetNumThreads () const;
diff --git a/lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp b/lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
index f167e1dd625..a9cb650e661 100644
--- a/lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
+++ b/lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
@@ -155,9 +155,6 @@ DNBArchImplX86_64::GetGPRState(bool force)
 {
     if (force || m_state.GetError(e_regSetGPR, Read))
     {
-        kern_return_t kret = ::thread_abort_safely(m_thread->ThreadID());
-        DNBLogThreadedIf (LOG_THREAD, "thread = 0x%4.4x calling thread_abort_safely (tid) => %u (GetGPRState() for stop_count = %u)", m_thread->ThreadID(), kret, m_thread->Process()->StopCount());    
-
 #if DEBUG_GPR_VALUES
         m_state.context.gpr.__rax = ('a' << 8) + 'x';
         m_state.context.gpr.__rbx = ('b' << 8) + 'x';
diff --git a/lldb/tools/debugserver/source/RNBRemote.cpp b/lldb/tools/debugserver/source/RNBRemote.cpp
index eaad18b4164..7f7e42ba0f5 100644
--- a/lldb/tools/debugserver/source/RNBRemote.cpp
+++ b/lldb/tools/debugserver/source/RNBRemote.cpp
@@ -171,6 +171,7 @@ RNBRemote::CreatePacketTable  ()
     t.push_back (Packet (query_shlib_notify_info_addr,  &RNBRemote::HandlePacket_qShlibInfoAddr,NULL, "qShlibInfoAddr", "Returns the address that contains info needed for getting shared library notifications"));
     t.push_back (Packet (query_step_packet_supported,   &RNBRemote::HandlePacket_qStepPacketSupported,NULL, "qStepPacketSupported", "Replys with OK if the 's' packet is supported."));
     t.push_back (Packet (query_vattachorwait_supported, &RNBRemote::HandlePacket_qVAttachOrWaitSupported,NULL, "qVAttachOrWaitSupported", "Replys with OK if the 'vAttachOrWait' packet is supported."));
+    t.push_back (Packet (query_sync_thread_state_supported, &RNBRemote::HandlePacket_qSyncThreadStateSupported,NULL, "qSyncThreadStateSupported", "Replys with OK if the 'QSyncThreadState:' packet is supported."));
     t.push_back (Packet (query_host_info,               &RNBRemote::HandlePacket_qHostInfo,     NULL, "qHostInfo", "Replies with multiple 'key:value;' tuples appended to each other."));
 //  t.push_back (Packet (query_symbol_lookup,           &RNBRemote::HandlePacket_UNIMPLEMENTED, NULL, "qSymbol", "Notify that host debugger is ready to do symbol lookups"));
     t.push_back (Packet (start_noack_mode,              &RNBRemote::HandlePacket_QStartNoAckMode        , NULL, "QStartNoAckMode", "Request that " DEBUGSERVER_PROGRAM_NAME " stop acking remote protocol packets"));
@@ -187,6 +188,7 @@ RNBRemote::CreatePacketTable  ()
     t.push_back (Packet (set_stderr,                    &RNBRemote::HandlePacket_QSetSTDIO              , NULL, "QSetSTDERR:", "Set the standard error for a process to be launched with the 'A' packet"));
     t.push_back (Packet (set_working_dir,               &RNBRemote::HandlePacket_QSetWorkingDir         , NULL, "QSetWorkingDir:", "Set the working directory for a process to be launched with the 'A' packet"));
     t.push_back (Packet (set_list_threads_in_stop_reply,&RNBRemote::HandlePacket_QListThreadsInStopReply , NULL, "QListThreadsInStopReply", "Set if the 'threads' key should be added to the stop reply packets with a list of all thread IDs."));
+    t.push_back (Packet (sync_thread_state,             &RNBRemote::HandlePacket_QSyncThreadState , NULL, "QSyncThreadState:", "Do whatever is necessary to make sure 'thread' is in a safe state to call functions on."));
 //  t.push_back (Packet (pass_signals_to_inferior,      &RNBRemote::HandlePacket_UNIMPLEMENTED, NULL, "QPassSignals:", "Specify which signals are passed to the inferior"));
     t.push_back (Packet (allocate_memory,               &RNBRemote::HandlePacket_AllocateMemory, NULL, "_M", "Allocate memory in the inferior process."));
     t.push_back (Packet (deallocate_memory,             &RNBRemote::HandlePacket_DeallocateMemory, NULL, "_m", "Deallocate memory in the inferior process."));
@@ -1304,6 +1306,13 @@ RNBRemote::HandlePacket_qStepPacketSupported (const char *p)
 }
 
 rnb_err_t
+RNBRemote::HandlePacket_qSyncThreadStateSupported (const char *p)
+{
+    // We support attachOrWait meaning attach if the process exists, otherwise wait to attach.
+    return SendPacket("OK");
+}
+
+rnb_err_t
 RNBRemote::HandlePacket_qVAttachOrWaitSupported (const char *p)
 {
     // We support attachOrWait meaning attach if the process exists, otherwise wait to attach.
@@ -1881,6 +1890,30 @@ RNBRemote::HandlePacket_QSetWorkingDir (const char *p)
 }
 
 rnb_err_t
+RNBRemote::HandlePacket_QSyncThreadState (const char *p)
+{
+    if (!m_ctx.HasValidProcessID())
+    {
+        // We allow gdb to connect to a server that hasn't started running
+        // the target yet.  gdb still wants to ask questions about it and
+        // freaks out if it gets an error.  So just return OK here.
+        return SendPacket ("OK");
+    }
+
+    errno = 0;
+    p += strlen("QSyncThreadState:");
+    nub_thread_t tid = strtoul (p, NULL, 16);
+    if (errno != 0 && tid == 0)
+    {
+        return HandlePacket_ILLFORMED (__FILE__, __LINE__, p, "Invalid thread number in QSyncThreadState packet");
+    }
+    if (DNBProcessSyncThreadState(m_ctx.ProcessID(), tid))
+        return SendPacket("OK");
+    else
+        return SendPacket ("E61");
+}
+
+rnb_err_t
 RNBRemote::HandlePacket_QListThreadsInStopReply (const char *p)
 {
     // If this packet is received, it allows us to send an extra key/value
diff --git a/lldb/tools/debugserver/source/RNBRemote.h b/lldb/tools/debugserver/source/RNBRemote.h
index b10449a850a..1b1b0a6c58c 100644
--- a/lldb/tools/debugserver/source/RNBRemote.h
+++ b/lldb/tools/debugserver/source/RNBRemote.h
@@ -93,6 +93,7 @@ public:
         query_shlib_notify_info_addr,   // 'qShlibInfoAddr'
         query_step_packet_supported,    // 'qStepPacketSupported'
         query_vattachorwait_supported,  // 'qVAttachOrWaitSupported'
+        query_sync_thread_state_supported,// 'QSyncThreadState'
         query_host_info,                // 'qHostInfo'
         pass_signals_to_inferior,       // 'QPassSignals'
         start_noack_mode,               // 'QStartNoAckMode'
@@ -109,6 +110,7 @@ public:
         set_stderr,                     // 'QSetSTDERR:'
         set_working_dir,                // 'QSetWorkingDir:'
         set_list_threads_in_stop_reply, // 'QListThreadsInStopReply:'
+        sync_thread_state,              // 'QSyncThreadState:'
         memory_region_info,             // 'qMemoryRegionInfo:'
         watchpoint_support_info,        // 'qWatchpointSupportInfo:'
         allocate_memory,                // '_M'
@@ -167,6 +169,7 @@ public:
     rnb_err_t HandlePacket_qShlibInfoAddr (const char *p);
     rnb_err_t HandlePacket_qStepPacketSupported (const char *p);
     rnb_err_t HandlePacket_qVAttachOrWaitSupported (const char *p);
+    rnb_err_t HandlePacket_qSyncThreadStateSupported (const char *p);
     rnb_err_t HandlePacket_qThreadInfo (const char *p);
     rnb_err_t HandlePacket_qThreadExtraInfo (const char *p);
     rnb_err_t HandlePacket_qThreadStopInfo (const char *p);
@@ -183,6 +186,7 @@ public:
     rnb_err_t HandlePacket_QEnvironmentHexEncoded (const char *p);
     rnb_err_t HandlePacket_QLaunchArch (const char *p);
     rnb_err_t HandlePacket_QListThreadsInStopReply (const char *p);
+    rnb_err_t HandlePacket_QSyncThreadState (const char *p);
     rnb_err_t HandlePacket_QPrefixRegisterPacketsWithThreadID (const char *p);
     rnb_err_t HandlePacket_last_signal (const char *p);
     rnb_err_t HandlePacket_m (const char *p);