Skip to content

Commit 3c84c55

Browse files
felipepiovezanfelipepiovezan
authored
Merge pull request #10064 from felipepiovezan/felipe/new_thread_detection_mechanism
[lldb] Fix new thread detection mechanism
2 parents e13d412 + 45ee7b8 commit 3c84c55

File tree

3 files changed

+77
-80
lines changed

3 files changed

+77
-80
lines changed

‎lldb/include/lldb/Target/Thread.h

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -198,11 +198,14 @@ class Thread : public std::enable_shared_from_this<Thread>,
198198
/// The User resume state for this thread.
199199
lldb::StateType GetResumeState() const { return m_resume_state; }
200200

201-
// This function is called on all the threads before "ShouldResume" and
202-
// "WillResume" in case a thread needs to change its state before the
203-
// ThreadList polls all the threads to figure out which ones actually will
204-
// get to run and how.
205-
void SetupForResume();
201+
/// This function is called on all the threads before "ShouldResume" and
202+
/// "WillResume" in case a thread needs to change its state before the
203+
/// ThreadList polls all the threads to figure out which ones actually will
204+
/// get to run and how.
205+
///
206+
/// \return
207+
/// True if we pushed a ThreadPlanStepOverBreakpoint
208+
bool SetupForResume();
206209

207210
// Do not override this function, it is for thread plan logic only
208211
bool ShouldResume(lldb::StateType resume_state);

‎lldb/source/Target/Thread.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -641,15 +641,15 @@ void Thread::WillStop() {
641641
current_plan->WillStop();
642642
}
643643

644-
void Thread::SetupForResume() {
644+
bool Thread::SetupForResume() {
645645
if (GetResumeState() != eStateSuspended) {
646646
// First check whether this thread is going to "actually" resume at all.
647647
// For instance, if we're stepping from one level to the next of an
648648
// virtual inlined call stack, we just change the inlined call stack index
649649
// without actually running this thread. In that case, for this thread we
650650
// shouldn't push a step over breakpoint plan or do that work.
651651
if (GetCurrentPlan()->IsVirtualStep())
652-
return;
652+
return false;
653653

654654
// If we're at a breakpoint push the step-over breakpoint plan. Do this
655655
// before telling the current plan it will resume, since we might change
@@ -687,11 +687,13 @@ void Thread::SetupForResume() {
687687
step_bp_plan->SetAutoContinue(true);
688688
}
689689
QueueThreadPlan(step_bp_plan_sp, false);
690+
return true;
690691
}
691692
}
692693
}
693694
}
694695
}
696+
return false;
695697
}
696698

