From: Pedro Alves <palves@redhat.com>
To: Yao Qi <qiyaoltc@gmail.com>
Cc: gdb-patches@sourceware.org
Subject: [pushed]Re: [PATCH/7.10 1/2] Linux gdbserver confused when event randomization returns a process exit event
Date: Thu, 06 Aug 2015 09:43:00 -0000 [thread overview]
Message-ID: <55C32C42.3020903@redhat.com> (raw)
In-Reply-To: <86io8weize.fsf@gmail.com>
On 08/03/2015 11:58 AM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
>
>> @@ -3128,7 +3116,7 @@ linux_wait_1 (ptid_t ptid,
>> || (gdb_breakpoint_here (event_child->stop_pc)
>> && gdb_condition_true_at_breakpoint (event_child->stop_pc)
>> && gdb_no_commands_at_breakpoint (event_child->stop_pc))
>> - || extended_event_reported (&event_child->waitstatus));
>> + || event_child->waitstatus.kind != TARGET_WAITKIND_IGNORE);
>>
>> run_breakpoint_commands (event_child->stop_pc);
>>
>> @@ -3150,9 +3138,11 @@ linux_wait_1 (ptid_t ptid,
>> paddress (event_child->stop_pc),
>> paddress (event_child->step_range_start),
>> paddress (event_child->step_range_end));
>> - if (extended_event_reported (&event_child->waitstatus))
>> + if (event_child->waitstatus.kind != TARGET_WAITKIND_IGNORE)
>> {
>> - char *str = target_waitstatus_to_string (ourstatus);
>> + char *str;
>> +
>> + str = target_waitstatus_to_string (&event_child->waitstatus);
>> debug_printf ("LWP %ld: extended event with waitstatus %s\n",
>> lwpid_of (get_lwp_thread (event_child)), str);
>> xfree (str);
>
> Looks this code is dead code even without your patch. This code is
> guarded by "if (!report_to_gdb)", but if report_to_gdb is false,
> "(event_child->waitstatus.kind != TARGET_WAITKIND_IGNORE)" is false too.
That bit went in separately, so here's the updated patch that I pushed.
I'll push it to 7.10 too in a bit.
---
From 00db26facc14ac830adef704bba9b24d0d366ddf Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Thu, 6 Aug 2015 10:30:17 +0100
Subject: [PATCH 2/3] Linux gdbserver confused when event randomization picks
process exit event
The tail end of linux_wait_1 isn't expecting that the select_event_lwp
machinery can pick a whole-process exit event to report to GDB. When
that happens, both gdb and gdbserver end up quite confused:
...
(gdb)
[Thread 24971.24971] #1 stopped.
0x0000003615a011f0 in ?? ()
c&
Continuing.
(gdb) [New Thread 24971.24981]
[New Thread 24983.24983]
[New Thread 24971.24982]
[Thread 24983.24983] #3 stopped.
0x0000003615ebc7cc in __libc_fork () at ../nptl/sysdeps/unix/sysv/linux/fork.c:130
130 pid = ARCH_FORK ();
[New Thread 24984.24984]
Error in re-setting breakpoint -16: PC register is not available
Error in re-setting breakpoint -17: PC register is not available
Error in re-setting breakpoint -18: PC register is not available
Error in re-setting breakpoint -19: PC register is not available
Error in re-setting breakpoint -24: PC register is not available
Error in re-setting breakpoint -25: PC register is not available
Error in re-setting breakpoint -26: PC register is not available
Error in re-setting breakpoint -27: PC register is not available
Error in re-setting breakpoint -28: PC register is not available
Error in re-setting breakpoint -29: PC register is not available
Error in re-setting breakpoint -30: PC register is not available
PC register is not available
(gdb)
gdb/gdbserver/ChangeLog:
2015-08-06 Pedro Alves <palves@redhat.com>
* linux-low.c (add_lwp): Set waitstatus to TARGET_WAITKIND_IGNORE.
(linux_thread_alive): Use lwp_is_marked_dead.
(extended_event_reported): Delete.
(linux_wait_1): Check if waitstatus is TARGET_WAITKIND_IGNORE
instead of extended_event_reported.
(mark_lwp_dead): Don't set the 'dead' flag. Store the waitstatus
as well.
(lwp_is_marked_dead): New function.
(lwp_running): Use lwp_is_marked_dead.
* linux-low.h: Delete 'dead' field, and update 'waitstatus's
comment.
---
gdb/gdbserver/ChangeLog | 14 +++++++++++
gdb/gdbserver/linux-low.c | 64 ++++++++++++++++++++++++++---------------------
gdb/gdbserver/linux-low.h | 11 +++-----
3 files changed, 54 insertions(+), 35 deletions(-)
diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index eb1101c..77b4330 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,5 +1,19 @@
2015-08-06 Pedro Alves <palves@redhat.com>
+ * linux-low.c (add_lwp): Set waitstatus to TARGET_WAITKIND_IGNORE.
+ (linux_thread_alive): Use lwp_is_marked_dead.
+ (extended_event_reported): Delete.
+ (linux_wait_1): Check if waitstatus is TARGET_WAITKIND_IGNORE
+ instead of extended_event_reported.
+ (mark_lwp_dead): Don't set the 'dead' flag. Store the waitstatus
+ as well.
+ (lwp_is_marked_dead): New function.
+ (lwp_running): Use lwp_is_marked_dead.
+ * linux-low.h: Delete 'dead' field, and update 'waitstatus's
+ comment.
+
+2015-08-06 Pedro Alves <palves@redhat.com>
+
* linux-low.c (linux_wait_1): Move fork event output out of the
!report_to_gdb check. Pass event_child->waitstatus to
target_waitstatus_to_string instead of ourstatus.
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 76b212d..6a7182a 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -267,6 +267,7 @@ static int linux_wait_for_event (ptid_t ptid, int *wstat, int options);
static struct lwp_info *add_lwp (ptid_t ptid);
static int linux_stopped_by_watchpoint (void);
static void mark_lwp_dead (struct lwp_info *lwp, int wstat);
+static int lwp_is_marked_dead (struct lwp_info *lwp);
static void proceed_all_lwps (void);
static int finish_step_over (struct lwp_info *lwp);
static int kill_lwp (unsigned long lwpid, int signo);
@@ -746,8 +747,9 @@ add_lwp (ptid_t ptid)
{
struct lwp_info *lwp;
- lwp = (struct lwp_info *) xmalloc (sizeof (*lwp));
- memset (lwp, 0, sizeof (*lwp));
+ lwp = (struct lwp_info *) xcalloc (1, sizeof (*lwp));
+
+ lwp->waitstatus.kind = TARGET_WAITKIND_IGNORE;
if (the_low_target.new_thread != NULL)
the_low_target.new_thread (lwp);
@@ -1396,7 +1398,7 @@ linux_thread_alive (ptid_t ptid)
exited but we still haven't been able to report it to GDB, we'll
hold on to the last lwp of the dead process. */
if (lwp != NULL)
- return !lwp->dead;
+ return !lwp_is_marked_dead (lwp);
else
return 0;
}
@@ -2750,20 +2752,6 @@ ignore_event (struct target_waitstatus *ourstatus)
return null_ptid;
}
-/* Return non-zero if WAITSTATUS reflects an extended linux
- event. Otherwise, return zero. */
-
-static int
-extended_event_reported (const struct target_waitstatus *waitstatus)
-{
- if (waitstatus == NULL)
- return 0;
-
- return (waitstatus->kind == TARGET_WAITKIND_FORKED
- || waitstatus->kind == TARGET_WAITKIND_VFORKED
- || waitstatus->kind == TARGET_WAITKIND_VFORK_DONE);
-}
-
/* Wait for process, returns status. */
static ptid_t
@@ -3131,7 +3119,7 @@ linux_wait_1 (ptid_t ptid,
|| (gdb_breakpoint_here (event_child->stop_pc)
&& gdb_condition_true_at_breakpoint (event_child->stop_pc)
&& gdb_no_commands_at_breakpoint (event_child->stop_pc))
- || extended_event_reported (&event_child->waitstatus));
+ || event_child->waitstatus.kind != TARGET_WAITKIND_IGNORE);
run_breakpoint_commands (event_child->stop_pc);
@@ -3183,7 +3171,7 @@ linux_wait_1 (ptid_t ptid,
if (debug_threads)
{
- if (extended_event_reported (&event_child->waitstatus))
+ if (event_child->waitstatus.kind != TARGET_WAITKIND_IGNORE)
{
char *str;
@@ -3271,12 +3259,11 @@ linux_wait_1 (ptid_t ptid,
unstop_all_lwps (1, event_child);
}
- if (extended_event_reported (&event_child->waitstatus))
+ if (event_child->waitstatus.kind != TARGET_WAITKIND_IGNORE)
{
- /* If the reported event is a fork, vfork or exec, let GDB know. */
- ourstatus->kind = event_child->waitstatus.kind;
- ourstatus->value = event_child->waitstatus.value;
-
+ /* If the reported event is an exit, fork, vfork or exec, let
+ GDB know. */
+ *ourstatus = event_child->waitstatus;
/* Clear the event lwp's waitstatus since we handled it already. */
event_child->waitstatus.kind = TARGET_WAITKIND_IGNORE;
}
@@ -3484,13 +3471,23 @@ suspend_and_send_sigstop_callback (struct inferior_list_entry *entry,
static void
mark_lwp_dead (struct lwp_info *lwp, int wstat)
{
- /* It's dead, really. */
- lwp->dead = 1;
-
/* Store the exit status for later. */
lwp->status_pending_p = 1;
lwp->status_pending = wstat;
+ /* Store in waitstatus as well, as there's nothing else to process
+ for this event. */
+ if (WIFEXITED (wstat))
+ {
+ lwp->waitstatus.kind = TARGET_WAITKIND_EXITED;
+ lwp->waitstatus.value.integer = WEXITSTATUS (wstat);
+ }
+ else if (WIFSIGNALED (wstat))
+ {
+ lwp->waitstatus.kind = TARGET_WAITKIND_SIGNALLED;
+ lwp->waitstatus.value.sig = gdb_signal_from_host (WTERMSIG (wstat));
+ }
+
/* Prevent trying to stop it. */
lwp->stopped = 1;
@@ -3498,6 +3495,17 @@ mark_lwp_dead (struct lwp_info *lwp, int wstat)
lwp->stop_expected = 0;
}
+/* Return true if LWP has exited already, and has a pending exit event
+ to report to GDB. */
+
+static int
+lwp_is_marked_dead (struct lwp_info *lwp)
+{
+ return (lwp->status_pending_p
+ && (WIFEXITED (lwp->status_pending)
+ || WIFSIGNALED (lwp->status_pending)));
+}
+
/* Wait for all children to stop for the SIGSTOPs we just queued. */
static void
@@ -3614,7 +3622,7 @@ lwp_running (struct inferior_list_entry *entry, void *data)
struct thread_info *thread = (struct thread_info *) entry;
struct lwp_info *lwp = get_thread_lwp (thread);
- if (lwp->dead)
+ if (lwp_is_marked_dead (lwp))
return 0;
if (lwp->stopped)
return 0;
diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h
index 24fb015..f8f6e78 100644
--- a/gdb/gdbserver/linux-low.h
+++ b/gdb/gdbserver/linux-low.h
@@ -261,16 +261,13 @@ struct lwp_info
event already received in a wait()). */
int stopped;
- /* If this flag is set, the lwp is known to be dead already (exit
- event already received in a wait(), and is cached in
- status_pending). */
- int dead;
-
/* When stopped is set, the last wait status recorded for this lwp. */
int last_status;
- /* This is used to store extended ptrace event information until
- it is reported to GDB. */
+ /* If WAITSTATUS->KIND != TARGET_WAITKIND_IGNORE, the waitstatus for
+ this LWP's last event, to pass to GDB without any further
+ processing. This is used to store extended ptrace event
+ information or exit status until it can be reported to GDB. */
struct target_waitstatus waitstatus;
/* When stopped is set, this is where the lwp last stopped, with
--
1.9.3
next prev parent reply other threads:[~2015-08-06 9:43 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-07-31 17:03 [PATCH/7.10 0/2] gdbserver: Fix several fork support (& co) issues Pedro Alves
2015-07-31 17:04 ` [PATCH/7.10 1/2] Linux gdbserver confused when event randomization returns a process exit event Pedro Alves
2015-08-03 10:59 ` Yao Qi
2015-08-06 9:34 ` [pushed][PATCH 1/3] Linux gdbserver fork event debug output (Re: [PATCH/7.10 1/2] Linux gdbserver confused when event randomization returns a process exit event) Pedro Alves
2015-08-06 9:43 ` Pedro Alves [this message]
2015-07-31 17:04 ` [PATCH/7.10 2/2] gdbserver: Fix non-stop / fork / step-over issues Pedro Alves
2015-07-31 18:04 ` Don Breazeal
2015-07-31 19:02 ` Pedro Alves
2015-08-05 22:19 ` Don Breazeal
2015-08-06 10:09 ` Pedro Alves
2015-08-03 15:14 ` Yao Qi
2015-08-03 16:20 ` Pedro Alves
2015-08-04 16:40 ` Pedro Alves
2015-08-05 11:41 ` Yao Qi
2015-08-05 15:10 ` Pedro Alves
2015-08-06 9:44 ` [pushed] " Pedro Alves
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=55C32C42.3020903@redhat.com \
--to=palves@redhat.com \
--cc=gdb-patches@sourceware.org \
--cc=qiyaoltc@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).