gdb: Eliminate the 'stop_pc' global

Message ID 20180621165501.17051-1-palves@redhat.com
State New
Headers show
Series
  • gdb: Eliminate the 'stop_pc' global
Related show

Commit Message

Pedro Alves June 21, 2018, 4:55 p.m.
In my multi-target work, I need to add a few more
scoped_restore_current_thread and switch_to_thread calls in some
places, and in some lower-level places I was fighting against the fact
that switch_to_thread reads/refreshes the stop_pc global.

Instead of piling on workarounds, let's just finally eliminate the
stop_pc global.  We already have the per-thread
thread_info->suspend.stop_pc field, so it's mainly a matter of using
that more/instead.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* gdbthread.h (struct thread_suspend_state) <stop_pc>: Extend
	comments.
	(switch_to_thread_no_regs): Adjust comment.
	* infcmd.c (stop_pc): Delete.
	(post_create_inferior, info_program_command): Replace references
	to stop_pc with references to thread_info->suspend.stop_pc.
	* inferior.h (stop_pc): Delete declaration.
	* infrun.c (proceed, handle_syscall_event, fill_in_stop_func)
	(handle_inferior_event_1, handle_signal_stop)
	(process_event_stop_test, keep_going_stepped_thread)
	(handle_step_into_function, handle_step_into_function_backward)
	(print_stop_location): Replace references to stop_pc with
	references to thread_info->suspend.stop_pc.
	(struct infcall_suspend_state) <stop_pc>: Delete field.
	(save_infcall_suspend_state, restore_infcall_suspend_state):
	Remove references to inf_stat->stop_pc.
	* linux-fork.c (fork_load_infrun_state): Likewise.
	* record-btrace.c (record_btrace_set_replay): Likewise.
	* record-full.c (record_full_goto_entry): Likewise.
	* remote.c (print_one_stopped_thread): Likewise.
	* target.c (target_resume): Extend comment.
	* thread.c (set_executing_thread): New.
	(set_executing): Use it.
	(switch_to_thread_no_regs, switch_to_no_thread, switch_to_thread):
	Remove references to stop_pc.
---
 gdb/gdbthread.h     |  20 ++++++---
 gdb/infcmd.c        |  12 +++---
 gdb/inferior.h      |   4 --
 gdb/infrun.c        | 117 +++++++++++++++++++++++++++++++---------------------
 gdb/linux-fork.c    |   3 +-
 gdb/record-btrace.c |   3 +-
 gdb/record-full.c   |   3 +-
 gdb/remote.c        |   2 +-
 gdb/target.c        |   3 +-
 gdb/thread.c        |  25 ++++++-----
 10 files changed, 112 insertions(+), 80 deletions(-)

-- 
2.14.3

Comments

Simon Marchi June 21, 2018, 8:10 p.m. | #1
On 2018-06-21 12:55, Pedro Alves wrote:
> In my multi-target work, I need to add a few more

> scoped_restore_current_thread and switch_to_thread calls in some

> places, and in some lower-level places I was fighting against the fact

> that switch_to_thread reads/refreshes the stop_pc global.

> 

> Instead of piling on workarounds, let's just finally eliminate the

> stop_pc global.  We already have the per-thread

> thread_info->suspend.stop_pc field, so it's mainly a matter of using

> that more/instead.


Ohh nice.  I don't know much about this area, but I read the patch and 
nothing caught my attention, so it LGTM.

Simon
Sergio Durigan Junior June 22, 2018, 4:56 p.m. | #2
On Thursday, June 21 2018, Pedro Alves wrote:

> In my multi-target work, I need to add a few more

> scoped_restore_current_thread and switch_to_thread calls in some

> places, and in some lower-level places I was fighting against the fact

> that switch_to_thread reads/refreshes the stop_pc global.


Hey,

My two cents: I read your message and the patch, and did not find
anything problematic.  Thanks for doing that, always good to see globals
go away.

> Instead of piling on workarounds, let's just finally eliminate the

> stop_pc global.  We already have the per-thread

> thread_info->suspend.stop_pc field, so it's mainly a matter of using

> that more/instead.

>

> gdb/ChangeLog:

> yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

>

> 	* gdbthread.h (struct thread_suspend_state) <stop_pc>: Extend

> 	comments.

> 	(switch_to_thread_no_regs): Adjust comment.

> 	* infcmd.c (stop_pc): Delete.

> 	(post_create_inferior, info_program_command): Replace references

> 	to stop_pc with references to thread_info->suspend.stop_pc.

> 	* inferior.h (stop_pc): Delete declaration.

> 	* infrun.c (proceed, handle_syscall_event, fill_in_stop_func)

> 	(handle_inferior_event_1, handle_signal_stop)

> 	(process_event_stop_test, keep_going_stepped_thread)

> 	(handle_step_into_function, handle_step_into_function_backward)

> 	(print_stop_location): Replace references to stop_pc with

> 	references to thread_info->suspend.stop_pc.

> 	(struct infcall_suspend_state) <stop_pc>: Delete field.

> 	(save_infcall_suspend_state, restore_infcall_suspend_state):

> 	Remove references to inf_stat->stop_pc.

> 	* linux-fork.c (fork_load_infrun_state): Likewise.

> 	* record-btrace.c (record_btrace_set_replay): Likewise.

> 	* record-full.c (record_full_goto_entry): Likewise.

> 	* remote.c (print_one_stopped_thread): Likewise.

> 	* target.c (target_resume): Extend comment.

> 	* thread.c (set_executing_thread): New.

> 	(set_executing): Use it.

> 	(switch_to_thread_no_regs, switch_to_no_thread, switch_to_thread):

> 	Remove references to stop_pc.

> ---

>  gdb/gdbthread.h     |  20 ++++++---

>  gdb/infcmd.c        |  12 +++---

>  gdb/inferior.h      |   4 --

>  gdb/infrun.c        | 117 +++++++++++++++++++++++++++++++---------------------

>  gdb/linux-fork.c    |   3 +-

>  gdb/record-btrace.c |   3 +-

>  gdb/record-full.c   |   3 +-

>  gdb/remote.c        |   2 +-

>  gdb/target.c        |   3 +-

>  gdb/thread.c        |  25 ++++++-----

>  10 files changed, 112 insertions(+), 80 deletions(-)

>

> diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h

> index bd5ab9193f..983e0f71fa 100644

> --- a/gdb/gdbthread.h

> +++ b/gdb/gdbthread.h

> @@ -168,10 +168,19 @@ struct thread_suspend_state

>  

>    /* Record the pc of the thread the last time it stopped.  (This is

>       not the current thread's PC as that may have changed since the

> -     last stop, e.g., "return" command, or "p $pc = 0xf000").  This is

> -     used in coordination with stop_reason and waitstatus_pending_p:

> -     if the thread's PC is changed since it last stopped, a pending

> -     breakpoint waitstatus is discarded.  */

> +     last stop, e.g., "return" command, or "p $pc = 0xf000").

> +

> +     - If the thread's PC has not changed since the thread last

> +       stopped, then proceed skips a breakpoint at the current PC,

> +       otherwise we let the thread run into the breakpoint.

> +

> +     - If the thread has an unprocessed event pending, as indicated by

> +       waitstatus_pending_p, this is used in coordination with

> +       stop_reason: if the thread's PC has changed since the thread

> +       last stopped, a pending breakpoint waitstatus is discarded.

> +

> +     - If the thread is running, this is set to -1, to avoid leaving

> +       it with a stale value, to make it easier to catch bugs.  */

>    CORE_ADDR stop_pc;

>  };

>  

> @@ -498,8 +507,7 @@ extern void switch_to_thread (thread_info *thr);

>  /* Switch context to no thread selected.  */

>  extern void switch_to_no_thread ();

>  

> -/* Switch from one thread to another.  Does not read registers and

> -   sets STOP_PC to -1.  */

> +/* Switch from one thread to another.  Does not read registers.  */

>  extern void switch_to_thread_no_regs (struct thread_info *thread);

>  

>  /* Marks or clears thread(s) PTID as resumed.  If PTID is

> diff --git a/gdb/infcmd.c b/gdb/infcmd.c

> index c6fd9ab3bc..26d5a2aa95 100644

> --- a/gdb/infcmd.c

> +++ b/gdb/infcmd.c

> @@ -94,10 +94,6 @@ static char *inferior_io_terminal_scratch;

>  

>  ptid_t inferior_ptid;

>  

> -/* Address at which inferior stopped.  */

