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

Pedro Alves
2016-04-19T10:43:42Z

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