[RFC] thread apply commands change selected frame

[RFC] thread apply commands change selected frame

[Denis PILAT]

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

Hi everyone,

I noticed a potential problem in thread.c: the "thread apply" commands 
change the selected frame, I think it should not.
If you are doing
    (gdb) up
    (gdb) thread apply all print 1
    (gdb) down
then the frame selected before the up is not being restored by the down, 
we have an error message instead.

In the following patch:
1- I've added the restoring of the the selected frame in the restoring 
of the current thread. All is done in the 
make_cleanup_restore_current_thread() and 
do_restore_current_thread_cleanup() functions.
I don't know if there could be some cases where we don't want to restore 
the selected frame when having changed the current thread ? If so may be 
we can separate both restoring.
I'm also wondering about getting the selected frame in 
make_cleanup_restore_current_thread() directly without passing it as an 
argument. You're comments are welcome.

2- I  removed the print_stack_frame from the restore_current_thread() 
function, I think it's up to the caller to decide if he need to print or 
not the stack frame?
About that I think I must print the selected frame, not the current. 
What's your opinion ?

3- I saw in info_threads_command() that the selected frame was restored 
because the usage of switch_to_thread() changed it, so I used in 
info_threads_command() the same principle as above, using cleanup 
function. I also removed show_stack_frame() since it does nothing.

I need your opinion before going ahead in this patch.
-- 
Denis


[-- Attachment #2: thread.c.patch --]
[-- Type: text/plain, Size: 5708 bytes --]

Index: thread.c
===================================================================
--- thread.c	(revision 549)
+++ thread.c	(working copy)
@@ -65,6 +65,8 @@ static void thread_apply_command (char *
 static void restore_current_thread (ptid_t);
 static void switch_to_thread (ptid_t ptid);
 static void prune_threads (void);
+static struct cleanup *make_cleanup_restore_current_thread (ptid_t,
+                                                            struct frame_id);
 
 void
 delete_step_resume_breakpoint (void *arg)
@@ -408,9 +410,14 @@ info_threads_command (char *arg, int fro
   struct thread_info *tp;
   ptid_t current_ptid;
   struct frame_info *cur_frame;
-  struct frame_id saved_frame_id = get_frame_id (get_selected_frame (NULL));
+  struct cleanup *old_chain;
+  struct frame_id saved_frame_id;
   char *extra_info;
 
+  /* Backup current thread and selected frame.  */
+  saved_frame_id = get_frame_id (get_selected_frame (NULL));
+  old_chain = make_cleanup_restore_current_thread (inferior_ptid, saved_frame_id);
+
   prune_threads ();
   target_find_new_threads ();
   current_ptid = inferior_ptid;
@@ -427,30 +434,22 @@ info_threads_command (char *arg, int fro
       if (extra_info)
 	printf_filtered (" (%s)", extra_info);
       puts_filtered ("  ");
-
+      /* That switch put us at the top of the stack (leaf frame).  */
       switch_to_thread (tp->ptid);
       print_stack_frame (get_selected_frame (NULL), 0, LOCATION);
     }
 
-  switch_to_thread (current_ptid);
+  /* Restores the current thread and the frame set by the user before
+     the "info threads" command.  */
+  do_cleanups (old_chain);
 
-  /* Restores the frame set by the user before the "info threads"
-     command.  We have finished the info-threads display by switching
-     back to the current thread.  That switch has put us at the top of
-     the stack (leaf frame).  */
-  cur_frame = frame_find_by_id (saved_frame_id);
-  if (cur_frame == NULL)
+  /*  if case we were not abale to find the backup frame, print the 
+      selected frame.  */
+  if (frame_find_by_id (saved_frame_id) == NULL)
     {
-      /* Ooops, can't restore, tell user where we are.  */
       warning (_("Couldn't restore frame in current thread, at frame 0"));
       print_stack_frame (get_selected_frame (NULL), 0, LOCATION);
     }
-  else
-    {
-      select_frame (cur_frame);
-      /* re-show current frame. */
-      show_stack_frame (cur_frame);
-    }
 }
 
 /* Switch from one thread to another. */
@@ -474,13 +473,27 @@ restore_current_thread (ptid_t ptid)
   if (!ptid_equal (ptid, inferior_ptid))
     {
       switch_to_thread (ptid);
-      print_stack_frame (get_current_frame (), 1, SRC_LINE);
+    }
+}
+
+static void
+restore_selected_frame (struct frame_id a_frame_id)
+{
+  struct frame_info *selected_frame_info = NULL;
+
+  if (frame_id_eq (a_frame_id, null_frame_id))
+    return;        
+
+  if ((selected_frame_info = frame_find_by_id (a_frame_id)) != NULL)
+    {
+      select_frame (selected_frame_info);
     }
 }
 
 struct current_thread_cleanup
 {
   ptid_t inferior_ptid;
+  struct frame_id selected_frame_id;
 };
 
 static void
@@ -488,15 +501,18 @@ do_restore_current_thread_cleanup (void 
 {
   struct current_thread_cleanup *old = arg;
   restore_current_thread (old->inferior_ptid);
+  restore_selected_frame (old->selected_frame_id);
   xfree (old);
 }
 
 static struct cleanup *
-make_cleanup_restore_current_thread (ptid_t inferior_ptid)
+make_cleanup_restore_current_thread (ptid_t inferior_ptid, 
+                                     struct frame_id a_frame_id)
 {
   struct current_thread_cleanup *old
     = xmalloc (sizeof (struct current_thread_cleanup));
   old->inferior_ptid = inferior_ptid;
+  old->selected_frame_id = a_frame_id;
   return make_cleanup (do_restore_current_thread_cleanup, old);
 }
 
@@ -516,11 +532,13 @@ thread_apply_all_command (char *cmd, int
   struct cleanup *old_chain;
   struct cleanup *saved_cmd_cleanup_chain;
   char *saved_cmd;
+  struct frame_id saved_frame_id;
 
   if (cmd == NULL || *cmd == '\000')
     error (_("Please specify a command following the thread ID list"));
-
-  old_chain = make_cleanup_restore_current_thread (inferior_ptid);
+  
+  saved_frame_id = get_frame_id (get_selected_frame (NULL));
+  old_chain = make_cleanup_restore_current_thread (inferior_ptid, saved_frame_id);
 
   /* It is safe to update the thread list now, before
      traversing it for "thread apply all".  MVS */
@@ -542,6 +560,7 @@ thread_apply_all_command (char *cmd, int
 
   do_cleanups (saved_cmd_cleanup_chain);
   do_cleanups (old_chain);
+  print_stack_frame (get_current_frame (), 1, SRC_LINE);
 }
 
 static void
@@ -552,6 +571,7 @@ thread_apply_command (char *tidlist, int
   struct cleanup *old_chain;
   struct cleanup *saved_cmd_cleanup_chain;
   char *saved_cmd;
+  struct frame_id saved_frame_id;
 
   if (tidlist == NULL || *tidlist == '\000')
     error (_("Please specify a thread ID list"));
@@ -561,7 +581,8 @@ thread_apply_command (char *tidlist, int
   if (*cmd == '\000')
     error (_("Please specify a command following the thread ID list"));
 
-  old_chain = make_cleanup_restore_current_thread (inferior_ptid);
+  saved_frame_id = get_frame_id (get_selected_frame (NULL));
+  old_chain = make_cleanup_restore_current_thread (inferior_ptid, saved_frame_id);
 
   /* Save a copy of the command in case it is clobbered by
      execute_command */
@@ -615,6 +636,7 @@ thread_apply_command (char *tidlist, int
 
   do_cleanups (saved_cmd_cleanup_chain);
   do_cleanups (old_chain);
+  print_stack_frame (get_current_frame (), 1, SRC_LINE);
 }
 
 /* Switch to the specified thread.  Will dispatch off to thread_apply_command

Re: [RFC] thread apply commands change selected frame

[Daniel Jacobowitz]

On Thu, Jan 11, 2007 at 12:24:35PM +0100, Denis PILAT wrote:
> Hi everyone,
> 
> I noticed a potential problem in thread.c: the "thread apply" commands 
> change the selected frame, I think it should not.
> If you are doing
>    (gdb) up
>    (gdb) thread apply all print 1
>    (gdb) down
> then the frame selected before the up is not being restored by the down, 
> we have an error message instead.

Yeah, that's a strange behavior.

> I need your opinion before going ahead in this patch.

It looks right to me.  With a ChangeLog entry and testsuite results
from some target with threads, it would be OK to commit.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFC] thread apply commands change selected frame

[Denis PILAT]

Daniel Jacobowitz wrote:
> On Thu, Jan 11, 2007 at 12:24:35PM +0100, Denis PILAT wrote:
>> Hi everyone,
>>
>> I noticed a potential problem in thread.c: the "thread apply" commands 
>> change the selected frame, I think it should not.
>> If you are doing
>>    (gdb) up
>>    (gdb) thread apply all print 1
>>    (gdb) down
>> then the frame selected before the up is not being restored by the down, 
>> we have an error message instead.
>
> Yeah, that's a strange behavior.
>
>> I need your opinion before going ahead in this patch.
>
> It looks right to me.  With a ChangeLog entry and testsuite results
> from some target with threads, it would be OK to commit.
>
There is one regression in the last test of gdb.threads/threadapply.exp 
since this test does not expect to get the stack frame. With my patch 
the current stack frame is alway printed at the end of a thread apply 
command.
I also changed the semantic since I remove the print_stack_frame from 
the cleanup command so from the error treatment.

I don't catch the original behaviour: it seems to print the stack frame 
only if the last thread we switched to (in the loop) is not the one we 
were before. In that case the user see that because he is in thread 2 
before typing the command:

(gdb) thread apply all p 1
[New Thread 1077955504 (LWP 27106)]
Thread 3 (Thread 1077955504 (LWP 27106)):
$1 = 1
Thread 2 (Thread 1075854256 (LWP 27105)):
$2 = 1
Thread 1 (Thread 1073748160 (LWP 27045)):
$3 = 1
12                      fputc ('x', stderr);

This last line is not printed if current thread is 1 before typing the 
command, that means 3 last line are always inconsistent.

What behavior do we want in normal case (we retrieve the original 
thread) and in case where we can't since the thread has died for instance ?

-- 
Denis

Re: [RFC] thread apply commands change selected frame

[Denis PILAT]

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


>>
>> It looks right to me.  With a ChangeLog entry and testsuite results
>> from some target with threads, it would be OK to commit.
>>
>
I propose a new implementation that does not change the old behavior of 
gdb with respect to the printing of the last frame in case we changed of 
thread during "thread apply" command execution.

This implementation does not make any regression in the testsuite for a 
i386-linux target.
To me the part that prints the stack frame at the end of execution of 
"thread apply" could be entirely removed but this will imply some 
changes in the testsuite. I can do that it's up to you.

About the bug initially fixed by this patch, I include some new tests in 
threadapply.exp.

-- 
Denis



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

2007-01-30  Denis Pilat  <denis.pilat@st.com>

	* thread.c (make_cleanup_restore_current_thread): New function.
	(info_threads_command): Use of make_cleanup_restore_current_thread
	to restore the current thread and the selected frame.
	(restore_selected_frame): New function.
	(struct current_thread_cleanup): Add frame_id field.
	(do_restore_current_thread_cleanup): Add restoring of the selected
	frame.
	(make_cleanup_restore_current_thread): Likewise.
	(thread_apply_all_command): backup the selected frame while 
	entering the function and restore it at exit.
	(thread_apply_command): Likewise.

Index: thread.c
===================================================================
--- thread.c	(revision 553)
+++ thread.c	(working copy)
@@ -65,6 +65,8 @@ static void thread_apply_command (char *
 static void restore_current_thread (ptid_t);
 static void switch_to_thread (ptid_t ptid);
 static void prune_threads (void);
+static struct cleanup *make_cleanup_restore_current_thread (ptid_t,
+                                                            struct frame_id);
 
 void
 delete_step_resume_breakpoint (void *arg)
@@ -408,9 +410,14 @@ info_threads_command (char *arg, int fro
   struct thread_info *tp;
   ptid_t current_ptid;
   struct frame_info *cur_frame;
-  struct frame_id saved_frame_id = get_frame_id (get_selected_frame (NULL));
+  struct cleanup *old_chain;
+  struct frame_id saved_frame_id;
   char *extra_info;
 
+  /* Backup current thread and selected frame.  */
+  saved_frame_id = get_frame_id (get_selected_frame (NULL));
+  old_chain = make_cleanup_restore_current_thread (inferior_ptid, saved_frame_id);
+
   prune_threads ();
   target_find_new_threads ();
   current_ptid = inferior_ptid;
@@ -427,30 +434,22 @@ info_threads_command (char *arg, int fro
       if (extra_info)
 	printf_filtered (" (%s)", extra_info);
       puts_filtered ("  ");
-
+      /* That switch put us at the top of the stack (leaf frame).  */
       switch_to_thread (tp->ptid);
       print_stack_frame (get_selected_frame (NULL), 0, LOCATION);
     }
 
-  switch_to_thread (current_ptid);
+  /* Restores the current thread and the frame selected before
+     the "info threads" command.  */
+  do_cleanups (old_chain);
 
-  /* Restores the frame set by the user before the "info threads"
-     command.  We have finished the info-threads display by switching
-     back to the current thread.  That switch has put us at the top of
-     the stack (leaf frame).  */
-  cur_frame = frame_find_by_id (saved_frame_id);
-  if (cur_frame == NULL)
+  /*  If case we were not able to find the original frame, print the
+      new selected frame.  */
+  if (frame_find_by_id (saved_frame_id) == NULL)
     {
-      /* Ooops, can't restore, tell user where we are.  */
       warning (_("Couldn't restore frame in current thread, at frame 0"));
       print_stack_frame (get_selected_frame (NULL), 0, LOCATION);
     }
-  else
-    {
-      select_frame (cur_frame);
-      /* re-show current frame. */
-      show_stack_frame (cur_frame);
-    }
 }
 
 /* Switch from one thread to another. */
@@ -474,13 +473,27 @@ restore_current_thread (ptid_t ptid)
   if (!ptid_equal (ptid, inferior_ptid))
     {
       switch_to_thread (ptid);
-      print_stack_frame (get_current_frame (), 1, SRC_LINE);
+    }
+}
+
+static void
+restore_selected_frame (struct frame_id a_frame_id)
+{
+  struct frame_info *selected_frame_info = NULL;
+
+  if (frame_id_eq (a_frame_id, null_frame_id))
+    return;        
+
+  if ((selected_frame_info = frame_find_by_id (a_frame_id)) != NULL)
+    {
+      select_frame (selected_frame_info);
     }
 }
 
 struct current_thread_cleanup
 {
   ptid_t inferior_ptid;
+  struct frame_id selected_frame_id;
 };
 
 static void
@@ -488,15 +501,18 @@ do_restore_current_thread_cleanup (void 
 {
   struct current_thread_cleanup *old = arg;
   restore_current_thread (old->inferior_ptid);
+  restore_selected_frame (old->selected_frame_id);
   xfree (old);
 }
 
 static struct cleanup *
-make_cleanup_restore_current_thread (ptid_t inferior_ptid)
+make_cleanup_restore_current_thread (ptid_t inferior_ptid, 
+                                     struct frame_id a_frame_id)
 {
   struct current_thread_cleanup *old
     = xmalloc (sizeof (struct current_thread_cleanup));
   old->inferior_ptid = inferior_ptid;
+  old->selected_frame_id = a_frame_id;
   return make_cleanup (do_restore_current_thread_cleanup, old);
 }
 
@@ -516,11 +532,16 @@ thread_apply_all_command (char *cmd, int
   struct cleanup *old_chain;
   struct cleanup *saved_cmd_cleanup_chain;
   char *saved_cmd;
+  struct frame_id saved_frame_id;
+  ptid_t current_ptid;
+  int thread_has_changed = 0;
 
   if (cmd == NULL || *cmd == '\000')
     error (_("Please specify a command following the thread ID list"));
-
-  old_chain = make_cleanup_restore_current_thread (inferior_ptid);
+  
+  current_ptid = inferior_ptid;
+  saved_frame_id = get_frame_id (get_selected_frame (NULL));
+  old_chain = make_cleanup_restore_current_thread (inferior_ptid, saved_frame_id);
 
   /* It is safe to update the thread list now, before
      traversing it for "thread apply all".  MVS */
@@ -540,8 +561,15 @@ thread_apply_all_command (char *cmd, int
 	strcpy (cmd, saved_cmd);	/* Restore exact command used previously */
       }
 
+  if (!ptid_equal (current_ptid, inferior_ptid))
+    thread_has_changed = 1;
+
   do_cleanups (saved_cmd_cleanup_chain);
   do_cleanups (old_chain);
+  /* Print stack frame only if we changed thread.  */
+  if (thread_has_changed)
+    print_stack_frame (get_current_frame (), 1, SRC_LINE);
+
 }
 
 static void
@@ -552,6 +580,9 @@ thread_apply_command (char *tidlist, int
   struct cleanup *old_chain;
   struct cleanup *saved_cmd_cleanup_chain;
   char *saved_cmd;
+  struct frame_id saved_frame_id;
+  ptid_t current_ptid;
+  int thread_has_changed = 0;
 
   if (tidlist == NULL || *tidlist == '\000')
     error (_("Please specify a thread ID list"));
@@ -561,7 +592,9 @@ thread_apply_command (char *tidlist, int
   if (*cmd == '\000')
     error (_("Please specify a command following the thread ID list"));
 
-  old_chain = make_cleanup_restore_current_thread (inferior_ptid);
+  current_ptid = inferior_ptid;
+  saved_frame_id = get_frame_id (get_selected_frame (NULL));
+  old_chain = make_cleanup_restore_current_thread (inferior_ptid, saved_frame_id);
 
   /* Save a copy of the command in case it is clobbered by
      execute_command */
@@ -613,8 +646,14 @@ thread_apply_command (char *tidlist, int
 	}
     }
 
+  if (!ptid_equal (current_ptid, inferior_ptid))
+    thread_has_changed = 1;
+
   do_cleanups (saved_cmd_cleanup_chain);
   do_cleanups (old_chain);
+  /* Print stack frame only if we changed thread.  */
+  if (thread_has_changed)
+    print_stack_frame (get_current_frame (), 1, SRC_LINE);
 }
 
 /* Switch to the specified thread.  Will dispatch off to thread_apply_command

[-- Attachment #3: threadapply.exp.patch --]
[-- Type: text/plain, Size: 947 bytes --]

2007-01-30  Denis Pilat  <denis.pilat@st.com>

	* gdb.threads/threadapply.exp: check that frame is not changed by
	the thread apply all command.

Index: testsuite/gdb.threads/threadapply.exp
===================================================================
--- testsuite/gdb.threads/threadapply.exp	(revision 553)
+++ testsuite/gdb.threads/threadapply.exp	(working copy)
@@ -69,3 +69,10 @@ gdb_test_multiple "define backthread" "d
 gdb_test "set backtrace limit 3" ""
 gdb_test "thread apply all backthread" "Thread ..*\\\$1 = 0x14.*Thread ..*\\\$2 = 0x14.*Thread ..*\\\$3 = 0x14.*Thread ..*\\\$4 = 0x14.*Thread ..*\\\$5 = 0x14.*Thread ..*\\\$. = 0x14"
 
+# Go into the thread_function to check that a simple "thread apply"
+# does not change the selected frame.
+gdb_test "step" "thread_function.*"
+gdb_test "up"
+gdb_test "thread apply all print 1"
+gdb_test "down" "#0.*thread_function.*" "Thread apply must not change the selected frame"
+

Re: [RFC] thread apply commands change selected frame

[Daniel Jacobowitz]

On Wed, Jan 31, 2007 at 09:58:02AM +0100, Denis PILAT wrote:
> I propose a new implementation that does not change the old behavior of 
> gdb with respect to the printing of the last frame in case we changed of 
> thread during "thread apply" command execution.
> 
> This implementation does not make any regression in the testsuite for a 
> i386-linux target.
> To me the part that prints the stack frame at the end of execution of 
> "thread apply" could be entirely removed but this will imply some 
> changes in the testsuite. I can do that it's up to you.

Thanks.  I don't have any opinion on this - for now, let's just leave
it the way it was.

> +# Go into the thread_function to check that a simple "thread apply"
> +# does not change the selected frame.
> +gdb_test "step" "thread_function.*"
> +gdb_test "up"
> +gdb_test "thread apply all print 1"
> +gdb_test "down" "#0.*thread_function.*" "Thread apply must not change the selected frame"

The default name for a test is the command, if you don't specify a
name.  These are pretty generic commands, but test names are supposed
to be unique within a .exp file.  I would recommend giving them names
(and probably checking their output, at least a little, too...).

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFC] thread apply commands change selected frame

[Denis PILAT]

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


> The default name for a test is the command, if you don't specify a
> name.  These are pretty generic commands, but test names are supposed
> to be unique within a .exp file.  I would recommend giving them names
> (and probably checking their output, at least a little, too...).
>
>   

Your remarks have been taken into account into the attached patch.
Are you ok for committing this one plus the previous patch regarding the 
thread.c ?
(Frederic Riss will do that for me)

-- 
Denis Pilat
STMicroelectronics

[-- Attachment #2: threadapply.exp.patch --]
[-- Type: text/plain, Size: 1176 bytes --]

2007-01-31  Denis Pilat  <denis.pilat@st.com>

	* gdb.threads/threadapply.exp: check that frame is not changed by
	the thread apply all command.


Index: testsuite/gdb.threads/threadapply.exp
===================================================================
--- testsuite/gdb.threads/threadapply.exp	(revision 553)
+++ testsuite/gdb.threads/threadapply.exp	(working copy)
@@ -69,3 +69,9 @@ gdb_test_multiple "define backthread" "d
 gdb_test "set backtrace limit 3" ""
 gdb_test "thread apply all backthread" "Thread ..*\\\$1 = 0x14.*Thread ..*\\\$2 = 0x14.*Thread ..*\\\$3 = 0x14.*Thread ..*\\\$4 = 0x14.*Thread ..*\\\$5 = 0x14.*Thread ..*\\\$. = 0x14"
 
+# Go into the thread_function to check that a simple "thread apply"
+# does not change the selected frame.
+gdb_test "step" "thread_function.*" "step to the thread_function"
+gdb_test "up" ".*in main.*" "go up in the stack frame" 
+gdb_test "thread apply all print 1"  "Thread ..*\\\$7 = 1.*Thread ..*\\\$8 = 1.*Thread ..*\\\$9 = 1.*Thread ..*\\\$10 = 1.*Thread ..*\\\$11 = 1.*Thread ..*\\\$12 = 1" "run a simple print command on all thread"
+gdb_test "down" "#0.*thread_function.*" "go down and check selected frame"

Re: [RFC] thread apply commands change selected frame

[Daniel Jacobowitz]

On Wed, Jan 31, 2007 at 04:57:50PM +0100, Denis PILAT wrote:
> Your remarks have been taken into account into the attached patch.
> Are you ok for committing this one plus the previous patch regarding the 
> thread.c ?
> (Frederic Riss will do that for me)

Yes, that's OK (with a minor fix, below).  Let me know if you would
like write access yourself.

> +gdb_test "thread apply all print 1"  "Thread ..*\\\$7 = 1.*Thread ..*\\\$8 = 1.*Thread ..*\\\$9 = 1.*Thread ..*\\\$10 = 1.*Thread ..*\\\$11 = 1.*Thread ..*\\\$12 = 1" "run a simple print command on all thread"

Checking for $[specific number] should be avoided, because adding more
tests earlier in the file will break it.  Use \\\$\[0-9]+ instead.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] thread apply commands change selected frame

[Denis PILAT]

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

Daniel Jacobowitz wrote:
> On Wed, Jan 31, 2007 at 04:57:50PM +0100, Denis PILAT wrote:
>   
>> Your remarks have been taken into account into the attached patch.
>> Are you ok for committing this one plus the previous patch regarding the 
>> thread.c ?
>> (Frederic Riss will do that for me)
>>     
>
> Yes, that's OK (with a minor fix, below).  Let me know if you would
> like write access yourself.
>   
Yes thanks.
> Checking for $[specific number] should be avoided, because adding more
> tests earlier in the file will break it.  Use \\\$\[0-9]+ instead.
>
>   
I prefer that as well, therefore I changed the previous test as well to 
keep the same style (see attached file).

-- 
Denis

[-- Attachment #2: threadapply.exp.patch --]
[-- Type: text/plain, Size: 1525 bytes --]

2007-02-01  Denis Pilat  <denis.pilat@st.com>

	* gdb.threads/threadapply.exp: check that frame is not changed by
	the thread apply all command.

Index: testsuite/gdb.threads/threadapply.exp
===================================================================
--- testsuite/gdb.threads/threadapply.exp	(revision 553)
+++ testsuite/gdb.threads/threadapply.exp	(working copy)
@@ -67,5 +67,11 @@ gdb_test_multiple "define backthread" "d
 # verify that the macro can get past the backtrace error and perform
 # subsequent commands.
 gdb_test "set backtrace limit 3" ""
-gdb_test "thread apply all backthread" "Thread ..*\\\$1 = 0x14.*Thread ..*\\\$2 = 0x14.*Thread ..*\\\$3 = 0x14.*Thread ..*\\\$4 = 0x14.*Thread ..*\\\$5 = 0x14.*Thread ..*\\\$. = 0x14"
+gdb_test "thread apply all backthread" "Thread ..*\\\$\[0-9]+ = 0x14.*Thread ..*\\\$\[0-9]+ = 0x14.*Thread ..*\\\$\[0-9]+ = 0x14.*Thread ..*\\\$\[0-9]+ = 0x14.*Thread ..*\\\$\[0-9]+ = 0x14.*Thread ..*\\\$\[0-9]+ = 0x14"
 
+# Go into the thread_function to check that a simple "thread apply"
+# does not change the selected frame.
+gdb_test "step" "thread_function.*" "step to the thread_function"
+gdb_test "up" ".*in main.*" "go up in the stack frame" 
+gdb_test "thread apply all print 1"  "Thread ..*\\\$\[0-9]+ = 1.*Thread ..*\\\$\[0-9]+ = 1.*Thread ..*\\\$\[0-9]+ = 1.*Thread ..*\\\$\[0-9]+ = 1.*Thread ..*\\\$\[0-9]+ = 1.*Thread ..*\\\$\[0-9]+ = 1" "run a simple print command on all threads"
+gdb_test "down" "#0.*thread_function.*" "go down and check selected frame"

Re: [RFA] thread apply commands change selected frame

[Daniel Jacobowitz]

On Thu, Feb 01, 2007 at 10:14:22AM +0100, Denis PILAT wrote:
> I prefer that as well, therefore I changed the previous test as well to 
> keep the same style (see attached file).

Thanks - I didn't notice that.  This is OK, and so is the code patch.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA] thread apply commands change selected frame

[Denis PILAT]

Daniel Jacobowitz wrote:
> On Thu, Feb 01, 2007 at 10:14:22AM +0100, Denis PILAT wrote:
>   
>> I prefer that as well, therefore I changed the previous test as well to 
>> keep the same style (see attached file).
>>     
>
> Thanks - I didn't notice that.  This is OK, and so is the code patch.
>
>   
Following your approval, I've just committed  "thread.c" and 
"testsuite/gdb.threads/threadapply.exp" with the previously send ChangeLog.

-- 
Denis