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

[Pedro Alves <palves at redhat dot com>]

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