[patch] Sort threads for thread apply all (bt)

[patch] Sort threads for thread apply all (bt)

[Jan Kratochvil]

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

Hi,

downstream Fedora request:
	Please make it easier to find the backtrace of the crashing thread
	https://bugzilla.redhat.com/show_bug.cgi?id=1024504

Currently after loading a core file GDB prints:

Core was generated by `./threadcrash1'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x000000000040062f in start (arg=0x0) at threadcrash1.c:8
8	*(volatile int *)0=0;
(gdb) _

there is nowhere seen which of the threads had crashed.  In reality GDB always
numbers that thread as #1 and it is the current thread that time.  But after
dumping all the info into a file for later analysis it is no longer obvious.
'thread apply all bt' even puts the thread #1 to the _end_ of the output!!!

Should GDB always print after loading a core file what "thread" command would
print?
[Current thread is 1 (Thread 0x7fcbe28fe700 (LWP 15453))]

Or should the "thread" output be printed only in non-interactive mode?

Or something different?

Or is the current behavior OK as is and the tools calling GDB in batch mode
should indicate the thread #1 on their own?

------------------------------------------------------------------------------

I find maybe as good enough and with no risk of UI change flamewar to just
sort the threads by their number.  Currently they are printed as they happen
in the internal GDB list which has no advantage.  Printing thread #1 as the
first one with assumed 'thread apply all bt' (after the core file is loaded)
should make the complaint resolved I guess.

No regressions on {x86_64,x86_64-m32,i686}-fedora22pre-linux-gnu.


Jan


diff of crashing different thread in 2-threaded program:
------------------------------------------------------------------------------
-Reading symbols from /home/jkratoch/t/threadcrash1...done.
-[New LWP 15453]
-[New LWP 15446]
+Reading symbols from /home/jkratoch/t/threadcrash2...done.
+[New LWP 29285]
+[New LWP 29288]
 [Thread debugging using libthread_db enabled]
 Using host libthread_db library "/lib64/libthread_db.so.1".
-Core was generated by `./threadcrash1'.
+Core was generated by `./threadcrash2'.
 Program terminated with signal SIGSEGV, Segmentation fault.
-#0  0x000000000040062f in start (arg=0x0) at threadcrash1.c:8
-8      *(volatile int *)0=0;
+#0  0x0000000000400684 in main () at threadcrash2.c:19
+19     *(volatile int *)0=0;
 (gdb) thread apply all bt
 
-Thread 2 (Thread 0x7fcbe2900700 (LWP 15446)):
+Thread 2 (Thread 0x7feb9c14c700 (LWP 29288)):
 Python Exception <type 'exceptions.ImportError'> No module named gdb.frames:
 #0  0x0000003424ec491d in nanosleep () at ../sysdeps/unix/syscall-template.S:81
 #1  0x0000003424ec47b4 in __sleep (seconds=0) at ../sysdeps/unix/sysv/linux/sleep.c:138
-#2  0x000000000040068a in main () at threadcrash1.c:19
+#2  0x000000000040062a in start (arg=0x0) at threadcrash2.c:7
+#3  0x000000342560752a in start_thread (arg=0x7feb9c14c700) at pthread_create.c:310
+#4  0x0000003424f0079d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:109
 
-Thread 1 (Thread 0x7fcbe28fe700 (LWP 15453)):
+Thread 1 (Thread 0x7feb9c14e700 (LWP 29285)):
 Python Exception <type 'exceptions.ImportError'> No module named gdb.frames:
-#0  0x000000000040062f in start (arg=0x0) at threadcrash1.c:8
-#1  0x000000342560752a in start_thread (arg=0x7fcbe28fe700) at pthread_create.c:310
-#2  0x0000003424f0079d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:109
+#0  0x0000000000400684 in main () at threadcrash2.c:19
 (gdb) thread
-[Current thread is 1 (Thread 0x7fcbe28fe700 (LWP 15453))]
+[Current thread is 1 (Thread 0x7feb9c14e700 (LWP 29285))]
 (gdb) q

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

gdb/ChangeLog
2015-01-15  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* thread.c (tp_array_compar): New function.
	(thread_apply_all_command): Sort tp_array using it.

diff --git a/gdb/thread.c b/gdb/thread.c
index ed20fbe..cdab5d8 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -1382,6 +1382,17 @@ make_cleanup_restore_current_thread (void)
 			    restore_current_thread_cleanup_dtor);
 }
 
+/* Sort an array for struct thread_info pointers by their ascending NUM.  */
+
+static int
+tp_array_compar (const void *ap_voidp, const void *bp_voidp)
+{
+  const struct thread_info *const *ap = ap_voidp;
+  const struct thread_info *const *bp = bp_voidp;
+
+  return ((*ap)->num > (*bp)->num) - ((*ap)->num < (*bp)->num);
+}
+
 /* Apply a GDB command to a list of threads.  List syntax is a whitespace
    seperated list of numbers, or ranges, or the keyword `all'.  Ranges consist
    of two numbers seperated by a hyphen.  Examples:
@@ -1431,6 +1442,8 @@ thread_apply_all_command (char *cmd, int from_tty)
           i++;
         }
 
+      qsort (tp_array, i, sizeof (*tp_array), tp_array_compar);
+
       make_cleanup (set_thread_refcount, &ta_cleanup);
 
       for (k = 0; k != i; k++)

