Fix expression evaluation with logs (step + verbose) enabled.

[chaoren]

Summary

The error is:

error: Couldn't allocate space for materialized struct:
Couldn't malloc: address space is full

It seems to be caused by StopInfoThreadPlan::ShouldStop calling
ThreadPlanCallFunction::ShouldStop after the thread plan has been taken down.

ReportRegisterState reads a bunch of (invalid?) register values and caches them.

[jingham]

I don't understand how calling ReportRegisterState when the takedown had already been done could cause the error you report - though without more details about what you are doing it's hard to say for sure.

After all, within ShouldStop the m_thread in the ThreadPlan must still be valid, and all ReportRegisterState does is pretty print its register context. That also should still be good, if it is not, something is wrong at a deeper level.

And anyway, this code only does anything at all if the Expression log is on and in verbose mode. Did you have that log on?

I don't have a strong objection to this change, after all this is just a log output. OTOH I worry that it is just papering over some real problem, so it would be better to figure out what that is.

[chaoren]

all ReportRegisterState does is pretty print its register context

If I invalidate the registers (specifically up to 67) after reading them
then the problem goes away. If I understand correctly, the registers should
be invalidated after every new process_stop_id right? I find it strange
that the StopInfoThreadPlan would be handled in a different process_stop_id
than the one in which the ThreadPlanCallFunction is popped.

The comments on StopInfo::ShouldStop says:

The ShouldStop method should not do anything that might run code.

Could this have anything to do with it? I tried caching the ShouldStop
value in PerformAction, but the problem still happens.

And anyway, this code only does anything at all if the Expression log is on

and in verbose mode. Did you have that log on?

Yeah, the error only happens with logging on.

[jingham]

The "step" log should show you if any code got run between the stop to evaluate "ShouldStop" and when you go to get the register context here. I would be surprised if this is happening, however. If any thread plans need to run code to do some task, they just push another ThreadPlanCallFunction, return false from ShouldStop and then the secondary call get done in the course of handling the thread plans.

I suspect the thread plugin is not properly managing its register context. The ThreadPlan code just calls m_thread.GetRegisterContext(), we're stopped so that should always return a valid register context. Maybe when we ran the old context was not invalidated as it should have been?

[chaoren]

You're right. This seems to be a deeper problem with the (GDBRemote?)RegisterContext. Reading some combination of registers corrupts the cache. There just happened to be no invalidation between StopInfoThreadPlan::ShouldStop and the next ThreadPlanCallFunction (and there shouldn't need to be).

source/Target/ThreadPlanCallFunction.cpp

 ThreadPlanCallFunction::ShouldStop (Event *event_ptr)
 {
     // We do some computation in DoPlanExplainsStop that may or may not set the plan as complete.
     // We need to do that here to make sure our state is correct.
     DoPlanExplainsStop(event_ptr);
     
     if (IsPlanComplete())
     {
+        if (!m_takedown_done)
             ReportRegisterState ("Function completed.  Register state was:");
         return true;
     }
     else
     {
         return false;
     }
 }