jingham created this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

It is possible that the thread we are running a function on could exit while we 
are waiting for the function call to return.

We sort of handled that case before, but it really only worked by accident 
because the ThreadPlan still had a pointer to the Thread, and it hadn't 
actually gone away when we touched it after stopping and finding that it had 
exited.  Now that ThreadPlans re-look up the thread after each stop, we were 
handing out a null Thread pointer and crashing.

I moved the checking for vanished threads to the helper routine that handles 
the stop event, added an expression result of eExpressionThreadVanished and 
handle it properly in RunThreadPlan.  I also added a test using a function that 
just called pthread_exit.  This crashed before these changes, and works 
correctly after.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80350

Files:
  lldb/include/lldb/lldb-enumerations.h
  lldb/source/Expression/FunctionCaller.cpp
  lldb/source/Expression/LLVMUserExpression.cpp
  lldb/source/Target/Process.cpp
  lldb/test/API/functionalities/thread/exit_during_expression/Makefile
  
lldb/test/API/functionalities/thread/exit_during_expression/TestExitDuringExpression.py
  lldb/test/API/functionalities/thread/exit_during_expression/main.c

Index: lldb/test/API/functionalities/thread/exit_during_expression/main.c
===================================================================
--- /dev/null
+++ lldb/test/API/functionalities/thread/exit_during_expression/main.c
@@ -0,0 +1,42 @@
+#include <pthread.h>
+#include <unistd.h>
+#include <errno.h>
+#include <stdio.h>
+
+static unsigned int g_timeout = 200;
+
+int
+function_to_call() {
+
+  errno = 0;
+  while(1) {
+    int result = usleep(g_timeout);
+    if (errno != EINTR)
+      break;
+  }
+  
+  pthread_exit((void *) 10);
+
+  return 20; // Prevent warning
+}
+
+void *
+exiting_thread_func (void *unused) {
+  function_to_call(); // Break here and cause the thread to exit
+  return NULL;
+}
+
+int
+main()
+{
+  char *exit_ptr;
+  pthread_t exiting_thread;
+
+  pthread_create(&exiting_thread, NULL, exiting_thread_func, NULL);
+
+  pthread_join(exiting_thread, &exit_ptr);
+  int ret_val = (int) exit_ptr;
+  usleep(g_timeout * 4); // Make sure in the "run all threads" case
+                         // that we don't run past our breakpoint.
+  return ret_val; // Break here to make sure the thread exited.
+}
Index: lldb/test/API/functionalities/thread/exit_during_expression/TestExitDuringExpression.py
===================================================================
--- /dev/null
+++ lldb/test/API/functionalities/thread/exit_during_expression/TestExitDuringExpression.py
@@ -0,0 +1,106 @@
+"""
+Make sure that we handle an expression on a thread, if
+the thread exits while the expression is running.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+class TestExitDuringExpression(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    NO_DEBUG_INFO_TESTCASE = True
+
+    @skipIfWindows
+    def test_exit_before_one_thread_unwind(self):
+        """Test the case where we exit within the one thread timeout"""
+        self.exiting_expression_test(True, True)
+
+    @skipIfWindows
+    def test_exit_before_one_thread_no_unwind(self):
+        """Test the case where we exit within the one thread timeout"""
+        self.exiting_expression_test(True, False)
+
+    @skipIfWindows
+    def test_exit_after_one_thread_unwind(self):
+        """Test the case where we exit within the one thread timeout"""
+        self.exiting_expression_test(False, True)
+
+    @skipIfWindows
+    def test_exit_after_one_thread_no_unwind(self):
+        """Test the case where we exit within the one thread timeout"""
+        self.exiting_expression_test(False, False)
+    
+    def setUp(self):
+        TestBase.setUp(self)
+        self.main_source_file = lldb.SBFileSpec("main.c")
+        self.build()
+        
+    def exiting_expression_test(self, before_one_thread_timeout , unwind):
+        """function_to_call sleeps for g_timeout microseconds, then calls pthread_exit.
+           This test calls function_to_call with an overall timeout of 500
+           microseconds, and a one_thread_timeout as passed in.
+           It also sets unwind_on_exit for the call to the unwind passed in.
+           This allows you to have the thread exit either before the one thread
+           timeout is passed. """
+        
+        (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self,
+                                   "Break here and cause the thread to exit", self.main_source_file)
+
+        # We'll continue to this breakpoint after running our expression:
+        return_bkpt = target.BreakpointCreateBySourceRegex("Break here to make sure the thread exited", self.main_source_file)
+        frame = thread.frames[0]
+        tid = thread.GetThreadID()
+        # Find the timeout:
+        var_options = lldb.SBVariablesOptions()
+        var_options.SetIncludeArguments(False)
+        var_options.SetIncludeLocals(False)
+        var_options.SetIncludeStatics(True)
+        
+        value_list = frame.GetVariables(var_options)
+        g_timeout = value_list.GetFirstValueByName("g_timeout")
+        self.assertTrue(g_timeout.IsValid(), "Found g_timeout")
+        
+        error = lldb.SBError()
+        timeout_value = g_timeout.GetValueAsUnsigned(error)
+        self.assertTrue(error.Success(), "Couldn't get timeout value: %s"%(error.GetCString()))
+        
+        one_thread_timeout = 0
+        if (before_one_thread_timeout):
+            one_thread_timeout = timeout_value * 2
+        else:
+            one_thread_timeout = int(timeout_value / 2)
+
+        options = lldb.SBExpressionOptions()
+        options.SetUnwindOnError(unwind)
+        options.SetOneThreadTimeoutInMicroSeconds(one_thread_timeout)
+        options.SetTimeoutInMicroSeconds(4 * timeout_value)
+            
+        result = frame.EvaluateExpression("function_to_call()", options)
+
+        # Make sure the thread actually exited:
+        thread = process.GetThreadByID(tid)
+        self.assertFalse(thread.IsValid(), "The thread exited")
+
+        # Make sure the expression failed:
+        self.assertFalse(result.GetError().Success(), "Expression failed.")
+
+        # Make sure we can keep going:
+        threads = lldbutil.continue_to_breakpoint(process, return_bkpt)
+        if not threads:
+            self.fail("didn't get any threads back after continuing")
+
+        self.assertEqual(len(threads), 1, "One thread hit our breakpoint")
+        thread = threads[0]
+        frame = thread.frames[0]
+        # Now get the return value, if we successfully caused the thread to exit
+        # it should be 10, not 20.
+        ret_val = frame.FindVariable("ret_val")
+        self.assertTrue(ret_val.GetError().Success(), "Found ret_val")
+        ret_val_value = ret_val.GetValueAsSigned(error)
+        self.assertTrue(error.Success(), "Got ret_val's value")
+        self.assertEqual(ret_val_value, 10, "We put the right value in ret_val")
+        
Index: lldb/test/API/functionalities/thread/exit_during_expression/Makefile
===================================================================
--- /dev/null
+++ lldb/test/API/functionalities/thread/exit_during_expression/Makefile
@@ -0,0 +1,4 @@
+C_SOURCES := main.c
+CFLAGS_EXTRAS := -std=c99
+
+include Makefile.rules
Index: lldb/source/Target/Process.cpp
===================================================================
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -4600,13 +4600,25 @@
 }
 
 static llvm::Optional<ExpressionResults>
-HandleStoppedEvent(Thread &thread, const ThreadPlanSP &thread_plan_sp,
+HandleStoppedEvent(lldb::tid_t thread_id, 
+                   const ThreadPlanSP &thread_plan_sp,
                    RestorePlanState &restorer, const EventSP &event_sp,
                    EventSP &event_to_broadcast_sp,
-                   const EvaluateExpressionOptions &options, bool handle_interrupts) {
+                   const EvaluateExpressionOptions &options, 
+                   bool handle_interrupts) {
   Log *log = GetLogIfAnyCategoriesSet(LIBLLDB_LOG_STEP | LIBLLDB_LOG_PROCESS);
 
-  ThreadPlanSP plan = thread.GetCompletedPlan();
+  ThreadSP thread_sp = 
+      thread_plan_sp->GetTarget().GetProcessSP()
+          ->GetThreadList().FindThreadByID(thread_id);
+  if (!thread_sp) {
+    LLDB_LOG(log, "The thread on which we were running the "
+                   "expression: tid = {0}, exited while "
+                   "the expression was running.", thread_id);
+    return eExpressionThreadVanished;
+  }
+  
+  ThreadPlanSP plan = thread_sp->GetCompletedPlan();
   if (plan == thread_plan_sp && plan->PlanSucceeded()) {
     LLDB_LOG(log, "execution completed successfully");
 
@@ -4616,7 +4628,7 @@
     return eExpressionCompleted;
   }
 
-  StopInfoSP stop_info_sp = thread.GetStopInfo();
+  StopInfoSP stop_info_sp = thread_sp->GetStopInfo();
   if (stop_info_sp && stop_info_sp->GetStopReason() == eStopReasonBreakpoint &&
       stop_info_sp->ShouldNotify(event_sp.get())) {
     LLDB_LOG(log, "stopped for breakpoint: {0}.", stop_info_sp->GetDescription());
@@ -4677,6 +4689,10 @@
                                  "RunThreadPlan called with invalid thread.");
     return eExpressionSetupError;
   }
+  
+  // Record the thread's id so we can tell when a thread we were using
+  // to run the expression exits during the expression evaluation.
+  lldb::tid_t expr_thread_id = thread->GetID();
 
   // We need to change some of the thread plan attributes for the thread plan
   // runner.  This will restore them when we are done:
@@ -4822,7 +4838,7 @@
       LLDB_LOGF(log,
                 "Process::RunThreadPlan(): Resuming thread %u - 0x%4.4" PRIx64
                 " to run thread plan \"%s\".",
-                thread->GetIndexID(), thread->GetID(), s.GetData());
+                thread_idx_id, expr_thread_id, s.GetData());
     }
 
     bool got_event;
@@ -5022,33 +5038,26 @@
 
             switch (stop_state) {
             case lldb::eStateStopped: {
-              // We stopped, figure out what we are going to do now.
-              ThreadSP thread_sp =
-                  GetThreadList().FindThreadByIndexID(thread_idx_id);
-              if (!thread_sp) {
-                // Ooh, our thread has vanished.  Unlikely that this was
-                // successful execution...
-                LLDB_LOGF(log,
-                          "Process::RunThreadPlan(): execution completed "
-                          "but our thread (index-id=%u) has vanished.",
-                          thread_idx_id);
-                return_value = eExpressionInterrupted;
-              } else if (Process::ProcessEventData::GetRestartedFromEvent(
+              if (Process::ProcessEventData::GetRestartedFromEvent(
                              event_sp.get())) {
                 // If we were restarted, we just need to go back up to fetch
                 // another event.
-                if (log) {
-                  LLDB_LOGF(log, "Process::RunThreadPlan(): Got a stop and "
-                                 "restart, so we'll continue waiting.");
-                }
+                LLDB_LOGF(log, "Process::RunThreadPlan(): Got a stop and "
+                               "restart, so we'll continue waiting.");
                 keep_going = true;
                 do_resume = false;
                 handle_running_event = true;
               } else {
                 const bool handle_interrupts = true;
-                return_value = *HandleStoppedEvent(
-                    *thread, thread_plan_sp, thread_plan_restorer, event_sp,
-                    event_to_broadcast_sp, options, handle_interrupts);
+                return_value = *HandleStoppedEvent(expr_thread_id, 
+                                                   thread_plan_sp, 
+                                                   thread_plan_restorer, 
+                                                   event_sp,
+                                                   event_to_broadcast_sp, 
+                                                   options, 
+                                                   handle_interrupts);
+                if (return_value == eExpressionThreadVanished)
+                  keep_going = false;
               }
             } break;
 
@@ -5169,14 +5178,18 @@
                 // delivered it, the process could have already finished its
                 // job.  Check that here:
                 const bool handle_interrupts = false;
-                if (auto result = HandleStoppedEvent(
-                        *thread, thread_plan_sp, thread_plan_restorer, event_sp,
-                        event_to_broadcast_sp, options, handle_interrupts)) {
+                if (auto result = HandleStoppedEvent(expr_thread_id, 
+                                                     thread_plan_sp, 
+                                                     thread_plan_restorer, 
+                                                     event_sp, 
+                                                     event_to_broadcast_sp, 
+                                                     options, 
+                                                     handle_interrupts)) {
                   return_value = *result;
                   back_to_top = false;
                   break;
                 }
-
+                
                 if (!options.GetTryAllThreads()) {
                   if (log)
                     log->PutCString("Process::RunThreadPlan(): try_all_threads "
@@ -5243,6 +5256,13 @@
         m_public_state.SetValueNoLock(old_state);
     }
 
+    // If our thread went away on us, we need to get out of here without
+    // doing any more work.  We don't have to clean up the thread plan, that
+    // will have happened when the Thread was destroyed.
+    if (return_value == eExpressionThreadVanished) {
+      return return_value;
+    }
+
     if (return_value != eExpressionCompleted && log) {
       // Print a backtrace into the log so we can figure out where we are:
       StreamString s;
@@ -5461,6 +5481,10 @@
   case eExpressionStoppedForDebug:
     result_name = "eExpressionStoppedForDebug";
     break;
+  case eExpressionThreadVanished:
+    result_name = "eExpressionThreadVanished";
+  default:
+    result_name = "<unknown>";
   }
   return result_name;
 }
Index: lldb/source/Expression/LLVMUserExpression.cpp
===================================================================
--- lldb/source/Expression/LLVMUserExpression.cpp
+++ lldb/source/Expression/LLVMUserExpression.cpp
@@ -134,6 +134,10 @@
       return lldb::eExpressionSetupError;
     }
 
+    // Store away the thread ID for error reporting, in case it exits
+    // during execution:
+    lldb::tid_t expr_thread_id = exe_ctx.GetThreadRef().GetID();
+    
     Address wrapper_address(m_jit_start_addr);
 
     std::vector<lldb::addr_t> args;
@@ -223,6 +227,12 @@
           "Use \"thread return -x\" to return to the state before expression "
           "evaluation.");
       return execution_result;
+    } else if (execution_result == lldb::eExpressionThreadVanished) {
+      diagnostic_manager.Printf(
+          eDiagnosticSeverityError, "Couldn't complete execution; the thread "
+          "on which the expression was being run: 0x%" PRIx64
+          " exited during its execution.");
+      return execution_result;
     } else if (execution_result != lldb::eExpressionCompleted) {
       diagnostic_manager.Printf(
           eDiagnosticSeverityError, "Couldn't execute function; result was %s",
Index: lldb/source/Expression/FunctionCaller.cpp
===================================================================
--- lldb/source/Expression/FunctionCaller.cpp
+++ lldb/source/Expression/FunctionCaller.cpp
@@ -364,8 +364,9 @@
     if (return_value != lldb::eExpressionCompleted) {
       LLDB_LOGF(log,
                 "== [FunctionCaller::ExecuteFunction] Execution of \"%s\" "
-                "completed abnormally ==",
-                m_name.c_str());
+                "completed abnormally: %s ==",
+                m_name.c_str(), 
+                Process::ExecutionResultAsCString(return_value));
     } else {
       LLDB_LOGF(log,
                 "== [FunctionCaller::ExecuteFunction] Execution of \"%s\" "
Index: lldb/include/lldb/lldb-enumerations.h
===================================================================
--- lldb/include/lldb/lldb-enumerations.h
+++ lldb/include/lldb/lldb-enumerations.h
@@ -269,7 +269,8 @@
   eExpressionHitBreakpoint,
   eExpressionTimedOut,
   eExpressionResultUnavailable,
-  eExpressionStoppedForDebug
+  eExpressionStoppedForDebug,
+  eExpressionThreadVanished
 };
 
 enum SearchDepth {
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to