[Lldb-commits] [lldb] r226528 - Fix a race condition where you could set the selected thread & target in the

Jim Ingham jingham at apple.com
Mon Jan 19 15:51:51 PST 2015


Author: jingham
Date: Mon Jan 19 17:51:51 2015
New Revision: 226528

URL: http://llvm.org/viewvc/llvm-project?rev=226528&view=rev
Log:
Fix a race condition where you could set the selected thread & target in the
CommandInterpreter's execution context AFTER the process had started running
and before it initially stopped.  Also fixed one test case that was implicitly
using this (and an abuse of the async mode) to accidentally succeed.

<rdar://problem/16814726>

Modified:
    lldb/trunk/include/lldb/Target/Process.h
    lldb/trunk/source/Target/ExecutionContext.cpp
    lldb/trunk/source/Target/Process.cpp
    lldb/trunk/test/functionalities/signal/TestSendSignal.py

Modified: lldb/trunk/include/lldb/Target/Process.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Process.h?rev=226528&r1=226527&r2=226528&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Target/Process.h (original)
+++ lldb/trunk/include/lldb/Target/Process.h Mon Jan 19 17:51:51 2015
@@ -678,13 +678,16 @@ public:
     bool
     IsLastResumeForUserExpression () const
     {
+        // If we haven't yet resumed the target, then it can't be for a user expression...
+        if (m_resume_id == 0)
+            return false;
+
         return m_resume_id == m_last_user_expression_resume;
     }
     
     void
     SetRunningUserExpression (bool on)
     {
-        // REMOVEME printf ("Setting running user expression %s at resume id %d - value: %d.\n", on ? "on" : "off", m_resume_id, m_running_user_expression);
         if (on)
             m_running_user_expression++;
         else

Modified: lldb/trunk/source/Target/ExecutionContext.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/ExecutionContext.cpp?rev=226528&r1=226527&r2=226528&view=diff
==============================================================================
--- lldb/trunk/source/Target/ExecutionContext.cpp (original)
+++ lldb/trunk/source/Target/ExecutionContext.cpp Mon Jan 19 17:51:51 2015
@@ -705,7 +705,11 @@ ExecutionContextRef::SetTargetPtr (Targe
                     if (process_sp)
                     {
                         // Only fill in the thread and frame if our process is stopped
-                        if (StateIsStoppedState (process_sp->GetState(), true))
+                        // Don't just check the state, since we might be in the middle of
+                        // resuming.
+                        Process::StopLocker stop_locker;
+
+                        if (stop_locker.TryLock(&process_sp->GetRunLock()) && StateIsStoppedState (process_sp->GetState(), true))
                         {
                             lldb::ThreadSP thread_sp (process_sp->GetThreadList().GetSelectedThread());
                             if (!thread_sp)

Modified: lldb/trunk/source/Target/Process.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Process.cpp?rev=226528&r1=226527&r2=226528&view=diff
==============================================================================
--- lldb/trunk/source/Target/Process.cpp (original)
+++ lldb/trunk/source/Target/Process.cpp Mon Jan 19 17:51:51 2015
@@ -3643,7 +3643,8 @@ Process::PrivateResume ()
         }
         else
         {
-            // Somebody wanted to run without running.  So generate a continue & a stopped event,
+            // Somebody wanted to run without running (e.g. we were faking a step from one frame of a set of inlined
+            // frames that share the same PC to another.)  So generate a continue & a stopped event,
             // and let the world handle them.
             if (log)
                 log->Printf ("Process::PrivateResume() asked to simulate a start & stop.");

Modified: lldb/trunk/test/functionalities/signal/TestSendSignal.py
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/test/functionalities/signal/TestSendSignal.py?rev=226528&r1=226527&r2=226528&view=diff
==============================================================================
--- lldb/trunk/test/functionalities/signal/TestSendSignal.py (original)
+++ lldb/trunk/test/functionalities/signal/TestSendSignal.py Mon Jan 19 17:51:51 2015
@@ -33,51 +33,75 @@ class SendSignalTestCase(TestBase):
         """Test that lldb command 'process signal SIGUSR1' sends a signal to the inferior process."""
 
         exe = os.path.join(os.getcwd(), "a.out")
-        self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
 
-        # Break inside the main() function and immediately send a signal to the inferior after resuming.
-        lldbutil.run_break_set_by_file_and_line (self, "main.c", self.line, num_expected_locations=1, loc_exact=True)
+        # Create a target by the debugger.
+        target = self.dbg.CreateTarget(exe)
+        self.assertTrue(target, VALID_TARGET)
+
+        # Now create a breakpoint on main.c by name 'c'.
+        breakpoint = target.BreakpointCreateByLocation ('main.c', self.line)
+        self.assertTrue(breakpoint and
+                        breakpoint.GetNumLocations() == 1,
+                        VALID_BREAKPOINT)
+
+        # Get the breakpoint location from breakpoint after we verified that,
+        # indeed, it has one location.
+        location = breakpoint.GetLocationAtIndex(0)
+        self.assertTrue(location and
+                        location.IsEnabled(),
+                        VALID_BREAKPOINT_LOCATION)
+
+        # Now launch the process, no arguments & do not stop at entry point.
+        launch_info = lldb.SBLaunchInfo([exe])
+        launch_info.SetWorkingDirectory(self.get_process_working_directory())
+        
+        process_listener = lldb.SBListener("signal_test_listener")
+        launch_info.SetListener(process_listener)
+        error = lldb.SBError()
+        process = target.Launch (launch_info, error)
+        self.assertTrue(process, PROCESS_IS_VALID)
 
-        self.runCmd("run", RUN_SUCCEEDED)
-        self.runCmd("thread backtrace")
+        self.runCmd("process handle -n False -p True -s True SIGUSR1")
 
-        # The stop reason of the thread should be breakpoint.
-        self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
-            substrs = ['stopped',
-                       'stop reason = breakpoint'])
-
-        # The breakpoint should have a hit count of 1.
-        self.expect("breakpoint list -f", BREAKPOINT_HIT_ONCE,
-            substrs = [' resolved, hit count = 1'])
-
-        self.runCmd("process status")
-        output = self.res.GetOutput()
-        pid = re.match("Process (.*) stopped", output).group(1)
+        thread = lldbutil.get_stopped_thread(process, lldb.eStopReasonBreakpoint)
+        self.assertTrue(thread.IsValid(), "We hit the first breakpoint.")
 
         # After resuming the process, send it a SIGUSR1 signal.
 
-        # It is necessary at this point to make command interpreter interaction
-        # be asynchronous, because we want to resume the process and to send it
-        # a signal.
-        self.dbg.SetAsync(True)
-        self.runCmd("process continue")
-        # Insert a delay of 1 second before doing the signaling stuffs.
-        time.sleep(1)
+        self.setAsync(True)
 
-        self.runCmd("process handle -n False -p True -s True SIGUSR1")
-        #os.kill(int(pid), signal.SIGUSR1)
-        self.runCmd("process signal SIGUSR1")
+        broadcaster = process.GetBroadcaster()
+
+        self.assertTrue(process_listener.IsValid(), "Got a good process listener")
+
+        # Disable our breakpoint, we don't want to hit it anymore...
+        breakpoint.SetEnabled(False)
 
-        # Insert a delay of 1 second before checking the process status.
-        time.sleep(1)
-        # Make the interaction mode be synchronous again.
-        self.dbg.SetAsync(False)
-        self.expect("process status", STOPPED_DUE_TO_SIGNAL,
-            startstr = "Process %s stopped" % pid,
-            substrs = ['stop reason = signal SIGUSR1'])
-        self.expect("thread backtrace", STOPPED_DUE_TO_SIGNAL,
-            substrs = ['stop reason = signal SIGUSR1'])
+        # Now continue:
+        process.Continue()
+
+        event = lldb.SBEvent()
+        got_event = process_listener.WaitForEventForBroadcasterWithType(5, broadcaster, lldb.SBProcess.eBroadcastBitStateChanged, event)
+        event_type = lldb.SBProcess.GetStateFromEvent(event)
+        self.assertTrue (got_event, "Got an event")
+        self.assertTrue (event_type == lldb.eStateRunning, "It was the running event.")
+        
+        # Now signal the process, and make sure it stops:
+        process.Signal(signal.SIGUSR1)
+
+        got_event = process_listener.WaitForEventForBroadcasterWithType(5, broadcaster, lldb.SBProcess.eBroadcastBitStateChanged, event)
+
+        event_type = lldb.SBProcess.GetStateFromEvent(event)
+        self.assertTrue (got_event, "Got an event")
+        self.assertTrue (event_type == lldb.eStateStopped, "It was the stopped event.")
+        
+        # Now make sure the thread was stopped with a SIGUSR1:
+        threads = lldbutil.get_stopped_threads (process, lldb.eStopReasonSignal)
+        self.assertTrue (len(threads) == 1, "One thread stopped for a signal.")
+        thread = threads[0]
 
+        self.assertTrue (thread.GetStopReasonDataCount() >= 1, "There was data in the event.")
+        self.assertTrue (thread.GetStopReasonDataAtIndex(0) == signal.SIGUSR1, "The stop signal was SIGUSR1")
 
 if __name__ == '__main__':
     import atexit





More information about the lldb-commits mailing list