* mourn the correct dead inferior
@ 2008-12-07 20:17 Pedro Alves
2008-12-10 21:45 ` Pedro Alves
0 siblings, 1 reply; 4+ messages in thread
From: Pedro Alves @ 2008-12-07 20:17 UTC (permalink / raw)
To: gdb-patches; +Cc: Ulrich Weigand
[-- Attachment #1: Type: text/plain, Size: 1856 bytes --]
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: 3823 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 | 2 +-
gdb/rs6000-nat.c | 2 +-
gdb/spu-linux-nat.c | 2 +-
5 files changed, 6 insertions(+), 4 deletions(-)
Index: src/gdb/infrun.c
===================================================================
--- src.orig/gdb/infrun.c 2008-12-07 19:34:12.000000000 +0000
+++ src/gdb/infrun.c 2008-12-07 19:39:05.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 19:34:30.000000000 +0000
+++ src/gdb/linux-thread-db.c 2008-12-07 19:37:33.000000000 +0000
@@ -889,7 +889,7 @@ thread_db_wait (ptid_t ptid, struct targ
if (ourstatus->kind == TARGET_WAITKIND_EXITED
|| ourstatus->kind == TARGET_WAITKIND_SIGNALLED)
- return pid_to_ptid (-1);
+ return ptid;
if (ourstatus->kind == TARGET_WAITKIND_EXECD)
{
Index: src/gdb/inf-ptrace.c
===================================================================
--- src.orig/gdb/inf-ptrace.c 2008-12-07 19:34:33.000000000 +0000
+++ src/gdb/inf-ptrace.c 2008-12-07 19:36:26.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 19:34:36.000000000 +0000
+++ src/gdb/rs6000-nat.c 2008-12-07 19:36:43.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 19:34:59.000000000 +0000
+++ src/gdb/spu-linux-nat.c 2008-12-07 19:41:33.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);
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: mourn the correct dead inferior
2008-12-07 20:17 mourn the correct dead inferior Pedro Alves
@ 2008-12-10 21:45 ` Pedro Alves
2008-12-11 14:18 ` Daniel Jacobowitz
0 siblings, 1 reply; 4+ messages in thread
From: Pedro Alves @ 2008-12-10 21:45 UTC (permalink / raw)
To: gdb-patches; +Cc: Ulrich Weigand, Mark Kettenis
[-- 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);
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2008-12-11 19:22 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-12-07 20:17 mourn the correct dead inferior Pedro Alves
2008-12-10 21:45 ` Pedro Alves
2008-12-11 14:18 ` Daniel Jacobowitz
2008-12-11 19:22 ` Pedro Alves
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox