Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: "Aktemur, Tankut Baris via Gdb-patches" <gdb-patches@sourceware.org>
To: Andrew Burgess <andrew.burgess@embecosm.com>
Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
Subject: RE: [PATCHv6 2/2] gdb: Track the current frame for each thread
Date: Fri, 18 Dec 2020 08:43:58 +0000	[thread overview]
Message-ID: <BL0PR11MB28824794A9B0E56E984B5D70C4C30@BL0PR11MB2882.namprd11.prod.outlook.com> (raw)
In-Reply-To: <201d792035b200d19e6a07558fb3fc25062bea2b.1607600155.git.andrew.burgess@embecosm.com>

On Thursday, December 10, 2020 12:39 PM, Andrew Burgess wrote:
> Currently in GDB, each time a user switches between threads, the inner
> most frame of the thread being switched to is selected.  In some
> situations however, it might be helpful for a user to have GDB
> remember which frame was selected in each thread, and restore this
> frame as the user switches between threads.
> 
> This commit the following two commands:

I think the verb is missing here.  Maybe "adds" or "introduces".

> 
>   set restore-selected-frame on|off
>   show restore-selected-frame
> 
> This new option is off by default, so the default behaviour of GDB is
> unchanged.
> 
> However, with this option turned on GDB will remember, and restore the
> selected frame for each thread.
> 
> My initial motivation for this change was to have the thread restored
> when switching threads with 'thread <num>', however, as I started to
> work on this feature I realised that there were a couple of other
> places where the sticky frame would naturally appear.  These are 'info
> threads' and 'thread apply all'.
> 
> With 'info threads' the output contains a 'Frame' column.  Previously,
> this was always the innermost frame, the info threads output was
> created by switching to each thread in turn and collecting information
> about the thread, this naturally placed us at the innermost frame.
> Now, the 'Frame' column displays the _selected_ frame for each
> thread.
> 
> I struggled to decide if this change was good or not.  In the end I
> felt that having 'info threads' display the selected frame would feel
> more natural, that's the frame you'll end up in if you switch to that
> thread, so if seemed to make sense.  However, it would be easy enough

"if" -> "it".

> to force the old behaviour if people would prefer.  Alternatively I
> could even investigate adding a switch to 'info threads' that allows
> the user to select displaying either the selected frame, or the inner
> most frame.

"innermost" is one word, AFAIK.

> For 'thread apply all', again, we used to always apply to the
> innermost frame.  Now it's possible for a user to adjust which frame
> will be current when the 'thread apply all' runs - this feels like a
> useful change to me.  It's easy enough to quickly restore the inner

Here, too: "inner most" -> "innermost"
There are a couple more instances below, in the texinfo file.

> most frame if required ('thread apply all -- frame 0') and having the
> flexibility to tweak the selected frame in just some threads feels
> like a nice advantage.  Again, I could potentially add a command flag
> here to force the inner most frame.
> 
> gdb/ChangeLog:
> 
> 	* NEWS: Describe new feature.
> 	* frame.c (cache_selected_frame_on_thread): New function.
> 	(select_frame): Call new function.
> 	* gdbthread.h (class thread_info) <selected_frame_id>: New
> 	member variable.
> 	<selected_frame_level>: Likewise.
> 	(switch_to_thread): Extra parameter.
> 	* thread.c (switch_to_thread_if_alive): Extra parameter, passed to
> 	switch_to_thread.
> 	(scoped_restore_current_thread::restore): Restore the frame either
> 	from the thread, or from the local object.
> 	(set_executing_thread): Reset the currently selected frame.
> 	(restore_selected_frame_per_thread): New file level static variable.
> 	(show_restore_selected_frame_per_thread): New function.
> 	(print_thread_info_1): Pass extra parameter to switch_to_thread.
> 	(switch_to_thread): Take extra parameter, restore the previous
> 	frame if appropriate.
> 	(thread_apply_all_command): Pass extra parameter to switch_to_thread.
> 	(thread_apply_command): Likewise.
> 	(thread_select): Pass extra parameter to switch_to_thread_if_alive.
> 	(_initialize_thread): Add new set/show variable.
> 
> gdb/doc/ChangeLog:
> 
> 	* gdb.texinfo (Threads): Add anchor to 'info threads'.  Describe
> 	the Frame column of 'info threads' more.  Describe which frame is
> 	selected when switching threads, and document the new option for
> 	restoring the previously selected frame.
> 
> gdb/testsuite/ChangeLog:
> 
> 	* gdb.threads/restore-selected-frame.c: New file.
> 	* gdb.threads/restore-selected-frame.exp: New file.

