Skip to content

Commit 38b252a

Browse files
jeffreytan81jeffreytan81
andauthored
Disable ThreadPlanSingleThreadTimeout during step over breakpoint (#104532)
This PR fixes another race condition in #90930. The failure was found by @labath with this log: https://paste.debian.net/hidden/30235a5c/: ``` dotest_wrapper. < 15> send packet: $z0,224505,1#65 ... b-remote.async> < 22> send packet: $vCont;s:p1dcf.1dcf#4c intern-state GDBRemoteClientBase::Lock::Lock sent packet: \x03 b-remote.async> < 818> read packet: $T13thread:p1dcf.1dcf;name:a.out;threads:1dcf,1dd2;jstopinfo:5b7b226e616d65223a22612e6f7574222c22726561736f6e223a227369676e616c222c227369676e616c223a31392c22746964223a373633317d2c7b226e616d65223a22612e6f7574222c22746964223a373633347d5d;thread-pcs:0000000000224505,00007f4e4302119a;00:0000000000000000;01:0000000000000000;02:0100000000000000;03:0000000000000000;04:9084997dfc7f0000;05:a8742a0000000000;06:b084997dfc7f0000;07:6084997dfc7f0000;08:0000000000000000;09:00d7e5424e7f0000;0a:d0d9e5424e7f0000;0b:0202000000000000;0c:80cc290000000000;0d:d8cc1c434e7f0000;0e:2886997dfc7f0000;0f:0100000000000000;10:0545220000000000;11:0602000000000000;12:3300000000000000;13:0000000000000000;14:0000000000000000;15:2b00000000000000;16:80fbe5424e7f0000;17:0000000000000000;18:0000000000000000;19:0000000000000000;reason:signal;#b9 ``` It shows an async interrupt "\x03" was sent immediately after `vCont;s` single step over breakpoint at address `0x224505` (which was disabled before vCont). And the later stop was still at the original PC (0x224505) not moving forward. The investigation shows the failure happens when timeout is short and async interrupt is sent to lldb-server immediately after vCont so ptrace() resumes and then async interrupts debuggee immediately so debuggee does not get a chance to execute and move PC. So it enters stop mode immediately at original PC. `ThreadPlanStepOverBreakpoint` does not expect PC not moving and reports stop at the original place. To fix this, the PR prevents `ThreadPlanSingleThreadTimeout` from being created during `ThreadPlanStepOverBreakpoint` by introduces a new `SupportsResumeOthers()` method and `ThreadPlanStepOverBreakpoint` returns false for it. This makes sense because we should never resume threads during step over breakpoint anyway otherwise it might cause other threads to miss breakpoint. --------- Co-authored-by: jeffreytan81 <[email protected]>
1 parent 17b7a9d commit 38b252a

File tree

4 files changed

+21
-1
lines changed

4 files changed

+21
-1
lines changed

lldb/include/lldb/Target/ThreadPlan.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -385,7 +385,13 @@ class ThreadPlan : public std::enable_shared_from_this<ThreadPlan>,
385385
virtual void SetStopOthers(bool new_value);
386386

387387
virtual bool StopOthers();
388-
388+
389+
// Returns true if the thread plan supports ThreadPlanSingleThreadTimeout to
390+
// resume other threads after timeout. If the thread plan returns false it
391+
// will prevent ThreadPlanSingleThreadTimeout from being created when this
392+
// thread plan is alive.
393+
virtual bool SupportsResumeOthers() { return true; }
394+
389395
virtual bool ShouldRunBeforePublicStop() { return false; }
390396

391397
// This is the wrapper for DoWillResume that does generic ThreadPlan logic,

lldb/include/lldb/Target/ThreadPlanStepOverBreakpoint.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ class ThreadPlanStepOverBreakpoint : public ThreadPlan {
2323
void GetDescription(Stream *s, lldb::DescriptionLevel level) override;
2424
bool ValidatePlan(Stream *error) override;
2525
bool ShouldStop(Event *event_ptr) override;
26+
bool SupportsResumeOthers() override;
2627
bool StopOthers() override;
2728
lldb::StateType GetPlanRunState() override;
2829
bool WillStop() override;

lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,9 @@ void ThreadPlanSingleThreadTimeout::PushNewWithTimeout(Thread &thread,
7676
if (!thread.GetCurrentPlan()->StopOthers())
7777
return;
7878

79+
if (!thread.GetCurrentPlan()->SupportsResumeOthers())
80+
return;
81+
7982
auto timeout_plan = new ThreadPlanSingleThreadTimeout(thread, info);
8083
ThreadPlanSP thread_plan_sp(timeout_plan);
8184
auto status = thread.QueueThreadPlan(thread_plan_sp,
@@ -102,6 +105,9 @@ void ThreadPlanSingleThreadTimeout::ResumeFromPrevState(Thread &thread,
102105
if (!thread.GetCurrentPlan()->StopOthers())
103106
return;
104107

108+
if (!thread.GetCurrentPlan()->SupportsResumeOthers())
109+
return;
110+
105111
auto timeout_plan = new ThreadPlanSingleThreadTimeout(thread, info);
106112
ThreadPlanSP thread_plan_sp(timeout_plan);
107113
auto status = thread.QueueThreadPlan(thread_plan_sp,

lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,13 @@ bool ThreadPlanStepOverBreakpoint::ShouldStop(Event *event_ptr) {
103103

104104
bool ThreadPlanStepOverBreakpoint::StopOthers() { return true; }
105105

106+
// This thread plan does a single instruction step over a breakpoint instruction
107+
// and needs to not resume other threads, so return false to stop the
108+
// ThreadPlanSingleThreadTimeout from timing out and trying to resume all
109+
// threads. If all threads gets resumed before we disable, single step and
110+
// re-enable the breakpoint, we can miss breakpoints on other threads.
111+
bool ThreadPlanStepOverBreakpoint::SupportsResumeOthers() { return false; }
112+
106113
StateType ThreadPlanStepOverBreakpoint::GetPlanRunState() {
107114
return eStateStepping;
108115
}

0 commit comments

Comments
 (0)