[pushed] Consecutive step-overs trigger internal error.

[pushed] Consecutive step-overs trigger internal error.

[Pedro Alves]

If a thread trips on a breakpoint that needs stepping over just after
finishing a step over, GDB currently fails an assertion.  This is a
regression caused by the "Handle multiple step-overs." patch
(99619beac6252113fed212fdb9e1ab97bface423) at
https://sourceware.org/ml/gdb-patches/2014-02/msg00765.html.

 (gdb) x /4i $pc
 => 0x400540 <main+4>:   movl   $0x0,0x2003da(%rip)        # 0x600924 <i>
    0x40054a <main+14>:  movl   $0x1,0x2003d0(%rip)        # 0x600924 <i>
    0x400554 <main+24>:  movl   $0x2,0x2003c6(%rip)        # 0x600924 <i>
    0x40055e <main+34>:  movl   $0x3,0x2003bc(%rip)        # 0x600924 <i>
 (gdb) PASS: gdb.base/consecutive-step-over.exp: get breakpoint addresses
 break *0x40054a
 Breakpoint 2 at 0x40054a: file ../../../src/gdb/testsuite/gdb.base/consecutive-step-over.c, line 23.
 (gdb) PASS: gdb.base/consecutive-step-over.exp: insn 1: set breakpoint
 condition $bpnum condition
 (gdb) PASS: gdb.base/consecutive-step-over.exp: insn 1: set condition
 break *0x400554
 Breakpoint 3 at 0x400554: file ../../../src/gdb/testsuite/gdb.base/consecutive-step-over.c, line 24.
 (gdb) PASS: gdb.base/consecutive-step-over.exp: insn 2: set breakpoint
 condition $bpnum condition
 (gdb) PASS: gdb.base/consecutive-step-over.exp: insn 2: set condition
 break *0x40055e
 Breakpoint 4 at 0x40055e: file ../../../src/gdb/testsuite/gdb.base/consecutive-step-over.c, line 25.
 (gdb) PASS: gdb.base/consecutive-step-over.exp: insn 3: set breakpoint
 condition $bpnum condition
 (gdb) PASS: gdb.base/consecutive-step-over.exp: insn 3: set condition
 break 27
 Breakpoint 5 at 0x400568: file ../../../src/gdb/testsuite/gdb.base/consecutive-step-over.c, line 27.
 (gdb) continue
 Continuing.
 ../../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.
 FAIL: gdb.base/consecutive-step-over.exp: continue to breakpoint: break here (GDB internal error)

The assertion fails, because the code is not expecting that the event
thread itself might need another step over.  IOW, not expecting that
TP in:

     tp = find_thread_needs_step_over (stepping_thread != NULL,
                                      stepping_thread);

could be the event thread.

A small fix for this would be to clear the event thread's
trap_expected earlier, before asserting.  But looking deeper, although
currently_stepping_or_nexting_callback's intention is finding the
thread that is doing a step/next, it also returns the thread that is
doing a step-over dance, with trap_expected set.  If there ever was a
reason for that (it was I who added
currently_stepping_or_nexting_callback , but I can't recall why I put
trap_expected there in the first place), the only remaining reason
nowadays is to aid in implementing switch_back_to_stepped_thread's
assertion that is now triggering, by piggybacking on the walk over all
threads, thus avoiding a separate walk.  This is quite obscure, and I
think we can do even better, by merging the walks that look for the
stepping thread, and the walk that looks for some thread that might
need a step over.

Tested on x86_64 Fedora 17, native and gdbserver, and also native on
top of my "software single-step on x86_64" series.

gdb/
2014-04-22  Pedro Alves  <palves@redhat.com>

	* infrun.c (schedlock_applies): New function, factored out from
	find_thread_needs_step_over.
	(find_thread_needs_step_over): Use it.
	(switch_back_to_stepped_thread): Always clear trap_expected if the
	step over is finished.  Return early if scheduler locking applies.
	Look for the stepping thread and a potential step-over thread with
	a single loop.
	(currently_stepping_or_nexting_callback): Delete.

