[03/17] Displaced stepping debug: fetch the right regcache

  Although not currently possible in practice when we get here,
'resume_ptid' can also be a wildcard throughout this function.  It's
clearer to fetch the regcache using the thread's ptid.

gdb/ChangeLog:
2015-04-01  Pedro Alves  <pedro@codesourcery.com>

	* infrun.c (resume) <displaced stepping debug output>: Get the
	leader thread's regcache, not resume_ptid's.
---
 gdb/infrun.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

On 04/01/2015 11:14 PM, Pedro Alves wrote:
> Although not currently possible in practice when we get here,
> 'resume_ptid' can also be a wildcard throughout this function.  It's
> clearer to fetch the regcache using the thread's ptid.
> 
> gdb/ChangeLog:
> 2015-04-01  Pedro Alves  <pedro@codesourcery.com>
> 
> 	* infrun.c (resume) <displaced stepping debug output>: Get the
> 	leader thread's regcache, not resume_ptid's.

Not much point in carrying this one around.  I pushed it in.

Thanks,
Pedro Alves
  

Pedro Alves <palves@redhat.com> writes:

> gdb/ChangeLog:
> 2015-04-01  Pedro Alves  <pedro@codesourcery.com>
>
> 	* infrun.c (resume) <displaced stepping debug output>: Get the
> 	leader thread's regcache, not resume_ptid's.

Hi Pedro,
From your change, I don't see why TP is the leader thread.

> ---
>  gdb/infrun.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index cf0ef32..f23e76e 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -2374,7 +2374,7 @@ resume (enum gdb_signal sig)
>        && tp->control.trap_expected
>        && use_displaced_stepping_now_p (gdbarch, sig))
>      {
> -      struct regcache *resume_regcache = get_thread_regcache (resume_ptid);
> +      struct regcache *resume_regcache = get_thread_regcache (tp->ptid);
>        struct gdbarch *resume_gdbarch = get_regcache_arch (resume_regcache);
>        CORE_ADDR actual_pc = regcache_read_pc (resume_regcache);

If we get recache from TP, can we remove local variables
resume_regcache, resume_gdbarch, actual_pc, and use regcache, gdbarch
and pc we've got at the beginning of function resume instead?
  

On 04/07/2015 02:55 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
>> gdb/ChangeLog:
>> 2015-04-01  Pedro Alves  <pedro@codesourcery.com>
>>
>> 	* infrun.c (resume) <displaced stepping debug output>: Get the
>> 	leader thread's regcache, not resume_ptid's.
> 
> Hi Pedro,
> From your change, I don't see why TP is the leader thread.

'resume' and 'target_resume' assume inferior_ptid is the "leader"
thread we want resumed -- TP is just the current thread
initialized at the top:

  struct thread_info *tp = inferior_thread ();

But e.g., if we're resuming with "set scheduler-locking off",
this:

  resume_ptid = user_visible_resume_ptid (user_step);

makes resume_ptid be a whole-process ptid, or
minus_one_ptid (all-threads).  In that case, the target_resume
backend implementation knows the thread that is the "leader" is
inferior_ptid, not the one passed as argument.

(though when we start a displaced step, resume_ptid is always
pointing at the current thread, never a wildcard ptid.)

>> @@ -2374,7 +2374,7 @@ resume (enum gdb_signal sig)
>>        && tp->control.trap_expected
>>        && use_displaced_stepping_now_p (gdbarch, sig))
>>      {
>> -      struct regcache *resume_regcache = get_thread_regcache (resume_ptid);
>> +      struct regcache *resume_regcache = get_thread_regcache (tp->ptid);
>>        struct gdbarch *resume_gdbarch = get_regcache_arch (resume_regcache);
>>        CORE_ADDR actual_pc = regcache_read_pc (resume_regcache);
> 
> If we get recache from TP, can we remove local variables
> resume_regcache, resume_gdbarch, actual_pc, and use regcache, gdbarch
> and pc we've got at the beginning of function resume instead?

Good point.  Indeed we should be able to.  The PC we got at the top would
be incorrect, given the thread's PC now points at the scratch pad,
but we already do:

	  /* Update pc to reflect the new address from which we will
	     execute instructions due to displaced stepping.  */
	  pc = regcache_read_pc (get_thread_regcache (inferior_ptid));

so the current PC's contents should work.  And that line could be:

  pc = regcache_read_pc (regcache);

too.

Thanks,
Pedro Alves