* [PATCH 13/18] infrun: Fix TARGET_WAITKIND_NO_RESUMED handling in non-stop mode
2015-10-14 15:28 [PATCH 00/18] Remote all-stop on top of non-stop Pedro Alves
@ 2015-10-14 15:28 ` Pedro Alves
2015-10-14 15:28 ` [PATCH 01/18] Fix mi-nonstop.exp with extended-remote Pedro Alves
` (19 subsequent siblings)
20 siblings, 0 replies; 60+ messages in thread
From: Pedro Alves @ 2015-10-14 15:28 UTC (permalink / raw)
To: gdb-patches
Running the testsuite against gdbserver with "maint set target-non-stop on"
stumbled on a set of problems. See code comments for details.
This handles my concerns expressed in PR14618.
gdb/ChangeLog:
2015-10-14 Pedro Alves <palves@redhat.com>
PR 14618
* infrun.c (handle_no_resumed): New function.
(handle_inferior_event_1) <TARGET_WAITKIND_NO_RESUMED>: Defer to
handle_no_resumed.
---
gdb/infrun.c | 111 ++++++++++++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 98 insertions(+), 13 deletions(-)
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 6052e8f..01d63fa 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -4612,6 +4612,102 @@ stop_all_threads (void)
fprintf_unfiltered (gdb_stdlog, "infrun: stop_all_threads done\n");
}
+/* Handle a TARGET_WAITKIND_NO_RESUMED event. */
+
+static int
+handle_no_resumed (struct execution_control_state *ecs)
+{
+ struct inferior *inf;
+ struct thread_info *thread;
+
+ if (target_can_async_p () && !sync_execution)
+ {
+ /* There were no unwaited-for children left in the target, but,
+ we're not synchronously waiting for events either. Just
+ ignore. */
+
+ if (debug_infrun)
+ fprintf_unfiltered (gdb_stdlog,
+ "infrun: TARGET_WAITKIND_NO_RESUMED " "(ignoring: bg)\n");
+ prepare_to_wait (ecs);
+ return 1;
+ }
+
+ /* Otherwise, if we were running a synchronous execution command, we
+ may need to cancel it and give the user back the terminal.
+
+ In non-stop mode, the target can't tell whether we've already
+ consumed previous stop events, so it can end up sending us a
+ no-resumed event like so:
+
+ #0 - thread 1 is left stopped
+
+ #1 - thread 2 is resumed and hits breakpoint
+ -> TARGET_WAITKIND_STOPPED
+
+ #2 - thread 3 is resumed and exits
+ this is the last resumed thread, so
+ -> TARGET_WAITKIND_NO_RESUMED
+
+ #3 - gdb processes stop for thread 2 and decides to re-resume
+ it.
+
+ #4 - gdb processes the TARGET_WAITKIND_NO_RESUMED event.
+ thread 2 is now resumed, so the event should be ignored.
+
+ IOW, if the stop for thread 2 doesn't end a foreground command,
+ then we need to ignore the following TARGET_WAITKIND_NO_RESUMED
+ event. But it could be that the event meant that thread 2 itself
+ (or whatever other thread was the last resumed thread) exited.
+
+ To address this we refresh the thread list and check whether we
+ have resumed threads _now_. In the example above, this removes
+ thread 3 from the thread list. If thread 2 was re-resumed, we
+ ignore this event. If we find no thread resumed, then we cancel
+ the synchronous command show "no unwaited-for " to the user. */
+ update_thread_list ();
+
+ ALL_NON_EXITED_THREADS (thread)
+ {
+ if (thread->executing
+ || thread->suspend.waitstatus_pending_p)
+ {
+ /* There were no unwaited-for children left in the target at
+ some point, but there are now. Just ignore. */
+ if (debug_infrun)
+ fprintf_unfiltered (gdb_stdlog,
+ "infrun: TARGET_WAITKIND_NO_RESUMED "
+ "(ignoring: found resumed)\n");
+ prepare_to_wait (ecs);
+ return 1;
+ }
+ }
+
+ /* Note however that we may find no resumed thread because the whole
+ process exited meanwhile (thus updating the thread list results
+ in an empty thread list). In this case we know we'll be getting
+ a process exit event shortly. */
+ ALL_INFERIORS (inf)
+ {
+ if (inf->pid == 0)
+ continue;
+
+ thread = any_live_thread_of_process (inf->pid);
+ if (thread == NULL)
+ {
+ if (debug_infrun)
+ fprintf_unfiltered (gdb_stdlog,
+ "infrun: TARGET_WAITKIND_NO_RESUMED "
+ "(expect process exit)\n");
+ prepare_to_wait (ecs);
+ return 1;
+ }
+ }
+
+ /* Go ahead and report the event. */
+ return 0;
+}
+
/* Given an execution control state that has been freshly filled in by
an event from the inferior, figure out what it means and take
appropriate action.
@@ -4656,19 +4752,8 @@ handle_inferior_event_1 (struct execution_control_state *ecs)
}
if (ecs->ws.kind == TARGET_WAITKIND_NO_RESUMED
- && target_can_async_p () && !sync_execution)
- {
- /* There were no unwaited-for children left in the target, but,
- we're not synchronously waiting for events either. Just
- ignore. Otherwise, if we were running a synchronous
- execution command, we need to cancel it and give the user
- back the terminal. */
- if (debug_infrun)
- fprintf_unfiltered (gdb_stdlog,
- "infrun: TARGET_WAITKIND_NO_RESUMED (ignoring)\n");
- prepare_to_wait (ecs);
- return;
- }
+ && handle_no_resumed (ecs))
+ return;
/* Cache the last pid/waitstatus. */
set_last_target_status (ecs->ptid, ecs->ws);
--
1.9.3
^ permalink raw reply [flat|nested] 60+ messages in thread* [PATCH 01/18] Fix mi-nonstop.exp with extended-remote
2015-10-14 15:28 [PATCH 00/18] Remote all-stop on top of non-stop Pedro Alves
2015-10-14 15:28 ` [PATCH 13/18] infrun: Fix TARGET_WAITKIND_NO_RESUMED handling in non-stop mode Pedro Alves
@ 2015-10-14 15:28 ` Pedro Alves
2015-10-14 15:28 ` [PATCH 03/18] attach + target always in non-stop mode: stop all threads Pedro Alves
` (18 subsequent siblings)
20 siblings, 0 replies; 60+ messages in thread
From: Pedro Alves @ 2015-10-14 15:28 UTC (permalink / raw)
To: gdb-patches
Testing with "maint set target-non-stop on" makes mi-nonstop.exp run
with the extended-remote board. That reveals that mi-nonstop.exp is
using the wrong predicate to check for "using remote protocol".
This is not visible today because non-stop tests all fail to run with
extended-remote board, because they spawn gdb and then do "set
non-stop on". However, with that board, gdb connects to the gdbserver
from within mi_gdb_start, and changing non-stop when already connected
doesn't work. Fix that by instead enabling non-stop mode on gdb's
command line.
gdb/testsuite/ChangeLog:
2015-10-14 Pedro Alves <palves@redhat.com>
* gdb.mi/mi-nonstop.exp: Append "set non-stop on" to GDBFLAGS
instead of issuing "-gdb-set non-stop 1" after starting gdb.
Use mi_is_target_remote instead of checking "is_remote target".
* lib/gdb.exp (gdb_is_target_remote): Rename to ...
(gdb_is_target_remote_prompt): ... this, and add 'prompt_regexp'
parameter.
(gdb_is_target_remote): Reimplement.
* lib/mi-support.exp (mi_is_target_remote): New procedure.
---
gdb/testsuite/gdb.mi/mi-nonstop.exp | 11 +++++++----
gdb/testsuite/lib/gdb.exp | 20 ++++++++++++++------
gdb/testsuite/lib/mi-support.exp | 9 +++++++++
3 files changed, 30 insertions(+), 10 deletions(-)
diff --git a/gdb/testsuite/gdb.mi/mi-nonstop.exp b/gdb/testsuite/gdb.mi/mi-nonstop.exp
index 4678506..5268716 100644
--- a/gdb/testsuite/gdb.mi/mi-nonstop.exp
+++ b/gdb/testsuite/gdb.mi/mi-nonstop.exp
@@ -23,8 +23,12 @@ load_lib mi-support.exp
set MIFLAGS "-i=mi"
gdb_exit
-if {[mi_gdb_start]} {
- continue
+
+save_vars { GDBFLAGS } {
+ append GDBFLAGS " -ex \"set non-stop on\""
+ if {[mi_gdb_start]} {
+ continue
+ }
}
proc mi_nonstop_resume { command test } {
@@ -49,7 +53,6 @@ if {[gdb_compile_pthreads "$srcdir/$subdir/$srcfile" $binfile executable $option
mi_gdb_reinitialize_dir $srcdir/$subdir
mi_gdb_load $binfile
-mi_gdb_test "-gdb-set non-stop 1" ".*"
mi_gdb_test "-gdb-set mi-async 1" ".*"
mi_detect_async
@@ -132,7 +135,7 @@ mi_gdb_test "-thread-select 2" "\\^done.*" "select first worker thread"
mi_gdb_test "-gdb-set --thread 3 variable exit_first_thread=1" ".*\\^done" "ask the second thread to exit"
set test "wait for thread exit"
-if { [is_remote target] } {
+if { [mi_is_target_remote] } {
# The remote protocol doesn't have support for thread exit
# notifications.
unsupported $test
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 9eaf721..3e8d8d6 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -3017,25 +3017,33 @@ proc skip_compile_feature_tests {} {
return $result
}
-# Check whether we're testing with the remote or extended-remote
-# targets.
+# Helper for gdb_is_target_remote. PROMPT_REGEXP is the expected
+# prompt.
-proc gdb_is_target_remote {} {
- global gdb_prompt
+proc gdb_is_target_remote_prompt { prompt_regexp } {
set test "probe for target remote"
gdb_test_multiple "maint print target-stack" $test {
- -re ".*emote serial target in gdb-specific protocol.*$gdb_prompt $" {
+ -re ".*emote serial target in gdb-specific protocol.*$prompt_regexp" {
pass $test
return 1
}
- -re "$gdb_prompt $" {
+ -re "$prompt_regexp" {
pass $test
}
}
return 0
}
+# Check whether we're testing with the remote or extended-remote
+# targets.
+
+proc gdb_is_target_remote {} {
+ global gdb_prompt
+
+ return [gdb_is_target_remote_prompt "$gdb_prompt $"]
+}
+
# Return 1 if the current remote target is an instance of our GDBserver, 0
# otherwise. Return -1 if there was an error and we can't tell.
diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp
index dd6c41a..34ef72d 100644
--- a/gdb/testsuite/lib/mi-support.exp
+++ b/gdb/testsuite/lib/mi-support.exp
@@ -2510,3 +2510,12 @@ proc mi_skip_python_tests {} {
global mi_gdb_prompt
return [skip_python_tests_prompt "$mi_gdb_prompt$"]
}
+
+# Check whether we're testing with the remote or extended-remote
+# targets.
+
+proc mi_is_target_remote {} {
+ global mi_gdb_prompt
+
+ return [gdb_is_target_remote_prompt "$mi_gdb_prompt"]
+}
--
1.9.3
^ permalink raw reply [flat|nested] 60+ messages in thread* [PATCH 03/18] attach + target always in non-stop mode: stop all threads
2015-10-14 15:28 [PATCH 00/18] Remote all-stop on top of non-stop Pedro Alves
2015-10-14 15:28 ` [PATCH 13/18] infrun: Fix TARGET_WAITKIND_NO_RESUMED handling in non-stop mode Pedro Alves
2015-10-14 15:28 ` [PATCH 01/18] Fix mi-nonstop.exp with extended-remote Pedro Alves
@ 2015-10-14 15:28 ` Pedro Alves
2015-10-26 13:22 ` Yao Qi
2015-10-14 15:28 ` [PATCH 02/18] Remote all-stop-on-top-of-non-stop Pedro Alves
` (17 subsequent siblings)
20 siblings, 1 reply; 60+ messages in thread
From: Pedro Alves @ 2015-10-14 15:28 UTC (permalink / raw)
To: gdb-patches
When running with "maint set target-non-stop on", and in all-stop
mode, nothing is stopping all threads after attaching. vAttach in
non-stop can leave all threads running and GDB has to explicitly pause
them.
This is not visible with the native target, as in that case, attach
always stops all threads (the core re-resumes them in case of
"attach&").
In addition, it's not defined whith thread manages to report the
initial attach stop, so always pick the lowest one (otherwise
multi-attach.exp regresses).
gdb/ChangeLog:
2015-10-14 Pedro Alves <palves@redhat.com>
* infcmd.c (attach_post_wait): If the target is always in non-stop
mode, and the UI is in all-stop mode, stop all threads and pick
the one with lowest number as current.
---
gdb/infcmd.c | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 9aae860..8fb0ff1 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -2697,8 +2697,30 @@ attach_post_wait (char *args, int from_tty, enum attach_post_wait_mode mode)
selected thread is stopped, others may still be executing.
Be sure to explicitly stop all threads of the process. This
should have no effect on already stopped threads. */
- if (target_is_non_stop_p ())
+ if (non_stop)
target_stop (pid_to_ptid (inferior->pid));
+ else if (target_is_non_stop_p ())
+ {
+ struct thread_info *thread;
+ struct thread_info *lowest = inferior_thread ();
+ int pid = current_inferior ()->pid;
+
+ stop_all_threads ();
+
+ /* It's not defined which thread will report the attach
+ stop. For consistency, always select the thread with
+ lowest number. */
+ ALL_NON_EXITED_THREADS (thread)
+ {
+ if (ptid_get_pid (thread->ptid) == pid)
+ {
+ if (thread->num < lowest->num)
+ lowest = thread;
+ }
+ }
+
+ switch_to_thread (lowest->ptid);
+ }
/* Tell the user/frontend where we're stopped. */
normal_stop ();
--
1.9.3
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH 03/18] attach + target always in non-stop mode: stop all threads
2015-10-14 15:28 ` [PATCH 03/18] attach + target always in non-stop mode: stop all threads Pedro Alves
@ 2015-10-26 13:22 ` Yao Qi
2015-11-23 18:15 ` Pedro Alves
0 siblings, 1 reply; 60+ messages in thread
From: Yao Qi @ 2015-10-26 13:22 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
Pedro Alves <palves@redhat.com> writes:
> In addition, it's not defined whith thread manages to report the
s/whith/which
> initial attach stop, so always pick the lowest one (otherwise
> multi-attach.exp regresses).
Shouldn't GDB pick the main thread rather than the lowest one?
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH 03/18] attach + target always in non-stop mode: stop all threads
2015-10-26 13:22 ` Yao Qi
@ 2015-11-23 18:15 ` Pedro Alves
2015-11-23 18:42 ` Pedro Alves
2015-11-26 16:12 ` Yao Qi
0 siblings, 2 replies; 60+ messages in thread
From: Pedro Alves @ 2015-11-23 18:15 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On 10/26/2015 10:22 AM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
>
>> In addition, it's not defined whith thread manages to report the
>
> s/whith/which
>
>> initial attach stop, so always pick the lowest one (otherwise
>> multi-attach.exp regresses).
>
> Shouldn't GDB pick the main thread rather than the lowest one?
It actually ends up being the same, and bit more generic to go with
lowest, because target_attach should always adds the main thread first.
Otherwise, there's no such concept of "main" thread in the common code.
"The thread with pid == lwpid" holds true for NTPL/Linux, but not everywhere,
and even then the leader thread may have exited already. I'll update
the comment.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH 03/18] attach + target always in non-stop mode: stop all threads
2015-11-23 18:15 ` Pedro Alves
@ 2015-11-23 18:42 ` Pedro Alves
2015-11-26 16:12 ` Yao Qi
1 sibling, 0 replies; 60+ messages in thread
From: Pedro Alves @ 2015-11-23 18:42 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On 11/23/2015 06:15 PM, Pedro Alves wrote:
> On 10/26/2015 10:22 AM, Yao Qi wrote:
>> Pedro Alves <palves@redhat.com> writes:
>>
>>> In addition, it's not defined whith thread manages to report the
>>
>> s/whith/which
>>
>>> initial attach stop, so always pick the lowest one (otherwise
>>> multi-attach.exp regresses).
>>
>> Shouldn't GDB pick the main thread rather than the lowest one?
>
> It actually ends up being the same, and bit more generic to go with
> lowest, because target_attach should always adds the main thread first.
> Otherwise, there's no such concept of "main" thread in the common code.
> "The thread with pid == lwpid" holds true for NTPL/Linux, but not everywhere,
> and even then the leader thread may have exited already. I'll update
> the comment.
>
Like so?
Subject: [PATCH] attach + target always in non-stop mode: stop all threads
When running with "maint set target-non-stop on", and in all-stop
mode, nothing is stopping all threads after attaching. vAttach in
non-stop can leave all threads running and GDB has to explicitly pause
them.
This is not visible with the native target, as in that case, attach
always stops all threads (the core re-resumes them in case of
"attach&").
In addition, it's not defined which thread manages to report the
initial attach stop, so always pick the lowest one (otherwise
multi-attach.exp regresses).
gdb/ChangeLog:
2015-11-23 Pedro Alves <palves@redhat.com>
* infcmd.c (attach_post_wait): If the target is always in non-stop
mode, and the UI is in all-stop mode, stop all threads and pick
the one with lowest number as current.
---
gdb/infcmd.c | 25 ++++++++++++++++++++++++-
1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 9aae860..e481feb 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -2697,8 +2697,31 @@ attach_post_wait (char *args, int from_tty, enum attach_post_wait_mode mode)
selected thread is stopped, others may still be executing.
Be sure to explicitly stop all threads of the process. This
should have no effect on already stopped threads. */
- if (target_is_non_stop_p ())
+ if (non_stop)
target_stop (pid_to_ptid (inferior->pid));
+ else if (target_is_non_stop_p ())
+ {
+ struct thread_info *thread;
+ struct thread_info *lowest = inferior_thread ();
+ int pid = current_inferior ()->pid;
+
+ stop_all_threads ();
+
+ /* It's not defined which thread will report the attach
+ stop. For consistency, always select the thread with
+ lowest number, which should be the main thread, if it
+ still exists. */
+ ALL_NON_EXITED_THREADS (thread)
+ {
+ if (ptid_get_pid (thread->ptid) == pid)
+ {
+ if (thread->num < lowest->num)
+ lowest = thread;
+ }
+ }
+
+ switch_to_thread (lowest->ptid);
+ }
/* Tell the user/frontend where we're stopped. */
normal_stop ();
--
1.9.3
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH 03/18] attach + target always in non-stop mode: stop all threads
2015-11-23 18:15 ` Pedro Alves
2015-11-23 18:42 ` Pedro Alves
@ 2015-11-26 16:12 ` Yao Qi
2015-11-26 16:23 ` Pedro Alves
1 sibling, 1 reply; 60+ messages in thread
From: Yao Qi @ 2015-11-26 16:12 UTC (permalink / raw)
To: Pedro Alves; +Cc: Yao Qi, gdb-patches
Pedro Alves <palves@redhat.com> writes:
> It actually ends up being the same, and bit more generic to go with
> lowest, because target_attach should always adds the main thread first.
> Otherwise, there's no such concept of "main" thread in the common code.
> "The thread with pid == lwpid" holds true for NTPL/Linux, but not everywhere,
> and even then the leader thread may have exited already. I'll update
> the comment.
Yes, there is no such concept of "main" thread. My real concern on
"choosing lowest one" is about pid recycling. Our intention is to pick up
main thread, but the lowest one may not be the main thread if pid
recycling is considered.
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH 03/18] attach + target always in non-stop mode: stop all threads
2015-11-26 16:12 ` Yao Qi
@ 2015-11-26 16:23 ` Pedro Alves
2015-11-27 9:33 ` Yao Qi
0 siblings, 1 reply; 60+ messages in thread
From: Pedro Alves @ 2015-11-26 16:23 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On 11/26/2015 04:12 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
>
>> It actually ends up being the same, and bit more generic to go with
>> lowest, because target_attach should always adds the main thread first.
>> Otherwise, there's no such concept of "main" thread in the common code.
>> "The thread with pid == lwpid" holds true for NTPL/Linux, but not everywhere,
>> and even then the leader thread may have exited already. I'll update
>> the comment.
>
> Yes, there is no such concept of "main" thread. My real concern on
> "choosing lowest one" is about pid recycling. Our intention is to pick up
> main thread, but the lowest one may not be the main thread if pid
> recycling is considered.
But this the GDB thread id. If a new thread recycles an old pid/tid, it'll
still get a new GDB thread id. (And if GDB thread ids wrap around we have
bigger problems elsewhere, which I think we'd likely sort by simply making
gdb thread ids 64-bit instead).
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH 03/18] attach + target always in non-stop mode: stop all threads
2015-11-26 16:23 ` Pedro Alves
@ 2015-11-27 9:33 ` Yao Qi
0 siblings, 0 replies; 60+ messages in thread
From: Yao Qi @ 2015-11-27 9:33 UTC (permalink / raw)
To: Pedro Alves; +Cc: Yao Qi, gdb-patches
Pedro Alves <palves@redhat.com> writes:
> But this the GDB thread id. If a new thread recycles an old pid/tid, it'll
> still get a new GDB thread id. (And if GDB thread ids wrap around we have
> bigger problems elsewhere, which I think we'd likely sort by simply making
> gdb thread ids 64-bit instead).
Oh, we compare thread->num, which is the thread id in GDB. I thought we
compare process/thread id. Sorry about that. Patch is good to me.
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH 02/18] Remote all-stop-on-top-of-non-stop
2015-10-14 15:28 [PATCH 00/18] Remote all-stop on top of non-stop Pedro Alves
` (2 preceding siblings ...)
2015-10-14 15:28 ` [PATCH 03/18] attach + target always in non-stop mode: stop all threads Pedro Alves
@ 2015-10-14 15:28 ` Pedro Alves
2015-10-24 22:39 ` Yao Qi
2015-10-14 15:28 ` [PATCH 15/18] gdbserver:prepare_access_memory: pick another thread Pedro Alves
` (16 subsequent siblings)
20 siblings, 1 reply; 60+ messages in thread
From: Pedro Alves @ 2015-10-14 15:28 UTC (permalink / raw)
To: gdb-patches
This is the first pass at implementing support for all-stop mode
running against the remote target using the non-stop variant of the
protocol.
The trickiest part here is the initial connection setup/synching. We
need to fetch all inferiors' target descriptions etc. before stopping
threads, because stop_all_threads needs to read the threads' registers
(to record each thread's stop_pc). But OTOH, the initial inferior
setup (target_post_attach, post_create_inferior, etc.), only works
correctly if the inferior is stopped... So I've split that initial
setup part from attach_command_post_wait to a separate function, and
added a "still needs setup" flag to the inferior structure. This is
similar to gdbserver/linux-low.c's handling of discovering the
process's target description). Then if on connection all threads of
the remote inferior are running, when we go about stopping them, as
soon as they stop we call setup_inferior, from within
stop_all_threads.
Also, in all-stop, we need to process all the initial stop replies to
learn about all the pending signal the threads may already be stopped
for, and pick the one to report as current. This is exposed by
gdb.threads/reconnect-signal.exp.
gdb/
2015-10-14 Pedro Alves <palves@redhat.com>
* gdbthread.h (switch_to_thread_no_regs): Declare.
* infcmd.c (setup_inferior): New function, factored out from ...
(attach_command_post_wait): ... this. Rename to ...
(attach_post_wait): ... this. Replace parameter async_exec with
attach_post_wait_mode parameter. Adjust.
(enum attach_post_wait_mode): New enum.
(struct attach_command_continuation_args): Replace 'async_exec'
field with 'mode' field.
(attach_command_continuation): Adjust.
(attach_command): Add comment. Mark the inferior as needing
setup. Adjust to use enum attach_post_wait_mode.
(notice_new_inferior): Use switch_to_thread_no_regs. Adjust to
use enum attach_post_wait_mode.
* inferior.h (setup_inferior): Declare.
(struct inferior) <needs_setup>: New field.
* infrun.c (set_last_target_status): Make extern.
(stop_all_threads): Make extern. Setup inferior, if necessary.
* infrun.h (set_last_target_status, stop_all_threads): Declare.
* remote-notif.c (remote_async_get_pending_events_handler)
(handle_notification): Replace non_stop checks with
target_is_non_stop_p() checks.
* remote.c (remote_notice_new_inferior): Remove non_stop check.
(remote_update_thread_list): Replace non_stop check with
target_is_non_stop_p() check.
(print_one_stopped_thread): New function.
(process_initial_stop_replies): New 'from_tty' parameter.
"Notice" all new live inferiors after storing initial stops as
pending status in each corresponding thread. If all-stop, stop
all threads, try picking a signalled thread as current, and print
the status of that one thread. Record the last target status.
(remote_start_remote): Replace non_stop checks with
target_is_non_stop_p() checks. Don't query for the remote current
thread of use qOffsets here. Pass from_tty to
process_initial_stop_replies.
(extended_remote_attach): Replace non_stop checks with
target_is_non_stop_p() checks.
(extended_remote_post_attach): Send qOffsets here.
(remote_vcont_resume, remote_resume, remote_stop)
(remote_interrupt, remote_parse_stop_reply, remote_wait): Replace
non_stop checks with target_is_non_stop_p() checks.
(remote_async): If target is non-stop, mark/clear the pending
events token.
* thread.c (switch_to_thread_no_regs): New function.
---
gdb/gdbthread.h | 7 +-
gdb/infcmd.c | 77 +++++++++++++++------
gdb/inferior.h | 12 ++++
gdb/infrun.c | 18 +++--
gdb/infrun.h | 6 ++
gdb/remote-notif.c | 4 +-
gdb/remote.c | 200 +++++++++++++++++++++++++++++++++++++++++------------
gdb/thread.c | 16 +++++
8 files changed, 267 insertions(+), 73 deletions(-)
diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index 0f9734d..eb7f8b8 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -411,9 +411,14 @@ extern struct thread_info *iterate_over_threads (thread_callback_func, void *);
extern int thread_count (void);
-/* Switch from one thread to another. */
+/* Switch from one thread to another. Also sets the STOP_PC
+ global. */
extern void switch_to_thread (ptid_t ptid);
+/* Switch from one thread to another. Does not read registers and
+ sets STOP_PC to -1. */
+extern void switch_to_thread_no_regs (struct thread_info *thread);
+
/* Marks or clears thread(s) PTID as resumed. If PTID is
MINUS_ONE_PTID, applies to all threads. If ptid_is_pid(PTID) is
true, applies to all threads of the process pointed at by PTID. */
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 54aa1ef..9aae860 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -2609,18 +2609,15 @@ proceed_after_attach (int pid)
do_cleanups (old_chain);
}
-/* attach_command --
- takes a program started up outside of gdb and ``attaches'' to it.
- This stops it cold in its tracks and allows us to start debugging it.
- and wait for the trace-trap that results from attaching. */
+/* See inferior.h. */
-static void
-attach_command_post_wait (char *args, int from_tty, int async_exec)
+void
+setup_inferior (int from_tty)
{
struct inferior *inferior;
inferior = current_inferior ();
- inferior->control.stop_soon = NO_STOP_QUIETLY;
+ inferior->needs_setup = 0;
/* If no exec file is yet known, try to determine it from the
process itself. */
@@ -2636,8 +2633,37 @@ attach_command_post_wait (char *args, int from_tty, int async_exec)
target_post_attach (ptid_get_pid (inferior_ptid));
post_create_inferior (¤t_target, from_tty);
+}
+
+/* What to do after the first program stops after attaching. */
+enum attach_post_wait_mode
+{
+ /* Do nothing. Leaves threads as they are. */
+ ATTACH_POST_WAIT_NOTHING,
+
+ /* Re-resume threads that are marked running. */
+ ATTACH_POST_WAIT_RESUME,
+
+ /* Stop all threads. */
+ ATTACH_POST_WAIT_STOP,
+};
+
+/* Called after we've attached to a process and we've seen it stop for
+ the first time. If ASYNC_EXEC is true, re-resume threads that
+ should be running. Else if ATTACH, */
+
+static void
+attach_post_wait (char *args, int from_tty, enum attach_post_wait_mode mode)
+{
+ struct inferior *inferior;
- if (async_exec)
+ inferior = current_inferior ();
+ inferior->control.stop_soon = NO_STOP_QUIETLY;
+
+ if (inferior->needs_setup)
+ setup_inferior (from_tty);
+
+ if (mode == ATTACH_POST_WAIT_RESUME)
{
/* The user requested an `attach&', so be sure to leave threads
that didn't get a signal running. */
@@ -2657,7 +2683,7 @@ attach_command_post_wait (char *args, int from_tty, int async_exec)
}
}
}
- else
+ else if (mode == ATTACH_POST_WAIT_STOP)
{
/* The user requested a plain `attach', so be sure to leave
the inferior stopped. */
@@ -2685,7 +2711,7 @@ struct attach_command_continuation_args
{
char *args;
int from_tty;
- int async_exec;
+ enum attach_post_wait_mode mode;
};
static void
@@ -2697,7 +2723,7 @@ attach_command_continuation (void *args, int err)
if (err)
return;
- attach_command_post_wait (a->args, a->from_tty, a->async_exec);
+ attach_post_wait (a->args, a->from_tty, a->mode);
}
static void
@@ -2710,12 +2736,18 @@ attach_command_continuation_free_args (void *args)
xfree (a);
}
+/* "attach" command entry point. Takes a program started up outside
+ of gdb and ``attaches'' to it. This stops it cold in its tracks
+ and allows us to start debugging it. */
+
void
attach_command (char *args, int from_tty)
{
int async_exec;
struct cleanup *args_chain;
struct target_ops *attach_target;
+ struct inferior *inferior = current_inferior ();
+ enum attach_post_wait_mode mode;
dont_repeat (); /* Not for the faint of heart */
@@ -2775,6 +2807,8 @@ attach_command (char *args, int from_tty)
init_wait_for_inferior ();
clear_proceed_status (0);
+ inferior->needs_setup = 1;
+
if (target_is_non_stop_p ())
{
/* If we find that the current thread isn't stopped, explicitly
@@ -2790,12 +2824,13 @@ attach_command (char *args, int from_tty)
target_stop (pid_to_ptid (ptid_get_pid (inferior_ptid)));
}
+ mode = async_exec ? ATTACH_POST_WAIT_RESUME : ATTACH_POST_WAIT_STOP;
+
/* Some system don't generate traps when attaching to inferior.
E.g. Mach 3 or GNU hurd. */
if (!target_attach_no_wait)
{
struct attach_command_continuation_args *a;
- struct inferior *inferior = current_inferior ();
/* Careful here. See comments in inferior.h. Basically some
OSes don't ignore SIGSTOPs on continue requests anymore. We
@@ -2808,7 +2843,7 @@ attach_command (char *args, int from_tty)
a = XNEW (struct attach_command_continuation_args);
a->args = xstrdup (args);
a->from_tty = from_tty;
- a->async_exec = async_exec;
+ a->mode = mode;
add_inferior_continuation (attach_command_continuation, a,
attach_command_continuation_free_args);
/* Done with ARGS. */
@@ -2822,7 +2857,7 @@ attach_command (char *args, int from_tty)
/* Done with ARGS. */
do_cleanups (args_chain);
- attach_command_post_wait (args, from_tty, async_exec);
+ attach_post_wait (args, from_tty, mode);
}
/* We had just found out that the target was already attached to an
@@ -2837,7 +2872,7 @@ void
notice_new_inferior (ptid_t ptid, int leave_running, int from_tty)
{
struct cleanup* old_chain;
- int async_exec;
+ enum attach_post_wait_mode mode;
old_chain = make_cleanup (null_cleanup, NULL);
@@ -2845,12 +2880,14 @@ notice_new_inferior (ptid_t ptid, int leave_running, int from_tty)
they're stopped for some reason other than us telling it to, the
target reports a signal != GDB_SIGNAL_0. We don't try to
resume threads with such a stop signal. */
- async_exec = non_stop;
+ mode = non_stop ? ATTACH_POST_WAIT_RESUME : ATTACH_POST_WAIT_NOTHING;
if (!ptid_equal (inferior_ptid, null_ptid))
make_cleanup_restore_current_thread ();
- switch_to_thread (ptid);
+ /* Avoid reading registers -- we haven't fetched the target
+ description yet. */
+ switch_to_thread_no_regs (find_thread_ptid (ptid));
/* When we "notice" a new inferior we need to do all the things we
would normally do if we had just attached to it. */
@@ -2871,7 +2908,7 @@ notice_new_inferior (ptid_t ptid, int leave_running, int from_tty)
a = XNEW (struct attach_command_continuation_args);
a->args = xstrdup ("");
a->from_tty = from_tty;
- a->async_exec = async_exec;
+ a->mode = mode;
add_inferior_continuation (attach_command_continuation, a,
attach_command_continuation_free_args);
@@ -2879,8 +2916,8 @@ notice_new_inferior (ptid_t ptid, int leave_running, int from_tty)
return;
}
- async_exec = leave_running;
- attach_command_post_wait ("" /* args */, from_tty, async_exec);
+ mode = leave_running ? ATTACH_POST_WAIT_RESUME : ATTACH_POST_WAIT_NOTHING;
+ attach_post_wait ("" /* args */, from_tty, mode);
do_cleanups (old_chain);
}
diff --git a/gdb/inferior.h b/gdb/inferior.h
index e09cb00..d3cf615 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -143,6 +143,12 @@ extern char *construct_inferior_arguments (int, char **);
/* From infcmd.c */
+/* Initial inferior setup. Determines the exec file is not yet known,
+ takes any necessary post-attaching actions, fetches the target
+ description and syncs the shared library list. */
+
+extern void setup_inferior (int from_tty);
+
extern void post_create_inferior (struct target_ops *, int);
extern void attach_command (char *, int);
@@ -364,6 +370,12 @@ struct inferior
specific thread, see `struct thread_info'. */
struct continuation *continuations;
+ /* True if setup_inferior wasn't called for this inferior yet.
+ Until that is done, we must not access inferior memory or
+ registers, as we haven't determined the target
+ architecture/description. */
+ int needs_setup;
+
/* Private data used by the target vector implementation. */
struct private_inferior *priv;
diff --git a/gdb/infrun.c b/gdb/infrun.c
index cf91370..d2587f1 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -2017,7 +2017,6 @@ static void keep_going_pass_signal (struct execution_control_state *ecs);
static void prepare_to_wait (struct execution_control_state *ecs);
static int keep_going_stepped_thread (struct thread_info *tp);
static int thread_still_needs_step_over (struct thread_info *tp);
-static void stop_all_threads (void);
/* Are there any pending step-over requests? If so, run all we can
now and return true. Otherwise, return false. */
@@ -3967,7 +3966,7 @@ init_thread_stepping_state (struct thread_info *tss)
/* Set the cached copy of the last ptid/waitstatus. */
-static void
+void
set_last_target_status (ptid_t ptid, struct target_waitstatus status)
{
target_last_wait_ptid = ptid;
@@ -4389,9 +4388,9 @@ save_waitstatus (struct thread_info *tp, struct target_waitstatus *ws)
}
}
-/* Stop all threads. */
+/* See infrun.h. */
-static void
+void
stop_all_threads (void)
{
/* We may need multiple passes to discover all threads. */
@@ -4500,6 +4499,8 @@ stop_all_threads (void)
}
else
{
+ struct inferior *inf;
+
t = find_thread_ptid (event_ptid);
if (t == NULL)
t = add_thread (event_ptid);
@@ -4509,6 +4510,15 @@ stop_all_threads (void)
t->resumed = 0;
t->control.may_range_step = 0;
+ /* This may be the first time we see the inferior report
+ a stop. */
+ inf = find_inferior_ptid (event_ptid);
+ if (inf->needs_setup)
+ {
+ switch_to_thread_no_regs (t);
+ setup_inferior (0);
+ }
+
if (ws.kind == TARGET_WAITKIND_STOPPED
&& ws.value.sig == GDB_SIGNAL_0)
{
diff --git a/gdb/infrun.h b/gdb/infrun.h
index ab27538..679bedf 100644
--- a/gdb/infrun.h
+++ b/gdb/infrun.h
@@ -114,6 +114,12 @@ extern int normal_stop (void);
extern void get_last_target_status (ptid_t *ptid,
struct target_waitstatus *status);
+extern void set_last_target_status (ptid_t ptid,
+ struct target_waitstatus status);
+
+/* Stop all threads. Only returns after everything is halted. */
+extern void stop_all_threads (void);
+
extern void prepare_for_detach (void);
extern void fetch_inferior_event (void *);
diff --git a/gdb/remote-notif.c b/gdb/remote-notif.c
index a8a6285..4377bda 100644
--- a/gdb/remote-notif.c
+++ b/gdb/remote-notif.c
@@ -116,7 +116,7 @@ remote_notif_process (struct remote_notif_state *state,
static void
remote_async_get_pending_events_handler (gdb_client_data data)
{
- gdb_assert (non_stop);
+ gdb_assert (target_is_non_stop_p ());
remote_notif_process ((struct remote_notif_state *) data, NULL);
}
@@ -166,7 +166,7 @@ handle_notification (struct remote_notif_state *state, char *buf)
/* Notify the event loop there's a stop reply to acknowledge
and that there may be more events to fetch. */
QUEUE_enque (notif_client_p, state->notif_queue, nc);
- if (non_stop)
+ if (target_is_non_stop_p ())
{
/* In non-stop, We mark REMOTE_ASYNC_GET_PENDING_EVENTS_TOKEN
in order to go on what we were doing and postpone
diff --git a/gdb/remote.c b/gdb/remote.c
index f40f791..ff25a76 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1885,7 +1885,7 @@ remote_notice_new_inferior (ptid_t currthread, int running)
{
struct remote_state *rs = get_remote_state ();
- if (non_stop || !rs->starting_up)
+ if (!rs->starting_up)
notice_new_inferior (currthread, running, 0);
}
}
@@ -3141,7 +3141,7 @@ remote_update_thread_list (struct target_ops *ops)
running until proven otherwise with a stop reply. In
all-stop, we can only get here if all threads are
stopped. */
- int running = non_stop ? 1 : 0;
+ int running = target_is_non_stop_p () ? 1 : 0;
remote_notice_new_inferior (item->ptid, running);
@@ -3680,15 +3680,44 @@ add_current_inferior_and_thread (char *wait_status)
add_thread_silent (inferior_ptid);
}
+/* Print info about a thread that was found already stopped on
+ connection. */
+
+static void
+print_one_stopped_thread (struct thread_info *thread)
+{
+ struct target_waitstatus *ws = &thread->suspend.waitstatus;
+
+ switch_to_thread (thread->ptid);
+ stop_pc = get_frame_pc (get_current_frame ());
+ set_current_sal_from_frame (get_current_frame ());
+
+ thread->suspend.waitstatus_pending_p = 0;
+
+ if (ws->kind == TARGET_WAITKIND_STOPPED)
+ {
+ enum gdb_signal sig = ws->value.sig;
+
+ if (signal_print_state (sig))
+ observer_notify_signal_received (sig);
+ }
+ observer_notify_normal_stop (NULL, 1);
+}
+
/* Process all initial stop replies the remote side sent in response
to the ? packet. These indicate threads that were already stopped
on initial connection. We mark these threads as stopped and print
their current frame before giving the user the prompt. */
static void
-process_initial_stop_replies (void)
+process_initial_stop_replies (int from_tty)
{
int pending_stop_replies = stop_reply_queue_length ();
+ struct inferior *inf;
+ struct thread_info *thread;
+ struct thread_info *selected = NULL;
+ struct thread_info *lowest = NULL;
+ struct thread_info *first = NULL;
/* Consume the initial pending events. */
while (pending_stop_replies-- > 0)
@@ -3697,6 +3726,7 @@ process_initial_stop_replies (void)
ptid_t event_ptid;
struct target_waitstatus ws;
int ignore_event = 0;
+ struct thread_info *thread;
memset (&ws, 0, sizeof (ws));
event_ptid = target_wait (waiton_ptid, &ws, TARGET_WNOHANG);
@@ -3725,12 +3755,7 @@ process_initial_stop_replies (void)
if (ignore_event)
continue;
- switch_to_thread (event_ptid);
- set_executing (event_ptid, 0);
- set_running (event_ptid, 0);
-
- stop_pc = get_frame_pc (get_current_frame ());
- set_current_sal_from_frame (get_current_frame ());
+ thread = find_thread_ptid (event_ptid);
if (ws.kind == TARGET_WAITKIND_STOPPED)
{
@@ -3740,15 +3765,106 @@ process_initial_stop_replies (void)
instead of signal 0. Suppress it. */
if (sig == GDB_SIGNAL_TRAP)
sig = GDB_SIGNAL_0;
- inferior_thread ()->suspend.stop_signal = sig;
+ thread->suspend.stop_signal = sig;
+ ws.value.sig = sig;
+ }
- if (signal_print_state (sig))
- observer_notify_signal_received (sig);
- }
+ thread->suspend.waitstatus = ws;
+
+ if (ws.kind != TARGET_WAITKIND_STOPPED
+ || ws.value.sig != GDB_SIGNAL_0)
+ thread->suspend.waitstatus_pending_p = 1;
+
+ set_executing (event_ptid, 0);
+ set_running (event_ptid, 0);
+ }
+
+ /* "Notice" the new inferiors before anything related to
+ registers/memory. */
+ ALL_INFERIORS (inf)
+ {
+ if (inf->pid == 0)
+ continue;
+
+ inf->needs_setup = 1;
+
+ if (non_stop)
+ {
+ thread = any_live_thread_of_process (inf->pid);
+ notice_new_inferior (thread->ptid,
+ thread->state == THREAD_RUNNING,
+ from_tty);
+ }
+ }
+
+ /* If all-stop on top of non-stop, pause all threads. Note this
+ records the threads' stop pc, so must be done after "noticing"
+ the inferiors. */
+ if (!non_stop)
+ {
+ stop_all_threads ();
+
+ /* If all threads of an inferior were already stopped, we
+ haven't setup the inferior yet. */
+ ALL_INFERIORS (inf)
+ {
+ if (inf->pid == 0)
+ continue;
- print_stack_frame (get_selected_frame (NULL), 1, SRC_AND_LOC, 1);
- observer_notify_normal_stop (NULL, 1);
+ if (inf->needs_setup)
+ {
+ thread = any_live_thread_of_process (inf->pid);
+ switch_to_thread_no_regs (thread);
+ setup_inferior (0);
+ }
+ }
}
+
+ /* Now go over all threads that are stopped, and print their current
+ frame. If all-stop, then if there's a signalled thread, pick
+ that as current. */
+ ALL_NON_EXITED_THREADS (thread)
+ {
+ struct target_waitstatus *ws;
+
+ if (first == NULL)
+ first = thread;
+
+ if (!non_stop)
+ set_running (thread->ptid, 0);
+ else if (thread->state != THREAD_STOPPED)
+ continue;
+
+ ws = &thread->suspend.waitstatus;
+
+ if (selected == NULL
+ && thread->suspend.waitstatus_pending_p)
+ selected = thread;
+
+ if (lowest == NULL || thread->num < lowest->num)
+ lowest = thread;
+
+ if (non_stop)
+ print_one_stopped_thread (thread);
+ }
+
+ /* In all-stop, we only print the status of one thread, and leave
+ others with their status pending. */
+ if (!non_stop)
+ {
+ thread = selected;
+ if (thread == NULL)
+ thread = lowest;
+ if (thread == NULL)
+ thread = first;
+
+ print_one_stopped_thread (thread);
+ }
+
+ /* For "info program". */
+ thread = inferior_thread ();
+ if (thread->state == THREAD_STOPPED)
+ set_last_target_status (inferior_ptid, thread->suspend.waitstatus);
}
static void
@@ -3826,7 +3942,7 @@ remote_start_remote (int from_tty, struct target_ops *target, int extended_p)
if (gdbarch_has_global_solist (target_gdbarch ()))
solib_add (NULL, from_tty, target, auto_solib_add);
- if (non_stop)
+ if (target_is_non_stop_p ())
{
if (packet_support (PACKET_QNonStop) != PACKET_ENABLE)
error (_("Non-stop mode requested, but remote "
@@ -3870,7 +3986,7 @@ remote_start_remote (int from_tty, struct target_ops *target, int extended_p)
putpkt ("?");
getpkt (&rs->buf, &rs->buf_size, 0);
- if (!non_stop)
+ if (!target_is_non_stop_p ())
{
ptid_t ptid;
int fake_pid_p = 0;
@@ -3963,8 +4079,6 @@ remote_start_remote (int from_tty, struct target_ops *target, int extended_p)
}
else
{
- ptid_t current_ptid;
-
/* Clear WFI global state. Do this before finding about new
threads and inferiors, and setting the current inferior.
Otherwise we would clear the proceed status of the current
@@ -3999,19 +4113,6 @@ remote_start_remote (int from_tty, struct target_ops *target, int extended_p)
return;
}
- /* Let the stub know that we want it to return the thread. */
-
- /* Force the stub to choose a thread. */
- set_general_thread (null_ptid);
-
- /* Query it. */
- current_ptid = remote_current_thread (minus_one_ptid);
- if (ptid_equal (inferior_ptid, minus_one_ptid))
- error (_("remote didn't report the current thread in non-stop mode"));
-
- inferior_ptid = current_ptid;
- get_offsets (); /* Get text, data & bss offsets. */
-
/* In non-stop mode, any cached wait status will be stored in
the stop reply queue. */
gdb_assert (wait_status == NULL);
@@ -4021,9 +4122,7 @@ remote_start_remote (int from_tty, struct target_ops *target, int extended_p)
/* If there are already stopped threads, mark them stopped and
report their stops before giving the prompt to the user. */
- process_initial_stop_replies ();
-
- switch_to_thread (current_ptid);
+ process_initial_stop_replies (from_tty);
if (target_can_async_p ())
target_async (1);
@@ -4973,7 +5072,7 @@ extended_remote_attach (struct target_ops *target, const char *args,
&remote_protocol_packets[PACKET_vAttach]))
{
case PACKET_OK:
- if (!non_stop)
+ if (!target_is_non_stop_p ())
{
/* Save the reply for later. */
wait_status = (char *) alloca (strlen (rs->buf) + 1);
@@ -4995,7 +5094,7 @@ extended_remote_attach (struct target_ops *target, const char *args,
inferior_ptid = pid_to_ptid (pid);
- if (non_stop)
+ if (target_is_non_stop_p ())
{
struct thread_info *thread;
@@ -5024,7 +5123,7 @@ extended_remote_attach (struct target_ops *target, const char *args,
this before anything involving memory or registers. */
target_find_description ();
- if (!non_stop)
+ if (!target_is_non_stop_p ())
{
/* Use the previously fetched status. */
gdb_assert (wait_status != NULL);
@@ -5054,6 +5153,9 @@ extended_remote_attach (struct target_ops *target, const char *args,
static void
extended_remote_post_attach (struct target_ops *ops, int pid)
{
+ /* Get text, data & bss offsets. */
+ get_offsets ();
+
/* In certain cases GDB might not have had the chance to start
symbol lookup up until now. This could happen if the debugged
binary is not using shared libraries, the vsyscall page is not
@@ -5286,7 +5388,7 @@ remote_vcont_resume (ptid_t ptid, int step, enum gdb_signal siggnal)
gdb_assert (strlen (rs->buf) < get_remote_packet_size ());
putpkt (rs->buf);
- if (non_stop)
+ if (target_is_non_stop_p ())
{
/* In non-stop, the stub replies to vCont with "OK". The stop
reply will be reported asynchronously by means of a `%Stop'
@@ -5314,7 +5416,7 @@ remote_resume (struct target_ops *ops,
it is safe to start a 'vNotif' sequence. It is good to do it
before resuming inferior, because inferior was stopped and no RSP
traffic at that moment. */
- if (!non_stop)
+ if (!target_is_non_stop_p ())
remote_notif_process (rs->notif_state, ¬if_client_stop);
rs->last_sent_signal = siggnal;
@@ -5378,7 +5480,7 @@ remote_resume (struct target_ops *ops,
only to the base all-stop protocol, however. In non-stop (which
only supports vCont), the stub replies with an "OK", and is
immediate able to process further serial input. */
- if (!non_stop)
+ if (!target_is_non_stop_p ())
rs->waiting_for_stop_reply = 1;
}
\f
@@ -5565,7 +5667,7 @@ remote_stop (struct target_ops *self, ptid_t ptid)
if (remote_debug)
fprintf_unfiltered (gdb_stdlog, "remote_stop called\n");
- if (non_stop)
+ if (target_is_non_stop_p ())
remote_stop_ns (ptid);
else
{
@@ -5583,7 +5685,7 @@ remote_interrupt (struct target_ops *self, ptid_t ptid)
if (remote_debug)
fprintf_unfiltered (gdb_stdlog, "remote_interrupt called\n");
- if (non_stop)
+ if (target_is_non_stop_p ())
{
/* We don't currently have a way to ^C the remote target in
non-stop mode. Stop it (with no signal) instead. */
@@ -6377,7 +6479,7 @@ Packet: '%s'\n"),
break;
}
- if (non_stop && ptid_equal (event->ptid, null_ptid))
+ if (target_is_non_stop_p () && ptid_equal (event->ptid, null_ptid))
error (_("No process or thread specified in stop reply: %s"), buf);
}
@@ -6733,7 +6835,7 @@ remote_wait (struct target_ops *ops,
{
ptid_t event_ptid;
- if (non_stop)
+ if (target_is_non_stop_p ())
event_ptid = remote_wait_ns (ptid, status, options);
else
event_ptid = remote_wait_as (ptid, status, options);
@@ -7962,7 +8064,9 @@ putpkt_binary (const char *buf, int cnt)
case it's not possible to issue a command while the target is
running. This is not a problem in non-stop mode, because in that
case, the stub is always ready to process serial input. */
- if (!non_stop && target_is_async_p () && rs->waiting_for_stop_reply)
+ if (!target_is_non_stop_p ()
+ && target_is_async_p ()
+ && rs->waiting_for_stop_reply)
{
error (_("Cannot execute this command while the target is running.\n"
"Use the \"interrupt\" command to stop the target\n"
@@ -12936,11 +13040,15 @@ remote_async (struct target_ops *ops, int enable)
event loop to process them. */
if (!QUEUE_is_empty (stop_reply_p, stop_reply_queue))
mark_async_event_handler (remote_async_inferior_event_token);
+ if (target_is_non_stop_p ())
+ mark_async_event_handler (rs->notif_state->get_pending_events_token);
}
else
{
serial_async (rs->remote_desc, NULL, NULL);
clear_async_event_handler (remote_async_inferior_event_token);
+ if (target_is_non_stop_p ())
+ clear_async_event_handler (rs->notif_state->get_pending_events_token);
}
}
diff --git a/gdb/thread.c b/gdb/thread.c
index 6d410fb..522d044 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -1312,6 +1312,22 @@ info_threads_command (char *arg, int from_tty)
print_thread_info (current_uiout, arg, -1);
}
+/* See gdbthread.h. */
+
+void
+switch_to_thread_no_regs (struct thread_info *thread)
+{
+ struct inferior *inf;
+
+ inf = find_inferior_ptid (thread->ptid);
+ gdb_assert (inf != NULL);
+ set_current_program_space (inf->pspace);
+ set_current_inferior (inf);
+
+ inferior_ptid = thread->ptid;
+ stop_pc = ~(CORE_ADDR) 0;
+}
+
/* Switch from one thread to another. */
void
--
1.9.3
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH 02/18] Remote all-stop-on-top-of-non-stop
2015-10-14 15:28 ` [PATCH 02/18] Remote all-stop-on-top-of-non-stop Pedro Alves
@ 2015-10-24 22:39 ` Yao Qi
2015-11-23 15:40 ` Pedro Alves
0 siblings, 1 reply; 60+ messages in thread
From: Yao Qi @ 2015-10-24 22:39 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
Pedro Alves <palves@redhat.com> writes:
> @@ -2636,8 +2633,37 @@ attach_command_post_wait (char *args, int from_tty, int async_exec)
> target_post_attach (ptid_get_pid (inferior_ptid));
>
> post_create_inferior (¤t_target, from_tty);
> +}
> +
> +/* What to do after the first program stops after attaching. */
> +enum attach_post_wait_mode
> +{
> + /* Do nothing. Leaves threads as they are. */
> + ATTACH_POST_WAIT_NOTHING,
> +
> + /* Re-resume threads that are marked running. */
> + ATTACH_POST_WAIT_RESUME,
> +
> + /* Stop all threads. */
> + ATTACH_POST_WAIT_STOP,
> +};
> +
> +/* Called after we've attached to a process and we've seen it stop for
> + the first time. If ASYNC_EXEC is true, re-resume threads that
> + should be running. Else if ATTACH, */
> +
Comments are not complete.
> +
> + /* Now go over all threads that are stopped, and print their current
> + frame. If all-stop, then if there's a signalled thread, pick
> + that as current. */
Is it the code you described in the last paragraph of the commit log?
> + ALL_NON_EXITED_THREADS (thread)
> + {
> + struct target_waitstatus *ws;
> +
> + if (first == NULL)
> + first = thread;
> +
> + if (!non_stop)
> + set_running (thread->ptid, 0);
> + else if (thread->state != THREAD_STOPPED)
> + continue;
> +
> + ws = &thread->suspend.waitstatus;
> +
> + if (selected == NULL
> + && thread->suspend.waitstatus_pending_p)
> + selected = thread;
> +
> + if (lowest == NULL || thread->num < lowest->num)
> + lowest = thread;
> +
> + if (non_stop)
> + print_one_stopped_thread (thread);
> + }
> +
> + /* In all-stop, we only print the status of one thread, and leave
> + others with their status pending. */
> + if (!non_stop)
> + {
> + thread = selected;
> + if (thread == NULL)
> + thread = lowest;
> + if (thread == NULL)
> + thread = first;
Looks lowest can't be NULL, so first isn't used.
> +
> + print_one_stopped_thread (thread);
> + }
> +
> + /* For "info program". */
> + thread = inferior_thread ();
> + if (thread->state == THREAD_STOPPED)
> + set_last_target_status (inferior_ptid, thread->suspend.waitstatus);
> }
>
> static void
> @@ -3826,7 +3942,7 @@ remote_start_remote (int from_tty, struct target_ops *target, int extended_p)
> @@ -12936,11 +13040,15 @@ remote_async (struct target_ops *ops, int enable)
> event loop to process them. */
> if (!QUEUE_is_empty (stop_reply_p, stop_reply_queue))
> mark_async_event_handler (remote_async_inferior_event_token);
> + if (target_is_non_stop_p ())
> + mark_async_event_handler (rs->notif_state->get_pending_events_token);
I don't understand why do we need to mark the pending event token.
Previously, we only need to do so when GDB sees a notification.
> }
> else
> {
> serial_async (rs->remote_desc, NULL, NULL);
> clear_async_event_handler (remote_async_inferior_event_token);
> + if (target_is_non_stop_p ())
> + clear_async_event_handler (rs->notif_state->get_pending_events_token);
> }
> }
>
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH 02/18] Remote all-stop-on-top-of-non-stop
2015-10-24 22:39 ` Yao Qi
@ 2015-11-23 15:40 ` Pedro Alves
2015-11-23 18:39 ` Pedro Alves
0 siblings, 1 reply; 60+ messages in thread
From: Pedro Alves @ 2015-11-23 15:40 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
Hi Yao,
Thanks for the review, and sorry for the delay in getting
back to this...
On 10/23/2015 04:30 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
>
>> @@ -2636,8 +2633,37 @@ attach_command_post_wait (char *args, int from_tty, int async_exec)
>> target_post_attach (ptid_get_pid (inferior_ptid));
>>
>> post_create_inferior (¤t_target, from_tty);
>> +}
>> +
>> +/* What to do after the first program stops after attaching. */
>> +enum attach_post_wait_mode
>> +{
>> + /* Do nothing. Leaves threads as they are. */
>> + ATTACH_POST_WAIT_NOTHING,
>> +
>> + /* Re-resume threads that are marked running. */
>> + ATTACH_POST_WAIT_RESUME,
>> +
>> + /* Stop all threads. */
>> + ATTACH_POST_WAIT_STOP,
>> +};
>> +
>> +/* Called after we've attached to a process and we've seen it stop for
>> + the first time. If ASYNC_EXEC is true, re-resume threads that
>> + should be running. Else if ATTACH, */
>> +
>
> Comments are not complete.
Whoops, at some point I was going to add an 'attach' parameter, then I
converted it to the new enum. Fixed.
>
>> +
>> + /* Now go over all threads that are stopped, and print their current
>> + frame. If all-stop, then if there's a signalled thread, pick
>> + that as current. */
>
> Is it the code you described in the last paragraph of the commit log?
Yes.
>
>> + ALL_NON_EXITED_THREADS (thread)
>> + {
>> + struct target_waitstatus *ws;
>> +
>> + if (first == NULL)
>> + first = thread;
>> +
>> + if (!non_stop)
>> + set_running (thread->ptid, 0);
>> + else if (thread->state != THREAD_STOPPED)
>> + continue;
>> +
>> + ws = &thread->suspend.waitstatus;
>> +
>> + if (selected == NULL
>> + && thread->suspend.waitstatus_pending_p)
>> + selected = thread;
>> +
>> + if (lowest == NULL || thread->num < lowest->num)
>> + lowest = thread;
>> +
>> + if (non_stop)
>> + print_one_stopped_thread (thread);
>> + }
>> +
>> + /* In all-stop, we only print the status of one thread, and leave
>> + others with their status pending. */
>> + if (!non_stop)
>> + {
>> + thread = selected;
>> + if (thread == NULL)
>> + thread = lowest;
>> + if (thread == NULL)
>> + thread = first;
>
> Looks lowest can't be NULL, so first isn't used.
"lowest" here actually means "lowest stopped". It can thus be NULL if
all threads are running. I've renamed it to 'lowest_stopped' now
to make that clearer.
>
>> +
>> + print_one_stopped_thread (thread);
>> + }
>> +
>> + /* For "info program". */
>> + thread = inferior_thread ();
>> + if (thread->state == THREAD_STOPPED)
>> + set_last_target_status (inferior_ptid, thread->suspend.waitstatus);
>> }
>>
>> static void
>> @@ -3826,7 +3942,7 @@ remote_start_remote (int from_tty, struct target_ops *target, int extended_p)
>
>> @@ -12936,11 +13040,15 @@ remote_async (struct target_ops *ops, int enable)
>> event loop to process them. */
>> if (!QUEUE_is_empty (stop_reply_p, stop_reply_queue))
>> mark_async_event_handler (remote_async_inferior_event_token);
>> + if (target_is_non_stop_p ())
>> + mark_async_event_handler (rs->notif_state->get_pending_events_token);
>
> I don't understand why do we need to mark the pending event token.
> Previously, we only need to do so when GDB sees a notification.
Thanks for pointing this out. I should have added some comments here,
as it took me a while to re-understand it. :-)
To explain why I added this, I need to start with the "else" branch ...
>
>> }
>> else
>> {
>> serial_async (rs->remote_desc, NULL, NULL);
>> clear_async_event_handler (remote_async_inferior_event_token);
>> + if (target_is_non_stop_p ())
>> + clear_async_event_handler (rs->notif_state->get_pending_events_token);
... here.
Currently, it's possible that target_async(0) is called while the
pending event token is marked. If we clear the pending event token when
disabling async, we need to mark it again when re-enabling async. To simplify
(and avoid keeping track of whether the token was marked), I just always
marked the token when enabling async. It just results in at most one
harmless spurious event-loop wake up.
That explains why we now need to mark the pending event token
while previously we didn't, but not why do we need to _clear_ the
pending event token while previously we didn't. To understand
that, it helps to identify the target_async(0) call in question. That's
called after hitting a breakpoint and reporting an event to the
user (in inf-loop.c).
case INF_EXEC_COMPLETE:
if (!non_stop)
{
/* Unregister the inferior from the event loop. This is done
so that when the inferior is not running we don't get
distracted by spurious inferior output. */
if (target_has_execution && target_can_async_p ())
target_async (0);
}
Thus if we don't clear the get_pending_events_token when disabling async,
target_wait ends up called even after target_async is disabled. That
would normally be harmless, but annoying as it breaks testing
with "set debug infrun 1", because you end up with a spurious
TARGET_WAITKIND_IGNORE event after gdb has already printed the prompt,
which breaks the testsuite.
But in this case, it's actually worse. It results in the event loop being
_continuously_ woken (#4 - #8):
#1 - the event loop wakes up for get_pending_events_token, which results in
#2 - remote_notif_stop_can_get_pending_events -> mark remote_async_inferior_event_token.
#3 - event loop wakes up for remote_async_inferior_event_token
#4 - fetch_inferior_event -> target_wait -> remote_wait -> TARGET_WAITKIND_IGNORE
#5 - handle_inferior_event -> prepare_to_wait
#6 - because async is off, prepare_to_wait calls mark_infrun_async_event_handler.
#7 - event loop wakes up for infrun_async_inferior_event_token -> fetch_inferior_event.
#8 - goto #4.
E.g., just running to main with logging enabled shows:
...
notif: discard queued event: 'Stop' in Thread 0
infrun: target_wait (-1.0.0, status) =
infrun: -1.0.0 [Thread 0],
infrun: status->kind = ignore
infrun: TARGET_WAITKIND_IGNORE
infrun: prepare_to_wait
notif: discard queued event: 'Stop' in Thread 0
infrun: target_wait (-1.0.0, status) =
infrun: -1.0.0 [Thread 0],
infrun: status->kind = ignore
infrun: TARGET_WAITKIND_IGNORE
infrun: prepare_to_wait
notif: discard queued event: 'Stop' in Thread 0
infrun: target_wait (-1.0.0, status) =
infrun: -1.0.0 [Thread 0],
infrun: status->kind = ignore
infrun: TARGET_WAITKIND_IGNORE
infrun: prepare_to_wait
notif: discard queued event: 'Stop' in Thread 0
infrun: target_wait (-1.0.0, status) =
infrun: -1.0.0 [Thread 0],
infrun: status->kind = ignore
infrun: TARGET_WAITKIND_IGNORE
infrun: prepare_to_wait
notif: discard queued event: 'Stop' in Thread 0
infrun: target_wait (-1.0.0, status) =
infrun: -1.0.0 [Thread 0],
infrun: status->kind = ignore
infrun: TARGET_WAITKIND_IGNORE
infrun: prepare_to_wait
notif: discard queued event: 'Stop' in Thread 0
infrun: target_wait (-1.0.0, status) =
infrun: -1.0.0 [Thread 0],
infrun: status->kind = ignore
infrun: TARGET_WAITKIND_IGNORE
infrun: prepare_to_wait
notif: discard queued event: 'Stop' in Thread 0
...
Forever and ever. The testsuite actually regresses due to this, as some
tests enabling infrun logging (e.g., breakpoint-in-ro-region.exp).
The patch below (applies on top of the series) fixes this by making it so
that the pending event token is already clear when we get to the
target_async(0) call in question. The testsuite does pass cleanly with
this patch and without the patch, but regresses if I apply only the
remote_async hunk (that is, if I revert the changes in the patch #2
under discussion).
The remote_notif_get_pending_events hunk below feels like a good
improvement to me, as it gets rid of unnecessary event loop wake ups.
However, I still think that we need the
clear_async_event_handler / mark_async_event_handler calls in
remote_async like I had them before.
That's because we have other target_async(0) calls that unlike the
inf-loop.c call happen while the target _is_ still running. In particular,
I'm thinking of the one in top.c:gdb_readline_wrapper, when we're about to
show a secondary prompt and nest a secondary event loop that should not
get woken by target events.
So I'm thinking that I should leave the original patch as is, and
add some extra comments to remote_async. WDYT?
From 2a73e1b66417edce50d97e566865f2273a988482 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Thu, 19 Nov 2015 18:40:13 +0000
Subject: [PATCH] clear pending events token before acking
---
gdb/remote.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/gdb/remote.c b/gdb/remote.c
index 9e69bfb..04057cc 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -6640,6 +6640,10 @@ remote_notif_get_pending_events (struct notif_client *nc)
"notif: process: '%s' ack pending event\n",
nc->name);
+ /* As we're acking the pending notification, we no longer need
+ to wake the event loop to do it. */
+ clear_async_event_handler (rs->notif_state->get_pending_events_token);
+
/* acknowledge */
nc->ack (nc, rs->buf, rs->notif_state->pending_event[nc->id]);
rs->notif_state->pending_event[nc->id] = NULL;
@@ -13158,15 +13162,11 @@ remote_async (struct target_ops *ops, int enable)
event loop to process them. */
if (!QUEUE_is_empty (stop_reply_p, stop_reply_queue))
mark_async_event_handler (remote_async_inferior_event_token);
- if (target_is_non_stop_p ())
- mark_async_event_handler (rs->notif_state->get_pending_events_token);
}
else
{
serial_async (rs->remote_desc, NULL, NULL);
clear_async_event_handler (remote_async_inferior_event_token);
- if (target_is_non_stop_p ())
- clear_async_event_handler (rs->notif_state->get_pending_events_token);
}
}
--
1.9.3
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH 02/18] Remote all-stop-on-top-of-non-stop
2015-11-23 15:40 ` Pedro Alves
@ 2015-11-23 18:39 ` Pedro Alves
2015-11-26 15:53 ` Yao Qi
0 siblings, 1 reply; 60+ messages in thread
From: Pedro Alves @ 2015-11-23 18:39 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On 11/23/2015 03:40 PM, Pedro Alves wrote:
> So I'm thinking that I should leave the original patch as is, and
> add some extra comments to remote_async. WDYT?
Like so:
diff --git i/gdb/remote.c w/gdb/remote.c
index 1618ea1..624f60d 100644
--- i/gdb/remote.c
+++ w/gdb/remote.c
@@ -13036,12 +13036,20 @@ remote_async (struct target_ops *ops, int enable)
event loop to process them. */
if (!QUEUE_is_empty (stop_reply_p, stop_reply_queue))
mark_async_event_handler (remote_async_inferior_event_token);
+ /* For simplicity, below we clear the pending events token
+ without remembering whether it is marked, so here we always
+ mark it. If there's actually no pending notification to
+ process, this ends up being a no-op (other than a spurious
+ event-loop wakeup). */
if (target_is_non_stop_p ())
mark_async_event_handler (rs->notif_state->get_pending_events_token);
}
else
{
serial_async (rs->remote_desc, NULL, NULL);
+ /* If the core is disabling async, it doesn't want to be
+ disturbed with target events. Clear all async event sources
+ too. */
clear_async_event_handler (remote_async_inferior_event_token);
if (target_is_non_stop_p ())
clear_async_event_handler (rs->notif_state->get_pending_events_token);
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH 02/18] Remote all-stop-on-top-of-non-stop
2015-11-23 18:39 ` Pedro Alves
@ 2015-11-26 15:53 ` Yao Qi
0 siblings, 0 replies; 60+ messages in thread
From: Yao Qi @ 2015-11-26 15:53 UTC (permalink / raw)
To: Pedro Alves; +Cc: Yao Qi, gdb-patches
Pedro Alves <palves@redhat.com> writes:
> diff --git i/gdb/remote.c w/gdb/remote.c
> index 1618ea1..624f60d 100644
> --- i/gdb/remote.c
> +++ w/gdb/remote.c
> @@ -13036,12 +13036,20 @@ remote_async (struct target_ops *ops, int enable)
> event loop to process them. */
> if (!QUEUE_is_empty (stop_reply_p, stop_reply_queue))
> mark_async_event_handler (remote_async_inferior_event_token);
> + /* For simplicity, below we clear the pending events token
> + without remembering whether it is marked, so here we always
> + mark it. If there's actually no pending notification to
> + process, this ends up being a no-op (other than a spurious
> + event-loop wakeup). */
> if (target_is_non_stop_p ())
> mark_async_event_handler (rs->notif_state->get_pending_events_token);
> }
> else
> {
> serial_async (rs->remote_desc, NULL, NULL);
> + /* If the core is disabling async, it doesn't want to be
> + disturbed with target events. Clear all async event sources
> + too. */
> clear_async_event_handler (remote_async_inferior_event_token);
> if (target_is_non_stop_p ())
> clear_async_event_handler (rs->notif_state->get_pending_events_token);
These comments are good to me.
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH 15/18] gdbserver:prepare_access_memory: pick another thread
2015-10-14 15:28 [PATCH 00/18] Remote all-stop on top of non-stop Pedro Alves
` (3 preceding siblings ...)
2015-10-14 15:28 ` [PATCH 02/18] Remote all-stop-on-top-of-non-stop Pedro Alves
@ 2015-10-14 15:28 ` Pedro Alves
2015-10-14 15:28 ` [PATCH 18/18] remote: enable "maint set target-non-stop" by default Pedro Alves
` (15 subsequent siblings)
20 siblings, 0 replies; 60+ messages in thread
From: Pedro Alves @ 2015-10-14 15:28 UTC (permalink / raw)
To: gdb-patches
Say GDB wants to access the inferior process's memory. The current
remote general thread is 3, but GDB's switched to thread 2. Because
both threads are of the same process, GDB skips making the remote
thread be thread 2 as well (sending an Hg packet) before accessing
memory (remote.c:set_general_process). However, if thread 3 has
exited meanwhile, thread 3 no longer exists on the server and
gdbserver points current_thread to NULL. The result is the memory
access fails, even through the process still exists.
Fix this by making prepare_to_access memory select the thread to
access memory through.
gdb/gdbserver/ChangeLog:
2015-10-14 Pedro Alves <palves@redhat.com>
* mem-break.c (check_gdb_bp_preconditions): Remove current_thread
check.
(set_gdb_breakpoint): If prepare_to_access_memory fails, set *ERR
to -1.
* target.c (struct thread_search): New structure.
(thread_search_callback): New function.
(prev_general_thread): New global.
(prepare_to_access_memory, done_accessing_memory): New functions.
* target.h (prepare_to_access_memory, done_accessing_memory):
Replace macros with function declarations.
---
gdb/gdbserver/mem-break.c | 17 +++-----
gdb/gdbserver/target.c | 107 ++++++++++++++++++++++++++++++++++++++++++++++
gdb/gdbserver/target.h | 14 ++----
3 files changed, 118 insertions(+), 20 deletions(-)
diff --git a/gdb/gdbserver/mem-break.c b/gdb/gdbserver/mem-break.c
index f077497..97d53b5 100644
--- a/gdb/gdbserver/mem-break.c
+++ b/gdb/gdbserver/mem-break.c
@@ -1011,13 +1011,8 @@ check_gdb_bp_preconditions (char z_type, int *err)
*err = 1;
return 0;
}
- else if (current_thread == NULL)
- {
- *err = -1;
- return 0;
- }
- else
- return 1;
+
+ return 1;
}
/* See mem-break.h. This is a wrapper for set_gdb_breakpoint_1 that
@@ -1035,9 +1030,11 @@ set_gdb_breakpoint (char z_type, CORE_ADDR addr, int size, int *err)
access memory. */
if (z_type == Z_PACKET_SW_BP)
{
- *err = prepare_to_access_memory ();
- if (*err != 0)
- return NULL;
+ if (prepare_to_access_memory () != 0)
+ {
+ *err = -1;
+ return NULL;
+ }
}
bp = set_gdb_breakpoint_1 (z_type, addr, size, err);
diff --git a/gdb/gdbserver/target.c b/gdb/gdbserver/target.c
index 301f90e..a59bcb0 100644
--- a/gdb/gdbserver/target.c
+++ b/gdb/gdbserver/target.c
@@ -37,6 +37,113 @@ set_desired_thread (int use_general)
return (current_thread != NULL);
}
+/* Structure used to look up a thread to use as current when accessing
+ memory. */
+
+struct thread_search
+{
+ /* The PTID of the current general thread. This is an input
+ parameter. */
+ ptid_t current_gen_ptid;
+
+ /* The first thread found. */
+ struct thread_info *first;
+
+ /* The first stopped thread found. */
+ struct thread_info *stopped;
+
+ /* The current general thread, if found. */
+ struct thread_info *current;
+};
+
+/* Callback for find_inferior. Search for a thread to use as current
+ when accessing memory. */
+
+static int
+thread_search_callback (struct inferior_list_entry *entry, void *args)
+{
+ struct thread_info *thread = (struct thread_info *) entry;
+ struct thread_search *s = (struct thread_search *) args;
+
+ if (ptid_get_pid (entry->id) == ptid_get_pid (s->current_gen_ptid)
+ && mythread_alive (ptid_of (thread)))
+ {
+ if (s->stopped == NULL && thread_stopped (thread))
+ s->stopped = thread;
+
+ if (s->first == NULL)
+ s->first = thread;
+
+ if (s->current == NULL && ptid_equal (s->current_gen_ptid, entry->id))
+ s->current = thread;
+ }
+
+ return 0;
+}
+
+/* The thread that was current before prepare_to_access_memory was
+ called. done_accessing_memory uses this to restore the previous
+ selected thread. */
+static ptid_t prev_general_thread;
+
+/* See target.h. */
+
+int
+prepare_to_access_memory (void)
+{
+ struct thread_search search;
+ struct thread_info *thread;
+
+ memset (&search, 0, sizeof (search));
+ search.current_gen_ptid = general_thread;
+ prev_general_thread = general_thread;
+
+ if (the_target->prepare_to_access_memory != NULL)
+ {
+ int res;
+
+ res = the_target->prepare_to_access_memory ();
+ if (res != 0)
+ return res;
+ }
+
+ find_inferior (&all_threads, thread_search_callback, &search);
+
+ /* Prefer a stopped thread. If none is found, try the current
+ thread. Otherwise, take the first thread in the process. If
+ none is found, undo the effects of
+ target->prepare_to_access_memory() and return error. */
+ if (search.stopped != NULL)
+ thread = search.stopped;
+ else if (search.current != NULL)
+ thread = search.current;
+ else if (search.first != NULL)
+ thread = search.first;
+ else
+ {
+ done_accessing_memory ();
+ return 1;
+ }
+
+ current_thread = thread;
+ general_thread = ptid_of (thread);
+
+ return 0;
+}
+
+/* See target.h. */
+
+void
+done_accessing_memory (void)
+{
+ if (the_target->done_accessing_memory != NULL)
+ the_target->done_accessing_memory ();
+
+ /* Restore the previous selected thread. */
+ general_thread = prev_general_thread;
+ current_thread = find_thread_ptid (general_thread);
+}
+
int
read_inferior_memory (CORE_ADDR memaddr, unsigned char *myaddr, int len)
{
diff --git a/gdb/gdbserver/target.h b/gdb/gdbserver/target.h
index a2842b4..aad8903 100644
--- a/gdb/gdbserver/target.h
+++ b/gdb/gdbserver/target.h
@@ -630,17 +630,11 @@ int start_non_stop (int nonstop);
ptid_t mywait (ptid_t ptid, struct target_waitstatus *ourstatus, int options,
int connected_wait);
-#define prepare_to_access_memory() \
- (the_target->prepare_to_access_memory \
- ? (*the_target->prepare_to_access_memory) () \
- : 0)
+/* Prepare to read or write memory from the inferior process. See the
+ corresponding target_ops methods for more details. */
-#define done_accessing_memory() \
- do \
- { \
- if (the_target->done_accessing_memory) \
- (*the_target->done_accessing_memory) (); \
- } while (0)
+int prepare_to_access_memory (void);
+void done_accessing_memory (void);
#define target_core_of_thread(ptid) \
(the_target->core_of_thread ? (*the_target->core_of_thread) (ptid) \
--
1.9.3
^ permalink raw reply [flat|nested] 60+ messages in thread* [PATCH 18/18] remote: enable "maint set target-non-stop" by default
2015-10-14 15:28 [PATCH 00/18] Remote all-stop on top of non-stop Pedro Alves
` (4 preceding siblings ...)
2015-10-14 15:28 ` [PATCH 15/18] gdbserver:prepare_access_memory: pick another thread Pedro Alves
@ 2015-10-14 15:28 ` Pedro Alves
2015-10-14 15:33 ` [PATCH 05/18] remote: stop reason and watchpoint data address per thread Pedro Alves
` (14 subsequent siblings)
20 siblings, 0 replies; 60+ messages in thread
From: Pedro Alves @ 2015-10-14 15:28 UTC (permalink / raw)
To: gdb-patches
... on remote targets that support it.
gdb/ChangeLog:
2015-10-14 Pedro Alves <palves@redhat.com>
* remote.c (remote_always_non_stop_p): New function.
(init_remote_ops): Install it as to_always_non_stop_p method.
---
gdb/remote.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/gdb/remote.c b/gdb/remote.c
index c7a01d2..5e88221 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -11598,6 +11598,20 @@ remote_supports_non_stop (struct target_ops *self)
return 1;
}
+/* to_always_non_stop_p implementation. */
+
+static int
+remote_always_non_stop_p (struct target_ops *self)
+{
+ struct remote_state *rs = get_remote_state ();
+
+ /* Only enable if the stub supports the minimum set of optional
+ packets that make AS/NS possible. */
+ return (packet_support (PACKET_QNonStop) == PACKET_ENABLE
+ && packet_support (PACKET_no_resumed) == PACKET_ENABLE
+ && packet_support (PACKET_QThreadEvents) == PACKET_ENABLE);
+}
+
static int
remote_supports_disable_randomization (struct target_ops *self)
{
@@ -12986,6 +13000,7 @@ Specify the serial device it is connected to\n\
remote_ops.to_terminal_inferior = remote_terminal_inferior;
remote_ops.to_terminal_ours = remote_terminal_ours;
remote_ops.to_supports_non_stop = remote_supports_non_stop;
+ remote_ops.to_always_non_stop_p = remote_always_non_stop_p;
remote_ops.to_supports_multi_process = remote_supports_multi_process;
remote_ops.to_supports_disable_randomization
= remote_supports_disable_randomization;
--
1.9.3
^ permalink raw reply [flat|nested] 60+ messages in thread* [PATCH 05/18] remote: stop reason and watchpoint data address per thread
2015-10-14 15:28 [PATCH 00/18] Remote all-stop on top of non-stop Pedro Alves
` (5 preceding siblings ...)
2015-10-14 15:28 ` [PATCH 18/18] remote: enable "maint set target-non-stop" by default Pedro Alves
@ 2015-10-14 15:33 ` Pedro Alves
2015-10-14 15:33 ` [PATCH 10/18] Remote thread create/exit events Pedro Alves
` (13 subsequent siblings)
20 siblings, 0 replies; 60+ messages in thread
From: Pedro Alves @ 2015-10-14 15:33 UTC (permalink / raw)
To: gdb-patches
Running local-watch-wrong-thread.exp with "maint set target-non-stop
on" exposes that gdb/remote.c only records whether the target stopped
for a breakpoint/watchpoint plus the watchpoint data address *for the
last reported remote event*. But in non-stop mode, we need to keep
that info per-thread, as each thread can end up with its own
last-status pending.
gdb/ChangeLog:
2015-10-14 Pedro Alves <palves@redhat.com>
* remote.c (struct remote_state) <remote_watch_data_address,
stop_reason>: Delete fields.
(struct private_thread_info) <stop_reason, watch_data_address>:
New fields.
(resume_clear_thread_private_info): New function.
(append_pending_thread_resumptions): Call it.
(remote_resume): Clear all threads' private info.
(process_stop_reply): Adjust.
(remote_wait_as): Don't reference remote_state's stop_reason
field.
(remote_stopped_by_sw_breakpoint)
(remote_stopped_by_hw_breakpoint, remote_stopped_by_watchpoint)
(remote_stopped_data_address): Adjust to refer get data from the
current thread.
---
gdb/remote.c | 69 ++++++++++++++++++++++++++++++++++++++----------------------
1 file changed, 44 insertions(+), 25 deletions(-)
diff --git a/gdb/remote.c b/gdb/remote.c
index ff25a76..10c65e2 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -401,13 +401,6 @@ struct remote_state
int use_threadinfo_query;
int use_threadextra_query;
- /* This is set to the data address of the access causing the target
- to stop for a watchpoint. */
- CORE_ADDR remote_watch_data_address;
-
- /* Whether the target stopped for a breakpoint/watchpoint. */
- enum target_stop_reason stop_reason;
-
threadref echo_nextthread;
threadref nextthread;
threadref resultthreadlist[MAXTHREADLISTRESULTS];
@@ -438,6 +431,13 @@ struct private_thread_info
{
char *extra;
int core;
+
+ /* Whether the target stopped for a breakpoint/watchpoint. */
+ enum target_stop_reason stop_reason;
+
+ /* This is set to the data address of the access causing the target
+ to stop for a watchpoint. */
+ CORE_ADDR watch_data_address;
};
static void
@@ -5299,6 +5299,18 @@ append_resumption (char *p, char *endp,
return p;
}
+/* Clear the thread's private info on resume. */
+
+static void
+resume_clear_thread_private_info (struct thread_info *thread)
+{
+ if (thread->priv != NULL)
+ {
+ thread->priv->stop_reason = TARGET_STOPPED_BY_NO_REASON;
+ thread->priv->watch_data_address = 0;
+ }
+}
+
/* Append a vCont continue-with-signal action for threads that have a
non-zero stop signal. */
@@ -5315,6 +5327,7 @@ append_pending_thread_resumptions (char *p, char *endp, ptid_t ptid)
p = append_resumption (p, endp, thread->ptid,
0, thread->suspend.stop_signal);
thread->suspend.stop_signal = GDB_SIGNAL_0;
+ resume_clear_thread_private_info (thread);
}
return p;
@@ -5409,6 +5422,7 @@ remote_resume (struct target_ops *ops,
{
struct remote_state *rs = get_remote_state ();
char *buf;
+ struct thread_info *thread;
/* In all-stop, we can't mark REMOTE_ASYNC_GET_PENDING_EVENTS_TOKEN
(explained in remote-notif.c:handle_notification) so
@@ -5435,6 +5449,9 @@ remote_resume (struct target_ops *ops,
else
set_continue_thread (ptid);
+ ALL_NON_EXITED_THREADS (thread)
+ resume_clear_thread_private_info (thread);
+
buf = rs->buf;
if (execution_direction == EXEC_REVERSE)
{
@@ -6581,6 +6598,7 @@ process_stop_reply (struct stop_reply *stop_reply,
&& status->kind != TARGET_WAITKIND_SIGNALLED)
{
struct remote_state *rs = get_remote_state ();
+ struct private_thread_info *remote_thr;
/* Expedited registers. */
if (stop_reply->regcache)
@@ -6597,11 +6615,11 @@ process_stop_reply (struct stop_reply *stop_reply,
VEC_free (cached_reg_t, stop_reply->regcache);
}
- rs->stop_reason = stop_reply->stop_reason;
- rs->remote_watch_data_address = stop_reply->watch_data_address;
-
remote_notice_new_inferior (ptid, 0);
- demand_private_info (ptid)->core = stop_reply->core;
+ remote_thr = demand_private_info (ptid);
+ remote_thr->core = stop_reply->core;
+ remote_thr->stop_reason = stop_reply->stop_reason;
+ remote_thr->watch_data_address = stop_reply->watch_data_address;
}
stop_reply_xfree (stop_reply);
@@ -6735,8 +6753,6 @@ remote_wait_as (ptid_t ptid, struct target_waitstatus *status, int options)
buf = rs->buf;
- rs->stop_reason = TARGET_STOPPED_BY_NO_REASON;
-
/* We got something. */
rs->waiting_for_stop_reply = 0;
@@ -9311,9 +9327,10 @@ remote_check_watch_resources (struct target_ops *self,
static int
remote_stopped_by_sw_breakpoint (struct target_ops *ops)
{
- struct remote_state *rs = get_remote_state ();
+ struct thread_info *thread = inferior_thread ();
- return rs->stop_reason == TARGET_STOPPED_BY_SW_BREAKPOINT;
+ return (thread->priv != NULL
+ && thread->priv->stop_reason == TARGET_STOPPED_BY_SW_BREAKPOINT);
}
/* The to_supports_stopped_by_sw_breakpoint method of target
@@ -9332,9 +9349,10 @@ remote_supports_stopped_by_sw_breakpoint (struct target_ops *ops)
static int
remote_stopped_by_hw_breakpoint (struct target_ops *ops)
{
- struct remote_state *rs = get_remote_state ();
+ struct thread_info *thread = inferior_thread ();
- return rs->stop_reason == TARGET_STOPPED_BY_HW_BREAKPOINT;
+ return (thread->priv != NULL
+ && thread->priv->stop_reason == TARGET_STOPPED_BY_HW_BREAKPOINT);
}
/* The to_supports_stopped_by_hw_breakpoint method of target
@@ -9351,24 +9369,25 @@ remote_supports_stopped_by_hw_breakpoint (struct target_ops *ops)
static int
remote_stopped_by_watchpoint (struct target_ops *ops)
{
- struct remote_state *rs = get_remote_state ();
+ struct thread_info *thread = inferior_thread ();
- return rs->stop_reason == TARGET_STOPPED_BY_WATCHPOINT;
+ return (thread->priv != NULL
+ && thread->priv->stop_reason == TARGET_STOPPED_BY_WATCHPOINT);
}
static int
remote_stopped_data_address (struct target_ops *target, CORE_ADDR *addr_p)
{
- struct remote_state *rs = get_remote_state ();
- int rc = 0;
+ struct thread_info *thread = inferior_thread ();
- if (remote_stopped_by_watchpoint (target))
+ if (thread->priv != NULL
+ && thread->priv->stop_reason == TARGET_STOPPED_BY_WATCHPOINT)
{
- *addr_p = rs->remote_watch_data_address;
- rc = 1;
+ *addr_p = thread->priv->watch_data_address;
+ return 1;
}
- return rc;
+ return 0;
}
--
1.9.3
^ permalink raw reply [flat|nested] 60+ messages in thread* [PATCH 10/18] Remote thread create/exit events
2015-10-14 15:28 [PATCH 00/18] Remote all-stop on top of non-stop Pedro Alves
` (6 preceding siblings ...)
2015-10-14 15:33 ` [PATCH 05/18] remote: stop reason and watchpoint data address per thread Pedro Alves
@ 2015-10-14 15:33 ` Pedro Alves
2015-10-14 16:35 ` Eli Zaretskii
` (2 more replies)
2015-10-14 15:36 ` [PATCH 12/18] testsuite: Range stepping and non-stop mode Pedro Alves
` (12 subsequent siblings)
20 siblings, 3 replies; 60+ messages in thread
From: Pedro Alves @ 2015-10-14 15:33 UTC (permalink / raw)
To: gdb-patches
When testing with "maint set target-non-stop on", a few
threading-related tests expose an issue that requires new RSP packets.
Say there are 3 threads running, 1-3. If GDB tries to stop thread 1,
2 and 3, and then waits for their stops, but meanwhile say, thread 2
exits, GDB hangs forever waiting for a stop for thread 2 that won't
ever happen.
This patch fixes the issue by adding support for thread exit events to
the protocol. However, we don't want these always enabled, as they're
useless most of the time, and would slow down remote debugging. So I
made it so that GDB can enable/disable them, and then made gdb do that
around the cases that need it, which currently is only
infrun.c:stop_all_threads.
In turn, if we have thread exit events, then the extra "thread x
exited" traffic slows down attach-many-short-lived-threads.exp enough
that gdb has trouble keeping up with new threads that are spawned
while gdb tries to stop existing ones. To fix that I added support
for the counterpart thread created events too. Enabling those when we
try to stop threads ensures that new threads never get a chance to
themselves start new threads, killing the race.
gdb/doc/ChangeLog:
2015-10-14 Pedro Alves <palves@redhat.com>
* gdb.texinfo (Remote Configuration): List "set/show remote
thread-events" command in configuration table.
(Stop Reply Packets): Document "T05 create" stop
reason and 'w' stop reply.
(General Query Packets): Document QThreadEvents packet. Document
QThreadEvents qSupported feature.
gdb/gdbserver/ChangeLog:
2015-10-14 Pedro Alves <palves@redhat.com>
* linux-low.c (handle_extended_wait): Assert that the LWP's
waitstatus is TARGET_WAITKIND_IGNORE. If GDB wants to hear about
thread create events, leave the new child's status pending.
(linux_low_filter_event): If GDB wants to hear about thread exit
events, leave the LWP marked dead and don't delete it.
(linux_wait_for_event_filtered): Don't check for thread exit.
(filter_exit_event): New function.
(linux_wait_1): Use it, when returning an exit event.
(linux_resume_one_lwp_throw): Assert that the LWP's
waitstatus is TARGET_WAITKIND_IGNORE.
* remote-utils.c (prepare_resume_reply): Handle
TARGET_WAITKIND_THREAD_CREATED and TARGET_WAITKIND_THREAD_EXITED.
* server.c (report_thread_events): New global.
(handle_general_set): Handle QThreadEvents.
* server.h (report_thread_events): Declare.
gdb/ChangeLog:
2015-10-14 Pedro Alves <palves@redhat.com>
* NEWS (New commands): Mention "set/show remote thread-events"
commands.
(New remote packets): Mention thread created/exited stop reasons
and QThreadEvents packet.
* infrun.c (disable_thread_events): New function.
(stop_all_threads): Disable/enable thread create/exit events.
Handle TARGET_WAITKIND_THREAD_EXITED.
(handle_inferior_event_1): Handle TARGET_WAITKIND_THREAD_CREATED
and TARGET_WAITKIND_THREAD_EXITED.
* remote.c (remove_child_of_pending_fork): Also remove threads of
threads that have TARGET_WAITKIND_THREAD_EXITED events.
(remote_parse_stop_reply): Handle "create" magic register. Handle
'w' stop reply.
(initialize_remote): Install remote_thread_events as
to_thread_events target hook.
(remote_thread_events): New function.
* target-delegates.c: Regenerate.
* target.c (target_thread_events): New function.
* target.h (struct target_ops) <to_thread_events>: New field.
(target_thread_events): Declare.
* target/waitstatus.c (target_waitstatus_to_string): Handle
TARGET_WAITKIND_THREAD_CREATED and TARGET_WAITKIND_THREAD_EXITED.
* target/waitstatus.h (enum target_waitkind)
<TARGET_WAITKIND_THREAD_CREATED, TARGET_WAITKIND_THREAD_EXITED):
New values.
---
gdb/NEWS | 18 ++++++++++
gdb/doc/gdb.texinfo | 62 ++++++++++++++++++++++++++++++++++
gdb/gdbserver/linux-low.c | 79 +++++++++++++++++++++++++++++---------------
gdb/gdbserver/remote-utils.c | 13 ++++++++
gdb/gdbserver/server.c | 36 ++++++++++++++++++++
gdb/gdbserver/server.h | 1 +
gdb/infrun.c | 31 ++++++++++++++++-
gdb/remote.c | 71 ++++++++++++++++++++++++++++++++++++---
gdb/target-delegates.c | 28 ++++++++++++++++
gdb/target.c | 8 +++++
gdb/target.h | 5 +++
gdb/target/waitstatus.c | 5 +++
gdb/target/waitstatus.h | 8 ++++-
13 files changed, 331 insertions(+), 34 deletions(-)
diff --git a/gdb/NEWS b/gdb/NEWS
index 84c75cc..cb3b2fc 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -42,6 +42,10 @@ set remote multiprocess-extensions-packet
show remote multiprocess-extensions-packet
Set/show the use of the remote protocol multiprocess extensions.
+set remote thread-events
+show remote thread-events
+ Set/show the use of thread create/exit events.
+
* The "disassemble" command accepts a new modifier: /s.
It prints mixed source+disassembly like /m with two differences:
- disassembled instructions are now printed in program order, and
@@ -79,6 +83,20 @@ vCtrlC
Equivalent to interrupting with the ^C character, but works in
non-stop mode.
+thread created stop reason (T05 create:...)
+ Indicates that the thread was just created and is stopped at entry.
+
+thread exit stop reply (w exitcode;tid)
+ Indicates that the thread has terminated.
+
+QThreadEvents
+ Enables/disables thread create and exit event reporting. For
+ example, this is used in non-stop mode when GDB stops a set of
+ threads and synchronously waits for the their corresponding stop
+ replies. Without exit events, if one of the threads exits, GDB
+ would hang forever not knowing that it should no longer expect a
+ stop for that same thread.
+
* Extended-remote exec events
** GDB now has support for exec events on extended-remote Linux targets.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 196a842..1da9e4a 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -20232,6 +20232,10 @@ are:
@tab @code{exec stop reason}
@tab @code{exec}
+@item @code{thread-events}
+@tab @code{QThreadEvents}
+@tab Tracking thread lifetime.
+
@end multitable
@node Remote Stub
@@ -35543,6 +35547,15 @@ appropriate @samp{qSupported} feature (@pxref{qSupported}). The
remote stub must also supply the appropriate @samp{qSupported} feature
indicating support.
+@cindex thread create event, remote reply
+@anchor{thread create event}
+@item create
+The packet indicates that the thread was just created. The new thread
+is stopped until @value{GDBN} sets it running with a resumption packet
+(@pxref{vCont packet}). This packet should not be sent by default;
+@value{GDBN} requests it with the @ref{QThreadEvents} packet. See
+also the @samp{w} (@ref{thread exit event}) remote reply below.
+
@end table
@item W @var{AA}
@@ -35564,6 +35577,14 @@ terminated process, can be used only when @value{GDBN} has reported
support for multiprocess protocol extensions; see @ref{multiprocess
extensions}. The @var{pid} is formatted as a big-endian hex string.
+@anchor{thread exit event}
+@cindex thread exit event, remote reply
+@item w @var{AA} ; @var{tid}
+
+The thread exited, and @var{AA} is the exit status. This response
+should not be sent by default; @value{GDBN} requests it with the
+@ref{QThreadEvents} packet. See also @ref{thread create event} above.
+
@item O @var{XX}@dots{}
@samp{@var{XX}@dots{}} is hex encoding of @sc{ascii} data, to be
written as the program's console output. This can happen at any time
@@ -35981,6 +36002,39 @@ command (@pxref{Remote Configuration, set remote program-signals}).
This packet is not probed by default; the remote stub must request it,
by supplying an appropriate @samp{qSupported} response (@pxref{qSupported}).
+@anchor{QThreadEvents}
+@item QThreadEvents:1
+@itemx QThreadEvents:0
+@cindex thread create/exit events, remote request
+@cindex @samp{QThreadEvents} packet
+
+Enable (@samp{QThreadEvents:1}) or disable (@samp{QThreadEvents:0})
+reporting of thread create and exit events. @xref{thread create
+event}, for the reply specifications. For example, this is used in
+non-stop mode when @value{GDBN} stops a set of threads and
+synchronously waits for the their corresponding stop replies. Without
+exit events, if one of the threads exits, @value{GDBN} would hang
+forever not knowing that it should no longer expect a stop for that
+same thread. @value{GDBN} does not enable this feature unless the
+stub reports that it supports it by including @samp{QThreadEvents+} in
+its @samp{qSupported} reply.
+
+Reply:
+@table @samp
+@item OK
+The request succeeded.
+
+@item E @var{nn}
+An error occurred. The error number @var{nn} is given as hex digits.
+
+@item @w{}
+An empty reply indicates that @samp{QThreadEvents} is not supported by
+the stub.
+@end table
+
+Use of this packet is controlled by the @code{set remote thread-events}
+command (@pxref{Remote Configuration, set remote thread-events}).
+
@item qRcmd,@var{command}
@cindex execute remote command, remote request
@cindex @samp{qRcmd} packet
@@ -36425,6 +36479,11 @@ These are the currently defined stub features and their properties:
@tab @samp{-}
@tab No
+@item @samp{QThreadEvents}
+@tab No
+@tab @samp{-}
+@tab No
+
@end multitable
These are the currently defined stub features, in more detail:
@@ -36637,6 +36696,9 @@ The remote stub reports the @samp{exec} stop reason for exec events.
The remote stub reports the supported actions in the reply to
@samp{vCont?} packet.
+@item QThreadEvents
+The remote stub understands the @samp{QThreadEvents} packet.
+
@end table
@item qSymbol::
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 0d228cc..1c447c2 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -457,6 +457,8 @@ handle_extended_wait (struct lwp_info **orig_event_lwp, int wstat)
struct thread_info *event_thr = get_lwp_thread (event_lwp);
struct lwp_info *new_lwp;
+ gdb_assert (event_lwp->waitstatus.kind == TARGET_WAITKIND_IGNORE);
+
if ((event == PTRACE_EVENT_FORK) || (event == PTRACE_EVENT_VFORK)
|| (event == PTRACE_EVENT_CLONE))
{
@@ -587,6 +589,12 @@ handle_extended_wait (struct lwp_info **orig_event_lwp, int wstat)
new_lwp->status_pending_p = 1;
new_lwp->status_pending = status;
}
+ else if (report_thread_events)
+ {
+ new_lwp->waitstatus.kind = TARGET_WAITKIND_THREAD_CREATED;
+ new_lwp->status_pending_p = 1;
+ new_lwp->status_pending = status;
+ }
/* Don't report the event. */
return 1;
@@ -2266,25 +2274,22 @@ linux_low_filter_event (int lwpid, int wstat)
{
if (debug_threads)
debug_printf ("LLFE: %d exited.\n", lwpid);
- if (num_lwps (pid_of (thread)) > 1)
+ /* If there is at least one more LWP, then the exit signal was
+ not the end of the debugged application and should be
+ ignored, unless GDB wants to hear about thread exits. */
+ if (report_thread_events
+ || last_thread_of_process_p (pid_of (thread)))
{
-
- /* If there is at least one more LWP, then the exit signal was
- not the end of the debugged application and should be
- ignored. */
- delete_lwp (child);
- return NULL;
+ /* Since events are serialized to GDB core, and we can't
+ report this one right now. Leave the status pending for
+ the next time we're able to report it. */
+ mark_lwp_dead (child, wstat);
+ return child;
}
else
{
- /* This was the last lwp in the process. Since events are
- serialized to GDB core, and we can't report this one
- right now, but GDB core and the other target layers will
- want to be notified about the exit code/signal, leave the
- status pending for the next time we're able to report
- it. */
- mark_lwp_dead (child, wstat);
- return child;
+ delete_lwp (child);
+ return NULL;
}
}
@@ -2627,18 +2632,6 @@ linux_wait_for_event_filtered (ptid_t wait_ptid, ptid_t filter_ptid,
current_thread = event_thread;
- /* Check for thread exit. */
- if (! WIFSTOPPED (*wstatp))
- {
- gdb_assert (last_thread_of_process_p (pid_of (event_thread)));
-
- if (debug_threads)
- debug_printf ("LWP %d is the last lwp of process. "
- "Process %ld exiting.\n",
- pid_of (event_thread), lwpid_of (event_thread));
- return lwpid_of (event_thread);
- }
-
return lwpid_of (event_thread);
}
@@ -2923,6 +2916,30 @@ ignore_event (struct target_waitstatus *ourstatus)
return null_ptid;
}
+/* Convenience function that is called when the kernel reports an exit
+ event. This decides whether to report the event to GDB as a
+ process exit event, a thread exit event, or to suppress the
+ event. */
+
+static ptid_t
+filter_exit_event (struct lwp_info *event_child,
+ struct target_waitstatus *ourstatus)
+{
+ struct thread_info *thread = get_lwp_thread (event_child);
+ ptid_t ptid = ptid_of (thread);
+
+ if (!last_thread_of_process_p (pid_of (thread)))
+ {
+ if (report_thread_events)
+ ourstatus->kind = TARGET_WAITKIND_THREAD_EXITED;
+ else
+ ourstatus->kind = TARGET_WAITKIND_IGNORE;
+
+ delete_lwp (event_child);
+ }
+ return ptid;
+}
+
/* Wait for process, returns status. */
static ptid_t
@@ -3028,6 +3045,9 @@ linux_wait_1 (ptid_t ptid,
}
}
+ if (ourstatus->kind == TARGET_WAITKIND_EXITED)
+ return filter_exit_event (event_child, ourstatus);
+
return ptid_of (current_thread);
}
@@ -3512,6 +3532,9 @@ linux_wait_1 (ptid_t ptid,
debug_exit ();
}
+ if (ourstatus->kind == TARGET_WAITKIND_EXITED)
+ return filter_exit_event (event_child, ourstatus);
+
return ptid_of (current_thread);
}
@@ -3912,6 +3935,8 @@ linux_resume_one_lwp_throw (struct lwp_info *lwp,
if (lwp->stopped == 0)
return;
+ gdb_assert (lwp->waitstatus.kind == TARGET_WAITKIND_IGNORE);
+
fast_tp_collecting = lwp->collecting_fast_tracepoint;
gdb_assert (!stabilizing_threads || fast_tp_collecting);
diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
index e366091..0d9cde3 100644
--- a/gdb/gdbserver/remote-utils.c
+++ b/gdb/gdbserver/remote-utils.c
@@ -1119,6 +1119,7 @@ prepare_resume_reply (char *buf, ptid_t ptid,
case TARGET_WAITKIND_VFORKED:
case TARGET_WAITKIND_VFORK_DONE:
case TARGET_WAITKIND_EXECD:
+ case TARGET_WAITKIND_THREAD_CREATED:
{
struct thread_info *saved_thread;
const char **regp;
@@ -1161,6 +1162,13 @@ prepare_resume_reply (char *buf, ptid_t ptid,
status->value.execd_pathname = NULL;
buf += strlen (buf);
}
+ else if (status->kind == TARGET_WAITKIND_THREAD_CREATED
+ && report_thread_events)
+ {
+ enum gdb_signal signal = GDB_SIGNAL_TRAP;
+
+ sprintf (buf, "T%02xcreate:;", signal);
+ }
else
sprintf (buf, "T%02x", status->value.sig);
@@ -1276,6 +1284,11 @@ prepare_resume_reply (char *buf, ptid_t ptid,
else
sprintf (buf, "X%02x", status->value.sig);
break;
+ case TARGET_WAITKIND_THREAD_EXITED:
+ sprintf (buf, "w%x;", status->value.integer);
+ buf += strlen (buf);
+ buf = write_ptid (buf, ptid);
+ break;
default:
error ("unhandled waitkind");
break;
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index e0ce524..2ffc5d0 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -60,6 +60,7 @@ int multi_process;
int report_fork_events;
int report_vfork_events;
int report_exec_events;
+int report_thread_events;
int non_stop;
int swbreak_feature;
int hwbreak_feature;
@@ -723,6 +724,39 @@ handle_general_set (char *own_buf)
if (handle_btrace_conf_general_set (own_buf))
return;
+ if (startswith (own_buf, "QThreadEvents:"))
+ {
+ char *mode = own_buf + strlen ("QThreadEvents:");
+ enum tribool req = TRIBOOL_UNKNOWN;
+
+ if (strcmp (mode, "0") == 0)
+ req = TRIBOOL_FALSE;
+ else if (strcmp (mode, "1") == 0)
+ req = TRIBOOL_TRUE;
+ else
+ {
+ char *mode_copy = xstrdup (mode);
+
+ /* We don't know what this mode is, so complain to GDB. */
+ sprintf (own_buf, "E.Unknown thread-events mode requested: %s\n",
+ mode_copy);
+ xfree (mode_copy);
+ return;
+ }
+
+ report_thread_events = (req == TRIBOOL_TRUE);
+
+ if (remote_debug)
+ {
+ const char *req_str = report_thread_events ? "enabled" : "disabled";
+
+ fprintf (stderr, "[thread events are now %s]\n", req_str);
+ }
+
+ write_ok (own_buf);
+ return;
+ }
+
/* Otherwise we didn't know what packet it was. Say we didn't
understand it. */
own_buf[0] = 0;
@@ -2259,6 +2293,8 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
strcat (own_buf, ";vContSupported+");
+ strcat (own_buf, ";QThreadEvents+");
+
/* Reinitialize components as needed for the new connection. */
hostio_handle_new_gdb_connection ();
target_handle_new_gdb_connection ();
diff --git a/gdb/gdbserver/server.h b/gdb/gdbserver/server.h
index 96ad4fa..dc0361f 100644
--- a/gdb/gdbserver/server.h
+++ b/gdb/gdbserver/server.h
@@ -87,6 +87,7 @@ extern int multi_process;
extern int report_fork_events;
extern int report_vfork_events;
extern int report_exec_events;
+extern int report_thread_events;
extern int non_stop;
extern int extended_protocol;
diff --git a/gdb/infrun.c b/gdb/infrun.c
index d2587f1..6052e8f 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -4388,6 +4388,14 @@ save_waitstatus (struct thread_info *tp, struct target_waitstatus *ws)
}
}
+/* A cleanup that disables thread create/exit events. */
+
+static void
+disable_thread_events (void *arg)
+{
+ target_thread_events (0);
+}
+
/* See infrun.h. */
void
@@ -4407,6 +4415,9 @@ stop_all_threads (void)
entry_ptid = inferior_ptid;
old_chain = make_cleanup (switch_to_thread_cleanup, &entry_ptid);
+ target_thread_events (1);
+ make_cleanup (disable_thread_events, NULL);
+
/* Request threads to stop, and then wait for the stops. Because
threads we already know about can spawn more threads while we're
trying to stop them, and we only learn about new threads when we
@@ -4484,7 +4495,8 @@ stop_all_threads (void)
{
/* All resumed threads exited. */
}
- else if (ws.kind == TARGET_WAITKIND_EXITED
+ else if (ws.kind == TARGET_WAITKIND_THREAD_EXITED
+ || ws.kind == TARGET_WAITKIND_EXITED
|| ws.kind == TARGET_WAITKIND_SIGNALLED)
{
if (debug_infrun)
@@ -4635,6 +4647,14 @@ handle_inferior_event_1 (struct execution_control_state *ecs)
return;
}
+ if (ecs->ws.kind == TARGET_WAITKIND_THREAD_EXITED)
+ {
+ if (debug_infrun)
+ fprintf_unfiltered (gdb_stdlog, "infrun: TARGET_WAITKIND_THREAD_EXITED\n");
+ prepare_to_wait (ecs);
+ return;
+ }
+
if (ecs->ws.kind == TARGET_WAITKIND_NO_RESUMED
&& target_can_async_p () && !sync_execution)
{
@@ -4839,6 +4859,15 @@ handle_inferior_event_1 (struct execution_control_state *ecs)
prepare_to_wait (ecs);
return;
+ case TARGET_WAITKIND_THREAD_CREATED:
+ if (debug_infrun)
+ fprintf_unfiltered (gdb_stdlog, "infrun: TARGET_WAITKIND_THREAD_CREATED\n");
+ if (!ptid_equal (ecs->ptid, inferior_ptid))
+ context_switch (ecs->ptid);
+ if (!switch_back_to_stepped_thread (ecs))
+ keep_going (ecs);
+ return;
+
case TARGET_WAITKIND_EXITED:
case TARGET_WAITKIND_SIGNALLED:
if (debug_infrun)
diff --git a/gdb/remote.c b/gdb/remote.c
index 9a23ca6..f4714bc 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -139,6 +139,8 @@ static int remote_is_async_p (struct target_ops *);
static void remote_async (struct target_ops *ops, int enable);
+static void remote_thread_events (struct target_ops *ops, int enable);
+
static void sync_remote_interrupt_twice (int signo);
static void interrupt_query (void);
@@ -1434,6 +1436,9 @@ enum {
/* Support for the QNonStop packet. */
PACKET_QNonStop,
+ /* Support for the QThreadEvents packet. */
+ PACKET_QThreadEvents,
+
/* Support for multi-process extensions. */
PACKET_multiprocess_feature,
@@ -4494,7 +4499,8 @@ static const struct protocol_feature remote_protocol_features[] = {
PACKET_exec_event_feature },
{ "Qbtrace-conf:pt:size", PACKET_DISABLE, remote_supported_packet,
PACKET_Qbtrace_conf_pt_size },
- { "vContSupported", PACKET_DISABLE, remote_supported_packet, PACKET_vContSupported }
+ { "vContSupported", PACKET_DISABLE, remote_supported_packet, PACKET_vContSupported },
+ { "QThreadEvents", PACKET_DISABLE, remote_supported_packet, PACKET_QThreadEvents },
};
static char *remote_support_xml;
@@ -4590,6 +4596,9 @@ remote_query_supported (void)
if (packet_set_cmd_state (PACKET_vContSupported) != AUTO_BOOLEAN_FALSE)
q = remote_query_supported_append (q, "vContSupported+");
+ if (packet_set_cmd_state (PACKET_QThreadEvents) != AUTO_BOOLEAN_FALSE)
+ q = remote_query_supported_append (q, "QThreadEvents+");
+
q = reconcat (q, "qSupported:", q, (char *) NULL);
putpkt (q);
@@ -6016,10 +6025,9 @@ remove_child_of_pending_fork (QUEUE (stop_reply_p) *q,
= (struct threads_listing_context *) param->input;
if (event->ws.kind == TARGET_WAITKIND_FORKED
- || event->ws.kind == TARGET_WAITKIND_VFORKED)
- {
- threads_listing_context_remove (&event->ws, context);
- }
+ || event->ws.kind == TARGET_WAITKIND_VFORKED
+ || event->ws.kind == TARGET_WAITKIND_THREAD_EXITED)
+ threads_listing_context_remove (&event->ws, context);
return 1;
}
@@ -6417,6 +6425,11 @@ Packet: '%s'\n"),
one used by the original program. */
skipregs = 1;
}
+ else if (strprefix (p, p1, "create"))
+ {
+ event->ws.kind = TARGET_WAITKIND_THREAD_CREATED;
+ p = skip_to_semicolon (p1 + 1);
+ }
else
{
ULONGEST pnum;
@@ -6487,6 +6500,19 @@ Packet: '%s'\n"),
event->ws.value.sig = GDB_SIGNAL_UNKNOWN;
}
break;
+ case 'w': /* Thread exited. */
+ {
+ char *p;
+ ULONGEST value;
+
+ event->ws.kind = TARGET_WAITKIND_THREAD_EXITED;
+ p = unpack_varlen_hex (&buf[1], &value);
+ event->ws.value.integer = value;
+ if (*p != ';')
+ error (_("stop reply packet badly formatted: %s"), buf);
+ event->ptid = read_ptid (++p, &p);
+ break;
+ }
case 'W': /* Target exited. */
case 'X':
{
@@ -12941,6 +12967,7 @@ Specify the serial device it is connected to\n\
remote_ops.to_can_async_p = remote_can_async_p;
remote_ops.to_is_async_p = remote_is_async_p;
remote_ops.to_async = remote_async;
+ remote_ops.to_thread_events = remote_thread_events;
remote_ops.to_can_do_single_step = remote_can_do_single_step;
remote_ops.to_terminal_inferior = remote_terminal_inferior;
remote_ops.to_terminal_ours = remote_terminal_ours;
@@ -13118,6 +13145,37 @@ remote_async (struct target_ops *ops, int enable)
}
}
+/* Implementation of the to_thread_events method. */
+
+static void
+remote_thread_events (struct target_ops *ops, int enable)
+{
+ struct remote_state *rs = get_remote_state ();
+ size_t size = get_remote_packet_size ();
+ char *p = rs->buf;
+
+ if (packet_support (PACKET_QThreadEvents) == PACKET_DISABLE)
+ return;
+
+ xsnprintf (rs->buf, size, "QThreadEvents:%x", enable ? 1 : 0);
+ putpkt (rs->buf);
+ getpkt (&rs->buf, &rs->buf_size, 0);
+
+ switch (packet_ok (rs->buf,
+ &remote_protocol_packets[PACKET_QThreadEvents]))
+ {
+ case PACKET_OK:
+ if (strcmp (rs->buf, "OK") != 0)
+ error (_("Remote refused setting thread events: %s"), rs->buf);
+ break;
+ case PACKET_ERROR:
+ warning (_("Remote failure reply: %s"), rs->buf);
+ break;
+ case PACKET_UNKNOWN:
+ break;
+ }
+}
+
static void
set_remote_cmd (char *args, int from_tty)
{
@@ -13662,6 +13720,9 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL,
add_packet_config_cmd (&remote_protocol_packets[PACKET_vCtrlC],
"vCtrlC", "ctrl-c", 0);
+ add_packet_config_cmd (&remote_protocol_packets[PACKET_QThreadEvents],
+ "QThreadEvents", "thread-events", 0);
+
/* Assert that we've registered "set remote foo-packet" commands
for all packet configs. */
{
diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index a7271fa..58dc133 100644
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -1809,6 +1809,30 @@ debug_async (struct target_ops *self, int arg1)
fputs_unfiltered (")\n", gdb_stdlog);
}
+static void
+delegate_thread_events (struct target_ops *self, int arg1)
+{
+ self = self->beneath;
+ self->to_thread_events (self, arg1);
+}
+
+static void
+tdefault_thread_events (struct target_ops *self, int arg1)
+{
+}
+
+static void
+debug_thread_events (struct target_ops *self, int arg1)
+{
+ fprintf_unfiltered (gdb_stdlog, "-> %s->to_thread_events (...)\n", debug_target.to_shortname);
+ debug_target.to_thread_events (&debug_target, arg1);
+ fprintf_unfiltered (gdb_stdlog, "<- %s->to_thread_events (", debug_target.to_shortname);
+ target_debug_print_struct_target_ops_p (&debug_target);
+ fputs_unfiltered (", ", gdb_stdlog);
+ target_debug_print_int (arg1);
+ fputs_unfiltered (")\n", gdb_stdlog);
+}
+
static int
delegate_supports_non_stop (struct target_ops *self)
{
@@ -4186,6 +4210,8 @@ install_delegators (struct target_ops *ops)
ops->to_is_async_p = delegate_is_async_p;
if (ops->to_async == NULL)
ops->to_async = delegate_async;
+ if (ops->to_thread_events == NULL)
+ ops->to_thread_events = delegate_thread_events;
if (ops->to_supports_non_stop == NULL)
ops->to_supports_non_stop = delegate_supports_non_stop;
if (ops->to_always_non_stop_p == NULL)
@@ -4424,6 +4450,7 @@ install_dummy_methods (struct target_ops *ops)
ops->to_can_async_p = tdefault_can_async_p;
ops->to_is_async_p = tdefault_is_async_p;
ops->to_async = tdefault_async;
+ ops->to_thread_events = tdefault_thread_events;
ops->to_supports_non_stop = tdefault_supports_non_stop;
ops->to_always_non_stop_p = tdefault_always_non_stop_p;
ops->to_find_memory_regions = dummy_find_memory_regions;
@@ -4579,6 +4606,7 @@ init_debug_target (struct target_ops *ops)
ops->to_can_async_p = debug_can_async_p;
ops->to_is_async_p = debug_is_async_p;
ops->to_async = debug_async;
+ ops->to_thread_events = debug_thread_events;
ops->to_supports_non_stop = debug_supports_non_stop;
ops->to_always_non_stop_p = debug_always_non_stop_p;
ops->to_find_memory_regions = debug_find_memory_regions;
diff --git a/gdb/target.c b/gdb/target.c
index b8b1e9b..6bc1c02 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -3833,6 +3833,14 @@ target_async (int enable)
current_target.to_async (¤t_target, enable);
}
+/* See target.h. */
+
+void
+target_thread_events (int enable)
+{
+ current_target.to_thread_events (¤t_target, enable);
+}
+
/* Controls if targets can report that they can/are async. This is
just for maintainers to use when debugging gdb. */
int target_async_permitted = 1;
diff --git a/gdb/target.h b/gdb/target.h
index 2b2db45..007f293 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -670,6 +670,8 @@ struct target_ops
TARGET_DEFAULT_RETURN (0);
void (*to_async) (struct target_ops *, int)
TARGET_DEFAULT_NORETURN (tcomplain ());
+ void (*to_thread_events) (struct target_ops *, int)
+ TARGET_DEFAULT_IGNORE ();
/* This method must be implemented in some situations. See the
comment on 'to_can_run'. */
int (*to_supports_non_stop) (struct target_ops *)
@@ -1791,6 +1793,9 @@ extern int target_async_permitted;
/* Enables/disabled async target events. */
extern void target_async (int enable);
+/* Enables/disables thread create and exit events. */
+extern void target_thread_events (int enable);
+
/* Whether support for controlling the target backends always in
non-stop mode is enabled. */
extern enum auto_boolean target_non_stop_enabled;
diff --git a/gdb/target/waitstatus.c b/gdb/target/waitstatus.c
index 739c4b8..42b5620 100644
--- a/gdb/target/waitstatus.c
+++ b/gdb/target/waitstatus.c
@@ -63,6 +63,11 @@ target_waitstatus_to_string (const struct target_waitstatus *ws)
return xstrprintf ("%sno-history", kind_str);
case TARGET_WAITKIND_NO_RESUMED:
return xstrprintf ("%sno-resumed", kind_str);
+ case TARGET_WAITKIND_THREAD_CREATED:
+ return xstrprintf ("%sthread created", kind_str);
+ case TARGET_WAITKIND_THREAD_EXITED:
+ return xstrprintf ("%sthread exited, status = %d",
+ kind_str, ws->value.integer);
default:
return xstrprintf ("%sunknown???", kind_str);
}
diff --git a/gdb/target/waitstatus.h b/gdb/target/waitstatus.h
index ffaddc1..2e39344 100644
--- a/gdb/target/waitstatus.h
+++ b/gdb/target/waitstatus.h
@@ -92,7 +92,13 @@ enum target_waitkind
TARGET_WAITKIND_NO_HISTORY,
/* There are no resumed children left in the program. */
- TARGET_WAITKIND_NO_RESUMED
+ TARGET_WAITKIND_NO_RESUMED,
+
+ /* The thread was created. */
+ TARGET_WAITKIND_THREAD_CREATED,
+
+ /* The thread has exited. The exit status is in value.integer. */
+ TARGET_WAITKIND_THREAD_EXITED,
};
struct target_waitstatus
--
1.9.3
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH 10/18] Remote thread create/exit events
2015-10-14 15:33 ` [PATCH 10/18] Remote thread create/exit events Pedro Alves
@ 2015-10-14 16:35 ` Eli Zaretskii
2015-10-26 16:50 ` Yao Qi
2015-12-01 15:12 ` Ulrich Weigand
2 siblings, 0 replies; 60+ messages in thread
From: Eli Zaretskii @ 2015-10-14 16:35 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
> From: Pedro Alves <palves@redhat.com>
> Date: Wed, 14 Oct 2015 16:27:58 +0100
>
> gdb/doc/ChangeLog:
> 2015-10-14 Pedro Alves <palves@redhat.com>
>
> * gdb.texinfo (Remote Configuration): List "set/show remote
> thread-events" command in configuration table.
> (Stop Reply Packets): Document "T05 create" stop
> reason and 'w' stop reply.
> (General Query Packets): Document QThreadEvents packet. Document
> QThreadEvents qSupported feature.
>
> gdb/gdbserver/ChangeLog:
> 2015-10-14 Pedro Alves <palves@redhat.com>
>
> * linux-low.c (handle_extended_wait): Assert that the LWP's
> waitstatus is TARGET_WAITKIND_IGNORE. If GDB wants to hear about
> thread create events, leave the new child's status pending.
> (linux_low_filter_event): If GDB wants to hear about thread exit
> events, leave the LWP marked dead and don't delete it.
> (linux_wait_for_event_filtered): Don't check for thread exit.
> (filter_exit_event): New function.
> (linux_wait_1): Use it, when returning an exit event.
> (linux_resume_one_lwp_throw): Assert that the LWP's
> waitstatus is TARGET_WAITKIND_IGNORE.
> * remote-utils.c (prepare_resume_reply): Handle
> TARGET_WAITKIND_THREAD_CREATED and TARGET_WAITKIND_THREAD_EXITED.
> * server.c (report_thread_events): New global.
> (handle_general_set): Handle QThreadEvents.
> * server.h (report_thread_events): Declare.
>
> gdb/ChangeLog:
> 2015-10-14 Pedro Alves <palves@redhat.com>
>
> * NEWS (New commands): Mention "set/show remote thread-events"
> commands.
> (New remote packets): Mention thread created/exited stop reasons
> and QThreadEvents packet.
> * infrun.c (disable_thread_events): New function.
> (stop_all_threads): Disable/enable thread create/exit events.
> Handle TARGET_WAITKIND_THREAD_EXITED.
> (handle_inferior_event_1): Handle TARGET_WAITKIND_THREAD_CREATED
> and TARGET_WAITKIND_THREAD_EXITED.
> * remote.c (remove_child_of_pending_fork): Also remove threads of
> threads that have TARGET_WAITKIND_THREAD_EXITED events.
> (remote_parse_stop_reply): Handle "create" magic register. Handle
> 'w' stop reply.
> (initialize_remote): Install remote_thread_events as
> to_thread_events target hook.
> (remote_thread_events): New function.
> * target-delegates.c: Regenerate.
> * target.c (target_thread_events): New function.
> * target.h (struct target_ops) <to_thread_events>: New field.
> (target_thread_events): Declare.
> * target/waitstatus.c (target_waitstatus_to_string): Handle
> TARGET_WAITKIND_THREAD_CREATED and TARGET_WAITKIND_THREAD_EXITED.
> * target/waitstatus.h (enum target_waitkind)
> <TARGET_WAITKIND_THREAD_CREATED, TARGET_WAITKIND_THREAD_EXITED):
> New values.
OK for the documentation parts, thanks.
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH 10/18] Remote thread create/exit events
2015-10-14 15:33 ` [PATCH 10/18] Remote thread create/exit events Pedro Alves
2015-10-14 16:35 ` Eli Zaretskii
@ 2015-10-26 16:50 ` Yao Qi
2015-11-23 15:41 ` Pedro Alves
2015-12-01 15:12 ` Ulrich Weigand
2 siblings, 1 reply; 60+ messages in thread
From: Yao Qi @ 2015-10-26 16:50 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
Pedro Alves <palves@redhat.com> writes:
Hi Pedro,
> @@ -6417,6 +6425,11 @@ Packet: '%s'\n"),
> one used by the original program. */
> skipregs = 1;
> }
> + else if (strprefix (p, p1, "create"))
> + {
> + event->ws.kind = TARGET_WAITKIND_THREAD_CREATED;
> + p = skip_to_semicolon (p1 + 1);
> + }
> else
> {
> ULONGEST pnum;
Looks you add thread create/exit events on current stop notification.
I am wondering why don't we add a new notification for thread
create/exit. It should straightforward to add a new notification under
current remote notification infrastructure.
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH 10/18] Remote thread create/exit events
2015-10-26 16:50 ` Yao Qi
@ 2015-11-23 15:41 ` Pedro Alves
0 siblings, 0 replies; 60+ messages in thread
From: Pedro Alves @ 2015-11-23 15:41 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On 10/26/2015 01:54 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
>
> Hi Pedro,
>
>> @@ -6417,6 +6425,11 @@ Packet: '%s'\n"),
>> one used by the original program. */
>> skipregs = 1;
>> }
>> + else if (strprefix (p, p1, "create"))
>> + {
>> + event->ws.kind = TARGET_WAITKIND_THREAD_CREATED;
>> + p = skip_to_semicolon (p1 + 1);
>> + }
>> else
>> {
>> ULONGEST pnum;
>
> Looks you add thread create/exit events on current stop notification.
> I am wondering why don't we add a new notification for thread
> create/exit. It should straightforward to add a new notification under
> current remote notification infrastructure.
Actually, TARGET_WAITKIND_THREAD_CREATED is really a stop, just like
TARGET_WAITKIND_FORKED, etc. That is, when it is reported, the thread
is stopped at its entry point.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH 10/18] Remote thread create/exit events
2015-10-14 15:33 ` [PATCH 10/18] Remote thread create/exit events Pedro Alves
2015-10-14 16:35 ` Eli Zaretskii
2015-10-26 16:50 ` Yao Qi
@ 2015-12-01 15:12 ` Ulrich Weigand
2015-12-01 16:06 ` Pedro Alves
2 siblings, 1 reply; 60+ messages in thread
From: Ulrich Weigand @ 2015-12-01 15:12 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
Pedro Alves wrote:
> + case 'w': /* Thread exited. */
> + {
> + char *p;
> + ULONGEST value;
> +
> + event->ws.kind = TARGET_WAITKIND_THREAD_EXITED;
> + p = unpack_varlen_hex (&buf[1], &value);
> + event->ws.value.integer = value;
> + if (*p != ';')
> + error (_("stop reply packet badly formatted: %s"), buf);
> + event->ptid = read_ptid (++p, &p);
This causes a build error on my RHEL 5 daily build system (using
the GCC 4.1.2 host compiler) due to:
gdb/remote.c: In function 'remote_parse_stop_reply':
gdb/remote.c:6549: warning: operation on 'p' may be undefined
I'm not 100% convinced this is really undefined, since the
value of &p doesn't change whether or not ++p is evaluated
before or after it.
But in any case, since p isn't used afterwards, any reason why
this couldn't instead just be:
event->ptid = read_ptid (++p, NULL);
Bye,
Ulrich
--
Dr. Ulrich Weigand
GNU/Linux compilers and toolchain
Ulrich.Weigand@de.ibm.com
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH 10/18] Remote thread create/exit events
2015-12-01 15:12 ` Ulrich Weigand
@ 2015-12-01 16:06 ` Pedro Alves
2015-12-01 17:10 ` Ulrich Weigand
0 siblings, 1 reply; 60+ messages in thread
From: Pedro Alves @ 2015-12-01 16:06 UTC (permalink / raw)
To: Ulrich Weigand; +Cc: gdb-patches
On 12/01/2015 03:12 PM, Ulrich Weigand wrote:
> Pedro Alves wrote:
>
>> + case 'w': /* Thread exited. */
>> + {
>> + char *p;
>> + ULONGEST value;
>> +
>> + event->ws.kind = TARGET_WAITKIND_THREAD_EXITED;
>> + p = unpack_varlen_hex (&buf[1], &value);
>> + event->ws.value.integer = value;
>> + if (*p != ';')
>> + error (_("stop reply packet badly formatted: %s"), buf);
>> + event->ptid = read_ptid (++p, &p);
>
> This causes a build error on my RHEL 5 daily build system (using
> the GCC 4.1.2 host compiler) due to:
>
> gdb/remote.c: In function 'remote_parse_stop_reply':
> gdb/remote.c:6549: warning: operation on 'p' may be undefined
>
> I'm not 100% convinced this is really undefined, since the
> value of &p doesn't change whether or not ++p is evaluated
> before or after it.
I agree.
>
> But in any case, since p isn't used afterwards, any reason why
> this couldn't instead just be:
>
> event->ptid = read_ptid (++p, NULL);
Indeed, that should work.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH 10/18] Remote thread create/exit events
2015-12-01 16:06 ` Pedro Alves
@ 2015-12-01 17:10 ` Ulrich Weigand
0 siblings, 0 replies; 60+ messages in thread
From: Ulrich Weigand @ 2015-12-01 17:10 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
Pedro Alves wrote:
> On 12/01/2015 03:12 PM, Ulrich Weigand wrote:
> > But in any case, since p isn't used afterwards, any reason why
> > this couldn't instead just be:
> >
> > event->ptid = read_ptid (++p, NULL);
>
> Indeed, that should work.
OK, I've now checked in the following patch to fix the build break.
Thanks,
Ulrich
ChangeLog:
* remote.c (remote_parse_stop_reply): Avoid GCC 4.1 "operation
may be undefined" warning.
diff --git a/gdb/remote.c b/gdb/remote.c
index c60f173..52c5df8 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -6548,7 +6548,7 @@ Packet: '%s'\n"),
event->ws.value.integer = value;
if (*p != ';')
error (_("stop reply packet badly formatted: %s"), buf);
- event->ptid = read_ptid (++p, &p);
+ event->ptid = read_ptid (++p, NULL);
break;
}
case 'W': /* Target exited. */
--
Dr. Ulrich Weigand
GNU/Linux compilers and toolchain
Ulrich.Weigand@de.ibm.com
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH 12/18] testsuite: Range stepping and non-stop mode
2015-10-14 15:28 [PATCH 00/18] Remote all-stop on top of non-stop Pedro Alves
` (7 preceding siblings ...)
2015-10-14 15:33 ` [PATCH 10/18] Remote thread create/exit events Pedro Alves
@ 2015-10-14 15:36 ` Pedro Alves
2015-10-14 15:36 ` [PATCH 17/18] gdbserver: don't exit until GDB disconnects Pedro Alves
` (11 subsequent siblings)
20 siblings, 0 replies; 60+ messages in thread
From: Pedro Alves @ 2015-10-14 15:36 UTC (permalink / raw)
To: gdb-patches
The range-stepping tests fail with "maint set target-non-stop on" mode
because exec_cmd_expect_vCont_count doesn't know that in non-stop
mode, vCont's reply is simply "OK".
gdb/testsuite/ChangeLog:
2015-10-14 Pedro Alves <palves@redhat.com>
* lib/range-stepping-support.exp (exec_cmd_expect_vCont_count):
Handle non-stop mode vCont replies.
---
gdb/testsuite/lib/range-stepping-support.exp | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/gdb/testsuite/lib/range-stepping-support.exp b/gdb/testsuite/lib/range-stepping-support.exp
index d6c0e85..78c1c7b 100644
--- a/gdb/testsuite/lib/range-stepping-support.exp
+++ b/gdb/testsuite/lib/range-stepping-support.exp
@@ -26,12 +26,15 @@ proc exec_cmd_expect_vCont_count { cmd exp_vCont_s exp_vCont_r } {
set r_counter 0
set s_counter 0
set ret 1
+ # We either get a stop reply in all-stop mode, or an OK in
+ # non-stop mode.
+ set vcont_reply "(T\[\[:xdigit:\]\]\[\[:xdigit:\]\]|OK)"
gdb_test_multiple $cmd $test {
- -re "vCont;s\[^\r\n\]*Packet received: T\[\[:xdigit:\]\]\[\[:xdigit:\]\]" {
+ -re "vCont;s\[^\r\n\]*Packet received: $vcont_reply" {
incr s_counter
exp_continue
}
- -re "vCont;r\[^\r\n\]*Packet received: T\[\[:xdigit:\]\]\[\[:xdigit:\]\]" {
+ -re "vCont;r\[^\r\n\]*Packet received: $vcont_reply" {
incr r_counter
exp_continue
}
--
1.9.3
^ permalink raw reply [flat|nested] 60+ messages in thread* [PATCH 17/18] gdbserver: don't exit until GDB disconnects
2015-10-14 15:28 [PATCH 00/18] Remote all-stop on top of non-stop Pedro Alves
` (8 preceding siblings ...)
2015-10-14 15:36 ` [PATCH 12/18] testsuite: Range stepping and non-stop mode Pedro Alves
@ 2015-10-14 15:36 ` Pedro Alves
2015-10-14 15:36 ` [PATCH 14/18] Implement TARGET_WAITKIND_NO_RESUMED in the remote protocol Pedro Alves
` (10 subsequent siblings)
20 siblings, 0 replies; 60+ messages in thread
From: Pedro Alves @ 2015-10-14 15:36 UTC (permalink / raw)
To: gdb-patches
When testing with "target remote" with "maint set target-non-stop on",
we regressions like this:
Running /home/pedro/gdb/mygit/build/../src/gdb/testsuite/gdb.threads/continue-pending-after-query.exp ...
FAIL: gdb.threads/continue-pending-after-query.exp: iter 4: continue until exit
FAIL: gdb.threads/continue-pending-after-query.exp: iter 6: continue until exit
FAIL: gdb.threads/continue-pending-after-query.exp: iter 10: continue until exit
=== gdb Summary ===
# of expected passes 28
# of unexpected failures 3
where gdb.log shows:
continue
Continuing.
Remote communication error. Target disconnected.: Connection reset by peer.
(gdb) FAIL: gdb.threads/continue-pending-after-query.exp: iter 4: continue until exit
Enabling gdb + gdbserver debug logs we see:
gdbserver: <<<< exiting linux_wait_1
gdbserver: handling possible serial event
gdbserver: Writing resume reply for LWP 11089.11089:0
gdbserver: handling possible serial event
gdbserver: GDBserver exiting
GDB: Packet received: OK
GDB: infrun: prepare_to_wait
GDB: Sending packet: $vStopped#55...Packet received: W0;process:2b51
GDB: Sending packet: $vStopped#55...Packet received: OK
GDB: infrun: target_wait (-1.0.0, status) =
GDB: infrun: -1.0.0 [Thread 0],
GDB: infrun: status->kind = no-resumed
GDB: Sending packet: $Hgp2b51.2b51#41...Remote connection closed
(gdb) FAIL: gdb.threads/continue-pending-after-query.exp: iter 1: continue until exit
Notice the "Packet received: W0;process:2b51" followed by
vStopped->OK.
That means the process exit notification was successfully sent to GDB
and GDB fetched it. That makes gdbserver exit, in
server.c:process_serial_event:
if (!extended_protocol && have_ran && !target_running ())
{
/* In non-stop, defer exiting until GDB had a chance to query
the whole vStopped list (until it gets an OK). */
if (QUEUE_is_empty (notif_event_p, notif_stop.queue))
{
/* Be transparent when GDB is connected through stdio -- no
need to spam GDB's console. */
if (!remote_connection_is_stdio ())
fprintf (stderr, "GDBserver exiting\n");
remote_close ();
exit (0);
}
}
However, GDB is still busy processing an earlier "no-resumed" event,
and sends a "Hg" packet, which errors out with "Remote connection
closed". IOW, it's not enough to wait for GDB to query the whole
vStopped list, gdbserver needs to wait until the exit event is really
processed.
The fix is to make gdbserver not disconnect until gdb does.
Tested on x86_64 Fedora, native gdbserver, remote + extended-remote +
with and without "maint set target-non-stop on".
gdb/gdbserver/ChangeLog:
2015-10-14 Pedro Alves <palves@redhat.com>
* remote-utils.c (readchar): Don't print "Got EOF" unless
debugging gdbserver.
* server.c (captured_main): Exit gdbserver if gdb disconnects when
in "target remote" mode and there are no processes left to debug.
(process_serial_event): Remove 'have_ran' static local and remove
logic that exits gdbserver in "target remote" mode.
---
gdb/gdbserver/remote-utils.c | 5 ++++-
gdb/gdbserver/server.c | 40 ++++++++++++++--------------------------
2 files changed, 18 insertions(+), 27 deletions(-)
diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
index 77280f7..05e3d63 100644
--- a/gdb/gdbserver/remote-utils.c
+++ b/gdb/gdbserver/remote-utils.c
@@ -882,7 +882,10 @@ readchar (void)
if (readchar_bufcnt <= 0)
{
if (readchar_bufcnt == 0)
- fprintf (stderr, "readchar: Got EOF\n");
+ {
+ if (remote_debug)
+ fprintf (stderr, "readchar: Got EOF\n");
+ }
else
perror ("readchar");
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index d83d608..5a9b471 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -3677,11 +3677,21 @@ captured_main (int argc, char *argv[])
start_event_loop ();
/* If an exit was requested (using the "monitor exit"
- command), terminate now. The only other way to get
- here is for getpkt to fail; close the connection
- and reopen it at the top of the loop. */
+ command), terminate now. */
+ if (exit_requested)
+ throw_quit ("Quit");
+
+ /* The only other way to get here is for getpkt to fail:
+
+ - If --once was specified, we're done.
- if (exit_requested || run_once)
+ - If not in extended-remote mode, and we're no longer
+ debugging anything, simply exit: GDB has disconnected
+ after processing the last process exit.
+
+ - Otherwise, close the connection and reopen it at the
+ top of the loop. */
+ if (run_once || (!extended_protocol && !target_running ()))
throw_quit ("Quit");
fprintf (stderr,
@@ -3846,13 +3856,6 @@ process_serial_event (void)
int packet_len;
int new_packet_len = -1;
- /* Used to decide when gdbserver should exit in
- multi-mode/remote. */
- static int have_ran = 0;
-
- if (!have_ran)
- have_ran = target_running ();
-
disable_async_io ();
response_needed = 0;
@@ -4291,21 +4294,6 @@ process_serial_event (void)
response_needed = 0;
- if (!extended_protocol && have_ran && !target_running ())
- {
- /* In non-stop, defer exiting until GDB had a chance to query
- the whole vStopped list (until it gets an OK). */
- if (QUEUE_is_empty (notif_event_p, notif_stop.queue))
- {
- /* Be transparent when GDB is connected through stdio -- no
- need to spam GDB's console. */
- if (!remote_connection_is_stdio ())
- fprintf (stderr, "GDBserver exiting\n");
- remote_close ();
- exit (0);
- }
- }
-
if (exit_requested)
return -1;
--
1.9.3
^ permalink raw reply [flat|nested] 60+ messages in thread* [PATCH 14/18] Implement TARGET_WAITKIND_NO_RESUMED in the remote protocol
2015-10-14 15:28 [PATCH 00/18] Remote all-stop on top of non-stop Pedro Alves
` (9 preceding siblings ...)
2015-10-14 15:36 ` [PATCH 17/18] gdbserver: don't exit until GDB disconnects Pedro Alves
@ 2015-10-14 15:36 ` Pedro Alves
2015-10-14 16:36 ` Eli Zaretskii
2015-10-19 16:21 ` Yao Qi
2015-10-14 15:36 ` [PATCH 11/18] gdbserver: fix killed-outside.exp Pedro Alves
` (9 subsequent siblings)
20 siblings, 2 replies; 60+ messages in thread
From: Pedro Alves @ 2015-10-14 15:36 UTC (permalink / raw)
To: gdb-patches
Testing with "maint set target-non-stop on" causes regressions in
tests that rely on TARGET_WAITKIND_NO_RESUMED, which isn't modelled on
the RSP. In real all-stop, gdbserver detects the situation and
reporst error to GDB, and so the tests (e.g.,
gdb.threads/no-unwaited-for-left.exp) at fail quickly. But with
"maint set target-non-stop on", GDB instead hangs forever waiting for
a stop reply that never comes, and so the tests take longer to time
out.
This adds a new "N" stop reply packet that maps 1-1 to
TARGET_WAITKIND_NO_RESUMED.
gdb/ChangeLog:
2015-10-14 Pedro Alves <palves@redhat.com>
PR 14618
* NEWS (New remote packets): Mention the N stop reply.
* remote.c (remote_protocol_features): Add "no-resumed" entry.
(remote_query_supported): Report no-resumed+ support.
(remote_parse_stop_reply): Handle 'N'.
(process_stop_reply): Handle TARGET_WAITKIND_NO_RESUMED.
(remote_wait_as): Handle 'N' / TARGET_WAITKIND_NO_RESUMED.
(_initialize_remote): Register "set/show remote
no-resumed-stop-reply" commands.
gdb/doc/ChangeLog:
2015-10-14 Pedro Alves <palves@redhat.com>
PR 14618
* gdb.texinfo (Stop Reply Packets): Document the N stop reply.
(Remote Configuration): Add the "set/show remote
no-resumed-stop-reply" to the available settings table.
(General Query Packets): Document the "no-resumed" qSupported
feature.
gdb/gdbserver/ChangeLog:
2015-10-14 Pedro Alves <palves@redhat.com>
PR 14618
* linux-low.c (linux_wait_1): If the last resumed thread is gone,
report TARGET_WAITKIND_NO_RESUMED.
* remote-utils.c (prepare_resume_reply): Handle
TARGET_WAITKIND_NO_RESUMED.
* server.c (report_no_resumed): New global.
(handle_query) <qSupported>: Handle "no-resumed+". Report
"no-resumed+" support.
(resume): When the target reports TARGET_WAITKIND_NO_RESUMED, only
return error if the client doesn't support no-resumed events.
(push_stop_notification): New function.
(handle_target_event): Use it. Report TARGET_WAITKIND_NO_RESUMED
events if the client supports them.
---
gdb/NEWS | 6 ++++++
gdb/doc/gdb.texinfo | 27 ++++++++++++++++++++++++++
gdb/gdbserver/linux-low.c | 15 ++++++++++++++-
gdb/gdbserver/remote-utils.c | 3 +++
gdb/gdbserver/server.c | 45 +++++++++++++++++++++++++++++++-------------
gdb/remote.c | 23 +++++++++++++++++++---
6 files changed, 102 insertions(+), 17 deletions(-)
diff --git a/gdb/NEWS b/gdb/NEWS
index cb3b2fc..1337650 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -97,6 +97,12 @@ QThreadEvents
would hang forever not knowing that it should no longer expect a
stop for that same thread.
+N stop reply
+
+ Indicates that there are no resumed threads left in the target (all
+ threads are stopped). The remote stub reports support for this stop
+ reply to GDB's qSupported query.
+
* Extended-remote exec events
** GDB now has support for exec events on extended-remote Linux targets.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 1da9e4a..1d7ee8b 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -20236,6 +20236,10 @@ are:
@tab @code{QThreadEvents}
@tab Tracking thread lifetime.
+@item @code{no-resumed-stop-reply}
+@tab @code{no resumed thread left stop reply}
+@tab Tracking thread lifetime.
+
@end multitable
@node Remote Stub
@@ -35585,6 +35589,21 @@ The thread exited, and @var{AA} is the exit status. This response
should not be sent by default; @value{GDBN} requests it with the
@ref{QThreadEvents} packet. See also @ref{thread create event} above.
+@item N
+There are no resumed threads left in the target. In other words, even
+though the process is alive, the last resumed thread has exited. For
+example, say the target process has two threads: thread 1 and thread
+2. The client leaves thread 1 stopped, and resumes thread 2, which
+subsequently exits. At this point, even though the process is still
+alive, and thus no @samp{W} stop reply is sent, no thread is actually
+executing either. The @samp{N} stop reply thus informs the client
+that it can stop waiting for stop replies. This packet should not be
+sent by default; older @value{GDBN} versions did not support it.
+@value{GDBN} requests it, by supplying an appropriate
+@samp{qSupported} feature (@pxref{qSupported}). The remote stub must
+also supply the appropriate @samp{qSupported} feature indicating
+support.
+
@item O @var{XX}@dots{}
@samp{@var{XX}@dots{}} is hex encoding of @sc{ascii} data, to be
written as the program's console output. This can happen at any time
@@ -36484,6 +36503,11 @@ These are the currently defined stub features and their properties:
@tab @samp{-}
@tab No
+@item @samp{no-resumed}
+@tab No
+@tab @samp{-}
+@tab No
+
@end multitable
These are the currently defined stub features, in more detail:
@@ -36699,6 +36723,9 @@ The remote stub reports the supported actions in the reply to
@item QThreadEvents
The remote stub understands the @samp{QThreadEvents} packet.
+@item no-resumed
+The remote stub reports the @samp{N} stop reply.
+
@end table
@item qSymbol::
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index af9ca22..73abe5c 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -2971,6 +2971,7 @@ linux_wait_1 (ptid_t ptid,
int report_to_gdb;
int trace_event;
int in_step_range;
+ int any_resumed;
if (debug_threads)
{
@@ -2988,6 +2989,18 @@ linux_wait_1 (ptid_t ptid,
in_step_range = 0;
ourstatus->kind = TARGET_WAITKIND_IGNORE;
+ /* Find a resumed LWP, if any. */
+ if (find_inferior (&all_threads,
+ status_pending_p_callback,
+ &minus_one_ptid) != NULL)
+ any_resumed = 1;
+ else if ((find_inferior (&all_threads,
+ not_stopped_callback,
+ &minus_one_ptid) != NULL))
+ any_resumed = 1;
+ else
+ any_resumed = 0;
+
if (ptid_equal (step_over_bkpt, null_ptid))
pid = linux_wait_for_event (ptid, &w, options);
else
@@ -2998,7 +3011,7 @@ linux_wait_1 (ptid_t ptid,
pid = linux_wait_for_event (step_over_bkpt, &w, options & ~WNOHANG);
}
- if (pid == 0)
+ if (pid == 0 || (pid == -1 && !any_resumed))
{
gdb_assert (target_options & TARGET_WNOHANG);
diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
index 0d9cde3..77280f7 100644
--- a/gdb/gdbserver/remote-utils.c
+++ b/gdb/gdbserver/remote-utils.c
@@ -1289,6 +1289,9 @@ prepare_resume_reply (char *buf, ptid_t ptid,
buf += strlen (buf);
buf = write_ptid (buf, ptid);
break;
+ case TARGET_WAITKIND_NO_RESUMED:
+ sprintf (buf, "N");
+ break;
default:
error ("unhandled waitkind");
break;
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 2ffc5d0..d83d608 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -61,6 +61,10 @@ int report_fork_events;
int report_vfork_events;
int report_exec_events;
int report_thread_events;
+
+/* Whether to report TARGET_WAITKING_NO_RESUMED events. */
+static int report_no_resumed;
+
int non_stop;
int swbreak_feature;
int hwbreak_feature;
@@ -2182,6 +2186,12 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
}
else if (strcmp (p, "vContSupported+") == 0)
vCont_supported = 1;
+ else if (strcmp (p, "no-resumed+") == 0)
+ {
+ /* GDB supports and wants TARGET_WAITKIND_NO_RESUMED
+ events. */
+ report_no_resumed = 1;
+ }
else
target_process_qsupported (p);
@@ -2295,6 +2305,8 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
strcat (own_buf, ";QThreadEvents+");
+ strcat (own_buf, ";no-resumed+");
+
/* Reinitialize components as needed for the new connection. */
hostio_handle_new_gdb_connection ();
target_handle_new_gdb_connection ();
@@ -2702,10 +2714,11 @@ resume (struct thread_resume *actions, size_t num_actions)
{
last_ptid = mywait (minus_one_ptid, &last_status, 0, 1);
- if (last_status.kind == TARGET_WAITKIND_NO_RESUMED)
+ if (last_status.kind == TARGET_WAITKIND_NO_RESUMED
+ && !report_no_resumed)
{
- /* No proper RSP support for this yet. At least return
- error. */
+ /* The client does not support this stop reply. At least
+ return error. */
sprintf (own_buf, "E.No unwaited-for children left.");
disable_async_io ();
return;
@@ -4318,6 +4331,19 @@ handle_serial_event (int err, gdb_client_data client_data)
return 0;
}
+/* Push a stop notification on the notification queue. */
+
+static void
+push_stop_notification (ptid_t ptid, struct target_waitstatus *status)
+{
+ struct vstop_notif *vstop_notif = XNEW (struct vstop_notif);
+
+ vstop_notif->status = *status;
+ vstop_notif->ptid = ptid;
+ /* Push Stop notification. */
+ notif_push (¬if_stop, (struct notif_event *) vstop_notif);
+}
+
/* Event-loop callback for target events. */
int
@@ -4331,7 +4357,8 @@ handle_target_event (int err, gdb_client_data client_data)
if (last_status.kind == TARGET_WAITKIND_NO_RESUMED)
{
- /* No RSP support for this yet. */
+ if (gdb_connected () && report_no_resumed)
+ push_stop_notification (null_ptid, &last_status);
}
else if (last_status.kind != TARGET_WAITKIND_IGNORE)
{
@@ -4386,15 +4413,7 @@ handle_target_event (int err, gdb_client_data client_data)
target_pid_to_str (last_ptid));
}
else
- {
- struct vstop_notif *vstop_notif = XNEW (struct vstop_notif);
-
- vstop_notif->status = last_status;
- vstop_notif->ptid = last_ptid;
- /* Push Stop notification. */
- notif_push (¬if_stop,
- (struct notif_event *) vstop_notif);
- }
+ push_stop_notification (last_ptid, &last_status);
}
/* Be sure to not change the selected thread behind GDB's back.
diff --git a/gdb/remote.c b/gdb/remote.c
index f4714bc..c7a01d2 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1486,6 +1486,9 @@ enum {
/* Support remote CTRL-C. */
PACKET_vCtrlC,
+ /* Support TARGET_WAITKIND_NO_RESUMED. */
+ PACKET_no_resumed,
+
PACKET_MAX
};
@@ -4501,6 +4504,7 @@ static const struct protocol_feature remote_protocol_features[] = {
PACKET_Qbtrace_conf_pt_size },
{ "vContSupported", PACKET_DISABLE, remote_supported_packet, PACKET_vContSupported },
{ "QThreadEvents", PACKET_DISABLE, remote_supported_packet, PACKET_QThreadEvents },
+ { "no-resumed", PACKET_DISABLE, remote_supported_packet, PACKET_no_resumed },
};
static char *remote_support_xml;
@@ -4599,6 +4603,9 @@ remote_query_supported (void)
if (packet_set_cmd_state (PACKET_QThreadEvents) != AUTO_BOOLEAN_FALSE)
q = remote_query_supported_append (q, "QThreadEvents+");
+ if (packet_set_cmd_state (PACKET_no_resumed) != AUTO_BOOLEAN_FALSE)
+ q = remote_query_supported_append (q, "no-resumed+");
+
q = reconcat (q, "qSupported:", q, (char *) NULL);
putpkt (q);
@@ -6567,6 +6574,10 @@ Packet: '%s'\n"),
event->ptid = pid_to_ptid (pid);
}
break;
+ case 'N':
+ event->ws.kind = TARGET_WAITKIND_NO_RESUMED;
+ event->ptid = minus_one_ptid;
+ break;
}
if (target_is_non_stop_p () && ptid_equal (event->ptid, null_ptid))
@@ -6668,7 +6679,8 @@ process_stop_reply (struct stop_reply *stop_reply,
ptid = inferior_ptid;
if (status->kind != TARGET_WAITKIND_EXITED
- && status->kind != TARGET_WAITKIND_SIGNALLED)
+ && status->kind != TARGET_WAITKIND_SIGNALLED
+ && status->kind != TARGET_WAITKIND_NO_RESUMED)
{
struct remote_state *rs = get_remote_state ();
struct private_thread_info *remote_thr;
@@ -6847,7 +6859,7 @@ remote_wait_as (ptid_t ptid, struct target_waitstatus *status, int options)
remote_fileio_request (buf, rs->ctrlc_pending_p);
rs->ctrlc_pending_p = 0;
break;
- case 'T': case 'S': case 'X': case 'W':
+ case 'N': case 'T': case 'S': case 'X': case 'W':
{
struct stop_reply *stop_reply
= (struct stop_reply *) remote_notif_parse (¬if_client_stop,
@@ -6891,7 +6903,9 @@ remote_wait_as (ptid_t ptid, struct target_waitstatus *status, int options)
break;
}
- if (status->kind == TARGET_WAITKIND_IGNORE)
+ if (status->kind == TARGET_WAITKIND_NO_RESUMED)
+ return minus_one_ptid;
+ else if (status->kind == TARGET_WAITKIND_IGNORE)
{
/* Nothing interesting happened. If we're doing a non-blocking
poll, we're done. Otherwise, go back to waiting. */
@@ -13723,6 +13737,9 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL,
add_packet_config_cmd (&remote_protocol_packets[PACKET_QThreadEvents],
"QThreadEvents", "thread-events", 0);
+ add_packet_config_cmd (&remote_protocol_packets[PACKET_no_resumed],
+ "N stop reply", "no-resumed-stop-reply", 0);
+
/* Assert that we've registered "set remote foo-packet" commands
for all packet configs. */
{
--
1.9.3
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH 14/18] Implement TARGET_WAITKIND_NO_RESUMED in the remote protocol
2015-10-14 15:36 ` [PATCH 14/18] Implement TARGET_WAITKIND_NO_RESUMED in the remote protocol Pedro Alves
@ 2015-10-14 16:36 ` Eli Zaretskii
2015-10-19 16:21 ` Yao Qi
1 sibling, 0 replies; 60+ messages in thread
From: Eli Zaretskii @ 2015-10-14 16:36 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
> From: Pedro Alves <palves@redhat.com>
> Date: Wed, 14 Oct 2015 16:28:02 +0100
>
> gdb/ChangeLog:
> 2015-10-14 Pedro Alves <palves@redhat.com>
>
> PR 14618
> * NEWS (New remote packets): Mention the N stop reply.
> * remote.c (remote_protocol_features): Add "no-resumed" entry.
> (remote_query_supported): Report no-resumed+ support.
> (remote_parse_stop_reply): Handle 'N'.
> (process_stop_reply): Handle TARGET_WAITKIND_NO_RESUMED.
> (remote_wait_as): Handle 'N' / TARGET_WAITKIND_NO_RESUMED.
> (_initialize_remote): Register "set/show remote
> no-resumed-stop-reply" commands.
>
> gdb/doc/ChangeLog:
> 2015-10-14 Pedro Alves <palves@redhat.com>
>
> PR 14618
> * gdb.texinfo (Stop Reply Packets): Document the N stop reply.
> (Remote Configuration): Add the "set/show remote
> no-resumed-stop-reply" to the available settings table.
> (General Query Packets): Document the "no-resumed" qSupported
> feature.
>
> gdb/gdbserver/ChangeLog:
> 2015-10-14 Pedro Alves <palves@redhat.com>
>
> PR 14618
> * linux-low.c (linux_wait_1): If the last resumed thread is gone,
> report TARGET_WAITKIND_NO_RESUMED.
> * remote-utils.c (prepare_resume_reply): Handle
> TARGET_WAITKIND_NO_RESUMED.
> * server.c (report_no_resumed): New global.
> (handle_query) <qSupported>: Handle "no-resumed+". Report
> "no-resumed+" support.
> (resume): When the target reports TARGET_WAITKIND_NO_RESUMED, only
> return error if the client doesn't support no-resumed events.
> (push_stop_notification): New function.
> (handle_target_event): Use it. Report TARGET_WAITKIND_NO_RESUMED
> events if the client supports them.
OK for the documentation parts.
Thanks.
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH 14/18] Implement TARGET_WAITKIND_NO_RESUMED in the remote protocol
2015-10-14 15:36 ` [PATCH 14/18] Implement TARGET_WAITKIND_NO_RESUMED in the remote protocol Pedro Alves
2015-10-14 16:36 ` Eli Zaretskii
@ 2015-10-19 16:21 ` Yao Qi
2015-10-19 16:48 ` Pedro Alves
1 sibling, 1 reply; 60+ messages in thread
From: Yao Qi @ 2015-10-19 16:21 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
Pedro Alves <palves@redhat.com> writes:
> Testing with "maint set target-non-stop on" causes regressions in
> tests that rely on TARGET_WAITKIND_NO_RESUMED, which isn't modelled on
> the RSP. In real all-stop, gdbserver detects the situation and
> reporst error to GDB, and so the tests (e.g.,
> gdb.threads/no-unwaited-for-left.exp) at fail quickly. But with
> "maint set target-non-stop on", GDB instead hangs forever waiting for
> a stop reply that never comes, and so the tests take longer to time
> out.
A quick comment here, we also need to get rid of
setup_kfail "gdb/14618" "*-*-*" in gdb.threads/no-unwaited-for-left.exp,
otherwise, we'll see
KPASS: gdb.threads/no-unwaited-for-left.exp: continue stops when thread 2 exits (PRMS gdb/14618)
KPASS: gdb.threads/no-unwaited-for-left.exp: continue stops when the main thread exits (PRMS gdb/14618)
gdb.threads/no-unwaited-for-left.exp also triggers a SIGSEGV in AArch64
GDBserver, because current_thread is de-referenced, but it is NULL.
Program received signal SIGSEGV, Segmentation fault.
inferior_regcache_data (inferior=inferior@entry=0x0) at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/inferiors.c:259
259 /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/inferiors.c: No such file or directory.
(gdb) bt
#0 inferior_regcache_data (inferior=inferior@entry=0x0) at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/inferiors.c:259
#1 0x0000000000406138 in get_thread_regcache (thread=0x0, fetch=fetch@entry=0) at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/regcache.c:31
#2 0x00000000004204b8 in is_64bit_tdesc () at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/linux-aarch64-low.c:84
#3 0x000000000042128c in aarch64_supports_z_point_type (z_type=<optimized out>) at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/linux-aarch64-low.c:263
#4 0x0000000000424c70 in linux_supports_z_point_type (z_type=<optimized out>) at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/linux-low.c:5634
#5 0x0000000000410fbc in z_type_supported (z_type=48 '0') at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/mem-break.c:918
#6 check_gdb_bp_preconditions (z_type=z_type@entry=48 '0', err=0x40e3c8 <process_serial_event+1032>, err@entry=0x7ffffff1dc)
at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/mem-break.c:1009
#7 0x0000000000412250 in delete_gdb_breakpoint (z_type=z_type@entry=48 '0', addr=4196920, size=0)
at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/mem-break.c:1081
#8 0x000000000040e3c8 in process_serial_event () at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/server.c:4182
#9 0x000000000040f754 in handle_serial_event (err=<optimized out>, client_data=<optimized out>)
at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/server.c:4312
Need to figure out what is the best fix.
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH 14/18] Implement TARGET_WAITKIND_NO_RESUMED in the remote protocol
2015-10-19 16:21 ` Yao Qi
@ 2015-10-19 16:48 ` Pedro Alves
0 siblings, 0 replies; 60+ messages in thread
From: Pedro Alves @ 2015-10-19 16:48 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On 10/19/2015 05:21 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
>
>> Testing with "maint set target-non-stop on" causes regressions in
>> tests that rely on TARGET_WAITKIND_NO_RESUMED, which isn't modelled on
>> the RSP. In real all-stop, gdbserver detects the situation and
>> reporst error to GDB, and so the tests (e.g.,
>> gdb.threads/no-unwaited-for-left.exp) at fail quickly. But with
>> "maint set target-non-stop on", GDB instead hangs forever waiting for
>> a stop reply that never comes, and so the tests take longer to time
>> out.
>
> A quick comment here, we also need to get rid of
> setup_kfail "gdb/14618" "*-*-*" in gdb.threads/no-unwaited-for-left.exp,
> otherwise, we'll see
>
> KPASS: gdb.threads/no-unwaited-for-left.exp: continue stops when thread 2 exits (PRMS gdb/14618)
> KPASS: gdb.threads/no-unwaited-for-left.exp: continue stops when the main thread exits (PRMS gdb/14618)
Indeed, I'll add that.
>
> gdb.threads/no-unwaited-for-left.exp also triggers a SIGSEGV in AArch64
> GDBserver, because current_thread is de-referenced, but it is NULL.
>
> Program received signal SIGSEGV, Segmentation fault.
> inferior_regcache_data (inferior=inferior@entry=0x0) at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/inferiors.c:259
> 259 /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/inferiors.c: No such file or directory.
> (gdb) bt
> #0 inferior_regcache_data (inferior=inferior@entry=0x0) at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/inferiors.c:259
> #1 0x0000000000406138 in get_thread_regcache (thread=0x0, fetch=fetch@entry=0) at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/regcache.c:31
> #2 0x00000000004204b8 in is_64bit_tdesc () at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/linux-aarch64-low.c:84
> #3 0x000000000042128c in aarch64_supports_z_point_type (z_type=<optimized out>) at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/linux-aarch64-low.c:263
> #4 0x0000000000424c70 in linux_supports_z_point_type (z_type=<optimized out>) at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/linux-low.c:5634
> #5 0x0000000000410fbc in z_type_supported (z_type=48 '0') at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/mem-break.c:918
> #6 check_gdb_bp_preconditions (z_type=z_type@entry=48 '0', err=0x40e3c8 <process_serial_event+1032>, err@entry=0x7ffffff1dc)
> at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/mem-break.c:1009
> #7 0x0000000000412250 in delete_gdb_breakpoint (z_type=z_type@entry=48 '0', addr=4196920, size=0)
> at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/mem-break.c:1081
> #8 0x000000000040e3c8 in process_serial_event () at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/server.c:4182
> #9 0x000000000040f754 in handle_serial_event (err=<optimized out>, client_data=<optimized out>)
> at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/server.c:4312
>
> Need to figure out what is the best fix.
Sounds like an issue similar to the one in patch #4 (and also patch #15).
I was thinking that we'll probably need to generalize a bit the fix in patch #4,
by adding a "set_general_process" function to gdbserver based on that
(naming mirrors gdb/remote.c's own set_general_process).
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH 11/18] gdbserver: fix killed-outside.exp
2015-10-14 15:28 [PATCH 00/18] Remote all-stop on top of non-stop Pedro Alves
` (10 preceding siblings ...)
2015-10-14 15:36 ` [PATCH 14/18] Implement TARGET_WAITKIND_NO_RESUMED in the remote protocol Pedro Alves
@ 2015-10-14 15:36 ` Pedro Alves
2015-10-27 12:02 ` Yao Qi
2015-10-14 15:36 ` [PATCH 06/18] New vCtrlC packet, non-stop mode equivalent of \003 Pedro Alves
` (8 subsequent siblings)
20 siblings, 1 reply; 60+ messages in thread
From: Pedro Alves @ 2015-10-14 15:36 UTC (permalink / raw)
To: gdb-patches
killed-outside.exp regresses with "maint set target-non-stop on". The
logs show:
(gdb) continue
Continuing.
infrun: clear_proceed_status_thread (Thread 9028.9028)
infrun: proceed (addr=0xffffffffffffffff, signal=GDB_SIGNAL_DEFAULT)
infrun: proceed: resuming Thread 9028.9028
Sending packet: $Z0,3615a03966,1#4b... Notification received: Stop:X9;process:2344
Packet received: E01
Sending packet: $Z0,3615a13970,1#47...Packet received: E01
Sending packet: $Z0,3615a14891,1#4a...Packet received: E01
infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [Thread 9028.9028] at 0x4005e4
Sending packet: $vCont;c:p2344.2344#1a...Packet received: E.target not running.
Sending packet: $qXfer:threads:read::0,fff#03...Packet received: l<threads>\n</threads>\n
Sending packet: $vStopped#55...Packet received: OK
Unexpected vCont reply in non-stop mode: E.target not running.
(gdb) remote_async_inferior_event_handler
infrun: target_wait (-1.0.0, status) =
infrun: 9028.0.0 [process 9028],
infrun: status->kind = signalled, signal = GDB_SIGNAL_KILL
infrun: TARGET_WAITKIND_SIGNALLED
Program terminated with signal SIGKILL, Killed.
The program no longer exists.
infrun: stop_waiting
infrun: clear_step_over_info
infrun: stop_all_threads
remote_thread_exit_events(1)
Note the "Unexpected vCont reply" error.
I traced it to a problem in status_pending_p_callback. It resumes an
LWP when it shouldn't.
gdb/gdbserver/ChangeLog:
2015-10-14 Pedro Alves <palves@redhat.com>
* linux-low.c (thread_still_has_status_pending_p): Don't check
vCont;t here.
(lwp_resumed): New function.
(status_pending_p_callback): Return early if the LWP is not
supposed to be resumed.
---
gdb/gdbserver/linux-low.c | 27 +++++++++++++++++++++------
1 file changed, 21 insertions(+), 6 deletions(-)
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 1c447c2..af9ca22 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -1535,12 +1535,6 @@ thread_still_has_status_pending_p (struct thread_info *thread)
if (!lp->status_pending_p)
return 0;
- /* If we got a `vCont;t', but we haven't reported a stop yet, do
- report any status pending the LWP may have. */
- if (thread->last_resume_kind == resume_stop
- && thread->last_status.kind != TARGET_WAITKIND_IGNORE)
- return 0;
-
if (thread->last_resume_kind != resume_stop
&& (lp->stop_reason == TARGET_STOPPED_BY_SW_BREAKPOINT
|| lp->stop_reason == TARGET_STOPPED_BY_HW_BREAKPOINT))
@@ -1597,6 +1591,24 @@ thread_still_has_status_pending_p (struct thread_info *thread)
return 1;
}
+/* Returns true if LWP is resumed from the client's perspective. */
+
+static int
+lwp_resumed (struct lwp_info *lwp)
+{
+ struct thread_info *thread = get_lwp_thread (lwp);
+
+ if (thread->last_resume_kind != resume_stop)
+ return 1;
+
+ /* Did we get a `vCont;t', but we haven't reported a stop yet? */
+ if (thread->last_resume_kind == resume_stop
+ && thread->last_status.kind == TARGET_WAITKIND_IGNORE)
+ return 1;
+
+ return 0;
+}
+
/* Return 1 if this lwp has an interesting status pending. */
static int
status_pending_p_callback (struct inferior_list_entry *entry, void *arg)
@@ -1610,6 +1622,9 @@ status_pending_p_callback (struct inferior_list_entry *entry, void *arg)
if (!ptid_match (ptid_of (thread), ptid))
return 0;
+ if (!lwp_resumed (lp))
+ return 0;
+
if (lp->status_pending_p
&& !thread_still_has_status_pending_p (thread))
{
--
1.9.3
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH 11/18] gdbserver: fix killed-outside.exp
2015-10-14 15:36 ` [PATCH 11/18] gdbserver: fix killed-outside.exp Pedro Alves
@ 2015-10-27 12:02 ` Yao Qi
2015-11-25 15:06 ` Pedro Alves
0 siblings, 1 reply; 60+ messages in thread
From: Yao Qi @ 2015-10-27 12:02 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
Pedro Alves <palves@redhat.com> writes:
> diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
> index 1c447c2..af9ca22 100644
> --- a/gdb/gdbserver/linux-low.c
> +++ b/gdb/gdbserver/linux-low.c
> @@ -1535,12 +1535,6 @@ thread_still_has_status_pending_p (struct thread_info *thread)
> if (!lp->status_pending_p)
> return 0;
>
> - /* If we got a `vCont;t', but we haven't reported a stop yet, do
> - report any status pending the LWP may have. */
> - if (thread->last_resume_kind == resume_stop
> - && thread->last_status.kind != TARGET_WAITKIND_IGNORE)
> - return 0;
> -
Shouldn't we return 1 instead of 0 here? This is consistent with the
comments above.
> if (thread->last_resume_kind != resume_stop
> && (lp->stop_reason == TARGET_STOPPED_BY_SW_BREAKPOINT
> || lp->stop_reason == TARGET_STOPPED_BY_HW_BREAKPOINT))
> @@ -1597,6 +1591,24 @@ thread_still_has_status_pending_p (struct thread_info *thread)
> return 1;
> }
>
> +/* Returns true if LWP is resumed from the client's perspective. */
> +
> +static int
> +lwp_resumed (struct lwp_info *lwp)
> +{
> + struct thread_info *thread = get_lwp_thread (lwp);
> +
> + if (thread->last_resume_kind != resume_stop)
> + return 1;
> +
> + /* Did we get a `vCont;t', but we haven't reported a stop yet? */
> + if (thread->last_resume_kind == resume_stop
> + && thread->last_status.kind == TARGET_WAITKIND_IGNORE)
> + return 1;
> +
I feel "reported" isn't precise. Is "got" better? The code means
GDBserver has already got vCont;t and sent SIGSTOP, but hasn't *got*
stop event out of event loop. After GDBserver got this event, it then
reports the event back to GDB if needed.
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH 11/18] gdbserver: fix killed-outside.exp
2015-10-27 12:02 ` Yao Qi
@ 2015-11-25 15:06 ` Pedro Alves
2015-11-26 16:51 ` Yao Qi
0 siblings, 1 reply; 60+ messages in thread
From: Pedro Alves @ 2015-11-25 15:06 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
Hi Yao,
It's taking me a bit to go through your comments.
Thanks for the review, and sorry about that.
On 10/27/2015 09:48 AM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
>
>> diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
>> index 1c447c2..af9ca22 100644
>> --- a/gdb/gdbserver/linux-low.c
>> +++ b/gdb/gdbserver/linux-low.c
>> @@ -1535,12 +1535,6 @@ thread_still_has_status_pending_p (struct thread_info *thread)
>> if (!lp->status_pending_p)
>> return 0;
>>
>> - /* If we got a `vCont;t', but we haven't reported a stop yet, do
>> - report any status pending the LWP may have. */
>> - if (thread->last_resume_kind == resume_stop
>> - && thread->last_status.kind != TARGET_WAITKIND_IGNORE)
>> - return 0;
>> -
>
> Shouldn't we return 1 instead of 0 here? This is consistent with the
> comments above.
I may have misunderstood what you mean, but if we remove this code,
that's what we get -- the "return 1;" at the end is reached.
>
>> if (thread->last_resume_kind != resume_stop
>> && (lp->stop_reason == TARGET_STOPPED_BY_SW_BREAKPOINT
>> || lp->stop_reason == TARGET_STOPPED_BY_HW_BREAKPOINT))
>> @@ -1597,6 +1591,24 @@ thread_still_has_status_pending_p (struct thread_info *thread)
>> return 1;
>> }
>>
>> +/* Returns true if LWP is resumed from the client's perspective. */
>> +
>> +static int
>> +lwp_resumed (struct lwp_info *lwp)
>> +{
>> + struct thread_info *thread = get_lwp_thread (lwp);
>> +
>> + if (thread->last_resume_kind != resume_stop)
>> + return 1;
>> +
>> + /* Did we get a `vCont;t', but we haven't reported a stop yet? */
>> + if (thread->last_resume_kind == resume_stop
>> + && thread->last_status.kind == TARGET_WAITKIND_IGNORE)
>> + return 1;
>> +
>
> I feel "reported" isn't precise. Is "got" better? The code means
> GDBserver has already got vCont;t and sent SIGSTOP, but hasn't *got*
> stop event out of event loop. After GDBserver got this event, it then
> reports the event back to GDB if needed.
Hmm, reported to gdb is really what I mean. How about this:
/* Did gdb send us a `vCont;t', but we haven't reported the
corresponding stop to gdb yet? If so, the thread is still
resumed/running from gdb's perspective. */
if (thread->last_resume_kind == resume_stop
&& thread->last_status.kind == TARGET_WAITKIND_IGNORE)
return 1;
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH 11/18] gdbserver: fix killed-outside.exp
2015-11-25 15:06 ` Pedro Alves
@ 2015-11-26 16:51 ` Yao Qi
2015-11-26 17:56 ` Pedro Alves
0 siblings, 1 reply; 60+ messages in thread
From: Yao Qi @ 2015-11-26 16:51 UTC (permalink / raw)
To: Pedro Alves; +Cc: Yao Qi, gdb-patches
Pedro Alves <palves@redhat.com> writes:
>>> - /* If we got a `vCont;t', but we haven't reported a stop yet, do
>>> - report any status pending the LWP may have. */
>>> - if (thread->last_resume_kind == resume_stop
>>> - && thread->last_status.kind != TARGET_WAITKIND_IGNORE)
>>> - return 0;
>>> -
>>
>> Shouldn't we return 1 instead of 0 here? This is consistent with the
>> comments above.
>
> I may have misunderstood what you mean, but if we remove this code,
> that's what we get -- the "return 1;" at the end is reached.
>
What I meant is to change "return 0" to "return 1", like,
/* If we got a `vCont;t', but we haven't reported a stop yet, do
report any status pending the LWP may have. */
if (thread->last_resume_kind == resume_stop
&& thread->last_status.kind != TARGET_WAITKIND_IGNORE)
- return 0;
+ return 1;
that is the same as removing them.
>>
>>> if (thread->last_resume_kind != resume_stop
>>> && (lp->stop_reason == TARGET_STOPPED_BY_SW_BREAKPOINT
>>> || lp->stop_reason == TARGET_STOPPED_BY_HW_BREAKPOINT))
>>> @@ -1597,6 +1591,24 @@ thread_still_has_status_pending_p (struct thread_info *thread)
>>> return 1;
>>> }
>>>
>>> +/* Returns true if LWP is resumed from the client's perspective. */
>>> +
>>> +static int
>>> +lwp_resumed (struct lwp_info *lwp)
>>> +{
>>> + struct thread_info *thread = get_lwp_thread (lwp);
>>> +
>>> + if (thread->last_resume_kind != resume_stop)
>>> + return 1;
>>> +
>>> + /* Did we get a `vCont;t', but we haven't reported a stop yet? */
>>> + if (thread->last_resume_kind == resume_stop
>>> + && thread->last_status.kind == TARGET_WAITKIND_IGNORE)
>>> + return 1;
>>> +
>>
>> I feel "reported" isn't precise. Is "got" better? The code means
>> GDBserver has already got vCont;t and sent SIGSTOP, but hasn't *got*
>> stop event out of event loop. After GDBserver got this event, it then
>> reports the event back to GDB if needed.
>
> Hmm, reported to gdb is really what I mean. How about this:
>
> /* Did gdb send us a `vCont;t', but we haven't reported the
> corresponding stop to gdb yet? If so, the thread is still
> resumed/running from gdb's perspective. */
> if (thread->last_resume_kind == resume_stop
> && thread->last_status.kind == TARGET_WAITKIND_IGNORE)
> return 1;
Oh, sounds it is about thread's state from GDB's perspective. Patch is
good to me.
By the way, after GDBserver reported the stop GDB, does the thread
changed to
(thread->last_resume_kind == resume_stop
&& thread->last_status.kind != TARGET_WAITKIND_IGNORE)
it is used in proceed_one_lwp.
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH 11/18] gdbserver: fix killed-outside.exp
2015-11-26 16:51 ` Yao Qi
@ 2015-11-26 17:56 ` Pedro Alves
0 siblings, 0 replies; 60+ messages in thread
From: Pedro Alves @ 2015-11-26 17:56 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On 11/26/2015 04:51 PM, Yao Qi wrote:
> By the way, after GDBserver reported the stop GDB, does the thread
> changed to
>
> (thread->last_resume_kind == resume_stop
> && thread->last_status.kind != TARGET_WAITKIND_IGNORE)
>
> it is used in proceed_one_lwp.
>
Yes, after the stop is reported out of linux_wait, server.c does:
else
{
/* We're reporting this thread as stopped. Update its
"want-stopped" state to what the client wants, until it
gets a new resume action. */
current_thread->last_resume_kind = resume_stop;
current_thread->last_status = last_status;
}
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH 06/18] New vCtrlC packet, non-stop mode equivalent of \003
2015-10-14 15:28 [PATCH 00/18] Remote all-stop on top of non-stop Pedro Alves
` (11 preceding siblings ...)
2015-10-14 15:36 ` [PATCH 11/18] gdbserver: fix killed-outside.exp Pedro Alves
@ 2015-10-14 15:36 ` Pedro Alves
2015-10-26 14:11 ` Yao Qi
2015-10-14 15:36 ` [PATCH 04/18] gdbserver crash running gdb.threads/non-ldr-exc-1.exp Pedro Alves
` (7 subsequent siblings)
20 siblings, 1 reply; 60+ messages in thread
From: Pedro Alves @ 2015-10-14 15:36 UTC (permalink / raw)
To: gdb-patches
There's currently no non-stop equivalent of the all-stop ^C (\003)
"packet" that GDB sends when a ctrl-c is pressed while a foreground
command is active. There's vCont;t, but that's defined to cause a
"signal 0" stop.
This fixes many tests that type ^C, when testing with extended-remote
with "maint set target-non-stop on". E.g.:
Continuing.
talk to me baby
PASS: gdb.base/interrupt.exp: process is alive
a
a
PASS: gdb.base/interrupt.exp: child process ate our char
^C
[Thread 22730.22730] #1 stopped.
0x0000003615ee6650 in __read_nocancel () at ../sysdeps/unix/syscall-template.S:81
81 T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS)
(gdb) FAIL: gdb.base/interrupt.exp: send_gdb control C
p func1 ()
gdb/
2015-10-14 Pedro Alves <palves@redhat.com>
* NEWS (New remote packets): Mention vCtrlC.
gdb/doc/
2015-10-14 Pedro Alves <palves@redhat.com>
* gdb.texinfo (Bootstrapping): Add
"interrupting remote targets" anchor.
(Packets): Document vCtrlC.
gdb/gdbserver/
2015-10-14 Pedro Alves <palves@redhat.com>
* server.c (handle_v_requests): Handle vCtrlC.
* remote.c (PACKET_vCtrlC): New enum value.
(async_remote_interrupt): Call target_interrupt instead of
target_stop.
(remote_interrupt_as): Remove 'ptid' parameter.
(remote_interrupt_ns): New function.
(remote_stop): Adjust.
(remote_interrupt): If the target is in non-stop mode, try
interrupting with vCtrlC.
(initialize_remote): Install set remote ctrl-c packet.
---
gdb/NEWS | 4 ++++
gdb/doc/gdb.texinfo | 34 +++++++++++++++++++++++----
gdb/gdbserver/server.c | 7 ++++++
gdb/remote.c | 64 ++++++++++++++++++++++++++++++++++++++++++++------
4 files changed, 98 insertions(+), 11 deletions(-)
diff --git a/gdb/NEWS b/gdb/NEWS
index 2e38d9a..84c75cc 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -75,6 +75,10 @@ exec-events feature in qSupported
response can contain the corresponding 'stubfeature'. Set and
show commands can be used to display whether these features are enabled.
+vCtrlC
+ Equivalent to interrupting with the ^C character, but works in
+ non-stop mode.
+
* Extended-remote exec events
** GDB now has support for exec events on extended-remote Linux targets.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index f298172..196a842 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -35087,6 +35087,24 @@ command in the @samp{vCont} packet.
The @samp{vCont} packet is not supported.
@end table
+@anchor{vCtrlC packet}
+@item vCtrlC
+@cindex @samp{vCtrlC} packet
+Interrupt remote target as if a control-C was pressed on the remote
+terminal. This is the equivalent to reacting to the @code{^C}
+(@samp{\003}, the control-C character) character in all-stop mode
+while the target is running, except this works in non-stop mode.
+@xref{interrupting remote targets}, for more info on the all-stop
+variant.
+
+Reply:
+@table @samp
+@item E @var{nn}
+for an error
+@item OK
+for success
+@end table
+
@item vFile:@var{operation}:@var{parameter}@dots{}
@cindex @samp{vFile} packet
Perform a file operation on the target system. For details,
@@ -37854,11 +37872,12 @@ operation.
@node Interrupts
@section Interrupts
@cindex interrupts (remote protocol)
+@anchor{interrupting remote targets}
-When a program on the remote target is running, @value{GDBN} may
-attempt to interrupt it by sending a @samp{Ctrl-C}, @code{BREAK} or
-a @code{BREAK} followed by @code{g},
-control of which is specified via @value{GDBN}'s @samp{interrupt-sequence}.
+In all-stop mode, when a program on the remote target is running,
+@value{GDBN} may attempt to interrupt it by sending a @samp{Ctrl-C},
+@code{BREAK} or a @code{BREAK} followed by @code{g}, control of which
+is specified via @value{GDBN}'s @samp{interrupt-sequence}.
The precise meaning of @code{BREAK} is defined by the transport
mechanism and may, in fact, be undefined. @value{GDBN} does not
@@ -37879,6 +37898,13 @@ and does @emph{not} represent an interrupt. E.g., an @samp{X} packet
When Linux kernel receives this sequence from serial port,
it stops execution and connects to gdb.
+In non-stop mode, because packet resumptions are asynchronous
+(@pxref{vCont packet}), @value{GDBN} is always free to send a remote
+command to the remote stub, even when the target is running. For that
+reason, @value{GDBN} instead sends a regular packet (@pxref{vCtrlC
+packet}) with the usual packet framing instead of the single byte
+@code{0x03}.
+
Stubs are not required to recognize these interrupt mechanisms and the
precise meaning associated with receipt of the interrupt is
implementation defined. If the target supports debugging of multiple
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index ec52f84..e0ce524 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -2849,6 +2849,13 @@ handle_v_requests (char *own_buf, int packet_len, int *new_packet_len)
{
if (!disable_packet_vCont)
{
+ if (strcmp (own_buf, "vCtrlC") == 0)
+ {
+ (*the_target->request_interrupt) ();
+ write_ok (own_buf);
+ return;
+ }
+
if (startswith (own_buf, "vCont;"))
{
require_running (own_buf);
diff --git a/gdb/remote.c b/gdb/remote.c
index 10c65e2..9a23ca6 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1478,6 +1478,9 @@ enum {
/* Support for query supported vCont actions. */
PACKET_vContSupported,
+ /* Support remote CTRL-C. */
+ PACKET_vCtrlC,
+
PACKET_MAX
};
@@ -5553,7 +5556,7 @@ async_remote_interrupt (gdb_client_data arg)
if (remote_debug)
fprintf_unfiltered (gdb_stdlog, "async_remote_interrupt called\n");
- target_stop (inferior_ptid);
+ target_interrupt (inferior_ptid);
}
/* Perform interrupt, if the first attempt did not succeed. Just give
@@ -5660,7 +5663,7 @@ remote_stop_ns (ptid_t ptid)
process reports the interrupt. */
static void
-remote_interrupt_as (ptid_t ptid)
+remote_interrupt_as (void)
{
struct remote_state *rs = get_remote_state ();
@@ -5676,6 +5679,37 @@ remote_interrupt_as (ptid_t ptid)
send_interrupt_sequence ();
}
+/* Non-stop version of target_interrupt. Uses `vCtrlC' to interrupt
+ the remote target. It is undefined which thread of which process
+ reports the interrupt. */
+
+static int
+remote_interrupt_ns (void)
+{
+ struct remote_state *rs = get_remote_state ();
+ char *p = rs->buf;
+ char *endp = rs->buf + get_remote_packet_size ();
+
+ xsnprintf (p, endp - p, "vCtrlC");
+
+ /* In non-stop, we get an immediate OK reply. The stop reply will
+ come in asynchronously by notification. */
+ putpkt (rs->buf);
+ getpkt (&rs->buf, &rs->buf_size, 0);
+
+ switch (packet_ok (rs->buf, &remote_protocol_packets[PACKET_vCtrlC]))
+ {
+ case PACKET_OK:
+ break;
+ case PACKET_UNKNOWN:
+ return 0;
+ case PACKET_ERROR:
+ error (_("Interrupting target failed: %s"), rs->buf);
+ }
+
+ return 1;
+}
+
/* Implement the to_stop function for the remote targets. */
static void
@@ -5690,7 +5724,7 @@ remote_stop (struct target_ops *self, ptid_t ptid)
{
/* We don't currently have a way to transparently pause the
remote target in all-stop mode. Interrupt it instead. */
- remote_interrupt_as (ptid);
+ remote_interrupt_as ();
}
}
@@ -5702,14 +5736,27 @@ remote_interrupt (struct target_ops *self, ptid_t ptid)
if (remote_debug)
fprintf_unfiltered (gdb_stdlog, "remote_interrupt called\n");
- if (target_is_non_stop_p ())
+ if (non_stop)
{
- /* We don't currently have a way to ^C the remote target in
- non-stop mode. Stop it (with no signal) instead. */
+ /* In non-stop mode, we always stop with no signal instead. */
remote_stop_ns (ptid);
}
else
- remote_interrupt_as (ptid);
+ {
+ /* In all-stop, we emulate ^C-ing the remote target's
+ terminal. */
+ if (target_is_non_stop_p ())
+ {
+ if (!remote_interrupt_ns ())
+ {
+ /* No support for ^C-ing the remote target. Stop it
+ (with no signal) instead. */
+ remote_stop_ns (ptid);
+ }
+ }
+ else
+ remote_interrupt_as ();
+ }
}
/* Ask the user what to do when an interrupt is received. */
@@ -13612,6 +13659,9 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL,
add_packet_config_cmd (&remote_protocol_packets[PACKET_exec_event_feature],
"exec-event-feature", "exec-event-feature", 0);
+ add_packet_config_cmd (&remote_protocol_packets[PACKET_vCtrlC],
+ "vCtrlC", "ctrl-c", 0);
+
/* Assert that we've registered "set remote foo-packet" commands
for all packet configs. */
{
--
1.9.3
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH 06/18] New vCtrlC packet, non-stop mode equivalent of \003
2015-10-14 15:36 ` [PATCH 06/18] New vCtrlC packet, non-stop mode equivalent of \003 Pedro Alves
@ 2015-10-26 14:11 ` Yao Qi
2015-11-30 18:25 ` Pedro Alves
0 siblings, 1 reply; 60+ messages in thread
From: Yao Qi @ 2015-10-26 14:11 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
Pedro Alves <palves@redhat.com> writes:
> gdb/
> 2015-10-14 Pedro Alves <palves@redhat.com>
>
> * NEWS (New remote packets): Mention vCtrlC.
>
> gdb/doc/
> 2015-10-14 Pedro Alves <palves@redhat.com>
>
> * gdb.texinfo (Bootstrapping): Add
> "interrupting remote targets" anchor.
> (Packets): Document vCtrlC.
>
> gdb/gdbserver/
> 2015-10-14 Pedro Alves <palves@redhat.com>
>
> * server.c (handle_v_requests): Handle vCtrlC.
These entries below should be hoisted above to gdb/ChangeLog,
> * remote.c (PACKET_vCtrlC): New enum value.
> (async_remote_interrupt): Call target_interrupt instead of
> target_stop.
> (remote_interrupt_as): Remove 'ptid' parameter.
> (remote_interrupt_ns): New function.
> (remote_stop): Adjust.
> (remote_interrupt): If the target is in non-stop mode, try
> interrupting with vCtrlC.
> (initialize_remote): Install set remote ctrl-c packet.
> ---
> gdb/NEWS | 4 ++++
> gdb/doc/gdb.texinfo | 34 +++++++++++++++++++++++----
> gdb/gdbserver/server.c | 7 ++++++
> gdb/remote.c | 64 ++++++++++++++++++++++++++++++++++++++++++++------
> 4 files changed, 98 insertions(+), 11 deletions(-)
>
>
> +/* Non-stop version of target_interrupt. Uses `vCtrlC' to interrupt
> + the remote target. It is undefined which thread of which process
> + reports the interrupt. */
> +
We need to document the return value of this function in comments.
> +static int
> +remote_interrupt_ns (void)
> +{
> + struct remote_state *rs = get_remote_state ();
> + char *p = rs->buf;
> + char *endp = rs->buf + get_remote_packet_size ();
> +
> + xsnprintf (p, endp - p, "vCtrlC");
> +
> + /* In non-stop, we get an immediate OK reply. The stop reply will
> + come in asynchronously by notification. */
> + putpkt (rs->buf);
> + getpkt (&rs->buf, &rs->buf_size, 0);
> +
> + switch (packet_ok (rs->buf, &remote_protocol_packets[PACKET_vCtrlC]))
> + {
> + case PACKET_OK:
> + break;
> + case PACKET_UNKNOWN:
> + return 0;
> + case PACKET_ERROR:
> + error (_("Interrupting target failed: %s"), rs->buf);
> + }
> +
> + return 1;
> +}
Patch looks good to me otherwise.
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH 06/18] New vCtrlC packet, non-stop mode equivalent of \003
2015-10-26 14:11 ` Yao Qi
@ 2015-11-30 18:25 ` Pedro Alves
0 siblings, 0 replies; 60+ messages in thread
From: Pedro Alves @ 2015-11-30 18:25 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
I see now that I never responded to this one...
On 10/26/2015 11:44 AM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
>
>> gdb/
>> 2015-10-14 Pedro Alves <palves@redhat.com>
>>
>> * NEWS (New remote packets): Mention vCtrlC.
>>
>> gdb/doc/
>> 2015-10-14 Pedro Alves <palves@redhat.com>
>>
>> * gdb.texinfo (Bootstrapping): Add
>> "interrupting remote targets" anchor.
>> (Packets): Document vCtrlC.
>>
>> gdb/gdbserver/
>> 2015-10-14 Pedro Alves <palves@redhat.com>
>>
>> * server.c (handle_v_requests): Handle vCtrlC.
>
> These entries below should be hoisted above to gdb/ChangeLog,
Whoops, indeed.
>
>> * remote.c (PACKET_vCtrlC): New enum value.
>> (async_remote_interrupt): Call target_interrupt instead of
>> target_stop.
>> (remote_interrupt_as): Remove 'ptid' parameter.
>> (remote_interrupt_ns): New function.
>> (remote_stop): Adjust.
>> (remote_interrupt): If the target is in non-stop mode, try
>> interrupting with vCtrlC.
>> (initialize_remote): Install set remote ctrl-c packet.
>> ---
>> gdb/NEWS | 4 ++++
>> gdb/doc/gdb.texinfo | 34 +++++++++++++++++++++++----
>> gdb/gdbserver/server.c | 7 ++++++
>> gdb/remote.c | 64 ++++++++++++++++++++++++++++++++++++++++++++------
>> 4 files changed, 98 insertions(+), 11 deletions(-)
>>
>
>>
>> +/* Non-stop version of target_interrupt. Uses `vCtrlC' to interrupt
>> + the remote target. It is undefined which thread of which process
>> + reports the interrupt. */
>> +
>
> We need to document the return value of this function in comments.
I've added this:
diff --git i/gdb/remote.c w/gdb/remote.c
index ae8c708..7256c23 100644
--- i/gdb/remote.c
+++ w/gdb/remote.c
@@ -5709,7 +5709,8 @@ remote_interrupt_as (void)
/* Non-stop version of target_interrupt. Uses `vCtrlC' to interrupt
the remote target. It is undefined which thread of which process
- reports the interrupt. */
+ reports the interrupt. Returns true if the packet is supported by
+ the server, false otherwise. */
static int
remote_interrupt_ns (void)
> Patch looks good to me otherwise.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH 04/18] gdbserver crash running gdb.threads/non-ldr-exc-1.exp
2015-10-14 15:28 [PATCH 00/18] Remote all-stop on top of non-stop Pedro Alves
` (12 preceding siblings ...)
2015-10-14 15:36 ` [PATCH 06/18] New vCtrlC packet, non-stop mode equivalent of \003 Pedro Alves
@ 2015-10-14 15:36 ` Pedro Alves
2015-10-26 13:54 ` Yao Qi
2015-10-14 15:37 ` [PATCH 07/18] gdbserver crash if gdb attaches too fast Pedro Alves
` (6 subsequent siblings)
20 siblings, 1 reply; 60+ messages in thread
From: Pedro Alves @ 2015-10-14 15:36 UTC (permalink / raw)
To: gdb-patches
This fixes a gdbserver crash when running
gdb.threads/non-ldr-exc-1.exp with "maint set target-non-stop on".
The problem is that qSymbol is called when gdbserver has
current_thread == NULL.
gdb/gdbserver/ChangeLog:
2015-10-14 Pedro Alves <palves@redhat.com>
* gdbthread.h (find_any_thread_of_pid): Declare.
* inferiors.c (thread_of_pid, find_any_thread_of_pid): New
functions.
* server.c (handle_query): If current_thread is NULL, look for
another thread of the selected process.
---
gdb/gdbserver/gdbthread.h | 4 ++++
gdb/gdbserver/inferiors.c | 23 +++++++++++++++++++++++
gdb/gdbserver/server.c | 23 +++++++++++++++++++++++
3 files changed, 50 insertions(+)
diff --git a/gdb/gdbserver/gdbthread.h b/gdb/gdbserver/gdbthread.h
index d6959f4..0510419 100644
--- a/gdb/gdbserver/gdbthread.h
+++ b/gdb/gdbserver/gdbthread.h
@@ -80,6 +80,10 @@ struct thread_info *get_first_thread (void);
struct thread_info *find_thread_ptid (ptid_t ptid);
+/* Find any thread of the PID process. Returns NULL if none is
+ found. */
+struct thread_info *find_any_thread_of_pid (int pid);
+
/* Get current thread ID (Linux task ID). */
#define current_ptid (current_thread->entry.id)
diff --git a/gdb/gdbserver/inferiors.c b/gdb/gdbserver/inferiors.c
index 21f45fa..84f5b7c 100644
--- a/gdb/gdbserver/inferiors.c
+++ b/gdb/gdbserver/inferiors.c
@@ -141,6 +141,29 @@ find_thread_ptid (ptid_t ptid)
return (struct thread_info *) find_inferior_id (&all_threads, ptid);
}
+/* Helper for find_any_thread_of_pid. Returns true if a thread
+ matches a PID. */
+
+static int
+thread_of_pid (struct inferior_list_entry *entry, void *pid_p)
+{
+ int pid = *(int *) pid_p;
+
+ return (ptid_get_pid (entry->id) == pid);
+}
+
+/* See gdbthread.h. */
+
+struct thread_info *
+find_any_thread_of_pid (int pid)
+{
+ struct inferior_list_entry *entry;
+
+ entry = find_inferior (&all_threads, thread_of_pid, &pid);
+
+ return (struct thread_info *) entry;
+}
+
ptid_t
gdb_id_to_thread_id (ptid_t gdb_id)
{
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index e25b7c7..ec52f84 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -1971,6 +1971,27 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
if (strcmp ("qSymbol::", own_buf) == 0)
{
+ struct thread_info *save_thread = current_thread;
+
+ /* For qSymbol, GDB only changes the current thread if the
+ previous current thread was of a different process. So if
+ the previous thread is gone, we need to pick another one of
+ the same process. This can happen e.g., if we followed an
+ exec in a non-leader thread. */
+ if (current_thread == NULL)
+ {
+ current_thread = find_any_thread_of_pid (ptid_get_pid (general_thread));
+
+ /* Just in case, if we didn't find a thread, then bail out
+ instead of crashing. */
+ if (current_thread == NULL)
+ {
+ write_enn (own_buf);
+ current_thread = save_thread;
+ return;
+ }
+ }
+
/* GDB is suggesting new symbols have been loaded. This may
mean a new shared library has been detected as loaded, so
take the opportunity to check if breakpoints we think are
@@ -1989,6 +2010,8 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
if (current_thread != NULL && the_target->look_up_symbols != NULL)
(*the_target->look_up_symbols) ();
+ current_thread = save_thread;
+
strcpy (own_buf, "OK");
return;
}
--
1.9.3
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH 04/18] gdbserver crash running gdb.threads/non-ldr-exc-1.exp
2015-10-14 15:36 ` [PATCH 04/18] gdbserver crash running gdb.threads/non-ldr-exc-1.exp Pedro Alves
@ 2015-10-26 13:54 ` Yao Qi
2015-11-24 16:34 ` Pedro Alves
0 siblings, 1 reply; 60+ messages in thread
From: Yao Qi @ 2015-10-26 13:54 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
Pedro Alves <palves@redhat.com> writes:
> diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
> index e25b7c7..ec52f84 100644
> --- a/gdb/gdbserver/server.c
> +++ b/gdb/gdbserver/server.c
> @@ -1971,6 +1971,27 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
>
> if (strcmp ("qSymbol::", own_buf) == 0)
> {
> + struct thread_info *save_thread = current_thread;
> +
> + /* For qSymbol, GDB only changes the current thread if the
> + previous current thread was of a different process. So if
> + the previous thread is gone, we need to pick another one of
> + the same process. This can happen e.g., if we followed an
> + exec in a non-leader thread. */
> + if (current_thread == NULL)
> + {
> + current_thread = find_any_thread_of_pid (ptid_get_pid (general_thread));
> +
Nit, this line is too long. Patch looks good to me, otherwise.
I do something similar in AArch64 GDBserver backend to fix the crash.
Could you include this patch in your series if it is OK to you? My
patch depends on your patch 04/18.
Note that I didn't add "set_general_process" as you suggested, because I
am not 100% sure the rules of switching current_thread.
--
Yao (齐尧)
From 58ad7851575c5b9c1a2343e6de3dfb4b7b51a6c6 Mon Sep 17 00:00:00 2001
From: Yao Qi <yao.qi@linaro.org>
Date: Fri, 23 Oct 2015 14:00:50 +0100
Subject: [PATCH] [aarch64] gdbserver crash running
gdb.threads/no-unwaited-for-left.exp
This fixes an aarch64 GDBserver crash when running
gdb.threads/no-unwaited-for-left.exp. The problem is that current_thread
is NULL when GDBserver checks whether is 64-bit or not. The fix is
straightforward, if current_thread is NULL, find other thread in the
same process. If none is found, assume the thread is 64-bit.
gdb/gdbserver:
2015-10-26 Yao Qi <yao.qi@linaro.org>
* linux-aarch64-low.c (is_64bit_tdesc): If current_thread is NULL,
select another thread of the same process, otherwise, select
current_thread.
diff --git a/gdb/gdbserver/linux-aarch64-low.c b/gdb/gdbserver/linux-aarch64-low.c
index cb49a04..54d8891 100644
--- a/gdb/gdbserver/linux-aarch64-low.c
+++ b/gdb/gdbserver/linux-aarch64-low.c
@@ -81,7 +81,24 @@ struct arch_process_info
static int
is_64bit_tdesc (void)
{
- struct regcache *regcache = get_thread_regcache (current_thread, 0);
+ struct thread_info *thread;
+ struct regcache *regcache;
+
+ /* If the current thread is gone, pick another one of the same
+ process. */
+ if (current_thread == NULL)
+ thread = find_any_thread_of_pid (ptid_get_pid (general_thread));
+ else
+ thread = current_thread;
+
+ if (thread == NULL)
+ {
+ /* If we didn't find a thread, assume the inferior will be an
+ aarch64 process. */
+ return 1;
+ }
+
+ regcache = get_thread_regcache (thread, 0);
return register_size (regcache->tdesc, 0) == 8;
}
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH 04/18] gdbserver crash running gdb.threads/non-ldr-exc-1.exp
2015-10-26 13:54 ` Yao Qi
@ 2015-11-24 16:34 ` Pedro Alves
2015-11-26 16:23 ` Yao Qi
0 siblings, 1 reply; 60+ messages in thread
From: Pedro Alves @ 2015-11-24 16:34 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On 10/26/2015 10:55 AM, Yao Qi wrote:
> I do something similar in AArch64 GDBserver backend to fix the crash.
> Could you include this patch in your series if it is OK to you? My
> patch depends on your patch 04/18.
> Note that I didn't add "set_general_process" as you suggested, because I
> am not 100% sure the rules of switching current_thread.
>
Hmm, using one of the new Aarch64 machines on the GCC compile farm, I see
that the crash comes from here:
(gdb) bt
#0 0x0000000000408074 in inferior_regcache_data (inferior=0x0) at ../../../src/gdb/gdbserver/inferiors.c:281
#1 0x000000000040840c in get_thread_regcache (thread=0x0, fetch=0) at ../../../src/gdb/gdbserver/regcache.c:31
#2 0x000000000042fb14 in is_64bit_tdesc () at ../../../src/gdb/gdbserver/linux-aarch64-low.c:84
#3 0x0000000000430098 in aarch64_supports_z_point_type (z_type=48 '0') at ../../../src/gdb/gdbserver/linux-aarch64-low.c:264
#4 0x00000000004422cc in linux_supports_z_point_type (z_type=48 '0') at ../../../src/gdb/gdbserver/linux-low.c:5629
#5 0x000000000041a0e8 in z_type_supported (z_type=48 '0') at ../../../src/gdb/gdbserver/mem-break.c:930
#6 0x000000000041a234 in check_gdb_bp_preconditions (z_type=48 '0', err=0x7fc1dcccb8) at ../../../src/gdb/gdbserver/mem-break.c:1021
#7 0x000000000041a380 in delete_gdb_breakpoint (z_type=48 '0', addr=4196520, kind=4) at ../../../src/gdb/gdbserver/mem-break.c:1093
#8 0x000000000041650c in process_serial_event () at ../../../src/gdb/gdbserver/server.c:4193
#9 0x000000000041690c in handle_serial_event (err=0, client_data=0x0) at ../../../src/gdb/gdbserver/server.c:4323
#10 0x000000000041e74c in handle_file_event (event_file_desc=4) at ../../../src/gdb/gdbserver/event-loop.c:428
#11 0x000000000041dbf8 in process_event () at ../../../src/gdb/gdbserver/event-loop.c:184
#12 0x000000000041eb64 in start_event_loop () at ../../../src/gdb/gdbserver/event-loop.c:547
#13 0x0000000000415204 in captured_main (argc=4, argv=0x7fc1dcd0e8) at ../../../src/gdb/gdbserver/server.c:3688
#14 0x0000000000415434 in main (argc=4, argv=0x7fc1dcd0e8) at ../../../src/gdb/gdbserver/server.c:3773
And that is_64bit_tdesc call was added by 6085d6f6:
--- a/gdb/gdbserver/linux-aarch64-low.c
+++ b/gdb/gdbserver/linux-aarch64-low.c
@@ -364,6 +364,22 @@ aarch64_supports_z_point_type (char z_type)
switch (z_type)
{
case Z_PACKET_SW_BP:
+ {
+ if (!extended_protocol && is_64bit_tdesc ())
+ {
+ /* Only enable Z0 packet in non-multi-arch debugging. If
+ extended protocol is used, don't enable Z0 packet because
+ GDBserver may attach to 32-bit process. */
+ return 1;
+ }
+ else
+ {
+ /* Disable Z0 packet so that GDBserver doesn't have to handle
+ different breakpoint instructions (aarch64, arm, thumb etc)
+ in multi-arch debugging. */
+ return 0;
+ }
+ }
Now that Antoine's series that teaches gdbserver about different breakpoint
kinds is in, perhaps we could just revert 6085d6f6 now?
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH 04/18] gdbserver crash running gdb.threads/non-ldr-exc-1.exp
2015-11-24 16:34 ` Pedro Alves
@ 2015-11-26 16:23 ` Yao Qi
2015-11-30 14:53 ` Pedro Alves
0 siblings, 1 reply; 60+ messages in thread
From: Yao Qi @ 2015-11-26 16:23 UTC (permalink / raw)
To: Pedro Alves; +Cc: Yao Qi, gdb-patches
Pedro Alves <palves@redhat.com> writes:
> Now that Antoine's series that teaches gdbserver about different breakpoint
> kinds is in, perhaps we could just revert 6085d6f6 now?
Before we revert 6085d6f6, I need to teach aarch64 GDBserver understand
various arm breakpoint instructions. I'll do that.
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH 04/18] gdbserver crash running gdb.threads/non-ldr-exc-1.exp
2015-11-26 16:23 ` Yao Qi
@ 2015-11-30 14:53 ` Pedro Alves
0 siblings, 0 replies; 60+ messages in thread
From: Pedro Alves @ 2015-11-30 14:53 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On 11/26/2015 04:22 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
>
>> Now that Antoine's series that teaches gdbserver about different breakpoint
>> kinds is in, perhaps we could just revert 6085d6f6 now?
>
> Before we revert 6085d6f6, I need to teach aarch64 GDBserver understand
> various arm breakpoint instructions. I'll do that.
Thanks Yao, that'd be great! FWIW, I tried to do it myself using an Aarch64
machine on the gcc compile farm but got stuck due to lack of a handy
toolchain targeting 32-bit/Aarch32 on those machines.
I believe I've addressed all review comments otherwise. Let me know if
I missed any.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH 07/18] gdbserver crash if gdb attaches too fast
2015-10-14 15:28 [PATCH 00/18] Remote all-stop on top of non-stop Pedro Alves
` (13 preceding siblings ...)
2015-10-14 15:36 ` [PATCH 04/18] gdbserver crash running gdb.threads/non-ldr-exc-1.exp Pedro Alves
@ 2015-10-14 15:37 ` Pedro Alves
2015-10-14 15:37 ` [PATCH 09/18] Make dprintf-non-stop.exp cope with remote testing Pedro Alves
` (5 subsequent siblings)
20 siblings, 0 replies; 60+ messages in thread
From: Pedro Alves @ 2015-10-14 15:37 UTC (permalink / raw)
To: gdb-patches
With "maint set target-non-stop on", the attach tests occasionally
crash gdbserver.
Basically, gdb attaches with vAttach;PID, and then shortly after reads
the xml target description for that process, to figure out the
process' architecture. On the gdbserver side, the target description
is only filled in when the first process/thread in the thread group
reports its initial PTRACE_ATTACH SIGSTOP. So if GDB is fast enough,
it can read the target description _before_ that initial stop, and
then gdbserver dies dereferencing a NULL tdesc pointer.
gdb/gdbserver/ChangeLog:
2015-10-14 Pedro Alves <palves@redhat.com>
* linux-low.c (linux_attach): In non-stop mode, wait for one stop
before returning.
---
gdb/gdbserver/linux-low.c | 53 ++++++++++++++++++++++++++++++++++++-----------
1 file changed, 41 insertions(+), 12 deletions(-)
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 3a1a6ae..90eafe2 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -1039,12 +1039,16 @@ attach_proc_task_lwp_callback (ptid_t ptid)
return 0;
}
+static void async_file_mark (void);
+
/* Attach to PID. If PID is the tgid, attach to it and all
of its threads. */
static int
linux_attach (unsigned long pid)
{
+ struct process_info *proc;
+ struct thread_info *initial_thread;
ptid_t ptid = ptid_build (pid, pid, 0);
int err;
@@ -1055,17 +1059,12 @@ linux_attach (unsigned long pid)
error ("Cannot attach to process %ld: %s",
pid, linux_ptrace_attach_fail_reason_string (ptid, err));
- linux_add_process (pid, 1);
-
- if (!non_stop)
- {
- struct thread_info *thread;
+ proc = linux_add_process (pid, 1);
- /* Don't ignore the initial SIGSTOP if we just attached to this
- process. It will be collected by wait shortly. */
- thread = find_thread_ptid (ptid_build (pid, pid, 0));
- thread->last_resume_kind = resume_stop;
- }
+ /* Don't ignore the initial SIGSTOP if we just attached to this
+ process. It will be collected by wait shortly. */
+ initial_thread = find_thread_ptid (ptid_build (pid, pid, 0));
+ initial_thread->last_resume_kind = resume_stop;
/* We must attach to every LWP. If /proc is mounted, use that to
find them now. On the one hand, the inferior may be using raw
@@ -1077,6 +1076,38 @@ linux_attach (unsigned long pid)
that once thread_db is loaded, we'll still use it to list threads
and associate pthread info with each LWP. */
linux_proc_attach_tgid_threads (pid, attach_proc_task_lwp_callback);
+
+ /* GDB will shortly read the xml target description for this
+ process, to figure out the process' architecture. But the target
+ description is only filled in when the first process/thread in
+ the thread group reports its initial PTRACE_ATTACH SIGSTOP. Do
+ that now, otherwise, if GDB is fast enough, it could read the
+ target description _before_ that initial stop. */
+ if (non_stop)
+ {
+ struct lwp_info *lwp;
+ int wstat, lwpid;
+ ptid_t pid_ptid = pid_to_ptid (pid);
+
+ lwpid = linux_wait_for_event_filtered (pid_ptid, pid_ptid,
+ &wstat, __WALL);
+ gdb_assert (lwpid > 0);
+
+ lwp = find_lwp_pid (pid_to_ptid (lwpid));
+
+ if (!WIFSTOPPED (wstat) || WSTOPSIG (wstat) != SIGSTOP)
+ {
+ lwp->status_pending_p = 1;
+ lwp->status_pending = wstat;
+ }
+
+ initial_thread->last_resume_kind = resume_continue;
+
+ async_file_mark ();
+
+ gdb_assert (proc->tdesc != NULL);
+ }
+
return 0;
}
@@ -2878,8 +2909,6 @@ linux_stabilize_threads (void)
}
}
-static void async_file_mark (void);
-
/* Convenience function that is called when the kernel reports an
event that is not passed out to GDB. */
--
1.9.3
^ permalink raw reply [flat|nested] 60+ messages in thread* [PATCH 09/18] Make dprintf-non-stop.exp cope with remote testing
2015-10-14 15:28 [PATCH 00/18] Remote all-stop on top of non-stop Pedro Alves
` (14 preceding siblings ...)
2015-10-14 15:37 ` [PATCH 07/18] gdbserver crash if gdb attaches too fast Pedro Alves
@ 2015-10-14 15:37 ` Pedro Alves
2015-10-14 15:37 ` [PATCH 16/18] gdbserver/linux: Always wake up event loop after resume Pedro Alves
` (4 subsequent siblings)
20 siblings, 0 replies; 60+ messages in thread
From: Pedro Alves @ 2015-10-14 15:37 UTC (permalink / raw)
To: gdb-patches
Testing with the extended-remote board with "maint set target-non-stop
on" shows a dprintf-non-stop.exp regression. The issue is simply that
the test is expecting output that is only valid for the native target:
native:
[process 8676] #1 stopped.
remote:
[Thread 8900.8900] #1 stopped.
In order to expose this without "maint set target-non-stop on", this
restarts gdb with non-stop mode already enabled.
gdb/testsuite/ChangeLog:
2015-10-14 Pedro Alves <palves@redhat.com>
* gdb.base/dprintf-non-stop.exp: Use build_executable instead of
prepare_for_testing. Start gdb with "set non-stop on" appended to
GDBFLAGS. Lax expected stop output.
---
gdb/testsuite/gdb.base/dprintf-non-stop.exp | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/gdb/testsuite/gdb.base/dprintf-non-stop.exp b/gdb/testsuite/gdb.base/dprintf-non-stop.exp
index 4e3e5bc..5b878aa 100644
--- a/gdb/testsuite/gdb.base/dprintf-non-stop.exp
+++ b/gdb/testsuite/gdb.base/dprintf-non-stop.exp
@@ -20,13 +20,17 @@ if [is_remote target] then {
}
standard_testfile
+set executable ${testfile}
-if [prepare_for_testing "failed to prepare for dprintf with non-stop" \
+if [build_executable "failed to prepare for dprintf with non-stop" \
${testfile} ${srcfile} {debug}] {
return -1
}
-gdb_test_no_output "set non-stop on"
+save_vars { GDBFLAGS } {
+ append GDBFLAGS " -ex \"set non-stop on\""
+ clean_restart ${executable}
+}
if ![runto main] {
fail "Can't run to main"
@@ -60,7 +64,7 @@ gdb_test_multiple $test $test {
set test "inferior stopped"
gdb_test_multiple "" $test {
- -re "\r\n\\\[.* \[0-9\]+\\\] #1 stopped\\\.\r\n" {
+ -re "\r\n\\\[.*\\\] #1 stopped\\\.\r\n" {
pass $test
}
}
--
1.9.3
^ permalink raw reply [flat|nested] 60+ messages in thread* [PATCH 16/18] gdbserver/linux: Always wake up event loop after resume
2015-10-14 15:28 [PATCH 00/18] Remote all-stop on top of non-stop Pedro Alves
` (15 preceding siblings ...)
2015-10-14 15:37 ` [PATCH 09/18] Make dprintf-non-stop.exp cope with remote testing Pedro Alves
@ 2015-10-14 15:37 ` Pedro Alves
2015-10-26 17:28 ` Yao Qi
2015-10-14 15:38 ` [PATCH 08/18] gdbserver resume_stop handling bug Pedro Alves
` (3 subsequent siblings)
20 siblings, 1 reply; 60+ messages in thread
From: Pedro Alves @ 2015-10-14 15:37 UTC (permalink / raw)
To: gdb-patches
Running killed-outside.exp in with "maint set target-non-stop on"
hangs currently. This test has the inferior process die with a
SIGKILL while stopped. gdbserver gets a SIGCHLD and reacts by
retrieveing the SIGKILL events out of waitpid. But because the
process is not resumed from GDB's perspective, the event is left
pending. When GDB resumes the process afterwards, the process is not
really resumed because it already has the event pending. But nothing
wakes up the event loop to consume the event.
Handle this in the same way nat/linux-nat.c:linux_nat_resume handles
this.
gdb/gdbserver/ChangeLog:
2015-10-14 Pedro Alves <palves@redhat.com>
* linux-low.c (linux_resume): Wake up the event loop before
returning.
---
gdb/gdbserver/linux-low.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 73abe5c..c464207 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -4773,6 +4773,11 @@ linux_resume (struct thread_resume *resume_info, size_t n)
debug_printf ("linux_resume done\n");
debug_exit ();
}
+
+ /* We may have events that were pending that can/should be sent to
+ the client now. Trigger a linux_wait call. */
+ if (target_is_async_p ())
+ async_file_mark ();
}
/* This function is called once per thread. We check the thread's
--
1.9.3
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH 16/18] gdbserver/linux: Always wake up event loop after resume
2015-10-14 15:37 ` [PATCH 16/18] gdbserver/linux: Always wake up event loop after resume Pedro Alves
@ 2015-10-26 17:28 ` Yao Qi
2015-11-25 15:31 ` Pedro Alves
0 siblings, 1 reply; 60+ messages in thread
From: Yao Qi @ 2015-10-26 17:28 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
Pedro Alves <palves@redhat.com> writes:
> Handle this in the same way nat/linux-nat.c:linux_nat_resume handles
> this.
Nit, it is linux-nat.c instead of nat/linux-nat.c. In linux_nat_resume,
async_file_mark is guarded by lwp_status_pending_p (lp), so we need to
check whether there is an event pending in GDBserver too.
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH 16/18] gdbserver/linux: Always wake up event loop after resume
2015-10-26 17:28 ` Yao Qi
@ 2015-11-25 15:31 ` Pedro Alves
0 siblings, 0 replies; 60+ messages in thread
From: Pedro Alves @ 2015-11-25 15:31 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On 10/26/2015 02:32 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
>
>> Handle this in the same way nat/linux-nat.c:linux_nat_resume handles
>> this.
>
> Nit, it is linux-nat.c instead of nat/linux-nat.c. In linux_nat_resume,
> async_file_mark is guarded by lwp_status_pending_p (lp), so we need to
> check whether there is an event pending in GDBserver too.
I don't think we can just check whether there is an event pending
in the current thread -- the pending event may be in any
other thread that is now resumed from the client's perspective.
As we'd have to walk all threads anyway, seems simplest to just
unconditionally mark the event loop and let linux_wait & friends
handle that.
On the native side, we may have a latent bug and things work anyhow
because something else always wakes up the event loop once. (E.g.,
nowadays target_async always marks infrun's event source.)
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH 08/18] gdbserver resume_stop handling bug
2015-10-14 15:28 [PATCH 00/18] Remote all-stop on top of non-stop Pedro Alves
` (16 preceding siblings ...)
2015-10-14 15:37 ` [PATCH 16/18] gdbserver/linux: Always wake up event loop after resume Pedro Alves
@ 2015-10-14 15:38 ` Pedro Alves
2015-10-14 16:37 ` Eli Zaretskii
2015-10-15 10:46 ` [PATCH 00/18] Remote all-stop on top of non-stop Pedro Alves
` (2 subsequent siblings)
20 siblings, 1 reply; 60+ messages in thread
From: Pedro Alves @ 2015-10-14 15:38 UTC (permalink / raw)
To: gdb-patches
Running attach-many-short-lived-threads.exp with the extended-remote
board with "maint set target-non-stop on" times out -- the attach
never completes. Enabling infrun debug logs, we see that GDB is stuck
stopping all threads:
infrun: target_wait (-1.0.0, status) =
infrun: 1639.22213.0 [Thread 1639.22213],
infrun: status->kind = stopped, signal = GDB_SIGNAL_0
infrun: Thread 1639.22260 not executing
infrun: Thread 1639.22256 not executing
infrun: Thread 1639.22258 not executing
infrun: Thread 1639.22257 not executing
infrun: Thread 1639.22259 not executing
infrun: Thread 1639.22255 not executing
infrun: Thread 1639.22253 executing, already stopping
infrun: Thread 1639.22251 executing, already stopping
infrun: Thread 1639.22252 executing, already stopping
infrun: Thread 1639.22250 executing, already stopping
infrun: Thread 1639.22254 executing, already stopping
infrun: Thread 1639.22247 executing, already stopping
infrun: Thread 1639.22213 not executing
infrun: Thread 1639.22207 not executing
infrun: Thread 1639.22201 not executing
infrun: Thread 1639.22219 not executing
infrun: Thread 1639.1639 not executing
** HANG HERE **
GDB is waiting for the stop replies of any of those "already stopping"
threads. Take 22253 for example. On the gdbserver logs we see:
...
resume_stop request for LWP 22253
stopping LWP 22253
Sending sigstop to lwp 22253
linux_resume done
...
and:
my_waitpid (-1, 0x40000001)
my_waitpid (-1, 0x80000001): status(3057f), 22253
LWFE: waitpid(-1, ...) returned 22253, ERRNO-OK
LLW: waitpid 22253 received Trace/breakpoint trap (stopped)
pc is 0x3615ef4ce1
HEW: Got clone event from LWP 22253, new child is LWP 22259
but from here on, we never see any other event for LWP 22253. In
particular, we never see the expected SIGSTOP (from "Sending sigstop"
above). The issue is that linux_resume_stopped_resumed_lwps never
re-resumes the 22253 after the clone event.
gdb/gdbserver/ChangeLog:
2015-10-14 Pedro Alves <palves@redhat.com>
* linux-low.c (resume_stopped_resumed_lwps): Don't check whether
the thread's last_resume_kind was resume_stop.
---
gdb/gdbserver/linux-low.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 90eafe2..0d228cc 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -2419,7 +2419,6 @@ resume_stopped_resumed_lwps (struct inferior_list_entry *entry)
if (lp->stopped
&& !lp->suspended
&& !lp->status_pending_p
- && thread->last_resume_kind != resume_stop
&& thread->last_status.kind == TARGET_WAITKIND_IGNORE)
{
int step = thread->last_resume_kind == resume_step;
--
1.9.3
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH 08/18] gdbserver resume_stop handling bug
2015-10-14 15:38 ` [PATCH 08/18] gdbserver resume_stop handling bug Pedro Alves
@ 2015-10-14 16:37 ` Eli Zaretskii
2015-11-25 15:12 ` Pedro Alves
0 siblings, 1 reply; 60+ messages in thread
From: Eli Zaretskii @ 2015-10-14 16:37 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
> From: Pedro Alves <palves@redhat.com>
> Date: Wed, 14 Oct 2015 16:27:56 +0100
>
> infrun: Thread 1639.22253 executing, already stopping
^^^^^^^^^^^^^^^^
This is bad English. Suggest to change to "already stopped".
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH 08/18] gdbserver resume_stop handling bug
2015-10-14 16:37 ` Eli Zaretskii
@ 2015-11-25 15:12 ` Pedro Alves
2015-11-25 17:53 ` Eli Zaretskii
0 siblings, 1 reply; 60+ messages in thread
From: Pedro Alves @ 2015-11-25 15:12 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: gdb-patches
Hi Eli,
On 10/14/2015 05:37 PM, Eli Zaretskii wrote:
>> From: Pedro Alves <palves@redhat.com>
>> Date: Wed, 14 Oct 2015 16:27:56 +0100
>>
>> infrun: Thread 1639.22253 executing, already stopping
> ^^^^^^^^^^^^^^^^
> This is bad English. Suggest to change to "already stopped".
In this case, "already stopped" would be misleading, as the thread
isn't really fully stopped yet. This is logging the process of
stopping all threads, conveying "already in progress of being stopped".
IOW, "Thread foo is executing, and we're already stopping it".
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH 08/18] gdbserver resume_stop handling bug
2015-11-25 15:12 ` Pedro Alves
@ 2015-11-25 17:53 ` Eli Zaretskii
0 siblings, 0 replies; 60+ messages in thread
From: Eli Zaretskii @ 2015-11-25 17:53 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
> Date: Wed, 25 Nov 2015 15:12:26 +0000
> From: Pedro Alves <palves@redhat.com>
> CC: gdb-patches@sourceware.org
>
> >> infrun: Thread 1639.22253 executing, already stopping
> > ^^^^^^^^^^^^^^^^
> > This is bad English. Suggest to change to "already stopped".
>
> In this case, "already stopped" would be misleading, as the thread
> isn't really fully stopped yet. This is logging the process of
> stopping all threads, conveying "already in progress of being stopped".
> IOW, "Thread foo is executing, and we're already stopping it".
The problem is that this sentence makes "executing" and "stopping"
seem to refer to the same entity, which is not what you want. As you
say above: the _thread_ is executing, and _we_ are stopping it.
So if my suggestion seems too inaccurate to you (personally, I don't
think that nit will matter, but that's me), then we should rephrase
this more radically. How about
infrun: Thread 1639.22253 executing, about to stop
or
infrun: Thread 1639.22253 in process of being stopped
or even
infrun: Thread 1639.22253 was requested to stop, still didn't
Or something along these lines.
Thanks.
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH 00/18] Remote all-stop on top of non-stop
2015-10-14 15:28 [PATCH 00/18] Remote all-stop on top of non-stop Pedro Alves
` (17 preceding siblings ...)
2015-10-14 15:38 ` [PATCH 08/18] gdbserver resume_stop handling bug Pedro Alves
@ 2015-10-15 10:46 ` Pedro Alves
2015-10-16 16:47 ` Yao Qi
2015-10-27 13:11 ` Yao Qi
20 siblings, 0 replies; 60+ messages in thread
From: Pedro Alves @ 2015-10-15 10:46 UTC (permalink / raw)
To: gdb-patches
Hello,
On 10/14/2015 04:27 PM, Pedro Alves wrote:
> This series implements remote support for all-stop on top of non-stop.
(I got offlist queries asking about a convenience testing
branch, which I forgotten to do.)
Now pushed to:
users/palves/remote-all-stop-non-stop
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH 00/18] Remote all-stop on top of non-stop
2015-10-14 15:28 [PATCH 00/18] Remote all-stop on top of non-stop Pedro Alves
` (18 preceding siblings ...)
2015-10-15 10:46 ` [PATCH 00/18] Remote all-stop on top of non-stop Pedro Alves
@ 2015-10-16 16:47 ` Yao Qi
2015-10-19 11:48 ` Yao Qi
2015-10-27 13:11 ` Yao Qi
20 siblings, 1 reply; 60+ messages in thread
From: Yao Qi @ 2015-10-16 16:47 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
Pedro Alves <palves@redhat.com> writes:
Hi Pedro,
thanks for get them done! I am reviewing your patches, and continue
reviewing them next week. Testing on arm-linux is still running.
There is one fail on multi-arch aarch64-linux in
gdb.base/range-stepping.exp,
FAIL: gdb.base/range-stepping.exp: multi insns: next: vCont;s=1 vCont;r=1
in the testing GDB and GDBserver is configured for aarch64-linux, but
the program is compiled for arm-linux. I checked gdb.log that there is
vCont;r but no vCont;s. I suspect that GDB does software single
step, but arm-linux-tdep.c:arm_linux_software_single_step has already disable
software single step if GDBserver can do single step (AArch64 GDBserver
can do hardware single step).
> As with the equivalent work done for the native target, it's easy to
> revert back to the old behavior by entering "maint set target-non-stop
> off" or reverting the last patch.
After I revert the last patch, the test pass. I run out of time today,
and will look at the fail on Monday.
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH 00/18] Remote all-stop on top of non-stop
2015-10-16 16:47 ` Yao Qi
@ 2015-10-19 11:48 ` Yao Qi
2015-10-19 15:28 ` Pedro Alves
0 siblings, 1 reply; 60+ messages in thread
From: Yao Qi @ 2015-10-19 11:48 UTC (permalink / raw)
To: Yao Qi; +Cc: Pedro Alves, gdb-patches
Yao Qi <qiyaoltc@gmail.com> writes:
> There is one fail on multi-arch aarch64-linux in
> gdb.base/range-stepping.exp,
>
> FAIL: gdb.base/range-stepping.exp: multi insns: next: vCont;s=1 vCont;r=1
>
> in the testing GDB and GDBserver is configured for aarch64-linux, but
> the program is compiled for arm-linux. I checked gdb.log that there is
> vCont;r but no vCont;s. I suspect that GDB does software single
> step, but arm-linux-tdep.c:arm_linux_software_single_step has already disable
> software single step if GDBserver can do single step (AArch64 GDBserver
> can do hardware single step).
Hi Pedro,
This fail above isn't caused by your patch series, but this series
exposes something we need to think about here.
In the test, after command "n" is issued, the test expects to see
vCont;s and vCont;r, because GDB first steps over the breakpoint and
then do range-stepping across the line of code. Here is an assumption
that the remote target can do range stepping must support single step
(either by hardware or by software done by remote target itself), and
that is why I check the number vCont;r and vCont;s in the tests. This
assumption is true for x86-linux and aarch64-linux.
However, it isn't the case when aarch64-linux GDBservers debugs
arm-linux program. Aarch64-linux GDBserver claims supporting
range-stepping by defining aarch64_supports_range_stepping in
linux-aarch64-low.c, gdb.base/range-stepping.exp is tested.
(Note that this test is skipped on pure arm-linux testing, because
arm-linux GDBserver doesn't support range-stepping). GDB will still
emit vCont;r to do range stepping, that is fine.
Before range-stepping, GDB needs to step over the breakpoint by in-line
stepping, GDB uses the right gdbarch (for arm-linux) to do that, so the
right decision on hardware single step vs software single step can be
made according to target_can_do_single_step ...
static int
arm_linux_software_single_step (struct frame_info *frame)
{
struct gdbarch *gdbarch = get_frame_arch (frame);
struct address_space *aspace = get_frame_address_space (frame);
CORE_ADDR next_pc;
if (arm_deal_with_atomic_sequence (frame))
return 1;
/* If the target does have hardware single step, GDB doesn't have
to bother software single step. */
if (target_can_do_single_step () == 1)
return 0;
in the multi-arch case, GDB stills emit vCont;s because it knows the
remote target can do single step. That is why these tests pass before.
With your patch applied, GDB prefers to step over the breakpoint by
out-of-line stepping, and nowadays gdbarch (for arm-linux) decides to
resume the instructions in scratchpad rather than single step them,
in infrun.c:resume:
/* Update pc to reflect the new address from which we will
execute instructions due to displaced stepping. */
pc = regcache_read_pc (get_thread_regcache (inferior_ptid));
displaced = get_displaced_stepping_state (ptid_get_pid (inferior_ptid));
step = gdbarch_displaced_step_hw_singlestep (gdbarch,
displaced->step_closure);
(gdbarch_displaced_step_hw_singlestep returns zero on arm-linux), so GDB
emits vCont;c rather than vCont;s. There are some ways fixing this
problem,
1. stop checking vCont;s packet anymore in range-stepping tests.
2. let gdbarch_displaced_step_hw_singlestep returns true for arm-linux
in the multi-arch case like this,
int
arm_displaced_step_hw_singlestep (struct gdbarch *gdbarch,
struct displaced_step_closure *closure)
{
if (target_can_do_single_step () == 1)
return 1;
return 0;
}
then further, we need to either,
2.1 teach GDB core to support single stepping multiple instructions in
scratch pad. Nowadays, GDB only expects one stop event when executing
instructions in the scratchpad. ARM is the only target that GDB
copies more than one instructions to the scratchpad, and resume
program there instead of single step. Other targets, like x86,
aarch64, GDB only copies *one* instruction to the scratchpad and
single step.
2.2 rewrite arm displaced stepping code to be aware that the target
may be able to do single step, so that each time GDB has only to copy
one instruction to the scratchpad, do single step and fix up if necessary.
Fix #1 looks reasonable and ideal to me, and the easiest one. Fix #2.1
and #2.2 will need much work, at least #2.2, and I don't know how useful
#2.1 is.
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH 00/18] Remote all-stop on top of non-stop
2015-10-19 11:48 ` Yao Qi
@ 2015-10-19 15:28 ` Pedro Alves
2015-10-19 15:47 ` Yao Qi
0 siblings, 1 reply; 60+ messages in thread
From: Pedro Alves @ 2015-10-19 15:28 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
Hi Yao,
Thanks for the investigation.
On 10/19/2015 12:47 PM, Yao Qi wrote:
> There are some ways fixing this problem,
>
> 1. stop checking vCont;s packet anymore in range-stepping tests.
> 2. let gdbarch_displaced_step_hw_singlestep returns true for arm-linux
> in the multi-arch case like this,
>
> int
> arm_displaced_step_hw_singlestep (struct gdbarch *gdbarch,
> struct displaced_step_closure *closure)
> {
> if (target_can_do_single_step () == 1)
> return 1;
>
> return 0;
> }
> then further, we need to either,
>
> 2.1 teach GDB core to support single stepping multiple instructions in
> scratch pad. Nowadays, GDB only expects one stop event when executing
> instructions in the scratchpad. ARM is the only target that GDB
> copies more than one instructions to the scratchpad, and resume
> program there instead of single step. Other targets, like x86,
> aarch64, GDB only copies *one* instruction to the scratchpad and
> single step.
> 2.2 rewrite arm displaced stepping code to be aware that the target
> may be able to do single step, so that each time GDB has only to copy
> one instruction to the scratchpad, do single step and fix up if necessary.
>
> Fix #1 looks reasonable and ideal to me, and the easiest one. Fix #2.1
> and #2.2 will need much work, at least #2.2, and I don't know how useful
> #2.1 is.
>
Agreed, looks like #1 is the way to go. #2.1 may even be counter
productive -- running to a breakpoint to skip multiple instructions at
once is likely faster than multiple hardware single-steps.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH 00/18] Remote all-stop on top of non-stop
2015-10-19 15:28 ` Pedro Alves
@ 2015-10-19 15:47 ` Yao Qi
0 siblings, 0 replies; 60+ messages in thread
From: Yao Qi @ 2015-10-19 15:47 UTC (permalink / raw)
To: Pedro Alves; +Cc: Yao Qi, gdb-patches
Pedro Alves <palves@redhat.com> writes:
> Agreed, looks like #1 is the way to go. #2.1 may even be counter
> productive -- running to a breakpoint to skip multiple instructions at
> once is likely faster than multiple hardware single-steps.
Thanks for the feedback. I'll post the patch.
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH 00/18] Remote all-stop on top of non-stop
2015-10-14 15:28 [PATCH 00/18] Remote all-stop on top of non-stop Pedro Alves
` (19 preceding siblings ...)
2015-10-16 16:47 ` Yao Qi
@ 2015-10-27 13:11 ` Yao Qi
2015-11-30 19:59 ` Pedro Alves
20 siblings, 1 reply; 60+ messages in thread
From: Yao Qi @ 2015-10-27 13:11 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
Pedro Alves <palves@redhat.com> writes:
> This series implements remote support for all-stop on top of non-stop.
Hi Pedro,
I reviewed all the patches in this series, and gave my comments and
questions to some of them. The rest of them (patch 1, 5, 7-9, 12-15,
17-18) are good to me.
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH 00/18] Remote all-stop on top of non-stop
2015-10-27 13:11 ` Yao Qi
@ 2015-11-30 19:59 ` Pedro Alves
0 siblings, 0 replies; 60+ messages in thread
From: Pedro Alves @ 2015-11-30 19:59 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
Hi Yao,
On 10/27/2015 09:57 AM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
>
>> This series implements remote support for all-stop on top of non-stop.
>
> Hi Pedro,
> I reviewed all the patches in this series, and gave my comments and
> questions to some of them. The rest of them (patch 1, 5, 7-9, 12-15,
> 17-18) are good to me.
FYI, I've now pushed the whole series except the last patch that
enables "maint set target-nonstop on" by default.
I retested against gdbserver on an Aarch64 box on the compile farm and
that showed no regressions. I'll leave that last patch for after the
Aarch64 gdbserver crash you found is addressed.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 60+ messages in thread