2014-04-22  Pedro Alves  <palves@redhat.com>

	* gdb.base/consecutive-step-over.c: New file.
	* gdb.base/consecutive-step-over.exp: New file.
---
 gdb/ChangeLog                                    |  11 ++
 gdb/infrun.c                                     | 125 +++++++++++++++--------
 gdb/testsuite/ChangeLog                          |   5 +
 gdb/testsuite/gdb.base/consecutive-step-over.c   |  28 +++++
 gdb/testsuite/gdb.base/consecutive-step-over.exp |  70 +++++++++++++
 5 files changed, 195 insertions(+), 44 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/consecutive-step-over.c
 create mode 100644 gdb/testsuite/gdb.base/consecutive-step-over.exp

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 120b70b..6de9f69 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,14 @@
+2014-04-22  Pedro Alves  <palves@redhat.com>
+
+	* infrun.c (schedlock_applies): New function, factored out from
+	find_thread_needs_step_over.
+	(find_thread_needs_step_over): Use it.
+	(switch_back_to_stepped_thread): Always clear trap_expected if the
+	step over is finished.  Return early if scheduler locking applies.
+	Look for the stepping thread and a potential step-over thread with
+	a single loop.
+	(currently_stepping_or_nexting_callback): Delete.
+
 2014-04-22  Nick Clifton  <nickc@redhat.com>
 
 	* NEWS: Mention that ARM sim now supports tracing.
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 31bb132..ab39b6e 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -85,9 +85,6 @@ static void set_schedlock_func (char *args, int from_tty,
 
 static int currently_stepping (struct thread_info *tp);
 
-static int currently_stepping_or_nexting_callback (struct thread_info *tp,
-						   void *data);
-
 static void xdb_handle_command (char *args, int from_tty);
 
 static void print_exited_reason (int exitstatus);
@@ -2107,6 +2104,17 @@ thread_still_needs_step_over (struct thread_info *tp)
   return 0;
 }
 
+/* Returns true if scheduler locking applies.  STEP indicates whether
+   we're about to do a step/next-like command to a thread.  */
+
+static int
+schedlock_applies (int step)
+{
+  return (scheduler_mode == schedlock_on
+	  || (scheduler_mode == schedlock_step
+	      && step));
+}
+
 /* 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
@@ -2116,21 +2124,16 @@ thread_still_needs_step_over (struct thread_info *tp)
 static struct thread_info *
 find_thread_needs_step_over (int step, struct thread_info *except)
 {
-  int schedlock_enabled;
   struct thread_info *tp, *current;
 
   /* With non-stop mode on, threads are always handled individually.  */
   gdb_assert (! non_stop);
 
-  schedlock_enabled = (scheduler_mode == schedlock_on
-		       || (scheduler_mode == schedlock_step
-			   && step));
-
   current = inferior_thread ();
 
   /* If scheduler locking applies, we can avoid iterating over all
      threads.  */