> -

> -CORE_ADDR stop_pc;

> -

>  /* Nonzero if stopped due to completion of a stack dummy routine.  */

>  

>  enum stop_stack_kind stop_stack_dummy;

> @@ -448,10 +444,12 @@ post_create_inferior (struct target_ops *target, int from_tty)

>    /* Now that we know the register layout, retrieve current PC.  But

>       if the PC is unavailable (e.g., we're opening a core file with

>       missing registers info), ignore it.  */

> -  stop_pc = 0;

> +  thread_info *thr = inferior_thread ();

> +

> +  thr->suspend.stop_pc = 0;

>    TRY

>      {

> -      stop_pc = regcache_read_pc (get_current_regcache ());

> +      thr->suspend.stop_pc = regcache_read_pc (get_current_regcache ());

>      }

>    CATCH (ex, RETURN_MASK_ERROR)

>      {

> @@ -2108,7 +2106,7 @@ info_program_command (const char *args, int from_tty)

>  

>    target_files_info ();

>    printf_filtered (_("Program stopped at %s.\n"),

> -		   paddress (target_gdbarch (), stop_pc));

> +		   paddress (target_gdbarch (), tp->suspend.stop_pc));

>    if (tp->control.stop_step)

>      printf_filtered (_("It stopped after being stepped.\n"));

>    else if (stat != 0)

> diff --git a/gdb/inferior.h b/gdb/inferior.h

> index 3f4d7a50d6..70cef161cc 100644

> --- a/gdb/inferior.h

> +++ b/gdb/inferior.h

> @@ -208,10 +208,6 @@ extern void prepare_execution_command (struct target_ops *target,

>     the target is started up with a shell.  */

>  extern int startup_with_shell;

>  

> -/* Address at which inferior stopped.  */

> -

> -extern CORE_ADDR stop_pc;

> -

>  /* Nonzero if stopped due to completion of a stack dummy routine.  */

>  

>  extern enum stop_stack_kind stop_stack_dummy;

> diff --git a/gdb/infrun.c b/gdb/infrun.c

> index 9548f9cafd..28a4391c35 100644

> --- a/gdb/infrun.c

> +++ b/gdb/infrun.c

> @@ -3009,7 +3009,7 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)

>  

>    if (addr == (CORE_ADDR) -1)

>      {

> -      if (pc == stop_pc

> +      if (pc == tp->suspend.stop_pc

>  	  && breakpoint_here_p (aspace, pc) == ordinary_breakpoint_here

>  	  && execution_direction != EXEC_REVERSE)

>  	/* There is a breakpoint at the address we will resume at,

> @@ -4249,7 +4249,7 @@ handle_syscall_event (struct execution_control_state *ecs)

>  

>    regcache = get_thread_regcache (ecs->event_thread);

>    syscall_number = ecs->ws.value.syscall_number;

> -  stop_pc = regcache_read_pc (regcache);

> +  ecs->event_thread->suspend.stop_pc = regcache_read_pc (regcache);

>  

>    if (catch_syscall_enabled () > 0

>        && catching_syscall_number (syscall_number) > 0)

> @@ -4260,7 +4260,8 @@ handle_syscall_event (struct execution_control_state *ecs)

>  

>        ecs->event_thread->control.stop_bpstat

>  	= bpstat_stop_status (regcache->aspace (),

> -			      stop_pc, ecs->event_thread, &ecs->ws);

> +			      ecs->event_thread->suspend.stop_pc,

> +			      ecs->event_thread, &ecs->ws);

>  

>        if (handle_stop_requested (ecs))

>  	return 0;

> @@ -4290,7 +4291,8 @@ fill_in_stop_func (struct gdbarch *gdbarch,

>      {

>        /* Don't care about return value; stop_func_start and stop_func_name

>  	 will both be 0 if it doesn't work.  */

> -      find_pc_partial_function (stop_pc, &ecs->stop_func_name,

> +      find_pc_partial_function (ecs->event_thread->suspend.stop_pc,

> +				&ecs->stop_func_name,

>  				&ecs->stop_func_start, &ecs->stop_func_end);

>        ecs->stop_func_start

>  	+= gdbarch_deprecated_function_start_offset (gdbarch);

> @@ -4937,7 +4939,8 @@ handle_inferior_event_1 (struct execution_control_state *ecs)

>  

>  	  ecs->event_thread->control.stop_bpstat

>  	    = bpstat_stop_status (regcache->aspace (),

> -				  stop_pc, ecs->event_thread, &ecs->ws);

> +				  ecs->event_thread->suspend.stop_pc,

> +				  ecs->event_thread, &ecs->ws);

>  

>  	  if (handle_stop_requested (ecs))

>  	    return;

> @@ -5183,11 +5186,13 @@ Cannot fill $_exitsignal with the correct signal number.\n"));

>  	 and not immediately.  */

>        ecs->event_thread->pending_follow = ecs->ws;

>  

> -      stop_pc = regcache_read_pc (get_thread_regcache (ecs->event_thread));

> +      ecs->event_thread->suspend.stop_pc

> +	= regcache_read_pc (get_thread_regcache (ecs->event_thread));

>  

>        ecs->event_thread->control.stop_bpstat

>  	= bpstat_stop_status (get_current_regcache ()->aspace (),

> -			      stop_pc, ecs->event_thread, &ecs->ws);

> +			      ecs->event_thread->suspend.stop_pc,

> +			      ecs->event_thread, &ecs->ws);

>  

>        if (handle_stop_requested (ecs))

>  	return;

> @@ -5289,16 +5294,18 @@ Cannot fill $_exitsignal with the correct signal number.\n"));

>           stop.  */

>        follow_exec (inferior_ptid, ecs->ws.value.execd_pathname);

>  

> -      stop_pc = regcache_read_pc (get_thread_regcache (ecs->event_thread));

> -

>        /* In follow_exec we may have deleted the original thread and

>  	 created a new one.  Make sure that the event thread is the

>  	 execd thread for that case (this is a nop otherwise).  */

>        ecs->event_thread = inferior_thread ();

>  

> +      ecs->event_thread->suspend.stop_pc

> +	= regcache_read_pc (get_thread_regcache (ecs->event_thread));

> +

>        ecs->event_thread->control.stop_bpstat

>  	= bpstat_stop_status (get_current_regcache ()->aspace (),

> -			      stop_pc, ecs->event_thread, &ecs->ws);

> +			      ecs->event_thread->suspend.stop_pc,

> +			      ecs->event_thread, &ecs->ws);

>  

>        /* Note that this may be referenced from inside

>  	 bpstat_stop_status above, through inferior_has_execd.  */

> @@ -5359,7 +5366,8 @@ Cannot fill $_exitsignal with the correct signal number.\n"));

>  	fprintf_unfiltered (gdb_stdlog, "infrun: stopped\n");

>  

>        delete_just_stopped_threads_single_step_breakpoints ();

> -      stop_pc = regcache_read_pc (get_thread_regcache (inferior_thread ()));

> +      ecs->event_thread->suspend.stop_pc

> +	= regcache_read_pc (get_thread_regcache (inferior_thread ()));

>  

>        if (handle_stop_requested (ecs))

>  	return;

> @@ -5651,7 +5659,8 @@ handle_signal_stop (struct execution_control_state *ecs)

>        && ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP)

>      ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_0;

>  

> -  stop_pc = regcache_read_pc (get_thread_regcache (ecs->event_thread));

> +  ecs->event_thread->suspend.stop_pc

> +    = regcache_read_pc (get_thread_regcache (ecs->event_thread));

>  

>    if (debug_infrun)

>      {

> @@ -5662,7 +5671,8 @@ handle_signal_stop (struct execution_control_state *ecs)

>        inferior_ptid = ecs->ptid;

>  

>        fprintf_unfiltered (gdb_stdlog, "infrun: stop_pc = %s\n",

> -                          paddress (gdbarch, stop_pc));

> +			  paddress (gdbarch,

> +				    ecs->event_thread->suspend.stop_pc));

>        if (target_stopped_by_watchpoint ())

