JDevlieghere created this revision.
JDevlieghere added reviewers: jingham, jasonmolenda, labath.
Herald added a subscriber: llvm-commits.

The OS plugins might have updated the thread list after a core file has
been loaded. The physical thread in the core file may no longer be the
one that should be selected. Hence we should run the thread 
selection logic after loading the core.

WIP because of some open questions:

1. What's the best/easiest way to test this?
2. Should we also do this for ELF cores?


Repository:
  rL LLVM

https://reviews.llvm.org/D44139

Files:
  include/lldb/Target/Process.h
  source/Plugins/Process/mach-core/ProcessMachCore.cpp
  source/Target/Process.cpp

Index: source/Target/Process.cpp
===================================================================
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -1036,6 +1036,82 @@
   return state;
 }
 
+StopInfoSP Process::UpdateSelectedThread() {
+  StopInfoSP curr_thread_stop_info_sp;
+  ThreadList &thread_list = GetThreadList();
+
+  // Lock the thread list so it doesn't change on us.
+  std::lock_guard<std::recursive_mutex> guard(thread_list.GetMutex());
+
+  ThreadSP curr_thread(thread_list.GetSelectedThread());
+  StopReason curr_thread_stop_reason = eStopReasonInvalid;
+  if (curr_thread) {
+    curr_thread_stop_reason = curr_thread->GetStopReason();
+    curr_thread_stop_info_sp = curr_thread->GetStopInfo();
+  }
+
+  if (!curr_thread || !curr_thread->IsValid() ||
+      curr_thread_stop_reason == eStopReasonInvalid ||
+      curr_thread_stop_reason == eStopReasonNone) {
+
+    // Prefer a thread that has just completed its plan over another thread as
+    // current thread.
+    ThreadSP thread;
+    ThreadSP plan_thread;
+    ThreadSP other_thread;
+
+    for (size_t i = 0, e = thread_list.GetSize(); i != e; ++i) {
+      thread = thread_list.GetThreadAtIndex(i);
+      StopReason thread_stop_reason = thread->GetStopReason();
+      switch (thread_stop_reason) {
+      case eStopReasonInvalid:
+      case eStopReasonNone:
+        break;
+
+      case eStopReasonSignal: {
+        // Don't select a signal thread if we weren't going to stop at that
+        // signal.  We have to have had another reason for stopping here, and
+        // the user doesn't want to see this thread.
+        uint64_t signo = thread->GetStopInfo()->GetValue();
+        if (this->GetUnixSignals()->GetShouldStop(signo)) {
+          if (!other_thread)
+            other_thread = thread;
+        }
+        break;
+      }
+      case eStopReasonTrace:
+      case eStopReasonBreakpoint:
+      case eStopReasonWatchpoint:
+      case eStopReasonException:
+      case eStopReasonExec:
+      case eStopReasonThreadExiting:
+      case eStopReasonInstrumentation:
+        if (!other_thread)
+          other_thread = thread;
+        break;
+      case eStopReasonPlanComplete:
+        if (!plan_thread)
+          plan_thread = thread;
+        break;
+      }
+    }
+    if (plan_thread)
+      thread_list.SetSelectedThreadByID(plan_thread->GetID());
+    else if (other_thread)
+      thread_list.SetSelectedThreadByID(other_thread->GetID());
+    else {
+      if (curr_thread && curr_thread->IsValid())
+        thread = curr_thread;
+      else
+        thread = thread_list.GetThreadAtIndex(0);
+
+      if (thread)
+        thread_list.SetSelectedThreadByID(thread->GetID());
+    }
+  }
+  return curr_thread_stop_info_sp;
+}
+
 bool Process::HandleProcessStateChangedEvent(const EventSP &event_sp,
                                              Stream *stream,
                                              bool &pop_process_io_handler) {
@@ -1111,87 +1187,8 @@
         }
       }
     } else {
-      StopInfoSP curr_thread_stop_info_sp;
-      // Lock the thread list so it doesn't change on us, this is the scope for
-      // the locker:
-      {
-        ThreadList &thread_list = process_sp->GetThreadList();
-        std::lock_guard<std::recursive_mutex> guard(thread_list.GetMutex());
-
-        ThreadSP curr_thread(thread_list.GetSelectedThread());
-        ThreadSP thread;
-        StopReason curr_thread_stop_reason = eStopReasonInvalid;
-        if (curr_thread) {
-          curr_thread_stop_reason = curr_thread->GetStopReason();
-          curr_thread_stop_info_sp = curr_thread->GetStopInfo();
-        }
-        if (!curr_thread || !curr_thread->IsValid() ||
-            curr_thread_stop_reason == eStopReasonInvalid ||
-            curr_thread_stop_reason == eStopReasonNone) {
-          // Prefer a thread that has just completed its plan over another
-          // thread as current thread.
-          ThreadSP plan_thread;
-          ThreadSP other_thread;
-
-          const size_t num_threads = thread_list.GetSize();
-          size_t i;
-          for (i = 0; i < num_threads; ++i) {
-            thread = thread_list.GetThreadAtIndex(i);
-            StopReason thread_stop_reason = thread->GetStopReason();
-            switch (thread_stop_reason) {
-            case eStopReasonInvalid:
-            case eStopReasonNone:
-              break;
+      StopInfoSP curr_thread_stop_info_sp = process_sp->UpdateSelectedThread();
 
-            case eStopReasonSignal: {
-              // Don't select a signal thread if we weren't going to stop at
-              // that
-              // signal.  We have to have had another reason for stopping here,
-              // and
-              // the user doesn't want to see this thread.
-              uint64_t signo = thread->GetStopInfo()->GetValue();
-              if (process_sp->GetUnixSignals()->GetShouldStop(signo)) {
-                if (!other_thread)
-                  other_thread = thread;
-              }
-              break;
-            }
-            case eStopReasonTrace:
-            case eStopReasonBreakpoint:
-            case eStopReasonWatchpoint:
-            case eStopReasonException:
-            case eStopReasonExec:
-            case eStopReasonThreadExiting:
-            case eStopReasonInstrumentation:
-              if (!other_thread)
-                other_thread = thread;
-              break;
-            case eStopReasonPlanComplete:
-              if (!plan_thread)
-                plan_thread = thread;
-              break;
-            }
-          }
-          if (plan_thread)
-            thread_list.SetSelectedThreadByID(plan_thread->GetID());
-          else if (other_thread)
-            thread_list.SetSelectedThreadByID(other_thread->GetID());
-          else {
-            if (curr_thread && curr_thread->IsValid())
-              thread = curr_thread;
-            else
-              thread = thread_list.GetThreadAtIndex(0);
-
-            if (thread)
-              thread_list.SetSelectedThreadByID(thread->GetID());
-          }
-        }
-      }
-      // Drop the ThreadList mutex by here, since GetThreadStatus below might
-      // have to run code,
-      // e.g. for Data formatters, and if we hold the ThreadList mutex, then the
-      // process is going to
-      // have a hard time restarting the process.
       if (stream) {
         Debugger &debugger = process_sp->GetTarget().GetDebugger();
         if (debugger.GetTargetList().GetSelectedTarget().get() ==
Index: source/Plugins/Process/mach-core/ProcessMachCore.cpp
===================================================================
--- source/Plugins/Process/mach-core/ProcessMachCore.cpp
+++ source/Plugins/Process/mach-core/ProcessMachCore.cpp
@@ -307,12 +307,12 @@
                          " from LC_NOTE 'main bin spec' load command.", m_mach_kernel_addr);
     }
   }
