[Lldb-commits] [lldb] r207139 - Fixed an issue where we would try to interrupt a process while it is in the process of naturally stopping due to another reason (breakpoint, or step).

Greg Clayton gclayton at apple.com
Thu Apr 24 12:54:32 PDT 2014


Author: gclayton
Date: Thu Apr 24 14:54:32 2014
New Revision: 207139

URL: http://llvm.org/viewvc/llvm-project?rev=207139&view=rev
Log:
Fixed an issue where we would try to interrupt a process while it is in the process of naturally stopping due to another reason (breakpoint, or step).

Added a new MachProcess::Interrupt() which correctly tracks such cases and "does the right thing".

<rdar://problem/16593556>


Modified:
    lldb/trunk/tools/debugserver/source/DNB.cpp
    lldb/trunk/tools/debugserver/source/DNB.h
    lldb/trunk/tools/debugserver/source/MacOSX/MachProcess.h
    lldb/trunk/tools/debugserver/source/MacOSX/MachProcess.mm
    lldb/trunk/tools/debugserver/source/RNBRemote.cpp

Modified: lldb/trunk/tools/debugserver/source/DNB.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/DNB.cpp?rev=207139&r1=207138&r2=207139&view=diff
==============================================================================
--- lldb/trunk/tools/debugserver/source/DNB.cpp (original)
+++ lldb/trunk/tools/debugserver/source/DNB.cpp Thu Apr 24 14:54:32 2014
@@ -787,6 +787,16 @@ DNBProcessSignal (nub_process_t pid, int
     return false;
 }
 
+
+nub_bool_t
+DNBProcessInterrupt(nub_process_t pid)
+{
+    MachProcessSP procSP;
+    if (GetProcessSP (pid, procSP))
+        return procSP->Interrupt();
+    return false;
+}
+
 nub_bool_t
 DNBProcessSendEvent (nub_process_t pid, const char *event)
 {

Modified: lldb/trunk/tools/debugserver/source/DNB.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/DNB.h?rev=207139&r1=207138&r2=207139&view=diff
==============================================================================
--- lldb/trunk/tools/debugserver/source/DNB.h (original)
+++ lldb/trunk/tools/debugserver/source/DNB.h Thu Apr 24 14:54:32 2014
@@ -63,6 +63,7 @@ nub_bool_t      DNBProcessResume
 nub_bool_t      DNBProcessHalt          (nub_process_t pid) DNB_EXPORT;
 nub_bool_t      DNBProcessDetach        (nub_process_t pid) DNB_EXPORT;
 nub_bool_t      DNBProcessSignal        (nub_process_t pid, int signal) DNB_EXPORT;
+nub_bool_t      DNBProcessInterrupt     (nub_process_t pid) DNB_EXPORT;
 nub_bool_t      DNBProcessKill          (nub_process_t pid) DNB_EXPORT;
 nub_bool_t      DNBProcessSendEvent     (nub_process_t pid, const char *event) DNB_EXPORT;
 nub_size_t      DNBProcessMemoryRead    (nub_process_t pid, nub_addr_t addr, nub_size_t size, void *buf) DNB_EXPORT;

Modified: lldb/trunk/tools/debugserver/source/MacOSX/MachProcess.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/MacOSX/MachProcess.h?rev=207139&r1=207138&r2=207139&view=diff
==============================================================================
--- lldb/trunk/tools/debugserver/source/MacOSX/MachProcess.h (original)
+++ lldb/trunk/tools/debugserver/source/MacOSX/MachProcess.h Thu Apr 24 14:54:32 2014
@@ -101,6 +101,7 @@ public:
 
     bool                    Resume (const DNBThreadResumeActions& thread_actions);
     bool                    Signal  (int signal, const struct timespec *timeout_abstime = NULL);
+    bool                    Interrupt();
     bool                    SendEvent (const char *event, DNBError &send_err);
     bool                    Kill (const struct timespec *timeout_abstime = NULL);
     bool                    Detach ();
@@ -325,6 +326,18 @@ private:
                                 m_image_infos_callback;
     void *                      m_image_infos_baton;
     std::string                 m_bundle_id;                 // If we are a SB or BKS process, this will be our bundle ID.
+    int                         m_sent_interrupt_signo;      // When we call MachProcess::Interrupt(), we want to send a single signal
+                                                             // to the inferior and only send the signal if we aren't already stopped.
+                                                             // If we end up sending a signal to stop the process we store it until we
+                                                             // receive an exception with this signal. This helps us to verify we got
+                                                             // the signal that interrupted the process. We might stop due to another
+                                                             // reason after an interrupt signal is sent, so this helps us ensure that
+                                                             // we don't report a spurious stop on the next resume.
+    int                         m_auto_resume_signo;         // If we resume the process and still haven't received our interrupt signal
+                                                             // acknownledgement, we will shortly after the next resume. We store the
+                                                             // interrupt signal in this variable so when we get the interrupt signal
+                                                             // as the sole reason for the process being stopped, we can auto resume
+                                                             // the process.
     bool                        m_did_exec;
 };
 

