Re: (maybe) Async mode failures on PPC

[Pedro Alves <pedro@codesourcery.com>]

Hi Luis,

Thanks for taking care of this.

> 2008-07-15  Luis Machado  <luisgpm@br.ibm.com>
>
> 	* infrun.c (handle_inferior_event): reinit_frame_cache before
> 	handling events on threads.

As I noted on IRC, this is a bit incomplete, but I know you're already
handling it.  :-)

On Tuesday 15 July 2008 15:46:40, Luis Machado wrote:
> On Mon, 2008-07-14 at 14:06 -0300, Luis Machado wrote:
> Here is a patch i discussed with Pedro to fix this regression. Tested
> and nothing showed up in the testsuite.

For everyone else not following the discussion on IRC:

The main issue was the insert_breakpoints call being done
before the stopped thread being tagged as stopped, as Andreas
also noticed.  Inside insert_breakpoints, when updating
watchpoints, there are calls into frame routines, which
bail out if the thread is considered to be running.  It isn't
at this point, but we were only tagging it as stopped a bit
afterwards.

handle_inferior_event()
{
...
  switch (infwait_state)
    {
...
    case infwait_nonstep_watch_state:
      if (debug_infrun)
        fprintf_unfiltered (gdb_stdlog,
			    "infrun: infwait_nonstep_watch_state\n");
      insert_breakpoints ();
...
      break;
...

    }
...

  reinit_frame_cache ();

  /* If it's a new process, add it to the thread database */

  ecs->new_thread_event = (!ptid_equal (ecs->ptid, inferior_ptid)
			   && !ptid_equal (ecs->ptid, minus_one_ptid)
			   && !in_thread_list (ecs->ptid));

  if (ecs->ws.kind != TARGET_WAITKIND_EXITED
      && ecs->ws.kind != TARGET_WAITKIND_SIGNALLED && ecs->new_thread_event)
    add_thread (ecs->ptid);

  if (ecs->ws.kind != TARGET_WAITKIND_IGNORE)
    {
      /* Mark the non-executing threads accordingly.  */
      if (!non_stop
 	  || ecs->ws.kind == TARGET_WAITKIND_EXITED
 	  || ecs->ws.kind == TARGET_WAITKIND_SIGNALLED)
 	set_executing (pid_to_ptid (-1), 0);
      else
 	set_executing (ecs->ptid, 0);
    }
...




> Index: gdb/infrun.c
> ===================================================================
> --- gdb.orig/infrun.c	2008-07-15 09:36:19.000000000 -0500
> +++ gdb/infrun.c	2008-07-15 09:37:20.000000000 -0500
> @@ -1828,6 +1828,29 @@
>
>    adjust_pc_after_break (ecs);
>
> +  reinit_frame_cache ();

As seen above, this was being done only after the insert_breakpoints call.
Inserting breakpoints ends up updating the watchpoints, which
triggers calls into frame handling (evaluating if the frame is
in scope, temporalily changing the selected frame, etc.).

There's a registers_changed call just before resuming and switching
to infwait_nonstep_watch_state (which calls reinit_frame_cache),
and we tell the target to step only one thread, so there should be no 
practical problem in leaving the call where it was, but, might
as well move up this call too while we're at it, in case something
else changes in the future, and uncovers this issue.

> +
> +  /* If it's a new process, add it to the thread database */
> +
> +  ecs->new_thread_event = (!ptid_equal (ecs->ptid, inferior_ptid)
> +			   && !ptid_equal (ecs->ptid, minus_one_ptid)
> +			   && !in_thread_list (ecs->ptid));
> +
> +  if (ecs->ws.kind != TARGET_WAITKIND_EXITED
> +      && ecs->ws.kind != TARGET_WAITKIND_SIGNALLED &&
> ecs->new_thread_event) +    add_thread (ecs->ptid);

This moves up along, so any new thread that ends up being added by
this is also tagged as stopped.  It's the default thread state, when
one adds a new thread, but we shouldn't rely on that.

> +
> +  if (ecs->ws.kind != TARGET_WAITKIND_IGNORE)
> +    {
> +      /* Mark the non-executing threads accordingly.  */
> +      if (!non_stop
> + 	  || ecs->ws.kind == TARGET_WAITKIND_EXITED
> + 	  || ecs->ws.kind == TARGET_WAITKIND_SIGNALLED)
> + 	set_executing (pid_to_ptid (-1), 0);
> +      else
> + 	set_executing (ecs->ptid, 0);
> +    }
> +

This moves up, because doing this later is what was creating
the problem in the first place, due to insert_breakpoints
ending up calling get_current_frame.

-- 
Pedro Alves