[Lldb-commits] [PATCH] D28945: Add completed_plan_stack to LLDB ThreadStateCheckpoint

[Boris Ulasevich via Phabricator via lldb-commits]

boris.ulasevich added a comment.

In https://reviews.llvm.org/D28945#651806, @jingham wrote:

> The plans should do whatever cleanup they are going to do when they get popped, and should not rely on the destructors to do this work.


Yes, you already made a note a time ago:

r123869 | jingham | 2011-01-20

Back up both the register AND the stop state when calling functions.
--------------------------------------------------------------------

123869    jingham //  N.B. Don't wait to do clean up target state till the destructor, since that will usually get called when
123869    jingham //  the target resumes, and you want to leave the target state correct for new plans in the time between when
123869    jingham //  your plan gets unshipped and the next resume.

> I don't think that letting the completed plans live a little longer should pose any problems

It is not something extraordinary, but just a life frame the plans intended to live without unexpected internall call.

By the way, I fixed my issue by restoring specific Thread's property - don't you think it is a potential bug for somebody who will use another property (destroyed plans?) without proper treatment in Thread State Checkpoint?

> But it would be worth a quick audit

As I see, actually destructors clears breakpoints - it is important, but not urgent job.

> we should document that requirement in the ThreadPlan dissertation at the beginning of ThreadPlan.h

For me the ThreadPlan dissertation is like the Bible: I feel it is very old and complicated, a lot on wisdom is hidden there, but nobody around have ever read it attentively :)

I would add words about Thread State there and define stack direction to make words 'below' and 'higher' more definite. Please see new diff.


Repository:
  rL LLVM

https://reviews.llvm.org/D28945