[-- Attachment #3: threadcrash1.c --]
[-- Type: text/plain, Size: 352 bytes --]

#include <pthread.h>
#include <assert.h>
#include <unistd.h>

static void *start (void *arg)
{
  sleep (1);
*(volatile int *)0=0;
  return arg;
}

int main (void)
{
  pthread_t thread1;
  int i;

  i = pthread_create (&thread1, NULL, start, NULL);
  assert (i == 0);
  sleep (100);
  i = pthread_join (thread1, NULL);
  assert (i == 0);

  return 0;
}

[-- Attachment #4: threadcrash2.c --]
[-- Type: text/plain, Size: 352 bytes --]

#include <pthread.h>
#include <assert.h>
#include <unistd.h>

static void *start (void *arg)
{
  sleep (100);
  return arg;
}

int main (void)
{
  pthread_t thread1;
  int i;

  i = pthread_create (&thread1, NULL, start, NULL);
  assert (i == 0);
  sleep (1);
*(volatile int *)0=0;
  i = pthread_join (thread1, NULL);
  assert (i == 0);

  return 0;
}

Re: [patch] Sort threads for thread apply all (bt)

[Doug Evans]

On Thu, Jan 15, 2015 at 10:33 AM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> Hi,
>
> downstream Fedora request:
>         Please make it easier to find the backtrace of the crashing thread
>         https://bugzilla.redhat.com/show_bug.cgi?id=1024504
>
> Currently after loading a core file GDB prints:
>
> Core was generated by `./threadcrash1'.
> Program terminated with signal SIGSEGV, Segmentation fault.
> #0  0x000000000040062f in start (arg=0x0) at threadcrash1.c:8
> 8       *(volatile int *)0=0;
> (gdb) _
>
> there is nowhere seen which of the threads had crashed.  In reality GDB always
> numbers that thread as #1 and it is the current thread that time.  But after
> dumping all the info into a file for later analysis it is no longer obvious.
> 'thread apply all bt' even puts the thread #1 to the _end_ of the output!!!
>
> Should GDB always print after loading a core file what "thread" command would
> print?
> [Current thread is 1 (Thread 0x7fcbe28fe700 (LWP 15453))]

Sounds reasonable to me.
Though there is the concern to not even talk about threads if there are "none".
So maybe only print that if there is more than one thread?

> Or should the "thread" output be printed only in non-interactive mode?
>
> Or something different?
>
> Or is the current behavior OK as is and the tools calling GDB in batch mode
> should indicate the thread #1 on their own?
>
> ------------------------------------------------------------------------------
>
> I find maybe as good enough and with no risk of UI change flamewar to just
> sort the threads by their number.  Currently they are printed as they happen
> in the internal GDB list which has no advantage.  Printing thread #1 as the
> first one with assumed 'thread apply all bt' (after the core file is loaded)
> should make the complaint resolved I guess.
>
> No regressions on {x86_64,x86_64-m32,i686}-fedora22pre-linux-gnu.

No objection to sorting the list, but if thread #1 is the important one,
then a concern could be it'll have scrolled off the screen (such a
concern has been voiced in another thread in another context),
and if not lost (say it's in an emacs buffer) one would still have
to scroll back to see it.
So one *could* still want #1 to be last.
Do we want an option to choose the sort direction?
[I wouldn't make it a global parameter, just an option to
thread apply.]

[patchv2] Sort threads for thread apply all (bt)

[Jan Kratochvil]

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

On Thu, 15 Jan 2015 20:29:07 +0100, Doug Evans wrote:
> On Thu, Jan 15, 2015 at 10:33 AM, Jan Kratochvil <jan.kratochvil@redhat.com> wrote:
> > I find maybe as good enough and with no risk of UI change flamewar to just
> > sort the threads by their number.  Currently they are printed as they happen
> > in the internal GDB list which has no advantage.  Printing thread #1 as the
> > first one with assumed 'thread apply all bt' (after the core file is loaded)
> > should make the complaint resolved I guess.
> >
> > No regressions on {x86_64,x86_64-m32,i686}-fedora22pre-linux-gnu.
> 
> No objection to sorting the list, but if thread #1 is the important one,
> then a concern could be it'll have scrolled off the screen (such a
> concern has been voiced in another thread in another context),
> and if not lost (say it's in an emacs buffer) one would still have
> to scroll back to see it.
> So one *could* still want #1 to be last.
> Do we want an option to choose the sort direction?
> [I wouldn't make it a global parameter, just an option to
> thread apply.]

Done.


Thanks,
Jan

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

gdb/ChangeLog
2015-01-16  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* thread.c (tp_array_compar_asc, tp_array_compar): New.
	(thread_apply_all_command): Parse CMD for tp_array_compar_asc.  Sort
	tp_array using tp_array_compar.
	(_initialize_thread): Extend thread_apply_all_command help.

gdb/doc/ChangeLog
2015-01-16  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.texinfo (Threads): Describe -asc for thread apply all.

diff --git a/gdb/thread.c b/gdb/thread.c
index ed20fbe..9685351 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -1382,6 +1382,20 @@ make_cleanup_restore_current_thread (void)
 			    restore_current_thread_cleanup_dtor);
 }
 
+static int tp_array_compar_asc;
+
+/* Sort an array for struct thread_info pointers by their ascending NUM.  */
+
+static int
+tp_array_compar (const void *ap_voidp, const void *bp_voidp)
+{
+  const struct thread_info *const *ap = ap_voidp;
+  const struct thread_info *const *bp = bp_voidp;
+
+  return ((((*ap)->num > (*bp)->num) - ((*ap)->num < (*bp)->num))
+	  * (tp_array_compar_asc ? +1 : -1));
+}
+
 /* Apply a GDB command to a list of threads.  List syntax is a whitespace
    seperated list of numbers, or ranges, or the keyword `all'.  Ranges consist
    of two numbers seperated by a hyphen.  Examples:
@@ -1398,6 +1412,13 @@ thread_apply_all_command (char *cmd, int from_tty)
   int tc;
   struct thread_array_cleanup ta_cleanup;
 
+  tp_array_compar_asc = 0;
+  if (cmd && (check_for_argument (&cmd, "-asc", strlen ("-asc"))))
+    {
+      cmd = skip_spaces (cmd);
+      tp_array_compar_asc = 1;
+    }
+
   if (cmd == NULL || *cmd == '\000')
     error (_("Please specify a command following the thread ID list"));
 
@@ -1431,6 +1452,8 @@ thread_apply_all_command (char *cmd, int from_tty)
           i++;
         }
 
+      qsort (tp_array, i, sizeof (*tp_array), tp_array_compar);
+
       make_cleanup (set_thread_refcount, &ta_cleanup);
 
       for (k = 0; k != i; k++)
@@ -1739,7 +1762,14 @@ The new thread ID must be currently known."),
 		  &thread_apply_list, "thread apply ", 1, &thread_cmd_list);
 
   add_cmd ("all", class_run, thread_apply_all_command,
-	   _("Apply a command to all threads."), &thread_apply_list);
+	   _("\
+Apply a command to all threads.\n\
+\n\
+Usage: thread apply all [-asc] <command>\n\
+-asc: Call <command> for all threads in ascending order.\n\
+      The default is descending order.\n\
+"),
+	   &thread_apply_list);
 
   add_cmd ("name", class_run, thread_name_command,
 	   _("Set the current thread's name.\n\
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index f413e23..2207ce4 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -2959,14 +2959,17 @@ information on convenience variables.
 
 @kindex thread apply
 @cindex apply command to several threads
-@item thread apply [@var{threadno} | all] @var{command}
+@item thread apply [@var{threadno} | all [-asc]] @var{command}
 The @code{thread apply} command allows you to apply the named
 @var{command} to one or more threads.  Specify the numbers of the
 threads that you want affected with the command argument
 @var{threadno}.  It can be a single thread number, one of the numbers
 shown in the first field of the @samp{info threads} display; or it
-could be a range of thread numbers, as in @code{2-4}.  To apply a
-command to all threads, type @kbd{thread apply all @var{command}}.
+could be a range of thread numbers, as in @code{2-4}.  To apply
+a command to all threads in descending order, type @kbd{thread apply all
+@var{command}}.  To apply a command to all threads in ascending order,
+type @kbd{thread apply all -asc @var{command}}.
+
 
 @kindex thread name
 @cindex name a thread

[patch] Print current thread after loading a core file [Re: [patch] Sort threads for thread apply all (bt)]

[Jan Kratochvil]

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

On Thu, 15 Jan 2015 20:29:07 +0100, Doug Evans wrote:
> On Thu, Jan 15, 2015 at 10:33 AM, Jan Kratochvil <jan.kratochvil@redhat.com> wrote:
> > Should GDB always print after loading a core file what "thread" command would
> > print?
> > [Current thread is 1 (Thread 0x7fcbe28fe700 (LWP 15453))]
> 
> Sounds reasonable to me.
> Though there is the concern to not even talk about threads if there are "none".
> So maybe only print that if there is more than one thread?

Attached.

BTW I think it will print the thread even when loading single/non-threaded
core file when other inferior(s) exist.  But that currently crashes
	[Bug threads/12074] multi-inferior internal error
	https://sourceware.org/bugzilla/show_bug.cgi?id=12074
plus I think that would be a correct behavior anyway.

No regressions on {x86_64,x86_64-m32}-fedora22pre-linux-gnu.


Jan

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

gdb/ChangeLog
2015-01-17  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* corelow.c (core_open): Call also thread_command.
	* gdbthread.h (thread_command): New prototype moved from ...
	* thread.c (thread_command): ... here.
	(thread_command): Make it global.

diff --git a/gdb/corelow.c b/gdb/corelow.c
index a9eadd5..c168d1a 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -456,6 +456,17 @@ core_open (const char *arg, int from_tty)
   /* Now, set up the frame cache, and print the top of stack.  */
   reinit_frame_cache ();
   print_stack_frame (get_selected_frame (NULL), 1, SRC_AND_LOC, 1);
+
+  /* Current thread should be NUM 1 but the user does not know that.  */
+  if (thread_count () >= 2)
+    {
+      TRY_CATCH (except, RETURN_MASK_ERROR)
+	{
+	  thread_command (NULL, from_tty);
+	}
+      if (except.reason < 0)
+	exception_print (gdb_stderr, except);
+    }
 }
 
 static void
diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index 15a979b..a2f378a 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -455,6 +455,8 @@ extern void finish_thread_state_cleanup (void *ptid_p);
 /* Commands with a prefix of `thread'.  */
 extern struct cmd_list_element *thread_cmd_list;
 
+extern void thread_command (char *tidstr, int from_tty);
+
 /* Print notices on thread events (attach, detach, etc.), set with
    `set print thread-events'.  */
 extern int print_thread_events;
diff --git a/gdb/thread.c b/gdb/thread.c
index ed20fbe..4bce212 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -62,7 +62,6 @@ static int highest_thread_num;
    spawned new threads we haven't heard of yet.  */
 static int threads_executing;
 
-static void thread_command (char *tidstr, int from_tty);
 static void thread_apply_all_command (char *, int);
 static int thread_alive (struct thread_info *);
 static void info_threads_command (char *, int);
@@ -1506,7 +1505,7 @@ thread_apply_command (char *tidlist, int from_tty)
 /* Switch to the specified thread.  Will dispatch off to thread_apply_command
    if prefix of arg is `apply'.  */
 
-static void
+void
 thread_command (char *tidstr, int from_tty)
 {
   if (!tidstr)

Re: [patchv2] Sort threads for thread apply all (bt)

[Doug Evans]

Jan Kratochvil writes:
 > On Thu, 15 Jan 2015 20:29:07 +0100, Doug Evans wrote:
 > > On Thu, Jan 15, 2015 at 10:33 AM, Jan Kratochvil <jan.kratochvil@redhat.com> wrote:
 > > > I find maybe as good enough and with no risk of UI change flamewar to just
 > > > sort the threads by their number.  Currently they are printed as they happen
 > > > in the internal GDB list which has no advantage.  Printing thread #1 as the
 > > > first one with assumed 'thread apply all bt' (after the core file is loaded)
 > > > should make the complaint resolved I guess.
 > > >
 > > > No regressions on {x86_64,x86_64-m32,i686}-fedora22pre-linux-gnu.
 > > 
 > > No objection to sorting the list, but if thread #1 is the important one,
 > > then a concern could be it'll have scrolled off the screen (such a
 > > concern has been voiced in another thread in another context),
 > > and if not lost (say it's in an emacs buffer) one would still have
 > > to scroll back to see it.
 > > So one *could* still want #1 to be last.
 > > Do we want an option to choose the sort direction?
 > > [I wouldn't make it a global parameter, just an option to
 > > thread apply.]
 > 
 > Done.
 > 
 > 
 > Thanks,
 > Jan
 > gdb/ChangeLog
 > 2015-01-16  Jan Kratochvil  <jan.kratochvil@redhat.com>
 > 
 > 	* thread.c (tp_array_compar_asc, tp_array_compar): New.
 > 	(thread_apply_all_command): Parse CMD for tp_array_compar_asc.  Sort
 > 	tp_array using tp_array_compar.
 > 	(_initialize_thread): Extend thread_apply_all_command help.
 > 
 > gdb/doc/ChangeLog
 > 2015-01-16  Jan Kratochvil  <jan.kratochvil@redhat.com>
 > 
 > 	* gdb.texinfo (Threads): Describe -asc for thread apply all.

Hi.
Just some nits, no need to resubmit for review.

 > 
 > diff --git a/gdb/thread.c b/gdb/thread.c
 > index ed20fbe..9685351 100644
 > --- a/gdb/thread.c
 > +++ b/gdb/thread.c
 > @@ -1382,6 +1382,20 @@ make_cleanup_restore_current_thread (void)
 >  			    restore_current_thread_cleanup_dtor);
 >  }
 >  
 > +static int tp_array_compar_asc;

