Re: mourn the correct dead inferior

[Pedro Alves <pedro@codesourcery.com>]

[-- Attachment #1: Type: text/plain, Size: 2305 bytes --]

Hi guys,

Can I take it that there are no objections to the target side bits?

Mark, adding you to CC in case you want to comment on the inf-ptrace bit?

I'd like to check this in so I can move on with some other
remote.c cleanups dependent on this.  I'll do any extra fixing if
it turns out necessary.

On Sunday 07 December 2008 20:16:09, Pedro Alves wrote:
> Here's a piece I mentioned a bit earlier today.  In multi-process,
> we may be calling target_mourn_inferior on the wrong inferior, if the
> inferior that exited happens to *not* be the current inferior the
> user has selected.  The obvious fix is to switch inferior_ptid
> to the ecs->ptid.
> 
> But, there are a few targets that return invalid ptids
> on a TARGET_WAITKIND_EXIT or TARGET_WAITKIND_SIGNALLED.  Even if
> they don't support multi-process, that means that later on,
> when target_mourn_inferior->generic_mourn_inferior is called,
> we would leave the dead inferior still listed (and if we have breakpoint
> locations per-inferior, as in the multiprocess branch, we will delete
> the breakpoint locations of the wrong inferior).
> 
> The linux-thread-db.c has no good reason to be filtering the ptid
> the beneath target returns, so that's easy to fix, just return
> that the beneath layer returned.
> 
> The other targets I'm adjusting, are doing this:
> 
>   if (pid == -1)
>     {
>       warning (_("Child process unexpectedly missing: %s"),
> 	       safe_strerror (save_errno));
> 
>       /* Claim it exited with unknown signal.  */
>       ourstatus->kind = TARGET_WAITKIND_SIGNALLED;
>       ourstatus->value.sig = TARGET_SIGNAL_UNKNOWN;
>       return minus_one_ptid;
>     }
> 
> I'm not 100% sure what this catches, and, if this can really happen.
> A 'kill -9' perhaps?  I've changed them to return inferior_ptid
> instead, which should still be OK, unless their target_mourn_inferior
> implementation could hang in that case --- I think not though.  If we find
> that this is a problem, then we'll need a new TARGET_WAITKIND_DISAPPEARED
> and perhaps a corresponding new target_ops method to handle it (or
> just call generic_mourn_inferior) ...
> 
> Comments?
> 
> (I think that with further cleanups, we'll be switching
> inferior_ptid unconditionaly to the event ptid in all-stop as well...)
> 

-- 
Pedro Alves

[-- Attachment #2: mourn_dead_inferior.diff --]
[-- Type: text/x-diff, Size: 3899 bytes --]

2008-12-07  Pedro Alves  <pedro@codesourcery.com>

	(handle_inferior_event): On a TARGET_WAITKIND_EXITED or
	TARGET_WAITKIND_SIGNALLED, switch inferior_ptid to the event ptid.
	* linux_thread_db.c (thread_db_wait): On a TARGET_WAITKIND_EXITED
	or TARGET_WAITKIND_SIGNALLED, return the ptid the beneath target
	returned.
	* inf-ptrace.c (inf_ptrace_wait): Return inferior_ptid instead of
	minus_one_ptid if the inferior disappeared.
	* rs6000-nat.c (rs6000_wait): Likewise.
	* spu-linux-nat.c (spu_child_wait): Likewise.

---
 gdb/inf-ptrace.c      |    2 +-
 gdb/infrun.c          |    2 ++
 gdb/linux-thread-db.c |    4 ++--
 gdb/rs6000-nat.c      |    2 +-
 gdb/spu-linux-nat.c   |    2 +-
 5 files changed, 7 insertions(+), 5 deletions(-)

Index: src/gdb/infrun.c
===================================================================
--- src.orig/gdb/infrun.c	2008-12-07 20:26:06.000000000 +0000
+++ src/gdb/infrun.c	2008-12-07 20:26:07.000000000 +0000
@@ -2271,6 +2271,7 @@ handle_inferior_event (struct execution_
     case TARGET_WAITKIND_EXITED:
       if (debug_infrun)
         fprintf_unfiltered (gdb_stdlog, "infrun: TARGET_WAITKIND_EXITED\n");
+      inferior_ptid = ecs->ptid;
       target_terminal_ours ();	/* Must do this before mourn anyway */
       print_stop_reason (EXITED, ecs->ws.value.integer);
 
@@ -2289,6 +2290,7 @@ handle_inferior_event (struct execution_
     case TARGET_WAITKIND_SIGNALLED:
       if (debug_infrun)
         fprintf_unfiltered (gdb_stdlog, "infrun: TARGET_WAITKIND_SIGNALLED\n");
+      inferior_ptid = ecs->ptid;
       stop_print_frame = 0;
       target_terminal_ours ();	/* Must do this before mourn anyway */
 
Index: src/gdb/linux-thread-db.c
===================================================================
--- src.orig/gdb/linux-thread-db.c	2008-12-07 20:26:06.000000000 +0000
+++ src/gdb/linux-thread-db.c	2008-12-07 20:26:39.000000000 +0000
@@ -888,8 +888,8 @@ thread_db_wait (ptid_t ptid, struct targ
     return ptid;
 
   if (ourstatus->kind == TARGET_WAITKIND_EXITED
-    || ourstatus->kind == TARGET_WAITKIND_SIGNALLED)
-    return pid_to_ptid (-1);
+      || ourstatus->kind == TARGET_WAITKIND_SIGNALLED)
+    return ptid;
 
   if (ourstatus->kind == TARGET_WAITKIND_EXECD)
     {
Index: src/gdb/inf-ptrace.c
===================================================================
--- src.orig/gdb/inf-ptrace.c	2008-12-07 20:26:06.000000000 +0000
+++ src/gdb/inf-ptrace.c	2008-12-07 20:26:07.000000000 +0000
@@ -410,7 +410,7 @@ inf_ptrace_wait (ptid_t ptid, struct tar
 	  /* Claim it exited with unknown signal.  */
 	  ourstatus->kind = TARGET_WAITKIND_SIGNALLED;
 	  ourstatus->value.sig = TARGET_SIGNAL_UNKNOWN;
-	  return minus_one_ptid;
+	  return inferior_ptid;
 	}
 
       /* Ignore terminated detached child processes.  */
Index: src/gdb/rs6000-nat.c
===================================================================
--- src.orig/gdb/rs6000-nat.c	2008-12-07 20:26:06.000000000 +0000
+++ src/gdb/rs6000-nat.c	2008-12-07 20:26:07.000000000 +0000
@@ -546,7 +546,7 @@ rs6000_wait (ptid_t ptid, struct target_
 	  /* Claim it exited with unknown signal.  */
 	  ourstatus->kind = TARGET_WAITKIND_SIGNALLED;
 	  ourstatus->value.sig = TARGET_SIGNAL_UNKNOWN;
-	  return minus_one_ptid;
+	  return inferior_ptid;
 	}
 
       /* Ignore terminated detached child processes.  */
Index: src/gdb/spu-linux-nat.c
===================================================================
--- src.orig/gdb/spu-linux-nat.c	2008-12-07 20:26:06.000000000 +0000
+++ src/gdb/spu-linux-nat.c	2008-12-07 20:26:07.000000000 +0000
@@ -444,7 +444,7 @@ spu_child_wait (ptid_t ptid, struct targ
       /* Claim it exited with unknown signal.  */
       ourstatus->kind = TARGET_WAITKIND_SIGNALLED;
       ourstatus->value.sig = TARGET_SIGNAL_UNKNOWN;
-      return minus_one_ptid;
+      return inferior_ptid;
     }
 
   store_waitstatus (ourstatus, status);