[Lldb-commits] [lldb] 8d024a7 - Fix a problem with "watchpoint triggers before" watchpoint handling.
Jim Ingham via lldb-commits
lldb-commits at lists.llvm.org
Mon Mar 20 15:17:22 PDT 2023
Author: Jim Ingham
Date: 2023-03-20T15:17:15-07:00
New Revision: 8d024a79ea783ed3fbb5691aeaf186ad3f0a4ae9
URL: https://github.com/llvm/llvm-project/commit/8d024a79ea783ed3fbb5691aeaf186ad3f0a4ae9
DIFF: https://github.com/llvm/llvm-project/commit/8d024a79ea783ed3fbb5691aeaf186ad3f0a4ae9.diff
LOG: Fix a problem with "watchpoint triggers before" watchpoint handling.
We need to step the watchpoint instruction in these cases, but the
when we queued the ThreadPlanStepOverWatchpoint to do this, we didn't
make it a Controlling plan. So if you are stepping, this plan returns as
though it were a utility plan, and the stepping plan keeps going.
This only partially fixes the problem on Darwin; there's another bug
with reporting a watchpoint when we're instruction single stepping over
an instruction that triggers a watchpoint. The kernel reports the
"single step completed" but not the watchpoint hit. So this commit
also refactors the test into a part that works (at least on Darwin) and
a part that still fails.
We may have to adjust the test result expectations for other systems after
this fix.
Differential Revision: https://reviews.llvm.org/D146337
Added:
Modified:
lldb/include/lldb/Target/Process.h
lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
lldb/source/Target/StopInfo.cpp
lldb/source/Target/Thread.cpp
lldb/test/API/commands/watchpoints/step_over_watchpoint/TestStepOverWatchpoint.py
lldb/test/API/commands/watchpoints/step_over_watchpoint/main.c
Removed:
################################################################################
diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h
index 3ffacb52299b9..6ce38f63cd249 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -311,6 +311,14 @@ class ProcessModID {
return m_last_natural_stop_event;
return lldb::EventSP();
}
+
+ void SetSafeToCallFunctions(bool safe) {
+ m_safe = safe;
+ }
+
+ bool GetSafeToCallFunctions() {
+ return m_safe;
+ }
private:
uint32_t m_stop_id = 0;
@@ -321,6 +329,7 @@ class ProcessModID {
uint32_t m_running_user_expression = false;
uint32_t m_running_utility_function = 0;
lldb::EventSP m_last_natural_stop_event;
+ std::atomic<bool> m_safe = true;
};
inline bool operator==(const ProcessModID &lhs, const ProcessModID &rhs) {
@@ -459,7 +468,7 @@ class Process : public std::enable_shared_from_this<Process>,
void SetRestarted(bool new_value) { m_restarted = new_value; }
void SetInterrupted(bool new_value) { m_interrupted = new_value; }
-
+
void AddRestartedReason(const char *reason) {
m_restarted_reasons.push_back(reason);
}
@@ -1250,6 +1259,14 @@ class Process : public std::enable_shared_from_this<Process>,
DiagnosticManager &diagnostic_manager);
static const char *ExecutionResultAsCString(lldb::ExpressionResults result);
+
+ void SetSafeToCallFunctions(bool safe) {
+ GetModID().SetSafeToCallFunctions(safe);
+ }
+
+ bool GetSafeToCallFunctions() {
+ return GetModID().GetSafeToCallFunctions();
+ }
void GetStatus(Stream &ostrm);
diff --git a/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp b/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
index aae15b2ef4624..458d44f6feb33 100644
--- a/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
+++ b/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
@@ -795,6 +795,19 @@ StopInfoSP StopInfoMachException::CreateStopReasonWithMachException(
case 9: // EXC_RPC_ALERT
case 10: // EXC_CRASH
break;
+ case 12: // EXC_GUARD
+ {
+ // Some EXC_GUARD exceptions are fatal, and the process will go away
+ // the next time you allow it to run. When we get one of those
+ // exceptions we have to make sure SafeToCallFunctions returns false to
+ // prevent us or other agents running the process. This has to be set
+ // on the process because even the threads that didn't get the exception
+ // can't run.
+ ProcessSP process_sp(thread.GetProcess());
+ if (process_sp)
+ process_sp->SetSafeToCallFunctions(false);
+
+ }
}
return StopInfoSP(new StopInfoMachException(thread, exc_type, exc_data_count,
diff --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp
index 9fdb29f9e4273..ebc355c90d0ab 100644
--- a/lldb/source/Target/StopInfo.cpp
+++ b/lldb/source/Target/StopInfo.cpp
@@ -831,6 +831,11 @@ class StopInfoWatchpoint : public StopInfo {
= std::static_pointer_cast<StopInfoWatchpoint>(shared_from_this());
ThreadPlanSP step_over_wp_sp(new ThreadPlanStepOverWatchpoint(
*(thread_sp.get()), me_as_siwp_sp, wp_sp));
+ // When this plan is done we want to stop, so set this as a Controlling
+ // plan.
+ step_over_wp_sp->SetIsControllingPlan(true);
+ step_over_wp_sp->SetOkayToDiscard(false);
+
Status error;
error = thread_sp->QueueThreadPlan(step_over_wp_sp, false);
// If we couldn't push the thread plan, just stop here:
diff --git a/lldb/source/Target/Thread.cpp b/lldb/source/Target/Thread.cpp
index d620f746339e7..df8bff5102b83 100644
--- a/lldb/source/Target/Thread.cpp
+++ b/lldb/source/Target/Thread.cpp
@@ -1664,6 +1664,9 @@ addr_t Thread::GetThreadLocalData(const ModuleSP module,
bool Thread::SafeToCallFunctions() {
Process *process = GetProcess().get();
if (process) {
+ if (!process->SafeToCallFunctions())
+ return false;
+
DynamicLoader *loader = GetProcess()->GetDynamicLoader();
if (loader && loader->IsFullyInitialized() == false)
return false;
diff --git a/lldb/test/API/commands/watchpoints/step_over_watchpoint/TestStepOverWatchpoint.py b/lldb/test/API/commands/watchpoints/step_over_watchpoint/TestStepOverWatchpoint.py
index 7d54156aebb5b..52fc899b13e61 100644
--- a/lldb/test/API/commands/watchpoints/step_over_watchpoint/TestStepOverWatchpoint.py
+++ b/lldb/test/API/commands/watchpoints/step_over_watchpoint/TestStepOverWatchpoint.py
@@ -11,36 +11,11 @@
class TestStepOverWatchpoint(TestBase):
NO_DEBUG_INFO_TESTCASE = True
- @expectedFailureAll(
- oslist=["freebsd", "linux"],
- archs=[
- 'aarch64',
- 'arm'],
- bugnumber="llvm.org/pr26031")
- # Read-write watchpoints not supported on SystemZ
- @expectedFailureAll(archs=['s390x'])
- @expectedFailureAll(
- oslist=["ios", "watchos", "tvos", "bridgeos", "macosx"],
- archs=['aarch64', 'arm'],
- bugnumber="<rdar://problem/34027183>")
- @add_test_categories(["basic_process"])
- def test(self):
+ def get_to_start(self, bkpt_text):
"""Test stepping over watchpoints."""
self.build()
- target = self.createTestTarget()
-
- lldbutil.run_break_set_by_symbol(self, 'main')
-
- process = target.LaunchSimple(None, None,
- self.get_process_working_directory())
- self.assertTrue(process.IsValid(), PROCESS_IS_VALID)
- self.assertState(process.GetState(), lldb.eStateStopped,
- PROCESS_STOPPED)
-
- thread = lldbutil.get_stopped_thread(process,
- lldb.eStopReasonBreakpoint)
- self.assertTrue(thread.IsValid(), "Failed to get thread.")
-
+ target, process, thread, bkpt = lldbutil.run_to_source_breakpoint(self, bkpt_text,
+ lldb.SBFileSpec("main.c"))
frame = thread.GetFrameAtIndex(0)
self.assertTrue(frame.IsValid(), "Failed to get frame.")
@@ -55,14 +30,45 @@ def test(self):
self.assertSuccess(error, "Error while setting watchpoint")
self.assertTrue(read_watchpoint, "Failed to set read watchpoint.")
+ # Disable the breakpoint we hit so we don't muddy the waters with
+ # stepping off from the breakpoint:
+ bkpt.SetEnabled(False)
+
+ return (target, process, thread, read_watchpoint)
+
+ @expectedFailureAll(
+ oslist=["freebsd", "linux"],
+ archs=[
+ 'aarch64',
+ 'arm'],
+ bugnumber="llvm.org/pr26031")
+ # Read-write watchpoints not supported on SystemZ
+ @expectedFailureAll(archs=['s390x'])
+ @add_test_categories(["basic_process"])
+ def test_step_over(self):
+ target, process, thread, wp = self.get_to_start("Set a breakpoint here")
+
thread.StepOver()
self.assertStopReason(thread.GetStopReason(), lldb.eStopReasonWatchpoint,
STOPPED_DUE_TO_WATCHPOINT)
self.assertEquals(thread.GetStopDescription(20), 'watchpoint 1')
- process.Continue()
- self.assertState(process.GetState(), lldb.eStateStopped,
- PROCESS_STOPPED)
+ @expectedFailureAll(
+ oslist=["freebsd", "linux"],
+ archs=[
+ 'aarch64',
+ 'arm'],
+ bugnumber="llvm.org/pr26031")
+ # Read-write watchpoints not supported on SystemZ
+ @expectedFailureAll(archs=['s390x'])
+ @expectedFailureAll(
+ oslist=["ios", "watchos", "tvos", "bridgeos", "macosx"],
+ archs=['aarch64', 'arm'],
+ bugnumber="<rdar://problem/34027183>")
+ @add_test_categories(["basic_process"])
+ def test_step_instruction(self):
+ target, process, thread, wp = self.get_to_start("Set breakpoint after call")
+
self.assertEquals(thread.GetStopDescription(20), 'step over')
self.step_inst_for_watchpoint(1)
diff --git a/lldb/test/API/commands/watchpoints/step_over_watchpoint/main.c b/lldb/test/API/commands/watchpoints/step_over_watchpoint/main.c
index 2d87d9a2f73fe..e48d43cb7a974 100644
--- a/lldb/test/API/commands/watchpoints/step_over_watchpoint/main.c
+++ b/lldb/test/API/commands/watchpoints/step_over_watchpoint/main.c
@@ -11,8 +11,8 @@ void watch_write() {
}
int main() {
- watch_read();
- g_temp = g_watch_me_read;
+ watch_read(); // Set a breakpoint here
+ g_temp = g_watch_me_read; // Set breakpoint after call
watch_write();
g_watch_me_write = g_temp;
return 0;
More information about the lldb-commits
mailing list