Modified: lldb/trunk/tools/debugserver/source/MacOSX/MachProcess.mm
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/MacOSX/MachProcess.mm?rev=207139&r1=207138&r2=207139&view=diff
==============================================================================
--- lldb/trunk/tools/debugserver/source/MacOSX/MachProcess.mm (original)
+++ lldb/trunk/tools/debugserver/source/MacOSX/MachProcess.mm Thu Apr 24 14:54:32 2014
@@ -138,6 +138,8 @@ MachProcess::MachProcess() :
     m_name_to_addr_baton(NULL),
     m_image_infos_callback(NULL),
     m_image_infos_baton(NULL),
+    m_sent_interrupt_signo (0),
+    m_auto_resume_signo (0),
     m_did_exec (false)
 {
     DNBLogThreadedIf(LOG_PROCESS | LOG_VERBOSE, "%s", __PRETTY_FUNCTION__);
@@ -468,6 +470,37 @@ MachProcess::Kill (const struct timespec
 }
 
 bool
+MachProcess::Interrupt()
+{
+    nub_state_t state = GetState();
+    if (IsRunning(state))
+    {
+        if (m_sent_interrupt_signo == 0)
+        {
+            m_sent_interrupt_signo = SIGSTOP;
+            if (Signal (m_sent_interrupt_signo))
+            {
+                DNBLogThreadedIf(LOG_PROCESS, "MachProcess::Interrupt() - sent %i signal to interrupt process", m_sent_interrupt_signo);
+            }
+            else
+            {
+                m_sent_interrupt_signo = 0;
+                DNBLogThreadedIf(LOG_PROCESS, "MachProcess::Interrupt() - failed to send %i signal to interrupt process", m_sent_interrupt_signo);
+            }
+        }
+        else
+        {
+            DNBLogThreadedIf(LOG_PROCESS, "MachProcess::Interrupt() - previously sent an interrupt signal %i that hasn't been received yet, interrupt aborted", m_sent_interrupt_signo);
+        }
+    }
+    else
+    {
+        DNBLogThreadedIf(LOG_PROCESS, "MachProcess::Interrupt() - process already stopped, no interrupt sent");
+    }
+    return false;
+}
+
+bool
 MachProcess::Signal (int signal, const struct timespec *timeout_abstime)
 {
     DNBLogThreadedIf(LOG_PROCESS, "MachProcess::Signal (signal = %d, timeout = %p)", signal, timeout_abstime);
@@ -746,7 +779,13 @@ void
 MachProcess::PrivateResume ()
 {
     PTHREAD_MUTEX_LOCKER (locker, m_exception_messages_mutex);
-
+    
+    m_auto_resume_signo = m_sent_interrupt_signo;
+    if (m_auto_resume_signo)
+        DNBLogThreadedIf(LOG_PROCESS, "MachProcess::PrivateResume() - task 0x%x resuming (with unhandled interrupt signal %i)...", m_task.TaskPort(), m_auto_resume_signo);
+    else
+        DNBLogThreadedIf(LOG_PROCESS, "MachProcess::PrivateResume() - task 0x%x resuming...", m_task.TaskPort());
+    
     ReplyToAllExceptions ();
 //    bool stepOverBreakInstruction = step;
 
@@ -1147,6 +1186,7 @@ MachProcess::ExceptionMessageBundleCompl
     // We have a complete bundle of exceptions for our child process.
     PTHREAD_MUTEX_LOCKER (locker, m_exception_messages_mutex);
     DNBLogThreadedIf(LOG_EXCEPTIONS, "%s: %llu exception messages.", __PRETTY_FUNCTION__, (uint64_t)m_exception_messages.size());
+    bool auto_resume = false;
     if (!m_exception_messages.empty())
     {
         m_did_exec = false;
@@ -1155,10 +1195,13 @@ MachProcess::ExceptionMessageBundleCompl
         size_t i;
         if (m_pid != 0)
         {
+            bool received_interrupt = false;
+            uint32_t num_task_exceptions = 0;
             for (i=0; i<m_exception_messages.size(); ++i)
             {
                 if (m_exception_messages[i].state.task_port == task)
                 {
+                    ++num_task_exceptions;
                     const int signo = m_exception_messages[i].state.SoftSignal();
                     if (signo == SIGTRAP)
                     {
@@ -1182,6 +1225,10 @@ MachProcess::ExceptionMessageBundleCompl
                         }
                         break;
                     }
+                    else if (m_sent_interrupt_signo != 0 && signo == m_sent_interrupt_signo)
+                    {
+                        received_interrupt = true;
+                    }
                 }
             }
             
@@ -1197,6 +1244,37 @@ MachProcess::ExceptionMessageBundleCompl
                 m_thread_list.Clear();
                 m_breakpoints.DisableAll();
             }
+            
+            if (m_sent_interrupt_signo != 0)
+            {
+                if (received_interrupt)
+                {
+                    DNBLogThreadedIf(LOG_PROCESS, "MachProcess::ExceptionMessageBundleComplete(): process successfully interrupted with signal %i", m_sent_interrupt_signo);
+                    
+                    // Mark that we received the interrupt signal
+                    m_sent_interrupt_signo = 0;
+                    // Not check if we had a case where:
+                    // 1 - We called MachProcess::Interrupt() but we stopped for another reason
+                    // 2 - We called MachProcess::Resume() (but still haven't gotten the interrupt signal)
+                    // 3 - We are now incorrectly stopped because we are handling the interrupt signal we missed
+                    // 4 - We might need to resume if we stopped only with the interrupt signal that we never handled
+                    if (m_auto_resume_signo != 0)
+                    {
+                        // Only auto_resume if we stopped with _only_ the interrupt signal
+                        if (num_task_exceptions == 1)
+                        {
+                            auto_resume = true;
+                            DNBLogThreadedIf(LOG_PROCESS, "MachProcess::ExceptionMessageBundleComplete(): auto resuming due to unhandled interrupt signal %i", m_auto_resume_signo);
+                        }
+                        m_auto_resume_signo = 0;
+                    }
+                }
+                else
+                {
+                    DNBLogThreadedIf(LOG_PROCESS, "MachProcess::ExceptionMessageBundleComplete(): didn't get signal %i after MachProcess::Interrupt()",
+                            m_sent_interrupt_signo);
+                }
+            }
         }
 
         // Let all threads recover from stopping and do any clean up based
@@ -1218,7 +1296,7 @@ MachProcess::ExceptionMessageBundleCompl
             m_thread_list.Dump();
 
         bool step_more = false;
-        if (m_thread_list.ShouldStop(step_more))
+        if (m_thread_list.ShouldStop(step_more) && auto_resume == false)
         {
             // Wait for the eEventProcessRunningStateChanged event to be reset
             // before changing state to stopped to avoid race condition with

Modified: lldb/trunk/tools/debugserver/source/RNBRemote.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/RNBRemote.cpp?rev=207139&r1=207138&r2=207139&view=diff
==============================================================================
--- lldb/trunk/tools/debugserver/source/RNBRemote.cpp (original)
+++ lldb/trunk/tools/debugserver/source/RNBRemote.cpp Thu Apr 24 14:54:32 2014
@@ -862,8 +862,6 @@ typedef struct register_map_entry
 // If the notion of registers differs from what is handed out by the
 // architecture, then flavors can be defined here.
 
-static const uint32_t MAX_REGISTER_BYTE_SIZE = 16;
-static const uint8_t k_zero_bytes[MAX_REGISTER_BYTE_SIZE] = {0};
 static std::vector<register_map_entry_t> g_dynamic_register_map;
 static register_map_entry_t *g_reg_entries = NULL;
 static size_t g_num_reg_entries = 0;
@@ -3684,9 +3682,12 @@ RNBRemote::HandlePacket_stop_process (co
     m_comm.Disconnect(true);
     return err;
 #else
-    DNBProcessSignal (m_ctx.ProcessID(), SIGSTOP);
-    //DNBProcessSignal (m_ctx.ProcessID(), SIGINT);
-    // Do not send any response packet! Wait for the stop reply packet to naturally happen
+    if (!DNBProcessInterrupt(m_ctx.ProcessID()))
+    {
+        // If we failed to interrupt the process, then send a stop
+        // reply packet as the process was probably already stopped
+        HandlePacket_last_signal (NULL);
+    }
     return rnb_success;
 #endif
 }





More information about the lldb-commits mailing list