>  	{

>            CORE_ADDR addr;

> @@ -5858,14 +5868,18 @@ handle_signal_stop (struct execution_control_state *ecs)

>  	 user had set a breakpoint on that inlined code, the missing

>  	 skip_inline_frames call would break things.  Fortunately

>  	 that's an extremely unlikely scenario.  */

> -      if (!pc_at_non_inline_function (aspace, stop_pc, &ecs->ws)

> +      if (!pc_at_non_inline_function (aspace,

> +				      ecs->event_thread->suspend.stop_pc,

> +				      &ecs->ws)

>  	  && !(ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP

>  	       && ecs->event_thread->control.trap_expected

>  	       && pc_at_non_inline_function (aspace,

>  					     ecs->event_thread->prev_pc,

>  					     &ecs->ws)))

>  	{

> -	  stop_chain = build_bpstat_chain (aspace, stop_pc, &ecs->ws);

> +	  stop_chain = build_bpstat_chain (aspace,

> +					   ecs->event_thread->suspend.stop_pc,

> +					   &ecs->ws);

>  	  skip_inline_frames (ecs->event_thread, stop_chain);

>  

>  	  /* Re-fetch current thread's frame in case that invalidated

> @@ -5915,7 +5929,8 @@ handle_signal_stop (struct execution_control_state *ecs)

>       handles this event.  */

>    ecs->event_thread->control.stop_bpstat

>      = bpstat_stop_status (get_current_regcache ()->aspace (),

> -			  stop_pc, ecs->event_thread, &ecs->ws, stop_chain);

> +			  ecs->event_thread->suspend.stop_pc,

> +			  ecs->event_thread, &ecs->ws, stop_chain);

>  

>    /* Following in case break condition called a

>       function.  */

> @@ -5967,7 +5982,8 @@ handle_signal_stop (struct execution_control_state *ecs)

>       been removed.  */

>    if (random_signal && target_stopped_by_sw_breakpoint ())

>      {

> -      if (program_breakpoint_here_p (gdbarch, stop_pc))

> +      if (program_breakpoint_here_p (gdbarch,

> +				     ecs->event_thread->suspend.stop_pc))

>  	{

>  	  struct regcache *regcache;

>  	  int decr_pc;

> @@ -5985,7 +6001,8 @@ handle_signal_stop (struct execution_control_state *ecs)

>  		restore_operation_disable.emplace

>  		  (record_full_gdb_operation_disable_set ());

>  

> -	      regcache_write_pc (regcache, stop_pc + decr_pc);

> +	      regcache_write_pc (regcache,

> +				 ecs->event_thread->suspend.stop_pc + decr_pc);

>  	    }

>  	}

>        else

> @@ -6077,7 +6094,7 @@ handle_signal_stop (struct execution_control_state *ecs)

>        if (signal_program[ecs->event_thread->suspend.stop_signal] == 0)

>  	ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_0;

>  

> -      if (ecs->event_thread->prev_pc == stop_pc

> +      if (ecs->event_thread->prev_pc == ecs->event_thread->suspend.stop_pc

>  	  && ecs->event_thread->control.trap_expected

>  	  && ecs->event_thread->control.step_resume_breakpoint == NULL)

>  	{

> @@ -6109,7 +6126,8 @@ handle_signal_stop (struct execution_control_state *ecs)

>  	}

>  

>        if (ecs->event_thread->suspend.stop_signal != GDB_SIGNAL_0

> -	  && (pc_in_thread_step_range (stop_pc, ecs->event_thread)

> +	  && (pc_in_thread_step_range (ecs->event_thread->suspend.stop_pc,

> +				       ecs->event_thread)

>  	      || ecs->event_thread->control.step_range_end == 1)

>  	  && frame_id_eq (get_stack_frame_id (frame),

>  			  ecs->event_thread->control.step_stack_frame_id)

> @@ -6339,7 +6357,7 @@ process_event_stop_test (struct execution_control_state *ecs)

>  	  return;

>  	}

>        fill_in_stop_func (gdbarch, ecs);

> -      if (stop_pc == ecs->stop_func_start

> +      if (ecs->event_thread->suspend.stop_pc == ecs->stop_func_start

>  	  && execution_direction == EXEC_REVERSE)

>  	{

>  	  /* We are stepping over a function call in reverse, and just

> @@ -6472,7 +6490,8 @@ process_event_stop_test (struct execution_control_state *ecs)

>       through a function epilogue and therefore must detect when

>       the current-frame changes in the middle of a line.  */

>  

> -  if (pc_in_thread_step_range (stop_pc, ecs->event_thread)

> +  if (pc_in_thread_step_range (ecs->event_thread->suspend.stop_pc,

> +			       ecs->event_thread)

>        && (execution_direction != EXEC_REVERSE

>  	  || frame_id_eq (get_frame_id (frame),

>  			  ecs->event_thread->control.step_frame_id)))

> @@ -6491,6 +6510,7 @@ process_event_stop_test (struct execution_control_state *ecs)

>        /* When stepping backward, stop at beginning of line range

>  	 (unless it's the function entry point, in which case

>  	 keep going back to the call point).  */

> +      CORE_ADDR stop_pc = ecs->event_thread->suspend.stop_pc;

>        if (stop_pc == ecs->event_thread->control.step_range_start

>  	  && stop_pc != ecs->stop_func_start

>  	  && execution_direction == EXEC_REVERSE)

> @@ -6517,10 +6537,11 @@ process_event_stop_test (struct execution_control_state *ecs)

>  

>    if (execution_direction != EXEC_REVERSE

>        && ecs->event_thread->control.step_over_calls == STEP_OVER_UNDEBUGGABLE

> -      && in_solib_dynsym_resolve_code (stop_pc))

> +      && in_solib_dynsym_resolve_code (ecs->event_thread->suspend.stop_pc))

>      {

>        CORE_ADDR pc_after_resolver =

> -	gdbarch_skip_solib_resolver (gdbarch, stop_pc);

> +	gdbarch_skip_solib_resolver (gdbarch,

> +				     ecs->event_thread->suspend.stop_pc);

>  

>        if (debug_infrun)

>  	 fprintf_unfiltered (gdb_stdlog,

> @@ -6544,7 +6565,8 @@ process_event_stop_test (struct execution_control_state *ecs)

>  

>    /* Step through an indirect branch thunk.  */

>    if (ecs->event_thread->control.step_over_calls != STEP_OVER_NONE

> -      && gdbarch_in_indirect_branch_thunk (gdbarch, stop_pc))

> +      && gdbarch_in_indirect_branch_thunk (gdbarch,

> +					   ecs->event_thread->suspend.stop_pc))

>      {

>        if (debug_infrun)

>  	 fprintf_unfiltered (gdb_stdlog,

> @@ -6576,13 +6598,14 @@ process_event_stop_test (struct execution_control_state *ecs)

>       call check below as on some targets return trampolines look

>       like subroutine calls (MIPS16 return thunks).  */

>    if (gdbarch_in_solib_return_trampoline (gdbarch,

> -					  stop_pc, ecs->stop_func_name)

> +					  ecs->event_thread->suspend.stop_pc,

> +					  ecs->stop_func_name)

>        && ecs->event_thread->control.step_over_calls != STEP_OVER_NONE)

>      {

>        /* Determine where this trampoline returns.  */

> -      CORE_ADDR real_stop_pc;

> -

> -      real_stop_pc = gdbarch_skip_trampoline_code (gdbarch, frame, stop_pc);

> +      CORE_ADDR stop_pc = ecs->event_thread->suspend.stop_pc;

> +      CORE_ADDR real_stop_pc

> +	= gdbarch_skip_trampoline_code (gdbarch, frame, stop_pc);

>  

>        if (debug_infrun)

>  	 fprintf_unfiltered (gdb_stdlog,

> @@ -6634,8 +6657,9 @@ process_event_stop_test (struct execution_control_state *ecs)

>  	  && (!frame_id_eq (ecs->event_thread->control.step_stack_frame_id,

>  			    outer_frame_id)

>  	      || (ecs->event_thread->control.step_start_function

> -		  != find_pc_function (stop_pc)))))

> +		  != find_pc_function (ecs->event_thread->suspend.stop_pc)))))

>      {

> +      CORE_ADDR stop_pc = ecs->event_thread->suspend.stop_pc;

>        CORE_ADDR real_stop_pc;

>  

>        if (debug_infrun)

> @@ -6793,6 +6817,8 @@ process_event_stop_test (struct execution_control_state *ecs)

>    if (execution_direction == EXEC_REVERSE

>        && ecs->event_thread->control.step_over_calls != STEP_OVER_NONE)

>      {

> +      CORE_ADDR stop_pc = ecs->event_thread->suspend.stop_pc;

> +

>        if (gdbarch_skip_trampoline_code (gdbarch, frame, stop_pc)

>  	  || (ecs->stop_func_start == 0

>  	      && in_solib_dynsym_resolve_code (stop_pc)))

> @@ -6820,7 +6846,7 @@ process_event_stop_test (struct execution_control_state *ecs)

>  	}

>      }

>  

> -  stop_pc_sal = find_pc_line (stop_pc, 0);

> +  stop_pc_sal = find_pc_line (ecs->event_thread->suspend.stop_pc, 0);

>  

>    /* NOTE: tausq/2004-05-24: This if block used to be done before all

>       the trampoline processing logic, however, there are some trampolines 

> @@ -6946,7 +6972,7 @@ process_event_stop_test (struct execution_control_state *ecs)

>        return;

>      }

>  

> -  if ((stop_pc == stop_pc_sal.pc)

> +  if ((ecs->event_thread->suspend.stop_pc == stop_pc_sal.pc)

>        && (ecs->event_thread->current_line != stop_pc_sal.line

>   	  || ecs->event_thread->current_symtab != stop_pc_sal.symtab))

>      {

> @@ -7180,7 +7206,7 @@ keep_going_stepped_thread (struct thread_info *tp)

>    reset_ecs (ecs, tp);

>    switch_to_thread (tp);

>  

> -  stop_pc = regcache_read_pc (get_thread_regcache (tp));

> +  tp->suspend.stop_pc = regcache_read_pc (get_thread_regcache (tp));

>    frame = get_current_frame ();

>  

>    /* If the PC of the thread we were trying to single-step has

> @@ -7196,7 +7222,7 @@ keep_going_stepped_thread (struct thread_info *tp)

>       This prevents us continuously moving the single-step breakpoint

>       forward, one instruction at a time, overstepping.  */

>  

> -  if (stop_pc != tp->prev_pc)

> +  if (tp->suspend.stop_pc != tp->prev_pc)

>      {

>        ptid_t resume_ptid;

>  

> @@ -7204,7 +7230,7 @@ keep_going_stepped_thread (struct thread_info *tp)

>  	fprintf_unfiltered (gdb_stdlog,

>  			    "infrun: expected thread advanced also (%s -> %s)\n",

>  			    paddress (target_gdbarch (), tp->prev_pc),

> -			    paddress (target_gdbarch (), stop_pc));

> +			    paddress (target_gdbarch (), tp->suspend.stop_pc));

>  

>        /* Clear the info of the previous step-over, as it's no longer

>  	 valid (if the thread was trying to step over a breakpoint, it

> @@ -7218,7 +7244,7 @@ keep_going_stepped_thread (struct thread_info *tp)

>  

>        insert_single_step_breakpoint (get_frame_arch (frame),

>  				     get_frame_address_space (frame),

> -				     stop_pc);

> +				     tp->suspend.stop_pc);

>  

>        tp->resumed = 1;

>        resume_ptid = internal_resume_ptid (tp->control.stepping_command);

> @@ -7259,7 +7285,8 @@ handle_step_into_function (struct gdbarch *gdbarch,

>  {

>    fill_in_stop_func (gdbarch, ecs);

>  

> -  compunit_symtab *cust = find_pc_compunit_symtab (stop_pc);

> +  compunit_symtab *cust

> +    = find_pc_compunit_symtab (ecs->event_thread->suspend.stop_pc);

>    if (cust != NULL && compunit_language (cust) != language_asm)

>      ecs->stop_func_start

>        = gdbarch_skip_prologue_noexcept (gdbarch, ecs->stop_func_start);

> @@ -7299,7 +7326,7 @@ handle_step_into_function (struct gdbarch *gdbarch,

>  					     ecs->stop_func_start);

>      }

>  

> -  if (ecs->stop_func_start == stop_pc)

> +  if (ecs->stop_func_start == ecs->event_thread->suspend.stop_pc)

>      {

>        /* We are already there: stop now.  */

>        end_stepping_range (ecs);

> @@ -7338,15 +7365,15 @@ handle_step_into_function_backward (struct gdbarch *gdbarch,

>  

>    fill_in_stop_func (gdbarch, ecs);

>  

> -  cust = find_pc_compunit_symtab (stop_pc);

> +  cust = find_pc_compunit_symtab (ecs->event_thread->suspend.stop_pc);

>    if (cust != NULL && compunit_language (cust) != language_asm)

>      ecs->stop_func_start

>        = gdbarch_skip_prologue_noexcept (gdbarch, ecs->stop_func_start);

>  

> -  stop_func_sal = find_pc_line (stop_pc, 0);

> +  stop_func_sal = find_pc_line (ecs->event_thread->suspend.stop_pc, 0);

>  

>    /* OK, we're just going to keep stepping here.  */

> -  if (stop_func_sal.pc == stop_pc)

> +  if (stop_func_sal.pc == ecs->event_thread->suspend.stop_pc)

>      {

>        /* We're there already.  Just stop stepping now.  */

>        end_stepping_range (ecs);

> @@ -7981,7 +8008,8 @@ print_stop_location (struct target_waitstatus *ws)

>        if (tp->control.stop_step

>  	  && frame_id_eq (tp->control.step_frame_id,

>  			  get_frame_id (get_current_frame ()))

> -	  && tp->control.step_start_function == find_pc_function (stop_pc))

> +	  && (tp->control.step_start_function

> +	      == find_pc_function (tp->suspend.stop_pc)))

>  	{

>  	  /* Finished step, just print source line.  */

>  	  source_flag = SRC_LINE;

> @@ -8785,7 +8813,6 @@ struct infcall_suspend_state

>    struct thread_suspend_state thread_suspend;

>  

>    /* Other fields:  */

> -  CORE_ADDR stop_pc;

>    readonly_detached_regcache *registers;

>  

>    /* Format of SIGINFO_DATA or NULL if it is not present.  */

> @@ -8840,8 +8867,6 @@ save_infcall_suspend_state (void)

>       GDB_SIGNAL_0 anyway.  */

>    tp->suspend.stop_signal = GDB_SIGNAL_0;

>  

> -  inf_state->stop_pc = stop_pc;

> -

>    inf_state->registers = new readonly_detached_regcache (*regcache);

>  

>    return inf_state;

> @@ -8858,8 +8883,6 @@ restore_infcall_suspend_state (struct infcall_suspend_state *inf_state)

>  

>    tp->suspend = inf_state->thread_suspend;

>  

> -  stop_pc = inf_state->stop_pc;

> -

>    if (inf_state->siginfo_gdbarch == gdbarch)

>      {

>        struct type *type = gdbarch_get_siginfo_type (gdbarch);

> diff --git a/gdb/linux-fork.c b/gdb/linux-fork.c

> index ae5a04b10d..80f9a3eb15 100644

> --- a/gdb/linux-fork.c

> +++ b/gdb/linux-fork.c

> @@ -267,7 +267,8 @@ fork_load_infrun_state (struct fork_info *fp)

>    registers_changed ();

>    reinit_frame_cache ();

>  

> -  stop_pc = regcache_read_pc (get_current_regcache ());

> +  inferior_thread ()->suspend.stop_pc

> +    = regcache_read_pc (get_current_regcache ());

>    nullify_last_target_wait_ptid ();

>  

>    /* Now restore the file positions of open file descriptors.  */

> diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c

> index 12d98d4987..b67d711619 100644

> --- a/gdb/record-btrace.c

> +++ b/gdb/record-btrace.c

> @@ -2805,7 +2805,8 @@ record_btrace_set_replay (struct thread_info *tp,

>    /* Start anew from the new replay position.  */

>    record_btrace_clear_histories (btinfo);

>  

> -  stop_pc = regcache_read_pc (get_current_regcache ());

> +  inferior_thread ()->suspend.stop_pc

> +    = regcache_read_pc (get_current_regcache ());

>    print_stack_frame (get_selected_frame (NULL), 1, SRC_AND_LOC, 1);

>  }

>  

> diff --git a/gdb/record-full.c b/gdb/record-full.c

> index 23bab315a6..4d8988db2a 100644

> --- a/gdb/record-full.c

> +++ b/gdb/record-full.c

> @@ -2011,7 +2011,8 @@ record_full_goto_entry (struct record_full_entry *p)

>  

>    registers_changed ();

>    reinit_frame_cache ();

> -  stop_pc = regcache_read_pc (get_current_regcache ());

> +  inferior_thread ()->suspend.stop_pc

> +    = regcache_read_pc (get_current_regcache ());

>    print_stack_frame (get_selected_frame (NULL), 1, SRC_AND_LOC, 1);

>  }

>  

> diff --git a/gdb/remote.c b/gdb/remote.c

> index 3b54715489..cf56a5636a 100644

> --- a/gdb/remote.c

> +++ b/gdb/remote.c

> @@ -4331,7 +4331,7 @@ print_one_stopped_thread (struct thread_info *thread)

>    struct target_waitstatus *ws = &thread->suspend.waitstatus;

>  

>    switch_to_thread (thread);

> -  stop_pc = get_frame_pc (get_current_frame ());

> +  thread->suspend.stop_pc = get_frame_pc (get_current_frame ());

>    set_current_sal_from_frame (get_current_frame ());

>  

>    thread->suspend.waitstatus_pending_p = 0;

> diff --git a/gdb/target.c b/gdb/target.c

> index a082957d5b..f560816a98 100644

> --- a/gdb/target.c

> +++ b/gdb/target.c

> @@ -2153,7 +2153,8 @@ target_resume (ptid_t ptid, int step, enum gdb_signal signal)

>  

>    registers_changed_ptid (ptid);

>    /* We only set the internal executing state here.  The user/frontend

> -     running state is set at a higher level.  */

> +     running state is set at a higher level.  This also clears the

> +     thread's stop_pc as side effect.  */

>    set_executing (ptid, 1);

>    clear_inline_frame_state (ptid);

>  }

> diff --git a/gdb/thread.c b/gdb/thread.c

> index 2b471cd292..77b497e281 100644

> --- a/gdb/thread.c

> +++ b/gdb/thread.c

> @@ -918,6 +918,18 @@ is_executing (ptid_t ptid)

>    return tp->executing;

>  }

>  

> +/* Helper for set_executing.  Set's the thread's 'executing' field

> +   from EXECUTING, and if EXECUTING is true also clears the thread's

> +   stop_pc.  */

> +

> +static void

> +set_executing_thread (thread_info *thr, bool executing)

> +{

> +  thr->executing = executing;

> +  if (executing)

> +    thr->suspend.stop_pc = ~(CORE_ADDR) 0;

> +}

> +

>  void

>  set_executing (ptid_t ptid, int executing)

>  {

> @@ -928,13 +940,13 @@ set_executing (ptid_t ptid, int executing)

>      {

>        for (tp = thread_list; tp; tp = tp->next)

>  	if (all || ptid_get_pid (tp->ptid) == ptid_get_pid (ptid))

> -	  tp->executing = executing;

> +	  set_executing_thread (tp, executing);

>      }

>    else

>      {

>        tp = find_thread_ptid (ptid);

>        gdb_assert (tp);

> -      tp->executing = executing;

> +      set_executing_thread (tp, executing);

>      }

>  

>    /* It only takes one running thread to spawn more threads.*/

> @@ -1328,7 +1340,6 @@ switch_to_thread_no_regs (struct thread_info *thread)

>    set_current_inferior (inf);

>  

>    inferior_ptid = thread->ptid;

> -  stop_pc = ~(CORE_ADDR) 0;

>  }

>  

>  /* See gdbthread.h.  */

> @@ -1341,7 +1352,6 @@ switch_to_no_thread ()

>  

>    inferior_ptid = null_ptid;

>    reinit_frame_cache ();

> -  stop_pc = ~(CORE_ADDR) 0;

>  }

>  

>  /* See gdbthread.h.  */

> @@ -1357,13 +1367,6 @@ switch_to_thread (thread_info *thr)

>    switch_to_thread_no_regs (thr);

>  

>    reinit_frame_cache ();

> -

> -  /* We don't check for is_stopped, because we're called at times

> -     while in the TARGET_RUNNING state, e.g., while handling an

> -     internal event.  */

> -  if (thr->state != THREAD_EXITED

> -      && !thr->executing)

> -    stop_pc = regcache_read_pc (get_thread_regcache (thr));

>  }

>  

>  /* See common/common-gdbthread.h.  */

> -- 

> 2.14.3


-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/
Pedro Alves June 28, 2018, 4:39 p.m. | #3
On 06/21/2018 05:55 PM, Pedro Alves wrote:
> In my multi-target work, I need to add a few more

> scoped_restore_current_thread and switch_to_thread calls in some

> places, and in some lower-level places I was fighting against the fact

> that switch_to_thread reads/refreshes the stop_pc global.

> 

> Instead of piling on workarounds, let's just finally eliminate the

> stop_pc global.  We already have the per-thread

> thread_info->suspend.stop_pc field, so it's mainly a matter of using

> that more/instead.


Thanks guys for the review.

I've pushed it in now.

Thanks,
Pedro Alves

Patch

diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index bd5ab9193f..983e0f71fa 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -168,10 +168,19 @@  struct thread_suspend_state
 
   /* Record the pc of the thread the last time it stopped.  (This is
      not the current thread's PC as that may have changed since the
-     last stop, e.g., "return" command, or "p $pc = 0xf000").  This is
-     used in coordination with stop_reason and waitstatus_pending_p:
-     if the thread's PC is changed since it last stopped, a pending
-     breakpoint waitstatus is discarded.  */
+     last stop, e.g., "return" command, or "p $pc = 0xf000").
+
+     - If the thread's PC has not changed since the thread last
+       stopped, then proceed skips a breakpoint at the current PC,
+       otherwise we let the thread run into the breakpoint.
+
+     - If the thread has an unprocessed event pending, as indicated by
+       waitstatus_pending_p, this is used in coordination with
+       stop_reason: if the thread's PC has changed since the thread
+       last stopped, a pending breakpoint waitstatus is discarded.
+
+     - If the thread is running, this is set to -1, to avoid leaving
+       it with a stale value, to make it easier to catch bugs.  */
   CORE_ADDR stop_pc;
 };
 
