* [PATCH 1/3] gdb/testsuite: update test gdb.base/step-over-syscall.exp
2021-06-30 19:55 [PATCH 0/3] Stepping over clone syscall Andrew Burgess
@ 2021-06-30 19:55 ` Andrew Burgess
2021-07-05 18:46 ` Simon Marchi
2021-06-30 19:55 ` [PATCH 2/3] gdb: support stepping over a clone syscall with displaced stepping off Andrew Burgess
2021-06-30 19:55 ` [PATCH 3/3] gdb: non-displaced step over clone, remote target, no QThreadEvents Andrew Burgess
2 siblings, 1 reply; 7+ messages in thread
From: Andrew Burgess @ 2021-06-30 19:55 UTC (permalink / raw)
To: gdb-patches
I was looking at bug gdb/19675 and the related test
gdb.base/step-over-syscall.exp. This test includes a call to kfail
when we are testing a displaced step over a clone syscall.
While looking at the test I removed the call to kfail and ran the
test, and was surprised that the test passed.
I ran the test a few times and it does sometimes fail, but mostly it
passed fine.
Bug gdb/19675 describes how, when we displaced step over a clone, the
new thread is created with a $pc in the displaced step buffer. GDB
then fails to "fix" this $pc (for the new thread), and the thread will
be set running with its current $pc value. This means that the new
thread will just start executing from whatever happens to be after the
displaced stepping buffer.
In the original gdb/19675 bug report Yao Qi was seeing the new thread
cause a segfault. But the problem is, what actually happens is
totally undefined.
On my machine, I'm seeing the new thread reenter main, it then starts
trying to run the test again (in the new thread). This just happens
to be safe enough (in this simple test) that most of the time the
inferior doesn't crash.
In this commit I try to make the test slightly more likely to fail by
doing a couple of things.
First, I added a static variable to main, this is set true when the
first thread enters main, if a second thread ever enters main then I
force an abort.
Second, when the test is finishing I want to ensure that the new
threads have had a chance to do "something bad" if they are going to.
So I added a global counter, as each thread starts successfully it
decrements the counter. The main thread does not proceed to the final
marker function (where GDB has placed a breakpoint) until all threads
have started successfully.
With these two changes my hope is that the test should fail more
reliably, and so, I have also changed the test to call setup_kfail
before the specific steps that we expect to misbehave instead of just
calling kfail and skipping parts of the test completely.
gdb/testsuite/ChangeLog:
PR gdb/19675
* gdb.base/step-over-clone.c (global_lock): New global variable.
(global_thread_count): Likewise.
(clone_fn): Decrement global_thread_count under a lock.
(main): Prevent thread reentering main, setup global_thread_count,
and wait for all threads to start before calling marker.
* gdb.base/step-over-syscall.exp (check_pc_after_cross_syscall):
Take additional parameter, figure out the current thread, and
check GDB is stopped in the correct thread.
(step_over_syscall): Link against pthreads library for clone
syscall test. Remove general call to kfail, pass extra variable
to check_pc_after_cross_syscall, call setup_kfail when
appropriate.
---
gdb/testsuite/ChangeLog | 16 +++++
gdb/testsuite/gdb.base/step-over-clone.c | 39 +++++++++++
gdb/testsuite/gdb.base/step-over-syscall.exp | 69 ++++++++++++++++----
3 files changed, 113 insertions(+), 11 deletions(-)
diff --git a/gdb/testsuite/gdb.base/step-over-clone.c b/gdb/testsuite/gdb.base/step-over-clone.c
index 581bf5fdde5..ef6fd922eb1 100644
--- a/gdb/testsuite/gdb.base/step-over-clone.c
+++ b/gdb/testsuite/gdb.base/step-over-clone.c
@@ -19,6 +19,7 @@
#include <stdlib.h>
#include <unistd.h>
#include <sched.h>
+#include <pthread.h>
static void
marker ()
@@ -26,9 +27,22 @@ marker ()
#define STACK_SIZE 0x1000
+/* These are used to signal that the threads have started correctly. The
+ GLOBAL_THREAD_COUNT is set to the number of threads in main, then
+ decremented (under a lock) in each new thread. */
+pthread_mutex_t global_lock = PTHREAD_MUTEX_INITIALIZER;
+int global_thread_count = 0;
+
static int
clone_fn (void *unused)
{
+ /* Signal that this thread has started correctly. */
+ if (pthread_mutex_lock (&global_lock) != 0)
+ abort ();
+ global_thread_count--;
+ if (pthread_mutex_unlock (&global_lock) != 0)
+ abort ();
+
return 0;
}
@@ -38,9 +52,21 @@ main (void)
int i, pid;
unsigned char *stack[6];
+ /* Due to bug gdb/19675 the cloned thread _might_ try to reenter main
+ (this depends on where the displaced instruction is placed for
+ execution). However, if we do reenter main then lets ensure we fail
+ hard rather then just silently executing the code below. */
+ static int started = 0;
+ if (!started)
+ started = 1;
+ else
+ abort ();
+
for (i = 0; i < (sizeof (stack) / sizeof (stack[0])); i++)
stack[i] = malloc (STACK_SIZE);
+ global_thread_count = (sizeof (stack) / sizeof (stack[0]));
+
for (i = 0; i < (sizeof (stack) / sizeof (stack[0])); i++)
{
pid = clone (clone_fn, stack[i] + STACK_SIZE, CLONE_FILES | CLONE_VM,
@@ -50,5 +76,18 @@ main (void)
for (i = 0; i < (sizeof (stack) / sizeof (stack[0])); i++)
free (stack[i]);
+ /* Set an alarm so we don't end up stuck waiting for threads that might
+ never start correctly. */
+ alarm (120);
+
+ /* Now wait for all the threads to start up. */
+ while (global_thread_count != 0)
+ {
+ /* Force memory barrier so GLOBAL_THREAD_COUNT will be refetched. */
+ asm volatile ("" ::: "memory");
+ sleep (1);
+ }
+
+ /* Call marker, this is what GDB is waiting for. */
marker ();
}
diff --git a/gdb/testsuite/gdb.base/step-over-syscall.exp b/gdb/testsuite/gdb.base/step-over-syscall.exp
index ecfb7be481d..5d4a75f810b 100644
--- a/gdb/testsuite/gdb.base/step-over-syscall.exp
+++ b/gdb/testsuite/gdb.base/step-over-syscall.exp
@@ -41,11 +41,50 @@ if { [istarget "i\[34567\]86-*-linux*"] || [istarget "x86_64-*-linux*"] } {
return -1
}
-proc_with_prefix check_pc_after_cross_syscall { syscall syscall_insn_next_addr } {
+proc_with_prefix check_pc_after_cross_syscall { displaced syscall syscall_insn_next_addr } {
+ global gdb_prompt
+
set syscall_insn_next_addr_found [get_hexadecimal_valueof "\$pc" "0"]
+ # After the 'stepi' we expect thread 1 to still be selected.
+ # However, when displaced stepping over a clone bug gdb/19675
+ # means this might not be the case.
+ #
+ # Which thread we end up in depends on a race between the original
+ # thread-1, and the new thread (created by the clone), so we can't
+ # guarantee which thread we will be in at this point.
+ #
+ # For the fork/vfork syscalls, which are correctly handled by
+ # displaced stepping we will always be in thread-1 or the original
+ # process at this point.
+ set curr_thread "unknown"
+ gdb_test_multiple "info threads" "" {
+ -re "Id\\s+Target Id\\s+Frame\\s*\r\n" {
+ exp_continue
+ }
+ -re "^\\* (\\d+)\\s+\[^\r\n\]+\r\n" {
+ set curr_thread $expect_out(1,string)
+ exp_continue
+ }
+ -re "^\\s+\\d+\\s+\[^\r\n\]+\r\n" {
+ exp_continue
+ }
+ -re "$gdb_prompt " {
+ }
+ }
+
+ # If we are displaced stepping over a clone, and we ended up in
+ # the wrong thread then the following check of the $pc value will
+ # fail.
+ if { $displaced == "on" && $syscall == "clone" && $curr_thread != 1 } {
+ # GDB doesn't support stepping over clone syscall with
+ # displaced stepping.
+ setup_kfail "*-*-*" "gdb/19675"
+ }
+
gdb_assert {$syscall_insn_next_addr != 0 \
- && $syscall_insn_next_addr == $syscall_insn_next_addr_found} \
+ && $syscall_insn_next_addr == $syscall_insn_next_addr_found \
+ && $curr_thread == 1} \
"single step over $syscall final pc"
}
@@ -203,7 +242,12 @@ proc step_over_syscall { syscall } {
set testfile "step-over-$syscall"
- if [build_executable ${testfile}.exp ${testfile} ${testfile}.c {debug}] {
+ set options [list debug]
+ if { $syscall == "clone" } {
+ lappend options "pthreads"
+ }
+
+ if [build_executable ${testfile}.exp ${testfile} ${testfile}.c $options] {
untested "failed to compile"
return -1
}
@@ -213,13 +257,6 @@ proc step_over_syscall { syscall } {
continue
}
- if { $displaced == "on" && $syscall == "clone" } {
- # GDB doesn't support stepping over clone syscall with
- # displaced stepping.
- kfail "gdb/19675" "single step over clone"
- continue
- }
-
set ret [setup $syscall]
set syscall_insn_addr [lindex $ret 0]
@@ -256,12 +293,22 @@ proc step_over_syscall { syscall } {
if {[gdb_test "stepi" "x/i .*=>.*" "single step over $syscall"] != 0} {
return -1
}
- check_pc_after_cross_syscall $syscall $syscall_insn_next_addr
+ check_pc_after_cross_syscall $displaced $syscall $syscall_insn_next_addr
# Delete breakpoint syscall insns to avoid interference to other syscalls.
delete_breakpoints
gdb_test "break marker" "Breakpoint.*at.* file .*${testfile}.c, line.*"
+
+ # If we are displaced stepping over a clone syscall then
+ # we expect the following check to fail. See also the
+ # code in check_pc_after_cross_syscall.
+ if { $displaced == "on" && $syscall == "clone" } {
+ # GDB doesn't support stepping over clone syscall with
+ # displaced stepping.
+ setup_kfail "*-*-*" "gdb/19675"
+ }
+
gdb_test "continue" "Continuing\\..*Breakpoint \[0-9\]+, marker \\(\\) at.*" \
"continue to marker ($syscall)"
}
--
2.25.4
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 2/3] gdb: support stepping over a clone syscall with displaced stepping off
2021-06-30 19:55 [PATCH 0/3] Stepping over clone syscall Andrew Burgess
2021-06-30 19:55 ` [PATCH 1/3] gdb/testsuite: update test gdb.base/step-over-syscall.exp Andrew Burgess
@ 2021-06-30 19:55 ` Andrew Burgess
2021-06-30 19:55 ` [PATCH 3/3] gdb: non-displaced step over clone, remote target, no QThreadEvents Andrew Burgess
2 siblings, 0 replies; 7+ messages in thread
From: Andrew Burgess @ 2021-06-30 19:55 UTC (permalink / raw)
To: gdb-patches
Not to be confused with bug gdb/19675, which is stepping over a clone
syscall when using displaced stepping, this commit adds support for
stepping over a clone syscall when using in-place stepping.
Currently, when a new thread is created through a clone syscall, GDB
will set the new thread running. With 'continue' this makes sense:
- all-stop mode, user issues 'continue', all threads are set running,
a newly created thread should also be set running.
- non-stop mode, user issues 'continue', other pre-existing threads
are not effected, but as the new thread is (sort-of) a child of the
thread the user asked to run, it makes sense (to me) that the new
threads should be created in the running state.
Similarly, if we are stopped at the clone syscall, and there's no
software breakpoint at this address, then the current behaviour is
fine I think:
- all-stop mode, user issues 'stepi', stepping will be done in
place (as there's no breakpoint to step over). While stepping the
thread of interest all the other threads will be allowed to
continue. A newly created thread will be set running, and then
stopped once the thread of interest has completed its step.
- non-stop mode, user issues 'stepi', stepping will be done in
place (as there's no breakpoint to step over). Other threads might
be running or stopped, but as with the continue case above, the new
thread will be created running. The only possible issue here is
that the new thread will be left running after the initial thread
has completed its stepi. The user would need to manually select
the thread and interrupt it, this might not be what the user
expects. However, this is not something I'm trying to change in
this commit.
The problem then is what happens when we try to step over a clone
syscall if there is a software breakpoint at the syscall address.
We can ignore the displaced stepping case, this is covered by bug
gdb/19675. For this commit we are only interested in the
non-displaced stepping case:
- For both all-stop and non-stop modes:
+ user issues 'stepi',
+ [non-stop mode only] GDB stops all threads. In all-stop mode all
threads are already stopped.
+ GDB removes s/w breakpoint at syscall address,
+ GDB single steps just the thread of interest, all other threads
are left stopped,
+ New thread is created running,
+ Initial thread completes its step,
+ [non-stop mode only] GDB resumes all threads that it previously
stopped.
There are two problems here:
1. The new thread might pass through the same code that the initial
thread is in (i.e. the clone syscall code), in which case it will
fail to hit the breakpoint in clone as this was removed so the
first thread can single step,
2. The new thread might trigger some other stop event before the
initial thread reports its step complete. If this happens we end
up triggering an assertion as GDB assumes that only the thread
being stepped should stop. The assert looks like this:
infrun.c:5899: internal-error: int finish_step_over(execution_control_state*): Assertion `ecs->event_thread->control.trap_expected' failed.
What is needed is for GDB to have more control over whether the new
thread is created running or not. Issue #1 above requires that the
new thread not be allowed to run until the s/w breakpoint has been
reinserted, the only way to guarantee this is if the new thread is
held in a stopped state until the single step has completed.
When looking for solutions to this problem I considered how GDB
handles fork/vfork as these seemed like they should have some of the
same issues. The main difference I see between fork/vfork and clone
is that the thread created events are not, by default, reported back
to core GDB, instead the thread created event is handled automatically
in the target code and the thread is immediately set running.
However, GDB does have a mechanism to request that it be told about
new threads, this is target_ops::thread_events. This on/off flag,
when on, indicates that the targets should notify GDB about new
threads (and also threads exiting). The new threads are still created
in a "running" state, that is, the infrun core of GDB sees the thread
as running, but the target will not actually start the thread
executing, this is perfect for what we need.
My proposal is that, before GDB steps a thread (when it is stepping
over a software breakpoint), we should first enable thread_events,
then, once the step has completed, disable thread_events.
What this means is that the new thread will be created in the infrun
running state, but not set executing. Once the step has completed
then:
- In all-stop mode
+ the stepping thread will stop,
+ all other threads (except the new one) are already stopped,
+ the new thread is not executing, but is marked running, however
GDB already calls stop_all_threads, which moves the thread into
the stopped state,
+ we're done.
- In non-stop mode
+ the stepping thread will stop,
+ all other threads (except the new one) are already stopped,
+ the new thread is not executing, but is marked running,
+ GDB calls restart_threads, the new thread will be set executing,
+ we're done.
This change worked fine for the native Linux target, but ran into
issues with the remote target.
The problem is that when enabling thread events for the remote target,
the event isn't actively reported to GDB, that is, if thread-events is
on, and a new thread is created, then gdbserver does hold the thread
in the stopped state, but doesn't report the stop to GDB.
This behaviour works fine for the current use of the thread-events
mechanism, which is stop_all_threads (infrun.c), in this case (for
remote targets) we're going to end up pulling the stop packets using
the vStopped mechanism anyway, so having gdbserver actively push the
stop to GDB is not important.
However, this causes problems for me, we enable thread-events and step
the target. The new thread is created but held in a stopped
state (with a pending thread-created event), when GDB resumes the
target the thread-created stop is immediately reported back to GDB.
My proposal in this commit is to have gdbserver discard pending
thread-created events if GDB is not interested in being told about
these events. This filtering is done earlier, at the point where we
pull pending stop events from the threads, prior to resuming any
threads. The consequence of this is that if GDB has said it is no
longer interested in thread creation events, any pending events are
discarded, and the thread is resumed.
I've tested this on GNU/Linux using the native target, and with the
native-gdbserver and native-extended-gdbserver targets.
gdb/ChangeLog:
PR gdb/27830
* infrun.c (displaced_step_finish): Disable thread events.
(do_target_resume): Enable thread events when stepping.
(finish_step_over): Disable thread events.
gdb/testsuite/ChangeLog:
PR gdb/27830
* gdb.threads/stepi-over-clone.c: New file.
* gdb.threads/stepi-over-clone.exp: New file.
gdbserver/ChangeLog:
PR gdb/27830
* linux-low.cc
(linux_process_target::thread_still_has_status_pending): Add
support for filtering out unwanted thread created events.
---
gdb/ChangeLog | 7 +
gdb/infrun.c | 20 ++
gdb/testsuite/ChangeLog | 6 +
gdb/testsuite/gdb.threads/stepi-over-clone.c | 90 +++++
.../gdb.threads/stepi-over-clone.exp | 328 ++++++++++++++++++
gdbserver/ChangeLog | 7 +
gdbserver/linux-low.cc | 27 +-
7 files changed, 478 insertions(+), 7 deletions(-)
create mode 100644 gdb/testsuite/gdb.threads/stepi-over-clone.c
create mode 100644 gdb/testsuite/gdb.threads/stepi-over-clone.exp
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 9469b74af39..98ce8c60b55 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1779,6 +1779,10 @@ displaced_step_finish (thread_info *event_thread, enum gdb_signal signal)
if (!displaced->in_progress ())
return DISPLACED_STEP_FINISH_STATUS_OK;
+ /* We no longer need to be notified about the creation of any new
+ threads. */
+ target_thread_events (false);
+
gdb_assert (event_thread->inf->displaced_step_state.in_progress_count > 0);
event_thread->inf->displaced_step_state.in_progress_count--;
@@ -2179,6 +2183,18 @@ do_target_resume (ptid_t resume_ptid, bool step, enum gdb_signal sig)
else
target_pass_signals (signal_pass);
+ /* If we are performing a step-over (in-place) then we will only be
+ setting a single thread running. We don't want any new thread
+ (spawned by the step) to start running, so request that the target
+ stop the thread.
+
+ Alternatively, if we are doing a displaced step then we will need to
+ fix the $pc address in any newly created threads, so again, in that
+ case, request that the target stop any new threads. */
+ if (step_over_info_valid_p ()
+ || displaced_step_in_progress (tp->inf))
+ target_thread_events (true);
+
target_resume (resume_ptid, step, sig);
if (target_can_async_p ())
@@ -5885,6 +5901,10 @@ finish_step_over (struct execution_control_state *ecs)
back an event. */
gdb_assert (ecs->event_thread->control.trap_expected);
+ /* We no longer need to be notified about any new threads that are
+ created. */
+ target_thread_events (false);
+
clear_step_over_info ();
}
diff --git a/gdb/testsuite/gdb.threads/stepi-over-clone.c b/gdb/testsuite/gdb.threads/stepi-over-clone.c
new file mode 100644
index 00000000000..0cb7007a8aa
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/stepi-over-clone.c
@@ -0,0 +1,90 @@
+/* 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 <stdio.h>
+#include <pthread.h>
+#include <unistd.h>
+#include <signal.h>
+#include <stdlib.h>
+
+/* Set this to non-zero from GDB to start a third worker thread. */
+volatile int start_third_thread = 0;
+
+void *
+thread_worker_2 (void *arg)
+{
+ int i;
+
+ printf ("Hello from the third thread.\n");
+ fflush (stdout);
+
+ for (i = 0; i < 600; ++i)
+ sleep (1);
+
+ return NULL;
+}
+
+void *
+thread_worker_1 (void *arg)
+{
+ int i;
+ pthread_t thr;
+ void *val;
+
+ if (start_third_thread)
+ pthread_create (&thr, NULL, thread_worker_2, NULL);
+
+ printf ("Hello from the first thread.\n");
+ fflush (stdout);
+
+ for (i = 0; i < 600; ++i)
+ sleep (1);
+
+ if (start_third_thread)
+ pthread_join (thr, &val);
+
+ return NULL;
+}
+
+void *
+thread_idle_loop (void *arg)
+{
+ int i;
+
+ for (i = 0; i < 600; ++i)
+ sleep (1);
+
+ return NULL;
+}
+
+int
+main ()
+{
+ pthread_t thr, thr_idle;
+ void *val;
+
+ if (getenv ("MAKE_EXTRA_THREAD") != NULL)
+ pthread_create (&thr_idle, NULL, thread_idle_loop, NULL);
+
+ pthread_create (&thr, NULL, thread_worker_1, NULL);
+ pthread_join (thr, &val);
+
+ if (getenv ("MAKE_EXTRA_THREAD") != NULL)
+ pthread_join (thr_idle, &val);
+
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.threads/stepi-over-clone.exp b/gdb/testsuite/gdb.threads/stepi-over-clone.exp
new file mode 100644
index 00000000000..8f0842cbe4f
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/stepi-over-clone.exp
@@ -0,0 +1,328 @@
+# 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 performing a 'stepi' over a clone syscall instruction.
+
+# This test relies on us being able to spot syscall instruction in
+# disassembly output. For now this is only implemented for x86-64.
+if { ![istarget x86_64-*-* ] } {
+ return
+}
+
+standard_testfile
+if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
+ executable [list debug additional_flags=-static]] != ""} {
+ return -1
+}
+
+clean_restart ${binfile}
+runto_main
+
+# Arrange to catch the 'clone' syscall, run until we catch the syscall, and
+# try to figure out the address of the actual syscall instruction so we can
+# place a breakpoint at this address.
+
+gdb_test_multiple "catch syscall clone" "" {
+ -re "The feature \'catch syscall\' is not supported.*\r\n$gdb_prompt $" {
+ return -1
+ }
+ -re ".*$gdb_prompt $" {
+ pass $gdb_test_name
+ }
+}
+
+gdb_test "continue" \
+ "Catchpoint $decimal \\(call to syscall clone\\), $hex in clone \\(\\)"
+
+proc is_syscall_insn { insn } {
+ if [istarget x86_64-*-* ] {
+ return { $insn == "syscall" }
+ } else {
+ return 0
+ }
+}
+
+set syscall_addrs {}
+gdb_test_multiple "disassemble" "" {
+ -re "Dump of assembler code for function \[^\r\n\]+:\r\n" {
+ exp_continue
+ }
+ -re "^(?:=>)?\\s+(${hex})\\s+<\\+${decimal}>:\\s+(\[^\r\n\]+)\r\n" {
+ set addr $expect_out(1,string)
+ set insn [string trim $expect_out(2,string)]
+ if [is_syscall_insn $insn] {
+ verbose -log "Found a syscall at: $addr"
+ lappend syscall_addrs $addr
+ }
+ exp_continue
+ }
+ -re "^End of assembler dump\\.\r\n$gdb_prompt $" {
+ if { [llength $syscall_addrs] == 0 } {
+ unsupported "no syscalls found"
+ return -1
+ }
+ }
+}
+
+# The test proc. NON_STOP and DISPLACED are either 'on' or 'off', and are
+# used to configure how GDB starts up. THIRD_THREAD is either True or False,
+# and is used to configure the inferior.
+proc test {non_stop displaced third_thread} {
+ global binfile srcfile
+ global syscall_addrs
+ global GDBFLAGS
+ global gdb_prompt hex decimal
+
+ for { set i 0 } { $i < 3 } { incr i } {
+ with_test_prefix "i=$i" {
+
+ # Arrange to start GDB in the correct mode.
+ save_vars { GDBFLAGS } {
+ append GDBFLAGS " -ex \"set non-stop $non_stop\""
+ append GDBFLAGS " -ex \"set displaced $displaced\""
+ clean_restart $binfile
+ }
+
+ runto_main
+
+ # Setup breakpoints at all the syscall instructions we
+ # might hit. We need to do this each time around the loop
+ # as runto_main deletes all breakpoints.
+ foreach addr $syscall_addrs {
+ gdb_test "break *${addr}" ".*"
+ }
+
+ # Continue until we hit the syscall.
+ gdb_test "continue"
+
+ if { $third_thread } {
+ gdb_test_no_output "set start_third_thread=1"
+ }
+
+ set stepi_error_count 0
+ set stepi_new_thread_count 0
+ set thread_1_stopped False
+ set thread_2_stopped False
+ set seen_prompt False
+
+ gdb_test_multiple "stepi" "" {
+ -re "^stepi\r\n" {
+ verbose -log "APB: Consume the initial command"
+ exp_continue
+ }
+ -re "^\\\[New Thread\[^\r\n\]+\\\]\r\n" {
+ verbose -log "APB: Consume new thread line"
+ incr stepi_new_thread_count
+ exp_continue
+ }
+ -re "^\\\[Switching to Thread\[^\r\n\]+\\\]\r\n" {
+ verbose -log "APB: Consume switching to thread line"
+ exp_continue
+ }
+ -re "^\\s*\r\n" {
+ verbose -log "APB: Consume blank line"
+ exp_continue
+ }
+ -re "^Hello from the first thread\\.\r\n" {
+ verbose -log "APB: Consume first worker thread message"
+ if { $third_thread } {
+ # If we are going to start a third thread then GDB
+ # should hit the breakpoint in clone before printing
+ # this message.
+ incr stepi_error_count
+ }
+ exp_continue
+ }
+ -re "^Hello from the third thread\\.\r\n" {
+ # We should never see this message.
+ verbose -log "APB: Consume third worker thread message"
+ incr stepi_error_count
+ exp_continue
+ }
+ -re "^$hex in clone \\(\\)\r\n" {
+ verbose -log "APB: Consume stop location line"
+ set thread_1_stopped True
+ if { !$seen_prompt } {
+ verbose -log "APB: Continuing to look for the prompt"
+ exp_continue
+ }
+ }
+ -re "^$gdb_prompt $" {
+ verbose -log "APB: Consume the final prompt"
+ gdb_assert { $stepi_error_count == 0 }
+ gdb_assert { $stepi_new_thread_count == 1 }
+ set seen_prompt True
+ if { $third_thread } {
+ if { $non_stop } {
+ # In non-stop mode if we are trying to start a
+ # third thread (from the second thread), then the
+ # second thread should hit the breakpoint in clone
+ # before actually starting the third thread. And
+ # so, at this point both thread 1, and thread 2
+ # should now be stopped.
+ if { !$thread_1_stopped || !$thread_2_stopped } {
+ verbose -log "APB: Continue looking for an additional stop event"
+ exp_continue
+ }
+ } else {
+ # All stop mode. Something should have stoppped
+ # by now otherwise we shouldn't have a prompt, but
+ # we can't know which thread will have stopped as
+ # that is a race condition.
+ gdb_assert { $thread_1_stopped || $thread_2_stopped }
+ }
+ }
+ }
+ -re "^Thread 2\[^\r\n\]+ hit Breakpoint $decimal, $hex in clone \\(\\)\r\n" {
+ verbose -log "APB: Consume thread 2 hit breakpoint"
+ set thread_2_stopped True
+ if { !$seen_prompt } {
+ verbose -log "APB: Continuing to look for the prompt"
+ exp_continue
+ }
+ }
+ -re "^PC register is not available\r\n" {
+ # This is the error we see for remote targets.
+ verbose -log "APB: Consume error line"
+ incr stepi_error_count
+ exp_continue
+ }
+ -re "^Couldn't get registers: No such process\\.\r\n" {
+ # This is the error we see for native linux targets.
+ verbose -log "APB: Consume error line"
+ incr stepi_error_count
+ exp_continue
+ }
+ }
+
+ # Ensure we are back at a GDB prompt, resynchronise.
+ verbose -log "APB: Have completed scanning the 'stepi' output"
+ gdb_test "p 1 + 2 + 3" " = 6"
+
+ # Check the number of threads we have, it should be exactly two.
+ set thread_count 0
+ set bad_threads 0
+
+ # Build up our expectations for what the current thread state
+ # should be. Thread 1 is the easiest, this is the thread we are
+ # stepping, so this thread should always be stopped, and should
+ # always still be in clone.
+ set match_code {}
+ lappend match_code {
+ -re "\\*?\\s+1\\s+Thread\[^\r\n\]+clone \\(\\)\r\n" {
+ incr thread_count
+ exp_continue
+ }
+ }
+
+ # What state should thread 2 be in?
+ if { $non_stop == "on" } {
+ if { $third_thread } {
+ # With non-stop mode on, and creation of a third thread
+ # having been requested, we expect Thread 2 to exist, and
+ # be stopped at the breakpoint in clone (just before the
+ # third thread is actually created).
+ lappend match_code {
+ -re "\\*?\\s+2\\s+Thread\[^\r\n\]+$hex in clone \\(\\)\r\n" {
+ incr thread_count
+ exp_continue
+ }
+ -re "\\*?\\s+2\\s+Thread\[^\r\n\]+\\(running\\)\r\n" {
+ incr thread_count
+ incr bad_threads
+ exp_continue
+ }
+ -re "\\*?\\s+2\\s+Thread\[^\r\n\]+\r\n" {
+ verbose -log "APB: thread 2 is bad, unknown state"
+ incr thread_count
+ incr bad_threads
+ exp_continue
+ }
+ }
+
+ } else {
+ # With non-stop mode on, and no third thread having been
+ # requested, then we expect Thread 2 to exist, and still
+ # be running.
+ lappend match_code {
+ -re "\\*?\\s+2\\s+Thread\[^\r\n\]+\\(running\\)\r\n" {
+ incr thread_count
+ exp_continue
+ }
+ -re "\\*?\\s+2\\s+Thread\[^\r\n\]+\r\n" {
+ verbose -log "APB: thread 2 is bad, unknown state"
+ incr thread_count
+ incr bad_threads
+ exp_continue
+ }
+ }
+ }
+ } else {
+ # With non-stop mode off then we expect Thread 2 to exist, and
+ # be stopped. We don't have any guarantee about where the
+ # thread will have stopped though, so we need to be vague.
+ lappend match_code {
+ -re "\\*?\\s+2\\s+Thread\[^\r\n\]+\\(running\\)\r\n" {
+ verbose -log "APB: thread 2 is bad, unexpetedly running"
+ incr thread_count
+ incr bad_threads
+ exp_continue
+ }
+ -re "\\*?\\s+2\\s+Thread\[^\r\n\]+\r\n" {
+ incr thread_count
+ exp_continue
+ }
+ }
+ }
+
+ # We don't expect to ever see a thread 3. Even when we are
+ # requesting that this third thread be created, thread 2, the
+ # thread that creates thread 3, should stop before executing the
+ # clone syscall. So, if we do ever see this then something has
+ # gone wrong.
+ lappend match_code {
+ -re "\\s+3\\s+Thread\[^\r\n\]+\r\n" {
+ incr thread_count
+ incr bad_threads
+ exp_continue
+ }
+ }
+
+ lappend match_code {
+ -re "$gdb_prompt $" {
+ gdb_assert { $thread_count == 2 }
+ gdb_assert { $bad_threads == 0 }
+ }
+ }
+
+ set match_code [join $match_code]
+ gdb_test_multiple "info threads" "" $match_code
+ }
+ }
+}
+
+# Run the test in all suitable configurations.
+foreach_with_prefix third_thread { False True } {
+ foreach_with_prefix non-stop { "on" "off" } {
+ foreach_with_prefix displaced { "off" "on" } {
+ if { $displaced == "on" } {
+ kfail "gdb/19675" "can't displaced step over clone"
+ continue
+ }
+ # Pass False as the last argument to leave QThreadEvents enabled.
+ test ${non-stop} ${displaced} ${third_thread}
+ }
+ }
+}
diff --git a/gdbserver/linux-low.cc b/gdbserver/linux-low.cc
index 5c6191d941c..3a44728ff0a 100644
--- a/gdbserver/linux-low.cc
+++ b/gdbserver/linux-low.cc
@@ -1646,13 +1646,14 @@ linux_process_target::thread_still_has_status_pending (thread_info *thread)
if (!lp->status_pending_p)
return 0;
+ int discard = 0;
+
if (thread->last_resume_kind != resume_stop
&& (lp->stop_reason == TARGET_STOPPED_BY_SW_BREAKPOINT
|| lp->stop_reason == TARGET_STOPPED_BY_HW_BREAKPOINT))
{
struct thread_info *saved_thread;
CORE_ADDR pc;
- int discard = 0;
gdb_assert (lp->last_status != 0);
@@ -1689,16 +1690,28 @@ linux_process_target::thread_still_has_status_pending (thread_info *thread)
#endif
current_thread = saved_thread;
-
- if (discard)
+ }
+ else if (lp->waitstatus.kind == TARGET_WAITKIND_THREAD_CREATED)
+ {
+ /* If this thread stopped due to a thread created event, but GDB has
+ since told us it no longer cares about these events, then just
+ ignore this event. */
+ client_state &cs = get_client_state ();
+ if (!cs.report_thread_events)
{
- if (debug_threads)
- debug_printf ("discarding pending breakpoint status\n");
- lp->status_pending_p = 0;
- return 0;
+ lp->waitstatus.kind = TARGET_WAITKIND_IGNORE;
+ discard = 1;
}
}
+ if (discard)
+ {
+ if (debug_threads)
+ debug_printf ("discarding pending thread event\n");
+ lp->status_pending_p = 0;
+ return 0;
+ }
+
return 1;
}
--
2.25.4
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 3/3] gdb: non-displaced step over clone, remote target, no QThreadEvents
2021-06-30 19:55 [PATCH 0/3] Stepping over clone syscall Andrew Burgess
2021-06-30 19:55 ` [PATCH 1/3] gdb/testsuite: update test gdb.base/step-over-syscall.exp Andrew Burgess
2021-06-30 19:55 ` [PATCH 2/3] gdb: support stepping over a clone syscall with displaced stepping off Andrew Burgess
@ 2021-06-30 19:55 ` Andrew Burgess
2021-07-05 19:19 ` Simon Marchi
2 siblings, 1 reply; 7+ messages in thread
From: Andrew Burgess @ 2021-06-30 19:55 UTC (permalink / raw)
To: gdb-patches
This commit compliments the previous commit. While the previous
commit aims to offer a solution for non-displaced stepping over a
clone syscall, the solution depends on a target supporting the
QThreadEvents packet.
As this packet is optional we should consider what would happen if a
target does not support this packet.
The short answer is that, without QThreadEvents support, GDB can't
correctly support non-displaced stepping of a clone syscall, all of
the bugs discussed in the previous commit can still be hit.
However, when GDB is in non-stop mode, and we do a non-displaced step
over a clone syscall instruction, for a remote target that does not
support QThreadEvents, then we run into an error, like this:
(gdb) stepi
[New Thread 2914644.2914648]
PC register is not available
The 'PC register is not available' is an error which is caused by GDB
attempting to read the $pc from a running thread.
The problem here is that when the remote target sees the new thread,
the thread is created in the running state, this is good because we
know that the remote will have set the new thread running already.
So, the thread_info::executing field will be true (indicating that the
target believes the thread is executing), but the thread_info::resumed
field will be false as infrun did not resume the thread.
Now in non-stop mode, when performing a non-displaced step, all
threads are stopped, once the step is complete GDB calls
infrun.c:restart_threads to restart all of the threads that GDB
previously stopped. The restart_threads function loops over all
threads, and looks that the thread state, if the thread needs
restarting then this is done.
However, the restart_threads function does not consider the case where
a thread might be executing, but not resumed, and so GDB tries to
restart the already executing thread, this requires a read of $pc,
which then fails.
In this commit I propose to add a new case to restart_threads, this
will spot the case where a new thread is already executing, but not
resumed. This thread will then be moved in to the resumed state with
no further action required. That is we acknowledge that the thread is
already executing, and just infrun just updates its internal state so
it thinks that it asked the thread to resume.
I updated an existing test to cover this case and tested using native
GNU/Linux target, native-gdbserver, and native-extended-gdbserver.
gdb/ChangeLog:
PR gdb/27830
* infrun.c (restart_threads): Threads that are already executing
in the target are already resumed.
gdb/testsuite/ChangeLog:
PR gdb/27830
* gdb.threads/stepi-over-clone.exp: Add additional tests.
---
gdb/ChangeLog | 6 +++++
gdb/infrun.c | 11 ++++++++
gdb/testsuite/ChangeLog | 5 ++++
.../gdb.threads/stepi-over-clone.exp | 25 +++++++++++++++----
4 files changed, 42 insertions(+), 5 deletions(-)
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 98ce8c60b55..288ad7f75bd 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -5819,6 +5819,17 @@ restart_threads (struct thread_info *event_thread)
continue;
}
+ /* If the target already thinks the thread is executing, but it is
+ not marked resumed, then this is a new thread that infrun has not
+ seen before. It can just be marked as resumed. */
+ if (tp->executing)
+ {
+ infrun_debug_printf ("restart threads: [%s] already executing",
+ target_pid_to_str (tp->ptid).c_str ());
+ tp->resumed = true;
+ continue;
+ }
+
if (thread_is_in_step_over_chain (tp))
{
infrun_debug_printf ("restart threads: [%s] needs step-over",
diff --git a/gdb/testsuite/gdb.threads/stepi-over-clone.exp b/gdb/testsuite/gdb.threads/stepi-over-clone.exp
index 8f0842cbe4f..3d4f13c0652 100644
--- a/gdb/testsuite/gdb.threads/stepi-over-clone.exp
+++ b/gdb/testsuite/gdb.threads/stepi-over-clone.exp
@@ -76,10 +76,12 @@ gdb_test_multiple "disassemble" "" {
}
}
-# The test proc. NON_STOP and DISPLACED are either 'on' or 'off', and are
-# used to configure how GDB starts up. THIRD_THREAD is either True or False,
-# and is used to configure the inferior.
-proc test {non_stop displaced third_thread} {
+# The test proc. NON_STOP and DISPLACED are either 'on' or 'off', and
+# are used to configure how GDB starts up. THIRD_THREAD is either
+# True or False, and is used to adjust how the test inferior runs.
+# DISABLE_QTHREADEVENTS is either True or False and is used to disable
+# use of the QThreadEvents packet within GDB.
+proc test {non_stop displaced third_thread disable_QThreadEvents } {
global binfile srcfile
global syscall_addrs
global GDBFLAGS
@@ -95,6 +97,10 @@ proc test {non_stop displaced third_thread} {
clean_restart $binfile
}
+ if { $disable_QThreadEvents } {
+ gdb_test_no_output "set remote thread-events-packet off"
+ }
+
runto_main
# Setup breakpoints at all the syscall instructions we
@@ -322,7 +328,16 @@ foreach_with_prefix third_thread { False True } {
continue
}
# Pass False as the last argument to leave QThreadEvents enabled.
- test ${non-stop} ${displaced} ${third_thread}
+ test ${non-stop} ${displaced} ${third_thread} False
}
}
}
+
+if {[target_info exists gdb_protocol]
+ && ([target_info gdb_protocol] == "remote"
+ || [target_info gdb_protocol] == "extended-remote")} {
+ # For remote targets, run this test again, non-stop mode on, displaced
+ # stepping off, no creation of a third thread, but do disable the
+ # QThreadEvents packet.
+ test "on" "off" False True
+}
--
2.25.4
^ permalink raw reply [flat|nested] 7+ messages in thread