-  if (schedlock_enabled)
+  if (schedlock_applies (step))
     {
       if (except != current
 	  && thread_still_needs_step_over (current))
@@ -5137,6 +5140,7 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
     {
       struct thread_info *tp;
       struct thread_info *stepping_thread;
+      struct thread_info *step_over;
 
       /* If any thread is blocked on some internal breakpoint, and we
 	 simply need to step over that breakpoint to get it going
@@ -5179,17 +5183,72 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
 	 return 1;
        }
 
-      stepping_thread
-	= iterate_over_threads (currently_stepping_or_nexting_callback,
-				ecs->event_thread);
+      /* Otherwise, we no longer expect a trap in the current thread.
+	 Clear the trap_expected flag before switching back -- this is
+	 what keep_going does as well, if we call it.  */
+      ecs->event_thread->control.trap_expected = 0;
+
+      /* If scheduler locking applies even if not stepping, there's no
+	 need to walk over threads.  Above we've checked whether the
+	 current thread is stepping.  If some other thread not the
+	 event thread is stepping, then it must be that scheduler
+	 locking is not in effect.  */
+      if (schedlock_applies (0))
+	return 0;
+
+      /* Look for the stepping/nexting thread, and check if any other
+	 thread other than the stepping thread needs to start a
+	 step-over.  Do all step-overs before actually proceeding with
+	 step/next/etc.  */
+      stepping_thread = NULL;
+      step_over = NULL;
+      ALL_THREADS (tp)
+        {
+	  /* Ignore threads of processes we're not resuming.  */
+	  if (!sched_multi
+	      && ptid_get_pid (tp->ptid) != ptid_get_pid (inferior_ptid))
+	    continue;
+
+	  /* When stepping over a breakpoint, we lock all threads
+	     except the one that needs to move past the breakpoint.
+	     If a non-event thread has this set, the "incomplete
+	     step-over" check above should have caught it earlier.  */
+	  gdb_assert (!tp->control.trap_expected);
+
+	  /* Did we find the stepping thread?  */
+	  if (tp->control.step_range_end)
+	    {
+	      /* Yep.  There should only one though.  */
+	      gdb_assert (stepping_thread == NULL);
+
+	      /* The event thread is handled at the top, before we
+		 enter this loop.  */
+	      gdb_assert (tp != ecs->event_thread);
+
+	      /* If some thread other than the event thread is
+		 stepping, then scheduler locking can't be in effect,
+		 otherwise we wouldn't have resumed the current event
+		 thread in the first place.  */
+	      gdb_assert (!schedlock_applies (1));
+
+	      stepping_thread = tp;
+	    }
+	  else if (thread_still_needs_step_over (tp))
+	    {
+	      step_over = tp;
+
+	      /* At the top we've returned early if the event thread
+		 is stepping.  If some other thread not the event
+		 thread is stepping, then scheduler locking can't be
+		 in effect, and we can resume this thread.  No need to
+		 keep looking for the stepping thread then.  */
+	      break;
+	    }
+	}
 
-      /* 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 (step_over != NULL)
 	{
+	  tp = step_over;
 	  if (debug_infrun)
 	    {
 	      fprintf_unfiltered (gdb_stdlog,
@@ -5197,14 +5256,9 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
 				  target_pid_to_str (tp->ptid));
 	    }
 
-	  gdb_assert (!tp->control.trap_expected);
+	  /* Only the stepping thread should have this set.  */
 	  gdb_assert (tp->control.step_range_end == 0);
 
-	  /* We no longer expect a trap in the current thread.  Clear
-	     the trap_expected flag before switching.  This is what
-	     keep_going would do as well, if we called it.  */
-	  ecs->event_thread->control.trap_expected = 0;
-
 	  ecs->ptid = tp->ptid;
 	  ecs->event_thread = tp;
 	  switch_to_thread (ecs->ptid);
@@ -5212,12 +5266,13 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
 	  return 1;
 	}
 
-      tp = stepping_thread;
-      if (tp != NULL)
+      if (stepping_thread != NULL)
 	{
 	  struct frame_info *frame;
 	  struct gdbarch *gdbarch;
 
+	  tp = stepping_thread;
+
 	  /* 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.
@@ -5250,11 +5305,6 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
 	      return 1;
 	    }
 
-	  /* Otherwise, we no longer expect a trap in the current thread.
-	     Clear the trap_expected flag before switching back -- this is
-	     what keep_going would do as well, if we called it.  */
-	  ecs->event_thread->control.trap_expected = 0;
-
 	  if (debug_infrun)
 	    fprintf_unfiltered (gdb_stdlog,
 				"infrun: switching back to stepped thread\n");
@@ -5325,19 +5375,6 @@ currently_stepping (struct thread_info *tp)
 	  || bpstat_should_step ());
 }
 