@@ -498,8 +507,7 @@  extern void switch_to_thread (thread_info *thr);
 /* Switch context to no thread selected.  */
 extern void switch_to_no_thread ();
 
-/* Switch from one thread to another.  Does not read registers and
-   sets STOP_PC to -1.  */
+/* Switch from one thread to another.  Does not read registers.  */
 extern void switch_to_thread_no_regs (struct thread_info *thread);
 
 /* Marks or clears thread(s) PTID as resumed.  If PTID is
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index c6fd9ab3bc..26d5a2aa95 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -94,10 +94,6 @@  static char *inferior_io_terminal_scratch;
 
 ptid_t inferior_ptid;
 
-/* Address at which inferior stopped.  */
-
-CORE_ADDR stop_pc;
-
 /* Nonzero if stopped due to completion of a stack dummy routine.  */
 
 enum stop_stack_kind stop_stack_dummy;
@@ -448,10 +444,12 @@  post_create_inferior (struct target_ops *target, int from_tty)
   /* Now that we know the register layout, retrieve current PC.  But
      if the PC is unavailable (e.g., we're opening a core file with
      missing registers info), ignore it.  */
-  stop_pc = 0;
+  thread_info *thr = inferior_thread ();
+
+  thr->suspend.stop_pc = 0;
   TRY
     {
-      stop_pc = regcache_read_pc (get_current_regcache ());
+      thr->suspend.stop_pc = regcache_read_pc (get_current_regcache ());
     }
   CATCH (ex, RETURN_MASK_ERROR)
     {
@@ -2108,7 +2106,7 @@  info_program_command (const char *args, int from_tty)
 
   target_files_info ();
   printf_filtered (_("Program stopped at %s.\n"),
-		   paddress (target_gdbarch (), stop_pc));
+		   paddress (target_gdbarch (), tp->suspend.stop_pc));
   if (tp->control.stop_step)
     printf_filtered (_("It stopped after being stepped.\n"));
   else if (stat != 0)
