* [PATCH 00/10] Step over thread exit (PR gdb/27338)
@ 2021-07-02 13:01 Pedro Alves
2021-07-02 13:01 ` [PATCH 01/10] displaced step: pass down target_waitstatus instead of gdb_signal Pedro Alves
` (10 more replies)
0 siblings, 11 replies; 23+ messages in thread
From: Pedro Alves @ 2021-07-02 13:01 UTC (permalink / raw)
To: gdb-patches
GDB doesn't handle correctly the case where a thread steps over a
breakpoint (using either in-line or displaced stepping), and the
executed instruction causes the thread to exit. This series fixes it.
Patch #2 is the core of the fix.
Patch #3 includes new remote protocol bits, and corresponding
documentation changes. There are no documentation changes anywhere
else in the series.
Pedro Alves (7):
displaced step: pass down target_waitstatus instead of gdb_signal
remote+gdbserver: stepped-thread-exited feature
struct resumed_pending_vcont_info -> struct resume_info
remote_target::update_thread_list: Don't delete stepping threads
stop_all_threads: (re-)enable async before waiting for stops
Tweak gdbserver's no-resumed handling
lib/my-syscalls.S: Refactor new SYSCALL macro
Simon Marchi (3):
gdb: clear step over information on thread exit (PR gdb/27338)
Implement stepped-thread-exited feature on native+gdbserver Linux
Testcases for stepping over thread exit syscall (PR gdb/27338)
gdb/NEWS | 5 +
gdb/displaced-stepping.c | 18 +-
gdb/displaced-stepping.h | 2 +-
gdb/doc/gdb.texinfo | 28 ++-
gdb/gdbarch.c | 4 +-
gdb/gdbarch.h | 10 +-
gdb/gdbarch.sh | 6 +-
gdb/infrun.c | 220 ++++++++++++++++--
gdb/linux-nat.c | 41 ++--
gdb/linux-tdep.c | 5 +-
gdb/linux-tdep.h | 2 +-
gdb/remote.c | 146 +++++++-----
gdb/target/target.h | 10 +-
...-over-thread-exit-while-stop-all-threads.c | 77 ++++++
...ver-thread-exit-while-stop-all-threads.exp | 69 ++++++
.../gdb.threads/step-over-thread-exit.c | 52 +++++
.../gdb.threads/step-over-thread-exit.exp | 70 ++++++
gdb/testsuite/lib/my-syscalls.S | 50 ++--
gdb/testsuite/lib/my-syscalls.h | 5 +
gdbserver/linux-low.cc | 44 ++--
gdbserver/linux-low.h | 2 +
gdbserver/server.cc | 28 ++-
gdbserver/target.cc | 6 +
gdbserver/target.h | 7 +
24 files changed, 764 insertions(+), 143 deletions(-)
create mode 100644 gdb/testsuite/gdb.threads/step-over-thread-exit-while-stop-all-threads.c
create mode 100644 gdb/testsuite/gdb.threads/step-over-thread-exit-while-stop-all-threads.exp
create mode 100644 gdb/testsuite/gdb.threads/step-over-thread-exit.c
create mode 100644 gdb/testsuite/gdb.threads/step-over-thread-exit.exp
base-commit: 62194b631d00112bac1f8856d3259d774df4c15e
--
2.26.2
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 01/10] displaced step: pass down target_waitstatus instead of gdb_signal
2021-07-02 13:01 [PATCH 00/10] Step over thread exit (PR gdb/27338) Pedro Alves
@ 2021-07-02 13:01 ` Pedro Alves
2021-07-05 22:19 ` Simon Marchi
2021-07-02 13:01 ` [PATCH 02/10] gdb: clear step over information on thread exit (PR gdb/27338) Pedro Alves
` (9 subsequent siblings)
10 siblings, 1 reply; 23+ messages in thread
From: Pedro Alves @ 2021-07-02 13:01 UTC (permalink / raw)
To: gdb-patches
A following patch will want to make displaced_step_buffers::finish
handle TARGET_WAITKIND_THREAD_EXITED.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <pedro@palves.net>
PR gdb/27338
* displaced-stepping.c
(displaced_step_instruction_executed_successfully)
(displaced_step_buffers::finish): Replace gdb_signal parameter
with a target_waitstatus parameter. Adjust.
* displaced-stepping.h (displaced_step_buffers::finish): Replace
gdb_signal parameter with a target_waitstatus parameter.
* gdbarch.sh (displaced_step_finish): Replace gdb_signal parameter
with a target_waitstatus parameter.
* infrun.c (displaced_step_finish): Replace gdb_signal parameter
with a target_waitstatus parameter. Adjust.
(handle_one, handle_inferior_event, finish_step_over): Pass down
target_waitstatus to displaced_step_finish.
* linux-tdep.c (linux_displaced_step_finish): Replace gdb_signal
parameter with a target_waitstatus parameter. Adjust.
* linux-tdep.h (linux_displaced_step_finish): Replace gdb_signal
parameter with a target_waitstatus parameter.
* gdbarch.c, gdbarch.h: Regenerate.
Change-Id: I4c5d338647b028071bc498c4e47063795a2db4c0
---
gdb/displaced-stepping.c | 11 ++++++-----
gdb/displaced-stepping.h | 2 +-
gdb/gdbarch.c | 4 ++--
gdb/gdbarch.h | 4 ++--
gdb/gdbarch.sh | 2 +-
gdb/infrun.c | 18 +++++++-----------
gdb/linux-tdep.c | 5 +++--
gdb/linux-tdep.h | 2 +-
8 files changed, 23 insertions(+), 25 deletions(-)
diff --git a/gdb/displaced-stepping.c b/gdb/displaced-stepping.c
index 59b78c22f6a..ac576d2d512 100644
--- a/gdb/displaced-stepping.c
+++ b/gdb/displaced-stepping.c
@@ -174,10 +174,11 @@ write_memory_ptid (ptid_t ptid, CORE_ADDR memaddr,
}
static bool
-displaced_step_instruction_executed_successfully (gdbarch *arch,
- gdb_signal signal)
+displaced_step_instruction_executed_successfully
+ (gdbarch *arch, const target_waitstatus &status)
{
- if (signal != GDB_SIGNAL_TRAP)
+ if (status.kind == TARGET_WAITKIND_STOPPED
+ && status.value.sig != GDB_SIGNAL_TRAP)
return false;
if (target_stopped_by_watchpoint ())
@@ -192,7 +193,7 @@ displaced_step_instruction_executed_successfully (gdbarch *arch,
displaced_step_finish_status
displaced_step_buffers::finish (gdbarch *arch, thread_info *thread,
- gdb_signal sig)
+ const target_waitstatus &status)
{
gdb_assert (thread->displaced_step_state.in_progress ());
@@ -238,7 +239,7 @@ displaced_step_buffers::finish (gdbarch *arch, thread_info *thread,
regcache *rc = get_thread_regcache (thread);
bool instruction_executed_successfully
- = displaced_step_instruction_executed_successfully (arch, sig);
+ = displaced_step_instruction_executed_successfully (arch, status);
if (instruction_executed_successfully)
{
diff --git a/gdb/displaced-stepping.h b/gdb/displaced-stepping.h
index 88c353d8fae..cf128b7186c 100644
--- a/gdb/displaced-stepping.h
+++ b/gdb/displaced-stepping.h
@@ -168,7 +168,7 @@ struct displaced_step_buffers
CORE_ADDR &displaced_pc);
displaced_step_finish_status finish (gdbarch *arch, thread_info *thread,
- gdb_signal sig);
+ const target_waitstatus &status);
const displaced_step_copy_insn_closure *
copy_insn_closure_by_addr (CORE_ADDR addr);
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index 830a86df89f..92f4c8b573a 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -4210,13 +4210,13 @@ set_gdbarch_displaced_step_prepare (struct gdbarch *gdbarch,
}
displaced_step_finish_status
-gdbarch_displaced_step_finish (struct gdbarch *gdbarch, thread_info *thread, gdb_signal sig)
+gdbarch_displaced_step_finish (struct gdbarch *gdbarch, thread_info *thread, const target_waitstatus &ws)
{
gdb_assert (gdbarch != NULL);
gdb_assert (gdbarch->displaced_step_finish != NULL);
if (gdbarch_debug >= 2)
fprintf_unfiltered (gdb_stdlog, "gdbarch_displaced_step_finish called\n");
- return gdbarch->displaced_step_finish (gdbarch, thread, sig);
+ return gdbarch->displaced_step_finish (gdbarch, thread, ws);
}
void
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index ece765b826f..06f44ec824f 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -1138,8 +1138,8 @@ extern void set_gdbarch_displaced_step_prepare (struct gdbarch *gdbarch, gdbarch
/* Clean up after a displaced step of THREAD. */
-typedef displaced_step_finish_status (gdbarch_displaced_step_finish_ftype) (struct gdbarch *gdbarch, thread_info *thread, gdb_signal sig);
-extern displaced_step_finish_status gdbarch_displaced_step_finish (struct gdbarch *gdbarch, thread_info *thread, gdb_signal sig);
+typedef displaced_step_finish_status (gdbarch_displaced_step_finish_ftype) (struct gdbarch *gdbarch, thread_info *thread, const target_waitstatus &ws);
+extern displaced_step_finish_status gdbarch_displaced_step_finish (struct gdbarch *gdbarch, thread_info *thread, const target_waitstatus &ws);
extern void set_gdbarch_displaced_step_finish (struct gdbarch *gdbarch, gdbarch_displaced_step_finish_ftype *displaced_step_finish);
/* Return the closure associated to the displaced step buffer that is at ADDR. */
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index d9332c2103e..d9960887d4f 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -844,7 +844,7 @@ M;void;displaced_step_fixup;struct displaced_step_copy_insn_closure *closure, CO
M;displaced_step_prepare_status;displaced_step_prepare;thread_info *thread, CORE_ADDR &displaced_pc;thread, displaced_pc
# Clean up after a displaced step of THREAD.
-m;displaced_step_finish_status;displaced_step_finish;thread_info *thread, gdb_signal sig;thread, sig;;NULL;;(! gdbarch->displaced_step_finish) != (! gdbarch->displaced_step_prepare)
+m;displaced_step_finish_status;displaced_step_finish;thread_info *thread, const target_waitstatus &ws;thread, ws;;NULL;;(! gdbarch->displaced_step_finish) != (! gdbarch->displaced_step_prepare)
# Return the closure associated to the displaced step buffer that is at ADDR.
F;const displaced_step_copy_insn_closure *;displaced_step_copy_insn_closure_by_addr;inferior *inf, CORE_ADDR addr;inf, addr
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 9469b74af39..869b3dd113a 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1771,7 +1771,8 @@ displaced_step_prepare (thread_info *thread)
DISPLACED_STEP_FINISH_STATUS_OK as well. */
static displaced_step_finish_status
-displaced_step_finish (thread_info *event_thread, enum gdb_signal signal)
+displaced_step_finish (thread_info *event_thread,
+ const target_waitstatus &event_status)
{
displaced_step_thread_state *displaced = &event_thread->displaced_step_state;
@@ -1793,7 +1794,7 @@ displaced_step_finish (thread_info *event_thread, enum gdb_signal signal)
/* Do the fixup, and release the resources acquired to do the displaced
step. */
return gdbarch_displaced_step_finish (displaced->get_original_gdbarch (),
- event_thread, signal);
+ event_thread, event_status);
}
/* Data to be passed around while handling an event. This data is
@@ -4900,7 +4901,7 @@ handle_one (const wait_one_event &event)
t->suspend.waitstatus.kind = TARGET_WAITKIND_IGNORE;
t->suspend.waitstatus_pending_p = 0;
- if (displaced_step_finish (t, GDB_SIGNAL_0)
+ if (displaced_step_finish (t, event.ws)
== DISPLACED_STEP_FINISH_STATUS_NOT_EXECUTED)
{
/* Add it back to the step-over queue. */
@@ -4915,7 +4916,6 @@ handle_one (const wait_one_event &event)
}
else
{
- enum gdb_signal sig;
struct regcache *regcache;
infrun_debug_printf
@@ -4926,10 +4926,7 @@ handle_one (const wait_one_event &event)
/* Record for later. */
save_waitstatus (t, &event.ws);
- sig = (event.ws.kind == TARGET_WAITKIND_STOPPED
- ? event.ws.value.sig : GDB_SIGNAL_0);
-
- if (displaced_step_finish (t, sig)
+ if (displaced_step_finish (t, event.ws)
== DISPLACED_STEP_FINISH_STATUS_NOT_EXECUTED)
{
/* Add it back to the step-over queue. */
@@ -5523,7 +5520,7 @@ handle_inferior_event (struct execution_control_state *ecs)
has been done. Perform cleanup for parent process here. Note
that this operation also cleans up the child process for vfork,
because their pages are shared. */
- displaced_step_finish (ecs->event_thread, GDB_SIGNAL_TRAP);
+ displaced_step_finish (ecs->event_thread, ecs->ws);
/* Start a new step-over in another thread if there's one
that needs it. */
start_step_over ();
@@ -5873,8 +5870,7 @@ resumed_thread_with_pending_status (struct thread_info *tp,
static int
finish_step_over (struct execution_control_state *ecs)
{
- displaced_step_finish (ecs->event_thread,
- ecs->event_thread->suspend.stop_signal);
+ displaced_step_finish (ecs->event_thread, ecs->ws);
bool had_step_over_info = step_over_info_valid_p ();
diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index 927e69bf1e1..b7b8295805a 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -2562,13 +2562,14 @@ linux_displaced_step_prepare (gdbarch *arch, thread_info *thread,
/* See linux-tdep.h. */
displaced_step_finish_status
-linux_displaced_step_finish (gdbarch *arch, thread_info *thread, gdb_signal sig)
+linux_displaced_step_finish (gdbarch *arch, thread_info *thread,
+ const target_waitstatus &status)
{
linux_info *per_inferior = get_linux_inferior_data (thread->inf);
gdb_assert (per_inferior->disp_step_bufs.has_value ());
- return per_inferior->disp_step_bufs->finish (arch, thread, sig);
+ return per_inferior->disp_step_bufs->finish (arch, thread, status);
}
/* See linux-tdep.h. */
diff --git a/gdb/linux-tdep.h b/gdb/linux-tdep.h
index 28b60e46579..e835a7f987a 100644
--- a/gdb/linux-tdep.h
+++ b/gdb/linux-tdep.h
@@ -72,7 +72,7 @@ extern displaced_step_prepare_status linux_displaced_step_prepare
/* Implementation of gdbarch_displaced_step_finish. */
extern displaced_step_finish_status linux_displaced_step_finish
- (gdbarch *arch, thread_info *thread, gdb_signal sig);
+ (gdbarch *arch, thread_info *thread, const target_waitstatus &status);
/* Implementation of gdbarch_displaced_step_copy_insn_closure_by_addr. */
--
2.26.2
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 02/10] gdb: clear step over information on thread exit (PR gdb/27338)
2021-07-02 13:01 [PATCH 00/10] Step over thread exit (PR gdb/27338) Pedro Alves
2021-07-02 13:01 ` [PATCH 01/10] displaced step: pass down target_waitstatus instead of gdb_signal Pedro Alves
@ 2021-07-02 13:01 ` Pedro Alves
2021-07-05 18:34 ` Simon Marchi
2021-07-02 13:01 ` [PATCH 03/10] remote+gdbserver: stepped-thread-exited feature Pedro Alves
` (8 subsequent siblings)
10 siblings, 1 reply; 23+ messages in thread
From: Pedro Alves @ 2021-07-02 13:01 UTC (permalink / raw)
To: gdb-patches
From: Simon Marchi <simon.marchi@efficios.com>
GDB doesn't handle correctly the case where a thread steps over a
breakpoint (using either in-line or displaced stepping), and the
executed instruction causes the thread to exit.
Using the test program included later in the series, this is what it
looks like with displaced-stepping, on x86-64 Linux, where we have two
displaced-step buffers:
$ ./gdb -q -nx --data-directory=data-directory build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/step-over-thread-exit/step-over-thread-exit -ex "b my_exit_syscall" -ex r
Reading symbols from build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/step-over-thread-exit/step-over-thread-exit...
Breakpoint 1 at 0x123c: file src/binutils-gdb/gdb/testsuite/lib/my-syscalls.S, line 68.
Starting program: build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/step-over-thread-exit/step-over-thread-exit
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/usr/lib/../lib/libthread_db.so.1".
[New Thread 0x7ffff7c5f640 (LWP 2915510)]
[Switching to Thread 0x7ffff7c5f640 (LWP 2915510)]
Thread 2 "step-over-threa" hit Breakpoint 1, my_exit_syscall () at src/binutils-gdb/gdb/testsuite/lib/my-syscalls.S:68
68 syscall
(gdb) c
Continuing.
[New Thread 0x7ffff7c5f640 (LWP 2915524)]
[Thread 0x7ffff7c5f640 (LWP 2915510) exited]
[Switching to Thread 0x7ffff7c5f640 (LWP 2915524)]
Thread 3 "step-over-threa" hit Breakpoint 1, my_exit_syscall () at src/binutils-gdb/gdb/testsuite/lib/my-syscalls.S:68
68 syscall
(gdb) c
Continuing.
[New Thread 0x7ffff7c5f640 (LWP 2915616)]
[Thread 0x7ffff7c5f640 (LWP 2915524) exited]
[Switching to Thread 0x7ffff7c5f640 (LWP 2915616)]
Thread 4 "step-over-threa" hit Breakpoint 1, my_exit_syscall () at src/binutils-gdb/gdb/testsuite/lib/my-syscalls.S:68
68 syscall
(gdb) c
Continuing.
... hangs ...
The first two times we do "continue", we displaced-step the syscall
instruction that causes the thread to exit. When the thread exits,
the main thread, waiting on pthread_join, is unblocked. It spawns a
new thread, which hits the breakpoint on the syscall again. However,
infrun was never notified that the displaced-stepping threads are done
using the displaced-step buffer, so they are now both marked as used.
So when we do the third continue, there are no buffers available to
displaced-step the syscall, so it waits forever.
When trying the same but with in-line step over (displaced-stepping
disabled):
$ ./gdb -q -nx --data-directory=data-directory \
build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/step-over-thread-exit/step-over-thread-exit \
-ex "b my_exit_syscall" -ex "set displaced-stepping off" -ex r
Reading symbols from build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/step-over-thread-exit/step-over-thread-exit...
Breakpoint 1 at 0x123c: file src/binutils-gdb/gdb/testsuite/lib/my-syscalls.S, line 68.
Starting program: build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/step-over-thread-exit/step-over-thread-exit
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/usr/lib/../lib/libthread_db.so.1".
[New Thread 0x7ffff7c5f640 (LWP 2928290)]
[Switching to Thread 0x7ffff7c5f640 (LWP 2928290)]
Thread 2 "step-over-threa" hit Breakpoint 1, my_exit_syscall () at src/binutils-gdb/gdb/testsuite/lib/my-syscalls.S:68
68 syscall
(gdb) c
Continuing.
[Thread 0x7ffff7c5f640 (LWP 2928290) exited]
No unwaited-for children left.
(gdb) i th
Id Target Id Frame
1 Thread 0x7ffff7c60740 (LWP 2928285) "step-over-threa" 0x00007ffff7f7c9b7 in __pthread_clockjoin_ex () from /usr/lib/libpthread.so.0
The current thread <Thread ID 2> has terminated. See `help thread'.
(gdb) thread 1
[Switching to thread 1 (Thread 0x7ffff7c60740 (LWP 2928285))]
#0 0x00007ffff7f7c9b7 in __pthread_clockjoin_ex () from /usr/lib/libpthread.so.0
(gdb) c
Continuing.
^C^C
... hangs ...
The "continue" causes an in-line step to occur, meaning the main
thread is stopped while we step the syscall. The thread exits when
executing the syscall, the linux-nat target notices there are no more
resumed threads to be waited for, so returns
TARGET_WAITKIND_NO_RESUMED, which causes the prompt to return. But
infrun never clears the in-line step over info. So if we try
continuing the main thread, GDB doesn't resume it, because it thinks
there's an in-line step in progress that we need to wait for to
finish, and we are stuck there.
To fix this, infrun needs to be informed when a thread doing a
displaced or in-line step over exits.
Thread events (thread created and thread exited) are not normally
reported by targets. For targets to report them, infrun requests them
to be reported by using target_ops::thread_events. This is currently
only used in stop_all_threads, when infrun tries to stop all threads
when doing an all-stop-on-top-of-non-stop stop. When an in-line or
displaced step is executed, they are therefore not enabled. The
linux-nat target gets the event about the thread exit and simply
deletes the thread without notifying the core. Infrun therefore never
has a chance to clean up the in-line or displaced step resources the
thread was using.
One possibility would be for infrun to enable reporting of thread
events when an in-line or displaced step is in progress. But doing
so, we could get thread events for threads we don't care about that
happen during the step (probably not a big deal, but a bit wasteful).
It would also add logic that is not trivial to enable and disable
thread events correctly.
Instead, this commit adds a line to the contract between infrun and
the targets: if the exiting thread was last stepped (i.e. asked to
step last time it was resumed), then the target must report the exit
event to infrun. This adds a bit of logic on the target side, but
makes it much easier for infrun.
This patch modifies handle_inferior_event in infrun.c to clean up any
step-over the exiting thread might have been doing at the time of the
exit. The cases to consider are:
- the exiting thread was doing an in-line step-over with an all-stop
target
- the exiting thread was doing an in-line step-over with a non-stop
target
- the exiting thread was doing a displaced step-over with a non-stop
target
The code handling TARGET_WAITKIND_THREAD_EXITED in
handle_inferior_event should also be prepared to handle such an event
for a thread that was not doing a step-over:
- maybe the thread was asked to step, stepped an instruction that
caused it to exit, but was not stepping over a breakpoint.
- maybe the target just freely reports all thread exit events
The displaced-stepping buffer implementation in displaced-stepping.c
is modified to account for the fact that it's possible that we
"finish" a displaced step for an exited thread. The buffer that the
now exited thread was using is marked as available again and the
original instructions under the scratch pad are restored. However, it
skips applying the fixup, which wouldn't make sense since the thread
does not exist anymore.
Another case handling is if a displaced-step thread exits, and the
event is reported while we are in stop_all_threads. We should call
displaced_step_finish in the handle_one function, in that case. It
was already called in other code paths, just not the "thread exited"
path.
This commit doesn't make any target report the
TARGET_WAITKIND_THREAD_EXITED for stepping threads yet, that'll be
done later in the series.
gdb/ChangeLog:
yyyy-mm-dd Simon Marchi <simon.marchi@efficios.com>
Pedro Alves <pedro@palves.net>
PR gdb/27338
* displaced-stepping.c (displaced_step_buffers::finish): Return
early if thread exited.
* gdbarch.sh (displaced_step_finish): Modify comment.
* gdbarch.c: Re-generate.
* gdbarch.h: Re-generate.
* infrun.c (handle_one): Finish displaceds step if
TARGET_WAITKIND_THREAD_EXITED.
(handle_no_resumed): Document return value. Don't clear
stop_print_frame here.
(handle_thread_exited): New.
(handle_inferior_event): Remove early handling of
TARGET_WAITKIND_THREAD_EXITED, and handle
TARGET_WAITKIND_THREAD_EXITED where other event kinds are
handled. Use handle_thread_exited.
(finish_step_over): Return early if the thread exited.
(normal_stop) <TARGET_WAITKIND_NO_RESUMED>: Clear stop_print_frame
here.
* target/target.h (target_wait): Document thread exit event
expectations.
Co-authored-by: Pedro Alves <pedro@palves.net>
Change-Id: I745c6955d7ef90beb83bcf0ff1d1ac8b9b6285a5
---
gdb/displaced-stepping.c | 7 ++
gdb/gdbarch.h | 6 +-
gdb/gdbarch.sh | 4 ++
gdb/infrun.c | 151 ++++++++++++++++++++++++++++++++++++---
gdb/target/target.h | 10 ++-
5 files changed, 167 insertions(+), 11 deletions(-)
diff --git a/gdb/displaced-stepping.c b/gdb/displaced-stepping.c
index ac576d2d512..670d4ea3dbf 100644
--- a/gdb/displaced-stepping.c
+++ b/gdb/displaced-stepping.c
@@ -236,6 +236,13 @@ displaced_step_buffers::finish (gdbarch *arch, thread_info *thread,
target_pid_to_str (thread->ptid).c_str (),
paddress (arch, buffer->addr));
+ /* If the thread exited while stepping, we are done. The code above
+ made the buffer available again, and we restored the bytes in the
+ buffer. We don't want to run the fixup: since the thread is now
+ dead there's nothing to adjust. */
+ if (status.kind == TARGET_WAITKIND_THREAD_EXITED)
+ return DISPLACED_STEP_FINISH_STATUS_OK;
+
regcache *rc = get_thread_regcache (thread);
bool instruction_executed_successfully
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 06f44ec824f..d7dcf4648e9 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -1136,7 +1136,11 @@ typedef displaced_step_prepare_status (gdbarch_displaced_step_prepare_ftype) (st
extern displaced_step_prepare_status gdbarch_displaced_step_prepare (struct gdbarch *gdbarch, thread_info *thread, CORE_ADDR &displaced_pc);
extern void set_gdbarch_displaced_step_prepare (struct gdbarch *gdbarch, gdbarch_displaced_step_prepare_ftype *displaced_step_prepare);
-/* Clean up after a displaced step of THREAD. */
+/* Clean up after a displaced step of THREAD.
+
+ It is possible for the displaced-stepped instruction to have caused the
+ thread to exit. The implementation can detect this case by checking if
+ WS.kind is TARGET_WAITKIND_THREAD_EXITED. */
typedef displaced_step_finish_status (gdbarch_displaced_step_finish_ftype) (struct gdbarch *gdbarch, thread_info *thread, const target_waitstatus &ws);
extern displaced_step_finish_status gdbarch_displaced_step_finish (struct gdbarch *gdbarch, thread_info *thread, const target_waitstatus &ws);
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index d9960887d4f..d59b26792a3 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -844,6 +844,10 @@ M;void;displaced_step_fixup;struct displaced_step_copy_insn_closure *closure, CO
M;displaced_step_prepare_status;displaced_step_prepare;thread_info *thread, CORE_ADDR &displaced_pc;thread, displaced_pc
# Clean up after a displaced step of THREAD.
+#
+# It is possible for the displaced-stepped instruction to have caused the
+# thread to exit. The implementation can detect this case by checking if
+# WS.kind is TARGET_WAITKIND_THREAD_EXITED.
m;displaced_step_finish_status;displaced_step_finish;thread_info *thread, const target_waitstatus &ws;thread, ws;;NULL;;(! gdbarch->displaced_step_finish) != (! gdbarch->displaced_step_prepare)
# Return the closure associated to the displaced step buffer that is at ADDR.
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 869b3dd113a..53331f27a0a 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -3765,6 +3765,7 @@ struct wait_one_event
static bool handle_one (const wait_one_event &event);
static void restart_threads (struct thread_info *event_thread);
+static int finish_step_over (struct execution_control_state *ecs);
/* Prepare and stabilize the inferior for detaching it. E.g.,
detaching while a thread is displaced stepping is a recipe for
@@ -4871,6 +4872,16 @@ handle_one (const wait_one_event &event)
event.ws);
save_waitstatus (t, &event.ws);
t->stop_requested = false;
+
+ if (event.ws.kind == TARGET_WAITKIND_THREAD_EXITED)
+ {
+ if (displaced_step_finish (t, event.ws)
+ != DISPLACED_STEP_FINISH_STATUS_OK)
+ {
+ gdb_assert_not_reached ("displaced_step_finish on "
+ "exited thread failed");
+ }
+ }
}
}
else
@@ -5070,7 +5081,9 @@ stop_all_threads (void)
}
}
-/* Handle a TARGET_WAITKIND_NO_RESUMED event. */
+/* Handle a TARGET_WAITKIND_NO_RESUMED event. Return true if we
+ handled the event and should continue waiting. Return false if we
+ should stop and report the event to the user. */
static bool
handle_no_resumed (struct execution_control_state *ecs)
@@ -5205,6 +5218,117 @@ handle_no_resumed (struct execution_control_state *ecs)
return false;
}
+/* Handle a TARGET_WAITKIND_THREAD_EXITED event. Return true if we
+ handled the event and should continue waiting. Return false if we
+ should stop and report the event to the user. */
+
+static bool
+handle_thread_exited (execution_control_state *ecs)
+{
+ context_switch (ecs);
+
+ /* Clear these so we don't re-start the thread stepping over a
+ breakpoint/watchpoint. */
+ ecs->event_thread->stepping_over_breakpoint = 0;
+ ecs->event_thread->stepping_over_watchpoint = 0;
+
+ /* Maybe the thread was doing a step-over, if so release
+ resources and start any further pending step-overs.
+
+ If we are on a non-stop target and the thread was doing an
+ in-line step, this also restarts the other threads. */
+ int ret = finish_step_over (ecs);
+
+ /* finish_step_over returns true if it moves ecs' wait status
+ back into the thread, so that we go handle another pending
+ event before this one. But we know it never does that if
+ the event thread has exited. */
+ gdb_assert (ret == 0);
+
+ /* If finish_step_over started a new in-line step-over, don't
+ try to restart anything else. */
+ if (step_over_info_valid_p ())
+ {
+ delete_thread (ecs->event_thread);
+ return true;
+ }
+
+ /* Maybe we are on an all-stop target and we got this event
+ while doing a step-like command on another thread. If so,
+ go back to doing that. If this thread was stepping,
+ switch_back_to_stepped_thread will consider that the thread
+ was interrupted mid-step and will try keep stepping it. We
+ don't want that, the thread is gone. So clear the proceed
+ status so it doesn't do that. */
+ clear_proceed_status_thread (ecs->event_thread);
+ if (switch_back_to_stepped_thread (ecs))
+ {
+ delete_thread (ecs->event_thread);
+ return true;
+ }
+
+ inferior *inf = ecs->event_thread->inf;
+ bool slock_applies = schedlock_applies (ecs->event_thread);
+
+ delete_thread (ecs->event_thread);
+ ecs->event_thread = nullptr;
+
+ auto handle_as_no_resumed = [ecs] ()
+ {
+ ecs->ws.kind = TARGET_WAITKIND_NO_RESUMED;
+ ecs->event_thread = nullptr;
+ ecs->ptid = minus_one_ptid;
+ return handle_no_resumed (ecs);
+ };
+
+ /* If we are on an all-stop target, the target has stopped all
+ threads to report the event. We don't actually want to
+ stop, so restart the threads. */
+ if (!target_is_non_stop_p ())
+ {
+ if (slock_applies)
+ {
+ /* Since the target is !non-stop, then everything is stopped
+ at this point, and we can't assume we'll get further
+ events until we resume the target again. Handle this
+ event like if it were a TARGET_WAITKIND_NO_RESUMED. Note
+ this refreshes the thread list and checks whether there
+ are other resumed threads before deciding whether to
+ print "no-unwaited-for left". This is important because
+ the user could have done:
+
+ (gdb) set scheduler-locking on
+ (gdb) thread 1
+ (gdb) c&
+ (gdb) thread 2
+ (gdb) c
+
+ ... and only one of the threads exited. */
+ return handle_as_no_resumed ();
+ }
+ else
+ {
+ /* Switch to the first non-exited thread we can find, and
+ resume. */
+ auto range = inf->non_exited_threads ();
+ if (range.begin () == range.end ())
+ {
+ /* Looks like the target reported a
+ TARGET_WAITKIND_THREAD_EXITED for its last known
+ thread. */
+ return handle_as_no_resumed ();
+ }
+ thread_info *non_exited_thread = *range.begin ();
+ switch_to_thread (non_exited_thread);
+ insert_breakpoints ();
+ resume (GDB_SIGNAL_0);
+ }
+ }
+
+ prepare_to_wait (ecs);
+ return true;
+}
+
/* 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.
@@ -5243,12 +5367,6 @@ handle_inferior_event (struct execution_control_state *ecs)
return;
}
- if (ecs->ws.kind == TARGET_WAITKIND_THREAD_EXITED)
- {
- prepare_to_wait (ecs);
- return;
- }
-
if (ecs->ws.kind == TARGET_WAITKIND_NO_RESUMED
&& handle_no_resumed (ecs))
return;
@@ -5263,7 +5381,6 @@ handle_inferior_event (struct execution_control_state *ecs)
{
/* No unwaited-for children left. IOW, all resumed children
have exited. */
- stop_print_frame = false;
stop_waiting (ecs);
return;
}
@@ -5412,6 +5529,15 @@ handle_inferior_event (struct execution_control_state *ecs)
keep_going (ecs);
return;
+ case TARGET_WAITKIND_THREAD_EXITED:
+ if (handle_thread_exited (ecs))
+ return;
+ /* Need to re-record the last target status because the waitkind
+ may have changed to TARGET_WAITKIND_NO_RESUMED. */
+ set_last_target_status (ecs->target, ecs->ptid, ecs->ws);
+ stop_waiting (ecs);
+ break;
+
case TARGET_WAITKIND_EXITED:
case TARGET_WAITKIND_SIGNALLED:
{
@@ -5925,6 +6051,13 @@ finish_step_over (struct execution_control_state *ecs)
if (ecs->event_thread->stepping_over_watchpoint)
return 0;
+ /* The code below is meant to avoid one thread hogging the event
+ loop by doing constant in-line step overs. If the stepping
+ thread exited, there's no risk for this to happen, so we can
+ safely let our caller process the event immediately. */
+ if (ecs->ws.kind == TARGET_WAITKIND_THREAD_EXITED)
+ return 0;
+
pending = iterate_over_threads (resumed_thread_with_pending_status,
NULL);
if (pending != NULL)
@@ -8568,6 +8701,8 @@ normal_stop (void)
if (last.kind == TARGET_WAITKIND_NO_RESUMED)
{
+ stop_print_frame = false;
+
SWITCH_THRU_ALL_UIS ()
if (current_ui->prompt_state == PROMPT_BLOCKED)
{
diff --git a/gdb/target/target.h b/gdb/target/target.h
index 82bebfa6a13..76ca2631268 100644
--- a/gdb/target/target.h
+++ b/gdb/target/target.h
@@ -83,8 +83,14 @@ extern void target_continue (ptid_t ptid, enum gdb_signal signal);
the debugging target from the stack; GDB isn't prepared to get back
to the prompt with a debugging target but without the frame cache,
stop_pc, etc., set up. OPTIONS is a bitwise OR of TARGET_W*
- options. */
-
+ options.
+
+ Unless thread exit event reporting has been explicitly enabled
+ (target_thread_events in gdb, QThreadEvents/cs.report_thread_events
+ in gdbserver), targets are free to not report thread exit events
+ for threads that were run free. Targets should report thread exit
+ events for threads that were stepped, regardless of whether thread
+ exit events were requested. */
extern ptid_t target_wait (ptid_t ptid, struct target_waitstatus *status,
target_wait_flags options);
--
2.26.2
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 03/10] remote+gdbserver: stepped-thread-exited feature
2021-07-02 13:01 [PATCH 00/10] Step over thread exit (PR gdb/27338) Pedro Alves
2021-07-02 13:01 ` [PATCH 01/10] displaced step: pass down target_waitstatus instead of gdb_signal Pedro Alves
2021-07-02 13:01 ` [PATCH 02/10] gdb: clear step over information on thread exit (PR gdb/27338) Pedro Alves
@ 2021-07-02 13:01 ` Pedro Alves
2021-07-02 17:13 ` Eli Zaretskii
2021-07-05 19:27 ` Simon Marchi
2021-07-02 13:01 ` [PATCH 04/10] struct resumed_pending_vcont_info -> struct resume_info Pedro Alves
` (7 subsequent siblings)
10 siblings, 2 replies; 23+ messages in thread
From: Pedro Alves @ 2021-07-02 13:01 UTC (permalink / raw)
To: gdb-patches; +Cc: Simon Marchi
This patch makes GDB and GDBserver each tell the other that they
support reporting 'w' (thread exit) stop replies for stepping threads
that exit, even without QThreadEvents active.
Having GDBserver tell GDB that it sends thread exited events for
stepping threads that exit is important to fix one race, in a
following patch.
This also avoids breaking backward compatibility. If we make
gdbserver send 'w' stop replies for stepping threads that exit, we'd
get:
Thread 2 "step-over-threa" hit Breakpoint 1, my_exit_syscall () at src/gdb/testsuite/lib/my-syscalls.S:68
68 syscall
(gdb) s
No unwaited-for children left.
After that same change (and without this fix), we get:
Thread 2 "step-over-threa" hit Breakpoint 1, my_exit_syscall () at src/gdb/testsuite/lib/my-syscalls.S:68
68 syscall
(gdb) s
warning: Invalid remote reply: w0;p27c295.27cb15
At this point, the target is stopped, but GDB is still waiting. All
you can do is stop debugging:
^C^CThe target is not responding to interrupt requests.
Stop debugging it? (y or n) Quit
(gdb)
With an old gdb vs new gdbserver, we'll instead get:
...
Thread 2 "step-over-threa" hit Breakpoint 1, my_exit_syscall () at src/gdb/testsuite/lib/my-syscalls.S:68
68 syscall
(gdb) s
warning: Remote failure reply: E.Thread exited.
Could not read registers; remote failure reply 'E01'
Could not read registers; remote failure reply 'E01'
(gdb) info threads
Id Target Id Frame
1 Thread 2486981.2486981 "step-over-threa" __pthread_clockjoin_ex (....) at pthread_join_common.c:145
The current thread <Thread ID 2> has terminated. See `help thread'.
(gdb)
This may look like a regression against before the change to make
gdbserver send 'w' stop replies for stepping threads that exit, but
keep in mind that old GDB does not release the displaced stepping
buffer on no-resumed, leading to hangs on subsequent resumes.
No GDBserver backend enables the feature yet. That will happen in a
following patch.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <pedro@palves.net>
Simon Marchi <simon.marchi@efficios.com>
PR gdb/27338
* NEWS: Mention stepped-thread-exited feature in qSupported.
* remote.c (PACKET_stepped_thread_exited): New.
(remote_protocol_features): Add "stepped-thread-exited" entry.
(remote_target::remote_query_supported): Report
stepped-thread-exited+.
(remote_target::wait_as): Handle 'w' (aka thread exit) stop reply.
(_initialize_remote): Register "set/show remote
stepped-thread-exited-stop-reply".
gdbserver/ChangeLog:
yyyy-mm-dd Pedro Alves <pedro@palves.net>
PR gdb/27338
* server.cc (handle_query): Handle and report
stepped-thread-exited+.
(resume): Handle TARGET_WAITKIND_THREAD_EXITED in all-stop mode.
gdb/doc/ChangeLog:
yyyy-mm-dd Pedro Alves <pedro@palves.net>
PR gdb/27338
* gdb.texinfo (Remote Configuration): Document
stepped-thread-exited-stop-reply.
(Stop Reply Packets) <w stop reply>: Mention
stepped-threads-exited feature.
(General Query Packets): Document stepped-thread-exited.
Co-Authored-By: Simon Marchi <simon.marchi@efficios.com>
Change-Id: I299f7e73c5702738dfaf5d18bc9e957da473055d
---
gdb/NEWS | 5 +++++
gdb/doc/gdb.texinfo | 28 +++++++++++++++++++++++++++-
gdb/remote.c | 20 +++++++++++++++++++-
gdbserver/server.cc | 28 +++++++++++++++++++++++++++-
gdbserver/target.cc | 6 ++++++
gdbserver/target.h | 7 +++++++
6 files changed, 91 insertions(+), 3 deletions(-)
diff --git a/gdb/NEWS b/gdb/NEWS
index 7f3ed4f02f0..967886c2958 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -239,6 +239,11 @@ QMemTags
Request the remote to store the specified allocation tags to the requested
memory range.
+stepped-thread-exited feature in qSupported
+ Indicates that GDB is expecting thread exit (w) stop replies for
+ threads that were single-stepping, even when QThreadEvents is not in
+ effect.
+
* Guile API
** Improved support for rvalue reference values:
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index f1c3e7ba847..e175ad14fa5 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -23463,6 +23463,10 @@ are:
@tab @code{no resumed thread left stop reply}
@tab Tracking thread lifetime.
+@item @code{stepped-thread-exited-stop-reply}
+@tab @code{stepped thread exited stop reply}
+@tab Tracking thread lifetime.
+
@end multitable
@node Remote Stub
@@ -40953,7 +40957,11 @@ hex strings.
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.
-@var{AA} is formatted as a big-endian hex string.
+If @value{GDBN} reported support for the @samp{stepped-thread-exited}
+@samp{qSupported} feature (@pxref{qSupported}), the remote stub should
+send this response for stepping threads that exit, even if thread
+create/exit events are disabled with @samp{QThreadEvents:0}. @var{AA}
+is formatted as a big-endian hex string.
@item N
There are no resumed threads left in the target. In other words, even
@@ -41870,6 +41878,16 @@ including @samp{exec-events+} in its @samp{qSupported} reply.
@item vContSupported
This feature indicates whether @value{GDBN} wants to know the
supported actions in the reply to @samp{vCont?} packet.
+
+@item stepped-thread-exited
+This feature indicates whether @value{GDBN} expects thread exited
+events for stepping threads, even if @samp{QThreadEvents:1} was not
+requested. This can also be used as indication that @value{GDBN}
+understands the @samp{w} stop reply (@pxref{thread exit event}) in
+all-stop mode. Prior versions of @value{GDBN} considered such a
+response an error. @value{GDBN} does not expect such events unless
+the stub also reports that it reports them by including
+@samp{stepped-thread-exited+} in its @samp{qSupported} reply.
@end table
Stubs should ignore any unknown values for
@@ -42143,6 +42161,11 @@ These are the currently defined stub features and their properties:
@tab @samp{-}
@tab No
+@item @samp{stepped-thread-exited}
+@tab No
+@tab @samp{-}
+@tab No
+
@item @samp{memory-tagging}
@tab No
@tab @samp{-}
@@ -42362,6 +42385,9 @@ The remote stub understands the @samp{QThreadEvents} packet.
@item no-resumed
The remote stub reports the @samp{N} stop reply.
+@item stepped-thread-exited
+The remote stub reports the @samp{w} stop reply for single-stepped
+threads that exit.
@item memory-tagging
The remote stub supports and implements the required memory tagging
diff --git a/gdb/remote.c b/gdb/remote.c
index adc53e324d0..e1162fc3664 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -2180,6 +2180,11 @@ enum {
/* Support TARGET_WAITKIND_NO_RESUMED. */
PACKET_no_resumed,
+ /* Support TARGET_WAITKIND_THREAD_EXITED / 'w' stop replies when
+ single-stepped threads exit. Also indicates that GDB understands
+ 'w' stop replies in the all-stop protocol. */
+ PACKET_stepped_thread_exited,
+
/* Support for memory tagging, allocation tag fetch/store
packets and the tag violation stop replies. */
PACKET_memory_tagging_feature,
@@ -5331,6 +5336,8 @@ static const struct protocol_feature remote_protocol_features[] = {
{ "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 },
+ { "stepped-thread-exited", PACKET_DISABLE, remote_supported_packet,
+ PACKET_stepped_thread_exited },
{ "memory-tagging", PACKET_DISABLE, remote_supported_packet,
PACKET_memory_tagging_feature },
};
@@ -5427,6 +5434,9 @@ remote_target::remote_query_supported ()
if (packet_set_cmd_state (PACKET_no_resumed) != AUTO_BOOLEAN_FALSE)
remote_query_supported_append (&q, "no-resumed+");
+ if (packet_set_cmd_state (PACKET_stepped_thread_exited) != AUTO_BOOLEAN_FALSE)
+ remote_query_supported_append (&q, "stepped-thread-exited+");
+
if (packet_set_cmd_state (PACKET_memory_tagging_feature)
!= AUTO_BOOLEAN_FALSE)
remote_query_supported_append (&q, "memory-tagging+");
@@ -8207,6 +8217,7 @@ remote_target::wait_as (ptid_t ptid, target_waitstatus *status,
status->kind = TARGET_WAITKIND_STOPPED;
status->value.sig = GDB_SIGNAL_0;
break;
+
case 'F': /* File-I/O request. */
/* GDB may access the inferior memory while handling the File-I/O
request, but we don't want GDB accessing memory while waiting
@@ -8219,7 +8230,8 @@ remote_target::wait_as (ptid_t ptid, target_waitstatus *status,
again. Keep waiting for events. */
rs->waiting_for_stop_reply = 1;
break;
- case 'N': case 'T': case 'S': case 'X': case 'W':
+
+ case 'N': case 'T': case 'S': case 'X': case 'W': case 'w':
{
/* There is a stop reply to handle. */
rs->waiting_for_stop_reply = 0;
@@ -8232,9 +8244,11 @@ remote_target::wait_as (ptid_t ptid, target_waitstatus *status,
event_ptid = process_stop_reply (stop_reply, status);
break;
}
+
case 'O': /* Console output. */
remote_console_output (buf + 1);
break;
+
case '\0':
if (rs->last_sent_signal != GDB_SIGNAL_0)
{
@@ -15245,6 +15259,10 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL,
add_packet_config_cmd (&remote_protocol_packets[PACKET_no_resumed],
"N stop reply", "no-resumed-stop-reply", 0);
+ add_packet_config_cmd (&remote_protocol_packets[PACKET_stepped_thread_exited],
+ "w stop reply (after step)",
+ "stepped-thread-exited-stop-reply", 0);
+
add_packet_config_cmd (&remote_protocol_packets[PACKET_memory_tagging_feature],
"memory-tagging-feature", "memory-tagging-feature", 0);
diff --git a/gdbserver/server.cc b/gdbserver/server.cc
index 32dcc05924e..eb4cd89dbad 100644
--- a/gdbserver/server.cc
+++ b/gdbserver/server.cc
@@ -85,6 +85,10 @@ bool run_once;
/* Whether to report TARGET_WAITKIND_NO_RESUMED events. */
static bool report_no_resumed;
+/* Whether to report TARGET_WAITKIND_THREAD_EXITED events when a
+ stepping thread exits. */
+static bool report_stepped_thread_exited;
+
/* The event loop checks this to decide whether to continue accepting
events. */
static bool keep_processing_events = true;
@@ -2398,6 +2402,13 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
events. */
report_no_resumed = true;
}
+ else if (feature == "stepped-thread-exited+")
+ {
+ /* GDB understands & expects
+ TARGET_WAITKIND_THREAD_EXITED for stepping
+ threads that exit. */
+ report_stepped_thread_exited = true;
+ }
else if (feature == "memory-tagging+")
{
/* GDB supports memory tagging features. */
@@ -2521,6 +2532,9 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
strcat (own_buf, ";no-resumed+");
+ if (target_supports_stepped_thread_exited ())
+ strcat (own_buf, ";stepped-thread-exited+");
+
if (target_supports_memory_tagging ())
strcat (own_buf, ";memory-tagging+");
@@ -2954,9 +2968,21 @@ resume (struct thread_resume *actions, size_t num_actions)
return;
}
+ if (cs.last_status.kind == TARGET_WAITKIND_THREAD_EXITED
+ && !report_stepped_thread_exited)
+ {
+ /* report_stepped_thread_exited also indicates whether the
+ client supports the 'w' stop reply in all-stop mode. At
+ least return error. */
+ sprintf (cs.own_buf, "E.Thread exited.");
+ disable_async_io ();
+ return;
+ }
+
if (cs.last_status.kind != TARGET_WAITKIND_EXITED
&& cs.last_status.kind != TARGET_WAITKIND_SIGNALLED
- && cs.last_status.kind != TARGET_WAITKIND_NO_RESUMED)
+ && cs.last_status.kind != TARGET_WAITKIND_NO_RESUMED
+ && cs.last_status.kind != TARGET_WAITKIND_THREAD_EXITED)
current_thread->last_status = cs.last_status;
/* From the client's perspective, all-stop mode always stops all
diff --git a/gdbserver/target.cc b/gdbserver/target.cc
index 6660cc04fd5..2d15a840711 100644
--- a/gdbserver/target.cc
+++ b/gdbserver/target.cc
@@ -470,6 +470,12 @@ process_stratum_target::supports_memory_tagging ()
return false;
}
+bool
+process_stratum_target::supports_stepped_thread_exited ()
+{
+ return false;
+}
+
bool
process_stratum_target::fetch_memtags (CORE_ADDR address, size_t len,
gdb::byte_vector &tags, int type)
diff --git a/gdbserver/target.h b/gdbserver/target.h
index 2c4393ec8c6..862300f72f5 100644
--- a/gdbserver/target.h
+++ b/gdbserver/target.h
@@ -500,6 +500,10 @@ class process_stratum_target
/* Returns true if the target supports memory tagging facilities. */
virtual bool supports_memory_tagging ();
+ /* Returns true if the target supports returning
+ TARGET_WAITKIND_THREAD_EXITED for stepping threads. */
+ virtual bool supports_stepped_thread_exited();
+
/* Return the allocated memory tags of type TYPE associated with
[ADDRESS, ADDRESS + LEN) in TAGS.
@@ -542,6 +546,9 @@ int kill_inferior (process_info *proc);
#define target_supports_memory_tagging() \
the_target->supports_memory_tagging ()
+#define target_supports_stepped_thread_exited() \
+ the_target->supports_stepped_thread_exited ()
+
#define target_handle_new_gdb_connection() \
the_target->handle_new_gdb_connection ()
--
2.26.2
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 04/10] struct resumed_pending_vcont_info -> struct resume_info
2021-07-02 13:01 [PATCH 00/10] Step over thread exit (PR gdb/27338) Pedro Alves
` (2 preceding siblings ...)
2021-07-02 13:01 ` [PATCH 03/10] remote+gdbserver: stepped-thread-exited feature Pedro Alves
@ 2021-07-02 13:01 ` Pedro Alves
2021-07-05 19:46 ` Simon Marchi
2021-07-02 13:01 ` [PATCH 05/10] remote_target::update_thread_list: Don't delete stepping threads Pedro Alves
` (6 subsequent siblings)
10 siblings, 1 reply; 23+ messages in thread
From: Pedro Alves @ 2021-07-02 13:01 UTC (permalink / raw)
To: gdb-patches
The next patch will want to know what was the last resume action done
on a given remote thread. This commit generalizes struct
resumed_pending_vcont_info a bit so that that info can be retrieved in
either RESUMED_PENDING_VCONT or RESUMED state.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <pedro@palves.net>
PR gdb/27338
* remote.c (struct resumed_pending_vcont_info): Rename to ...
(struct resume_info): ... this.
(resumed_pending_vcont_info::set_resumed_pending_vcont): Adjust.
(resumed_pending_vcont_info): Rename to ...
(resumed_pending_vcont_info::resume_info): ... this.
(resumed_pending_vcont_info::set_resumed): Add 'step' and 'sig'
parameters. Use them.
(resumed_pending_vcont_info::m_resumed_pending_vcont_info): Rename
to ...
(resumed_pending_vcont_info::m_resume_info): ... this.
(remote_target::remote_add_thread, remote_target::start_remote):
Adjust.
(remote_target::resume): Record step and siggnal for the main
resumed thread.
(remote_target::commit_resumed): Adjust.
(remote_target::remote_stop_ns): Adjust.
Change-Id: Ic1f884dd822f5bbc5c24698ec02c13dadb7773c8
---
gdb/remote.c | 107 +++++++++++++++++++++++++++------------------------
1 file changed, 56 insertions(+), 51 deletions(-)
diff --git a/gdb/remote.c b/gdb/remote.c
index e1162fc3664..179095a842f 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1084,12 +1084,10 @@ enum class resume_state
RESUMED,
};
-/* Information about a thread's pending vCont-resume. Used when a thread is in
- the remote_resume_state::RESUMED_PENDING_VCONT state. remote_target::resume
- stores this information which is then picked up by
- remote_target::commit_resume to know which is the proper action for this
- thread to include in the vCont packet. */
-struct resumed_pending_vcont_info
+/* Information about a thread's resume state. Used when a thread is
+ in the resume_state::RESUMED_PENDING_VCONT or resume_state::RESUMED
+ states. */
+struct resume_info
{
/* True if the last resume call for this thread was a step request, false
if a continue request. */
@@ -1133,45 +1131,54 @@ struct remote_thread_info : public private_thread_info
void set_resumed_pending_vcont (bool step, gdb_signal sig)
{
m_resume_state = resume_state::RESUMED_PENDING_VCONT;
- m_resumed_pending_vcont_info.step = step;
- m_resumed_pending_vcont_info.sig = sig;
- }
-
- /* Get the information this thread's pending vCont-resumption.
-
- Must only be called if the thread is in the RESUMED_PENDING_VCONT resume
- state. */
- const struct resumed_pending_vcont_info &resumed_pending_vcont_info () const
- {
- gdb_assert (m_resume_state == resume_state::RESUMED_PENDING_VCONT);
-
- return m_resumed_pending_vcont_info;
+ m_resume_info.step = step;
+ m_resume_info.sig = sig;
}
/* Put the thread in the VCONT_RESUMED state. */
- void set_resumed ()
+ void set_resumed (bool step, gdb_signal sig)
{
m_resume_state = resume_state::RESUMED;
+ m_resume_info.step = step;
+ m_resume_info.sig = sig;
}
-private:
- /* Resume state for this thread. This is used to implement vCont action
- coalescing (only when the target operates in non-stop mode).
+ /* Get the information this thread's pending vCont-resumption.
- remote_target::resume moves the thread to the RESUMED_PENDING_VCONT state,
- which notes that this thread must be considered in the next commit_resume
- call.
+ Must only be called if the thread is in the RESUMED_PENDING_VCONT
+ or RESUMED states. */
+ const struct resume_info &resume_info () const
+ {
+ gdb_assert (m_resume_state == resume_state::RESUMED_PENDING_VCONT
+ || m_resume_state == resume_state::RESUMED);
- remote_target::commit_resume sends a vCont packet with actions for the
- threads in the RESUMED_PENDING_VCONT state and moves them to the
- VCONT_RESUMED state.
+ return m_resume_info;
+ }
- When reporting a stop to the core for a thread, that thread is moved back
- to the NOT_RESUMED state. */
+private:
+ /* Resume state for this thread. This is used to implement vCont
+ action coalescing (only when the target operates in non-stop
+ mode), and also generally be able to retrieve the last resume
+ action on a given thread.
+
+ In non-stop mode, remote_target::resume moves the thread to the
+ RESUMED_PENDING_VCONT state, which notes that this thread must be
+ considered in the next commit_resume call. In all-stop,
+ remote_target::resume resumes the thread immediately and moves it
+ to the RESUMED state.
+
+ In non-stop mode, remote_target::commit_resume sends a vCont
+ packet with actions for the threads in the RESUMED_PENDING_VCONT
+ state and moves them to the VCONT_RESUMED state.
+ remote_target::commit_resume is a no-op in all-stop mode.
+
+ When reporting a stop to the core for a thread, that thread is
+ moved back to the NOT_RESUMED state. */
enum resume_state m_resume_state = resume_state::NOT_RESUMED;
- /* Extra info used if the thread is in the RESUMED_PENDING_VCONT state. */
- struct resumed_pending_vcont_info m_resumed_pending_vcont_info;
+ /* Extra info used if the thread is in the RESUMED_PENDING_VCONT or
+ RESUMED state. */
+ struct resume_info m_resume_info;
};
remote_state::remote_state ()
@@ -2548,7 +2555,7 @@ remote_target::remote_add_thread (ptid_t ptid, bool running, bool executing)
/* We start by assuming threads are resumed. That state then gets updated
when we process a matching stop reply. */
- get_remote_thread_info (thread)->set_resumed ();
+ get_remote_thread_info (thread)->set_resumed (false, GDB_SIGNAL_0);
set_executing (this, ptid, executing);
set_running (this, ptid, running);
@@ -4835,7 +4842,7 @@ remote_target::start_remote (int from_tty, int extended_p)
non-threaded target as single-threaded; add a main
thread. */
thread_info *tp = add_current_inferior_and_thread (wait_status);
- get_remote_thread_info (tp)->set_resumed ();
+ get_remote_thread_info (tp)->set_resumed (false, GDB_SIGNAL_0);
}
else
{
@@ -6426,6 +6433,13 @@ remote_target::resume (ptid_t ptid, int step, enum gdb_signal siggnal)
{
struct remote_state *rs = get_remote_state ();
+ remote_thread_info *remote_thr;
+
+ if (minus_one_ptid == ptid || ptid.is_pid ())
+ remote_thr = get_remote_thread_info (this, inferior_ptid);
+ else
+ remote_thr = get_remote_thread_info (this, ptid);
+
/* When connected in non-stop mode, the core resumes threads
individually. Resuming remote threads directly in target_resume
would thus result in sending one packet per thread. Instead, to
@@ -6435,13 +6449,6 @@ remote_target::resume (ptid_t ptid, int step, enum gdb_signal siggnal)
able to do vCont action coalescing. */
if (target_is_non_stop_p () && ::execution_direction != EXEC_REVERSE)
{
- remote_thread_info *remote_thr;
-
- if (minus_one_ptid == ptid || ptid.is_pid ())
- remote_thr = get_remote_thread_info (this, inferior_ptid);
- else
- remote_thr = get_remote_thread_info (this, ptid);
-
/* We don't expect the core to ask to resume an already resumed (from
its point of view) thread. */
gdb_assert (remote_thr->get_resume_state () == resume_state::NOT_RESUMED);
@@ -6467,7 +6474,8 @@ remote_target::resume (ptid_t ptid, int step, enum gdb_signal siggnal)
/* Update resumed state tracked by the remote target. */
for (thread_info *tp : all_non_exited_threads (this, ptid))
- get_remote_thread_info (tp)->set_resumed ();
+ get_remote_thread_info (tp)->set_resumed (false, GDB_SIGNAL_0);
+ remote_thr->set_resumed (step, siggnal);
/* We are about to start executing the inferior, let's register it
with the event loop. NOTE: this is the one place where all the
@@ -6777,8 +6785,7 @@ remote_target::commit_resumed ()
for (const auto &stop_reply : rs->stop_reply_queue)
gdb_assert (stop_reply->ptid != tp->ptid);
- const resumed_pending_vcont_info &info
- = remote_thr->resumed_pending_vcont_info ();
+ const resume_info &info = remote_thr->resume_info ();
/* Check if we need to send a specific action for this thread. If not,
it will be included in a wildcard resume instead. */
@@ -6786,7 +6793,7 @@ remote_target::commit_resumed ()
|| !get_remote_inferior (tp->inf)->may_wildcard_vcont)
vcont_builder.push_action (tp->ptid, info.step, info.sig);
- remote_thr->set_resumed ();
+ remote_thr->set_resumed (info.step, info.sig);
}
/* Now check whether we can send any process-wide wildcard. This is
@@ -6874,8 +6881,7 @@ remote_target::remote_stop_ns (ptid_t ptid)
if (remote_thr->get_resume_state ()
== resume_state::RESUMED_PENDING_VCONT)
{
- const resumed_pending_vcont_info &info
- = remote_thr->resumed_pending_vcont_info ();
+ const resume_info &info = remote_thr->resume_info ();
if (info.sig != GDB_SIGNAL_0)
{
/* This signal must be forwarded to the inferior. We
@@ -6904,8 +6910,7 @@ remote_target::remote_stop_ns (ptid_t ptid)
/* Check that the thread wasn't resumed with a signal.
Generating a phony stop would result in losing the
signal. */
- const resumed_pending_vcont_info &info
- = remote_thr->resumed_pending_vcont_info ();
+ const resume_info &info = remote_thr->resume_info ();
gdb_assert (info.sig == GDB_SIGNAL_0);
stop_reply *sr = new stop_reply ();
@@ -6926,7 +6931,7 @@ remote_target::remote_stop_ns (ptid_t ptid)
queue, we'll end up reporting a stop event to the core
for that thread while it is running on the remote
target... that would be bad. */
- remote_thr->set_resumed ();
+ remote_thr->set_resumed (false, GDB_SIGNAL_0);
}
}
--
2.26.2
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 05/10] remote_target::update_thread_list: Don't delete stepping threads
2021-07-02 13:01 [PATCH 00/10] Step over thread exit (PR gdb/27338) Pedro Alves
` (3 preceding siblings ...)
2021-07-02 13:01 ` [PATCH 04/10] struct resumed_pending_vcont_info -> struct resume_info Pedro Alves
@ 2021-07-02 13:01 ` Pedro Alves
2021-07-05 19:55 ` Simon Marchi
2021-07-02 13:01 ` [PATCH 06/10] stop_all_threads: (re-)enable async before waiting for stops Pedro Alves
` (5 subsequent siblings)
10 siblings, 1 reply; 23+ messages in thread
From: Pedro Alves @ 2021-07-02 13:01 UTC (permalink / raw)
To: gdb-patches
Without this patch, the
gdb.threads/step-over-thread-exit-while-stop-all-threads.exp testcase
added later in the series would randomly fail against gdbserver. It
would hit this assertion failure:
src/gdb/displaced-stepping.c:54: internal-error: displaced_step_prepare_status displaced_step_buffers::prepare(thread_info*, CORE_ADDR&): Assertion `buf.current_thread != thread' failed.
That assertion was a bit of a head scratcher. Turns out that is was
caused by a 'buf.current_thread' being a stale pointer to a thread
that had already been deleted. I confirmed that this is what was
happening with the following hack:
--- c/gdb/displaced-stepping.c
+++ w/gdb/displaced-stepping.c
@@ -145,6 +145,7 @@ displaced_step_buffers::prepare (thread_info *thread, CORE_ADDR &displaced_pc)
/* This marks the buffer as being in use. */
buffer->current_thread = thread;
+ buffer->current_thread_id = thread->global_num;
When the assertion triggers, I had for example:
(top-gdb) p buf.current_thread_id
$1 = 205
(top-gdb) p thread->global_num
$2 = 308
So, same pointer, but different threads.
Here's how it happens:
While GDB is stopping threads with stop_all_threads, it updates the
thread list, which reaches remote_target::update_thread_list. Any
thread that is no longer found in the remote end is removed from GDB's
thread list by that function. If the thread that is gone was one that
was stepping over a breakpoint set on top of a thread exit syscall,
then update_thread_list deletes the thread straight away, which leaves
the displaced stepping buffer with a stale pointer to the thread.
Later, when a new thread_info object is created (the testcase
continuously spawns threads), the heap may well return the same memory
address for the new object as the displaced stepping buffer's stale
pointer still points to. That is what happened in the assertion
triggered above -- in reality the buffer's 'buf.current_thread'
conceptually pointed to a different thread not 'thread', but in
practice, the pointers had the same value.
What we really want to do is release the displaced stepping buffer, so
some other thread can reuse it, and we want to let another thread have
its turn at stepping over a breakpoint. The simplest way is to make
update_thread_list not delete threads that are stepping, as we know
that we will get a 'w' (TARGET_WAITKIND_THREAD_EXITED) stop reply for
them shortly, so that infrun takes care of all that.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <pedro@palves.net>
PR gdb/27338
* remote.c (remote_target::update_thread_list): Don't delete
missing threads that were stepping.
Change-Id: I4a8687b48da8bf1d9cc23981c427722dd63fecb6
---
gdb/remote.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/gdb/remote.c b/gdb/remote.c
index 179095a842f..71df343120b 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -3943,6 +3943,8 @@ remote_target::update_thread_list ()
if (!context.contains_thread (tp->ptid))
{
+ /* Not found. */
+
/* Do not remove the thread if it is the last thread in
the inferior. This situation happens when we have a
pending exit process status to process. Otherwise we
@@ -3951,7 +3953,22 @@ remote_target::update_thread_list ()
if (has_single_non_exited_thread (tp->inf))
continue;
- /* Not found. */
+ /* If the thread was stepping, then don't delete it yet,
+ let infrun process the thread exit event which should
+ come shortly. If this thread was stepping over a
+ breakpoint, we want infrun to see the exit so it can
+ free the displaced stepping buffer (if used), and
+ maybe start a new step-over in another thread (if any
+ is waiting). */
+ remote_thread_info *remote_thr = get_remote_thread_info (tp);
+ if (packet_support (PACKET_stepped_thread_exited) == PACKET_ENABLE
+ && remote_thr->get_resume_state () != resume_state::NOT_RESUMED
+ && remote_thr->resume_info ().step)
+ continue;
+
+ /* The core doesn't particularly care about the thread,
+ and a THREAD_EXITED event is likely not coming. Go
+ ahead and delete it immediately. */
delete_thread (tp);
}
}
--
2.26.2
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 06/10] stop_all_threads: (re-)enable async before waiting for stops
2021-07-02 13:01 [PATCH 00/10] Step over thread exit (PR gdb/27338) Pedro Alves
` (4 preceding siblings ...)
2021-07-02 13:01 ` [PATCH 05/10] remote_target::update_thread_list: Don't delete stepping threads Pedro Alves
@ 2021-07-02 13:01 ` Pedro Alves
2021-07-05 21:32 ` Simon Marchi
2021-07-02 13:01 ` [PATCH 07/10] Tweak gdbserver's no-resumed handling Pedro Alves
` (4 subsequent siblings)
10 siblings, 1 reply; 23+ messages in thread
From: Pedro Alves @ 2021-07-02 13:01 UTC (permalink / raw)
To: gdb-patches
Running the gdb.threads/interrupt-while-step-over.exp testcase added
later in the series against gdbserver, after the
TARGET_WAITKIND_NO_RESUMED fix from the following patch would run into
an infinite loop in stop_all_threads. I added a hacky gdb_assert to
catch the loop, and it looked like this:
[infrun] stop_all_threads: Thread 3492141.3492141 not executing
[infrun] stop_all_threads: Thread 3492141.3492716 not executing
[infrun] stop_all_threads: Thread 3492141.3492717 not executing
[infrun] stop_all_threads: Thread 3492141.3492718 not executing
[infrun] stop_all_threads: Thread 3492141.3492719 not executing
[infrun] stop_all_threads: Thread 3492141.3492720 executing, need stop
[remote] stop: enter
[remote] remote_stop_ns: Enqueueing phony stop reply for thread pending vCont-resume (3492141, 3492720, 0)
[remote] stop: exit
[infrun] stop_all_threads: Thread 3492141.3492721 not executing
[infrun] stop_all_threads: Thread 3492141.3492722 not executing
[infrun] stop_all_threads: Thread 3492141.3492723 not executing
[infrun] stop_all_threads: Thread 3492141.3492724 not executing
[infrun] stop_all_threads: Thread 3492141.3492725 not executing
[infrun] stop_all_threads: Thread 3492141.3492726 not executing
[infrun] stop_all_threads: Thread 3492141.3492727 not executing
[infrun] stop_all_threads: Thread 3492141.3492728 not executing
[infrun] stop_all_threads: Thread 3492141.3492729 not executing
[infrun] stop_all_threads: Thread 3492141.3492730 not executing
[infrun] stop_all_threads: Thread 3492141.3492731 not executing
[infrun] stop_all_threads: Thread 3492141.3492732 not executing
[infrun] stop_all_threads: Thread 3492141.3492733 not executing
[infrun] stop_all_threads: Thread 3492141.3492734 not executing
[infrun] stop_all_threads: Thread 3492141.3492735 not executing
[remote] wait: enter
[remote] wait: exit
[infrun] print_target_wait_results: target_wait (-1.0.0 [process -1], status) =
[infrun] print_target_wait_results: -1.0.0 [process -1],
[infrun] print_target_wait_results: status->kind = no-resumed
[infrun] infrun_async: enable=0
[infrun] handle_one: status->kind = no-resumed process -1
[remote] Sending packet: $qXfer:threads:read::0,1000#92
[remote] Packet received: l<threads>\n<thread id="p35492d.35492d" core="16" name="interrupt-while" handle="40d7d8f7ff7f0000"/>\n<thread id="p35492d.354b6c" core="12" name="interrupt-while" handle="00c7d8f7ff7f0000"/>\n<thread id="p35492d.354b6d" core="5" name="interrupt-while" handle="00b758f7ff7f0000"/>\n<thread id="p35492d.354b6e" core="19" name="interrupt-while" handle="00a7d8f6ff7f0000"/>\n<thread id="p35492d.354b6f" core="15" name="interrupt-while" handle="009758f6ff7f0000"/>\n<thread id="p35492d.354b70" core="6" name="interrupt-whil [1370 bytes omitted]
[infrun] stop_all_threads: Thread 3492141.3492141 not executing
[infrun] stop_all_threads: Thread 3492141.3492716 not executing
[infrun] stop_all_threads: Thread 3492141.3492717 not executing
[infrun] stop_all_threads: Thread 3492141.3492718 not executing
[infrun] stop_all_threads: Thread 3492141.3492719 not executing
[infrun] stop_all_threads: Thread 3492141.3492720 executing, already stopping
[infrun] stop_all_threads: Thread 3492141.3492721 not executing
[infrun] stop_all_threads: Thread 3492141.3492722 not executing
[infrun] stop_all_threads: Thread 3492141.3492723 not executing
[infrun] stop_all_threads: Thread 3492141.3492724 not executing
[infrun] stop_all_threads: Thread 3492141.3492725 not executing
[infrun] stop_all_threads: Thread 3492141.3492726 not executing
[infrun] stop_all_threads: Thread 3492141.3492727 not executing
[infrun] stop_all_threads: Thread 3492141.3492728 not executing
[infrun] stop_all_threads: Thread 3492141.3492729 not executing
[infrun] stop_all_threads: Thread 3492141.3492730 not executing
[infrun] stop_all_threads: Thread 3492141.3492731 not executing
[infrun] stop_all_threads: Thread 3492141.3492732 not executing
[infrun] stop_all_threads: Thread 3492141.3492733 not executing
[infrun] stop_all_threads: Thread 3492141.3492734 not executing
[infrun] stop_all_threads: Thread 3492141.3492735 not executing
../../src/gdb/infrun.c:5066: internal-error: void stop_all_threads(): Assertion `0' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Quit this debugging session? (y or n)
What happened above was that gdbserver had sent a
TARGET_WAITKIND_NO_RESUMED stop reply (N) to GDB already, and that
event was still in transit, held in remote.c's stop reply queue, not
yet seen by infrun. In stop_all_threads, GDB told remote.c to stop
thread 3492141.3492720. That thread had a pending vCont resume, so
remote.c queued a phony stop. This event is queued _after_ the
TARGET_WAITKIND_NO_RESUMED stop reply that was already there.
stop_all_threads calls wait_one to collect one event and gets back
that TARGET_WAITKIND_NO_RESUMED. Seeing TARGET_WAITKIND_NO_RESUMED,
wait_one disables target_async for that target to stop listing to
events from that target. No other unresumed target has async active,
so wait_one returns TARGET_WAITKIND_NO_RESUMED. stop_all_threads
still hasn't seen the stop for 3492141.3492720, so we go for another
iteration. But since the target is not async, wait_one returns
TARGET_WAITKIND_NO_RESUMED without calling target_wait. The queued
event for 3492141.3492720 remains pending forever, and
stop_all_threads keeps iterating forever.
The comment in the patch describes a different scenario that does not
involve the phony stop:
/* wait_one waits for events until it sees a
TARGET_WAITKIND_NO_RESUMED. When it sees one, it disables
target_async for the target and no longer waits for events from
that target. TARGET_WAITKIND_NO_RESUMED can be delayed though,
consider:
#1 - threads 2-5 are stopped, thread 1 is running
#2 - target reports breakpoint hit for thread 1, event
is queued.
#3 - target reports no-resumed left, event is queued
#4 - user resumes all threads
#5 - gdb decides to stop all threads, stops threads 1-5
#6 - wait_one returns the queued breakpoint hit for
thread 1
#7 - wait_one returns the queued no-resumed event,
wait_one stops waiting for events from target.
#8 - we still haven't seen the stops for threads 2-5, so
we do another pass.
#9 - if the target is not async wait_one doesn't wait on
the target, so it won't see the stops.
#a - we'd loop forever with WAITS_NEEDED > 0.
Fix this by explicitly enabling target async on each iteration, before
starting the wait_one loop.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <pedro@palves.net>
PR gdb/27338
* infrun.c (reenable_target_async): New.
(stop_all_threads): Enable/re-enable async on targets that can
async before waiting for stops.
Change-Id: Ie3ffb0df89635585a6631aa842689cecc989e33f
---
gdb/infrun.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 51 insertions(+)
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 53331f27a0a..a2d63c4fe89 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -4961,6 +4961,55 @@ handle_one (const wait_one_event &event)
return false;
}
+/* Helper for stop_all_threads. wait_one waits for events until it
+ sees a TARGET_WAITKIND_NO_RESUMED. When it sees one, it disables
+ target_async for the target and no longer waits for events from
+ that target. TARGET_WAITKIND_NO_RESUMED can be delayed though,
+ consider:
+
+ #1 - threads 2-5 are stopped, thread 1 is running
+
+ #2 - target reports breakpoint hit for thread 1, event is queued
+
+ #3 - target reports no-resumed left, event is queued
+
+ #4 - user resumes all threads
+
+ #5 - gdb decides to stop all threads, stops threads 1-5
+
+ #6 - wait_one returns the queued breakpoint hit for thread 1
+
+ #7 - wait_one returns the queued no-resumed event, wait_one stops
+ waiting for events from target
+
+ #8 - we still haven't seen the stops for threads 2-5, so we do
+ another pass
+
+ #9 - if the target is not async wait_one doesn't wait on the
+ target, so it won't see the stops.
+
+ #a - we'd loop forever with WAITS_NEEDED > 0.
+
+ To handle this, we need to explicitly (re-)enable target async on
+ all targets that can async. */
+
+static void
+reenable_target_async ()
+{
+ for (inferior *inf : all_inferiors ())
+ {
+ process_stratum_target *target = inf->process_target ();
+ if (target != nullptr
+ && target->threads_executing
+ && target->can_async_p ()
+ && !target->is_async_p ())
+ {
+ switch_to_inferior_no_thread (inf);
+ target_async (1);
+ }
+ }
+}
+
/* See infrun.h. */
void
@@ -5071,6 +5120,8 @@ stop_all_threads (void)
if (pass > 0)
pass = -1;
+ reenable_target_async ();
+
for (int i = 0; i < waits_needed; i++)
{
wait_one_event event = wait_one ();
--
2.26.2
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 07/10] Tweak gdbserver's no-resumed handling
2021-07-02 13:01 [PATCH 00/10] Step over thread exit (PR gdb/27338) Pedro Alves
` (5 preceding siblings ...)
2021-07-02 13:01 ` [PATCH 06/10] stop_all_threads: (re-)enable async before waiting for stops Pedro Alves
@ 2021-07-02 13:01 ` Pedro Alves
2021-07-05 21:51 ` Simon Marchi
2021-07-02 13:01 ` [PATCH 08/10] Implement stepped-thread-exited feature on native+gdbserver Linux Pedro Alves
` (3 subsequent siblings)
10 siblings, 1 reply; 23+ messages in thread
From: Pedro Alves @ 2021-07-02 13:01 UTC (permalink / raw)
To: gdb-patches
If we make gdbserver report TARGET_WAITKIND_THREAD_EXITED for threads
that were stepping when they exit, we regress the
TARGET_WAITKIND_NO_RESUMED support.
The issue is that gdbserver no longer sends TARGET_WAITKIND_NO_RESUMED
if the last resumed thread exits, and, it was stepping. We report the
TARGET_WAITKIND_THREAD_EXITED, and then the next time around, all
threads are already not-resumed when we get to
linux_process_target::wait_1's entry, so the 'any_resumed' variable is
set to false.
This is exposed by the new gdb.threads/step-over-thread-exit.exp
testcase at the end of this series:
Thread 2 "step-over-threa" hit Breakpoint 2, my_exit_syscall () at /home/pedro/rocm/gdb/build/gdb/../../src/gdb/testsuite/lib/my-syscalls.S:68
68 syscall
(gdb) PASS: gdb.threads/step-over-thread-exit.exp: displaced-stepping=off: target-non-stop=on: schedlock=on: continue until syscall
continue
Continuing.
FAIL: gdb.threads/step-over-thread-exit.exp: displaced-stepping=off: target-non-stop=on: schedlock=on: continue stops when thread exits (timeout)
A fixed GDB shows instead:
(gdb) PASS: gdb.threads/step-over-thread-exit.exp: displaced-stepping=off: target-non-stop=on: schedlock=on: continue until syscall
continue
Continuing.
No unwaited-for children left.
(gdb) PASS: gdb.threads/step-over-thread-exit.exp: displaced-stepping=off: target-non-stop=on: schedlock=on: continue stops when thread exits
Fix this by changing gdbserver to check whether any thread is resumed
after calling wait_for_event instead of before. This actually makes
it a bit more similar to native gdb.
While at it, merge the two callback functions into one so that we only
have to iterate the thread list once (one find_thread call).
gdbserver/ChangeLog:
yyyy-mm-dd Pedro Alves <pedro@palves.net>
PR gdb/27338
* linux-low.cc (linux_process_target::wait_1): Delete
'any_resumed' local. Check whether any thread is resumed after
calling wait_for_event, instead of before.
Change-Id: Ie4969f338b10a6fa6e99162255d158220a6d8fc4
---
gdbserver/linux-low.cc | 34 +++++++++++++++-------------------
1 file changed, 15 insertions(+), 19 deletions(-)
diff --git a/gdbserver/linux-low.cc b/gdbserver/linux-low.cc
index 5c6191d941c..26693e2dc48 100644
--- a/gdbserver/linux-low.cc
+++ b/gdbserver/linux-low.cc
@@ -2953,7 +2953,6 @@ linux_process_target::wait_1 (ptid_t ptid, target_waitstatus *ourstatus,
int report_to_gdb;
int trace_event;
int in_step_range;
- int any_resumed;
if (debug_threads)
{
@@ -2971,23 +2970,11 @@ linux_process_target::wait_1 (ptid_t ptid, target_waitstatus *ourstatus,
in_step_range = 0;
ourstatus->kind = TARGET_WAITKIND_IGNORE;
- auto status_pending_p_any = [&] (thread_info *thread)
- {
- return status_pending_p_callback (thread, minus_one_ptid);
- };
-
- auto not_stopped = [&] (thread_info *thread)
- {
- return not_stopped_callback (thread, minus_one_ptid);
- };
-
- /* Find a resumed LWP, if any. */
- if (find_thread (status_pending_p_any) != NULL)
- any_resumed = 1;
- else if (find_thread (not_stopped) != NULL)
- any_resumed = 1;
- else
- any_resumed = 0;
+ auto any_resumed = [&] (thread_info *thread)
+ {
+ return (status_pending_p_callback (thread, minus_one_ptid)
+ || not_stopped_callback (thread, minus_one_ptid));
+ };
if (step_over_bkpt == null_ptid)
pid = wait_for_event (ptid, &w, options);
@@ -2999,7 +2986,16 @@ linux_process_target::wait_1 (ptid_t ptid, target_waitstatus *ourstatus,
pid = wait_for_event (step_over_bkpt, &w, options & ~WNOHANG);
}
- if (pid == 0 || (pid == -1 && !any_resumed))
+ if (pid == -1)
+ {
+ /* Check if we have any resumed LWP. If not, then report
+ TARGET_WAITKIND_NO_RESUMED. If we do, ignore the -1
+ return. */
+ if (find_thread (any_resumed) != nullptr)
+ pid = 0;
+ }
+
+ if (pid == 0)
{
gdb_assert (target_options & TARGET_WNOHANG);
--
2.26.2
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 08/10] Implement stepped-thread-exited feature on native+gdbserver Linux
2021-07-02 13:01 [PATCH 00/10] Step over thread exit (PR gdb/27338) Pedro Alves
` (6 preceding siblings ...)
2021-07-02 13:01 ` [PATCH 07/10] Tweak gdbserver's no-resumed handling Pedro Alves
@ 2021-07-02 13:01 ` Pedro Alves
2021-07-02 13:01 ` [PATCH 09/10] lib/my-syscalls.S: Refactor new SYSCALL macro Pedro Alves
` (2 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: Pedro Alves @ 2021-07-02 13:01 UTC (permalink / raw)
To: gdb-patches
From: Simon Marchi <simon.marchi@efficios.com>
gdb/ChangeLog:
yyyy-mm-dd Simon Marchi <simon.marchi@efficios.com>
Pedro Alves <pedro@palves.net>
PR gdb/27338
* linux-nat.c (exit_lwp): Add del_thread parameter and handle it.
(wait_lwp): Adjust.
(linux_nat_filter_event): Report thread exit event if thread was
stepping.
(check_zombie_leaders): Adjust.
(filter_exit_event): Report thread exit event if thread was
stepping. If so, don't delete thread_info associated to thread.
(linux_nat_wait_1): Pass last_resume_kind to filter_exit_event.
gdbserver/ChangeLog:
yyyy-mm-dd Simon Marchi <simon.marchi@efficios.com>
Pedro Alves <pedro@palves.net>
PR gdb/27338
* linux-low.cc (linux_process_target::filter_event): Report thread
exit event if thread was stepping from gdb's perspective.
(linux_process_target::supports_stepped_thread_exited): New.
* linux-low.h
(linux_process_target::supports_stepped_thread_exited): Declare.
Change-Id: Id9ea2d512631972017bb89e21e447cfcd04f7017
Co-authored-by: Pedro Alves <pedro@palves.net>
---
gdb/linux-nat.c | 41 ++++++++++++++++++++++++++++-------------
gdbserver/linux-low.cc | 10 +++++++++-
gdbserver/linux-low.h | 2 ++
3 files changed, 39 insertions(+), 14 deletions(-)
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index f206b874929..0da0b6200b9 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -1001,10 +1001,13 @@ linux_nat_switch_fork (ptid_t new_ptid)
registers_changed ();
}
-/* Handle the exit of a single thread LP. */
+/* Handle the exit of a single thread LP.
+
+ If DEL_THREAD is true, delete the thread_info associated to LP, if
+ it exists. */
static void
-exit_lwp (struct lwp_info *lp)
+exit_lwp (struct lwp_info *lp, bool del_thread)
{
struct thread_info *th = find_thread_ptid (linux_target, lp->ptid);
@@ -1014,7 +1017,8 @@ exit_lwp (struct lwp_info *lp)
printf_unfiltered (_("[%s exited]\n"),
target_pid_to_str (lp->ptid).c_str ());
- delete_thread (th);
+ if (del_thread)
+ delete_thread (th);
}
delete_lwp (lp->ptid);
@@ -2188,7 +2192,7 @@ wait_lwp (struct lwp_info *lp)
if (thread_dead)
{
- exit_lwp (lp);
+ exit_lwp (lp, true);
return 0;
}
@@ -2925,6 +2929,7 @@ linux_nat_filter_event (int lwpid, int status)
if (WIFEXITED (status) || WIFSIGNALED (status))
{
if (!report_thread_events
+ && lp->last_resume_kind != resume_step
&& num_lwps (lp->ptid.pid ()) > 1)
{
linux_nat_debug_printf ("%s exited.",
@@ -2933,7 +2938,7 @@ linux_nat_filter_event (int lwpid, int status)
/* If there is at least one more LWP, then the exit signal
was not the end of the debugged application and should be
ignored. */
- exit_lwp (lp);
+ exit_lwp (lp, true);
return;
}
@@ -3100,7 +3105,7 @@ check_zombie_leaders (void)
thread execs). */
linux_nat_debug_printf ("Thread group leader %d vanished.", inf->pid);
- exit_lwp (leader_lp);
+ exit_lwp (leader_lp, true);
}
}
}
@@ -3112,18 +3117,28 @@ check_zombie_leaders (void)
static ptid_t
filter_exit_event (struct lwp_info *event_child,
- struct target_waitstatus *ourstatus)
+ struct target_waitstatus *ourstatus,
+ resume_kind last_resume_kind)
{
ptid_t ptid = event_child->ptid;
if (num_lwps (ptid.pid ()) > 1)
{
- if (report_thread_events)
- ourstatus->kind = TARGET_WAITKIND_THREAD_EXITED;
+ /* If we decide to report the thread exited event to the core, we must
+ not delete the thread_info structure ourselves. */
+ if (report_thread_events
+ || last_resume_kind == resume_step)
+ {
+ ourstatus->kind = TARGET_WAITKIND_THREAD_EXITED;
+ /* Delete lwp, but not thread_info, infrun will need it to process
+ the event. */
+ exit_lwp (event_child, false);
+ }
else
- ourstatus->kind = TARGET_WAITKIND_IGNORE;
-
- exit_lwp (event_child);
+ {
+ ourstatus->kind = TARGET_WAITKIND_IGNORE;
+ exit_lwp (event_child, true);
+ }
}
return ptid;
@@ -3346,7 +3361,7 @@ linux_nat_wait_1 (ptid_t ptid, struct target_waitstatus *ourstatus,
lp->core = linux_common_core_of_thread (lp->ptid);
if (ourstatus->kind == TARGET_WAITKIND_EXITED)
- return filter_exit_event (lp, ourstatus);
+ return filter_exit_event (lp, ourstatus, last_resume_kind);
return lp->ptid;
}
diff --git a/gdbserver/linux-low.cc b/gdbserver/linux-low.cc
index 26693e2dc48..4f08bb7a109 100644
--- a/gdbserver/linux-low.cc
+++ b/gdbserver/linux-low.cc
@@ -2312,6 +2312,7 @@ linux_process_target::filter_event (int lwpid, int wstat)
not the end of the debugged application and should be
ignored, unless GDB wants to hear about thread exits. */
if (cs.report_thread_events
+ || thread->last_resume_kind == resume_step
|| last_thread_of_process_p (pid_of (thread)))
{
/* Since events are serialized to GDB core, and we can't
@@ -2895,7 +2896,8 @@ linux_process_target::filter_exit_event (lwp_info *event_child,
if (!last_thread_of_process_p (pid_of (thread)))
{
- if (cs.report_thread_events)
+ if (cs.report_thread_events
+ || thread->last_resume_kind == resume_step)
ourstatus->kind = TARGET_WAITKIND_THREAD_EXITED;
else
ourstatus->kind = TARGET_WAITKIND_IGNORE;
@@ -7125,6 +7127,12 @@ linux_process_target::thread_handle (ptid_t ptid, gdb_byte **handle,
}
#endif
+bool
+linux_process_target::supports_stepped_thread_exited ()
+{
+ return true;
+}
+
/* Default implementation of linux_target_ops method "set_pc" for
32-bit pc register which is literally named "pc". */
diff --git a/gdbserver/linux-low.h b/gdbserver/linux-low.h
index d59ad386bd5..712d273223e 100644
--- a/gdbserver/linux-low.h
+++ b/gdbserver/linux-low.h
@@ -317,6 +317,8 @@ class linux_process_target : public process_stratum_target
visibility because proc-service uses it. */
virtual const regs_info *get_regs_info () = 0;
+ bool supports_stepped_thread_exited () override;
+
private:
/* Handle a GNU/Linux extended wait response. If we see a clone,
--
2.26.2
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 09/10] lib/my-syscalls.S: Refactor new SYSCALL macro
2021-07-02 13:01 [PATCH 00/10] Step over thread exit (PR gdb/27338) Pedro Alves
` (7 preceding siblings ...)
2021-07-02 13:01 ` [PATCH 08/10] Implement stepped-thread-exited feature on native+gdbserver Linux Pedro Alves
@ 2021-07-02 13:01 ` Pedro Alves
2021-07-05 22:11 ` Simon Marchi
2021-07-02 13:01 ` [PATCH 10/10] Testcases for stepping over thread exit syscall (PR gdb/27338) Pedro Alves
2021-07-05 8:42 ` [PATCH 00/10] Step over thread exit " Andrew Burgess
10 siblings, 1 reply; 23+ messages in thread
From: Pedro Alves @ 2021-07-02 13:01 UTC (permalink / raw)
To: gdb-patches
Move the syscall assembly code behind a macro so that it's easy to add
new syscalls without duplicating code.
gdb/testsuite/ChangeLog:
PR gdb/27338
* lib/my-syscalls.S (SYSCALL): New macro, refactored from ...
(my_execve, my_execve_syscall): ... these. Redefine using
SYSCALL.
Change-Id: I8acf1463b11a084d6b4579aaffb49b5d0dea3bba
---
gdb/testsuite/lib/my-syscalls.S | 46 +++++++++++++++++++--------------
1 file changed, 27 insertions(+), 19 deletions(-)
diff --git a/gdb/testsuite/lib/my-syscalls.S b/gdb/testsuite/lib/my-syscalls.S
index d727098a6f1..a08740ee01b 100644
--- a/gdb/testsuite/lib/my-syscalls.S
+++ b/gdb/testsuite/lib/my-syscalls.S
@@ -21,36 +21,44 @@
#include <asm/unistd.h>
-/* int my_execve (const char *file, char *argv[], char *envp[]); */
-
-.global my_execve
-my_execve:
-
#if defined(__x86_64__)
- mov $__NR_execve, %rax
- /* rdi, rsi and rdx already contain the right arguments. */
-my_execve_syscall:
- syscall
- ret
+#define SYSCALL(NAME, NR) \
+.global NAME ;\
+NAME: ;\
+ mov $NR, %rax ;\
+ /* rdi, rsi and rdx already contain the right arguments. */ \
+NAME ## _syscall: ;\
+ syscall ;\
+ ret ;
#elif defined(__i386__)
- mov $__NR_execve, %eax
- mov 4(%esp), %ebx
- mov 8(%esp), %ecx
- mov 12(%esp), %edx
-my_execve_syscall:
- int $0x80
+#define SYSCALL(NAME, NR) \
+.global NAME ;\
+NAME: ;\
+ mov $NR, %eax ;\
+ mov 4(%esp), %ebx ;\
+ mov 8(%esp), %ecx ;\
+ mov 12(%esp), %edx ;\
+NAME ## _syscall: ;\
+ int $0x80 ;\
ret
#elif defined(__aarch64__)
- mov x8, #__NR_execve
- /* x0, x1 and x2 already contain the right arguments. */
-my_execve_syscall:
+#define SYSCALL(NAME, NR) \
+.global NAME ;\
+NAME: ;\
+ mov x8, NR ;\
+ /* x0, x1 and x2 already contain the right arguments. */ \
+NAME ## _syscall: ;\
svc #0
#else
# error "Unsupported architecture"
#endif
+
+/* int my_execve (const char *file, char *argv[], char *envp[]); */
+
+SYSCALL (my_execve, __NR_execve)
--
2.26.2
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 10/10] Testcases for stepping over thread exit syscall (PR gdb/27338)
2021-07-02 13:01 [PATCH 00/10] Step over thread exit (PR gdb/27338) Pedro Alves
` (8 preceding siblings ...)
2021-07-02 13:01 ` [PATCH 09/10] lib/my-syscalls.S: Refactor new SYSCALL macro Pedro Alves
@ 2021-07-02 13:01 ` Pedro Alves
2021-07-05 8:42 ` [PATCH 00/10] Step over thread exit " Andrew Burgess
10 siblings, 0 replies; 23+ messages in thread
From: Pedro Alves @ 2021-07-02 13:01 UTC (permalink / raw)
To: gdb-patches
From: Simon Marchi <simon.marchi@efficios.com>
gdb/testsuite/ChangeLog:
yyyy-mm-dd Simon Marchi <simon.marchi@efficios.com>
Pedro Alves <pedro@palves.net>
PR gdb/27338
* gdb.threads/step-over-thread-exit.c: New.
* gdb.threads/step-over-thread-exit.exp: New.
* gdb.threads/step-over-thread-exit-while-stop-all-threads.c: New.
* gdb.threads/step-over-thread-exit-while-stop-all-threads.exp:
New.
* lib/my-syscalls.S (my_exit): New syscall.
* lib/my-syscalls.h (my_exit): Declare.
Change-Id: Ie8b2c5747db99b7023463a897a8390d9e814a9c9
Co-authored-by: Pedro Alves <pedro@palves.net>
---
...-over-thread-exit-while-stop-all-threads.c | 77 +++++++++++++++++++
...ver-thread-exit-while-stop-all-threads.exp | 69 +++++++++++++++++
.../gdb.threads/step-over-thread-exit.c | 52 +++++++++++++
.../gdb.threads/step-over-thread-exit.exp | 70 +++++++++++++++++
gdb/testsuite/lib/my-syscalls.S | 4 +
gdb/testsuite/lib/my-syscalls.h | 5 ++
6 files changed, 277 insertions(+)
create mode 100644 gdb/testsuite/gdb.threads/step-over-thread-exit-while-stop-all-threads.c
create mode 100644 gdb/testsuite/gdb.threads/step-over-thread-exit-while-stop-all-threads.exp
create mode 100644 gdb/testsuite/gdb.threads/step-over-thread-exit.c
create mode 100644 gdb/testsuite/gdb.threads/step-over-thread-exit.exp
diff --git a/gdb/testsuite/gdb.threads/step-over-thread-exit-while-stop-all-threads.c b/gdb/testsuite/gdb.threads/step-over-thread-exit-while-stop-all-threads.c
new file mode 100644
index 00000000000..615fc146a8d
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/step-over-thread-exit-while-stop-all-threads.c
@@ -0,0 +1,77 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2021 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include <unistd.h>
+#include <stdlib.h>
+#include <pthread.h>
+#include "../lib/my-syscalls.h"
+
+#define NUM_THREADS 32
+
+static void *
+stepper_over_exit_thread (void *v)
+{
+ my_exit (0);
+
+ /* my_exit above should exit the thread, we don't expect to reach
+ here. */
+ abort ();
+}
+
+static void *
+spawner_thread (void *v)
+{
+ for (;;)
+ {
+ pthread_t threads[NUM_THREADS];
+ int i;
+
+ for (i = 0; i < NUM_THREADS; i++)
+ pthread_create (&threads[i], NULL, stepper_over_exit_thread, NULL);
+
+ for (i = 0; i < NUM_THREADS; i++)
+ pthread_join (threads[i], NULL);
+ }
+}
+
+static void
+break_here (void)
+{
+}
+
+static void *
+breakpoint_hitter_thread (void *v)
+{
+ for (;;)
+ break_here ();
+}
+
+int
+main ()
+{
+ pthread_t breakpoint_hitter;
+ pthread_t spawner;
+
+ alarm (60);
+
+ pthread_create (&spawner, NULL, spawner_thread, NULL);
+ pthread_create (&breakpoint_hitter, NULL, breakpoint_hitter_thread, NULL);
+
+ pthread_join (spawner, NULL);
+
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.threads/step-over-thread-exit-while-stop-all-threads.exp b/gdb/testsuite/gdb.threads/step-over-thread-exit-while-stop-all-threads.exp
new file mode 100644
index 00000000000..588a8b8b175
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/step-over-thread-exit-while-stop-all-threads.exp
@@ -0,0 +1,69 @@
+# Copyright 2021 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# Test stepping over a breakpoint installed on an instruction that
+# exits the thread, while another thread is repeatedly hitting a
+# breakpoint, causing GDB to stop all threads.
+
+standard_testfile .c
+
+set syscalls_src $srcdir/lib/my-syscalls.S
+
+if { [build_executable "failed to prepare" $testfile \
+ [list $srcfile $syscalls_src] {debug pthreads}] == -1 } {
+ return
+}
+
+proc test {displaced-stepping target-non-stop} {
+ save_vars ::GDBFLAGS {
+ append ::GDBFLAGS " -ex \"maintenance set target-non-stop on\""
+ clean_restart $::binfile
+ }
+
+ gdb_test_no_output "set displaced-stepping ${displaced-stepping}"
+
+ if { ![runto_main] } {
+ return
+ }
+
+ # The "stepper over exit" threads will step over an instruction
+ # that causes them to exit.
+ gdb_test "break my_exit_syscall if 0"
+
+ # The "breakpoint hitter" thread will repeatedly hit this
+ # breakpoint, causing GDB to stop all threads.
+ gdb_test "break break_here"
+
+ # To avoid flooding the log with thread created/exited messages.
+ gdb_test_no_output "set print thread-events off"
+
+ # Make sure the target reports the breakpoint stops.
+ gdb_test_no_output "set breakpoint condition-evaluation host"
+
+ for { set i 0 } { $i < 30 } { incr i } {
+ with_test_prefix "iter $i" {
+ if { [gdb_test "continue" "hit Breakpoint $::decimal, break_here .*"] != 0 } {
+ # Exit if there's a failure to avoid lengthy timeouts.
+ break
+ }
+ }
+ }
+}
+
+foreach_with_prefix displaced-stepping {off auto} {
+ foreach_with_prefix target-non-stop {off on} {
+ test ${displaced-stepping} ${target-non-stop}
+ }
+}
diff --git a/gdb/testsuite/gdb.threads/step-over-thread-exit.c b/gdb/testsuite/gdb.threads/step-over-thread-exit.c
new file mode 100644
index 00000000000..c5f0e331c71
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/step-over-thread-exit.c
@@ -0,0 +1,52 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2021 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include <pthread.h>
+#include <assert.h>
+#include <stdlib.h>
+#include "../lib/my-syscalls.h"
+
+static void *
+thread_func (void *arg)
+{
+ my_exit (0);
+
+ /* my_exit above should exit the thread, we don't expect to reach
+ here. */
+ abort ();
+}
+
+int
+main (void)
+{
+ int i;
+
+ /* Spawn and join a thread, 100 times. */
+ for (i = 0; i < 100; i++)
+ {
+ pthread_t thread;
+ int ret;
+
+ ret = pthread_create (&thread, NULL, thread_func, NULL);
+ assert (ret == 0);
+
+ ret = pthread_join (thread, NULL);
+ assert (ret == 0);
+ }
+
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.threads/step-over-thread-exit.exp b/gdb/testsuite/gdb.threads/step-over-thread-exit.exp
new file mode 100644
index 00000000000..50d0b9c7cbb
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/step-over-thread-exit.exp
@@ -0,0 +1,70 @@
+# Copyright 2021 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# Test stepping over a breakpoint installed on an instruction that
+# exits the thread.
+
+standard_testfile .c
+
+set syscalls_src $srcdir/lib/my-syscalls.S
+
+if { [build_executable "failed to prepare" $testfile \
+ [list $srcfile $syscalls_src] {debug pthreads}] == -1 } {
+ return
+}
+
+proc test {displaced-stepping target-non-stop schedlock} {
+ save_vars ::GDBFLAGS {
+ append ::GDBFLAGS " -ex \"maintenance set target-non-stop ${target-non-stop}\""
+ clean_restart $::binfile
+ }
+
+ gdb_test_no_output "set displaced-stepping ${displaced-stepping}"
+
+ if { ![runto_main] } {
+ return
+ }
+
+ gdb_breakpoint "my_exit_syscall"
+
+ gdb_test_no_output "set scheduler-locking ${schedlock}"
+
+ if {$schedlock} {
+ gdb_test "continue" \
+ "hit Breakpoint $::decimal.* my_exit_syscall .*" \
+ "continue until syscall"
+
+ gdb_test "continue" \
+ "No unwaited-for children left." \
+ "continue stops when thread exits"
+ } else {
+ for { set i 0 } { $i < 100 } { incr i } {
+ with_test_prefix "iter $i" {
+ if { [gdb_test "continue" "hit Breakpoint $::decimal.* my_exit_syscall .*"] != 0 } {
+ # Exit if there's a failure to avoid lengthy timeouts.
+ break
+ }
+ }
+ }
+ }
+}
+
+foreach_with_prefix displaced-stepping {off auto} {
+ foreach_with_prefix target-non-stop {off on} {
+ foreach_with_prefix schedlock {off on} {
+ test ${displaced-stepping} ${target-non-stop} ${schedlock}
+ }
+ }
+}
diff --git a/gdb/testsuite/lib/my-syscalls.S b/gdb/testsuite/lib/my-syscalls.S
index a08740ee01b..f03df838261 100644
--- a/gdb/testsuite/lib/my-syscalls.S
+++ b/gdb/testsuite/lib/my-syscalls.S
@@ -62,3 +62,7 @@ NAME ## _syscall: ;\
/* int my_execve (const char *file, char *argv[], char *envp[]); */
SYSCALL (my_execve, __NR_execve)
+
+/* void my_exit (int code); */
+
+SYSCALL (my_exit, __NR_exit)
diff --git a/gdb/testsuite/lib/my-syscalls.h b/gdb/testsuite/lib/my-syscalls.h
index 18823052a28..0eecbcb820c 100644
--- a/gdb/testsuite/lib/my-syscalls.h
+++ b/gdb/testsuite/lib/my-syscalls.h
@@ -22,4 +22,9 @@
int my_execve (const char *file, char *argv[], char *envp[]);
+/* `exit` syscall, which makes the thread exit (as opposed to
+ `exit_group`, which makes the process exit). */
+
+void my_exit (int code);
+
#endif /* MY_SYSCALLS_H */
--
2.26.2
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 03/10] remote+gdbserver: stepped-thread-exited feature
2021-07-02 13:01 ` [PATCH 03/10] remote+gdbserver: stepped-thread-exited feature Pedro Alves
@ 2021-07-02 17:13 ` Eli Zaretskii
2021-07-05 19:27 ` Simon Marchi
1 sibling, 0 replies; 23+ messages in thread
From: Eli Zaretskii @ 2021-07-02 17:13 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches, simon.marchi
> From: Pedro Alves <pedro@palves.net>
> Date: Fri, 2 Jul 2021 14:01:47 +0100
> gdb/ChangeLog:
> yyyy-mm-dd Pedro Alves <pedro@palves.net>
> Simon Marchi <simon.marchi@efficios.com>
>
> PR gdb/27338
> * NEWS: Mention stepped-thread-exited feature in qSupported.
> * remote.c (PACKET_stepped_thread_exited): New.
> (remote_protocol_features): Add "stepped-thread-exited" entry.
> (remote_target::remote_query_supported): Report
> stepped-thread-exited+.
> (remote_target::wait_as): Handle 'w' (aka thread exit) stop reply.
> (_initialize_remote): Register "set/show remote
> stepped-thread-exited-stop-reply".
>
> gdbserver/ChangeLog:
> yyyy-mm-dd Pedro Alves <pedro@palves.net>
>
> PR gdb/27338
> * server.cc (handle_query): Handle and report
> stepped-thread-exited+.
> (resume): Handle TARGET_WAITKIND_THREAD_EXITED in all-stop mode.
>
> gdb/doc/ChangeLog:
> yyyy-mm-dd Pedro Alves <pedro@palves.net>
>
> PR gdb/27338
> * gdb.texinfo (Remote Configuration): Document
> stepped-thread-exited-stop-reply.
> (Stop Reply Packets) <w stop reply>: Mention
> stepped-threads-exited feature.
> (General Query Packets): Document stepped-thread-exited.
>
Thanks.
In the documentation parts, I'm confused by the terms "thread exited
event" and "stepping threads". They are not explained anywhere,
AFAICS. Could we explain them, or perhaps use some more clear
terminology? E.g., at one place you use "single-stepped thread that
exit", which is clear (assuming that's what you mean by "stepping
threads").
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 00/10] Step over thread exit (PR gdb/27338)
2021-07-02 13:01 [PATCH 00/10] Step over thread exit (PR gdb/27338) Pedro Alves
` (9 preceding siblings ...)
2021-07-02 13:01 ` [PATCH 10/10] Testcases for stepping over thread exit syscall (PR gdb/27338) Pedro Alves
@ 2021-07-05 8:42 ` Andrew Burgess
2021-07-05 11:43 ` Pedro Alves
10 siblings, 1 reply; 23+ messages in thread
From: Andrew Burgess @ 2021-07-05 8:42 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
I'm a little short on time right now, so haven't looked through this
series in depth yet. I don't think there's much overlap, but this is
clearly related:
https://sourceware.org/pipermail/gdb-patches/2021-June/180517.html
I just wanted to make sure you'd seen this.
Thanks,
Andrew
* Pedro Alves <pedro@palves.net> [2021-07-02 14:01:44 +0100]:
> GDB doesn't handle correctly the case where a thread steps over a
> breakpoint (using either in-line or displaced stepping), and the
> executed instruction causes the thread to exit. This series fixes it.
>
> Patch #2 is the core of the fix.
>
> Patch #3 includes new remote protocol bits, and corresponding
> documentation changes. There are no documentation changes anywhere
> else in the series.
>
> Pedro Alves (7):
> displaced step: pass down target_waitstatus instead of gdb_signal
> remote+gdbserver: stepped-thread-exited feature
> struct resumed_pending_vcont_info -> struct resume_info
> remote_target::update_thread_list: Don't delete stepping threads
> stop_all_threads: (re-)enable async before waiting for stops
> Tweak gdbserver's no-resumed handling
> lib/my-syscalls.S: Refactor new SYSCALL macro
>
> Simon Marchi (3):
> gdb: clear step over information on thread exit (PR gdb/27338)
> Implement stepped-thread-exited feature on native+gdbserver Linux
> Testcases for stepping over thread exit syscall (PR gdb/27338)
>
> gdb/NEWS | 5 +
> gdb/displaced-stepping.c | 18 +-
> gdb/displaced-stepping.h | 2 +-
> gdb/doc/gdb.texinfo | 28 ++-
> gdb/gdbarch.c | 4 +-
> gdb/gdbarch.h | 10 +-
> gdb/gdbarch.sh | 6 +-
> gdb/infrun.c | 220 ++++++++++++++++--
> gdb/linux-nat.c | 41 ++--
> gdb/linux-tdep.c | 5 +-
> gdb/linux-tdep.h | 2 +-
> gdb/remote.c | 146 +++++++-----
> gdb/target/target.h | 10 +-
> ...-over-thread-exit-while-stop-all-threads.c | 77 ++++++
> ...ver-thread-exit-while-stop-all-threads.exp | 69 ++++++
> .../gdb.threads/step-over-thread-exit.c | 52 +++++
> .../gdb.threads/step-over-thread-exit.exp | 70 ++++++
> gdb/testsuite/lib/my-syscalls.S | 50 ++--
> gdb/testsuite/lib/my-syscalls.h | 5 +
> gdbserver/linux-low.cc | 44 ++--
> gdbserver/linux-low.h | 2 +
> gdbserver/server.cc | 28 ++-
> gdbserver/target.cc | 6 +
> gdbserver/target.h | 7 +
> 24 files changed, 764 insertions(+), 143 deletions(-)
> create mode 100644 gdb/testsuite/gdb.threads/step-over-thread-exit-while-stop-all-threads.c
> create mode 100644 gdb/testsuite/gdb.threads/step-over-thread-exit-while-stop-all-threads.exp
> create mode 100644 gdb/testsuite/gdb.threads/step-over-thread-exit.c
> create mode 100644 gdb/testsuite/gdb.threads/step-over-thread-exit.exp
>
>
> base-commit: 62194b631d00112bac1f8856d3259d774df4c15e
> --
> 2.26.2
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 00/10] Step over thread exit (PR gdb/27338)
2021-07-05 8:42 ` [PATCH 00/10] Step over thread exit " Andrew Burgess
@ 2021-07-05 11:43 ` Pedro Alves
2021-07-13 15:08 ` Pedro Alves
0 siblings, 1 reply; 23+ messages in thread
From: Pedro Alves @ 2021-07-05 11:43 UTC (permalink / raw)
To: Andrew Burgess; +Cc: gdb-patches
On 2021-07-05 9:42 a.m., Andrew Burgess wrote:
> I'm a little short on time right now, so haven't looked through this
> series in depth yet. I don't think there's much overlap, but this is
> clearly related:
>
> https://sourceware.org/pipermail/gdb-patches/2021-June/180517.html
>
> I just wanted to make sure you'd seen this.
Thanks. Hmm, I think there's actually a lot of overlap. This series
makes the server report thread events when stepping exactly to avoid
issuing QThreadEvents, while your series issues the QThreadEvents anyhow.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 02/10] gdb: clear step over information on thread exit (PR gdb/27338)
2021-07-02 13:01 ` [PATCH 02/10] gdb: clear step over information on thread exit (PR gdb/27338) Pedro Alves
@ 2021-07-05 18:34 ` Simon Marchi
0 siblings, 0 replies; 23+ messages in thread
From: Simon Marchi @ 2021-07-05 18:34 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
On 2021-07-02 9:01 a.m., Pedro Alves wrote:
> From: Simon Marchi <simon.marchi@efficios.com>
>
> GDB doesn't handle correctly the case where a thread steps over a
> breakpoint (using either in-line or displaced stepping), and the
> executed instruction causes the thread to exit.
>
> Using the test program included later in the series, this is what it
> looks like with displaced-stepping, on x86-64 Linux, where we have two
> displaced-step buffers:
>
> $ ./gdb -q -nx --data-directory=data-directory build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/step-over-thread-exit/step-over-thread-exit -ex "b my_exit_syscall" -ex r
> Reading symbols from build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/step-over-thread-exit/step-over-thread-exit...
> Breakpoint 1 at 0x123c: file src/binutils-gdb/gdb/testsuite/lib/my-syscalls.S, line 68.
> Starting program: build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/step-over-thread-exit/step-over-thread-exit
> [Thread debugging using libthread_db enabled]
> Using host libthread_db library "/usr/lib/../lib/libthread_db.so.1".
> [New Thread 0x7ffff7c5f640 (LWP 2915510)]
> [Switching to Thread 0x7ffff7c5f640 (LWP 2915510)]
>
> Thread 2 "step-over-threa" hit Breakpoint 1, my_exit_syscall () at src/binutils-gdb/gdb/testsuite/lib/my-syscalls.S:68
> 68 syscall
> (gdb) c
> Continuing.
> [New Thread 0x7ffff7c5f640 (LWP 2915524)]
> [Thread 0x7ffff7c5f640 (LWP 2915510) exited]
> [Switching to Thread 0x7ffff7c5f640 (LWP 2915524)]
>
> Thread 3 "step-over-threa" hit Breakpoint 1, my_exit_syscall () at src/binutils-gdb/gdb/testsuite/lib/my-syscalls.S:68
> 68 syscall
> (gdb) c
> Continuing.
> [New Thread 0x7ffff7c5f640 (LWP 2915616)]
> [Thread 0x7ffff7c5f640 (LWP 2915524) exited]
> [Switching to Thread 0x7ffff7c5f640 (LWP 2915616)]
>
> Thread 4 "step-over-threa" hit Breakpoint 1, my_exit_syscall () at src/binutils-gdb/gdb/testsuite/lib/my-syscalls.S:68
> 68 syscall
> (gdb) c
> Continuing.
> ... hangs ...
>
> The first two times we do "continue", we displaced-step the syscall
> instruction that causes the thread to exit. When the thread exits,
> the main thread, waiting on pthread_join, is unblocked. It spawns a
> new thread, which hits the breakpoint on the syscall again. However,
> infrun was never notified that the displaced-stepping threads are done
> using the displaced-step buffer, so they are now both marked as used.
> So when we do the third continue, there are no buffers available to
> displaced-step the syscall, so it waits forever.
>
> When trying the same but with in-line step over (displaced-stepping
> disabled):
>
> $ ./gdb -q -nx --data-directory=data-directory \
> build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/step-over-thread-exit/step-over-thread-exit \
> -ex "b my_exit_syscall" -ex "set displaced-stepping off" -ex r
> Reading symbols from build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/step-over-thread-exit/step-over-thread-exit...
> Breakpoint 1 at 0x123c: file src/binutils-gdb/gdb/testsuite/lib/my-syscalls.S, line 68.
> Starting program: build/binutils-gdb/gdb/testsuite/outputs/gdb.threads/step-over-thread-exit/step-over-thread-exit
> [Thread debugging using libthread_db enabled]
> Using host libthread_db library "/usr/lib/../lib/libthread_db.so.1".
> [New Thread 0x7ffff7c5f640 (LWP 2928290)]
> [Switching to Thread 0x7ffff7c5f640 (LWP 2928290)]
>
> Thread 2 "step-over-threa" hit Breakpoint 1, my_exit_syscall () at src/binutils-gdb/gdb/testsuite/lib/my-syscalls.S:68
> 68 syscall
> (gdb) c
> Continuing.
> [Thread 0x7ffff7c5f640 (LWP 2928290) exited]
> No unwaited-for children left.
> (gdb) i th
> Id Target Id Frame
> 1 Thread 0x7ffff7c60740 (LWP 2928285) "step-over-threa" 0x00007ffff7f7c9b7 in __pthread_clockjoin_ex () from /usr/lib/libpthread.so.0
>
> The current thread <Thread ID 2> has terminated. See `help thread'.
> (gdb) thread 1
> [Switching to thread 1 (Thread 0x7ffff7c60740 (LWP 2928285))]
> #0 0x00007ffff7f7c9b7 in __pthread_clockjoin_ex () from /usr/lib/libpthread.so.0
> (gdb) c
> Continuing.
> ^C^C
> ... hangs ...
>
> The "continue" causes an in-line step to occur, meaning the main
> thread is stopped while we step the syscall. The thread exits when
> executing the syscall, the linux-nat target notices there are no more
> resumed threads to be waited for, so returns
> TARGET_WAITKIND_NO_RESUMED, which causes the prompt to return. But
> infrun never clears the in-line step over info. So if we try
> continuing the main thread, GDB doesn't resume it, because it thinks
> there's an in-line step in progress that we need to wait for to
> finish, and we are stuck there.
>
> To fix this, infrun needs to be informed when a thread doing a
> displaced or in-line step over exits.
>
> Thread events (thread created and thread exited) are not normally
> reported by targets. For targets to report them, infrun requests them
> to be reported by using target_ops::thread_events. This is currently
> only used in stop_all_threads, when infrun tries to stop all threads
> when doing an all-stop-on-top-of-non-stop stop. When an in-line or
> displaced step is executed, they are therefore not enabled. The
> linux-nat target gets the event about the thread exit and simply
> deletes the thread without notifying the core. Infrun therefore never
> has a chance to clean up the in-line or displaced step resources the
> thread was using.
>
> One possibility would be for infrun to enable reporting of thread
> events when an in-line or displaced step is in progress. But doing
> so, we could get thread events for threads we don't care about that
> happen during the step (probably not a big deal, but a bit wasteful).
> It would also add logic that is not trivial to enable and disable
> thread events correctly.
>
> Instead, this commit adds a line to the contract between infrun and
> the targets: if the exiting thread was last stepped (i.e. asked to
> step last time it was resumed), then the target must report the exit
> event to infrun. This adds a bit of logic on the target side, but
> makes it much easier for infrun.
>
> This patch modifies handle_inferior_event in infrun.c to clean up any
> step-over the exiting thread might have been doing at the time of the
> exit. The cases to consider are:
>
> - the exiting thread was doing an in-line step-over with an all-stop
> target
> - the exiting thread was doing an in-line step-over with a non-stop
> target
> - the exiting thread was doing a displaced step-over with a non-stop
> target
>
> The code handling TARGET_WAITKIND_THREAD_EXITED in
> handle_inferior_event should also be prepared to handle such an event
> for a thread that was not doing a step-over:
>
> - maybe the thread was asked to step, stepped an instruction that
> caused it to exit, but was not stepping over a breakpoint.
>
> - maybe the target just freely reports all thread exit events
>
> The displaced-stepping buffer implementation in displaced-stepping.c
> is modified to account for the fact that it's possible that we
> "finish" a displaced step for an exited thread. The buffer that the
> now exited thread was using is marked as available again and the
> original instructions under the scratch pad are restored. However, it
> skips applying the fixup, which wouldn't make sense since the thread
> does not exist anymore.
>
> Another case handling is if a displaced-step thread exits, and the
> event is reported while we are in stop_all_threads. We should call
> displaced_step_finish in the handle_one function, in that case. It
> was already called in other code paths, just not the "thread exited"
> path.
>
> This commit doesn't make any target report the
> TARGET_WAITKIND_THREAD_EXITED for stepping threads yet, that'll be
> done later in the series.
>
> gdb/ChangeLog:
> yyyy-mm-dd Simon Marchi <simon.marchi@efficios.com>
> Pedro Alves <pedro@palves.net>
>
> PR gdb/27338
> * displaced-stepping.c (displaced_step_buffers::finish): Return
> early if thread exited.
> * gdbarch.sh (displaced_step_finish): Modify comment.
> * gdbarch.c: Re-generate.
> * gdbarch.h: Re-generate.
> * infrun.c (handle_one): Finish displaceds step if
> TARGET_WAITKIND_THREAD_EXITED.
> (handle_no_resumed): Document return value. Don't clear
> stop_print_frame here.
> (handle_thread_exited): New.
> (handle_inferior_event): Remove early handling of
> TARGET_WAITKIND_THREAD_EXITED, and handle
> TARGET_WAITKIND_THREAD_EXITED where other event kinds are
> handled. Use handle_thread_exited.
> (finish_step_over): Return early if the thread exited.
> (normal_stop) <TARGET_WAITKIND_NO_RESUMED>: Clear stop_print_frame
> here.
> * target/target.h (target_wait): Document thread exit event
> expectations.
>
> Co-authored-by: Pedro Alves <pedro@palves.net>
> Change-Id: I745c6955d7ef90beb83bcf0ff1d1ac8b9b6285a5
We have not settled on a format yet (see [1]), but it would be good to
add a reference to the bug by URL, like
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=27338
Otherwise, that LGTM.
Simon
[1] https://sourceware.org/pipermail/gdb-patches/2021-June/180380.html
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 03/10] remote+gdbserver: stepped-thread-exited feature
2021-07-02 13:01 ` [PATCH 03/10] remote+gdbserver: stepped-thread-exited feature Pedro Alves
2021-07-02 17:13 ` Eli Zaretskii
@ 2021-07-05 19:27 ` Simon Marchi
1 sibling, 0 replies; 23+ messages in thread
From: Simon Marchi @ 2021-07-05 19:27 UTC (permalink / raw)
To: Pedro Alves, gdb-patches; +Cc: Simon Marchi
> diff --git a/gdbserver/target.h b/gdbserver/target.h
> index 2c4393ec8c6..862300f72f5 100644
> --- a/gdbserver/target.h
> +++ b/gdbserver/target.h
> @@ -500,6 +500,10 @@ class process_stratum_target
> /* Returns true if the target supports memory tagging facilities. */
> virtual bool supports_memory_tagging ();
>
> + /* Returns true if the target supports returning
> + TARGET_WAITKIND_THREAD_EXITED for stepping threads. */
> + virtual bool supports_stepped_thread_exited();
Space before parens.
Otherwise, LGTM.
Simon
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 04/10] struct resumed_pending_vcont_info -> struct resume_info
2021-07-02 13:01 ` [PATCH 04/10] struct resumed_pending_vcont_info -> struct resume_info Pedro Alves
@ 2021-07-05 19:46 ` Simon Marchi
0 siblings, 0 replies; 23+ messages in thread
From: Simon Marchi @ 2021-07-05 19:46 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
> @@ -6467,7 +6474,8 @@ remote_target::resume (ptid_t ptid, int step, enum gdb_signal siggnal)
>
> /* Update resumed state tracked by the remote target. */
> for (thread_info *tp : all_non_exited_threads (this, ptid))
> - get_remote_thread_info (tp)->set_resumed ();
> + get_remote_thread_info (tp)->set_resumed (false, GDB_SIGNAL_0);
> + remote_thr->set_resumed (step, siggnal);
This is really a small detail, but I would find it a bit easier to
understand like:
for (thread_info *tp : all_non_exited_threads (this, ptid))
if (get_remote_thread_info (tp) == remote_thr)
get_remote_thread_info (tp)->set_resumed (step, siggnal);
else
get_remote_thread_info (tp)->set_resumed (false, GDB_SIGNAL_0);
Maybe this wasn't possible for some reason, but I imagine that we could
add an assertion in set_resumed:
gdb_assert (m_resume_state != resume_state::RESUMED);
to indicate that a RESUMED -> RESUMED state transition is not a valid
one (if we were to draw the state machine diagram, there would be no
arrow from RESUMED to RESUMED). So, I would find it more logical to
write the code with that in mind.
> @@ -6926,7 +6931,7 @@ remote_target::remote_stop_ns (ptid_t ptid)
> queue, we'll end up reporting a stop event to the core
> for that thread while it is running on the remote
> target... that would be bad. */
> - remote_thr->set_resumed ();
> + remote_thr->set_resumed (false, GDB_SIGNAL_0);
Could it happen that a thread going though this, in the
RESUMED_PENDING_VCONT state, would have step == true? And if so, should
be keep step == true when moving it to the RESUMED state? It might not
matter today, but it might one day.
Other than these minor comments, the patch LGTM.
Simon
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 05/10] remote_target::update_thread_list: Don't delete stepping threads
2021-07-02 13:01 ` [PATCH 05/10] remote_target::update_thread_list: Don't delete stepping threads Pedro Alves
@ 2021-07-05 19:55 ` Simon Marchi
0 siblings, 0 replies; 23+ messages in thread
From: Simon Marchi @ 2021-07-05 19:55 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
On 2021-07-02 9:01 a.m., Pedro Alves wrote:
> Without this patch, the
> gdb.threads/step-over-thread-exit-while-stop-all-threads.exp testcase
> added later in the series would randomly fail against gdbserver. It
> would hit this assertion failure:
>
> src/gdb/displaced-stepping.c:54: internal-error: displaced_step_prepare_status displaced_step_buffers::prepare(thread_info*, CORE_ADDR&): Assertion `buf.current_thread != thread' failed.
>
> That assertion was a bit of a head scratcher. Turns out that is was
> caused by a 'buf.current_thread' being a stale pointer to a thread
> that had already been deleted. I confirmed that this is what was
> happening with the following hack:
>
> --- c/gdb/displaced-stepping.c
> +++ w/gdb/displaced-stepping.c
> @@ -145,6 +145,7 @@ displaced_step_buffers::prepare (thread_info *thread, CORE_ADDR &displaced_pc)
>
> /* This marks the buffer as being in use. */
> buffer->current_thread = thread;
> + buffer->current_thread_id = thread->global_num;
>
> When the assertion triggers, I had for example:
>
> (top-gdb) p buf.current_thread_id
> $1 = 205
> (top-gdb) p thread->global_num
> $2 = 308
>
> So, same pointer, but different threads.
>
> Here's how it happens:
>
> While GDB is stopping threads with stop_all_threads, it updates the
> thread list, which reaches remote_target::update_thread_list. Any
> thread that is no longer found in the remote end is removed from GDB's
> thread list by that function. If the thread that is gone was one that
> was stepping over a breakpoint set on top of a thread exit syscall,
> then update_thread_list deletes the thread straight away, which leaves
> the displaced stepping buffer with a stale pointer to the thread.
>
> Later, when a new thread_info object is created (the testcase
> continuously spawns threads), the heap may well return the same memory
> address for the new object as the displaced stepping buffer's stale
> pointer still points to. That is what happened in the assertion
> triggered above -- in reality the buffer's 'buf.current_thread'
> conceptually pointed to a different thread not 'thread', but in
> practice, the pointers had the same value.
>
> What we really want to do is release the displaced stepping buffer, so
> some other thread can reuse it, and we want to let another thread have
> its turn at stepping over a breakpoint. The simplest way is to make
> update_thread_list not delete threads that are stepping, as we know
> that we will get a 'w' (TARGET_WAITKIND_THREAD_EXITED) stop reply for
> them shortly, so that infrun takes care of all that.
>
> gdb/ChangeLog:
> yyyy-mm-dd Pedro Alves <pedro@palves.net>
>
> PR gdb/27338
> * remote.c (remote_target::update_thread_list): Don't delete
> missing threads that were stepping.
>
> Change-Id: I4a8687b48da8bf1d9cc23981c427722dd63fecb6
> ---
> gdb/remote.c | 19 ++++++++++++++++++-
> 1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/gdb/remote.c b/gdb/remote.c
> index 179095a842f..71df343120b 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -3943,6 +3943,8 @@ remote_target::update_thread_list ()
>
> if (!context.contains_thread (tp->ptid))
> {
> + /* Not found. */
> +
> /* Do not remove the thread if it is the last thread in
> the inferior. This situation happens when we have a
> pending exit process status to process. Otherwise we
> @@ -3951,7 +3953,22 @@ remote_target::update_thread_list ()
> if (has_single_non_exited_thread (tp->inf))
> continue;
>
> - /* Not found. */
> + /* If the thread was stepping, then don't delete it yet,
> + let infrun process the thread exit event which should
> + come shortly. If this thread was stepping over a
> + breakpoint, we want infrun to see the exit so it can
> + free the displaced stepping buffer (if used), and
> + maybe start a new step-over in another thread (if any
> + is waiting). */
> + remote_thread_info *remote_thr = get_remote_thread_info (tp);
> + if (packet_support (PACKET_stepped_thread_exited) == PACKET_ENABLE
> + && remote_thr->get_resume_state () != resume_state::NOT_RESUMED
> + && remote_thr->resume_info ().step)
> + continue;
I think this change is in concordance with the new line in the
infrun/target contract, where targets should report exit events for
stepping threads. Simply deleting the thread here would cause such an
event not to be reported.
Therefore, I think the comment justification could say: we must do
this because this is part of the internal API contract.
Otherwise, LGTM.
Simon
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 06/10] stop_all_threads: (re-)enable async before waiting for stops
2021-07-02 13:01 ` [PATCH 06/10] stop_all_threads: (re-)enable async before waiting for stops Pedro Alves
@ 2021-07-05 21:32 ` Simon Marchi
0 siblings, 0 replies; 23+ messages in thread
From: Simon Marchi @ 2021-07-05 21:32 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
> The comment in the patch describes a different scenario that does not
> involve the phony stop:
>
> /* wait_one waits for events until it sees a
> TARGET_WAITKIND_NO_RESUMED. When it sees one, it disables
> target_async for the target and no longer waits for events from
> that target. TARGET_WAITKIND_NO_RESUMED can be delayed though,
> consider:
>
> #1 - threads 2-5 are stopped, thread 1 is running
>
> #2 - target reports breakpoint hit for thread 1, event
> is queued.
>
> #3 - target reports no-resumed left, event is queued
>
> #4 - user resumes all threads
>
> #5 - gdb decides to stop all threads, stops threads 1-5
>
> #6 - wait_one returns the queued breakpoint hit for
> thread 1
>
> #7 - wait_one returns the queued no-resumed event,
> wait_one stops waiting for events from target.
>
> #8 - we still haven't seen the stops for threads 2-5, so
> we do another pass.
>
> #9 - if the target is not async wait_one doesn't wait on
> the target, so it won't see the stops.
>
> #a - we'd loop forever with WAITS_NEEDED > 0.
I don't understand this. In step #2, what do you mean by "event is
queued"? I don't know of any event queueing system for events reported
by targets. Same for the event in #3. And I don't understand the rest
either, probably because I don't understand #2 and #3.
Simon
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 07/10] Tweak gdbserver's no-resumed handling
2021-07-02 13:01 ` [PATCH 07/10] Tweak gdbserver's no-resumed handling Pedro Alves
@ 2021-07-05 21:51 ` Simon Marchi
0 siblings, 0 replies; 23+ messages in thread
From: Simon Marchi @ 2021-07-05 21:51 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
On 2021-07-02 9:01 a.m., Pedro Alves wrote:
> If we make gdbserver report TARGET_WAITKIND_THREAD_EXITED for threads
> that were stepping when they exit, we regress the
> TARGET_WAITKIND_NO_RESUMED support.
>
> The issue is that gdbserver no longer sends TARGET_WAITKIND_NO_RESUMED
> if the last resumed thread exits, and, it was stepping. We report the
> TARGET_WAITKIND_THREAD_EXITED, and then the next time around, all
> threads are already not-resumed when we get to
> linux_process_target::wait_1's entry, so the 'any_resumed' variable is
> set to false.
>
> This is exposed by the new gdb.threads/step-over-thread-exit.exp
> testcase at the end of this series:
>
> Thread 2 "step-over-threa" hit Breakpoint 2, my_exit_syscall () at /home/pedro/rocm/gdb/build/gdb/../../src/gdb/testsuite/lib/my-syscalls.S:68
> 68 syscall
> (gdb) PASS: gdb.threads/step-over-thread-exit.exp: displaced-stepping=off: target-non-stop=on: schedlock=on: continue until syscall
> continue
> Continuing.
> FAIL: gdb.threads/step-over-thread-exit.exp: displaced-stepping=off: target-non-stop=on: schedlock=on: continue stops when thread exits (timeout)
>
> A fixed GDB shows instead:
>
> (gdb) PASS: gdb.threads/step-over-thread-exit.exp: displaced-stepping=off: target-non-stop=on: schedlock=on: continue until syscall
> continue
> Continuing.
> No unwaited-for children left.
> (gdb) PASS: gdb.threads/step-over-thread-exit.exp: displaced-stepping=off: target-non-stop=on: schedlock=on: continue stops when thread exits
A comment not related to this patch, but I always had the feeling that
this message, "No unwaited-for children left", isn't very user-friendly.
The concept of 'wait' is mostly an internal GDB detail, so as a user I
wouldn't really know what an "unwaited-for children" is. Perhaps that
message could use words the user is familiar with, lik "No resumed
threads left"?
> @@ -2971,23 +2970,11 @@ linux_process_target::wait_1 (ptid_t ptid, target_waitstatus *ourstatus,
> in_step_range = 0;
> ourstatus->kind = TARGET_WAITKIND_IGNORE;
>
> - auto status_pending_p_any = [&] (thread_info *thread)
> - {
> - return status_pending_p_callback (thread, minus_one_ptid);
> - };
> -
> - auto not_stopped = [&] (thread_info *thread)
> - {
> - return not_stopped_callback (thread, minus_one_ptid);
> - };
> -
> - /* Find a resumed LWP, if any. */
> - if (find_thread (status_pending_p_any) != NULL)
> - any_resumed = 1;
> - else if (find_thread (not_stopped) != NULL)
> - any_resumed = 1;
> - else
> - any_resumed = 0;
> + auto any_resumed = [&] (thread_info *thread)
> + {
> + return (status_pending_p_callback (thread, minus_one_ptid)
> + || not_stopped_callback (thread, minus_one_ptid));
> + };
Style proposal: can we use 'this->...' when calling a method from
another method (and I would add, access a field that is not prefixed
with _m)? I find it really helps understand that we call a method from
this object and not a free function, which can subsequently help
understand what the code is doing at a higher level. In this particular
case, I thought that the [&] was superflous and I was going to suggest
removing it (using [] instead) until I actually tried it.
Otherwise, any_resumed could be declared lower, in the scope where it is
used.
>
> if (step_over_bkpt == null_ptid)
> pid = wait_for_event (ptid, &w, options);
> @@ -2999,7 +2986,16 @@ linux_process_target::wait_1 (ptid_t ptid, target_waitstatus *ourstatus,
> pid = wait_for_event (step_over_bkpt, &w, options & ~WNOHANG);
> }
>
> - if (pid == 0 || (pid == -1 && !any_resumed))
> + if (pid == -1)
> + {
> + /* Check if we have any resumed LWP. If not, then report
> + TARGET_WAITKIND_NO_RESUMED. If we do, ignore the -1
> + return. */
> + if (find_thread (any_resumed) != nullptr)
> + pid = 0;
> + }
I am a bit confused about why we need to do this. If wait_for_event
returns -1, isn't it because there aren't any resumed lwps? If so, why
do we need to check again here? Is it related to if ptid !=
minus_one_ptid?
Simon
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 09/10] lib/my-syscalls.S: Refactor new SYSCALL macro
2021-07-02 13:01 ` [PATCH 09/10] lib/my-syscalls.S: Refactor new SYSCALL macro Pedro Alves
@ 2021-07-05 22:11 ` Simon Marchi
0 siblings, 0 replies; 23+ messages in thread
From: Simon Marchi @ 2021-07-05 22:11 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
On 2021-07-02 9:01 a.m., Pedro Alves wrote:
> Move the syscall assembly code behind a macro so that it's easy to add
> new syscalls without duplicating code.
>
> gdb/testsuite/ChangeLog:
>
> PR gdb/27338
> * lib/my-syscalls.S (SYSCALL): New macro, refactored from ...
> (my_execve, my_execve_syscall): ... these. Redefine using
> SYSCALL.
>
> Change-Id: I8acf1463b11a084d6b4579aaffb49b5d0dea3bba
> ---
> gdb/testsuite/lib/my-syscalls.S | 46 +++++++++++++++++++--------------
> 1 file changed, 27 insertions(+), 19 deletions(-)
>
> diff --git a/gdb/testsuite/lib/my-syscalls.S b/gdb/testsuite/lib/my-syscalls.S
> index d727098a6f1..a08740ee01b 100644
> --- a/gdb/testsuite/lib/my-syscalls.S
> +++ b/gdb/testsuite/lib/my-syscalls.S
> @@ -21,36 +21,44 @@
>
> #include <asm/unistd.h>
>
> -/* int my_execve (const char *file, char *argv[], char *envp[]); */
> -
> -.global my_execve
> -my_execve:
> -
> #if defined(__x86_64__)
>
> - mov $__NR_execve, %rax
> - /* rdi, rsi and rdx already contain the right arguments. */
> -my_execve_syscall:
> - syscall
> - ret
> +#define SYSCALL(NAME, NR) \
> +.global NAME ;\
> +NAME: ;\
> + mov $NR, %rax ;\
> + /* rdi, rsi and rdx already contain the right arguments. */ \
That comment made sense for the execve syscall because it takes three
parameters. It would also work for syscalls with 0, 1 and 2
parameters, but not with 4 parameters (we would need to move rcx to
r10). I'm wondering if we should have different macros for different
number of paramenters, SYSCALL0, SYSCALL1, and so on (only defining the
ones we actually need).
> +NAME ## _syscall: ;\
> + syscall ;\
> + ret ;
>
> #elif defined(__i386__)
>
> - mov $__NR_execve, %eax
> - mov 4(%esp), %ebx
> - mov 8(%esp), %ecx
> - mov 12(%esp), %edx
> -my_execve_syscall:
> - int $0x80
> +#define SYSCALL(NAME, NR) \
> +.global NAME ;\
> +NAME: ;\
> + mov $NR, %eax ;\
> + mov 4(%esp), %ebx ;\
> + mov 8(%esp), %ecx ;\
> + mov 12(%esp), %edx ;\
> +NAME ## _syscall: ;\
> + int $0x80 ;\
> ret
I'm wondering %ebx should be saved and restored, since it's a
callee-saved register. It was wrong previously, but it didn't really
matter, as it was only used for execve syscalls that never failed. But
if the macro is generic, then it could be used for syscalls like read,
and then not-saving ebx could be problematic.
Simon
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 01/10] displaced step: pass down target_waitstatus instead of gdb_signal
2021-07-02 13:01 ` [PATCH 01/10] displaced step: pass down target_waitstatus instead of gdb_signal Pedro Alves
@ 2021-07-05 22:19 ` Simon Marchi
0 siblings, 0 replies; 23+ messages in thread
From: Simon Marchi @ 2021-07-05 22:19 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
On 2021-07-02 9:01 a.m., Pedro Alves wrote:
> A following patch will want to make displaced_step_buffers::finish
> handle TARGET_WAITKIND_THREAD_EXITED.
There are some more (probably trivial) spots to update:
CXX rs6000-tdep.o
/home/simark/src/binutils-gdb/gdb/rs6000-tdep.c: In function ‘displaced_step_finish_status ppc_displaced_step_finish(gdbarch*, thread_info*, gdb_signal)’:
/home/simark/src/binutils-gdb/gdb/rs6000-tdep.c:1106:61: error: cannot convert ‘gdb_signal’ to ‘const target_waitstatus&’
1106 | return per_inferior->disp_step_buf->finish (arch, thread, sig);
| ^~~
| |
| gdb_signal
In file included from /home/simark/src/binutils-gdb/gdb/gdbthread.h:35,
from /home/simark/src/binutils-gdb/gdb/inferior.h:60,
from /home/simark/src/binutils-gdb/gdb/rs6000-tdep.c:22:
/home/simark/src/binutils-gdb/gdb/displaced-stepping.h:171:65: note: initializing argument 3 of ‘displaced_step_finish_status displaced_step_buffers::finish(gdbarch*, thread_info*, const target_waitstatus&)’
171 | const target_waitstatus &status);
| ~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~
Simon
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 00/10] Step over thread exit (PR gdb/27338)
2021-07-05 11:43 ` Pedro Alves
@ 2021-07-13 15:08 ` Pedro Alves
0 siblings, 0 replies; 23+ messages in thread
From: Pedro Alves @ 2021-07-13 15:08 UTC (permalink / raw)
To: Andrew Burgess; +Cc: gdb-patches
Hi Andrew,
On 2021-07-05 12:43 p.m., Pedro Alves wrote:
> On 2021-07-05 9:42 a.m., Andrew Burgess wrote:
>> I'm a little short on time right now, so haven't looked through this
>> series in depth yet. I don't think there's much overlap, but this is
>> clearly related:
>>
>> https://sourceware.org/pipermail/gdb-patches/2021-June/180517.html
>>
>> I just wanted to make sure you'd seen this.
>
> Thanks. Hmm, I think there's actually a lot of overlap. This series
> makes the server report thread events when stepping exactly to avoid
> issuing QThreadEvents, while your series issues the QThreadEvents anyhow.
>
I just wanted to mention that I'm very interested in getting those of these
series in, but haven't had a chance to think much more about them since.
I'm thinking that the best course of action may be to join the two series into
one, and make sure we have a design that addresses everything. I'll like to take a
stab at it, but probably won't be able to until the second week of August
the earliest.
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2021-07-13 15:08 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-02 13:01 [PATCH 00/10] Step over thread exit (PR gdb/27338) Pedro Alves
2021-07-02 13:01 ` [PATCH 01/10] displaced step: pass down target_waitstatus instead of gdb_signal Pedro Alves
2021-07-05 22:19 ` Simon Marchi
2021-07-02 13:01 ` [PATCH 02/10] gdb: clear step over information on thread exit (PR gdb/27338) Pedro Alves
2021-07-05 18:34 ` Simon Marchi
2021-07-02 13:01 ` [PATCH 03/10] remote+gdbserver: stepped-thread-exited feature Pedro Alves
2021-07-02 17:13 ` Eli Zaretskii
2021-07-05 19:27 ` Simon Marchi
2021-07-02 13:01 ` [PATCH 04/10] struct resumed_pending_vcont_info -> struct resume_info Pedro Alves
2021-07-05 19:46 ` Simon Marchi
2021-07-02 13:01 ` [PATCH 05/10] remote_target::update_thread_list: Don't delete stepping threads Pedro Alves
2021-07-05 19:55 ` Simon Marchi
2021-07-02 13:01 ` [PATCH 06/10] stop_all_threads: (re-)enable async before waiting for stops Pedro Alves
2021-07-05 21:32 ` Simon Marchi
2021-07-02 13:01 ` [PATCH 07/10] Tweak gdbserver's no-resumed handling Pedro Alves
2021-07-05 21:51 ` Simon Marchi
2021-07-02 13:01 ` [PATCH 08/10] Implement stepped-thread-exited feature on native+gdbserver Linux Pedro Alves
2021-07-02 13:01 ` [PATCH 09/10] lib/my-syscalls.S: Refactor new SYSCALL macro Pedro Alves
2021-07-05 22:11 ` Simon Marchi
2021-07-02 13:01 ` [PATCH 10/10] Testcases for stepping over thread exit syscall (PR gdb/27338) Pedro Alves
2021-07-05 8:42 ` [PATCH 00/10] Step over thread exit " Andrew Burgess
2021-07-05 11:43 ` Pedro Alves
2021-07-13 15:08 ` Pedro Alves
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).