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

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to