There are some related FIXME comments in frame.h:

   292  /* For every stopped thread, GDB tracks two frames: current and
   293     selected.  Current frame is the inner most frame of the selected
   294     thread.  Selected frame is the one being examined by the GDB
   295     CLI (selected using `up', `down', ...).  The frames are created
   296     on-demand (via get_prev_frame()) and then held in a frame cache.  */
   297  /* FIXME: cagney/2002-11-28: Er, there is a lie here.  If you do the
   298     sequence: `thread 1; up; thread 2; thread 1' you lose thread 1's
   299     selected frame.  At present GDB only tracks the selected frame of
   300     the current thread.  But be warned, that might change.  */
   301  /* FIXME: cagney/2002-11-14: At any time, only one thread's selected
   302     and current frame can be active.  Switching threads causes gdb to
   303     discard all that cached frame information.  Ulgh!  Instead, current
   304     and selected frame should be bound to a thread.  */
   305
   306  /* On demand, create the inner most frame using information found in
   307     the inferior.  If the inner most frame can't be created, throw an
   308     error.  */
   309  extern struct frame_info *get_current_frame (void);
...
   325  /* Return the selected frame.  Always returns non-NULL.  If there
   326     isn't an inferior sufficient for creating a frame, an error is
   327     thrown.  When MESSAGE is non-NULL, use it for the error message,
   328     otherwise use a generic error message.  */
   329  /* FIXME: cagney/2002-11-28: At present, when there is no selected
   330     frame, this function always returns the current (inner most) frame.
   331     It should instead, when a thread has previously had its frame
   332     selected (but not resumed) and the frame cache invalidated, find
   333     and then return that thread's previously selected frame.  */
   334  extern struct frame_info *get_selected_frame (const char *message = nullptr);

It might be worth updating them, too.


