* Re: [PATCH 5/6] Handle multiple step-overs
@ 2014-03-26 16:54 Alan Lawrence
2014-04-04 11:54 ` Pedro Alves
2014-04-22 18:42 ` Pedro Alves
0 siblings, 2 replies; 6+ messages in thread
From: Alan Lawrence @ 2014-03-26 16:54 UTC (permalink / raw)
To: Pedro Alves, gdb-patches
Following this patch, we're seeing an assertion failure of infrun.c:5192,
gdb_assert (!tp->control.trap_expected);
on the AArch64 platform. The testcase is that added in git commit
beb460e8d2ddf5327a6ab146055a6e6e9f552a4b, condbreak-call-false.{c,exp} - I've
tried this testcase both before and after your multiple step-over patch, and it
succeeds without the patch. I'm not very familiar with gdb internals and
stepwise comparing AArch64 against ARM (on which the test passes) sounds at best
laborious; hoping there may be some experts here who can help?
I have reproduced the failure on (a) qemu (-2.0.0-rc0) running on x86_64 host,
(b) native AArch64 hardware, (c) running cross gdb hosted on x86_64-linux
against remote target running natively on aarch64-linux; in the latter case,
it's the host gdb which is critical (i.e.: fails/succeeds with/out the multiple
stepover patch, regardless of whether the remote gdbserver includes this patch
or not); remote gdbserver succeeds in all 4=(remote/local)(with/without) cases.
firstly:
$ aarch64-none-linux-gnu-gcc -g -o condbreak-call-false
binutils-gdb/gdb/testsuite/gdb.base/condbreak-call-false.c
$ aarch64-linux-user/qemu-aarch64 -g 23456 -L $MY_QEMU_LIB_PATH
condbreak-call-false &
then:
$ aarch64-none-linux-gnu-gdb
GNU gdb (unknown) 7.7.50.20140326-cvs
Copyright (C) 2014 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law. Type "show copying"
and "show warranty" for details.
This GDB was configured as "--host=x86_64-unknown-linux-gnu
--target=aarch64-none-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
<http://www.gnu.org/software/gdb/documentation/>.
For help, type "help".
Type "apropos word" to search for commands related to "word".
(gdb) file condbreak-call-false
Reading symbols from condbreak-call-false...done.
(gdb) target remote localhost:23456
Remote debugging using dsgci-1.euhpc.arm.com:23456
warning: Unable to find dynamic linker breakpoint function.
GDB will be unable to debug shared library initializers
and track explicitly loaded dynamic code.
0x0000004000a01d00 in ?? ()
(gdb) break main
Breakpoint 1 at 0x400504: file
srcfsf/binutils-gdb/gdb/testsuite/gdb.base/condbreak-call-false.c, line 37.
(gdb) continue
Continuing.
warning: `/lib64/libc.so.6': Shared library architecture unknown is not
compatible with target architecture aarch64.
warning: Could not load shared library symbols for /lib/ld-linux-aarch64.so.1.
Do you need "set solib-search-path" or "set sysroot"?
Breakpoint 1, main ()
at srcfsf/binutils-gdb/gdb/testsuite/gdb.base/condbreak-call-false.c:37
37 foo ();
(gdb) break foo if zero()
Breakpoint 2 at 0x4004f0: file
srcfsf/binutils-gdb/gdb/testsuite/gdb.base/condbreak-call-false.c, line 25.
(gdb) continue
Continuing.
/work/alalaw01/oban/srca64t/binutils-gdb/gdb/infrun.c:5192: internal-error:
switch_back_to_stepped_thread: Assertion `!tp->control.trap_expected' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Quit this debugging session? (y or n)
or:
$ aarch64-none-elf-gdb
GNU gdb (unknown) 7.7.50.20140320-cvs
Copyright (C) 2014 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law. Type "show copying"
and "show warranty" for details.
This GDB was configured as "--host=x86_64-unknown-linux-gnu
--target=aarch64-none-elf".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
<http://www.gnu.org/software/gdb/documentation/>.
For help, type "help".
Type "apropos word" to search for commands related to "word".
(gdb) file condbreak-call-false
Reading symbols from condbreak-call-false...done.
(gdb) target remote localhost:23456
Remote debugging using localhost:23456
_start () at /work/alalaw01/oban/srcfsf/binutils-gdb/libgloss/aarch64/crt0.S:90
90 adr x1, .LC0
(gdb) break main
Breakpoint 1 at 0x4002d4: file
srcfsf/binutils-gdb/gdb/testsuite/gdb.base/condbreak-call-false.c, line 37.
(gdb) continue # can do this in either order with next
Continuing.
Breakpoint 1, main ()
at srcfsf/binutils-gdb/gdb/testsuite/gdb.base/condbreak-call-false.c:37
37 foo ();
(gdb) break foo if zero() # can do this in either order with previous
Breakpoint 2 at 0x4002c0: file
srcfsf/binutils-gdb/gdb/testsuite/gdb.base/condbreak-call-false.c, line 25.
(gdb) continue
Continuing.
/work/alalaw01/oban/srca64t/binutils-gdb/gdb/infrun.c:5192: internal-error:
switch_back_to_stepped_thread: Assertion `!tp->control.trap_expected' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Quit this debugging session? (y or n)
>
> From: Pedro Alves <palves at redhat dot com>
> To: gdb-patches at sourceware dot org
> Date: Tue, 25 Feb 2014 20:32:42 +0000
> Subject: [PATCH 5/6] Handle multiple step-overs.
> Authentication-results: sourceware.org; auth=none
> References: <1393360363-5603-1-git-send-email-palves at redhat dot com>
>
> This test fails with current mainline.
>
> If the program stopped for a breakpoint in thread 1, and then the user
> switches to thread 2, and resumes the program, GDB first switches back
> to thread 1 to step it over the breakpoint, in order to make progress.
>
> However, that logic only considers the last reported event, assuming
> only one thread needs that stepping over dance.
>
> That's actually not true when we play with scheduler-locking. The
> patch adds an example to the testsuite of multiple threads needing a
> step-over before the stepping thread can be resumed. With current
> mainline, the program re-traps the same breakpoint it had already
> trapped before.
>
> E.g.:
>
> Breakpoint 2, main () at ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c:99
> 99 wait_threads (); /* set wait-threads breakpoint here */
> (gdb) PASS: gdb.threads/multiple-step-overs.exp: step: continue to breakpoint: run to breakpoint
> info threads
> Id Target Id Frame
> 3 Thread 0x7ffff77c9700 (LWP 4310) "multiple-step-o" 0x00000000004007ca in child_function_3 (arg=0x1) at ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c:43
> 2 Thread 0x7ffff7fca700 (LWP 4309) "multiple-step-o" 0x0000000000400827 in child_function_2 (arg=0x0) at ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c:60
> * 1 Thread 0x7ffff7fcb740 (LWP 4305) "multiple-step-o" main () at ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c:99
> (gdb) PASS: gdb.threads/multiple-step-overs.exp: step: info threads shows all threads
> set scheduler-locking on
> (gdb) PASS: gdb.threads/multiple-step-overs.exp: step: set scheduler-locking on
> break 44
> Breakpoint 3 at 0x4007d3: file ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c, line 44.
> (gdb) break 61
> Breakpoint 4 at 0x40082d: file ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c, line 61.
> (gdb) thread 3
> [Switching to thread 3 (Thread 0x7ffff77c9700 (LWP 4310))]
> #0 0x00000000004007ca in child_function_3 (arg=0x1) at ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c:43
> 43 (*myp) ++;
> (gdb) PASS: gdb.threads/multiple-step-overs.exp: step: thread 3
> continue
> Continuing.
>
> Breakpoint 3, child_function_3 (arg=0x1) at ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c:44
> 44 callme (); /* set breakpoint thread 3 here */
> (gdb) PASS: gdb.threads/multiple-step-overs.exp: step: continue to breakpoint: run to breakpoint in thread 3
> p *myp = 0
> $1 = 0
> (gdb) PASS: gdb.threads/multiple-step-overs.exp: step: unbreak loop in thread 3
> thread 2
> [Switching to thread 2 (Thread 0x7ffff7fca700 (LWP 4309))]
> #0 0x0000000000400827 in child_function_2 (arg=0x0) at ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c:60
> 60 (*myp) ++;
> (gdb) PASS: gdb.threads/multiple-step-overs.exp: step: thread 2
> continue
> Continuing.
>
> Breakpoint 4, child_function_2 (arg=0x0) at ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c:61
> 61 callme (); /* set breakpoint thread 2 here */
> (gdb) PASS: gdb.threads/multiple-step-overs.exp: step: continue to breakpoint: run to breakpoint in thread 2
> p *myp = 0
> $2 = 0
> (gdb) PASS: gdb.threads/multiple-step-overs.exp: step: unbreak loop in thread 2
> thread 1
> [Switching to thread 1 (Thread 0x7ffff7fcb740 (LWP 4305))]
> #0 main () at ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c:99
> 99 wait_threads (); /* set wait-threads breakpoint here */
> (gdb) PASS: gdb.threads/multiple-step-overs.exp: step: thread 1
> set scheduler-locking off
> (gdb) PASS: gdb.threads/multiple-step-overs.exp: step: set scheduler-locking off
>
> At this point all thread are stopped for a breakpoint that needs stepping over.
>
> (gdb) step
>
> Breakpoint 2, main () at ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c:99
> 99 wait_threads (); /* set wait-threads breakpoint here */
> (gdb) FAIL: gdb.threads/multiple-step-overs.exp: step
>
> But that "step" retriggers the same breakpoint instead of making
> progress.
>
> The patch teaches GDB to step over all breakpoints of all threads
> before resuming the stepping thread.
>
> Tested on x86_64 Fedora 17, against pristine mainline, and also my
> branch that implements software single-stepping on x86.
>
> gdb/
> 2014-02-25 Pedro Alves <palves@redhat.com>
>
> * infrun.c (prepare_to_proceed): Delete.
> (thread_still_needs_step_over): New function.
> (find_thread_needs_step_over): New function.
> (proceed): If the current thread needs a step-over, set its
> steping_over_breakpoint flag. Adjust to use
> find_thread_needs_step_over instead of prepare_to_proceed.
> (process_event_stop_test): For BPSTAT_WHAT_STOP_NOISY and
> BPSTAT_WHAT_STOP_SILENT, assume the thread stopped for a
> breakpoint.
> (switch_back_to_stepped_thread): Step over breakpoints of all
> threads not the stepping thread, before switching back to the
> stepping thread.
>
> gdb/testsuite/
> 2014-02-25 Pedro Alves <palves@redhat.com>
>
> * gdb.threads/multiple-step-overs.c: New file.
> * gdb.threads/multiple-step-overs.exp: New file.
> * gdb.threads/signal-while-stepping-over-bp-other-thread.exp:
> Adjust expected infrun debug output.
> ---
> gdb/infrun.c | 238 ++++++++++++---------
> gdb/testsuite/gdb.threads/multiple-step-overs.c | 105 +++++++++
> gdb/testsuite/gdb.threads/multiple-step-overs.exp | 80 +++++++
> .../signal-while-stepping-over-bp-other-thread.exp | 2 +-
> 4 files changed, 326 insertions(+), 99 deletions(-)
> create mode 100644 gdb/testsuite/gdb.threads/multiple-step-overs.c
> create mode 100644 gdb/testsuite/gdb.threads/multiple-step-overs.exp
>
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index 1ea8d04..bde40c5 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -90,8 +90,6 @@ static int currently_stepping_or_nexting_callback (struct thread_info *tp,
>
> static void xdb_handle_command (char *args, int from_tty);
>
> -static int prepare_to_proceed (int);
> -
> static void print_exited_reason (int exitstatus);
>
> static void print_signal_exited_reason (enum gdb_signal siggnal);
> @@ -2053,75 +2051,74 @@ clear_proceed_status (void)
> }
> }
>
> -/* Check the current thread against the thread that reported the most recent
> - event. If a step-over is required return TRUE and set the current thread
> - to the old thread. Otherwise return FALSE.
> -
> - This should be suitable for any targets that support threads. */
> +/* Returns true if TP is still stopped at a breakpoint that needs
> + stepping-over in order to make progress. If the breakpoint is gone
> + meanwhile, we can skip the whole step-over dance. */
>
> static int
> -prepare_to_proceed (int step)
> +thread_still_needs_step_over (struct thread_info *tp)
> +{
> + if (tp->stepping_over_breakpoint)
> + {
> + struct regcache *regcache = get_thread_regcache (tp->ptid);
> +
> + if (breakpoint_here_p (get_regcache_aspace (regcache),
> + regcache_read_pc (regcache)))
> + return 1;
> +
> + tp->stepping_over_breakpoint = 0;
> + }
> +
> + return 0;
> +}
> +
> +/* Look a thread other than EXCEPT that has previously reported a
> + breakpoint event, and thus needs a step-over in order to make
> + progress. Returns NULL is none is found. STEP indicates whether
> + we're about to step the current thread, in order to decide whether
> + "set scheduler-locking step" applies. */
> +
> +static struct thread_info *
> +find_thread_needs_step_over (int step, struct thread_info *except)
> {
> - ptid_t wait_ptid;
> - struct target_waitstatus wait_status;
> int schedlock_enabled;
> + struct thread_info *tp, *current;
>
> /* With non-stop mode on, threads are always handled individually. */
> gdb_assert (! non_stop);
>
> - /* Get the last target status returned by target_wait(). */
> - get_last_target_status (&wait_ptid, &wait_status);
> -
> - /* Make sure we were stopped at a breakpoint. */
> - if (wait_status.kind != TARGET_WAITKIND_STOPPED
> - || (wait_status.value.sig != GDB_SIGNAL_TRAP
> - && wait_status.value.sig != GDB_SIGNAL_ILL
> - && wait_status.value.sig != GDB_SIGNAL_SEGV
> - && wait_status.value.sig != GDB_SIGNAL_EMT))
> - {
> - return 0;
> - }
> -
> schedlock_enabled = (scheduler_mode == schedlock_on
> || (scheduler_mode == schedlock_step
> && step));
>
> - /* Don't switch over to WAIT_PTID if scheduler locking is on. */
> - if (schedlock_enabled)
> - return 0;
> -
> - /* Don't switch over if we're about to resume some other process
> - other than WAIT_PTID's, and schedule-multiple is off. */
> - if (!sched_multi
> - && ptid_get_pid (wait_ptid) != ptid_get_pid (inferior_ptid))
> - return 0;
> + current = inferior_thread ();
>
> - /* Switched over from WAIT_PID. */
> - if (!ptid_equal (wait_ptid, minus_one_ptid)
> - && !ptid_equal (inferior_ptid, wait_ptid))
> + /* If scheduler locking applies, we can avoid iterating over all
> + threads. */
> + if (schedlock_enabled)
> {
> - struct regcache *regcache = get_thread_regcache (wait_ptid);
> + if (except != current
> + && thread_still_needs_step_over (current))
> + return current;
>
> - if (breakpoint_here_p (get_regcache_aspace (regcache),
> - regcache_read_pc (regcache)))
> - {
> - /* Switch back to WAIT_PID thread. */
> - switch_to_thread (wait_ptid);
> + return NULL;
> + }
>
> - if (debug_infrun)
> - fprintf_unfiltered (gdb_stdlog,
> - "infrun: prepare_to_proceed (step=%d), "
> - "switched to [%s]\n",
> - step, target_pid_to_str (inferior_ptid));
> + ALL_THREADS (tp)
> + {
> + /* Ignore the EXCEPT thread. */
> + if (tp == except)
> + continue;
> + /* Ignore threads of processes we're not resuming. */
> + if (!sched_multi
> + && ptid_get_pid (tp->ptid) != ptid_get_pid (inferior_ptid))
> + continue;
>
> - /* We return 1 to indicate that there is a breakpoint here,
> - so we need to step over it before continuing to avoid
> - hitting it straight away. */
> - return 1;
> - }
> + if (thread_still_needs_step_over (tp))
> + return tp;
> }
>
> - return 0;
> + return NULL;
> }
>
> /* Basic routine for continuing the program in various fashions.
> @@ -2144,8 +2141,6 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal, int step)
> struct thread_info *tp;
> CORE_ADDR pc;
> struct address_space *aspace;
> - /* GDB may force the inferior to step due to various reasons. */
> - int force_step = 0;
>
> /* If we're stopped at a fork/vfork, follow the branch set by the
> "set follow-fork-mode" command; otherwise, we'll just proceed
> @@ -2173,6 +2168,9 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal, int step)
> if (step < 0)
> stop_after_trap = 1;
>
> + /* Fill in with reasonable starting values. */
> + init_thread_stepping_state (tp);
> +
> if (addr == (CORE_ADDR) -1)
> {
> if (pc == stop_pc && breakpoint_here_p (aspace, pc)
> @@ -2185,14 +2183,13 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal, int step)
> Note, we don't do this in reverse, because we won't
> actually be executing the breakpoint insn anyway.
> We'll be (un-)executing the previous instruction. */
> -
> - force_step = 1;
> + tp->stepping_over_breakpoint = 1;
> else if (gdbarch_single_step_through_delay_p (gdbarch)
> && gdbarch_single_step_through_delay (gdbarch,
> get_current_frame ()))
> /* We stepped onto an instruction that needs to be stepped
> again before re-inserting the breakpoint, do so. */
> - force_step = 1;
> + tp->stepping_over_breakpoint = 1;
> }
> else
> {
> @@ -2211,6 +2208,8 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal, int step)
> ;
> else
> {
> + struct thread_info *step_over;
> +
> /* In a multi-threaded task we may select another thread and
> then continue or step.
>
> @@ -2219,31 +2218,29 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal, int step)
> execution (i.e. it will report a breakpoint hit incorrectly).
> So we must step over it first.
>
> - prepare_to_proceed checks the current thread against the
> - thread that reported the most recent event. If a step-over
> - is required it returns TRUE and sets the current thread to
> - the old thread. */
> -
> - /* Store the prev_pc for the stepping thread too, needed by
> - switch_back_to_stepping thread. */
> - tp->prev_pc = regcache_read_pc (get_current_regcache ());
> -
> - if (prepare_to_proceed (step))
> + Look for a thread other than the current (TP) that reported a
> + breakpoint hit and hasn't been resumed yet since. */
> + step_over = find_thread_needs_step_over (step, tp);
> + if (step_over != NULL)
> {
> - force_step = 1;
> - /* The current thread changed. */
> - tp = inferior_thread ();
> + if (debug_infrun)
> + fprintf_unfiltered (gdb_stdlog,
> + "infrun: need to step-over [%s] first\n",
> + target_pid_to_str (step_over->ptid));
> +
> + /* Store the prev_pc for the stepping thread too, needed by
> + switch_back_to_stepping thread. */
> + tp->prev_pc = regcache_read_pc (get_current_regcache ());
> + switch_to_thread (step_over->ptid);
> + tp = step_over;
> }
> }
>
> - if (force_step)
> - tp->control.trap_expected = 1;
> -
> /* If we need to step over a breakpoint, and we're not using
> displaced stepping to do so, insert all breakpoints (watchpoints,
> etc.) but the one we're stepping over, step one instruction, and
> then re-insert the breakpoint when that step is finished. */
> - if (tp->control.trap_expected && !use_displaced_stepping (gdbarch))
> + if (tp->stepping_over_breakpoint && !use_displaced_stepping (gdbarch))
> {
> struct regcache *regcache = get_current_regcache ();
>
> @@ -2258,6 +2255,8 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal, int step)
>
> insert_breakpoints ();
>
> + tp->control.trap_expected = tp->stepping_over_breakpoint;
> +
> if (!non_stop)
> {
> /* Pass the last stop signal to the thread we're resuming,
> @@ -2321,14 +2320,11 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal, int step)
> correctly when the inferior is stopped. */
> tp->prev_pc = regcache_read_pc (get_current_regcache ());
>
> - /* Fill in with reasonable starting values. */
> - init_thread_stepping_state (tp);
> -
> /* Reset to normal state. */
> init_infwait_state ();
>
> /* Resume inferior. */
> - resume (force_step || step || bpstat_should_step (),
> + resume (tp->control.trap_expected || step || bpstat_should_step (),
> tp->suspend.stop_signal);
>
> /* Wait for it to stop (if not standalone)
> @@ -4468,8 +4464,10 @@ process_event_stop_test (struct execution_control_state *ecs)
> fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_STOP_NOISY\n");
> stop_print_frame = 1;
>
> - /* We are about to nuke the step_resume_breakpointt via the
> - cleanup chain, so no need to worry about it here. */
> + /* Assume the thread stopped for a breapoint. We'll still check
> + whether a/the breakpoint is there when the thread is next
> + resumed. */
> + ecs->event_thread->stepping_over_breakpoint = 1;
>
> stop_stepping (ecs);
> return;
> @@ -4479,9 +4477,10 @@ process_event_stop_test (struct execution_control_state *ecs)
> fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_STOP_SILENT\n");
> stop_print_frame = 0;
>
> - /* We are about to nuke the step_resume_breakpoin via the
> - cleanup chain, so no need to worry about it here. */
> -
> + /* Assume the thread stopped for a breapoint. We'll still check
> + whether a/the breakpoint is there when the thread is next
> + resumed. */
> + ecs->event_thread->stepping_over_breakpoint = 1;
> stop_stepping (ecs);
> return;
>
> @@ -5106,25 +5105,68 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
> if (!non_stop)
> {
> struct thread_info *tp;
> -
> - tp = iterate_over_threads (currently_stepping_or_nexting_callback,
> - ecs->event_thread);
> - if (tp)
> + struct thread_info *stepping_thread;
> +
> + /* If any thread is blocked on some internal breakpoint, and we
> + simply need to step over that breakpoint to get it going
> + again, do that first. */
> +
> + /* However, if we see an event for the stepping thread, then we
> + know all other threads have been moved past their breakpoints
> + already. Let the caller check whether the step is finished,
> + etc., before deciding to move it past a breakpoint. */
> + if (ecs->event_thread->control.step_range_end)
> + return 0;
> +
> + /* Check if the current thread is blocking on an incomplete
> + step-over, interrupted by a random signal. */
> + if (ecs->event_thread->control.trap_expected
> + && ecs->event_thread->suspend.stop_signal != GDB_SIGNAL_TRAP)
> {
> - struct frame_info *frame;
> - struct gdbarch *gdbarch;
> + if (debug_infrun)
> + {
> + fprintf_unfiltered (gdb_stdlog,
> + "infrun: need to finish step-over of [%s]\n",
> + target_pid_to_str (ecs->event_thread->ptid));
> + }
> + keep_going (ecs);
> + return 1;
> + }
>
> - /* However, if the current thread is blocked on some internal
> - breakpoint, and we simply need to step over that breakpoint
> - to get it going again, do that first. */
> - if ((ecs->event_thread->control.trap_expected
> - && ecs->event_thread->suspend.stop_signal != GDB_SIGNAL_TRAP)
> - || ecs->event_thread->stepping_over_breakpoint)
> + stepping_thread
> + = iterate_over_threads (currently_stepping_or_nexting_callback,
> + ecs->event_thread);
> +
> + /* Check if any other thread except the stepping thread that
> + needs to start a step-over. Do that before actually
> + proceeding with step/next/etc. */
> + tp = find_thread_needs_step_over (stepping_thread != NULL,
> + stepping_thread);
> + if (tp != NULL)
> + {
> + if (debug_infrun)
> {
> - keep_going (ecs);
> - return 1;
> + fprintf_unfiltered (gdb_stdlog,
> + "infrun: need to step-over [%s]\n",
> + target_pid_to_str (tp->ptid));
> }
>
> + gdb_assert (!tp->control.trap_expected);
> + gdb_assert (!tp->control.step_range_end);
> +
> + ecs->ptid = tp->ptid;
> + ecs->event_thread = tp;
> + switch_to_thread (ecs->ptid);
> + keep_going (ecs);
> + return 1;
> + }
> +
> + tp = stepping_thread;
> + if (tp != NULL)
> + {
> + struct frame_info *frame;
> + struct gdbarch *gdbarch;
> +
> /* If the stepping thread exited, then don't try to switch
> back and resume it, which could fail in several different
> ways depending on the target. Instead, just keep going.
> @@ -5687,7 +5729,7 @@ keep_going (struct execution_control_state *ecs)
> (watchpoints, etc.) but the one we're stepping over, step one
> instruction, and then re-insert the breakpoint when that step
> is finished. */
> - if (ecs->event_thread->stepping_over_breakpoint
> + if (thread_still_needs_step_over (ecs->event_thread)
> && !use_displaced_stepping (get_regcache_arch (regcache)))
> {
> /* Can't step over more than one breakpoint simultaneously
> diff --git a/gdb/testsuite/gdb.threads/multiple-step-overs.c b/gdb/testsuite/gdb.threads/multiple-step-overs.c
> new file mode 100644
> index 0000000..523a88a
> --- /dev/null
> +++ b/gdb/testsuite/gdb.threads/multiple-step-overs.c
> @@ -0,0 +1,105 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> + Copyright 2009-2014 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 <unistd.h>
> +#include <stdlib.h>
> +#include <signal.h>
> +
> +unsigned int args[2];
> +
> +pthread_barrier_t barrier;
> +pthread_t child_thread_2, child_thread_3;
> +
> +void
> +callme (void)
> +{
> +}
> +
> +void *
> +child_function_3 (void *arg)
> +{
> + int my_number = (long) arg;
> + volatile int *myp = (int *) &args[my_number];
> +
> + pthread_barrier_wait (&barrier);
> +
> + while (*myp > 0)
> + {
> + (*myp) ++;
> + callme (); /* set breakpoint thread 3 here */
> + }
> +
> + pthread_exit (NULL);
> +}
> +
> +void *
> +child_function_2 (void *arg)
> +{
> + int my_number = (long) arg;
> + volatile int *myp = (int *) &args[my_number];
> +
> + pthread_barrier_wait (&barrier);
> +
> + while (*myp > 0)
> + {
> + (*myp) ++;
> + callme (); /* set breakpoint thread 2 here */
> + }
> +
> + pthread_exit (NULL);
> +}
> +
> +static int
> +wait_threads (void)
> +{
> + return 1; /* in wait_threads */
> +}
> +
> +int
> +main ()
> +{
> + int res;
> + long i;
> +
> + /* Call these early so that PLTs for these are resolved soon,
> + instead of in the threads. RTLD_NOW should work as well. */
> + usleep (0);
> + pthread_barrier_init (&barrier, NULL, 1);
> + pthread_barrier_wait (&barrier);
> +
> + pthread_barrier_init (&barrier, NULL, 2);
> +
> + i = 0;
> + args[i] = 1;
> + res = pthread_create (&child_thread_2,
> + NULL, child_function_2, (void *) i);
> + pthread_barrier_wait (&barrier);
> + callme ();
> +
> + i = 1;
> + args[i] = 1;
> + res = pthread_create (&child_thread_3,
> + NULL, child_function_3, (void *) i);
> + pthread_barrier_wait (&barrier);
> + wait_threads (); /* set wait-threads breakpoint here */
> +
> + pthread_join (child_thread_2, NULL);
> + pthread_join (child_thread_3, NULL);
> +
> + exit(EXIT_SUCCESS);
> +}
> diff --git a/gdb/testsuite/gdb.threads/multiple-step-overs.exp b/gdb/testsuite/gdb.threads/multiple-step-overs.exp
> new file mode 100644
> index 0000000..4426586
> --- /dev/null
> +++ b/gdb/testsuite/gdb.threads/multiple-step-overs.exp
> @@ -0,0 +1,80 @@
> +# Copyright (C) 2011-2014 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 that GDB steps over all breakpoints of threads not the stepping
> +# thread, before actually proceeding with the stepped thread.
> +
> +standard_testfile
> +set executable ${testfile}
> +
> +if [target_info exists gdb,nosignals] {
> + verbose "Skipping ${testfile}.exp because of nosignals."
> + return -1
> +}
> +
> +if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
> + executable [list debug "incdir=${objdir}"]] != "" } {
> + return -1
> +}
> +
> +# Prepare environment for test. PREFIX is used as prefix in test
> +# messages.
> +
> +proc setup { prefix } {
> + global executable
> +
> + with_test_prefix $prefix {
> + clean_restart $executable
> +
> + if ![runto_main] {
> + return -1
> + }
> +
> + gdb_breakpoint [gdb_get_line_number "set wait-threads breakpoint here"]
> + gdb_continue_to_breakpoint "run to breakpoint"
> + gdb_test "info threads" "3 .* 2 .*\\\* 1.*" "info threads shows all threads"
> +
> + gdb_test_no_output "set scheduler-locking on"
> +
> + gdb_breakpoint [gdb_get_line_number "set breakpoint thread 3 here"]
> + gdb_breakpoint [gdb_get_line_number "set breakpoint thread 2 here"]
> +
> + gdb_test "thread 3" "Switching.*"
> + gdb_continue_to_breakpoint "run to breakpoint in thread 3"
> + gdb_test "p *myp = 0" " = 0" "unbreak loop in thread 3"
> +
> + gdb_test "thread 2" "Switching.*"
> + gdb_continue_to_breakpoint "run to breakpoint in thread 2"
> + gdb_test "p *myp = 0" " = 0" "unbreak loop in thread 2"
> +
> + # Switch back to thread 1 and disable scheduler locking.
> + gdb_test "thread 1" "Switching.*"
> + gdb_test "set scheduler-locking off"
> +
> + # Now all 3 threads are stopped for a breakpoint that needs to
> + # be stepped over before thread 1 is resumed.
> + }
> +}
> +
> +setup "step"
> +gdb_test "step" "in wait_threads .*"
> +
> +setup "next"
> +gdb_test "set debug infrun 1" ".*"
> +gdb_test "next" "pthread_join.*"
> +
> +setup "continue"
> +gdb_breakpoint [gdb_get_line_number "EXIT_SUCCESS"]
> +gdb_test "continue" "EXIT_SUCCESS.*"
> diff --git a/gdb/testsuite/gdb.threads/signal-while-stepping-over-bp-other-thread.exp b/gdb/testsuite/gdb.threads/signal-while-stepping-over-bp-other-thread.exp
> index bf5ea60..c46dad2 100644
> --- a/gdb/testsuite/gdb.threads/signal-while-stepping-over-bp-other-thread.exp
> +++ b/gdb/testsuite/gdb.threads/signal-while-stepping-over-bp-other-thread.exp
> @@ -107,7 +107,7 @@ gdb_test "set debug infrun 1"
>
> gdb_test \
> "step" \
> - ".*prepare_to_proceed \\(step=1\\), switched to.*signal arrived while stepping over breakpoint.*switching back to stepped thread.*stepped to a different line.*callme.*" \
> + ".*need to step-over.*resume \\(step=1.*signal arrived while stepping over breakpoint.*switching back to stepped thread.*stepped to a different line.*callme.*" \
> "step"
>
> set cnt_after [get_value "args\[$my_number\]" "get count after step"]
> --
> 1.7.11.7
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 5/6] Handle multiple step-overs
2014-03-26 16:54 [PATCH 5/6] Handle multiple step-overs Alan Lawrence
@ 2014-04-04 11:54 ` Pedro Alves
2014-04-22 18:42 ` Pedro Alves
1 sibling, 0 replies; 6+ messages in thread
From: Pedro Alves @ 2014-04-04 11:54 UTC (permalink / raw)
To: Alan Lawrence; +Cc: gdb-patches
Sorry for the delay -- just a note to say "ack". I haven't managed
to find time to look at this yet. I had found another regression this
series caused (hbreak2.exp against gdbserver), and been trying to
fix that one first.
--
Pedro Alves
On 03/26/2014 04:54 PM, Alan Lawrence wrote:
> Following this patch, we're seeing an assertion failure of infrun.c:5192,
>
> gdb_assert (!tp->control.trap_expected);
>
> on the AArch64 platform. The testcase is that added in git commit
> beb460e8d2ddf5327a6ab146055a6e6e9f552a4b, condbreak-call-false.{c,exp} - I've
> tried this testcase both before and after your multiple step-over patch, and it
> succeeds without the patch. I'm not very familiar with gdb internals and
> stepwise comparing AArch64 against ARM (on which the test passes) sounds at best
> laborious; hoping there may be some experts here who can help?
>
> I have reproduced the failure on (a) qemu (-2.0.0-rc0) running on x86_64 host,
> (b) native AArch64 hardware, (c) running cross gdb hosted on x86_64-linux
> against remote target running natively on aarch64-linux; in the latter case,
> it's the host gdb which is critical (i.e.: fails/succeeds with/out the multiple
> stepover patch, regardless of whether the remote gdbserver includes this patch
> or not); remote gdbserver succeeds in all 4=(remote/local)(with/without) cases.
>
> firstly:
> $ aarch64-none-linux-gnu-gcc -g -o condbreak-call-false
> binutils-gdb/gdb/testsuite/gdb.base/condbreak-call-false.c
> $ aarch64-linux-user/qemu-aarch64 -g 23456 -L $MY_QEMU_LIB_PATH
> condbreak-call-false &
>
> then:
> $ aarch64-none-linux-gnu-gdb
> GNU gdb (unknown) 7.7.50.20140326-cvs
> Copyright (C) 2014 Free Software Foundation, Inc.
> License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
> This is free software: you are free to change and redistribute it.
> There is NO WARRANTY, to the extent permitted by law. Type "show copying"
> and "show warranty" for details.
> This GDB was configured as "--host=x86_64-unknown-linux-gnu
> --target=aarch64-none-linux-gnu".
> Type "show configuration" for configuration details.
> For bug reporting instructions, please see:
> <http://www.gnu.org/software/gdb/bugs/>.
> Find the GDB manual and other documentation resources online at:
> <http://www.gnu.org/software/gdb/documentation/>.
> For help, type "help".
> Type "apropos word" to search for commands related to "word".
> (gdb) file condbreak-call-false
> Reading symbols from condbreak-call-false...done.
> (gdb) target remote localhost:23456
> Remote debugging using dsgci-1.euhpc.arm.com:23456
> warning: Unable to find dynamic linker breakpoint function.
> GDB will be unable to debug shared library initializers
> and track explicitly loaded dynamic code.
> 0x0000004000a01d00 in ?? ()
> (gdb) break main
> Breakpoint 1 at 0x400504: file
> srcfsf/binutils-gdb/gdb/testsuite/gdb.base/condbreak-call-false.c, line 37.
> (gdb) continue
> Continuing.
> warning: `/lib64/libc.so.6': Shared library architecture unknown is not
> compatible with target architecture aarch64.
> warning: Could not load shared library symbols for /lib/ld-linux-aarch64.so.1.
> Do you need "set solib-search-path" or "set sysroot"?
>
> Breakpoint 1, main ()
> at srcfsf/binutils-gdb/gdb/testsuite/gdb.base/condbreak-call-false.c:37
> 37 foo ();
> (gdb) break foo if zero()
> Breakpoint 2 at 0x4004f0: file
> srcfsf/binutils-gdb/gdb/testsuite/gdb.base/condbreak-call-false.c, line 25.
> (gdb) continue
> Continuing.
> /work/alalaw01/oban/srca64t/binutils-gdb/gdb/infrun.c:5192: internal-error:
> switch_back_to_stepped_thread: Assertion `!tp->control.trap_expected' failed.
> A problem internal to GDB has been detected,
> further debugging may prove unreliable.
> Quit this debugging session? (y or n)
>
> or:
> $ aarch64-none-elf-gdb
> GNU gdb (unknown) 7.7.50.20140320-cvs
> Copyright (C) 2014 Free Software Foundation, Inc.
> License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
> This is free software: you are free to change and redistribute it.
> There is NO WARRANTY, to the extent permitted by law. Type "show copying"
> and "show warranty" for details.
> This GDB was configured as "--host=x86_64-unknown-linux-gnu
> --target=aarch64-none-elf".
> Type "show configuration" for configuration details.
> For bug reporting instructions, please see:
> <http://www.gnu.org/software/gdb/bugs/>.
> Find the GDB manual and other documentation resources online at:
> <http://www.gnu.org/software/gdb/documentation/>.
> For help, type "help".
> Type "apropos word" to search for commands related to "word".
> (gdb) file condbreak-call-false
> Reading symbols from condbreak-call-false...done.
> (gdb) target remote localhost:23456
> Remote debugging using localhost:23456
> _start () at /work/alalaw01/oban/srcfsf/binutils-gdb/libgloss/aarch64/crt0.S:90
> 90 adr x1, .LC0
> (gdb) break main
> Breakpoint 1 at 0x4002d4: file
> srcfsf/binutils-gdb/gdb/testsuite/gdb.base/condbreak-call-false.c, line 37.
> (gdb) continue # can do this in either order with next
> Continuing.
>
> Breakpoint 1, main ()
> at srcfsf/binutils-gdb/gdb/testsuite/gdb.base/condbreak-call-false.c:37
> 37 foo ();
> (gdb) break foo if zero() # can do this in either order with previous
> Breakpoint 2 at 0x4002c0: file
> srcfsf/binutils-gdb/gdb/testsuite/gdb.base/condbreak-call-false.c, line 25.
> (gdb) continue
> Continuing.
> /work/alalaw01/oban/srca64t/binutils-gdb/gdb/infrun.c:5192: internal-error:
> switch_back_to_stepped_thread: Assertion `!tp->control.trap_expected' failed.
> A problem internal to GDB has been detected,
> further debugging may prove unreliable.
> Quit this debugging session? (y or n)
>
>
>
>>
>> From: Pedro Alves <palves at redhat dot com>
>> To: gdb-patches at sourceware dot org
>> Date: Tue, 25 Feb 2014 20:32:42 +0000
>> Subject: [PATCH 5/6] Handle multiple step-overs.
>> Authentication-results: sourceware.org; auth=none
>> References: <1393360363-5603-1-git-send-email-palves at redhat dot com>
>>
>> This test fails with current mainline.
>>
>> If the program stopped for a breakpoint in thread 1, and then the user
>> switches to thread 2, and resumes the program, GDB first switches back
>> to thread 1 to step it over the breakpoint, in order to make progress.
>>
>> However, that logic only considers the last reported event, assuming
>> only one thread needs that stepping over dance.
>>
>> That's actually not true when we play with scheduler-locking. The
>> patch adds an example to the testsuite of multiple threads needing a
>> step-over before the stepping thread can be resumed. With current
>> mainline, the program re-traps the same breakpoint it had already
>> trapped before.
>>
>> E.g.:
>>
>> Breakpoint 2, main () at ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c:99
>> 99 wait_threads (); /* set wait-threads breakpoint here */
>> (gdb) PASS: gdb.threads/multiple-step-overs.exp: step: continue to breakpoint: run to breakpoint
>> info threads
>> Id Target Id Frame
>> 3 Thread 0x7ffff77c9700 (LWP 4310) "multiple-step-o" 0x00000000004007ca in child_function_3 (arg=0x1) at ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c:43
>> 2 Thread 0x7ffff7fca700 (LWP 4309) "multiple-step-o" 0x0000000000400827 in child_function_2 (arg=0x0) at ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c:60
>> * 1 Thread 0x7ffff7fcb740 (LWP 4305) "multiple-step-o" main () at ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c:99
>> (gdb) PASS: gdb.threads/multiple-step-overs.exp: step: info threads shows all threads
>> set scheduler-locking on
>> (gdb) PASS: gdb.threads/multiple-step-overs.exp: step: set scheduler-locking on
>> break 44
>> Breakpoint 3 at 0x4007d3: file ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c, line 44.
>> (gdb) break 61
>> Breakpoint 4 at 0x40082d: file ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c, line 61.
>> (gdb) thread 3
>> [Switching to thread 3 (Thread 0x7ffff77c9700 (LWP 4310))]
>> #0 0x00000000004007ca in child_function_3 (arg=0x1) at ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c:43
>> 43 (*myp) ++;
>> (gdb) PASS: gdb.threads/multiple-step-overs.exp: step: thread 3
>> continue
>> Continuing.
>>
>> Breakpoint 3, child_function_3 (arg=0x1) at ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c:44
>> 44 callme (); /* set breakpoint thread 3 here */
>> (gdb) PASS: gdb.threads/multiple-step-overs.exp: step: continue to breakpoint: run to breakpoint in thread 3
>> p *myp = 0
>> $1 = 0
>> (gdb) PASS: gdb.threads/multiple-step-overs.exp: step: unbreak loop in thread 3
>> thread 2
>> [Switching to thread 2 (Thread 0x7ffff7fca700 (LWP 4309))]
>> #0 0x0000000000400827 in child_function_2 (arg=0x0) at ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c:60
>> 60 (*myp) ++;
>> (gdb) PASS: gdb.threads/multiple-step-overs.exp: step: thread 2
>> continue
>> Continuing.
>>
>> Breakpoint 4, child_function_2 (arg=0x0) at ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c:61
>> 61 callme (); /* set breakpoint thread 2 here */
>> (gdb) PASS: gdb.threads/multiple-step-overs.exp: step: continue to breakpoint: run to breakpoint in thread 2
>> p *myp = 0
>> $2 = 0
>> (gdb) PASS: gdb.threads/multiple-step-overs.exp: step: unbreak loop in thread 2
>> thread 1
>> [Switching to thread 1 (Thread 0x7ffff7fcb740 (LWP 4305))]
>> #0 main () at ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c:99
>> 99 wait_threads (); /* set wait-threads breakpoint here */
>> (gdb) PASS: gdb.threads/multiple-step-overs.exp: step: thread 1
>> set scheduler-locking off
>> (gdb) PASS: gdb.threads/multiple-step-overs.exp: step: set scheduler-locking off
>>
>> At this point all thread are stopped for a breakpoint that needs stepping over.
>>
>> (gdb) step
>>
>> Breakpoint 2, main () at ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c:99
>> 99 wait_threads (); /* set wait-threads breakpoint here */
>> (gdb) FAIL: gdb.threads/multiple-step-overs.exp: step
>>
>> But that "step" retriggers the same breakpoint instead of making
>> progress.
>>
>> The patch teaches GDB to step over all breakpoints of all threads
>> before resuming the stepping thread.
>>
>> Tested on x86_64 Fedora 17, against pristine mainline, and also my
>> branch that implements software single-stepping on x86.
>>
>> gdb/
>> 2014-02-25 Pedro Alves <palves@redhat.com>
>>
>> * infrun.c (prepare_to_proceed): Delete.
>> (thread_still_needs_step_over): New function.
>> (find_thread_needs_step_over): New function.
>> (proceed): If the current thread needs a step-over, set its
>> steping_over_breakpoint flag. Adjust to use
>> find_thread_needs_step_over instead of prepare_to_proceed.
>> (process_event_stop_test): For BPSTAT_WHAT_STOP_NOISY and
>> BPSTAT_WHAT_STOP_SILENT, assume the thread stopped for a
>> breakpoint.
>> (switch_back_to_stepped_thread): Step over breakpoints of all
>> threads not the stepping thread, before switching back to the
>> stepping thread.
>>
>> gdb/testsuite/
>> 2014-02-25 Pedro Alves <palves@redhat.com>
>>
>> * gdb.threads/multiple-step-overs.c: New file.
>> * gdb.threads/multiple-step-overs.exp: New file.
>> * gdb.threads/signal-while-stepping-over-bp-other-thread.exp:
>> Adjust expected infrun debug output.
>> ---
>> gdb/infrun.c | 238 ++++++++++++---------
>> gdb/testsuite/gdb.threads/multiple-step-overs.c | 105 +++++++++
>> gdb/testsuite/gdb.threads/multiple-step-overs.exp | 80 +++++++
>> .../signal-while-stepping-over-bp-other-thread.exp | 2 +-
>> 4 files changed, 326 insertions(+), 99 deletions(-)
>> create mode 100644 gdb/testsuite/gdb.threads/multiple-step-overs.c
>> create mode 100644 gdb/testsuite/gdb.threads/multiple-step-overs.exp
>>
>> diff --git a/gdb/infrun.c b/gdb/infrun.c
>> index 1ea8d04..bde40c5 100644
>> --- a/gdb/infrun.c
>> +++ b/gdb/infrun.c
>> @@ -90,8 +90,6 @@ static int currently_stepping_or_nexting_callback (struct thread_info *tp,
>>
>> static void xdb_handle_command (char *args, int from_tty);
>>
>> -static int prepare_to_proceed (int);
>> -
>> static void print_exited_reason (int exitstatus);
>>
>> static void print_signal_exited_reason (enum gdb_signal siggnal);
>> @@ -2053,75 +2051,74 @@ clear_proceed_status (void)
>> }
>> }
>>
>> -/* Check the current thread against the thread that reported the most recent
>> - event. If a step-over is required return TRUE and set the current thread
>> - to the old thread. Otherwise return FALSE.
>> -
>> - This should be suitable for any targets that support threads. */
>> +/* Returns true if TP is still stopped at a breakpoint that needs
>> + stepping-over in order to make progress. If the breakpoint is gone
>> + meanwhile, we can skip the whole step-over dance. */
>>
>> static int
>> -prepare_to_proceed (int step)
>> +thread_still_needs_step_over (struct thread_info *tp)
>> +{
>> + if (tp->stepping_over_breakpoint)
>> + {
>> + struct regcache *regcache = get_thread_regcache (tp->ptid);
>> +
>> + if (breakpoint_here_p (get_regcache_aspace (regcache),
>> + regcache_read_pc (regcache)))
>> + return 1;
>> +
>> + tp->stepping_over_breakpoint = 0;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +/* Look a thread other than EXCEPT that has previously reported a
>> + breakpoint event, and thus needs a step-over in order to make
>> + progress. Returns NULL is none is found. STEP indicates whether
>> + we're about to step the current thread, in order to decide whether
>> + "set scheduler-locking step" applies. */
>> +
>> +static struct thread_info *
>> +find_thread_needs_step_over (int step, struct thread_info *except)
>> {
>> - ptid_t wait_ptid;
>> - struct target_waitstatus wait_status;
>> int schedlock_enabled;
>> + struct thread_info *tp, *current;
>>
>> /* With non-stop mode on, threads are always handled individually. */
>> gdb_assert (! non_stop);
>>
>> - /* Get the last target status returned by target_wait(). */
>> - get_last_target_status (&wait_ptid, &wait_status);
>> -
>> - /* Make sure we were stopped at a breakpoint. */
>> - if (wait_status.kind != TARGET_WAITKIND_STOPPED
>> - || (wait_status.value.sig != GDB_SIGNAL_TRAP
>> - && wait_status.value.sig != GDB_SIGNAL_ILL
>> - && wait_status.value.sig != GDB_SIGNAL_SEGV
>> - && wait_status.value.sig != GDB_SIGNAL_EMT))
>> - {
>> - return 0;
>> - }
>> -
>> schedlock_enabled = (scheduler_mode == schedlock_on
>> || (scheduler_mode == schedlock_step
>> && step));
>>
>> - /* Don't switch over to WAIT_PTID if scheduler locking is on. */
>> - if (schedlock_enabled)
>> - return 0;
>> -
>> - /* Don't switch over if we're about to resume some other process
>> - other than WAIT_PTID's, and schedule-multiple is off. */
>> - if (!sched_multi
>> - && ptid_get_pid (wait_ptid) != ptid_get_pid (inferior_ptid))
>> - return 0;
>> + current = inferior_thread ();
>>
>> - /* Switched over from WAIT_PID. */
>> - if (!ptid_equal (wait_ptid, minus_one_ptid)
>> - && !ptid_equal (inferior_ptid, wait_ptid))
>> + /* If scheduler locking applies, we can avoid iterating over all
>> + threads. */
>> + if (schedlock_enabled)
>> {
>> - struct regcache *regcache = get_thread_regcache (wait_ptid);
>> + if (except != current
>> + && thread_still_needs_step_over (current))
>> + return current;
>>
>> - if (breakpoint_here_p (get_regcache_aspace (regcache),
>> - regcache_read_pc (regcache)))
>> - {
>> - /* Switch back to WAIT_PID thread. */
>> - switch_to_thread (wait_ptid);
>> + return NULL;
>> + }
>>
>> - if (debug_infrun)
>> - fprintf_unfiltered (gdb_stdlog,
>> - "infrun: prepare_to_proceed (step=%d), "
>> - "switched to [%s]\n",
>> - step, target_pid_to_str (inferior_ptid));
>> + ALL_THREADS (tp)
>> + {
>> + /* Ignore the EXCEPT thread. */
>> + if (tp == except)
>> + continue;
>> + /* Ignore threads of processes we're not resuming. */
>> + if (!sched_multi
>> + && ptid_get_pid (tp->ptid) != ptid_get_pid (inferior_ptid))
>> + continue;
>>
>> - /* We return 1 to indicate that there is a breakpoint here,
>> - so we need to step over it before continuing to avoid
>> - hitting it straight away. */
>> - return 1;
>> - }
>> + if (thread_still_needs_step_over (tp))
>> + return tp;
>> }
>>
>> - return 0;
>> + return NULL;
>> }
>>
>> /* Basic routine for continuing the program in various fashions.
>> @@ -2144,8 +2141,6 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal, int step)
>> struct thread_info *tp;
>> CORE_ADDR pc;
>> struct address_space *aspace;
>> - /* GDB may force the inferior to step due to various reasons. */
>> - int force_step = 0;
>>
>> /* If we're stopped at a fork/vfork, follow the branch set by the
>> "set follow-fork-mode" command; otherwise, we'll just proceed
>> @@ -2173,6 +2168,9 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal, int step)
>> if (step < 0)
>> stop_after_trap = 1;
>>
>> + /* Fill in with reasonable starting values. */
>> + init_thread_stepping_state (tp);
>> +
>> if (addr == (CORE_ADDR) -1)
>> {
>> if (pc == stop_pc && breakpoint_here_p (aspace, pc)
>> @@ -2185,14 +2183,13 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal, int step)
>> Note, we don't do this in reverse, because we won't
>> actually be executing the breakpoint insn anyway.
>> We'll be (un-)executing the previous instruction. */
>> -
>> - force_step = 1;
>> + tp->stepping_over_breakpoint = 1;
>> else if (gdbarch_single_step_through_delay_p (gdbarch)
>> && gdbarch_single_step_through_delay (gdbarch,
>> get_current_frame ()))
>> /* We stepped onto an instruction that needs to be stepped
>> again before re-inserting the breakpoint, do so. */
>> - force_step = 1;
>> + tp->stepping_over_breakpoint = 1;
>> }
>> else
>> {
>> @@ -2211,6 +2208,8 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal, int step)
>> ;
>> else
>> {
>> + struct thread_info *step_over;
>> +
>> /* In a multi-threaded task we may select another thread and
>> then continue or step.
>>
>> @@ -2219,31 +2218,29 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal, int step)
>> execution (i.e. it will report a breakpoint hit incorrectly).
>> So we must step over it first.
>>
>> - prepare_to_proceed checks the current thread against the
>> - thread that reported the most recent event. If a step-over
>> - is required it returns TRUE and sets the current thread to
>> - the old thread. */
>> -
>> - /* Store the prev_pc for the stepping thread too, needed by
>> - switch_back_to_stepping thread. */
>> - tp->prev_pc = regcache_read_pc (get_current_regcache ());
>> -
>> - if (prepare_to_proceed (step))
>> + Look for a thread other than the current (TP) that reported a
>> + breakpoint hit and hasn't been resumed yet since. */
>> + step_over = find_thread_needs_step_over (step, tp);
>> + if (step_over != NULL)
>> {
>> - force_step = 1;
>> - /* The current thread changed. */
>> - tp = inferior_thread ();
>> + if (debug_infrun)
>> + fprintf_unfiltered (gdb_stdlog,
>> + "infrun: need to step-over [%s] first\n",
>> + target_pid_to_str (step_over->ptid));
>> +
>> + /* Store the prev_pc for the stepping thread too, needed by
>> + switch_back_to_stepping thread. */
>> + tp->prev_pc = regcache_read_pc (get_current_regcache ());
>> + switch_to_thread (step_over->ptid);
>> + tp = step_over;
>> }
>> }
>>
>> - if (force_step)
>> - tp->control.trap_expected = 1;
>> -
>> /* If we need to step over a breakpoint, and we're not using
>> displaced stepping to do so, insert all breakpoints (watchpoints,
>> etc.) but the one we're stepping over, step one instruction, and
>> then re-insert the breakpoint when that step is finished. */
>> - if (tp->control.trap_expected && !use_displaced_stepping (gdbarch))
>> + if (tp->stepping_over_breakpoint && !use_displaced_stepping (gdbarch))
>> {
>> struct regcache *regcache = get_current_regcache ();
>>
>> @@ -2258,6 +2255,8 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal, int step)
>>
>> insert_breakpoints ();
>>
>> + tp->control.trap_expected = tp->stepping_over_breakpoint;
>> +
>> if (!non_stop)
>> {
>> /* Pass the last stop signal to the thread we're resuming,
>> @@ -2321,14 +2320,11 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal, int step)
>> correctly when the inferior is stopped. */
>> tp->prev_pc = regcache_read_pc (get_current_regcache ());
>>
>> - /* Fill in with reasonable starting values. */
>> - init_thread_stepping_state (tp);
>> -
>> /* Reset to normal state. */
>> init_infwait_state ();
>>
>> /* Resume inferior. */
>> - resume (force_step || step || bpstat_should_step (),
>> + resume (tp->control.trap_expected || step || bpstat_should_step (),
>> tp->suspend.stop_signal);
>>
>> /* Wait for it to stop (if not standalone)
>> @@ -4468,8 +4464,10 @@ process_event_stop_test (struct execution_control_state *ecs)
>> fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_STOP_NOISY\n");
>> stop_print_frame = 1;
>>
>> - /* We are about to nuke the step_resume_breakpointt via the
>> - cleanup chain, so no need to worry about it here. */
>> + /* Assume the thread stopped for a breapoint. We'll still check
>> + whether a/the breakpoint is there when the thread is next
>> + resumed. */
>> + ecs->event_thread->stepping_over_breakpoint = 1;
>>
>> stop_stepping (ecs);
>> return;
>> @@ -4479,9 +4477,10 @@ process_event_stop_test (struct execution_control_state *ecs)
>> fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_STOP_SILENT\n");
>> stop_print_frame = 0;
>>
>> - /* We are about to nuke the step_resume_breakpoin via the
>> - cleanup chain, so no need to worry about it here. */
>> -
>> + /* Assume the thread stopped for a breapoint. We'll still check
>> + whether a/the breakpoint is there when the thread is next
>> + resumed. */
>> + ecs->event_thread->stepping_over_breakpoint = 1;
>> stop_stepping (ecs);
>> return;
>>
>> @@ -5106,25 +5105,68 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
>> if (!non_stop)
>> {
>> struct thread_info *tp;
>> -
>> - tp = iterate_over_threads (currently_stepping_or_nexting_callback,
>> - ecs->event_thread);
>> - if (tp)
>> + struct thread_info *stepping_thread;
>> +
>> + /* If any thread is blocked on some internal breakpoint, and we
>> + simply need to step over that breakpoint to get it going
>> + again, do that first. */
>> +
>> + /* However, if we see an event for the stepping thread, then we
>> + know all other threads have been moved past their breakpoints
>> + already. Let the caller check whether the step is finished,
>> + etc., before deciding to move it past a breakpoint. */
>> + if (ecs->event_thread->control.step_range_end)
>> + return 0;
>> +
>> + /* Check if the current thread is blocking on an incomplete
>> + step-over, interrupted by a random signal. */
>> + if (ecs->event_thread->control.trap_expected
>> + && ecs->event_thread->suspend.stop_signal != GDB_SIGNAL_TRAP)
>> {
>> - struct frame_info *frame;
>> - struct gdbarch *gdbarch;
>> + if (debug_infrun)
>> + {
>> + fprintf_unfiltered (gdb_stdlog,
>> + "infrun: need to finish step-over of [%s]\n",
>> + target_pid_to_str (ecs->event_thread->ptid));
>> + }
>> + keep_going (ecs);
>> + return 1;
>> + }
>>
>> - /* However, if the current thread is blocked on some internal
>> - breakpoint, and we simply need to step over that breakpoint
>> - to get it going again, do that first. */
>> - if ((ecs->event_thread->control.trap_expected
>> - && ecs->event_thread->suspend.stop_signal != GDB_SIGNAL_TRAP)
>> - || ecs->event_thread->stepping_over_breakpoint)
>> + stepping_thread
>> + = iterate_over_threads (currently_stepping_or_nexting_callback,
>> + ecs->event_thread);
>> +
>> + /* Check if any other thread except the stepping thread that
>> + needs to start a step-over. Do that before actually
>> + proceeding with step/next/etc. */
>> + tp = find_thread_needs_step_over (stepping_thread != NULL,
>> + stepping_thread);
>> + if (tp != NULL)
>> + {
>> + if (debug_infrun)
>> {
>> - keep_going (ecs);
>> - return 1;
>> + fprintf_unfiltered (gdb_stdlog,
>> + "infrun: need to step-over [%s]\n",
>> + target_pid_to_str (tp->ptid));
>> }
>>
>> + gdb_assert (!tp->control.trap_expected);
>> + gdb_assert (!tp->control.step_range_end);
>> +
>> + ecs->ptid = tp->ptid;
>> + ecs->event_thread = tp;
>> + switch_to_thread (ecs->ptid);
>> + keep_going (ecs);
>> + return 1;
>> + }
>> +
>> + tp = stepping_thread;
>> + if (tp != NULL)
>> + {
>> + struct frame_info *frame;
>> + struct gdbarch *gdbarch;
>> +
>> /* If the stepping thread exited, then don't try to switch
>> back and resume it, which could fail in several different
>> ways depending on the target. Instead, just keep going.
>> @@ -5687,7 +5729,7 @@ keep_going (struct execution_control_state *ecs)
>> (watchpoints, etc.) but the one we're stepping over, step one
>> instruction, and then re-insert the breakpoint when that step
>> is finished. */
>> - if (ecs->event_thread->stepping_over_breakpoint
>> + if (thread_still_needs_step_over (ecs->event_thread)
>> && !use_displaced_stepping (get_regcache_arch (regcache)))
>> {
>> /* Can't step over more than one breakpoint simultaneously
>> diff --git a/gdb/testsuite/gdb.threads/multiple-step-overs.c b/gdb/testsuite/gdb.threads/multiple-step-overs.c
>> new file mode 100644
>> index 0000000..523a88a
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.threads/multiple-step-overs.c
>> @@ -0,0 +1,105 @@
>> +/* This testcase is part of GDB, the GNU debugger.
>> +
>> + Copyright 2009-2014 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 <unistd.h>
>> +#include <stdlib.h>
>> +#include <signal.h>
>> +
>> +unsigned int args[2];
>> +
>> +pthread_barrier_t barrier;
>> +pthread_t child_thread_2, child_thread_3;
>> +
>> +void
>> +callme (void)
>> +{
>> +}
>> +
>> +void *
>> +child_function_3 (void *arg)
>> +{
>> + int my_number = (long) arg;
>> + volatile int *myp = (int *) &args[my_number];
>> +
>> + pthread_barrier_wait (&barrier);
>> +
>> + while (*myp > 0)
>> + {
>> + (*myp) ++;
>> + callme (); /* set breakpoint thread 3 here */
>> + }
>> +
>> + pthread_exit (NULL);
>> +}
>> +
>> +void *
>> +child_function_2 (void *arg)
>> +{
>> + int my_number = (long) arg;
>> + volatile int *myp = (int *) &args[my_number];
>> +
>> + pthread_barrier_wait (&barrier);
>> +
>> + while (*myp > 0)
>> + {
>> + (*myp) ++;
>> + callme (); /* set breakpoint thread 2 here */
>> + }
>> +
>> + pthread_exit (NULL);
>> +}
>> +
>> +static int
>> +wait_threads (void)
>> +{
>> + return 1; /* in wait_threads */
>> +}
>> +
>> +int
>> +main ()
>> +{
>> + int res;
>> + long i;
>> +
>> + /* Call these early so that PLTs for these are resolved soon,
>> + instead of in the threads. RTLD_NOW should work as well. */
>> + usleep (0);
>> + pthread_barrier_init (&barrier, NULL, 1);
>> + pthread_barrier_wait (&barrier);
>> +
>> + pthread_barrier_init (&barrier, NULL, 2);
>> +
>> + i = 0;
>> + args[i] = 1;
>> + res = pthread_create (&child_thread_2,
>> + NULL, child_function_2, (void *) i);
>> + pthread_barrier_wait (&barrier);
>> + callme ();
>> +
>> + i = 1;
>> + args[i] = 1;
>> + res = pthread_create (&child_thread_3,
>> + NULL, child_function_3, (void *) i);
>> + pthread_barrier_wait (&barrier);
>> + wait_threads (); /* set wait-threads breakpoint here */
>> +
>> + pthread_join (child_thread_2, NULL);
>> + pthread_join (child_thread_3, NULL);
>> +
>> + exit(EXIT_SUCCESS);
>> +}
>> diff --git a/gdb/testsuite/gdb.threads/multiple-step-overs.exp b/gdb/testsuite/gdb.threads/multiple-step-overs.exp
>> new file mode 100644
>> index 0000000..4426586
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.threads/multiple-step-overs.exp
>> @@ -0,0 +1,80 @@
>> +# Copyright (C) 2011-2014 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 that GDB steps over all breakpoints of threads not the stepping
>> +# thread, before actually proceeding with the stepped thread.
>> +
>> +standard_testfile
>> +set executable ${testfile}
>> +
>> +if [target_info exists gdb,nosignals] {
>> + verbose "Skipping ${testfile}.exp because of nosignals."
>> + return -1
>> +}
>> +
>> +if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
>> + executable [list debug "incdir=${objdir}"]] != "" } {
>> + return -1
>> +}
>> +
>> +# Prepare environment for test. PREFIX is used as prefix in test
>> +# messages.
>> +
>> +proc setup { prefix } {
>> + global executable
>> +
>> + with_test_prefix $prefix {
>> + clean_restart $executable
>> +
>> + if ![runto_main] {
>> + return -1
>> + }
>> +
>> + gdb_breakpoint [gdb_get_line_number "set wait-threads breakpoint here"]
>> + gdb_continue_to_breakpoint "run to breakpoint"
>> + gdb_test "info threads" "3 .* 2 .*\\\* 1.*" "info threads shows all threads"
>> +
>> + gdb_test_no_output "set scheduler-locking on"
>> +
>> + gdb_breakpoint [gdb_get_line_number "set breakpoint thread 3 here"]
>> + gdb_breakpoint [gdb_get_line_number "set breakpoint thread 2 here"]
>> +
>> + gdb_test "thread 3" "Switching.*"
>> + gdb_continue_to_breakpoint "run to breakpoint in thread 3"
>> + gdb_test "p *myp = 0" " = 0" "unbreak loop in thread 3"
>> +
>> + gdb_test "thread 2" "Switching.*"
>> + gdb_continue_to_breakpoint "run to breakpoint in thread 2"
>> + gdb_test "p *myp = 0" " = 0" "unbreak loop in thread 2"
>> +
>> + # Switch back to thread 1 and disable scheduler locking.
>> + gdb_test "thread 1" "Switching.*"
>> + gdb_test "set scheduler-locking off"
>> +
>> + # Now all 3 threads are stopped for a breakpoint that needs to
>> + # be stepped over before thread 1 is resumed.
>> + }
>> +}
>> +
>> +setup "step"
>> +gdb_test "step" "in wait_threads .*"
>> +
>> +setup "next"
>> +gdb_test "set debug infrun 1" ".*"
>> +gdb_test "next" "pthread_join.*"
>> +
>> +setup "continue"
>> +gdb_breakpoint [gdb_get_line_number "EXIT_SUCCESS"]
>> +gdb_test "continue" "EXIT_SUCCESS.*"
>> diff --git a/gdb/testsuite/gdb.threads/signal-while-stepping-over-bp-other-thread.exp b/gdb/testsuite/gdb.threads/signal-while-stepping-over-bp-other-thread.exp
>> index bf5ea60..c46dad2 100644
>> --- a/gdb/testsuite/gdb.threads/signal-while-stepping-over-bp-other-thread.exp
>> +++ b/gdb/testsuite/gdb.threads/signal-while-stepping-over-bp-other-thread.exp
>> @@ -107,7 +107,7 @@ gdb_test "set debug infrun 1"
>>
>> gdb_test \
>> "step" \
>> - ".*prepare_to_proceed \\(step=1\\), switched to.*signal arrived while stepping over breakpoint.*switching back to stepped thread.*stepped to a different line.*callme.*" \
>> + ".*need to step-over.*resume \\(step=1.*signal arrived while stepping over breakpoint.*switching back to stepped thread.*stepped to a different line.*callme.*" \
>> "step"
>>
>> set cnt_after [get_value "args\[$my_number\]" "get count after step"]
>> --
>> 1.7.11.7
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 5/6] Handle multiple step-overs
2014-03-26 16:54 [PATCH 5/6] Handle multiple step-overs Alan Lawrence
2014-04-04 11:54 ` Pedro Alves
@ 2014-04-22 18:42 ` Pedro Alves
2014-04-29 11:02 ` Alan Lawrence
1 sibling, 1 reply; 6+ messages in thread
From: Pedro Alves @ 2014-04-22 18:42 UTC (permalink / raw)
To: Alan Lawrence; +Cc: gdb-patches
Hi Alan,
Thanks. This should now be fixed in mainline:
https://sourceware.org/ml/gdb-patches/2014-04/msg00436.html
Could you give it a try please?
On 03/26/2014 04:54 PM, Alan Lawrence wrote:
> Following this patch, we're seeing an assertion failure of infrun.c:5192,
>
> gdb_assert (!tp->control.trap_expected);
>
> on the AArch64 platform. The testcase is that added in git commit
> beb460e8d2ddf5327a6ab146055a6e6e9f552a4b, condbreak-call-false.{c,exp} - I've
> tried this testcase both before and after your multiple step-over patch, and it
> succeeds without the patch. I'm not very familiar with gdb internals and
> stepwise comparing AArch64 against ARM (on which the test passes) sounds at best
> laborious; hoping there may be some experts here who can help?
Against user mode qemu-aarch64:
Breakpoint 1, main () at testsuite/gdb.base/condbreak-call-false.c:37
37 foo ();
Breakpoint 2 at 0x400518: file testsuite/gdb.base/condbreak-call-false.c, line 25.
(gdb) c
Continuing.
infrun: clear_proceed_status_thread (Remote target)
infrun: proceed (addr=0xffffffffffffffff, signal=GDB_SIGNAL_DEFAULT, step=0)
infrun: resume (step=1, signal=GDB_SIGNAL_0), trap_expected=1, current thread [Remote target] at 0x40052c
infrun: wait_for_inferior ()
infrun: target_wait (-1, status) =
infrun: 42000 [Remote target],
infrun: status->kind = stopped, signal = GDB_SIGNAL_TRAP
infrun: infwait_normal_state
infrun: TARGET_WAITKIND_STOPPED
infrun: stop_pc = 0x400518
infrun: clear_proceed_status_thread (Remote target)
infrun: proceed (addr=0x400510, signal=GDB_SIGNAL_0, step=0)
infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [Remote target] at 0x400510
infrun: wait_for_inferior ()
infrun: target_wait (-1, status) =
infrun: 42000 [Remote target],
infrun: status->kind = stopped, signal = GDB_SIGNAL_TRAP
infrun: infwait_normal_state
infrun: TARGET_WAITKIND_STOPPED
infrun: stop_pc = 0x4003b0
infrun: BPSTAT_WHAT_STOP_SILENT
infrun: stop_stepping
infrun: BPSTAT_WHAT_SINGLE
infrun: need to step-over [Remote target]
../../src/gdb/infrun.c:5200: internal-error: switch_back_to_stepped_thread: Assertion `!tp->control.trap_expected' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Quit this debugging session? (y or n)
The bug triggers if the thread trips on a breakpoint that needs stepping
over, just after stepping over another breakpoint. The condbreak-call-false.c
test involves an infcall, and therefore saving/restoring of trap_expected,
but that's not really necessary to trigger the bug.
On aarch64, the "foo" function ends up set on the function's first instruction:
(gdb) b foo if zero ()
Breakpoint 4 at 0x400518: file testsuite/gdb.base/condbreak-call-false.c, line 25.
(gdb) disassemble foo
Dump of assembler code for function foo:
0x0000000000400518 <+0>: mov w0, #0x17 // #23
0x000000000040051c <+4>: ret
While on x86_64, and most probably ARM too, it ends up set a couple
instructions further down:
(gdb) b foo if zero ()
Breakpoint 7 at 0x40054b: file ../../../src/gdb/testsuite/gdb.base/condbreak-call-false.c, line 25.
(gdb) disassemble foo
Dump of assembler code for function foo:
0x0000000000400547 <+0>: push %rbp
0x0000000000400548 <+1>: mov %rsp,%rbp
0x000000000040054b <+4>: mov $0x17,%eax
0x0000000000400550 <+9>: pop %rbp
0x0000000000400551 <+10>: retq
End of assembler dump.
So on aarch64, we hit the breakpoint at foo, which doesn't cause a stop,
just after stepping over the breakpoint at main.
The new gdb.base/consecutive-step-over.exp test added by the patch
linked above should trigger this issue, without infcalls, on all
platforms.
Thanks,
--
Pedro Alves
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 5/6] Handle multiple step-overs
2014-04-22 18:42 ` Pedro Alves
@ 2014-04-29 11:02 ` Alan Lawrence
2014-04-29 12:35 ` Pedro Alves
0 siblings, 1 reply; 6+ messages in thread
From: Alan Lawrence @ 2014-04-29 11:02 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
Hi Pedro,
Sorry for the delay in replying, I've been away a few days lately. I can confirm
this does indeed appear to be fixed now. Thanks! :)
--Alan
Pedro Alves wrote:
> Hi Alan,
>
> Thanks. This should now be fixed in mainline:
>
> https://sourceware.org/ml/gdb-patches/2014-04/msg00436.html
>
> Could you give it a try please?
>
> On 03/26/2014 04:54 PM, Alan Lawrence wrote:
>> Following this patch, we're seeing an assertion failure of infrun.c:5192,
>>
>> gdb_assert (!tp->control.trap_expected);
>>
>> on the AArch64 platform. The testcase is that added in git commit
>> beb460e8d2ddf5327a6ab146055a6e6e9f552a4b, condbreak-call-false.{c,exp} - I've
>> tried this testcase both before and after your multiple step-over patch, and it
>> succeeds without the patch. I'm not very familiar with gdb internals and
>> stepwise comparing AArch64 against ARM (on which the test passes) sounds at best
>> laborious; hoping there may be some experts here who can help?
>
> Against user mode qemu-aarch64:
>
> Breakpoint 1, main () at testsuite/gdb.base/condbreak-call-false.c:37
> 37 foo ();
> Breakpoint 2 at 0x400518: file testsuite/gdb.base/condbreak-call-false.c, line 25.
> (gdb) c
> Continuing.
> infrun: clear_proceed_status_thread (Remote target)
> infrun: proceed (addr=0xffffffffffffffff, signal=GDB_SIGNAL_DEFAULT, step=0)
> infrun: resume (step=1, signal=GDB_SIGNAL_0), trap_expected=1, current thread [Remote target] at 0x40052c
> infrun: wait_for_inferior ()
> infrun: target_wait (-1, status) =
> infrun: 42000 [Remote target],
> infrun: status->kind = stopped, signal = GDB_SIGNAL_TRAP
> infrun: infwait_normal_state
> infrun: TARGET_WAITKIND_STOPPED
> infrun: stop_pc = 0x400518
> infrun: clear_proceed_status_thread (Remote target)
> infrun: proceed (addr=0x400510, signal=GDB_SIGNAL_0, step=0)
> infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [Remote target] at 0x400510
> infrun: wait_for_inferior ()
> infrun: target_wait (-1, status) =
> infrun: 42000 [Remote target],
> infrun: status->kind = stopped, signal = GDB_SIGNAL_TRAP
> infrun: infwait_normal_state
> infrun: TARGET_WAITKIND_STOPPED
> infrun: stop_pc = 0x4003b0
> infrun: BPSTAT_WHAT_STOP_SILENT
> infrun: stop_stepping
> infrun: BPSTAT_WHAT_SINGLE
> infrun: need to step-over [Remote target]
> ../../src/gdb/infrun.c:5200: internal-error: switch_back_to_stepped_thread: Assertion `!tp->control.trap_expected' failed.
> A problem internal to GDB has been detected,
> further debugging may prove unreliable.
> Quit this debugging session? (y or n)
>
> The bug triggers if the thread trips on a breakpoint that needs stepping
> over, just after stepping over another breakpoint. The condbreak-call-false.c
> test involves an infcall, and therefore saving/restoring of trap_expected,
> but that's not really necessary to trigger the bug.
>
> On aarch64, the "foo" function ends up set on the function's first instruction:
>
> (gdb) b foo if zero ()
> Breakpoint 4 at 0x400518: file testsuite/gdb.base/condbreak-call-false.c, line 25.
> (gdb) disassemble foo
> Dump of assembler code for function foo:
> 0x0000000000400518 <+0>: mov w0, #0x17 // #23
> 0x000000000040051c <+4>: ret
>
> While on x86_64, and most probably ARM too, it ends up set a couple
> instructions further down:
>
> (gdb) b foo if zero ()
> Breakpoint 7 at 0x40054b: file ../../../src/gdb/testsuite/gdb.base/condbreak-call-false.c, line 25.
> (gdb) disassemble foo
> Dump of assembler code for function foo:
> 0x0000000000400547 <+0>: push %rbp
> 0x0000000000400548 <+1>: mov %rsp,%rbp
> 0x000000000040054b <+4>: mov $0x17,%eax
> 0x0000000000400550 <+9>: pop %rbp
> 0x0000000000400551 <+10>: retq
> End of assembler dump.
>
> So on aarch64, we hit the breakpoint at foo, which doesn't cause a stop,
> just after stepping over the breakpoint at main.
>
> The new gdb.base/consecutive-step-over.exp test added by the patch
> linked above should trigger this issue, without infcalls, on all
> platforms.
>
> Thanks,
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 5/6] Handle multiple step-overs
2014-04-29 11:02 ` Alan Lawrence
@ 2014-04-29 12:35 ` Pedro Alves
0 siblings, 0 replies; 6+ messages in thread
From: Pedro Alves @ 2014-04-29 12:35 UTC (permalink / raw)
To: Alan Lawrence; +Cc: gdb-patches
On 04/29/2014 12:02 PM, Alan Lawrence wrote:
> Hi Pedro,
>
> Sorry for the delay in replying, I've been away a few days lately. I can confirm
> this does indeed appear to be fixed now. Thanks! :)
Great, thanks for confirming!
--
Pedro Alves
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 5/6] Handle multiple step-overs.
2014-02-25 20:32 [PATCH 0/6] Fix a bunch of run control bugs Pedro Alves
@ 2014-02-25 20:33 ` Pedro Alves
0 siblings, 0 replies; 6+ messages in thread
From: Pedro Alves @ 2014-02-25 20:33 UTC (permalink / raw)
To: gdb-patches
This test fails with current mainline.
If the program stopped for a breakpoint in thread 1, and then the user
switches to thread 2, and resumes the program, GDB first switches back
to thread 1 to step it over the breakpoint, in order to make progress.
However, that logic only considers the last reported event, assuming
only one thread needs that stepping over dance.
That's actually not true when we play with scheduler-locking. The
patch adds an example to the testsuite of multiple threads needing a
step-over before the stepping thread can be resumed. With current
mainline, the program re-traps the same breakpoint it had already
trapped before.
E.g.:
Breakpoint 2, main () at ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c:99
99 wait_threads (); /* set wait-threads breakpoint here */
(gdb) PASS: gdb.threads/multiple-step-overs.exp: step: continue to breakpoint: run to breakpoint
info threads
Id Target Id Frame
3 Thread 0x7ffff77c9700 (LWP 4310) "multiple-step-o" 0x00000000004007ca in child_function_3 (arg=0x1) at ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c:43
2 Thread 0x7ffff7fca700 (LWP 4309) "multiple-step-o" 0x0000000000400827 in child_function_2 (arg=0x0) at ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c:60
* 1 Thread 0x7ffff7fcb740 (LWP 4305) "multiple-step-o" main () at ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c:99
(gdb) PASS: gdb.threads/multiple-step-overs.exp: step: info threads shows all threads
set scheduler-locking on
(gdb) PASS: gdb.threads/multiple-step-overs.exp: step: set scheduler-locking on
break 44
Breakpoint 3 at 0x4007d3: file ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c, line 44.
(gdb) break 61
Breakpoint 4 at 0x40082d: file ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c, line 61.
(gdb) thread 3
[Switching to thread 3 (Thread 0x7ffff77c9700 (LWP 4310))]
#0 0x00000000004007ca in child_function_3 (arg=0x1) at ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c:43
43 (*myp) ++;
(gdb) PASS: gdb.threads/multiple-step-overs.exp: step: thread 3
continue
Continuing.
Breakpoint 3, child_function_3 (arg=0x1) at ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c:44
44 callme (); /* set breakpoint thread 3 here */
(gdb) PASS: gdb.threads/multiple-step-overs.exp: step: continue to breakpoint: run to breakpoint in thread 3
p *myp = 0
$1 = 0
(gdb) PASS: gdb.threads/multiple-step-overs.exp: step: unbreak loop in thread 3
thread 2
[Switching to thread 2 (Thread 0x7ffff7fca700 (LWP 4309))]
#0 0x0000000000400827 in child_function_2 (arg=0x0) at ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c:60
60 (*myp) ++;
(gdb) PASS: gdb.threads/multiple-step-overs.exp: step: thread 2
continue
Continuing.
Breakpoint 4, child_function_2 (arg=0x0) at ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c:61
61 callme (); /* set breakpoint thread 2 here */
(gdb) PASS: gdb.threads/multiple-step-overs.exp: step: continue to breakpoint: run to breakpoint in thread 2
p *myp = 0
$2 = 0
(gdb) PASS: gdb.threads/multiple-step-overs.exp: step: unbreak loop in thread 2
thread 1
[Switching to thread 1 (Thread 0x7ffff7fcb740 (LWP 4305))]
#0 main () at ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c:99
99 wait_threads (); /* set wait-threads breakpoint here */
(gdb) PASS: gdb.threads/multiple-step-overs.exp: step: thread 1
set scheduler-locking off
(gdb) PASS: gdb.threads/multiple-step-overs.exp: step: set scheduler-locking off
At this point all thread are stopped for a breakpoint that needs stepping over.
(gdb) step
Breakpoint 2, main () at ../../../src/gdb/testsuite/gdb.threads/multiple-step-overs.c:99
99 wait_threads (); /* set wait-threads breakpoint here */
(gdb) FAIL: gdb.threads/multiple-step-overs.exp: step
But that "step" retriggers the same breakpoint instead of making
progress.
The patch teaches GDB to step over all breakpoints of all threads
before resuming the stepping thread.
Tested on x86_64 Fedora 17, against pristine mainline, and also my
branch that implements software single-stepping on x86.
gdb/
2014-02-25 Pedro Alves <palves@redhat.com>
* infrun.c (prepare_to_proceed): Delete.
(thread_still_needs_step_over): New function.
(find_thread_needs_step_over): New function.
(proceed): If the current thread needs a step-over, set its
steping_over_breakpoint flag. Adjust to use
find_thread_needs_step_over instead of prepare_to_proceed.
(process_event_stop_test): For BPSTAT_WHAT_STOP_NOISY and
BPSTAT_WHAT_STOP_SILENT, assume the thread stopped for a
breakpoint.
(switch_back_to_stepped_thread): Step over breakpoints of all
threads not the stepping thread, before switching back to the
stepping thread.
gdb/testsuite/
2014-02-25 Pedro Alves <palves@redhat.com>
* gdb.threads/multiple-step-overs.c: New file.
* gdb.threads/multiple-step-overs.exp: New file.
* gdb.threads/signal-while-stepping-over-bp-other-thread.exp:
Adjust expected infrun debug output.
---
gdb/infrun.c | 238 ++++++++++++---------
gdb/testsuite/gdb.threads/multiple-step-overs.c | 105 +++++++++
gdb/testsuite/gdb.threads/multiple-step-overs.exp | 80 +++++++
.../signal-while-stepping-over-bp-other-thread.exp | 2 +-
4 files changed, 326 insertions(+), 99 deletions(-)
create mode 100644 gdb/testsuite/gdb.threads/multiple-step-overs.c
create mode 100644 gdb/testsuite/gdb.threads/multiple-step-overs.exp
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 1ea8d04..bde40c5 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -90,8 +90,6 @@ static int currently_stepping_or_nexting_callback (struct thread_info *tp,
static void xdb_handle_command (char *args, int from_tty);
-static int prepare_to_proceed (int);
-
static void print_exited_reason (int exitstatus);
static void print_signal_exited_reason (enum gdb_signal siggnal);
@@ -2053,75 +2051,74 @@ clear_proceed_status (void)
}
}
-/* Check the current thread against the thread that reported the most recent
- event. If a step-over is required return TRUE and set the current thread
- to the old thread. Otherwise return FALSE.
-
- This should be suitable for any targets that support threads. */
+/* Returns true if TP is still stopped at a breakpoint that needs
+ stepping-over in order to make progress. If the breakpoint is gone
+ meanwhile, we can skip the whole step-over dance. */
static int
-prepare_to_proceed (int step)
+thread_still_needs_step_over (struct thread_info *tp)
+{
+ if (tp->stepping_over_breakpoint)
+ {
+ struct regcache *regcache = get_thread_regcache (tp->ptid);
+
+ if (breakpoint_here_p (get_regcache_aspace (regcache),
+ regcache_read_pc (regcache)))
+ return 1;
+
+ tp->stepping_over_breakpoint = 0;
+ }
+
+ return 0;
+}
+
+/* Look a thread other than EXCEPT that has previously reported a
+ breakpoint event, and thus needs a step-over in order to make
+ progress. Returns NULL is none is found. STEP indicates whether
+ we're about to step the current thread, in order to decide whether
+ "set scheduler-locking step" applies. */
+
+static struct thread_info *
+find_thread_needs_step_over (int step, struct thread_info *except)
{
- ptid_t wait_ptid;
- struct target_waitstatus wait_status;
int schedlock_enabled;
+ struct thread_info *tp, *current;
/* With non-stop mode on, threads are always handled individually. */
gdb_assert (! non_stop);
- /* Get the last target status returned by target_wait(). */
- get_last_target_status (&wait_ptid, &wait_status);
-
- /* Make sure we were stopped at a breakpoint. */
- if (wait_status.kind != TARGET_WAITKIND_STOPPED
- || (wait_status.value.sig != GDB_SIGNAL_TRAP
- && wait_status.value.sig != GDB_SIGNAL_ILL
- && wait_status.value.sig != GDB_SIGNAL_SEGV
- && wait_status.value.sig != GDB_SIGNAL_EMT))
- {
- return 0;
- }
-
schedlock_enabled = (scheduler_mode == schedlock_on
|| (scheduler_mode == schedlock_step
&& step));
- /* Don't switch over to WAIT_PTID if scheduler locking is on. */
- if (schedlock_enabled)
- return 0;
-
- /* Don't switch over if we're about to resume some other process
- other than WAIT_PTID's, and schedule-multiple is off. */
- if (!sched_multi
- && ptid_get_pid (wait_ptid) != ptid_get_pid (inferior_ptid))
- return 0;
+ current = inferior_thread ();
- /* Switched over from WAIT_PID. */
- if (!ptid_equal (wait_ptid, minus_one_ptid)
- && !ptid_equal (inferior_ptid, wait_ptid))
+ /* If scheduler locking applies, we can avoid iterating over all
+ threads. */
+ if (schedlock_enabled)
{
- struct regcache *regcache = get_thread_regcache (wait_ptid);
+ if (except != current
+ && thread_still_needs_step_over (current))
+ return current;
- if (breakpoint_here_p (get_regcache_aspace (regcache),
- regcache_read_pc (regcache)))
- {
- /* Switch back to WAIT_PID thread. */
- switch_to_thread (wait_ptid);
+ return NULL;
+ }
- if (debug_infrun)
- fprintf_unfiltered (gdb_stdlog,
- "infrun: prepare_to_proceed (step=%d), "
- "switched to [%s]\n",
- step, target_pid_to_str (inferior_ptid));
+ ALL_THREADS (tp)
+ {
+ /* Ignore the EXCEPT thread. */
+ if (tp == except)
+ continue;
+ /* Ignore threads of processes we're not resuming. */
+ if (!sched_multi
+ && ptid_get_pid (tp->ptid) != ptid_get_pid (inferior_ptid))
+ continue;
- /* We return 1 to indicate that there is a breakpoint here,
- so we need to step over it before continuing to avoid
- hitting it straight away. */
- return 1;
- }
+ if (thread_still_needs_step_over (tp))
+ return tp;
}
- return 0;
+ return NULL;
}
/* Basic routine for continuing the program in various fashions.
@@ -2144,8 +2141,6 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal, int step)
struct thread_info *tp;
CORE_ADDR pc;
struct address_space *aspace;
- /* GDB may force the inferior to step due to various reasons. */
- int force_step = 0;
/* If we're stopped at a fork/vfork, follow the branch set by the
"set follow-fork-mode" command; otherwise, we'll just proceed
@@ -2173,6 +2168,9 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal, int step)
if (step < 0)
stop_after_trap = 1;
+ /* Fill in with reasonable starting values. */
+ init_thread_stepping_state (tp);
+
if (addr == (CORE_ADDR) -1)
{
if (pc == stop_pc && breakpoint_here_p (aspace, pc)
@@ -2185,14 +2183,13 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal, int step)
Note, we don't do this in reverse, because we won't
actually be executing the breakpoint insn anyway.
We'll be (un-)executing the previous instruction. */
-
- force_step = 1;
+ tp->stepping_over_breakpoint = 1;
else if (gdbarch_single_step_through_delay_p (gdbarch)
&& gdbarch_single_step_through_delay (gdbarch,
get_current_frame ()))
/* We stepped onto an instruction that needs to be stepped
again before re-inserting the breakpoint, do so. */
- force_step = 1;
+ tp->stepping_over_breakpoint = 1;
}
else
{
@@ -2211,6 +2208,8 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal, int step)
;
else
{
+ struct thread_info *step_over;
+
/* In a multi-threaded task we may select another thread and
then continue or step.
@@ -2219,31 +2218,29 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal, int step)
execution (i.e. it will report a breakpoint hit incorrectly).
So we must step over it first.
- prepare_to_proceed checks the current thread against the
- thread that reported the most recent event. If a step-over
- is required it returns TRUE and sets the current thread to
- the old thread. */
-
- /* Store the prev_pc for the stepping thread too, needed by
- switch_back_to_stepping thread. */
- tp->prev_pc = regcache_read_pc (get_current_regcache ());
-
- if (prepare_to_proceed (step))
+ Look for a thread other than the current (TP) that reported a
+ breakpoint hit and hasn't been resumed yet since. */
+ step_over = find_thread_needs_step_over (step, tp);
+ if (step_over != NULL)
{
- force_step = 1;
- /* The current thread changed. */
- tp = inferior_thread ();
+ if (debug_infrun)
+ fprintf_unfiltered (gdb_stdlog,
+ "infrun: need to step-over [%s] first\n",
+ target_pid_to_str (step_over->ptid));
+
+ /* Store the prev_pc for the stepping thread too, needed by
+ switch_back_to_stepping thread. */
+ tp->prev_pc = regcache_read_pc (get_current_regcache ());
+ switch_to_thread (step_over->ptid);
+ tp = step_over;
}
}
- if (force_step)
- tp->control.trap_expected = 1;
-
/* If we need to step over a breakpoint, and we're not using
displaced stepping to do so, insert all breakpoints (watchpoints,
etc.) but the one we're stepping over, step one instruction, and
then re-insert the breakpoint when that step is finished. */
- if (tp->control.trap_expected && !use_displaced_stepping (gdbarch))
+ if (tp->stepping_over_breakpoint && !use_displaced_stepping (gdbarch))
{
struct regcache *regcache = get_current_regcache ();
@@ -2258,6 +2255,8 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal, int step)
insert_breakpoints ();
+ tp->control.trap_expected = tp->stepping_over_breakpoint;
+
if (!non_stop)
{
/* Pass the last stop signal to the thread we're resuming,
@@ -2321,14 +2320,11 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal, int step)
correctly when the inferior is stopped. */
tp->prev_pc = regcache_read_pc (get_current_regcache ());
- /* Fill in with reasonable starting values. */
- init_thread_stepping_state (tp);
-
/* Reset to normal state. */
init_infwait_state ();
/* Resume inferior. */
- resume (force_step || step || bpstat_should_step (),
+ resume (tp->control.trap_expected || step || bpstat_should_step (),
tp->suspend.stop_signal);
/* Wait for it to stop (if not standalone)
@@ -4468,8 +4464,10 @@ process_event_stop_test (struct execution_control_state *ecs)
fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_STOP_NOISY\n");
stop_print_frame = 1;
- /* We are about to nuke the step_resume_breakpointt via the
- cleanup chain, so no need to worry about it here. */
+ /* Assume the thread stopped for a breapoint. We'll still check
+ whether a/the breakpoint is there when the thread is next
+ resumed. */
+ ecs->event_thread->stepping_over_breakpoint = 1;
stop_stepping (ecs);
return;
@@ -4479,9 +4477,10 @@ process_event_stop_test (struct execution_control_state *ecs)
fprintf_unfiltered (gdb_stdlog, "infrun: BPSTAT_WHAT_STOP_SILENT\n");
stop_print_frame = 0;
- /* We are about to nuke the step_resume_breakpoin via the
- cleanup chain, so no need to worry about it here. */
-
+ /* Assume the thread stopped for a breapoint. We'll still check
+ whether a/the breakpoint is there when the thread is next
+ resumed. */
+ ecs->event_thread->stepping_over_breakpoint = 1;
stop_stepping (ecs);
return;
@@ -5106,25 +5105,68 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
if (!non_stop)
{
struct thread_info *tp;
-
- tp = iterate_over_threads (currently_stepping_or_nexting_callback,
- ecs->event_thread);
- if (tp)
+ struct thread_info *stepping_thread;
+
+ /* If any thread is blocked on some internal breakpoint, and we
+ simply need to step over that breakpoint to get it going
+ again, do that first. */
+
+ /* However, if we see an event for the stepping thread, then we
+ know all other threads have been moved past their breakpoints
+ already. Let the caller check whether the step is finished,
+ etc., before deciding to move it past a breakpoint. */
+ if (ecs->event_thread->control.step_range_end)
+ return 0;
+
+ /* Check if the current thread is blocking on an incomplete
+ step-over, interrupted by a random signal. */
+ if (ecs->event_thread->control.trap_expected
+ && ecs->event_thread->suspend.stop_signal != GDB_SIGNAL_TRAP)
{
- struct frame_info *frame;
- struct gdbarch *gdbarch;
+ if (debug_infrun)
+ {
+ fprintf_unfiltered (gdb_stdlog,
+ "infrun: need to finish step-over of [%s]\n",
+ target_pid_to_str (ecs->event_thread->ptid));
+ }
+ keep_going (ecs);
+ return 1;
+ }
- /* However, if the current thread is blocked on some internal
- breakpoint, and we simply need to step over that breakpoint
- to get it going again, do that first. */
- if ((ecs->event_thread->control.trap_expected
- && ecs->event_thread->suspend.stop_signal != GDB_SIGNAL_TRAP)
- || ecs->event_thread->stepping_over_breakpoint)
+ stepping_thread
+ = iterate_over_threads (currently_stepping_or_nexting_callback,
+ ecs->event_thread);
+
+ /* Check if any other thread except the stepping thread that
+ needs to start a step-over. Do that before actually
+ proceeding with step/next/etc. */
+ tp = find_thread_needs_step_over (stepping_thread != NULL,
+ stepping_thread);
+ if (tp != NULL)
+ {
+ if (debug_infrun)
{
- keep_going (ecs);
- return 1;
+ fprintf_unfiltered (gdb_stdlog,
+ "infrun: need to step-over [%s]\n",
+ target_pid_to_str (tp->ptid));
}
+ gdb_assert (!tp->control.trap_expected);
+ gdb_assert (!tp->control.step_range_end);
+
+ ecs->ptid = tp->ptid;
+ ecs->event_thread = tp;
+ switch_to_thread (ecs->ptid);
+ keep_going (ecs);
+ return 1;
+ }
+
+ tp = stepping_thread;
+ if (tp != NULL)
+ {
+ struct frame_info *frame;
+ struct gdbarch *gdbarch;
+
/* If the stepping thread exited, then don't try to switch
back and resume it, which could fail in several different
ways depending on the target. Instead, just keep going.
@@ -5687,7 +5729,7 @@ keep_going (struct execution_control_state *ecs)
(watchpoints, etc.) but the one we're stepping over, step one
instruction, and then re-insert the breakpoint when that step
is finished. */
- if (ecs->event_thread->stepping_over_breakpoint
+ if (thread_still_needs_step_over (ecs->event_thread)
&& !use_displaced_stepping (get_regcache_arch (regcache)))
{
/* Can't step over more than one breakpoint simultaneously
diff --git a/gdb/testsuite/gdb.threads/multiple-step-overs.c b/gdb/testsuite/gdb.threads/multiple-step-overs.c
new file mode 100644
index 0000000..523a88a
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/multiple-step-overs.c
@@ -0,0 +1,105 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2009-2014 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 <unistd.h>
+#include <stdlib.h>
+#include <signal.h>
+
+unsigned int args[2];
+
+pthread_barrier_t barrier;
+pthread_t child_thread_2, child_thread_3;
+
+void
+callme (void)
+{
+}
+
+void *
+child_function_3 (void *arg)
+{
+ int my_number = (long) arg;
+ volatile int *myp = (int *) &args[my_number];
+
+ pthread_barrier_wait (&barrier);
+
+ while (*myp > 0)
+ {
+ (*myp) ++;
+ callme (); /* set breakpoint thread 3 here */
+ }
+
+ pthread_exit (NULL);
+}
+
+void *
+child_function_2 (void *arg)
+{
+ int my_number = (long) arg;
+ volatile int *myp = (int *) &args[my_number];
+
+ pthread_barrier_wait (&barrier);
+
+ while (*myp > 0)
+ {
+ (*myp) ++;
+ callme (); /* set breakpoint thread 2 here */
+ }
+
+ pthread_exit (NULL);
+}
+
+static int
+wait_threads (void)
+{
+ return 1; /* in wait_threads */
+}
+
+int
+main ()
+{
+ int res;
+ long i;
+
+ /* Call these early so that PLTs for these are resolved soon,
+ instead of in the threads. RTLD_NOW should work as well. */
+ usleep (0);
+ pthread_barrier_init (&barrier, NULL, 1);
+ pthread_barrier_wait (&barrier);
+
+ pthread_barrier_init (&barrier, NULL, 2);
+
+ i = 0;
+ args[i] = 1;
+ res = pthread_create (&child_thread_2,
+ NULL, child_function_2, (void *) i);
+ pthread_barrier_wait (&barrier);
+ callme ();
+
+ i = 1;
+ args[i] = 1;
+ res = pthread_create (&child_thread_3,
+ NULL, child_function_3, (void *) i);
+ pthread_barrier_wait (&barrier);
+ wait_threads (); /* set wait-threads breakpoint here */
+
+ pthread_join (child_thread_2, NULL);
+ pthread_join (child_thread_3, NULL);
+
+ exit(EXIT_SUCCESS);
+}
diff --git a/gdb/testsuite/gdb.threads/multiple-step-overs.exp b/gdb/testsuite/gdb.threads/multiple-step-overs.exp
new file mode 100644
index 0000000..4426586
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/multiple-step-overs.exp
@@ -0,0 +1,80 @@
+# Copyright (C) 2011-2014 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 that GDB steps over all breakpoints of threads not the stepping
+# thread, before actually proceeding with the stepped thread.
+
+standard_testfile
+set executable ${testfile}
+
+if [target_info exists gdb,nosignals] {
+ verbose "Skipping ${testfile}.exp because of nosignals."
+ return -1
+}
+
+if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
+ executable [list debug "incdir=${objdir}"]] != "" } {
+ return -1
+}
+
+# Prepare environment for test. PREFIX is used as prefix in test
+# messages.
+
+proc setup { prefix } {
+ global executable
+
+ with_test_prefix $prefix {
+ clean_restart $executable
+
+ if ![runto_main] {
+ return -1
+ }
+
+ gdb_breakpoint [gdb_get_line_number "set wait-threads breakpoint here"]
+ gdb_continue_to_breakpoint "run to breakpoint"
+ gdb_test "info threads" "3 .* 2 .*\\\* 1.*" "info threads shows all threads"
+
+ gdb_test_no_output "set scheduler-locking on"
+
+ gdb_breakpoint [gdb_get_line_number "set breakpoint thread 3 here"]
+ gdb_breakpoint [gdb_get_line_number "set breakpoint thread 2 here"]
+
+ gdb_test "thread 3" "Switching.*"
+ gdb_continue_to_breakpoint "run to breakpoint in thread 3"
+ gdb_test "p *myp = 0" " = 0" "unbreak loop in thread 3"
+
+ gdb_test "thread 2" "Switching.*"
+ gdb_continue_to_breakpoint "run to breakpoint in thread 2"
+ gdb_test "p *myp = 0" " = 0" "unbreak loop in thread 2"
+
+ # Switch back to thread 1 and disable scheduler locking.
+ gdb_test "thread 1" "Switching.*"
+ gdb_test "set scheduler-locking off"
+
+ # Now all 3 threads are stopped for a breakpoint that needs to
+ # be stepped over before thread 1 is resumed.
+ }
+}
+
+setup "step"
+gdb_test "step" "in wait_threads .*"
+
+setup "next"
+gdb_test "set debug infrun 1" ".*"
+gdb_test "next" "pthread_join.*"
+
+setup "continue"
+gdb_breakpoint [gdb_get_line_number "EXIT_SUCCESS"]
+gdb_test "continue" "EXIT_SUCCESS.*"
diff --git a/gdb/testsuite/gdb.threads/signal-while-stepping-over-bp-other-thread.exp b/gdb/testsuite/gdb.threads/signal-while-stepping-over-bp-other-thread.exp
index bf5ea60..c46dad2 100644
--- a/gdb/testsuite/gdb.threads/signal-while-stepping-over-bp-other-thread.exp
+++ b/gdb/testsuite/gdb.threads/signal-while-stepping-over-bp-other-thread.exp
@@ -107,7 +107,7 @@ gdb_test "set debug infrun 1"
gdb_test \
"step" \
- ".*prepare_to_proceed \\(step=1\\), switched to.*signal arrived while stepping over breakpoint.*switching back to stepped thread.*stepped to a different line.*callme.*" \
+ ".*need to step-over.*resume \\(step=1.*signal arrived while stepping over breakpoint.*switching back to stepped thread.*stepped to a different line.*callme.*" \
"step"
set cnt_after [get_value "args\[$my_number\]" "get count after step"]
--
1.7.11.7
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-04-29 12:35 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-26 16:54 [PATCH 5/6] Handle multiple step-overs Alan Lawrence
2014-04-04 11:54 ` Pedro Alves
2014-04-22 18:42 ` Pedro Alves
2014-04-29 11:02 ` Alan Lawrence
2014-04-29 12:35 ` Pedro Alves
-- strict thread matches above, loose matches on Subject: below --
2014-02-25 20:32 [PATCH 0/6] Fix a bunch of run control bugs Pedro Alves
2014-02-25 20:33 ` [PATCH 5/6] Handle multiple step-overs 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).