697699
bool Thread::ShouldResume(StateType resume_state) {

‎lldb/source/Target/ThreadList.cpp

Lines changed: 65 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -504,58 +504,7 @@ bool ThreadList::WillResume() {
504504

505505
collection::iterator pos, end = m_threads.end();
506506

507-
// See if any thread wants to run stopping others. If it does, then we won't
508-
// setup the other threads for resume, since they aren't going to get a
509-
// chance to run. This is necessary because the SetupForResume might add
510-
// "StopOthers" plans which would then get to be part of the who-gets-to-run
511-
// negotiation, but they're coming in after the fact, and the threads that
512-
// are already set up should take priority.
513-
514-
bool wants_solo_run = false;
515-
516-
for (pos = m_threads.begin(); pos != end; ++pos) {
517-
lldbassert((*pos)->GetCurrentPlan() &&
518-
"thread should not have null thread plan");
519-
if ((*pos)->GetResumeState() != eStateSuspended &&
520-
(*pos)->GetCurrentPlan()->StopOthers()) {
521-
if ((*pos)->IsOperatingSystemPluginThread() &&
522-
!(*pos)->GetBackingThread())
523-
continue;
524-
wants_solo_run = true;
525-
break;
526-
}
527-
}
528-
529-
if (wants_solo_run) {
530-
Log *log = GetLog(LLDBLog::Step);
531-
if (log && log->GetVerbose())
532-
LLDB_LOGF(log, "Turning on notification of new threads while single "
533-
"stepping a thread.");
534-
m_process.StartNoticingNewThreads();
535-
} else {
536-
Log *log = GetLog(LLDBLog::Step);
537-
if (log && log->GetVerbose())
538-
LLDB_LOGF(log, "Turning off notification of new threads while single "
539-
"stepping a thread.");
540-
m_process.StopNoticingNewThreads();
541-
}
542-
543-
// Give all the threads that are likely to run a last chance to set up their
544-
// state before we negotiate who is actually going to get a chance to run...
545-
// Don't set to resume suspended threads, and if any thread wanted to stop
546-
// others, only call setup on the threads that request StopOthers...
547-
548-
for (pos = m_threads.begin(); pos != end; ++pos) {
549-
if ((*pos)->GetResumeState() != eStateSuspended &&
550-
(!wants_solo_run || (*pos)->GetCurrentPlan()->StopOthers())) {
551-
if ((*pos)->IsOperatingSystemPluginThread() &&
552-
!(*pos)->GetBackingThread())
553-
continue;
554-
(*pos)->SetupForResume();
555-
}
556-
}
557-
558-
// Now go through the threads and see if any thread wants to run just itself.
507+
// Go through the threads and see if any thread wants to run just itself.
559508
// if so then pick one and run it.
560509

561510
ThreadList run_me_only_list(m_process);
@@ -568,34 +517,90 @@ bool ThreadList::WillResume() {
568517
// There are two special kinds of thread that have priority for "StopOthers":
569518
// a "ShouldRunBeforePublicStop thread, or the currently selected thread. If
570519
// we find one satisfying that critereon, put it here.
571-
ThreadSP stop_others_thread_sp;
572-
520+
ThreadSP thread_to_run;
573521
for (pos = m_threads.begin(); pos != end; ++pos) {
574522
ThreadSP thread_sp(*pos);
575523
if (thread_sp->GetResumeState() != eStateSuspended &&
576524
thread_sp->GetCurrentPlan()->StopOthers()) {
577-
if ((*pos)->IsOperatingSystemPluginThread() &&
578-
!(*pos)->GetBackingThread())
525+
if (thread_sp->IsOperatingSystemPluginThread() &&
526+
!thread_sp->GetBackingThread())
579527
continue;
580528

581529
// You can't say "stop others" and also want yourself to be suspended.
582530
assert(thread_sp->GetCurrentPlan()->RunState() != eStateSuspended);
583531
run_me_only_list.AddThread(thread_sp);
584532

585533
if (thread_sp == GetSelectedThread())
586-
stop_others_thread_sp = thread_sp;
587-
534+
thread_to_run = thread_sp;
535+
588536
if (thread_sp->ShouldRunBeforePublicStop()) {
589537
// This takes precedence, so if we find one of these, service it:
590-
stop_others_thread_sp = thread_sp;
538+
thread_to_run = thread_sp;
591539
break;
592540
}
593541
}
594542
}
595543

544+
if (run_me_only_list.GetSize(false) > 0 && !thread_to_run) {
545+
if (run_me_only_list.GetSize(false) == 1) {
546+
thread_to_run = run_me_only_list.GetThreadAtIndex(0);
547+
} else {
548+
int random_thread =
549+
(int)((run_me_only_list.GetSize(false) * (double)rand()) /
550+
(RAND_MAX + 1.0));
551+
thread_to_run = run_me_only_list.GetThreadAtIndex(random_thread);
552+
}
553+
}
554+
555+
// Give all the threads that are likely to run a last chance to set up their
556+
// state before we negotiate who is actually going to get a chance to run...
557+
// Don't set to resume suspended threads, and if any thread wanted to stop
558+
// others, only call setup on the threads that request StopOthers...
559+
if (thread_to_run != nullptr) {
560+
// See if any thread wants to run stopping others. If it does, then we
561+
// won't setup the other threads for resume, since they aren't going to get
562+
// a chance to run. This is necessary because the SetupForResume might add
563+
// "StopOthers" plans which would then get to be part of the who-gets-to-run
564+
// negotiation, but they're coming in after the fact, and the threads that
565+
// are already set up should take priority.
566+
thread_to_run->SetupForResume();
567+
} else {
568+
for (pos = m_threads.begin(); pos != end; ++pos) {
569+
ThreadSP thread_sp(*pos);
570+
if (thread_sp->GetResumeState() != eStateSuspended) {
571+
if (thread_sp->IsOperatingSystemPluginThread() &&
572+
!thread_sp->GetBackingThread())
573+
continue;
574+
if (thread_sp->SetupForResume()) {
575+
// You can't say "stop others" and also want yourself to be suspended.
576+
assert(thread_sp->GetCurrentPlan()->RunState() != eStateSuspended);
577+
thread_to_run = thread_sp;
578+
if (thread_sp->ShouldRunBeforePublicStop()) {
579+
// This takes precedence, so if we find one of these, service it:
580+
break;
581+
}
582+
}
583+
}
584+
}
585+
}
586+
587+
if (thread_to_run != nullptr) {
588+
Log *log = GetLog(LLDBLog::Step);
589+
if (log && log->GetVerbose())
590+
LLDB_LOGF(log, "Turning on notification of new threads while single "
591+
"stepping a thread.");
592+
m_process.StartNoticingNewThreads();
593+
} else {
594+
Log *log = GetLog(LLDBLog::Step);
595+
if (log && log->GetVerbose())
596+
LLDB_LOGF(log, "Turning off notification of new threads while single "
597+
"stepping a thread.");
598+
m_process.StopNoticingNewThreads();
599+
}
600+
596601
bool need_to_resume = true;
597602

598-
if (run_me_only_list.GetSize(false) == 0) {
603+
if (thread_to_run == nullptr) {
599604
// Everybody runs as they wish:
600605
for (pos = m_threads.begin(); pos != end; ++pos) {
601606
ThreadSP thread_sp(*pos);
@@ -608,19 +613,6 @@ bool ThreadList::WillResume() {
608613
need_to_resume = false;
609614
}
610615
} else {
611-
ThreadSP thread_to_run;
612-
613-
if (stop_others_thread_sp) {
614-
thread_to_run = stop_others_thread_sp;
615-
} else if (run_me_only_list.GetSize(false) == 1) {
616-
thread_to_run = run_me_only_list.GetThreadAtIndex(0);
617-
} else {
618-
int random_thread =
619-
(int)((run_me_only_list.GetSize(false) * (double)rand()) /
620-
(RAND_MAX + 1.0));
621-
thread_to_run = run_me_only_list.GetThreadAtIndex(random_thread);
622-
}
623-
624616
for (pos = m_threads.begin(); pos != end; ++pos) {
625617
ThreadSP thread_sp(*pos);
626618
if (thread_sp == thread_to_run) {

0 commit comments

Comments
 (0)