[Lldb-commits] [PATCH] D129814: Fix stepping over watchpoints in architectures that raise the exception before executing the instruction

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Generally, this makes sense to me, but I do have one question (not necessarily for Jim). These tests work on (arm) linux, and I am wondering why is that the case. Could it be related to the fact that lldb-server preserves the stop reason for threads that are not running? I.e. if we have two threads hit a breakpoint (or whatever), and then step one of them, then the other thread will still report a stop_reason=breakpoint. Do you know if this is supposed to happen (e.g. does debugserver do that)?

(It wouldn't surprise me if this was a bug in lldb-server, as I think it's related to some strange behavior with multiple threads stopped (in these situations, lldb will sometimes keep switching away from the debugged thread after every stop), but it was never clear to me whether this was a problem in lldb or lldb-server.)



================
Comment at: lldb/source/Target/StopInfo.cpp:805
+
+      m_step_over_wp_sp.reset(new ThreadPlanStepOverWatchpoint(
+          *(thread_sp.get()), shared_from_this(), wp_sp));
----------------
This creates a shared pointer loop, does it not?

IIUC, this `m_step_over_wp_sp` is used only to check whether the plan is complete. Maybe, instead of the pointer, we could have a flag that the thread plan sets when it finishes?


================
Comment at: lldb/source/Target/StopInfo.cpp:898-901
                 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();
----------------
How does this relate to the thread plan part of the patch?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129814/new/

https://reviews.llvm.org/D129814