This should probably have a comment.

 > +
 > +/* Sort an array for struct thread_info pointers by their ascending NUM.  */
 > +
 > +static int
 > +tp_array_compar (const void *ap_voidp, const void *bp_voidp)
 > +{
 > +  const struct thread_info *const *ap = ap_voidp;
 > +  const struct thread_info *const *bp = bp_voidp;
 > +
 > +  return ((((*ap)->num > (*bp)->num) - ((*ap)->num < (*bp)->num))
 > +	  * (tp_array_compar_asc ? +1 : -1));
 > +}

This triggers my "passing parameters as global variables" alarm,
and while one could instead have two different functions,
this is ok, at least for now.

 > +
 >  /* Apply a GDB command to a list of threads.  List syntax is a whitespace
 >     seperated list of numbers, or ranges, or the keyword `all'.  Ranges consist
 >     of two numbers seperated by a hyphen.  Examples:
 > @@ -1398,6 +1412,13 @@ thread_apply_all_command (char *cmd, int from_tty)
 >    int tc;
 >    struct thread_array_cleanup ta_cleanup;
 >  
 > +  tp_array_compar_asc = 0;
 > +  if (cmd && (check_for_argument (&cmd, "-asc", strlen ("-asc"))))

cmd != NULL

 > +    {
 > +      cmd = skip_spaces (cmd);
 > +      tp_array_compar_asc = 1;
 > +    }
 > +
 >    if (cmd == NULL || *cmd == '\000')
 >      error (_("Please specify a command following the thread ID list"));
 >  
 > @@ -1431,6 +1452,8 @@ thread_apply_all_command (char *cmd, int from_tty)
 >            i++;
 >          }
 >  
 > +      qsort (tp_array, i, sizeof (*tp_array), tp_array_compar);
 > +
 >        make_cleanup (set_thread_refcount, &ta_cleanup);
 >  
 >        for (k = 0; k != i; k++)
 > @@ -1739,7 +1762,14 @@ The new thread ID must be currently known."),
 >  		  &thread_apply_list, "thread apply ", 1, &thread_cmd_list);
 >  
 >    add_cmd ("all", class_run, thread_apply_all_command,
 > -	   _("Apply a command to all threads."), &thread_apply_list);
 > +	   _("\
 > +Apply a command to all threads.\n\
 > +\n\
 > +Usage: thread apply all [-asc] <command>\n\
 > +-asc: Call <command> for all threads in ascending order.\n\
 > +      The default is descending order.\n\

No final trailing newline.

 > +"),
 > +	   &thread_apply_list);
 >  
 >    add_cmd ("name", class_run, thread_name_command,
 >  	   _("Set the current thread's name.\n\
 > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
 > index f413e23..2207ce4 100644
 > --- a/gdb/doc/gdb.texinfo
 > +++ b/gdb/doc/gdb.texinfo
 > @@ -2959,14 +2959,17 @@ information on convenience variables.
 >  
 >  @kindex thread apply
 >  @cindex apply command to several threads
 > -@item thread apply [@var{threadno} | all] @var{command}
 > +@item thread apply [@var{threadno} | all [-asc]] @var{command}
 >  The @code{thread apply} command allows you to apply the named
 >  @var{command} to one or more threads.  Specify the numbers of the
 >  threads that you want affected with the command argument
 >  @var{threadno}.  It can be a single thread number, one of the numbers
 >  shown in the first field of the @samp{info threads} display; or it
 > -could be a range of thread numbers, as in @code{2-4}.  To apply a
 > -command to all threads, type @kbd{thread apply all @var{command}}.
 > +could be a range of thread numbers, as in @code{2-4}.  To apply
 > +a command to all threads in descending order, type @kbd{thread apply all
 > +@var{command}}.  To apply a command to all threads in ascending order,
 > +type @kbd{thread apply all -asc @var{command}}.
 > +
 >  
 >  @kindex thread name
 >  @cindex name a thread

Re: [patch] Print current thread after loading a core file [Re: [patch] Sort threads for thread apply all (bt)]

[Doug Evans]

Jan Kratochvil writes:
 > On Thu, 15 Jan 2015 20:29:07 +0100, Doug Evans wrote:
 > > On Thu, Jan 15, 2015 at 10:33 AM, Jan Kratochvil <jan.kratochvil@redhat.com> wrote:
 > > > Should GDB always print after loading a core file what "thread" command would
 > > > print?
 > > > [Current thread is 1 (Thread 0x7fcbe28fe700 (LWP 15453))]
 > > 
 > > Sounds reasonable to me.
 > > Though there is the concern to not even talk about threads if there are "none".
 > > So maybe only print that if there is more than one thread?
 > 
 > Attached.
 > 
 > BTW I think it will print the thread even when loading single/non-threaded
 > core file when other inferior(s) exist.  But that currently crashes
 > 	[Bug threads/12074] multi-inferior internal error
 > 	https://sourceware.org/bugzilla/show_bug.cgi?id=12074
 > plus I think that would be a correct behavior anyway.
 > 
 > No regressions on {x86_64,x86_64-m32}-fedora22pre-linux-gnu.
 > 
 > 
 > Jan
 > gdb/ChangeLog
 > 2015-01-17  Jan Kratochvil  <jan.kratochvil@redhat.com>
 > 
 > 	* corelow.c (core_open): Call also thread_command.
 > 	* gdbthread.h (thread_command): New prototype moved from ...
 > 	* thread.c (thread_command): ... here.
 > 	(thread_command): Make it global.

Hi.
LGTM with one nit.

 > 
 > diff --git a/gdb/corelow.c b/gdb/corelow.c
 > index a9eadd5..c168d1a 100644
 > --- a/gdb/corelow.c
 > +++ b/gdb/corelow.c
 > @@ -456,6 +456,17 @@ core_open (const char *arg, int from_tty)
 >    /* Now, set up the frame cache, and print the top of stack.  */
 >    reinit_frame_cache ();
 >    print_stack_frame (get_selected_frame (NULL), 1, SRC_AND_LOC, 1);
 > +
 > +  /* Current thread should be NUM 1 but the user does not know that.  */
 > +  if (thread_count () >= 2)

Can you add a comment explaining why the test is >= 2 here?
E.g., something like

  /* Current thread should be NUM 1 but the user does not know that.
     If a program is single threaded gdb in general does not mention
     anything about threads.  That is why the test is >= 2.  */

 > +    {
 > +      TRY_CATCH (except, RETURN_MASK_ERROR)
 > +	{
 > +	  thread_command (NULL, from_tty);
 > +	}
 > +      if (except.reason < 0)
 > +	exception_print (gdb_stderr, except);
 > +    }
 >  }
 >  
 >  static void
 > diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
 > index 15a979b..a2f378a 100644
 > --- a/gdb/gdbthread.h
 > +++ b/gdb/gdbthread.h
 > @@ -455,6 +455,8 @@ extern void finish_thread_state_cleanup (void *ptid_p);
 >  /* Commands with a prefix of `thread'.  */
 >  extern struct cmd_list_element *thread_cmd_list;
 >  
 > +extern void thread_command (char *tidstr, int from_tty);
 > +
 >  /* Print notices on thread events (attach, detach, etc.), set with
 >     `set print thread-events'.  */
 >  extern int print_thread_events;
 > diff --git a/gdb/thread.c b/gdb/thread.c
 > index ed20fbe..4bce212 100644
 > --- a/gdb/thread.c
 > +++ b/gdb/thread.c
 > @@ -62,7 +62,6 @@ static int highest_thread_num;
 >     spawned new threads we haven't heard of yet.  */
 >  static int threads_executing;
 >  
 > -static void thread_command (char *tidstr, int from_tty);
 >  static void thread_apply_all_command (char *, int);
 >  static int thread_alive (struct thread_info *);
 >  static void info_threads_command (char *, int);
 > @@ -1506,7 +1505,7 @@ thread_apply_command (char *tidlist, int from_tty)
 >  /* Switch to the specified thread.  Will dispatch off to thread_apply_command
 >     if prefix of arg is `apply'.  */
 >  
 > -static void
 > +void
 >  thread_command (char *tidstr, int from_tty)
 >  {
 >    if (!tidstr)

Re: [patchv2] Sort threads for thread apply all (bt)

[Pedro Alves]

With http://sourceware.org/bugzilla/show_bug.cgi?id=16445 in
mind, should we add a counterpart option to explicitly force
descending order at the same time, so we're a bit more free
to change the default order at some point?

Thanks,
Pedro Alves

Re: [patchv2] Sort threads for thread apply all (bt)

[Jan Kratochvil]

On Thu, 22 Jan 2015 12:18:27 +0100, Pedro Alves wrote:
> With http://sourceware.org/bugzilla/show_bug.cgi?id=16445 in
> mind, should we add a counterpart option to explicitly force
> descending order at the same time, so we're a bit more free
> to change the default order at some point?

Isn't the PR # a typo?  I do not see how 16445 could be related.


Jan

Re: [patchv2] Sort threads for thread apply all (bt)

[Pedro Alves]

On 01/22/2015 04:43 PM, Jan Kratochvil wrote:
> On Thu, 22 Jan 2015 12:18:27 +0100, Pedro Alves wrote:
>> With http://sourceware.org/bugzilla/show_bug.cgi?id=16445 in
>> mind, should we add a counterpart option to explicitly force
>> descending order at the same time, so we're a bit more free
>> to change the default order at some point?
> 
> Isn't the PR # a typo?  I do not see how 16445 could be related.

It is, sorry.  Here's the right one:

 https://sourceware.org/bugzilla/show_bug.cgi?id=17539

My Firefox has this annoying bug where sometimes the url
line edit contents get stale...  I follow links, paste a new
url, or open new tabs, and the visible url doesn't
change...  sigh.

Thanks,
Pedro Alves

Re: [patchv2] Sort threads for thread apply all (bt)

[Jan Kratochvil]

On Thu, 22 Jan 2015 18:07:33 +0100, Pedro Alves wrote:
>  https://sourceware.org/bugzilla/show_bug.cgi?id=17539

While we're getting to the classical bikeshed before coding in the '-desc'
option I would like to reassure we should not go the global setting way
instead which I would find more appropriate myself in such situation, contrary
to what Doug said:

On Thu, 15 Jan 2015 20:29:07 +0100, Doug Evans wrote:
# Do we want an option to choose the sort direction?
# [I wouldn't make it a global parameter, just an option to
# thread apply.]


> My Firefox has this annoying bug where sometimes the url
> line edit contents get stale...  I follow links, paste a new
> url, or open new tabs, and the visible url doesn't
> change...  sigh.

BTW I always type 'escape ctrl-l ctrl-c' instead of just 'ctrl-l ctrl-c'.


Jan

Re: [patchv2] Sort threads for thread apply all (bt)

[Doug Evans]

On Thu, Jan 22, 2015 at 9:17 AM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> On Thu, 22 Jan 2015 18:07:33 +0100, Pedro Alves wrote:
>>  https://sourceware.org/bugzilla/show_bug.cgi?id=17539
>
> While we're getting to the classical bikeshed before coding in the '-desc'
> option I would like to reassure we should not go the global setting way
> instead which I would find more appropriate myself in such situation, contrary
> to what Doug said:
>
> On Thu, 15 Jan 2015 20:29:07 +0100, Doug Evans wrote:
> # Do we want an option to choose the sort direction?
> # [I wouldn't make it a global parameter, just an option to
> # thread apply.]

I'd rather not add to the global state until there's a justified reason for it.
We're not there yet IMO.

And we can add -desc later.
[Though -asc vs -desc is a bit awkward.
-a and -d?
-ascending and -descending ?]

So I'm happy with the patch as is.
[Which isn't to say I wouldn't be happy with
something different though.]

Re: [patchv2] Sort threads for thread apply all (bt)

[Jan Kratochvil]

On Thu, 22 Jan 2015 19:07:37 +0100, Doug Evans wrote:
> I'd rather not add to the global state until there's a justified reason for it.
> We're not there yet IMO.

OK.


> And we can add -desc later.

I also think so.


> [Though -asc vs -desc is a bit awkward.
> -a and -d?
> -ascending and -descending ?]

I can put there "-ascending".


> So I'm happy with the patch as is.


Jan

[commit] [patch] Print current thread after loading a core file [Re: [patch] Sort threads for thread apply all (bt)]

[Jan Kratochvil]

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

On Thu, 22 Jan 2015 01:36:42 +0100, Doug Evans wrote:
> Jan Kratochvil writes:
>  > +  /* Current thread should be NUM 1 but the user does not know that.  */
>  > +  if (thread_count () >= 2)
> 
> Can you add a comment explaining why the test is >= 2 here?
> E.g., something like
> 
>   /* Current thread should be NUM 1 but the user does not know that.
>      If a program is single threaded gdb in general does not mention
>      anything about threads.  That is why the test is >= 2.  */

Done.


> LGTM with one nit.

Checked in.


Thanks,
Jan

[-- Attachment #2: Type: message/rfc822, Size: 4207 bytes --]

From: Jan Kratochvil <jan.kratochvil@redhat.com>
Subject: [PATCH] Print current thread after loading a core file
Date: Thu, 22 Jan 2015 19:24:44 +0100

downstream Fedora request:
	Please make it easier to find the backtrace of the crashing thread
	https://bugzilla.redhat.com/show_bug.cgi?id=1024504

Currently after loading a core file GDB prints:

Core was generated by `./threadcrash1'.
Program terminated with signal SIGSEGV, Segmentation fault.
8       *(volatile int *)0=0;
(gdb) _

there is nowhere seen which of the threads had crashed.  In reality GDB always
numbers that thread as #1 and it is the current thread that time.  But after
dumping all the info into a file for later analysis it is no longer obvious.
'thread apply all bt' even puts the thread #1 to the _end_ of the output!!!

Should GDB always print after loading a core file what "thread" command would
print?
[Current thread is 1 (Thread 0x7fcbe28fe700 (LWP 15453))]

BTW I think it will print the thread even when loading single/non-threaded
core file when other inferior(s) exist.  But that currently crashes
        [Bug threads/12074] multi-inferior internal error
        https://sourceware.org/bugzilla/show_bug.cgi?id=12074
plus I think that would be a correct behavior anyway.

gdb/ChangeLog
2015-01-22  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* corelow.c (core_open): Call also thread_command.
	* gdbthread.h (thread_command): New prototype moved from ...
	* thread.c (thread_command): ... here.
	(thread_command): Make it global.
---
 gdb/ChangeLog   |  7 +++++++
 gdb/corelow.c   | 13 +++++++++++++
 gdb/gdbthread.h |  2 ++
 gdb/thread.c    |  3 +--
 4 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index a6ba992..dfb9c90 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2015-01-22  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
+	* corelow.c (core_open): Call also thread_command.
+	* gdbthread.h (thread_command): New prototype moved from ...
+	* thread.c (thread_command): ... here.
+	(thread_command): Make it global.
+
 2015-01-22  Eli Zaretskii  <eliz@gnu.org>
 
 	* gdb/tui/tui.c (tui_enable) [__MINGW32__]: If the call to 'newterm'
diff --git a/gdb/corelow.c b/gdb/corelow.c
index a9eadd5..6a67a98 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -456,6 +456,19 @@ core_open (const char *arg, int from_tty)
   /* Now, set up the frame cache, and print the top of stack.  */
   reinit_frame_cache ();
   print_stack_frame (get_selected_frame (NULL), 1, SRC_AND_LOC, 1);
+
+  /* Current thread should be NUM 1 but the user does not know that.
+     If a program is single threaded gdb in general does not mention
+     anything about threads.  That is why the test is >= 2.  */
+  if (thread_count () >= 2)
+    {
+      TRY_CATCH (except, RETURN_MASK_ERROR)
+	{
+	  thread_command (NULL, from_tty);
+	}
+      if (except.reason < 0)
+	exception_print (gdb_stderr, except);
+    }
 }
 
 static void
diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index 15a979b..a2f378a 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -455,6 +455,8 @@ extern void finish_thread_state_cleanup (void *ptid_p);
 /* Commands with a prefix of `thread'.  */
 extern struct cmd_list_element *thread_cmd_list;
 
+extern void thread_command (char *tidstr, int from_tty);
+
 /* Print notices on thread events (attach, detach, etc.), set with
    `set print thread-events'.  */
 extern int print_thread_events;
diff --git a/gdb/thread.c b/gdb/thread.c
index ed20fbe..4bce212 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -62,7 +62,6 @@ static int highest_thread_num;
    spawned new threads we haven't heard of yet.  */
 static int threads_executing;
 
-static void thread_command (char *tidstr, int from_tty);
 static void thread_apply_all_command (char *, int);
 static int thread_alive (struct thread_info *);
 static void info_threads_command (char *, int);
@@ -1506,7 +1505,7 @@ thread_apply_command (char *tidlist, int from_tty)
 /* Switch to the specified thread.  Will dispatch off to thread_apply_command
    if prefix of arg is `apply'.  */
 
-static void
+void
 thread_command (char *tidstr, int from_tty)
 {
   if (!tidstr)
-- 
2.1.0

[patchv3] Sort threads for thread apply all (bt)

[Jan Kratochvil]

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

On Thu, 22 Jan 2015 01:26:46 +0100, Doug Evans wrote:
> Just some nits, no need to resubmit for review.

Resubmitting it as a request for doc approval from Eli.


>  > +static int tp_array_compar_asc;
> 
> This should probably have a comment.

Done, I forgot.


>  > +
>  > +/* Sort an array for struct thread_info pointers by their ascending NUM.  */
>  > +
>  > +static int
>  > +tp_array_compar (const void *ap_voidp, const void *bp_voidp)
[...]
>  > +  return ((((*ap)->num > (*bp)->num) - ((*ap)->num < (*bp)->num))
>  > +	  * (tp_array_compar_asc ? +1 : -1));
>  > +}
> 
> This triggers my "passing parameters as global variables" alarm,
> and while one could instead have two different functions,

I believe qsort_r() is more appropriate, using two different functions I would
feel rather as a workaround of missing qsort_r(). But I guess (I cannot easily
test due to missing slaves and no patch testing feature in Sergio's Buildbot
yet) some of the supported platforms do not provide qsort_r() so it would need
a new gdb/gnulib/ module. But at least one of my gdb/gnulib/ patches is still
under review so it would create dependency between unrelated patchsets.


>  > +  if (cmd && (check_for_argument (&cmd, "-asc", strlen ("-asc"))))
> 
> cmd != NULL

BTW this has been copy-paste but yes, my fault, I am aware of it.


Thanks,
Jan

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

gdb/ChangeLog
2015-01-16  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* thread.c (tp_array_compar_ascending, tp_array_compar): New.
	(thread_apply_all_command): Parse CMD for tp_array_compar_ascending.
	Sort tp_array using tp_array_compar.
	(_initialize_thread): Extend thread_apply_all_command help.

gdb/doc/ChangeLog
2015-01-16  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.texinfo (Threads): Describe -ascending for thread apply all.

diff --git a/gdb/thread.c b/gdb/thread.c
index 4bce212..870a956 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -1381,6 +1381,24 @@ make_cleanup_restore_current_thread (void)
 			    restore_current_thread_cleanup_dtor);
 }
 