-/* Returns true if any thread *but* the one passed in "data" is in the
-   middle of stepping or of handling a "next".  */
-
-static int
-currently_stepping_or_nexting_callback (struct thread_info *tp, void *data)
-{
-  if (tp == data)
-    return 0;
-
-  return (tp->control.step_range_end
- 	  || tp->control.trap_expected);
-}
-
 /* Inferior has stepped into a subroutine call with source code that
    we should not step over.  Do step to the first line of code in
    it.  */
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 1de6dc0..2f42ad1 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,10 @@
 2014-04-22  Pedro Alves  <palves@redhat.com>
 
+	* gdb.base/consecutive-step-over.c: New file.
+	* gdb.base/consecutive-step-over.exp: New file.
+
+2014-04-22  Pedro Alves  <palves@redhat.com>
+
 	* lib/gdb.exp (gdb_continue_to_breakpoint): Use gdb_test_multiple
 	instead of send_gdb/gdb_expect.
 
diff --git a/gdb/testsuite/gdb.base/consecutive-step-over.c b/gdb/testsuite/gdb.base/consecutive-step-over.c
new file mode 100644
index 0000000..e7c8e9d
--- /dev/null
+++ b/gdb/testsuite/gdb.base/consecutive-step-over.c
@@ -0,0 +1,28 @@
+/* Copyright 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/>.  */
+
+volatile int i;
+volatile int condition;
+
+int
+main (void)
+{
+  i = 0;
+  i = 1;
+  i = 2;
+  i = 3;
+
+  return 0; /* break here */
+}
diff --git a/gdb/testsuite/gdb.base/consecutive-step-over.exp b/gdb/testsuite/gdb.base/consecutive-step-over.exp
new file mode 100644
index 0000000..3f78042
--- /dev/null
+++ b/gdb/testsuite/gdb.base/consecutive-step-over.exp
@@ -0,0 +1,70 @@
+# Copyright 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/>.
+
+#
+# Regression test for a bug where GDB would internal error if a thread
+# runs into a breakpoint that needs stepping over, just after stepping
+# over another breakpoint, without a user visible stop in between.
+#
+standard_testfile
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
+    return -1
+}
+
+if ![runto_main] then {
+    fail "Can't run to main"
+    return 0
+}
+
+# Make sure the target doesn't hide the breakpoint hits (that don't
+# cause a user visible stop) from GDB.
+gdb_test_no_output "set breakpoint condition-evaluation host"
+
+set up_to_nl "\[^\r\n\]+\[\r\n\]+"
+
+# Number of consecutive breakpoints in a row to try.
+set n_insns 3
+
+# Extract addresses of a few consecutive instructions.
+set test "get breakpoint addresses"
+if { [gdb_test_multiple "x /[expr $n_insns + 1]i \$pc" $test {
+    -re "=> $hex${up_to_nl}   ($hex)${up_to_nl}   ($hex)${up_to_nl}   ($hex)${up_to_nl}$gdb_prompt $" {
+	for {set i 1} {$i <= $n_insns} {incr i} {
+	    set bp_addrs($i) $expect_out($i,string)
+	}
+	pass $test
+    }
+}] != 0 } {
+    # No use proceeding if bp_addrs wasn't set.
+    return
+}
+
+for {set i 1} {$i <= $n_insns} {incr i} {
+    with_test_prefix "insn $i" {
+	gdb_test "break \*$bp_addrs($i)" \
+	    "Breakpoint $decimal at $bp_addrs($i): file .*" \
+	    "set breakpoint"
+
+	# Give the breakpoint a condition that always fails, so that
+	# the thread is immediately re-resumed.
+	gdb_test_no_output "condition \$bpnum condition" \
+	    "set condition"
+    }
+}
+
+set lineno [gdb_get_line_number "break here"]
+gdb_breakpoint $lineno
+gdb_continue_to_breakpoint "break here" ".*break here.*"
-- 
1.7.11.7

Regression with default scheduler-locking=step [Re: [pushed] Consecutive step-overs trigger internal error.]

[Jan Kratochvil]

[-- Attachment #1: Type: text/plain, Size: 1277 bytes --]

On Tue, 22 Apr 2014 20:24:28 +0200, Pedro Alves wrote:
> Tested on x86_64 Fedora 17, native and gdbserver, and also native on
> top of my "software single-step on x86_64" series.

483805cf9ea5a6dace41415d8830e93fccc49c43 is the first bad commit
commit 483805cf9ea5a6dace41415d8830e93fccc49c43
Author: Pedro Alves <palves@redhat.com>
Date:   Tue Apr 22 15:00:56 2014 +0100
    Consecutive step-overs trigger internal error.
    
(gdb) next^M
[Thread 0x7ffff7fda700 (LWP 27168) exited]^M
[New LWP 27168]^M
[Thread 0x7ffff74ee700 (LWP 27174) exited]^M
process 27168 is executing new program: /home/jkratoch/redhat/gdb-clean/gdb/testsuite/gdb.threads/thread-execl^M
[Thread debugging using libthread_db enabled]^M
Using host libthread_db library "/lib64/libthread_db.so.1".^M
infrun.c:5225: internal-error: switch_back_to_stepped_thread: Assertion `!schedlock_applies (1)' failed.^M
A problem internal to GDB has been detected,^M
further debugging may prove unreliable.^M
Quit this debugging session? (y or n) FAIL: gdb.threads/thread-execl.exp: get to main in new image (GDB internal error)
Resyncing due to internal error.

The regressions happens only with the attached patch which I am not sure if it
is considered as a valid FSF GDB regression or not but I think it is.


Jan

[-- Attachment #2: execl.patch --]
[-- Type: text/plain, Size: 440 bytes --]

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 47604c7..3f0d46b 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1661,7 +1661,7 @@ static const char *const scheduler_enums[] = {
   schedlock_step,
   NULL
 };
-static const char *scheduler_mode = schedlock_off;
+static const char *scheduler_mode = schedlock_step;
 static void
 show_scheduler_mode (struct ui_file *file, int from_tty,
 		     struct cmd_list_element *c, const char *value)

Re: Regression with default scheduler-locking=step [Re: [pushed] Consecutive step-overs trigger internal error.]

[Pedro Alves]

On 06/17/2014 08:24 PM, Jan Kratochvil wrote:
> On Tue, 22 Apr 2014 20:24:28 +0200, Pedro Alves wrote:
>> Tested on x86_64 Fedora 17, native and gdbserver, and also native on
>> top of my "software single-step on x86_64" series.
> 
> 483805cf9ea5a6dace41415d8830e93fccc49c43 is the first bad commit
> commit 483805cf9ea5a6dace41415d8830e93fccc49c43
> Author: Pedro Alves <palves@redhat.com>
> Date:   Tue Apr 22 15:00:56 2014 +0100
>     Consecutive step-overs trigger internal error.
>     
> (gdb) next^M
> [Thread 0x7ffff7fda700 (LWP 27168) exited]^M
> [New LWP 27168]^M
> [Thread 0x7ffff74ee700 (LWP 27174) exited]^M
> process 27168 is executing new program: /home/jkratoch/redhat/gdb-clean/gdb/testsuite/gdb.threads/thread-execl^M
> [Thread debugging using libthread_db enabled]^M
> Using host libthread_db library "/lib64/libthread_db.so.1".^M
> infrun.c:5225: internal-error: switch_back_to_stepped_thread: Assertion `!schedlock_applies (1)' failed.^M
> A problem internal to GDB has been detected,^M
> further debugging may prove unreliable.^M
> Quit this debugging session? (y or n) FAIL: gdb.threads/thread-execl.exp: get to main in new image (GDB internal error)
> Resyncing due to internal error.

Thanks Jan.

> 
> The regressions happens only with the attached patch which I am not sure if it
> is considered as a valid FSF GDB regression or not but I think it is.

If it worked before, then it's certainly a regression.  The user is
free to do "set scheduler-locking step" herself.

Here's a fix.  Let me know what you think.

8<---------------------------------
From f717378c16cb04f8350935a1336767d2541b36a5 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Wed, 18 Jun 2014 14:20:31 +0100
Subject: [PATCH] Fix next over threaded execl with "set scheduler-locking
 step".

Running gdb.threads/thread-execl.exp with scheduler-locking set to
"step" reveals a problem:

 (gdb) next^M
 [Thread 0x7ffff7fda700 (LWP 27168) exited]^M
 [New LWP 27168]^M
 [Thread 0x7ffff74ee700 (LWP 27174) exited]^M
 process 27168 is executing new program: /home/jkratoch/redhat/gdb-clean/gdb/testsuite/gdb.threads/thread-execl^M
 [Thread debugging using libthread_db enabled]^M
 Using host libthread_db library "/lib64/libthread_db.so.1".^M
 infrun.c:5225: internal-error: switch_back_to_stepped_thread: Assertion `!schedlock_applies (1)' failed.^M
 A problem internal to GDB has been detected,^M
 further debugging may prove unreliable.^M
 Quit this debugging session? (y or n) FAIL: gdb.threads/thread-execl.exp: schedlock step: get to main in new image (GDB internal error)

The assertion is correct.  The issue is that GDB is mistakenly trying
to switch back to an exited thread, that was previously stepping when
it exited.  This is exactly the sort of thing the test wants to make
sure doesn't happen:

	# Now set a breakpoint at `main', and step over the execl call.  The
	# breakpoint at main should be reached.  GDB should not try to revert
	# back to the old thread from the old image and resume stepping it

We don't see this bug with schedlock off only because a different
sequence of events makes GDB manage to delete the thread instead of
marking it exited.

This particular internal error can be fixed by making the loop over
all threads in switch_back_to_stepped_thread skip exited threads.
But, looking over other ALL_THREADS users, all either can or should be
skipping exited threads too.  So for simplicity, this patch replaces
ALL_THREADS with a new macro that skips exited threads itself, and
updates everything to use it.

Tested on x86_64 Fedora 20.

gdb/
2014-06-18  Pedro Alves  <palves@redhat.com>

	* gdbthread.h (ALL_THREADS): Delete.
	(ALL_NON_EXITED_THREADS): New macro.
	* btrace.c (btrace_free_objfile): Use ALL_NON_EXITED_THREADS
	instead of ALL_THREADS.
	* infrun.c (find_thread_needs_step_over)
	(switch_back_to_stepped_thread): Use ALL_NON_EXITED_THREADS
	instead of ALL_THREADS.
	* record-btrace.c (record_btrace_open)
	(record_btrace_stop_recording, record_btrace_close)
	(record_btrace_is_replaying, record_btrace_resume)
	(record_btrace_find_thread_to_move, record_btrace_wait): Likewise.
	* remote.c (append_pending_thread_resumptions): Likewise.

gdb/testsuite/
2014-06-18  Pedro Alves  <palves@redhat.com>

	* gdb.threads/thread-execl.exp (do_test): New procedure, factored
	out from ...
	(top level): ... here.  Iterate running tests under different
	scheduler-locking settings.
---
 gdb/btrace.c                               |  2 +-
 gdb/gdbthread.h                            |  8 ++++--
 gdb/infrun.c                               |  4 +--
 gdb/record-btrace.c                        | 14 +++++-----
 gdb/remote.c                               |  2 +-
 gdb/testsuite/gdb.threads/thread-execl.exp | 44 ++++++++++++++++++++----------
 gdb/thread.c                               |  2 +-
 7 files changed, 46 insertions(+), 30 deletions(-)

diff --git a/gdb/btrace.c b/gdb/btrace.c
index 601eb41..87a171e 100644
--- a/gdb/btrace.c
+++ b/gdb/btrace.c
@@ -958,7 +958,7 @@ btrace_free_objfile (struct objfile *objfile)
 
   DEBUG ("free objfile");
 
-  ALL_THREADS (tp)
+  ALL_NON_EXITED_THREADS (tp)
     btrace_clear (tp);
 }
 
diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index 9f5dee6..64e37c3 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -317,10 +317,12 @@ void thread_change_ptid (ptid_t old_ptid, ptid_t new_ptid);
 typedef int (*thread_callback_func) (struct thread_info *, void *);
 extern struct thread_info *iterate_over_threads (thread_callback_func, void *);
 
-/* Traverse all threads.  */
+/* Traverse all threads, except those that have THREAD_EXITED
+   state.  */
 
-#define ALL_THREADS(T)				\
-  for (T = thread_list; T; T = T->next)
+#define ALL_NON_EXITED_THREADS(T)				\
+  for (T = thread_list; T; T = T->next) \
+    if ((T)->state != THREAD_EXITED)
 
 extern int thread_count (void);
 
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 4e808d9..e8e26e0 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -2160,7 +2160,7 @@ find_thread_needs_step_over (int step, struct thread_info *except)
       return NULL;
     }
 
-  ALL_THREADS (tp)
+  ALL_NON_EXITED_THREADS (tp)
     {
       /* Ignore the EXCEPT thread.  */
       if (tp == except)
@@ -5204,7 +5204,7 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
 	 step/next/etc.  */
       stepping_thread = NULL;
       step_over = NULL;
-      ALL_THREADS (tp)
+      ALL_NON_EXITED_THREADS (tp)
         {
 	  /* Ignore threads of processes we're not resuming.  */
 	  if (!sched_multi
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index e0c537a8..6a9bfe1 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -206,7 +206,7 @@ record_btrace_open (char *args, int from_tty)
   gdb_assert (record_btrace_thread_observer == NULL);
 
   disable_chain = make_cleanup (null_cleanup, NULL);
-  ALL_THREADS (tp)
+  ALL_NON_EXITED_THREADS (tp)
     if (args == NULL || *args == 0 || number_is_in_list (args, tp->num))
       {
 	btrace_enable (tp);
@@ -238,7 +238,7 @@ record_btrace_stop_recording (struct target_ops *self)
 
   record_btrace_auto_disable ();
 
-  ALL_THREADS (tp)
+  ALL_NON_EXITED_THREADS (tp)
     if (tp->btrace.target != NULL)
       btrace_disable (tp);
 }
@@ -259,7 +259,7 @@ record_btrace_close (struct target_ops *self)
 
   /* We should have already stopped recording.
      Tear down btrace in case we have not.  */
-  ALL_THREADS (tp)
+  ALL_NON_EXITED_THREADS (tp)
     btrace_teardown (tp);
 }
 
@@ -835,7 +835,7 @@ record_btrace_is_replaying (struct target_ops *self)
 {
   struct thread_info *tp;
 
-  ALL_THREADS (tp)
+  ALL_NON_EXITED_THREADS (tp)
     if (btrace_is_replaying (tp))
       return 1;
 
@@ -1522,7 +1522,7 @@ record_btrace_resume (struct target_ops *ops, ptid_t ptid, int step,
 
   /* Stop replaying other threads if the thread to resume is not replaying.  */
   if (!btrace_is_replaying (tp) && execution_direction != EXEC_REVERSE)
-    ALL_THREADS (other)
+    ALL_NON_EXITED_THREADS (other)
       record_btrace_stop_replaying (other);
 
   /* As long as we're not replaying, just forward the request.  */
@@ -1572,7 +1572,7 @@ record_btrace_find_thread_to_move (ptid_t ptid)
     return tp;
 
   /* Otherwise, find one other thread that has been resumed.  */
-  ALL_THREADS (tp)
+  ALL_NON_EXITED_THREADS (tp)
     if ((tp->btrace.flags & BTHR_MOVE) != 0)
       return tp;
 
@@ -1777,7 +1777,7 @@ record_btrace_wait (struct target_ops *ops, ptid_t ptid,
 
   /* Stop all other threads. */
   if (!non_stop)
-    ALL_THREADS (other)
+    ALL_NON_EXITED_THREADS (other)
       other->btrace.flags &= ~BTHR_MOVE;
 
   /* Start record histories anew from the current position.  */
diff --git a/gdb/remote.c b/gdb/remote.c
index 6915dd8..b5318f1 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -4626,7 +4626,7 @@ append_pending_thread_resumptions (char *p, char *endp, ptid_t ptid)
 {
   struct thread_info *thread;
 
-  ALL_THREADS (thread)
+  ALL_NON_EXITED_THREADS (thread)
     if (ptid_match (thread->ptid, ptid)
 	&& !ptid_equal (inferior_ptid, thread->ptid)
 	&& thread->suspend.stop_signal != GDB_SIGNAL_0
diff --git a/gdb/testsuite/gdb.threads/thread-execl.exp b/gdb/testsuite/gdb.threads/thread-execl.exp
index d837fe4..14b96d2 100644
--- a/gdb/testsuite/gdb.threads/thread-execl.exp
+++ b/gdb/testsuite/gdb.threads/thread-execl.exp
@@ -28,19 +28,33 @@ if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
     return -1
 }
 
-clean_restart ${binfile}
-
-runto_main
-
-# Get ourselves to the thread that execs
-gdb_breakpoint "thread_execler"
-gdb_test "continue" ".*thread_execler.*" "continue to thread start"
-
-# Now set a breakpoint at `main', and step over the execl call.  The
-# breakpoint at main should be reached.  GDB should not try to revert
-# back to the old thread from the old image and resume stepping it
-# (since it is gone).
-gdb_breakpoint "main"
-gdb_test "next" ".*main.*" "get to main in new image"
+# Run the test proper.  SCHEDLOCK specifies what scheduler-locking
+# should be set to.
+
+proc do_test { schedlock } {
+    global binfile
+
+    with_test_prefix "schedlock $schedlock" {
+	clean_restart ${binfile}
+
+	if ![runto_main] {
+	    return 0
+	}
+
+	# Get ourselves to the thread that execs.
+	gdb_breakpoint "thread_execler"
+	gdb_test "continue" ".*thread_execler.*" "continue to thread start"
+
+	# Now set a breakpoint at `main', and step over the execl call.  The
+	# breakpoint at main should be reached.  GDB should not try to revert
+	# back to the old thread from the old image and resume stepping it
+	# (since it is gone).
+	gdb_breakpoint "main"
+	gdb_test_no_output "set scheduler-locking $schedlock"
+	gdb_test "next" ".*main.*" "get to main in new image"
+    }
+}
 
-return 0
+foreach schedlock {"off" "step" "on"} {
+    do_test $schedlock
+}
diff --git a/gdb/thread.c b/gdb/thread.c
index 7bc5271..532149d 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -1243,7 +1243,7 @@ thread_apply_all_command (char *cmd, int from_tty)
       ta_cleanup.tp_array = tp_array;
       ta_cleanup.count = tc;
 
-      ALL_THREADS (tp)
+      ALL_NON_EXITED_THREADS (tp)
         {
           tp_array[i] = tp;
           tp->refcount++;
-- 
1.9.3

Re: Regression with default scheduler-locking=step [Re: [pushed] Consecutive step-overs trigger internal error.]

[Jan Kratochvil]

On Wed, 18 Jun 2014 15:52:13 +0200, Pedro Alves wrote:
> Here's a fix.  Let me know what you think.

The change looks OK and as it fixes the assertion I am for the patch.


Thanks,
Jan

Re: Regression with default scheduler-locking=step [Re: [pushed] Consecutive step-overs trigger internal error.]

[Pedro Alves]

On 06/18/2014 06:57 PM, Jan Kratochvil wrote:
> On Wed, 18 Jun 2014 15:52:13 +0200, Pedro Alves wrote:
>> Here's a fix.  Let me know what you think.
> 
> The change looks OK and as it fixes the assertion I am for the patch.

Thanks.  Now pushed to both master and gdb-7.8-branch.

-- 
Pedro Alves