Re: [rfc] [2/7] infrun cleanup: miscellaneous cleanups

[Pedro Alves <pedro@codesourcery.com>]

On Sunday 07 December 2008 18:23:06, Ulrich Weigand wrote:

> Well, I was concerned about the cases where today handle_inferior_event
> *modifies* the ecs->ptid it gets as input (e.g. thread-hopping or switching
> back to the single-stepped thread).  I'm not sure if that can today actually
> ever cause it switch to a different *inferior*, but it appears it could.
> Using simply the caller's ptid means we will miss that switch.  Using
> inferior_ptid should catch it, however, as handle_inferior_event will
> have done a switch_to_thread in addition to changing ecs->ptid.

Do those cases cause handle_inferior_event to return 0?  Notice
that normal_stop uses the last target event (get_last_target_status)
to check if this was an inferior exit, because inferior_ptid will
usually be set to null_ptid on that case, at that point.  But
I do see what you mean.
There are several cases where we don't switch inferior_ptid
to the event ptid, which are wrong in multiprocess.  I just haven't
been afected by them yet (e.g., TARGET_WAITKIND_LOADED,
TARGET_WAITKIND_SYSCALL_ENTRY, new_thread_event), so I didn't
address it.

The issues I was mentioning here in particular were related to
inferior exits.

1) we are not switching inferior_ptid to the event ptid on an inferior
   exit (before calling target_mourn_inferior).  This is of course
   bad in multi-process, as we mourn the wrong inferior.  Furthermore,
   some targets return pid_to_ptid (-1) instead of the pid of the process
   that exited, when returning a TARGET_WAITKING_EXITED / TARGET_WAITKING_SIGNALLED.

2) On an inferior exit, the remote target is deleting the inferior from
   gdb's list even before target_mourn_inferior is called.  Calling
   current_inferior here will assert.

3) most targets' target_mourn_inferior calls generic_mourn_inferior,
   which deletes the dead inferior from gdb's list, and sets inferior_ptid
   to null_ptid.  This is done inside handle_inferior_event, so, on an inferior exit,
   inferior_ptid is most of the times null_ptid in the hunk we're talking about
   in fetch_inferior_event.

I'm going to post patches that address #1 and #2.  Changing
#3 involves further design decisions, so best to leave that out of
this series, IMO.

The 3 points above mean that unconditionally calling
current_inferior in fetch_inferior_event like you were doing asserts.

We could make fetch_inferior_event call normal_stop like:

if (ecs->ws.kind == TARGET_WAITKIND_EXITED
   || ecs->ws.kind == TARGET_WAITKIND_SIGNALLED
   || current_inferior ()->stop_soon == NO_STOP_QUIETLY)
  normal_stop ();

Maybe also add a:
   || ptid_equal (inferior_ptid, null_ptid))

... but that will still not be 100% correct due to the
missing inferior_ptid switches...

-- 
Pedro Alves