[Lldb-commits] [PATCH] D80112: Check if thread was suspended during previous stop added.

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

Adding a ShouldStop to that if test doesn't seem right.  You should still run the stop actions even if the thread doesn't want to stop.

If I understand the problem you are describing, it is that you suspended a thread as a user-level suspend, so it's stop reason got stuck.  That seems okay, its probably useful to keep the stop reason of the thread at what it was when you suspended it.  But that means  when gets asked to do its action again, the action is stale and does the wrong thing.  If that's what's going wrong, then it would make more sense short-circuit suspended threads earlier on in the iteration, like:

  lldb::ThreadSP thread_sp = curr_thread_list.GetThreadAtIndex(idx);
  // This thread was suspended by the user during this run, so it's actions are stale:
  if (thread_sp->GetResumeState() == eStateSuspended)
      continue;

That mirrors what the ThreadList::ShouldStop does.  You actually probably don't want to do it quite this way, because it is possible that one thread's breakpoint action could resume another thread.  So you should probably first run through the thread list and get all the suspended threads, then run through again doing the actions and skipping the suspended threads you found in the first iteration.

It would be good to write a end-to-end test that mirrors this behavior.  It would be pretty straightforward to write an API test that runs a process, sets a breakpoint, hits it on one thread, suspends that thread, then hits it on another thread and make sure the action didn't run twice on the second stop.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80112