Re: [PATCH 2/2] Replace address and aspace with thread in struct step_over_info

[Pedro Alves <palves@redhat.com>]

On 04/15/2016 02:29 PM, Yao Qi wrote:
> This patch replaces the fields aspace and address in
> 'struct step_over_info' with 'thread', because aspace and thread can
> be got from thread.
> 

>  
> @@ -1366,10 +1350,21 @@ int
>  stepping_past_instruction_at (struct address_space *aspace,
>  			      CORE_ADDR address)
>  {
> -  return (step_over_info.aspace != NULL
> -	  && breakpoint_address_match (aspace, address,
> -				       step_over_info.aspace,
> -				       step_over_info.address));
> +  if (step_over_info.thread != NULL)
> +    {
> +      struct regcache *regcache;
> +
> +      regcache = get_thread_regcache (step_over_info.thread->ptid);
> +
> +      /* The step-over isn't finished or is still valid, so the PC got
> +	 from regcache is the value when thread stops, rather than the
> +	 value after step-over.  */

I think this is problematic.

While a thread is being stepped past a breakpoint, it's possible that the
user sets some other breakpoint, and then we end up in stepping_past_instruction_at
deciding whether we can insert that new breakpoint, while the step-over thread
is running.

As soon as the step-over thread is resumed for the actual step-over, it's
regcache is flushed (target_resume -> registers_changed_ptid).  From that point
and until the thread stops again, trying to fetch its regcache will error out,
because you can't read registers from a thread that is running.

Example (haven't tried it):

- A program with two threads, thread 1 and thread 2.

- non-stop mode on.

- Thread 1 continuously stepping over this:

  while (1) i++;     << breakpoint here:

  E.g., with:

  (gdb) thread 1
  (gdb) b $breakpoint_here_line
  (gdb) n&

- Switch to thread 2, which is stopped elsewhere (so inserting
  a breakpoint works when native debugging), and set some breakpoint:

  (gdb) thread 2
  (gdb) b foo

Thanks,
Pedro Alves