The gdb.base/watchpoint.exp on Windows with non-stop support added
(later in the series) exposed an issue with the currently_stepping
logic when tested with software watchpoints.
The issue only happens when:
- You have multiple threads. gdb.base/watchpoint.exp exposed it on
Windows because there the OS always spawns a few extra threads.
- Displaced stepping is not available. The Windows non-stop work
does not implement displaced stepping yet. That is left as an
optimization for later.
- The target backend is working in non-stop mode.
I've written a new test that exposes the issue on GNU/Linux as well
(on hardware single-step targets, like x86-64). There, we see:
continue
Continuing.
../../src/gdb/infrun.c:2918: internal-error: resume_1: Assertion `!(thread_has_single_step_breakpoints_set (tp) && step)' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
----- Backtrace -----
FAIL: gdb.threads/sw-watchpoint-step-over-bp-with-threads.exp: target-non-stop=on: displaced-stepping=off: continue until exit (GDB internal error)
Currently, software watchpoints are implemented by forcing
single-stepping. That is done by currently_stepping returning true
when we have a software watchpoint. proceed calls resume, which calls
resume_1, which then ends up always requesting a single-step resume,
even if the higher layers wanted a continue.
Now, if you set a software watchpoint, and then continue the program,
and there's a breakpoint at the current PC, GDB needs to step over
that breakpoint first. If displaced stepping is not available, then
GDB temporarily pauses all threads, removes the breakpoint,
single-steps the thread that needs to move past the breakpoint, and
then finally, reinserts the breakpoint, and restarts all threads
again. That last restarting step happens in the restart_threads
infrun function.
restart_threads iterates over all threads trying to restart them one
by one. There, we have:
if (currently_stepping (tp))
{
infrun_debug_printf ("restart threads: [%s] was stepping",
tp->ptid.to_string ().c_str ());
but, what if TP is actually a new thread that hasn't yet ever set
stepping? currently_stepping still returns true, due to the software
watchpoint, and we end up in keep_going_stepped_thread, here:
if (tp->stop_pc () != tp->prev_pc)
{
ptid_t resume_ptid;
infrun_debug_printf ("expected thread advanced also (%s -> %s)",
paddress (current_inferior ()->arch (), tp->prev_pc),
paddress (current_inferior ()->arch (),
tp->stop_pc ()));
... because prev_pc was stale at that point (we had no reason to
update it earlier). E.g. on Windows we see something like:
[infrun] restart_threads: start: event_thread=
1867996.
1867996.0, inf=-1
[infrun] restart_threads: restart threads: [
1867996.
1867996.0] is event thread
[infrun] restart_threads: restart threads: [
1867996.
1868003.0] was stepping
[infrun] keep_going_stepped_thread: resuming previously stepped thread
[infrun] keep_going_stepped_thread: expected thread advanced also (0 -> 0x7ffff7ce57f8)
[infrun] clear_step_over_info: clearing step over info
[infrun] do_target_resume: resume_ptid=
1867996.
1868003.0, step=0, sig=GDB_SIGNAL_0
On GNU/Linux, we may see:
[infrun] keep_going_stepped_thread: expected thread advanced also (0x7ffff7d2683d -> 0x7ffff7ce57f8)
there prev_pc might have been updated on an earlier proceed call,
which makes the issue harder to see, but it is stale too here.
That means we insert a single-step breakpoint at the current PC, and
continue the thread, with target_resume directly, asking for a normal
continue.
Eventually, something causes a user-visible stop. For example, the
software watchpoint triggers. That makes GDB stop all threads.
Now, if the user re-resumes the program, say, with "continue", we fail
this assertion in resume_1 coming from proceed:
/* If STEP is set, it's a request to use hardware stepping
facilities. But in that case, we should never
use singlestep breakpoint. */
gdb_assert (!(thread_has_single_step_breakpoints_set (tp) && step));
"step" is true because currently_stepping returns true since we have a
software watchpoint. And the thread has a single-step breakpoint
installed from earlier, because of that code mentioned above, in
keep_going_stepped_thread reached from restart_threads.
This patch fixes the root cause -- the currently_stepping call in
restart_threads returned true for a thread that has never set stepping
in the first place. This is because currently_stepping really serves
two purposes currently:
#1 - for a thread that we are about to resume, should we set it
stepping?
#2 - for a thread that just stopped, was it stepping before?
The fix is thus to decouple those two aspects:
- for #1, we simply rename currently_stepping to should_step.
- for #2, we record whether the thread was stepping before in a new
currently_stepping flag in thread_info.
As mentioned, there's a new testcase included. I tested this on
x86-64 GNU/Linux, native and gdbserver, and on Windows x64 with the
non-stop series. The assertion triggers on all of those with the fix,
and is fixed by this patch on all of those, too.
Change-Id: Id7aa00692531450695771f8110893cc25626262f
the finished single step. */
int trap_expected = 0;
+ /* True if the thread TP is in the middle of (software or hardware)
+ single-stepping. */
+ bool currently_stepping = false;
+
/* Nonzero if the thread is being proceeded for a "finish" command
or a similar situation when return value should be printed. */
int proceed_to_finish = 0;
static void follow_inferior_reset_breakpoints (void);
-static bool currently_stepping (struct thread_info *tp);
+static bool should_step (thread_info *tp);
static void insert_hp_step_resume_breakpoint_at_frame (const frame_info_ptr &);
"status %s (currently_stepping=%d).",
tp->ptid.to_string ().c_str (),
tp->pending_waitstatus ().to_string ().c_str (),
- currently_stepping (tp));
+ tp->control.currently_stepping);
tp->inf->process_target ()->threads_executing = true;
tp->set_resumed (true);
tp->stepped_breakpoint = 0;
/* Depends on stepped_breakpoint. */
- step = currently_stepping (tp);
+ step = tp->control.currently_stepping = should_step (tp);
if (current_inferior ()->thread_waiting_for_vfork_done != nullptr)
{
("thread %s has pending wait status %s (currently_stepping=%d).",
tp->ptid.to_string ().c_str (),
tp->pending_waitstatus ().to_string ().c_str (),
- currently_stepping (tp));
+ tp->control.currently_stepping);
}
}
we also need to back up to the breakpoint address. */
if (thread_has_single_step_breakpoints_set (thread)
- || !currently_stepping (thread)
+ || !thread->control.currently_stepping
|| (thread->stepped_breakpoint
&& thread->prev_pc == breakpoint_pc))
regcache_write_pc (regcache, breakpoint_pc);
&& software_breakpoint_inserted_here_p (aspace, pc))
tp->set_stop_reason (TARGET_STOPPED_BY_SW_BREAKPOINT);
else if (!thread_has_single_step_breakpoints_set (tp)
- && currently_stepping (tp))
+ && tp->control.currently_stepping)
tp->set_stop_reason (TARGET_STOPPED_BY_SINGLE_STEP);
}
}
paddress (current_inferior ()->arch (),
t->stop_pc ()),
t->ptid.to_string ().c_str (),
- currently_stepping (t));
+ t->control.currently_stepping);
}
}
tp->ptid.to_string ().c_str ());
}
- if (currently_stepping (tp))
+ if (tp->control.currently_stepping)
{
infrun_debug_printf ("restart threads: [%s] was stepping",
tp->ptid.to_string ().c_str ());
paddress (current_inferior ()->arch (),
tp->stop_pc ()),
tp->ptid.to_string ().c_str (),
- currently_stepping (tp));
+ tp->control.currently_stepping);
/* This in-line step-over finished; clear this so we won't
start a new one. This is what handle_signal_stop would
/* If not, perhaps stepping/nexting can. */
if (random_signal)
random_signal = !(ecs->event_thread->stop_signal () == GDB_SIGNAL_TRAP
- && currently_stepping (ecs->event_thread));
+ && ecs->event_thread->control.currently_stepping);
/* Perhaps the thread hit a single-step breakpoint of _another_
thread. Single-step breakpoints are transparent to the
return true;
}
-/* Is thread TP in the middle of (software or hardware)
- single-stepping? (Note the result of this function must never be
- passed directly as target_resume's STEP parameter.) */
+/* Should thread TP be stepped (software or hardware)? (Note the
+ result of this function must never be passed directly as
+ target_resume's STEP parameter.) */
static bool
-currently_stepping (struct thread_info *tp)
+should_step (thread_info *tp)
{
return ((tp->control.step_range_end
&& tp->control.step_resume_breakpoint == nullptr)
--- /dev/null
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2025 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 <unistd.h>
+
+static pthread_barrier_t threads_started_barrier;
+
+static void *
+thread_func (void *arg)
+{
+ pthread_barrier_wait (&threads_started_barrier);
+
+ while (1)
+ sleep (1);
+
+ return NULL;
+}
+
+static void
+dummy ()
+{
+}
+
+static unsigned watched_global = 0;
+
+int
+main (void)
+{
+ pthread_t thread;
+ int ret;
+
+ alarm (30);
+
+ pthread_barrier_init (&threads_started_barrier, NULL, 2);
+
+ ret = pthread_create (&thread, NULL, thread_func, NULL);
+ assert (ret == 0);
+
+ /* Make sure all threads are scheduled before hitting the
+ breakpoint. */
+ pthread_barrier_wait (&threads_started_barrier);
+
+ ++watched_global; /* break here start */
+
+ dummy (); /* just so there's extra code here */
+
+ return 0; /* break here end */
+}
--- /dev/null
+# Copyright 2025 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 continuing with a software watchpoint installed, when there are
+# multiple threads, and previously we stepped over a breakpoint.
+#
+# This is a regression test for a GDB bug where stepping over a
+# breakpoint in-line made GDB insert a software single-step breakpoint
+# in some threads by mistake, which later would cause an assertion to
+# fail.
+#
+# The issue only triggered when:
+#
+# - The program has multiple threads.
+# - The target backend is working in non-stop mode.
+# - Displaced stepping is not available.
+# - The target supports hardware single-step.
+#
+# However, we exercise more combinations for completeness.
+
+standard_testfile .c
+
+if { [build_executable "failed to prepare" $testfile $srcfile \
+ {debug pthreads}] \
+ == -1 } {
+ return
+}
+
+proc test {target-non-stop displaced-stepping} {
+
+ save_vars ::GDBFLAGS {
+ append ::GDBFLAGS " -ex \"maintenance set target-non-stop ${target-non-stop}\""
+ append ::GDBFLAGS " -ex \"set displaced ${displaced-stepping}\""
+ clean_restart $::binfile
+ }
+
+ if ![runto_main] {
+ return
+ }
+
+ # Run to a breakpoint, and leave it installed, so that GDB needs
+ # to step over it before continuing.
+ gdb_breakpoint [gdb_get_line_number "break here start"]
+ gdb_continue_to_breakpoint "started"
+
+ # GDB should know about at least two threads by now.
+ gdb_test "p \$_inferior_thread_count >= 2" " = 1"
+
+ # Set a software watchpoint. This makes GDB single-step all
+ # instructions when we next continue.
+ gdb_test_no_output "set can-use-hw-watchpoints 0"
+ gdb_test "watch watched_global" "Watchpoint $::decimal: watched_global"
+
+ # Continue with the software watchpoint in place. In the original
+ # bug, with displaced stepping disabled, this would make GDB
+ # incorrectly install a software single-step breakpoint on threads
+ # other than the main one.
+ gdb_test_multiple "cont" "continue to watchpoint" {
+ -re -wrap "Continuing.*Watchpoint.*watched_global.*Old value = 0.*New value = 1.*" {
+ pass $gdb_test_name
+ }
+ }
+
+ # The final continue, with the software watchpoint set, so that
+ # GDB single-steps all threads (if the target is non-stop). With
+ # the buggy GDB, the non-main thread had a software single-step
+ # breakpoint set, and on hardware single-step targets, GDB would
+ # fail an assertion that checks that we never ask the target to
+ # hardware single-step a thread when we have a software
+ # single-step breakpoint set for that thread.
+ gdb_breakpoint [gdb_get_line_number "break here end"]
+ gdb_continue_to_breakpoint "end"
+}
+
+foreach_with_prefix target-non-stop {auto on off} {
+ foreach_with_prefix displaced-stepping {auto on off} {
+ test ${target-non-stop} ${displaced-stepping}
+ }
+}
projects / thirdparty / binutils-gdb.git / commitdiff