diff --git a/gdb/inferior.h b/gdb/inferior.h
index 3f4d7a50d6..70cef161cc 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -208,10 +208,6 @@  extern void prepare_execution_command (struct target_ops *target,
    the target is started up with a shell.  */
 extern int startup_with_shell;
 
-/* Address at which inferior stopped.  */
-
-extern CORE_ADDR stop_pc;
-
 /* Nonzero if stopped due to completion of a stack dummy routine.  */
 
 extern enum stop_stack_kind stop_stack_dummy;
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 9548f9cafd..28a4391c35 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -3009,7 +3009,7 @@  proceed (CORE_ADDR addr, enum gdb_signal siggnal)
 
   if (addr == (CORE_ADDR) -1)
     {
-      if (pc == stop_pc
+      if (pc == tp->suspend.stop_pc
 	  && breakpoint_here_p (aspace, pc) == ordinary_breakpoint_here
 	  && execution_direction != EXEC_REVERSE)
 	/* There is a breakpoint at the address we will resume at,
@@ -4249,7 +4249,7 @@  handle_syscall_event (struct execution_control_state *ecs)
 
   regcache = get_thread_regcache (ecs->event_thread);
   syscall_number = ecs->ws.value.syscall_number;
-  stop_pc = regcache_read_pc (regcache);
+  ecs->event_thread->suspend.stop_pc = regcache_read_pc (regcache);
 
   if (catch_syscall_enabled () > 0
       && catching_syscall_number (syscall_number) > 0)
@@ -4260,7 +4260,8 @@  handle_syscall_event (struct execution_control_state *ecs)
 
       ecs->event_thread->control.stop_bpstat
 	= bpstat_stop_status (regcache->aspace (),
-			      stop_pc, ecs->event_thread, &ecs->ws);
+			      ecs->event_thread->suspend.stop_pc,
+			      ecs->event_thread, &ecs->ws);
 
       if (handle_stop_requested (ecs))
 	return 0;
@@ -4290,7 +4291,8 @@  fill_in_stop_func (struct gdbarch *gdbarch,
     {
       /* Don't care about return value; stop_func_start and stop_func_name
 	 will both be 0 if it doesn't work.  */
-      find_pc_partial_function (stop_pc, &ecs->stop_func_name,
+      find_pc_partial_function (ecs->event_thread->suspend.stop_pc,
+				&ecs->stop_func_name,
 				&ecs->stop_func_start, &ecs->stop_func_end);
       ecs->stop_func_start
 	+= gdbarch_deprecated_function_start_offset (gdbarch);
@@ -4937,7 +4939,8 @@  handle_inferior_event_1 (struct execution_control_state *ecs)
 
 	  ecs->event_thread->control.stop_bpstat
 	    = bpstat_stop_status (regcache->aspace (),
-				  stop_pc, ecs->event_thread, &ecs->ws);
+				  ecs->event_thread->suspend.stop_pc,
+				  ecs->event_thread, &ecs->ws);
 
 	  if (handle_stop_requested (ecs))
 	    return;
@@ -5183,11 +5186,13 @@  Cannot fill $_exitsignal with the correct signal number.\n"));
 	 and not immediately.  */
       ecs->event_thread->pending_follow = ecs->ws;
 
-      stop_pc = regcache_read_pc (get_thread_regcache (ecs->event_thread));
+      ecs->event_thread->suspend.stop_pc
+	= regcache_read_pc (get_thread_regcache (ecs->event_thread));
 
       ecs->event_thread->control.stop_bpstat
 	= bpstat_stop_status (get_current_regcache ()->aspace (),
-			      stop_pc, ecs->event_thread, &ecs->ws);
+			      ecs->event_thread->suspend.stop_pc,
+			      ecs->event_thread, &ecs->ws);
 
       if (handle_stop_requested (ecs))
 	return;
@@ -5289,16 +5294,18 @@  Cannot fill $_exitsignal with the correct signal number.\n"));
          stop.  */
       follow_exec (inferior_ptid, ecs->ws.value.execd_pathname);
 
-      stop_pc = regcache_read_pc (get_thread_regcache (ecs->event_thread));
-
       /* In follow_exec we may have deleted the original thread and
 	 created a new one.  Make sure that the event thread is the
 	 execd thread for that case (this is a nop otherwise).  */
       ecs->event_thread = inferior_thread ();
 
+      ecs->event_thread->suspend.stop_pc
+	= regcache_read_pc (get_thread_regcache (ecs->event_thread));
+
       ecs->event_thread->control.stop_bpstat
 	= bpstat_stop_status (get_current_regcache ()->aspace (),
-			      stop_pc, ecs->event_thread, &ecs->ws);
+			      ecs->event_thread->suspend.stop_pc,
+			      ecs->event_thread, &ecs->ws);
 
       /* Note that this may be referenced from inside
 	 bpstat_stop_status above, through inferior_has_execd.  */
@@ -5359,7 +5366,8 @@  Cannot fill $_exitsignal with the correct signal number.\n"));
 	fprintf_unfiltered (gdb_stdlog, "infrun: stopped\n");
 
       delete_just_stopped_threads_single_step_breakpoints ();
-      stop_pc = regcache_read_pc (get_thread_regcache (inferior_thread ()));
+      ecs->event_thread->suspend.stop_pc
+	= regcache_read_pc (get_thread_regcache (inferior_thread ()));
 
       if (handle_stop_requested (ecs))
 	return;
@@ -5651,7 +5659,8 @@  handle_signal_stop (struct execution_control_state *ecs)
       && ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP)
     ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_0;
 