-  
+
   // This checks for the presence of an LC_IDENT string in a core file;
   // LC_IDENT is very obsolete and should not be used in new code, but
   // if the load command is present, let's use the contents.
   std::string corefile_identifier = core_objfile->GetIdentifierString();
-  if (found_main_binary_definitively == false 
+  if (found_main_binary_definitively == false
       && corefile_identifier.find("Darwin Kernel") != std::string::npos) {
       UUID uuid;
       addr_t addr = LLDB_INVALID_ADDRESS;
@@ -366,7 +366,7 @@
     }
   }
 
-  if (found_main_binary_definitively == false 
+  if (found_main_binary_definitively == false
        && m_mach_kernel_addr != LLDB_INVALID_ADDRESS) {
     // In the case of multiple kernel images found in the core file via
     // exhaustive
@@ -511,7 +511,9 @@
   // Let all threads recover from stopping and do any clean up based
   // on the previous thread state (if any).
   m_thread_list.RefreshStateAfterStop();
-  // SetThreadStopInfo (m_last_stop_packet);
+  // The OS plugins might have updated the thread list. This might affect which
+  // thread should be selected.
+  UpdateSelectedThread();
 }
 
 Status ProcessMachCore::DoDestroy() { return Status(); }
Index: include/lldb/Target/Process.h
===================================================================
--- include/lldb/Target/Process.h
+++ include/lldb/Target/Process.h
@@ -2367,6 +2367,9 @@
 
   void UpdateThreadListIfNeeded();
 
+
+  lldb::StopInfoSP UpdateSelectedThread();
+
   ThreadList &GetThreadList() { return m_thread_list; }
 
   // When ExtendedBacktraces are requested, the HistoryThreads that are
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to