[Lldb-commits] [lldb] 0948f1c - Reapply the commits to enable accurate hit-count detection for watchpoints.
Jim Ingham via lldb-commits
lldb-commits at lists.llvm.org
Fri Aug 5 11:01:34 PDT 2022
Author: Jim Ingham
Date: 2022-08-05T11:01:27-07:00
New Revision: 0948f1cf8177e378bdea2239b8c3ffb9db31f9ad
URL: https://github.com/llvm/llvm-project/commit/0948f1cf8177e378bdea2239b8c3ffb9db31f9ad
DIFF: https://github.com/llvm/llvm-project/commit/0948f1cf8177e378bdea2239b8c3ffb9db31f9ad.diff
LOG: Reapply the commits to enable accurate hit-count detection for watchpoints.
This commit combines the initial commit (7c240de609af), a fix for x86_64 Linux
(3a0581501e76) and a fix for thinko in a last minute rewrite that I really
should have run the testsuite on.
Also, make sure that all the "I need to step over watchpoint" plans execute
before we call a public stop. Otherwise, e.g. if you have N watchpoints and
a Signal, the signal stop info will get us to stop with the watchpoints in a
half-done state.
Differential Revision: https://reviews.llvm.org/D130674
Added:
Modified:
lldb/include/lldb/Breakpoint/Watchpoint.h
lldb/include/lldb/Target/StopInfo.h
lldb/include/lldb/Target/Thread.h
lldb/include/lldb/Target/ThreadPlan.h
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Target/StopInfo.cpp
lldb/source/Target/Thread.cpp
lldb/source/Target/ThreadList.cpp
lldb/test/API/commands/watchpoints/watchpoint_commands/condition/TestWatchpointConditionCmd.py
lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentDelayWatchBreak.py
lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentManyWatchpoints.py
lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentNWatchNBreak.py
lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalNWatchNBreak.py
lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalWatch.py
lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalWatchBreak.py
lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointThreads.py
lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneBreakpoint.py
lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneDelayBreakpoint.py
lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneSignal.py
lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentWatchBreak.py
Removed:
################################################################################
diff --git a/lldb/include/lldb/Breakpoint/Watchpoint.h b/lldb/include/lldb/Breakpoint/Watchpoint.h
index 41b723a66b6a3..a5a72e3ad5a1b 100644
--- a/lldb/include/lldb/Breakpoint/Watchpoint.h
+++ b/lldb/include/lldb/Breakpoint/Watchpoint.h
@@ -75,7 +75,7 @@ class Watchpoint : public std::enable_shared_from_this<Watchpoint>,
bool IsHardware() const override;
bool ShouldStop(StoppointCallbackContext *context) override;
-
+
bool WatchpointRead() const;
bool WatchpointWrite() const;
uint32_t GetIgnoreCount() const;
@@ -157,12 +157,15 @@ class Watchpoint : public std::enable_shared_from_this<Watchpoint>,
private:
friend class Target;
friend class WatchpointList;
+ friend class StopInfoWatchpoint; // This needs to call UndoHitCount()
void ResetHistoricValues() {
m_old_value_sp.reset();
m_new_value_sp.reset();
}
+ void UndoHitCount() { m_hit_counter.Decrement(); }
+
Target &m_target;
bool m_enabled; // Is this watchpoint enabled
bool m_is_hardware; // Is this a hardware watchpoint
diff --git a/lldb/include/lldb/Target/StopInfo.h b/lldb/include/lldb/Target/StopInfo.h
index cdb906dcd7ede..9527a6ea553e3 100644
--- a/lldb/include/lldb/Target/StopInfo.h
+++ b/lldb/include/lldb/Target/StopInfo.h
@@ -17,7 +17,7 @@
namespace lldb_private {
-class StopInfo {
+class StopInfo : public std::enable_shared_from_this<StopInfo> {
friend class Process::ProcessEventData;
friend class ThreadPlanBase;
diff --git a/lldb/include/lldb/Target/Thread.h b/lldb/include/lldb/Target/Thread.h
index f5f024434c8ee..efe82721a04e3 100644
--- a/lldb/include/lldb/Target/Thread.h
+++ b/lldb/include/lldb/Target/Thread.h
@@ -240,6 +240,7 @@ class Thread : public std::enable_shared_from_this<Thread>,
// this just calls through to the ThreadSpec's ThreadPassesBasicTests method.
virtual bool MatchesSpec(const ThreadSpec *spec);
+ // Get the current public stop info, calculating it if necessary.
lldb::StopInfoSP GetStopInfo();
lldb::StopReason GetStopReason();
@@ -1121,7 +1122,7 @@ class Thread : public std::enable_shared_from_this<Thread>,
// "checkpointed and restored" stop info, so if it is still around it is
// right even if you have not calculated this yourself, or if it disagrees
// with what you might have calculated.
- virtual lldb::StopInfoSP GetPrivateStopInfo();
+ virtual lldb::StopInfoSP GetPrivateStopInfo(bool calculate = true);
// Calculate the stop info that will be shown to lldb clients. For instance,
// a "step out" is implemented by running to a breakpoint on the function
@@ -1165,6 +1166,14 @@ class Thread : public std::enable_shared_from_this<Thread>,
void ResetStopInfo();
void SetShouldReportStop(Vote vote);
+
+ void SetShouldRunBeforePublicStop(bool newval) {
+ m_should_run_before_public_stop = newval;
+ }
+
+ bool ShouldRunBeforePublicStop() {
+ return m_should_run_before_public_stop;
+ }
/// Sets the extended backtrace token for this thread
///
@@ -1254,6 +1263,9 @@ class Thread : public std::enable_shared_from_this<Thread>,
uint32_t m_stop_info_override_stop_id; // The stop ID containing the last time
// the stop info was checked against
// the stop info override
+ bool m_should_run_before_public_stop; // If this thread has "stop others"
+ // private work to do, then it will
+ // set this.
const uint32_t m_index_id; ///< A unique 1 based index assigned to each thread
/// for easy UI/command line access.
lldb::RegisterContextSP m_reg_context_sp; ///< The register context for this
diff --git a/lldb/include/lldb/Target/ThreadPlan.h b/lldb/include/lldb/Target/ThreadPlan.h
index 616939f89fc8e..bf68a42e54d18 100644
--- a/lldb/include/lldb/Target/ThreadPlan.h
+++ b/lldb/include/lldb/Target/ThreadPlan.h
@@ -384,6 +384,8 @@ class ThreadPlan : public std::enable_shared_from_this<ThreadPlan>,
virtual void SetStopOthers(bool new_value);
virtual bool StopOthers();
+
+ virtual bool ShouldRunBeforePublicStop() { return false; }
// This is the wrapper for DoWillResume that does generic ThreadPlan logic,
// then calls DoWillResume.
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index e3dada1864da5..233a665f65904 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1675,7 +1675,15 @@ ThreadSP ProcessGDBRemote::SetThreadStopInfo(
if (dispatch_queue_t != LLDB_INVALID_ADDRESS)
gdb_thread->SetQueueLibdispatchQueueAddress(dispatch_queue_t);
- // Make sure we update our thread stop reason just once
+ // Make sure we update our thread stop reason just once, but don't
+ // overwrite the stop info for threads that haven't moved:
+ StopInfoSP current_stop_info_sp = thread_sp->GetPrivateStopInfo(false);
+ if (thread_sp->GetTemporaryResumeState() == eStateSuspended &&
+ current_stop_info_sp) {
+ thread_sp->SetStopInfo(current_stop_info_sp);
+ return thread_sp;
+ }
+
if (!thread_sp->StopInfoIsUpToDate()) {
thread_sp->SetStopInfo(StopInfoSP());
// If there's a memory thread backed by this thread, we need to use it
diff --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp
index 00d30070c8c9f..d8fb10441d892 100644
--- a/lldb/source/Target/StopInfo.cpp
+++ b/lldb/source/Target/StopInfo.cpp
@@ -20,6 +20,7 @@
#include "lldb/Target/Target.h"
#include "lldb/Target/Thread.h"
#include "lldb/Target/ThreadPlan.h"
+#include "lldb/Target/ThreadPlanStepInstruction.h"
#include "lldb/Target/UnixSignals.h"
#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/Log.h"
@@ -690,39 +691,184 @@ class StopInfoWatchpoint : public StopInfo {
}
protected:
+ using StopInfoWatchpointSP = std::shared_ptr<StopInfoWatchpoint>;
+ // This plan is used to orchestrate stepping over the watchpoint for
+ // architectures (e.g. ARM) that report the watch before running the watched
+ // access. This is the sort of job you have to defer to the thread plans,
+ // if you try to do it directly in the stop info and there are other threads
+ // that needed to process this stop you will have yanked control away from
+ // them and they won't behave correctly.
+ class ThreadPlanStepOverWatchpoint : public ThreadPlanStepInstruction {
+ public:
+ ThreadPlanStepOverWatchpoint(Thread &thread,
+ StopInfoWatchpointSP stop_info_sp,
+ WatchpointSP watch_sp)
+ : ThreadPlanStepInstruction(thread, false, true, eVoteNoOpinion,
+ eVoteNoOpinion),
+ m_stop_info_sp(stop_info_sp), m_watch_sp(watch_sp) {
+ assert(watch_sp);
+ m_watch_index = watch_sp->GetHardwareIndex();
+ }
+
+ bool DoWillResume(lldb::StateType resume_state,
+ bool current_plan) override {
+ if (resume_state == eStateSuspended)
+ return true;
+
+ if (!m_did_disable_wp) {
+ GetThread().GetProcess()->DisableWatchpoint(m_watch_sp.get(), false);
+ m_did_disable_wp = true;
+ }
+ return true;
+ }
+
+ bool DoPlanExplainsStop(Event *event_ptr) override {
+ if (ThreadPlanStepInstruction::DoPlanExplainsStop(event_ptr))
+ return true;
+ StopInfoSP stop_info_sp = GetThread().GetPrivateStopInfo();
+ // lldb-server resets the stop info for threads that didn't get to run,
+ // so we might have not gotten to run, but still have a watchpoint stop
+ // reason, in which case this will indeed be for us.
+ if (stop_info_sp
+ && stop_info_sp->GetStopReason() == eStopReasonWatchpoint)
+ return true;
+ return false;
+ }
+
+ void DidPop() override {
+ // Don't artifically keep the watchpoint alive.
+ m_watch_sp.reset();
+ }
+
+ bool ShouldStop(Event *event_ptr) override {
+ bool should_stop = ThreadPlanStepInstruction::ShouldStop(event_ptr);
+ bool plan_done = MischiefManaged();
+ if (plan_done) {
+ m_stop_info_sp->SetStepOverPlanComplete();
+ GetThread().SetStopInfo(m_stop_info_sp);
+ ResetWatchpoint();
+ }
+ return should_stop;
+ }
+
+ bool ShouldRunBeforePublicStop() override {
+ return true;
+ }
+
+ protected:
+ void ResetWatchpoint() {
+ if (!m_did_disable_wp)
+ return;
+ m_did_disable_wp = true;
+ GetThread().GetProcess()->EnableWatchpoint(m_watch_sp.get(), true);
+ m_watch_sp->SetHardwareIndex(m_watch_index);
+ }
+
+ private:
+ StopInfoWatchpointSP m_stop_info_sp;
+ WatchpointSP m_watch_sp;
+ uint32_t m_watch_index = LLDB_INVALID_INDEX32;
+ bool m_did_disable_wp = false;
+ };
+
bool ShouldStopSynchronous(Event *event_ptr) override {
- // ShouldStop() method is idempotent and should not affect hit count. See
- // Process::RunPrivateStateThread()->Process()->HandlePrivateEvent()
- // -->Process()::ShouldBroadcastEvent()->ThreadList::ShouldStop()->
- // Thread::ShouldStop()->ThreadPlanBase::ShouldStop()->
- // StopInfoWatchpoint::ShouldStop() and
- // Event::DoOnRemoval()->Process::ProcessEventData::DoOnRemoval()->
- // StopInfoWatchpoint::PerformAction().
+ // If we are running our step-over the watchpoint plan, stop if it's done
+ // and continue if it's not:
if (m_should_stop_is_valid)
return m_should_stop;
+ // If we are running our step over plan, then stop here and let the regular
+ // ShouldStop figure out what we should do: Otherwise, give our plan
+ // more time to get run:
+ if (m_using_step_over_plan)
+ return m_step_over_plan_complete;
+
+ Log *log = GetLog(LLDBLog::Process);
ThreadSP thread_sp(m_thread_wp.lock());
- if (thread_sp) {
- WatchpointSP wp_sp(
- thread_sp->CalculateTarget()->GetWatchpointList().FindByID(
- GetValue()));
- if (wp_sp) {
- // Check if we should stop at a watchpoint.
- ExecutionContext exe_ctx(thread_sp->GetStackFrameAtIndex(0));
- StoppointCallbackContext context(event_ptr, exe_ctx, true);
- m_should_stop = wp_sp->ShouldStop(&context);
- } else {
- Log *log = GetLog(LLDBLog::Process);
+ assert(thread_sp);
+
+ if (thread_sp->GetTemporaryResumeState() == eStateSuspended) {
+ // This is the second firing of a watchpoint so don't process it again.
+ LLDB_LOG(log, "We didn't run but stopped with a StopInfoWatchpoint, we "
+ "have already handled this one, don't do it again.");
+ m_should_stop = false;
+ m_should_stop_is_valid = true;
+ return m_should_stop;
+ }
+
+ WatchpointSP wp_sp(
+ thread_sp->CalculateTarget()->GetWatchpointList().FindByID(GetValue()));
+ // If we can no longer find the watchpoint, we just have to stop:
+ if (!wp_sp) {
- LLDB_LOGF(log,
- "Process::%s could not find watchpoint location id: %" PRId64
- "...",
- __FUNCTION__, GetValue());
+ LLDB_LOGF(log,
+ "Process::%s could not find watchpoint location id: %" PRId64
+ "...",
+ __FUNCTION__, GetValue());
+
+ m_should_stop = true;
+ m_should_stop_is_valid = true;
+ return true;
+ }
+
+ ExecutionContext exe_ctx(thread_sp->GetStackFrameAtIndex(0));
+ StoppointCallbackContext context(event_ptr, exe_ctx, true);
+ m_should_stop = wp_sp->ShouldStop(&context);
+ if (!m_should_stop) {
+ // This won't happen at present because we only allow one watchpoint per
+ // watched range. So we won't stop at a watched address with a disabled
+ // watchpoint. If we start allowing overlapping watchpoints, then we
+ // will have to make watchpoints be real "WatchpointSite" and delegate to
+ // all the watchpoints sharing the site. In that case, the code below
+ // would be the right thing to do.
+ m_should_stop_is_valid = true;
+ return m_should_stop;
+ }
+ // If this is a system where we need to execute the watchpoint by hand
+ // after the hit, queue a thread plan to do that, and then say not to stop.
+ // Otherwise, let the async action figure out whether the watchpoint should
+ // stop
+
+ ProcessSP process_sp = exe_ctx.GetProcessSP();
+ uint32_t num;
+ bool wp_triggers_after;
+
+ if (!process_sp->GetWatchpointSupportInfo(num, wp_triggers_after)
+ .Success()) {
+ m_should_stop_is_valid = true;
+ m_should_stop = true;
+ return m_should_stop;
+ }
+
+ if (!wp_triggers_after) {
+ // We have to step over the watchpoint before we know what to do:
+ StopInfoWatchpointSP me_as_siwp_sp
+ = std::static_pointer_cast<StopInfoWatchpoint>(shared_from_this());
+ ThreadPlanSP step_over_wp_sp(new ThreadPlanStepOverWatchpoint(
+ *(thread_sp.get()), me_as_siwp_sp, wp_sp));
+ Status error;
+ error = thread_sp->QueueThreadPlan(step_over_wp_sp, false);
+ // If we couldn't push the thread plan, just stop here:
+ if (!error.Success()) {
+ LLDB_LOGF(log, "Could not push our step over watchpoint plan: %s",
+ error.AsCString());
m_should_stop = true;
+ m_should_stop_is_valid = true;
+ return true;
+ } else {
+ // Otherwise, don't set m_should_stop, we don't know that yet. Just
+ // say we should continue, and tell the thread we really should do so:
+ thread_sp->SetShouldRunBeforePublicStop(true);
+ m_using_step_over_plan = true;
+ return false;
}
+ } else {
+ // We didn't have to do anything special
+ m_should_stop_is_valid = true;
+ return m_should_stop;
}
- m_should_stop_is_valid = true;
+
return m_should_stop;
}
@@ -749,57 +895,12 @@ class StopInfoWatchpoint : public StopInfo {
thread_sp->CalculateTarget()->GetWatchpointList().FindByID(
GetValue()));
if (wp_sp) {
- ExecutionContext exe_ctx(thread_sp->GetStackFrameAtIndex(0));
- ProcessSP process_sp = exe_ctx.GetProcessSP();
-
- {
- // check if this process is running on an architecture where
- // watchpoints trigger before the associated instruction runs. if so,
- // disable the WP, single-step and then re-enable the watchpoint
- if (process_sp) {
- uint32_t num;
- bool wp_triggers_after;
-
- if (process_sp->GetWatchpointSupportInfo(num, wp_triggers_after)
- .Success()) {
- if (!wp_triggers_after) {
- // We need to preserve the watch_index before watchpoint is
- // disable. Since Watchpoint::SetEnabled will clear the watch
- // index. This will fix TestWatchpointIter failure
- Watchpoint *wp = wp_sp.get();
- uint32_t watch_index = wp->GetHardwareIndex();
- process_sp->DisableWatchpoint(wp, false);
- StopInfoSP stored_stop_info_sp = thread_sp->GetStopInfo();
- assert(stored_stop_info_sp.get() == this);
-
- Status new_plan_status;
- ThreadPlanSP new_plan_sp(
- thread_sp->QueueThreadPlanForStepSingleInstruction(
- false, // step-over
- false, // abort_other_plans
- true, // stop_other_threads
- new_plan_status));
- if (new_plan_sp && new_plan_status.Success()) {
- new_plan_sp->SetIsControllingPlan(true);
- new_plan_sp->SetOkayToDiscard(false);
- new_plan_sp->SetPrivate(true);
- }
- process_sp->GetThreadList().SetSelectedThreadByID(
- thread_sp->GetID());
- process_sp->ResumeSynchronous(nullptr);
- process_sp->GetThreadList().SetSelectedThreadByID(
- thread_sp->GetID());
- thread_sp->SetStopInfo(stored_stop_info_sp);
- process_sp->EnableWatchpoint(wp, false);
- wp->SetHardwareIndex(watch_index);
- }
- }
- }
- }
-
// This sentry object makes sure the current watchpoint is disabled
// while performing watchpoint actions, and it is then enabled after we
// are finished.
+ ExecutionContext exe_ctx(thread_sp->GetStackFrameAtIndex(0));
+ ProcessSP process_sp = exe_ctx.GetProcessSP();
+
WatchpointSentry sentry(process_sp, wp_sp);
/*
@@ -825,18 +926,10 @@ class StopInfoWatchpoint : public StopInfo {
}
}
- // TODO: This condition should be checked in the synchronous part of the
- // watchpoint code
- // (Watchpoint::ShouldStop), so that we avoid pulling an event even if
- // the watchpoint fails the ignore count condition. It is moved here
- // temporarily, because for archs with
- // watchpoint_exceptions_received=before, the code in the previous
- // lines takes care of moving the inferior to next PC. We have to check
- // the ignore count condition after this is done, otherwise we will hit
- // same watchpoint multiple times until we pass ignore condition, but
- // we won't actually be ignoring them.
- if (wp_sp->GetHitCount() <= wp_sp->GetIgnoreCount())
+ if (wp_sp->GetHitCount() <= wp_sp->GetIgnoreCount()) {
m_should_stop = false;
+ m_should_stop_is_valid = true;
+ }
Debugger &debugger = exe_ctx.GetTargetRef().GetDebugger();
@@ -859,10 +952,9 @@ class StopInfoWatchpoint : public StopInfo {
Scalar scalar_value;
if (result_value_sp->ResolveValue(scalar_value)) {
if (scalar_value.ULongLong(1) == 0) {
- // We have been vetoed. This takes precedence over querying
- // the watchpoint whether it should stop (aka ignore count
- // and friends). See also StopInfoWatchpoint::ShouldStop()
- // as well as Process::ProcessEventData::DoOnRemoval().
+ // The condition failed, which we consider "not having hit
+ // the watchpoint" so undo the hit count here.
+ wp_sp->UndoHitCount();
m_should_stop = false;
} else
m_should_stop = true;
@@ -946,9 +1038,16 @@ class StopInfoWatchpoint : public StopInfo {
}
private:
+ void SetStepOverPlanComplete() {
+ assert(m_using_step_over_plan);
+ m_step_over_plan_complete = true;
+ }
+
bool m_should_stop = false;
bool m_should_stop_is_valid = false;
lldb::addr_t m_watch_hit_addr;
+ bool m_step_over_plan_complete = false;
+ bool m_using_step_over_plan = false;
};
// StopInfoUnixSignal
diff --git a/lldb/source/Target/Thread.cpp b/lldb/source/Target/Thread.cpp
index f63b57fd4e628..6c3381cf6e9e5 100644
--- a/lldb/source/Target/Thread.cpp
+++ b/lldb/source/Target/Thread.cpp
@@ -221,6 +221,7 @@ Thread::Thread(Process &process, lldb::tid_t tid, bool use_invalid_index_id)
Thread::GetStaticBroadcasterClass().AsCString()),
m_process_wp(process.shared_from_this()), m_stop_info_sp(),
m_stop_info_stop_id(0), m_stop_info_override_stop_id(0),
+ m_should_run_before_public_stop(false),
m_index_id(use_invalid_index_id ? LLDB_INVALID_INDEX32
: process.GetNextThreadIndexID(tid)),
m_reg_context_sp(), m_state(eStateUnloaded), m_state_mutex(),
@@ -369,7 +370,10 @@ void Thread::CalculatePublicStopInfo() {
SetStopInfo(GetStopInfo());
}
-lldb::StopInfoSP Thread::GetPrivateStopInfo() {
+lldb::StopInfoSP Thread::GetPrivateStopInfo(bool calculate) {
+ if (!calculate)
+ return m_stop_info_sp;
+
if (m_destroy_called)
return m_stop_info_sp;
@@ -377,9 +381,15 @@ lldb::StopInfoSP Thread::GetPrivateStopInfo() {
if (process_sp) {
const uint32_t process_stop_id = process_sp->GetStopID();
if (m_stop_info_stop_id != process_stop_id) {
+ // We preserve the old stop info for a variety of reasons:
+ // 1) Someone has already updated it by the time we get here
+ // 2) We didn't get to execute the breakpoint instruction we stopped at
+ // 3) This is a virtual step so we didn't actually run
+ // 4) If this thread wasn't allowed to run the last time round.
if (m_stop_info_sp) {
if (m_stop_info_sp->IsValid() || IsStillAtLastBreakpointHit() ||
- GetCurrentPlan()->IsVirtualStep())
+ GetCurrentPlan()->IsVirtualStep()
+ || GetTemporaryResumeState() == eStateSuspended)
SetStopInfo(m_stop_info_sp);
else
m_stop_info_sp.reset();
@@ -723,7 +733,11 @@ bool Thread::ShouldResume(StateType resume_state) {
return need_to_resume;
}
-void Thread::DidResume() { SetResumeSignal(LLDB_INVALID_SIGNAL_NUMBER); }
+void Thread::DidResume() {
+ SetResumeSignal(LLDB_INVALID_SIGNAL_NUMBER);
+ // This will get recomputed each time when we stop.
+ SetShouldRunBeforePublicStop(false);
+}
void Thread::DidStop() { SetState(eStateStopped); }
@@ -763,6 +777,9 @@ bool Thread::ShouldStop(Event *event_ptr) {
: LLDB_INVALID_ADDRESS);
return false;
}
+
+ // Clear the "must run me before stop" if it was set:
+ SetShouldRunBeforePublicStop(false);
if (log) {
LLDB_LOGF(log,
@@ -845,9 +862,14 @@ bool Thread::ShouldStop(Event *event_ptr) {
// stack below.
done_processing_current_plan =
(plan_ptr->IsControllingPlan() && !plan_ptr->OkayToDiscard());
- } else
+ } else {
+ bool should_force_run = plan_ptr->ShouldRunBeforePublicStop();
+ if (should_force_run) {
+ SetShouldRunBeforePublicStop(true);
+ should_stop = false;
+ }
done_processing_current_plan = true;
-
+ }
break;
}
}
diff --git a/lldb/source/Target/ThreadList.cpp b/lldb/source/Target/ThreadList.cpp
index 7359bfcd3cfc2..006c8648be524 100644
--- a/lldb/source/Target/ThreadList.cpp
+++ b/lldb/source/Target/ThreadList.cpp
@@ -245,14 +245,17 @@ bool ThreadList::ShouldStop(Event *event_ptr) {
for (lldb::ThreadSP thread_sp : m_threads) {
// This is an optimization... If we didn't let a thread run in between
// the previous stop and this one, we shouldn't have to consult it for
- // ShouldStop. So just leave it off the list we are going to inspect. On
- // Linux, if a thread-specific conditional breakpoint was hit, it won't
+ // ShouldStop. So just leave it off the list we are going to inspect.
+ // If the thread didn't run but had work to do before declaring a public
+ // stop, then also include it.
+ // On Linux, if a thread-specific conditional breakpoint was hit, it won't
// necessarily be the thread that hit the breakpoint itself that
// evaluates the conditional expression, so the thread that hit the
// breakpoint could still be asked to stop, even though it hasn't been
// allowed to run since the previous stop.
if (thread_sp->GetTemporaryResumeState() != eStateSuspended ||
- thread_sp->IsStillAtLastBreakpointHit())
+ thread_sp->IsStillAtLastBreakpointHit()
+ || thread_sp->ShouldRunBeforePublicStop())
threads_copy.push_back(thread_sp);
}
@@ -300,6 +303,10 @@ bool ThreadList::ShouldStop(Event *event_ptr) {
thread_sp->GetStopInfo();
}
+ // If a thread needs to finish some job that can be done just on this thread
+ // before broadcastion the stop, it will signal that by returning true for
+ // ShouldRunBeforePublicStop. This variable gathers the results from that.
+ bool a_thread_needs_to_run = false;
for (pos = threads_copy.begin(); pos != end; ++pos) {
ThreadSP thread_sp(*pos);
@@ -329,11 +336,23 @@ bool ThreadList::ShouldStop(Event *event_ptr) {
did_anybody_stop_for_a_reason |= thread_sp->ThreadStoppedForAReason();
const bool thread_should_stop = thread_sp->ShouldStop(event_ptr);
+
if (thread_should_stop)
should_stop |= true;
+ else {
+ bool this_thread_forces_run = thread_sp->ShouldRunBeforePublicStop();
+ a_thread_needs_to_run |= this_thread_forces_run;
+ if (this_thread_forces_run)
+ LLDB_LOG(log,
+ "ThreadList::{0} thread: {1:x}, "
+ "says it needs to run before public stop.",
+ __FUNCTION__, thread_sp->GetID());
+ }
}
- if (!should_stop && !did_anybody_stop_for_a_reason) {
+ if (a_thread_needs_to_run) {
+ should_stop = false;
+ } else if (!should_stop && !did_anybody_stop_for_a_reason) {
should_stop = true;
LLDB_LOGF(log,
"ThreadList::%s we stopped but no threads had a stop reason, "
@@ -368,9 +387,17 @@ Vote ThreadList::ShouldReportStop(Event *event_ptr) {
// Run through the threads and ask whether we should report this event. For
// stopping, a YES vote wins over everything. A NO vote wins over NO
- // opinion.
+ // opinion. The exception is if a thread has work it needs to force before
+ // a public stop, which overrides everyone else's opinion:
for (pos = m_threads.begin(); pos != end; ++pos) {
ThreadSP thread_sp(*pos);
+ if (thread_sp->ShouldRunBeforePublicStop()) {
+ LLDB_LOG(log, "Thread {0:x} has private business to complete, overrode "
+ "the should report stop.", thread_sp->GetID());
+ result = eVoteNo;
+ break;
+ }
+
const Vote vote = thread_sp->ShouldReportStop(event_ptr);
switch (vote) {
case eVoteNoOpinion:
@@ -550,7 +577,13 @@ bool ThreadList::WillResume() {
run_me_only_list.SetStopID(m_process->GetStopID());
- bool run_only_current_thread = false;
+ // One or more threads might want to "Stop Others". We want to handle all
+ // those requests first. And if there is a thread that wanted to "resume
+ // before a public stop", let it get the first crack:
+ // There are two special kinds of thread that have priority for "StopOthers":
+ // a "ShouldRunBeforePublicStop thread, or the currently selected thread. If
+ // we find one satisfying that critereon, put it here.
+ ThreadSP stop_others_thread_sp;
for (pos = m_threads.begin(); pos != end; ++pos) {
ThreadSP thread_sp(*pos);
@@ -562,17 +595,16 @@ bool ThreadList::WillResume() {
// You can't say "stop others" and also want yourself to be suspended.
assert(thread_sp->GetCurrentPlan()->RunState() != eStateSuspended);
+ run_me_only_list.AddThread(thread_sp);
- if (thread_sp == GetSelectedThread()) {
- // If the currently selected thread wants to run on its own, always let
- // it.
- run_only_current_thread = true;
- run_me_only_list.Clear();
- run_me_only_list.AddThread(thread_sp);
+ if (thread_sp == GetSelectedThread())
+ stop_others_thread_sp = thread_sp;
+
+ if (thread_sp->ShouldRunBeforePublicStop()) {
+ // This takes precedence, so if we find one of these, service it:
+ stop_others_thread_sp = thread_sp;
break;
}
-
- run_me_only_list.AddThread(thread_sp);
}
}
@@ -593,8 +625,8 @@ bool ThreadList::WillResume() {
} else {
ThreadSP thread_to_run;
- if (run_only_current_thread) {
- thread_to_run = GetSelectedThread();
+ if (stop_others_thread_sp) {
+ thread_to_run = stop_others_thread_sp;
} else if (run_me_only_list.GetSize(false) == 1) {
thread_to_run = run_me_only_list.GetThreadAtIndex(0);
} else {
@@ -607,6 +639,9 @@ bool ThreadList::WillResume() {
for (pos = m_threads.begin(); pos != end; ++pos) {
ThreadSP thread_sp(*pos);
if (thread_sp == thread_to_run) {
+ // Note, a thread might be able to fulfil it's plan w/o actually
+ // resuming. An example of this is a step that changes the current
+ // inlined function depth w/o moving the PC. Check that here:
if (!thread_sp->ShouldResume(thread_sp->GetCurrentPlan()->RunState()))
need_to_resume = false;
} else
@@ -624,7 +659,7 @@ void ThreadList::DidResume() {
// Don't clear out threads that aren't going to get a chance to run, rather
// leave their state for the next time around.
ThreadSP thread_sp(*pos);
- if (thread_sp->GetResumeState() != eStateSuspended)
+ if (thread_sp->GetTemporaryResumeState() != eStateSuspended)
thread_sp->DidResume();
}
}
diff --git a/lldb/test/API/commands/watchpoints/watchpoint_commands/condition/TestWatchpointConditionCmd.py b/lldb/test/API/commands/watchpoints/watchpoint_commands/condition/TestWatchpointConditionCmd.py
index b3b0e102e6c3b..fee6b8b582c24 100644
--- a/lldb/test/API/commands/watchpoints/watchpoint_commands/condition/TestWatchpointConditionCmd.py
+++ b/lldb/test/API/commands/watchpoints/watchpoint_commands/condition/TestWatchpointConditionCmd.py
@@ -82,4 +82,4 @@ def test_watchpoint_cond(self):
# Use the '-v' option to do verbose listing of the watchpoint.
# The hit count should now be 2.
self.expect("watchpoint list -v",
- substrs=['hit_count = 5'])
+ substrs=['hit_count = 1'])
diff --git a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentDelayWatchBreak.py b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentDelayWatchBreak.py
index ba648b87ea8d0..0a958c0c38c79 100644
--- a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentDelayWatchBreak.py
+++ b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentDelayWatchBreak.py
@@ -11,10 +11,6 @@ class ConcurrentDelayWatchBreak(ConcurrentEventsBase):
# Atomic sequences are not supported yet for MIPS in LLDB.
@skipIf(triple='^mips')
- @skipIf(
- oslist=["ios", "watchos", "tvos", "bridgeos", "macosx"],
- archs=['arm64', 'arm64e', 'arm64_32', 'arm'],
- bugnumber="rdar://81811539")
@add_test_categories(["watchpoint"])
def test(self):
"""Test (1-second delay) watchpoint and a breakpoint in multiple threads."""
diff --git a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentManyWatchpoints.py b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentManyWatchpoints.py
index cd82dab827560..b74d0c7097d6c 100644
--- a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentManyWatchpoints.py
+++ b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentManyWatchpoints.py
@@ -10,10 +10,6 @@ class ConcurrentManyWatchpoints(ConcurrentEventsBase):
# Atomic sequences are not supported yet for MIPS in LLDB.
@skipIf(triple='^mips')
- @skipIf(
- oslist=["ios", "watchos", "tvos", "bridgeos", "macosx"],
- archs=['arm64', 'arm64e', 'arm64_32', 'arm'],
- bugnumber="rdar://93863107")
@add_test_categories(["watchpoint"])
@skipIfOutOfTreeDebugserver
def test(self):
diff --git a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentNWatchNBreak.py b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentNWatchNBreak.py
index 0dab66598eaca..40b548e708e4a 100644
--- a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentNWatchNBreak.py
+++ b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentNWatchNBreak.py
@@ -13,10 +13,6 @@ class ConcurrentNWatchNBreak(ConcurrentEventsBase):
@skipIf(triple='^mips')
@expectedFailureAll(archs=["aarch64"], oslist=["freebsd"],
bugnumber="llvm.org/pr49433")
- @skipIf(
- oslist=["ios", "watchos", "tvos", "bridgeos", "macosx"],
- archs=['arm64', 'arm64e', 'arm64_32', 'arm'],
- bugnumber="rdar://93863107")
@add_test_categories(["watchpoint"])
def test(self):
"""Test with 5 watchpoint and breakpoint threads."""
diff --git a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalNWatchNBreak.py b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalNWatchNBreak.py
index 2478a49de4e38..bcf1c6409b92b 100644
--- a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalNWatchNBreak.py
+++ b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalNWatchNBreak.py
@@ -14,10 +14,6 @@ class ConcurrentSignalNWatchNBreak(ConcurrentEventsBase):
@expectedFailureNetBSD
@expectedFailureAll(archs=["aarch64"], oslist=["freebsd"],
bugnumber="llvm.org/pr49433")
- @skipIf(
- oslist=["ios", "watchos", "tvos", "bridgeos", "macosx"],
- archs=['arm64', 'arm64e', 'arm64_32', 'arm'],
- bugnumber="rdar://93863107")
@add_test_categories(["watchpoint"])
def test(self):
"""Test one signal thread with 5 watchpoint and breakpoint threads."""
diff --git a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalWatch.py b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalWatch.py
index 2922070f9da6d..421db5a31132b 100644
--- a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalWatch.py
+++ b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalWatch.py
@@ -11,10 +11,6 @@ class ConcurrentSignalWatch(ConcurrentEventsBase):
# Atomic sequences are not supported yet for MIPS in LLDB.
@skipIf(triple='^mips')
- @skipIf(
- oslist=["ios", "watchos", "tvos", "bridgeos", "macosx"],
- archs=['arm64', 'arm64e', 'arm64_32', 'arm'],
- bugnumber="rdar://93863107")
@add_test_categories(["watchpoint"])
def test(self):
"""Test a watchpoint and a signal in multiple threads."""
diff --git a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalWatchBreak.py b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalWatchBreak.py
index 3ca3a8661c016..50fead6f499f5 100644
--- a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalWatchBreak.py
+++ b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalWatchBreak.py
@@ -12,10 +12,6 @@ class ConcurrentSignalWatchBreak(ConcurrentEventsBase):
# Atomic sequences are not supported yet for MIPS in LLDB.
@skipIf(triple='^mips')
@expectedFailureNetBSD
- @skipIf(
- oslist=["ios", "watchos", "tvos", "bridgeos", "macosx"],
- archs=['arm64', 'arm64e', 'arm64_32', 'arm'],
- bugnumber="rdar://93863107")
@add_test_categories(["watchpoint"])
def test(self):
"""Test a signal/watchpoint/breakpoint in multiple threads."""
diff --git a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointThreads.py b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointThreads.py
index 5cddeee587481..1e8a2d0632863 100644
--- a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointThreads.py
+++ b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointThreads.py
@@ -11,10 +11,6 @@ class ConcurrentTwoWatchpointThreads(ConcurrentEventsBase):
# Atomic sequences are not supported yet for MIPS in LLDB.
@skipIf(triple='^mips')
- @skipIf(
- oslist=["ios", "watchos", "tvos", "bridgeos", "macosx"],
- archs=['arm64', 'arm64e', 'arm64_32', 'arm'],
- bugnumber="rdar://93863107")
@add_test_categories(["watchpoint"])
def test(self):
"""Test two threads that trigger a watchpoint. """
diff --git a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneBreakpoint.py b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneBreakpoint.py
index c83862d25d50a..88eae0accdf41 100644
--- a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneBreakpoint.py
+++ b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneBreakpoint.py
@@ -11,10 +11,6 @@ class ConcurrentTwoWatchpointsOneBreakpoint(ConcurrentEventsBase):
# Atomic sequences are not supported yet for MIPS in LLDB.
@skipIf(triple='^mips')
- @skipIf(
- oslist=["ios", "watchos", "tvos", "bridgeos", "macosx"],
- archs=['arm64', 'arm64e', 'arm64_32', 'arm'],
- bugnumber="rdar://93863107")
@add_test_categories(["watchpoint"])
def test(self):
"""Test two threads that trigger a watchpoint and one breakpoint thread. """
diff --git a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneDelayBreakpoint.py b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneDelayBreakpoint.py
index f1b65220a091a..8cad1213221ac 100644
--- a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneDelayBreakpoint.py
+++ b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneDelayBreakpoint.py
@@ -11,10 +11,6 @@ class ConcurrentTwoWatchpointsOneDelayBreakpoint(ConcurrentEventsBase):
# Atomic sequences are not supported yet for MIPS in LLDB.
@skipIf(triple='^mips')
- @skipIf(
- oslist=["ios", "watchos", "tvos", "bridgeos", "macosx"],
- archs=['arm64', 'arm64e', 'arm64_32', 'arm'],
- bugnumber="rdar://93863107")
@add_test_categories(["watchpoint"])
def test(self):
"""Test two threads that trigger a watchpoint and one (1 second delay) breakpoint thread. """
diff --git a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneSignal.py b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneSignal.py
index acc08e1dd0772..4cca7ecc64e72 100644
--- a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneSignal.py
+++ b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneSignal.py
@@ -12,10 +12,6 @@ class ConcurrentTwoWatchpointsOneSignal(ConcurrentEventsBase):
# Atomic sequences are not supported yet for MIPS in LLDB.
@skipIf(triple='^mips')
@expectedFailureNetBSD
- @skipIf(
- oslist=["ios", "watchos", "tvos", "bridgeos", "macosx"],
- archs=['arm64', 'arm64e', 'arm64_32', 'arm'],
- bugnumber="rdar://93863107")
@add_test_categories(["watchpoint"])
def test(self):
"""Test two threads that trigger a watchpoint and one signal thread. """
diff --git a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentWatchBreak.py b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentWatchBreak.py
index 7d3a759e09cee..1e95aa2c19f2b 100644
--- a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentWatchBreak.py
+++ b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentWatchBreak.py
@@ -12,10 +12,6 @@ class ConcurrentWatchBreak(ConcurrentEventsBase):
# Atomic sequences are not supported yet for MIPS in LLDB.
@skipIf(triple='^mips')
@add_test_categories(["watchpoint"])
- @skipIf(
- oslist=["ios", "watchos", "tvos", "bridgeos", "macosx"],
- archs=['arm64', 'arm64e', 'arm64_32', 'arm'],
- bugnumber="rdar://93863107")
def test(self):
"""Test watchpoint and a breakpoint in multiple threads."""
More information about the lldb-commits
mailing list