-  stop_pc = regcache_read_pc (get_thread_regcache (ecs->event_thread));
+  ecs->event_thread->suspend.stop_pc
+    = regcache_read_pc (get_thread_regcache (ecs->event_thread));
 
   if (debug_infrun)
     {
@@ -5662,7 +5671,8 @@  handle_signal_stop (struct execution_control_state *ecs)
       inferior_ptid = ecs->ptid;
 
       fprintf_unfiltered (gdb_stdlog, "infrun: stop_pc = %s\n",
-                          paddress (gdbarch, stop_pc));
+			  paddress (gdbarch,
+				    ecs->event_thread->suspend.stop_pc));
       if (target_stopped_by_watchpoint ())
 	{
           CORE_ADDR addr;
@@ -5858,14 +5868,18 @@  handle_signal_stop (struct execution_control_state *ecs)
 	 user had set a breakpoint on that inlined code, the missing
 	 skip_inline_frames call would break things.  Fortunately
 	 that's an extremely unlikely scenario.  */
-      if (!pc_at_non_inline_function (aspace, stop_pc, &ecs->ws)
+      if (!pc_at_non_inline_function (aspace,
+				      ecs->event_thread->suspend.stop_pc,
+				      &ecs->ws)
 	  && !(ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP
 	       && ecs->event_thread->control.trap_expected
 	       && pc_at_non_inline_function (aspace,
 					     ecs->event_thread->prev_pc,
 					     &ecs->ws)))
 	{
-	  stop_chain = build_bpstat_chain (aspace, stop_pc, &ecs->ws);
+	  stop_chain = build_bpstat_chain (aspace,
+					   ecs->event_thread->suspend.stop_pc,
+					   &ecs->ws);
 	  skip_inline_frames (ecs->event_thread, stop_chain);
 
 	  /* Re-fetch current thread's frame in case that invalidated
@@ -5915,7 +5929,8 @@  handle_signal_stop (struct execution_control_state *ecs)
      handles this event.  */
   ecs->event_thread->control.stop_bpstat
     = bpstat_stop_status (get_current_regcache ()->aspace (),
-			  stop_pc, ecs->event_thread, &ecs->ws, stop_chain);
+			  ecs->event_thread->suspend.stop_pc,
+			  ecs->event_thread, &ecs->ws, stop_chain);
 
   /* Following in case break condition called a
      function.  */
@@ -5967,7 +5982,8 @@  handle_signal_stop (struct execution_control_state *ecs)
      been removed.  */
   if (random_signal && target_stopped_by_sw_breakpoint ())
     {
-      if (program_breakpoint_here_p (gdbarch, stop_pc))
+      if (program_breakpoint_here_p (gdbarch,
+				     ecs->event_thread->suspend.stop_pc))
 	{
 	  struct regcache *regcache;
 	  int decr_pc;
@@ -5985,7 +6001,8 @@  handle_signal_stop (struct execution_control_state *ecs)
 		restore_operation_disable.emplace
 		  (record_full_gdb_operation_disable_set ());
 
-	      regcache_write_pc (regcache, stop_pc + decr_pc);
+	      regcache_write_pc (regcache,
+				 ecs->event_thread->suspend.stop_pc + decr_pc);
 	    }
 	}
       else
@@ -6077,7 +6094,7 @@  handle_signal_stop (struct execution_control_state *ecs)
       if (signal_program[ecs->event_thread->suspend.stop_signal] == 0)
 	ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_0;
 
-      if (ecs->event_thread->prev_pc == stop_pc
+      if (ecs->event_thread->prev_pc == ecs->event_thread->suspend.stop_pc
 	  && ecs->event_thread->control.trap_expected
 	  && ecs->event_thread->control.step_resume_breakpoint == NULL)
 	{
@@ -6109,7 +6126,8 @@  handle_signal_stop (struct execution_control_state *ecs)
 	}
 
       if (ecs->event_thread->suspend.stop_signal != GDB_SIGNAL_0
-	  && (pc_in_thread_step_range (stop_pc, ecs->event_thread)
+	  && (pc_in_thread_step_range (ecs->event_thread->suspend.stop_pc,
+				       ecs->event_thread)
 	      || ecs->event_thread->control.step_range_end == 1)
 	  && frame_id_eq (get_stack_frame_id (frame),
 			  ecs->event_thread->control.step_stack_frame_id)
@@ -6339,7 +6357,7 @@  process_event_stop_test (struct execution_control_state *ecs)
 	  return;
 	}
       fill_in_stop_func (gdbarch, ecs);
-      if (stop_pc == ecs->stop_func_start
+      if (ecs->event_thread->suspend.stop_pc == ecs->stop_func_start
 	  && execution_direction == EXEC_REVERSE)
 	{
 	  /* We are stepping over a function call in reverse, and just
@@ -6472,7 +6490,8 @@  process_event_stop_test (struct execution_control_state *ecs)
      through a function epilogue and therefore must detect when
      the current-frame changes in the middle of a line.  */
 
-  if (pc_in_thread_step_range (stop_pc, ecs->event_thread)
+  if (pc_in_thread_step_range (ecs->event_thread->suspend.stop_pc,
+			       ecs->event_thread)
       && (execution_direction != EXEC_REVERSE
 	  || frame_id_eq (get_frame_id (frame),
 			  ecs->event_thread->control.step_frame_id)))
@@ -6491,6 +6510,7 @@  process_event_stop_test (struct execution_control_state *ecs)
       /* When stepping backward, stop at beginning of line range
 	 (unless it's the function entry point, in which case
 	 keep going back to the call point).  */
+      CORE_ADDR stop_pc = ecs->event_thread->suspend.stop_pc;
       if (stop_pc == ecs->event_thread->control.step_range_start
 	  && stop_pc != ecs->stop_func_start
 	  && execution_direction == EXEC_REVERSE)
@@ -6517,10 +6537,11 @@  process_event_stop_test (struct execution_control_state *ecs)
 
   if (execution_direction != EXEC_REVERSE
       && ecs->event_thread->control.step_over_calls == STEP_OVER_UNDEBUGGABLE
-      && in_solib_dynsym_resolve_code (stop_pc))
+      && in_solib_dynsym_resolve_code (ecs->event_thread->suspend.stop_pc))
     {
       CORE_ADDR pc_after_resolver =
-	gdbarch_skip_solib_resolver (gdbarch, stop_pc);
+	gdbarch_skip_solib_resolver (gdbarch,
+				     ecs->event_thread->suspend.stop_pc);
 
       if (debug_infrun)
 	 fprintf_unfiltered (gdb_stdlog,
@@ -6544,7 +6565,8 @@  process_event_stop_test (struct execution_control_state *ecs)
 
   /* Step through an indirect branch thunk.  */
   if (ecs->event_thread->control.step_over_calls != STEP_OVER_NONE
-      && gdbarch_in_indirect_branch_thunk (gdbarch, stop_pc))
+      && gdbarch_in_indirect_branch_thunk (gdbarch,
+					   ecs->event_thread->suspend.stop_pc))
     {
       if (debug_infrun)
 	 fprintf_unfiltered (gdb_stdlog,
@@ -6576,13 +6598,14 @@  process_event_stop_test (struct execution_control_state *ecs)
      call check below as on some targets return trampolines look
      like subroutine calls (MIPS16 return thunks).  */
   if (gdbarch_in_solib_return_trampoline (gdbarch,
-					  stop_pc, ecs->stop_func_name)
+					  ecs->event_thread->suspend.stop_pc,
+					  ecs->stop_func_name)
       && ecs->event_thread->control.step_over_calls != STEP_OVER_NONE)
     {
       /* Determine where this trampoline returns.  */
-      CORE_ADDR real_stop_pc;
-
-      real_stop_pc = gdbarch_skip_trampoline_code (gdbarch, frame, stop_pc);
+      CORE_ADDR stop_pc = ecs->event_thread->suspend.stop_pc;
+      CORE_ADDR real_stop_pc
+	= gdbarch_skip_trampoline_code (gdbarch, frame, stop_pc);
 
       if (debug_infrun)
 	 fprintf_unfiltered (gdb_stdlog,
@@ -6634,8 +6657,9 @@  process_event_stop_test (struct execution_control_state *ecs)
 	  && (!frame_id_eq (ecs->event_thread->control.step_stack_frame_id,
 			    outer_frame_id)
 	      || (ecs->event_thread->control.step_start_function
-		  != find_pc_function (stop_pc)))))
+		  != find_pc_function (ecs->event_thread->suspend.stop_pc)))))
     {
+      CORE_ADDR stop_pc = ecs->event_thread->suspend.stop_pc;
       CORE_ADDR real_stop_pc;
 
       if (debug_infrun)
@@ -6793,6 +6817,8 @@  process_event_stop_test (struct execution_control_state *ecs)
   if (execution_direction == EXEC_REVERSE
       && ecs->event_thread->control.step_over_calls != STEP_OVER_NONE)
     {
+      CORE_ADDR stop_pc = ecs->event_thread->suspend.stop_pc;
+
       if (gdbarch_skip_trampoline_code (gdbarch, frame, stop_pc)
 	  || (ecs->stop_func_start == 0
 	      && in_solib_dynsym_resolve_code (stop_pc)))
@@ -6820,7 +6846,7 @@  process_event_stop_test (struct execution_control_state *ecs)
 	}
     }
 
-  stop_pc_sal = find_pc_line (stop_pc, 0);
+  stop_pc_sal = find_pc_line (ecs->event_thread->suspend.stop_pc, 0);
 
   /* NOTE: tausq/2004-05-24: This if block used to be done before all
      the trampoline processing logic, however, there are some trampolines 
@@ -6946,7 +6972,7 @@  process_event_stop_test (struct execution_control_state *ecs)
       return;
     }
 
-  if ((stop_pc == stop_pc_sal.pc)
+  if ((ecs->event_thread->suspend.stop_pc == stop_pc_sal.pc)
       && (ecs->event_thread->current_line != stop_pc_sal.line
  	  || ecs->event_thread->current_symtab != stop_pc_sal.symtab))
     {
@@ -7180,7 +7206,7 @@  keep_going_stepped_thread (struct thread_info *tp)
   reset_ecs (ecs, tp);
   switch_to_thread (tp);
 
-  stop_pc = regcache_read_pc (get_thread_regcache (tp));
+  tp->suspend.stop_pc = regcache_read_pc (get_thread_regcache (tp));
   frame = get_current_frame ();
 
   /* If the PC of the thread we were trying to single-step has
@@ -7196,7 +7222,7 @@  keep_going_stepped_thread (struct thread_info *tp)
      This prevents us continuously moving the single-step breakpoint
      forward, one instruction at a time, overstepping.  */
 
-  if (stop_pc != tp->prev_pc)
+  if (tp->suspend.stop_pc != tp->prev_pc)
     {
       ptid_t resume_ptid;
 
@@ -7204,7 +7230,7 @@  keep_going_stepped_thread (struct thread_info *tp)
 	fprintf_unfiltered (gdb_stdlog,
 			    "infrun: expected thread advanced also (%s -> %s)\n",
 			    paddress (target_gdbarch (), tp->prev_pc),
-			    paddress (target_gdbarch (), stop_pc));
+			    paddress (target_gdbarch (), tp->suspend.stop_pc));
 
       /* Clear the info of the previous step-over, as it's no longer
 	 valid (if the thread was trying to step over a breakpoint, it
@@ -7218,7 +7244,7 @@  keep_going_stepped_thread (struct thread_info *tp)
 
       insert_single_step_breakpoint (get_frame_arch (frame),
 				     get_frame_address_space (frame),
-				     stop_pc);
+				     tp->suspend.stop_pc);
 
       tp->resumed = 1;
       resume_ptid = internal_resume_ptid (tp->control.stepping_command);
@@ -7259,7 +7285,8 @@  handle_step_into_function (struct gdbarch *gdbarch,
 {
   fill_in_stop_func (gdbarch, ecs);
 
-  compunit_symtab *cust = find_pc_compunit_symtab (stop_pc);
+  compunit_symtab *cust
+    = find_pc_compunit_symtab (ecs->event_thread->suspend.stop_pc);
   if (cust != NULL && compunit_language (cust) != language_asm)
     ecs->stop_func_start
       = gdbarch_skip_prologue_noexcept (gdbarch, ecs->stop_func_start);
@@ -7299,7 +7326,7 @@  handle_step_into_function (struct gdbarch *gdbarch,
 					     ecs->stop_func_start);
     }
 
-  if (ecs->stop_func_start == stop_pc)
+  if (ecs->stop_func_start == ecs->event_thread->suspend.stop_pc)
     {
       /* We are already there: stop now.  */
       end_stepping_range (ecs);
@@ -7338,15 +7365,15 @@  handle_step_into_function_backward (struct gdbarch *gdbarch,
 
   fill_in_stop_func (gdbarch, ecs);
 
-  cust = find_pc_compunit_symtab (stop_pc);
+  cust = find_pc_compunit_symtab (ecs->event_thread->suspend.stop_pc);
   if (cust != NULL && compunit_language (cust) != language_asm)
     ecs->stop_func_start
       = gdbarch_skip_prologue_noexcept (gdbarch, ecs->stop_func_start);
 
-  stop_func_sal = find_pc_line (stop_pc, 0);
+  stop_func_sal = find_pc_line (ecs->event_thread->suspend.stop_pc, 0);
 
   /* OK, we're just going to keep stepping here.  */
-  if (stop_func_sal.pc == stop_pc)
+  if (stop_func_sal.pc == ecs->event_thread->suspend.stop_pc)
     {
       /* We're there already.  Just stop stepping now.  */
       end_stepping_range (ecs);
@@ -7981,7 +8008,8 @@  print_stop_location (struct target_waitstatus *ws)
       if (tp->control.stop_step
 	  && frame_id_eq (tp->control.step_frame_id,
 			  get_frame_id (get_current_frame ()))
-	  && tp->control.step_start_function == find_pc_function (stop_pc))
+	  && (tp->control.step_start_function
+	      == find_pc_function (tp->suspend.stop_pc)))
 	{
 	  /* Finished step, just print source line.  */
 	  source_flag = SRC_LINE;
@@ -8785,7 +8813,6 @@  struct infcall_suspend_state
   struct thread_suspend_state thread_suspend;
 
   /* Other fields:  */
-  CORE_ADDR stop_pc;
   readonly_detached_regcache *registers;
 
   /* Format of SIGINFO_DATA or NULL if it is not present.  */
@@ -8840,8 +8867,6 @@  save_infcall_suspend_state (void)
      GDB_SIGNAL_0 anyway.  */
   tp->suspend.stop_signal = GDB_SIGNAL_0;
 
-  inf_state->stop_pc = stop_pc;
-
   inf_state->registers = new readonly_detached_regcache (*regcache);
 
   return inf_state;
@@ -8858,8 +8883,6 @@  restore_infcall_suspend_state (struct infcall_suspend_state *inf_state)
 
   tp->suspend = inf_state->thread_suspend;
 
-  stop_pc = inf_state->stop_pc;
-
   if (inf_state->siginfo_gdbarch == gdbarch)
     {
       struct type *type = gdbarch_get_siginfo_type (gdbarch);
diff --git a/gdb/linux-fork.c b/gdb/linux-fork.c
index ae5a04b10d..80f9a3eb15 100644
--- a/gdb/linux-fork.c
+++ b/gdb/linux-fork.c
@@ -267,7 +267,8 @@  fork_load_infrun_state (struct fork_info *fp)
   registers_changed ();
   reinit_frame_cache ();
 
-  stop_pc = regcache_read_pc (get_current_regcache ());
+  inferior_thread ()->suspend.stop_pc
+    = regcache_read_pc (get_current_regcache ());
   nullify_last_target_wait_ptid ();
 
   /* Now restore the file positions of open file descriptors.  */
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 12d98d4987..b67d711619 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -2805,7 +2805,8 @@  record_btrace_set_replay (struct thread_info *tp,
   /* Start anew from the new replay position.  */
   record_btrace_clear_histories (btinfo);
 
-  stop_pc = regcache_read_pc (get_current_regcache ());
+  inferior_thread ()->suspend.stop_pc
+    = regcache_read_pc (get_current_regcache ());
   print_stack_frame (get_selected_frame (NULL), 1, SRC_AND_LOC, 1);
 }
 
diff --git a/gdb/record-full.c b/gdb/record-full.c
index 23bab315a6..4d8988db2a 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -2011,7 +2011,8 @@  record_full_goto_entry (struct record_full_entry *p)
 
   registers_changed ();
   reinit_frame_cache ();
-  stop_pc = regcache_read_pc (get_current_regcache ());
+  inferior_thread ()->suspend.stop_pc
+    = regcache_read_pc (get_current_regcache ());
   print_stack_frame (get_selected_frame (NULL), 1, SRC_AND_LOC, 1);
 }
 
diff --git a/gdb/remote.c b/gdb/remote.c
index 3b54715489..cf56a5636a 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -4331,7 +4331,7 @@  print_one_stopped_thread (struct thread_info *thread)
   struct target_waitstatus *ws = &thread->suspend.waitstatus;
 
   switch_to_thread (thread);
-  stop_pc = get_frame_pc (get_current_frame ());
+  thread->suspend.stop_pc = get_frame_pc (get_current_frame ());
   set_current_sal_from_frame (get_current_frame ());
 
   thread->suspend.waitstatus_pending_p = 0;
diff --git a/gdb/target.c b/gdb/target.c
index a082957d5b..f560816a98 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -2153,7 +2153,8 @@  target_resume (ptid_t ptid, int step, enum gdb_signal signal)
 
   registers_changed_ptid (ptid);
   /* We only set the internal executing state here.  The user/frontend
-     running state is set at a higher level.  */
+     running state is set at a higher level.  This also clears the
+     thread's stop_pc as side effect.  */
   set_executing (ptid, 1);
   clear_inline_frame_state (ptid);
 }
diff --git a/gdb/thread.c b/gdb/thread.c
index 2b471cd292..77b497e281 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -918,6 +918,18 @@  is_executing (ptid_t ptid)
   return tp->executing;
 }
 
+/* Helper for set_executing.  Set's the thread's 'executing' field
+   from EXECUTING, and if EXECUTING is true also clears the thread's
+   stop_pc.  */
+
+static void
+set_executing_thread (thread_info *thr, bool executing)
+{
+  thr->executing = executing;
+  if (executing)
+    thr->suspend.stop_pc = ~(CORE_ADDR) 0;
+}
+
 void
 set_executing (ptid_t ptid, int executing)
 {
@@ -928,13 +940,13 @@  set_executing (ptid_t ptid, int executing)
     {
       for (tp = thread_list; tp; tp = tp->next)
 	if (all || ptid_get_pid (tp->ptid) == ptid_get_pid (ptid))
-	  tp->executing = executing;
+	  set_executing_thread (tp, executing);
     }
   else
     {
       tp = find_thread_ptid (ptid);
       gdb_assert (tp);
-      tp->executing = executing;
+      set_executing_thread (tp, executing);
     }
 
   /* It only takes one running thread to spawn more threads.*/
@@ -1328,7 +1340,6 @@  switch_to_thread_no_regs (struct thread_info *thread)
   set_current_inferior (inf);
 
   inferior_ptid = thread->ptid;
-  stop_pc = ~(CORE_ADDR) 0;
 }
 
 /* See gdbthread.h.  */
@@ -1341,7 +1352,6 @@  switch_to_no_thread ()
 
   inferior_ptid = null_ptid;
   reinit_frame_cache ();
-  stop_pc = ~(CORE_ADDR) 0;
 }
 
 /* See gdbthread.h.  */
@@ -1357,13 +1367,6 @@  switch_to_thread (thread_info *thr)
   switch_to_thread_no_regs (thr);
 
   reinit_frame_cache ();
-
-  /* We don't check for is_stopped, because we're called at times
-     while in the TARGET_RUNNING state, e.g., while handling an
-     internal event.  */
-  if (thr->state != THREAD_EXITED
-      && !thr->executing)
-    stop_pc = regcache_read_pc (get_thread_regcache (thr));
 }
 
 /* See common/common-gdbthread.h.  */