+/* If non-zero tp_array_compar should sort in ascending order, otherwise in
+   descending order.  */
+
+static int tp_array_compar_ascending;
+
+/* Sort an array for struct thread_info pointers by their NUM, order is
+   determined by TP_ARRAY_COMPAR_ASCENDING.  */
+
+static int
+tp_array_compar (const void *ap_voidp, const void *bp_voidp)
+{
+  const struct thread_info *const *ap = ap_voidp;
+  const struct thread_info *const *bp = bp_voidp;
+
+  return ((((*ap)->num > (*bp)->num) - ((*ap)->num < (*bp)->num))
+	  * (tp_array_compar_ascending ? +1 : -1));
+}
+
 /* Apply a GDB command to a list of threads.  List syntax is a whitespace
    seperated list of numbers, or ranges, or the keyword `all'.  Ranges consist
    of two numbers seperated by a hyphen.  Examples:
@@ -1397,6 +1415,13 @@ thread_apply_all_command (char *cmd, int from_tty)
   int tc;
   struct thread_array_cleanup ta_cleanup;
 
+  tp_array_compar_ascending = 0;
+  if (cmd != NULL && (check_for_argument (&cmd, "-ascending", strlen ("-asc"))))
+    {
+      cmd = skip_spaces (cmd);
+      tp_array_compar_ascending = 1;
+    }
+
   if (cmd == NULL || *cmd == '\000')
     error (_("Please specify a command following the thread ID list"));
 
@@ -1430,6 +1455,8 @@ thread_apply_all_command (char *cmd, int from_tty)
           i++;
         }
 
+      qsort (tp_array, i, sizeof (*tp_array), tp_array_compar);
+
       make_cleanup (set_thread_refcount, &ta_cleanup);
 
       for (k = 0; k != i; k++)
@@ -1738,7 +1765,14 @@ The new thread ID must be currently known."),
 		  &thread_apply_list, "thread apply ", 1, &thread_cmd_list);
 
   add_cmd ("all", class_run, thread_apply_all_command,
-	   _("Apply a command to all threads."), &thread_apply_list);
+	   _("\
+Apply a command to all threads.\n\
+\n\
+Usage: thread apply all [-ascending] <command>\n\
+-ascending: Call <command> for all threads in ascending order.\n\
+            The default is descending order.\
+"),
+	   &thread_apply_list);
 
   add_cmd ("name", class_run, thread_name_command,
 	   _("Set the current thread's name.\n\
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index f413e23..277df25 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -2959,14 +2959,17 @@ information on convenience variables.
 
 @kindex thread apply
 @cindex apply command to several threads
-@item thread apply [@var{threadno} | all] @var{command}
+@item thread apply [@var{threadno} | all [-ascending]] @var{command}
 The @code{thread apply} command allows you to apply the named
 @var{command} to one or more threads.  Specify the numbers of the
 threads that you want affected with the command argument
 @var{threadno}.  It can be a single thread number, one of the numbers
 shown in the first field of the @samp{info threads} display; or it
-could be a range of thread numbers, as in @code{2-4}.  To apply a
-command to all threads, type @kbd{thread apply all @var{command}}.
+could be a range of thread numbers, as in @code{2-4}.  To apply
+a command to all threads in descending order, type @kbd{thread apply all
+@var{command}}.  To apply a command to all threads in ascending order,
+type @kbd{thread apply all -ascending @var{command}}.
+
 
 @kindex thread name
 @cindex name a thread

Re: [patchv3] Sort threads for thread apply all (bt)

[Eli Zaretskii]

> Date: Thu, 22 Jan 2015 19:46:55 +0100
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> Cc: gdb-patches <gdb-patches@sourceware.org>, Doug Evans <dje@google.com>
> 
> On Thu, 22 Jan 2015 01:26:46 +0100, Doug Evans wrote:
> > Just some nits, no need to resubmit for review.
> 
> Resubmitting it as a request for doc approval from Eli.

Sorry for missing that.  The documentation parts are OK.

(Do we want a NEWS entry?)

Re: [patchv3] Sort threads for thread apply all (bt)

[Doug Evans]

On Thu, Jan 22, 2015 at 10:46 AM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> ...
>> This triggers my "passing parameters as global variables" alarm,
>> and while one could instead have two different functions,
>
> I believe qsort_r() is more appropriate, using two different functions I would
> feel rather as a workaround of missing qsort_r(). But I guess (I cannot easily
> test due to missing slaves and no patch testing feature in Sergio's Buildbot
> yet) some of the supported platforms do not provide qsort_r() so it would need
> a new gdb/gnulib/ module. But at least one of my gdb/gnulib/ patches is still
> under review so it would create dependency between unrelated patchsets.

I think you're splitting hairs here.

> +  if (cmd != NULL && (check_for_argument (&cmd, "-ascending", strlen ("-asc"))))

strlen ("-ascending")

Also, remove the extra parens around check_for_argument?

Re: [patchv3] Sort threads for thread apply all (bt)

[Doug Evans]

On Thu, Jan 22, 2015 at 10:46 AM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> ...
>> This triggers my "passing parameters as global variables" alarm,
>> and while one could instead have two different functions,
>
> I believe qsort_r() is more appropriate, using two different functions I would
> feel rather as a workaround of missing qsort_r(). But I guess (I cannot easily
> test due to missing slaves and no patch testing feature in Sergio's Buildbot
> yet) some of the supported platforms do not provide qsort_r() so it would need
> a new gdb/gnulib/ module. But at least one of my gdb/gnulib/ patches is still
> under review so it would create dependency between unrelated patchsets.

I think you're splitting hairs here.

> +  if (cmd != NULL && (check_for_argument (&cmd, "-ascending", strlen ("-asc"))))

strlen ("-ascending")

Also, remove the extra parens around check_for_argument?

[patchv5] Sort threads for thread apply all (bt)

[Jan Kratochvil]

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

On Thu, 22 Jan 2015 19:59:55 +0100, Doug Evans wrote:
> I think you're splitting hairs here.
> 
> > +  if (cmd != NULL && (check_for_argument (&cmd, "-ascending", strlen ("-asc"))))
> 
> strlen ("-ascending")

OK, thanks.  GDB still cannot use starts_with().


> Also, remove the extra parens around check_for_argument?

Yes.


Jan

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

gdb/ChangeLog
2015-01-16  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* NEWS (Changes since GDB 7.9): Add 'thread apply all' option
	'-ascending'.
	* thread.c (tp_array_compar_ascending, tp_array_compar): New.
	(thread_apply_all_command): Parse CMD for tp_array_compar_ascending.
	Sort tp_array using tp_array_compar.
	(_initialize_thread): Extend thread_apply_all_command help.

gdb/doc/ChangeLog
2015-01-16  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.texinfo (Threads): Describe -ascending for thread apply all.

diff --git a/gdb/NEWS b/gdb/NEWS
index 2d2c941..c0cf706 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -3,6 +3,11 @@
 
 *** Changes since GDB 7.9
 
+* New options
+
+* The command 'thread apply all' can now support new option '-ascending'
+  to call its specified command for all threads in ascending order.
+
 *** Changes in GDB 7.9
 
 * GDB now supports hardware watchpoints on x86 GNU Hurd.
diff --git a/gdb/thread.c b/gdb/thread.c
index 4bce212..a2a5a9b 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -1381,6 +1381,24 @@ make_cleanup_restore_current_thread (void)
 			    restore_current_thread_cleanup_dtor);
 }
 
+/* If non-zero tp_array_compar should sort in ascending order, otherwise in
+   descending order.  */
+
+static int tp_array_compar_ascending;
+
+/* Sort an array for struct thread_info pointers by their NUM, order is
+   determined by TP_ARRAY_COMPAR_ASCENDING.  */
+
+static int
+tp_array_compar (const void *ap_voidp, const void *bp_voidp)
+{
+  const struct thread_info *const *ap = ap_voidp;
+  const struct thread_info *const *bp = bp_voidp;
+
+  return ((((*ap)->num > (*bp)->num) - ((*ap)->num < (*bp)->num))
+	  * (tp_array_compar_ascending ? +1 : -1));
+}
+
 /* Apply a GDB command to a list of threads.  List syntax is a whitespace
    seperated list of numbers, or ranges, or the keyword `all'.  Ranges consist
    of two numbers seperated by a hyphen.  Examples:
@@ -1397,6 +1415,14 @@ thread_apply_all_command (char *cmd, int from_tty)
   int tc;
   struct thread_array_cleanup ta_cleanup;
 
+  tp_array_compar_ascending = 0;
+  if (cmd != NULL
+      && check_for_argument (&cmd, "-ascending", strlen ("-ascending")))
+    {
+      cmd = skip_spaces (cmd);
+      tp_array_compar_ascending = 1;
+    }
+
   if (cmd == NULL || *cmd == '\000')
     error (_("Please specify a command following the thread ID list"));
 
@@ -1430,6 +1456,8 @@ thread_apply_all_command (char *cmd, int from_tty)
           i++;
         }
 
+      qsort (tp_array, i, sizeof (*tp_array), tp_array_compar);
+
       make_cleanup (set_thread_refcount, &ta_cleanup);
 
       for (k = 0; k != i; k++)
@@ -1738,7 +1766,14 @@ The new thread ID must be currently known."),
 		  &thread_apply_list, "thread apply ", 1, &thread_cmd_list);
 
   add_cmd ("all", class_run, thread_apply_all_command,
-	   _("Apply a command to all threads."), &thread_apply_list);
+	   _("\
+Apply a command to all threads.\n\
+\n\
+Usage: thread apply all [-ascending] <command>\n\
+-ascending: Call <command> for all threads in ascending order.\n\
+            The default is descending order.\
+"),
+	   &thread_apply_list);
 
   add_cmd ("name", class_run, thread_name_command,
 	   _("Set the current thread's name.\n\
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index f413e23..277df25 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -2959,14 +2959,17 @@ information on convenience variables.
 
 @kindex thread apply
 @cindex apply command to several threads
-@item thread apply [@var{threadno} | all] @var{command}
+@item thread apply [@var{threadno} | all [-ascending]] @var{command}
 The @code{thread apply} command allows you to apply the named
 @var{command} to one or more threads.  Specify the numbers of the
 threads that you want affected with the command argument
 @var{threadno}.  It can be a single thread number, one of the numbers
 shown in the first field of the @samp{info threads} display; or it
-could be a range of thread numbers, as in @code{2-4}.  To apply a
-command to all threads, type @kbd{thread apply all @var{command}}.
+could be a range of thread numbers, as in @code{2-4}.  To apply
+a command to all threads in descending order, type @kbd{thread apply all
+@var{command}}.  To apply a command to all threads in ascending order,
+type @kbd{thread apply all -ascending @var{command}}.
+
 
 @kindex thread name
 @cindex name a thread

[patchv4] Sort threads for thread apply all (bt)

[Jan Kratochvil]

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

On Thu, 22 Jan 2015 19:52:29 +0100, Eli Zaretskii wrote:
> (Do we want a NEWS entry?)

OK, I think so.

I have taken this entry as a template:
------------------------------------------------------------------------------
* The command 'tsave' can now support new option '-ctf' to save trace
  buffer in Common Trace Format.
------------------------------------------------------------------------------


Thanks,
Jan

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

gdb/ChangeLog
2015-01-16  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* NEWS (Changes since GDB 7.9): Add 'thread apply all' option
	'-ascending'.
	* thread.c (tp_array_compar_ascending, tp_array_compar): New.
	(thread_apply_all_command): Parse CMD for tp_array_compar_ascending.
	Sort tp_array using tp_array_compar.
	(_initialize_thread): Extend thread_apply_all_command help.

gdb/doc/ChangeLog
2015-01-16  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.texinfo (Threads): Describe -ascending for thread apply all.

diff --git a/gdb/NEWS b/gdb/NEWS
index 2d2c941..c0cf706 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -3,6 +3,11 @@
 
 *** Changes since GDB 7.9
 
+* New options
+
+* The command 'thread apply all' can now support new option '-ascending'
+  to call its specified command for all threads in ascending order.
+
 *** Changes in GDB 7.9
 
 * GDB now supports hardware watchpoints on x86 GNU Hurd.
diff --git a/gdb/thread.c b/gdb/thread.c
index 4bce212..870a956 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -1381,6 +1381,24 @@ make_cleanup_restore_current_thread (void)
 			    restore_current_thread_cleanup_dtor);
 }
 
+/* If non-zero tp_array_compar should sort in ascending order, otherwise in
+   descending order.  */
+
+static int tp_array_compar_ascending;
+
+/* Sort an array for struct thread_info pointers by their NUM, order is
+   determined by TP_ARRAY_COMPAR_ASCENDING.  */
+
+static int
+tp_array_compar (const void *ap_voidp, const void *bp_voidp)
+{
+  const struct thread_info *const *ap = ap_voidp;
+  const struct thread_info *const *bp = bp_voidp;
+
+  return ((((*ap)->num > (*bp)->num) - ((*ap)->num < (*bp)->num))
+	  * (tp_array_compar_ascending ? +1 : -1));
+}
+
 /* Apply a GDB command to a list of threads.  List syntax is a whitespace
    seperated list of numbers, or ranges, or the keyword `all'.  Ranges consist
    of two numbers seperated by a hyphen.  Examples:
@@ -1397,6 +1415,13 @@ thread_apply_all_command (char *cmd, int from_tty)
   int tc;
   struct thread_array_cleanup ta_cleanup;
 
+  tp_array_compar_ascending = 0;
+  if (cmd != NULL && (check_for_argument (&cmd, "-ascending", strlen ("-asc"))))
+    {
+      cmd = skip_spaces (cmd);
+      tp_array_compar_ascending = 1;
+    }
+
   if (cmd == NULL || *cmd == '\000')
     error (_("Please specify a command following the thread ID list"));
 
@@ -1430,6 +1455,8 @@ thread_apply_all_command (char *cmd, int from_tty)
           i++;
         }
 
+      qsort (tp_array, i, sizeof (*tp_array), tp_array_compar);
+
       make_cleanup (set_thread_refcount, &ta_cleanup);
 
       for (k = 0; k != i; k++)
@@ -1738,7 +1765,14 @@ The new thread ID must be currently known."),
 		  &thread_apply_list, "thread apply ", 1, &thread_cmd_list);
 
   add_cmd ("all", class_run, thread_apply_all_command,
-	   _("Apply a command to all threads."), &thread_apply_list);
+	   _("\
+Apply a command to all threads.\n\
+\n\
+Usage: thread apply all [-ascending] <command>\n\
+-ascending: Call <command> for all threads in ascending order.\n\
+            The default is descending order.\
+"),
+	   &thread_apply_list);
 
   add_cmd ("name", class_run, thread_name_command,
 	   _("Set the current thread's name.\n\
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index f413e23..277df25 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -2959,14 +2959,17 @@ information on convenience variables.
 
 @kindex thread apply
 @cindex apply command to several threads
-@item thread apply [@var{threadno} | all] @var{command}
+@item thread apply [@var{threadno} | all [-ascending]] @var{command}
 The @code{thread apply} command allows you to apply the named
 @var{command} to one or more threads.  Specify the numbers of the
 threads that you want affected with the command argument
 @var{threadno}.  It can be a single thread number, one of the numbers
 shown in the first field of the @samp{info threads} display; or it
-could be a range of thread numbers, as in @code{2-4}.  To apply a
-command to all threads, type @kbd{thread apply all @var{command}}.
+could be a range of thread numbers, as in @code{2-4}.  To apply
+a command to all threads in descending order, type @kbd{thread apply all
+@var{command}}.  To apply a command to all threads in ascending order,
+type @kbd{thread apply all -ascending @var{command}}.
+
 
 @kindex thread name
 @cindex name a thread

Re: [patchv4] Sort threads for thread apply all (bt)

[Doug Evans]

On Thu, Jan 22, 2015 at 11:01 AM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> On Thu, 22 Jan 2015 19:52:29 +0100, Eli Zaretskii wrote:
>> (Do we want a NEWS entry?)
>
> OK, I think so.
>
> I have taken this entry as a template:
> ------------------------------------------------------------------------------
> * The command 'tsave' can now support new option '-ctf' to save trace
>   buffer in Common Trace Format.
> ------------------------------------------------------------------------------
>
>
> Thanks,
> Jan
>
> gdb/ChangeLog
> 2015-01-16  Jan Kratochvil  <jan.kratochvil@redhat.com>
>
>         * NEWS (Changes since GDB 7.9): Add 'thread apply all' option
>         '-ascending'.
>         * thread.c (tp_array_compar_ascending, tp_array_compar): New.
>         (thread_apply_all_command): Parse CMD for tp_array_compar_ascending.
>         Sort tp_array using tp_array_compar.
>         (_initialize_thread): Extend thread_apply_all_command help.

Code parts LGTM.
Thanks.

Re: [patchv4] Sort threads for thread apply all (bt)

[Doug Evans]

On Thu, Jan 22, 2015 at 11:01 AM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> On Thu, 22 Jan 2015 19:52:29 +0100, Eli Zaretskii wrote:
>> (Do we want a NEWS entry?)
>
> OK, I think so.
>
> I have taken this entry as a template:
> ------------------------------------------------------------------------------
> * The command 'tsave' can now support new option '-ctf' to save trace
>   buffer in Common Trace Format.
> ------------------------------------------------------------------------------
>
>
> Thanks,
> Jan
>
> gdb/ChangeLog
> 2015-01-16  Jan Kratochvil  <jan.kratochvil@redhat.com>
>
>         * NEWS (Changes since GDB 7.9): Add 'thread apply all' option
>         '-ascending'.
>         * thread.c (tp_array_compar_ascending, tp_array_compar): New.
>         (thread_apply_all_command): Parse CMD for tp_array_compar_ascending.
>         Sort tp_array using tp_array_compar.
>         (_initialize_thread): Extend thread_apply_all_command help.

Code parts LGTM.
Thanks.

Re: [patchv3] Sort threads for thread apply all (bt)

[Doug Evans]

On Thu, Jan 22, 2015 at 10:46 AM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> ...
>> This triggers my "passing parameters as global variables" alarm,
>> and while one could instead have two different functions,
>
> I believe qsort_r() is more appropriate, using two different functions I would
> feel rather as a workaround of missing qsort_r(). But I guess (I cannot easily
> test due to missing slaves and no patch testing feature in Sergio's Buildbot
> yet) some of the supported platforms do not provide qsort_r() so it would need
> a new gdb/gnulib/ module. But at least one of my gdb/gnulib/ patches is still
> under review so it would create dependency between unrelated patchsets.

I think you're splitting hairs here.

> +  if (cmd != NULL && (check_for_argument (&cmd, "-ascending", strlen ("-asc"))))

strlen ("-ascending")

Also, remove the extra parens around check_for_argument?

Re: [patchv4] Sort threads for thread apply all (bt)

[Eli Zaretskii]

> Date: Thu, 22 Jan 2015 20:01:41 +0100
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> Cc: gdb-patches@sourceware.org, dje@google.com
> 
> On Thu, 22 Jan 2015 19:52:29 +0100, Eli Zaretskii wrote:
> > (Do we want a NEWS entry?)
> 
> OK, I think so.
> 
> I have taken this entry as a template:
> ------------------------------------------------------------------------------
> * The command 'tsave' can now support new option '-ctf' to save trace
>   buffer in Common Trace Format.
> ------------------------------------------------------------------------------

LGTM, thanks.

[commit] [patchv5] Sort threads for thread apply all (bt)

[Jan Kratochvil]

On Thu, 22 Jan 2015 20:17:01 +0100, Doug Evans wrote:
> Code parts LGTM.
> Thanks.

On Thu, 22 Jan 2015 20:34:26 +0100, Eli Zaretskii wrote:
> LGTM, thanks.

Checked in:
	253828f102691732d014e8f1d62f9b5dc779b39c


Thanks,
Jan