> ---
>  gdb/ChangeLog                                 |  24 ++
>  gdb/NEWS                                      |  11 +
>  gdb/doc/ChangeLog                             |   7 +
>  gdb/doc/gdb.texinfo                           |  23 +-
>  gdb/frame.c                                   |  26 ++
>  gdb/gdbthread.h                               |  13 +-
>  gdb/testsuite/ChangeLog                       |   5 +
>  .../gdb.threads/restore-selected-frame.c      |  85 +++++
>  .../gdb.threads/restore-selected-frame.exp    | 336 ++++++++++++++++++
>  gdb/thread.c                                  |  61 +++-
>  10 files changed, 578 insertions(+), 13 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.threads/restore-selected-frame.c
>  create mode 100644 gdb/testsuite/gdb.threads/restore-selected-frame.exp
> 
> diff --git a/gdb/NEWS b/gdb/NEWS
> index cc23bcf1ffd..fbd1cb52889 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -48,6 +48,17 @@ show restore-selected-thread
>    available, then GDB falls back to selecting the first non-exited
>    thread.
> 
> +set restore-selected-frame [on|off]
> +show restore-selected-frame
> +
> +  When turned on, GDB will record the currently selected frame in each
> +  thread.  When switching between threads, GDB will attempt to restore
> +  the previously selected frame in the thread being switched too.
> +  Executing a thread will cause GDB to discard any previously selected
> +  frame (GDB will select the inner most frame the next time the thread
> +  stops).  The 'info threads' command will show the selected frame in
> +  its 'frame' field.
> +
>  * Changed commands
> 
>  break [PROBE_MODIFIER] [LOCATION] [thread THREADNUM]
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 41d73b347a6..fdf91f0e179 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -3598,6 +3598,7 @@
>  @end smallexample
> 
>  @table @code
> +@anchor{info threads}
>  @kindex info threads
>  @item info threads @r{[}@var{thread-id-list}@r{]}
> 
> @@ -3625,7 +3626,9 @@
>  program itself.
> 
>  @item
> -the current stack frame summary for that thread
> +the current stack frame summary for that thread, this is the inner most
> +frame for the thread (@pxref{set restore-selected-frame} to display the
> +threads selected frame instead).
>  @end enumerate
> 
>  @noindent
> @@ -3697,6 +3700,24 @@
>  @samp{Switching to} depends on your system's conventions for identifying
>  threads.
> 
> +When switching between threads, @value{GDBN} will select the inner most
> +frame in the thread being switched too (@pxref{set restore-selected-frame}
> +to change this behaviour).
> +
> +@anchor{set restore-selected-frame}
> +@item set restore-selected-frame @r{[}on|off@r{]}
> +@itemx show restore-selected-frame
> +When @code{restore-selected-frame} is on, @value{GDBN} will restore
> +the previously selected frame when switching to a different thread.
> +Also the @code{info threads} command (@pxref{info threads}) will display the
> +currently selected frame for each thread.
> +
> +If a thread has been running, then when it stops, the previously
> +selected frame is discarded, and the inner most frame is again
> +selected.
> +
> +This option is @code{off} by default.
> +
>  @anchor{thread apply all}
>  @kindex thread apply
>  @cindex apply command to several threads
> diff --git a/gdb/frame.c b/gdb/frame.c
> index 4618da6c81e..a8e96aa534b 100644
> --- a/gdb/frame.c
> +++ b/gdb/frame.c
> @@ -1862,12 +1862,38 @@ deprecated_safe_get_selected_frame (void)
>    return get_selected_frame (NULL);
>  }
> 
> +/* When RESTORE_SELECTED_FRAME_PER_THREAD is true, then update in the
> +   current thread the information required to identify frame FI so the
> +   frame can be selected again later if we switch threads.  */
> +
> +static void
> +cache_selected_frame_on_thread ()
> +{
> +  struct frame_info *fi = selected_frame;
> +  struct thread_info *tp
> +    = find_thread_ptid (current_inferior (), inferior_ptid);
> +  if (fi != nullptr && tp != nullptr)
> +    {
> +      /* We only record the selected frame if the level is greater than 0,
> +	 this avoids having to calculate the frame id when selecting the
> +	 innermost frame.  When the cached selected frame is cleared then
> +	 we select the innermost frame anyway, so calculating the frame id
> +	 for frame #0 adds no value.  */
> +      if (frame_relative_level (fi) > 0)
> +	save_selected_frame (&tp->selected_frame_id,
> +			     &tp->selected_frame_level);
> +      else
> +	tp->selected_frame_level = -1;
> +    }
> +}
> +
>  /* Select frame FI (or NULL - to invalidate the selected frame).  */
> 
>  void
>  select_frame (struct frame_info *fi)
>  {
>    selected_frame = fi;
> +  cache_selected_frame_on_thread ();
>    selected_frame_level = frame_relative_level (fi);
>    if (selected_frame_level == 0)
>      {
> diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
> index e5484ac54ca..686a463d5f7 100644
> --- a/gdb/gdbthread.h
> +++ b/gdb/gdbthread.h
> @@ -370,6 +370,12 @@ class thread_info : public refcounted_object
>       bp_longjmp_call_dummy.  */
>    struct frame_id initiating_frame = null_frame_id;
> 
> +  /* Information for the last frame successfully selected in this thread.
> +     If the user configurable setting is on then GDB will try to reselect
> +     this frame when switching threads.  */
> +  struct frame_id selected_frame_id {};
> +  int selected_frame_level = -1;
> +
>    /* Private data used by the target vector implementation.  */
>    std::unique_ptr<private_thread_info> priv;
> 
> @@ -575,8 +581,11 @@ extern int thread_count (process_stratum_target *proc_target);
>  /* Return true if we have any thread in any inferior.  */
>  extern bool any_thread_p ();
> 
> -/* Switch context to thread THR.  Also sets the STOP_PC global.  */
> -extern void switch_to_thread (struct thread_info *thr);
> +/* Switch context to thread THR.  Also sets the STOP_PC global.  When
> +   RESTORE_PREVIOUS_FRAME is true then, if this thread has a previously
> +   selected frame cached, the previous frame is restored.  */
> +extern void switch_to_thread (struct thread_info *thr,
> +			      bool restore_previous_frame = false);
> 
>  /* Switch context to no thread selected.  */
>  extern void switch_to_no_thread ();
> diff --git a/gdb/testsuite/gdb.threads/restore-selected-frame.c
> b/gdb/testsuite/gdb.threads/restore-selected-frame.c
> new file mode 100644
> index 00000000000..c72b0b8b54b
> --- /dev/null
> +++ b/gdb/testsuite/gdb.threads/restore-selected-frame.c
> @@ -0,0 +1,85 @@
> +#include <sys/types.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +#include <stdlib.h>
> +#include <pthread.h>
> +
> +volatile int loop_count = 10;
> +volatile int thread_count = 3;
> +
> +static void
> +thread_level_5 (int id, int count)
> +{
> +  printf ("Thread %d reached %s, #%d\n",
> +	  id, __PRETTY_FUNCTION__, count);
> +}
> +
> +static void
> +thread_level_4 (int id, int count)
> +{
> +  thread_level_5 (id, count);
> +}
> +
> +static void
> +thread_level_3 (int id, int count)
> +{
> +  thread_level_4 (id, count);
> +}
> +
> +static void
> +thread_level_2 (int id, int count)
> +{
> +  thread_level_3 (id, count);
> +}
> +
> +static void
> +thread_level_1 (int id, int count)
> +{
> +  thread_level_2 (id, count);
> +}
> +
> +static void *
> +thread_worker (void *arg)
> +{
> +  int i, max, id;
> +
> +  id = *((int *) arg);
> +  max = loop_count;
> +  for (i = 0; i < max; ++i)
> +    thread_level_1 (id, (i + 1));
> +
> +  return NULL;
> +}
> +
> +struct thread_info
> +{
> +  pthread_t thread;
> +  int id;
> +};
> +
> +int
> +main ()
> +{
> +  int i, max = thread_count;
> +
> +  struct thread_info *info = malloc (sizeof (struct thread_info) * max);
> +  if (info == NULL)
> +    abort ();
> +
> +  for (i = 0; i < max; ++i)
> +    {
> +      struct thread_info *thr = &info[i];
> +      thr->id = i + 1;
> +      if (pthread_create (&thr->thread, NULL, thread_worker, &thr->id) != 0)
> +	abort ();
> +    }
> +
> +  for (i = 0; i < max; ++i)
> +    {
> +      struct thread_info *thr = &info[i];
> +      if (pthread_join (thr->thread, NULL) != 0)
> +	abort ();
> +    }
> +
> +  free (info);
> +}
> diff --git a/gdb/testsuite/gdb.threads/restore-selected-frame.exp
> b/gdb/testsuite/gdb.threads/restore-selected-frame.exp
> new file mode 100644
> index 00000000000..b40386fc58e
> --- /dev/null
> +++ b/gdb/testsuite/gdb.threads/restore-selected-frame.exp
> @@ -0,0 +1,336 @@
> +# Copyright 2020 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/>.
> +
> +# This tests GDB's tracking of the currently selected frame on a
> +# per-thread basis.
> +#
> +# We setup a couple of inferiors, each with mutliple theads, we then
> +# switch between threads and modify the current frame.  We use 'info
> +# threads' to check that GDB is correctly tracking the current frame.
> +#
> +# Toward the end of the test we check that when a thread executes the
> +# currently selected frame is reset.
> +#
> +# Finally we disable tracking of the currently selected frame and
> +# ensure GDB no longer restores the current frame.
> +
> +standard_testfile
> +
> +set options { debug pthreads }
> +if {[prepare_for_testing "failed to prepare" $testfile $srcfile \
> +	 $options] == -1} {
> +    return -1
> +}
> +
> +# Run the 'info threads' command, and check that the frame part of
> +# each threads output matches the corresponding pattern in FRAME_INFO,
> +# with thread 1 using entry 0 from FRAME_INFO, thread 2 using entry 1,
> +# and so on.
> +proc test_info_threads { testname frame_info } {
> +    global decimal hex gdb_prompt
> +
> +    set thread_count 0
> +    gdb_test_multiple "info threads" ${testname} {
> +	-re ".*  Id\\s+Target Id\\s+Frame\\s*\r\n" {
> +	    # Discard the info threads header line as well as any
> +	    # output before it in the expect buffer.
> +	    exp_continue
> +	}
> +
> +	-re "^\[* \]\\s+(($decimal)\.)?($decimal)\\s+Thread $hex \\(LWP $decimal\\)
> \"\[^\"\]+\"\\s+(\[^\r\n\]*)\r\n" {
> +	    if {[info exists expect_out(2,string)]} {
> +		set id "$expect_out(2,string).$expect_out(3,string)"
> +		set index [expr [expr [expr $expect_out(2,string) - 1] * 4] \
> +			       + [expr $expect_out(3,string) - 1]]
> +	    } else {
> +		set id $expect_out(3,string)
> +		set index [expr $id - 1]
> +	    }
> +	    set frame $expect_out(4,string)
> +	    set pattern [lindex $frame_info $index]
> +	    gdb_assert {[regexp -- $pattern $frame]} \
> +		"$testname: thread $id matches"
> +	    incr thread_count
> +	    exp_continue
> +	}
> +	-re "^$gdb_prompt " {
> +	}
> +    }
> +    gdb_assert {$thread_count == [llength $frame_info]} \
> +	"$testname: all threads seen"
> +}
> +
> +# Run 'thread THREAD_NUM' and check that we switch thread.
> +proc switch_thread { thread_num } {
> +    gdb_test "thread ${thread_num}" \
> +	"Switching to thread (2\.)?${thread_num} .*"
> +}
> +
> +# Used during startup, continue the inferior and wait for all threads
> +# to stop at the breakpoint.
> +proc run_all_threads_to_breakpoint { } {
> +    global gdb_prompt
> +
> +    set stopped_thread_count 0
> +    gdb_test_multiple "continue" "wait for worker threads to stop" {
> +	-re "Thread (2\.)?\[234\] \"\[^\"\]+\" hit Breakpoint" {
> +	    incr stopped_thread_count
> +	    if {$stopped_thread_count < 3} {
> +		exp_continue
> +	    }
> +	}
> +
> +	-re "$gdb_prompt" {
> +	    exp_continue
> +	}
> +    }
> +
> +    gdb_assert {$stopped_thread_count == 3} \
> +	"all worker threads stopped"
> +}
> +
> +# Switch to thread #1, and interrupt it.
> +proc switch_to_and_stop_thread_1 {} {
> +    global gdb_prompt
> +    # There's a bit of a wart here in that after sending "interrupt"
> +    # the output seems to appear out of order this is probably a
> +    # consequence of being in non-stop mode, so this is what I'd like
> +    # to see:
> +    #
> +    #   (gdb) interrupt
> +    #   Thread 1 "...." stopped.
> +    #   (gdb)
> +    #
> +    # But what we actually see is:
> +    #
> +    #   (gdb) interrupt
> +    #   (gdb)
> +    #   Thread 1 "...." stopped.
> +    #
> +    # What happens of course is that GDB processes the interrupt,
> +    # sends a SIGSTOP to the inferior and then returns to the prompt,
> +    # at this point we process the stop event from the inferior and
> +    # print the stopped message.
> +    #
> +    # It would be nice if GDB could be smart enough to reprint the
> +    # prompt after the stop message though.
> +    #
> +    # The first 'interrupt\n' here causes the interior to stop, while
> +    # the following lone '\n' causes the prompt to be reprinted.  This
> +    # allows us to match all the output up to the final prompt,
> +    # ensuring we don't leave any stray output in expect's output
> +    # buffer.
> +    switch_thread 1
> +    gdb_test_multiple "interrupt\\n" "interrupt thread 1" {
> +	-re "^interrupt\\\\n\\r\\n$gdb_prompt " {
> +	    pass $gdb_test_name
> +	}
> +    }
> +    gdb_test_multiple "" "wait for thread 1 to stop" {
> +	-re "Thread (2\.)?1 \"\[^\"\]+\" stopped\." {
> +	    send_gdb "\n"
> +	    gdb_test_multiple "" \
> +		"wait for prompt after thread 1 stopped" {
> +		-re ".*$gdb_prompt " {
> +		    pass $gdb_test_name
> +		}
> +	    }
> +	}
> +    }
> +}
> +
> +# Setup for this test.  Place GDB in non-stop mode, create an initial
> +# breakpoint, run all of the threads to the breakpoint, then stop
> +# thread 1 (which doesn't hit the breakpoint).
> +proc setup_for_test {} {
> +    gdb_test_no_output "set non-stop on"
> +
> +    if ![runto_main] {
> +	fail "runto main"
> +	return
> +    }
> +
> +    gdb_test_no_output "set restore-selected-frame on"
> +
> +    gdb_breakpoint "thread_level_5"
> +
> +    with_test_prefix "setup inferior 1" {
> +	# Now run the inferior, and wait for all of the expected threads
> +	# to hit the thread_level_5 breakpoint.
> +	run_all_threads_to_breakpoint
> +
> +	# The main thread will still be running at this point, waiting for
> +	# the stopped threads to finish so it can join with them.  Lets go
> +	# and interrupt it.
> +	switch_to_and_stop_thread_1
> +    }
> +}
> +
> +setup_for_test
> +
> +# We can't rely on frames being within 'pthread_join' actually being
> +# in a frame called pthread_join.  Different versions of pthreads
> +# might call the function something different.  So, just have a
> +# match all pattern.
> +set pthread_join_pattern ".*"
> +
> +set frame_info [list "$hex in ${pthread_join_pattern}" \
> +		    "thread_level_5" \
> +		    "thread_level_5" \
> +		    "thread_level_5" ]
> +
> +
> +# We now have all threads stopped in known locations.  Lets check that
> +# everyone is where we expect them to be.
> +test_info_threads "info threads #1" $frame_info
> +
> +# First, lets move thread 1.  Then check that the info threads output
> +# reflects this.
> +gdb_test "up" ".*"
> +set frame_info [lreplace $frame_info 0 0 "$hex in main"]
> +test_info_threads "info threads #2" $frame_info
> +
> +# Now lets change the other threads, one at a time, checking the
> +# output of info threads after each change.
> +foreach spec [list [list 2 5 "$hex in thread_worker"] \
> +		  [list 3 3 "$hex in thread_level_2"] \
> +		  [list 4 1 "$hex in thread_level_4"] ] {
> +    set thr [lindex $spec 0]
> +    with_test_prefix "change frame for thread $thr" {
> +	switch_thread $thr
> +	gdb_test "frame [lindex $spec 1]" ".*"
> +	set idx [expr $thr - 1]
> +	set frame_info [lreplace $frame_info $idx $idx [lindex $spec 2]]
> +	test_info_threads "info threads #3" $frame_info
> +    }
> +}
> +
> +# Start a new inferior, and runto main.
> +gdb_test "add-inferior" "Added inferior 2 .*" \
> +    "add empty inferior 2"
> +gdb_test "inferior 2" "Switching to inferior 2 .*" \
> +    "switch to inferior 2"
> +gdb_test "file ${binfile}" ".*" "load file in inferior 2"
> +
> +with_test_prefix "start inferior 2" {
> +    # Disable deleting of breakpoints.
> +    proc delete_breakpoints {} {}
> +    runto_main
> +}
> +
> +with_test_prefix "setup inferior 2" {
> +    run_all_threads_to_breakpoint
> +    switch_to_and_stop_thread_1
> +}
> +
> +set frame_info [concat $frame_info [list "$hex in ${pthread_join_pattern}" \
> +					"thread_level_5" \
> +					"thread_level_5" \
> +					"thread_level_5" ]]
> +test_info_threads "info threads #4" $frame_info
> +
> +# Now lets change the other threads, one at a time, checking the
> +# output of info threads after each change.
> +foreach spec [list [list 2 2 "$hex in thread_level_3"] \
> +		  [list 3 2 "$hex in thread_level_3"] \
> +		  [list 4 2 "$hex in thread_level_3"] ] {
> +    set thr [lindex $spec 0]
> +    with_test_prefix "change frame for thread $thr" {
> +	switch_thread "2.$thr"
> +	gdb_test "frame [lindex $spec 1]" ".*"
> +	set idx [expr 4 + $thr - 1]
> +	set frame_info [lreplace $frame_info $idx $idx [lindex $spec 2]]
> +	test_info_threads "info threads #5" $frame_info
> +    }
> +}
> +
> +# Now step one of the threads.  The thread that is stepped should
> +# discard its stored selected frame, but all other threads should
> +# retain their selected frame.
> +switch_thread "2.2"
> +gdb_test "step" ".*" \
> +    "step in thread 2.2"
> +set frame_info [lreplace $frame_info 5 5 "thread_level_5"]
> +test_info_threads "info threads #6" $frame_info
> +
> +# Same again for a thread in inferior #1.
> +switch_thread "1.3"
> +gdb_test "step" ".*" \
> +    "step in thread 1.3"
> +set frame_info [lreplace $frame_info 2 2 "thread_level_5"]
> +test_info_threads "info threads #7" $frame_info
> +
> +# Now switch to another thread that already has a frame other than its
> +# innermost selected.
> +switch_thread "1.2"
> +
> +# Now disable restoring of the selected frame.
> +gdb_test_no_output "set restore-selected-frame off"
> +
> +# And check to see which frame each thread has selected.  Our current
> +# thread shouldn't change.
> +set frame_info [list "$hex in ${pthread_join_pattern}" \
> +		    "thread_worker" \
> +		    "thread_level_5" \
> +		    "thread_level_5" \
> +		    "$hex in ${pthread_join_pattern}" \
> +		    "thread_level_5" \
> +		    "thread_level_5" \
> +		    "thread_level_5"]
> +test_info_threads "info threads #8" $frame_info
> +
> +# Now switch to some other thread, at this point GDB should forget the
> +# selected frame for thread 1.2.
> +switch_thread "1.4"
> +set frame_info [lreplace $frame_info 1 1 "thread_level_5"]
> +test_info_threads "info threads #9" $frame_info
> +
> +# A new test that will cover 'thread apply all'.  This test ensures
> +# that any changes to the selected thread in 'thread apply all' are
> +# sticky outside of the 'thread apply all'.
> +with_test_prefix "thr apply all" {
> +    clean_restart $binfile
> +    setup_for_test
> +
> +    # Move all threads up a frame.
> +    gdb_test "thread apply all -- up" ".*" \
> +	"all threads up, first time"
> +    set frame_info [list "$hex in main" \
> +			"$hex in thread_level_4" \
> +			"$hex in thread_level_4" \
> +			"$hex in thread_level_4" ]
> +    test_info_threads "info threads #10" $frame_info
> +
> +    # Move every thread back to frame 0.
> +    gdb_test "thread apply all -- frame 0" ".*"
> +    set frame_info [list "$hex in ${pthread_join_pattern}" \
> +			"thread_level_5" \
> +			"thread_level_5" \
> +			"thread_level_5" ]
> +    test_info_threads "info threads #11" $frame_info
> +
> +    # Disable restoring the current frame.
> +    gdb_test_no_output "set restore-selected-frame off"
> +
> +    # Move all threads up a frame, no frame should change after this
> +    # though.
> +    gdb_test "thread apply all -- up" ".*" \
> +	"all threads up, second time"
> +    set frame_info [list "$hex in ${pthread_join_pattern}" \
> +			"thread_level_5" \
> +			"thread_level_5" \
> +			"thread_level_5" ]
> +    test_info_threads "info threads #12" $frame_info
> +}
> diff --git a/gdb/thread.c b/gdb/thread.c
> index 856bdee97b3..63261247e19 100644
> --- a/gdb/thread.c
> +++ b/gdb/thread.c
> @@ -721,7 +721,7 @@ thread_alive (thread_info *tp)
>     switched, false otherwise.  */
> 
>  static bool
> -switch_to_thread_if_alive (thread_info *thr)
> +switch_to_thread_if_alive (thread_info *thr, bool restore_previous_frame)
>  {
>    scoped_restore_current_thread restore_thread;
> 
> @@ -731,7 +731,7 @@ switch_to_thread_if_alive (thread_info *thr)
> 
>    if (thread_alive (thr))
>      {
> -      switch_to_thread (thr);
> +      switch_to_thread (thr, restore_previous_frame);
>        restore_thread.dont_restore ();
>        return true;
>      }
> @@ -903,7 +903,10 @@ set_executing_thread (thread_info *thr, bool executing)
>  {
>    thr->executing = executing;
>    if (executing)
> -    thr->suspend.stop_pc = ~(CORE_ADDR) 0;
> +    {
> +      thr->suspend.stop_pc = ~(CORE_ADDR) 0;
> +      thr->selected_frame_level = -1;
> +    }
>  }
> 
>  void
> @@ -1066,6 +1069,23 @@ thread_target_id_str (thread_info *tp)
>      return target_id;
>  }
> 
> +/* When this is true, GDB restore the thread's previously selected frame

"restore" -> "restores"

Regards
-Baris



Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

  reply	other threads:[~2020-12-18  8:44 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-25 22:24 [PATCHv3 0/3] Restore thread and frame patches Andrew Burgess
2020-09-25 22:24 ` [PATCHv3 1/3] gdb: unify two copies of restore_selected_frame Andrew Burgess
2020-09-25 22:24 ` [PATCHv3 2/3] gdb: Restore previously selected thread when switching inferior Andrew Burgess
2020-09-26  6:09   ` Eli Zaretskii via Gdb-patches
2020-09-25 22:24 ` [PATCHv3 3/3] gdb: Track the current frame for each thread Andrew Burgess
2020-09-26  6:16   ` Eli Zaretskii via Gdb-patches
2020-09-26  8:31     ` Andreas Schwab
2020-09-26  8:54       ` Eli Zaretskii via Gdb-patches
2020-10-08  9:59 ` [PATCHv3 0/3] Restore thread and frame patches Andrew Burgess
2020-11-06 23:02   ` [PATCHv4 0/2] " Andrew Burgess
2020-11-06 23:02     ` [PATCHv4 1/2] gdb: Restore previously selected thread when switching inferior Andrew Burgess
2020-11-09 15:02       ` Aktemur, Tankut Baris via Gdb-patches
2020-11-06 23:02     ` [PATCHv4 2/2] gdb: Track the current frame for each thread Andrew Burgess
2020-11-12 11:59     ` [PATCHv5 0/2] Restore thread and frame patches Andrew Burgess
2020-12-10 11:39       ` [PATCHv6 " Andrew Burgess
2020-12-10 11:39         ` [PATCHv6 1/2] gdb: Restore previously selected thread when switching inferior Andrew Burgess
2020-12-10 11:39         ` [PATCHv6 2/2] gdb: Track the current frame for each thread Andrew Burgess
2020-12-18  8:43           ` Aktemur, Tankut Baris via Gdb-patches [this message]
2021-01-04 15:07             ` Andrew Burgess
2021-01-04 16:08               ` Eli Zaretskii via Gdb-patches
2021-01-07 10:25         ` [PATCHv6 0/2] Restore thread and frame patches Andrew Burgess
2021-02-12 18:20         ` [PATCHv7 " Andrew Burgess
2021-02-12 18:20           ` [PATCHv7 1/2] gdb: Restore previously selected thread when switching inferior Andrew Burgess
2021-02-12 18:20           ` [PATCHv7 2/2] gdb: Track the current frame for each thread Andrew Burgess
2021-03-23 11:13           ` [PATCHv8] gdb: Restore previously selected thread when switching inferior Andrew Burgess
2021-03-23 11:43             ` Eli Zaretskii via Gdb-patches
2020-11-12 11:59     ` [PATCHv5 1/2] " Andrew Burgess
2020-11-12 11:59     ` [PATCHv5 2/2] gdb: Track the current frame for each thread Andrew Burgess

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=BL0PR11MB28824794A9B0E56E984B5D70C4C30@BL0PR11MB2882.namprd11.prod.outlook.com \
    --to=gdb-patches@sourceware.org \
    --cc=andrew.burgess@embecosm.com \
    --cc=tankut.baris.aktemur@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox