[PATCH] Fix displaced stepping watchpoint check order

[Luis Machado]

On 8/19/21 3:23 PM, Simon Marchi wrote:
>> Although hardware watchpoints are usually per-thread, I don't think GDB handles it in that level of detail. It seems to mostly ignore insertion of hardware watchpoints if we're trying to step over a PC that caused a hardware watchpoint trigger.
>>
>> See, for example, breakpoint.c:should_be_inserted
>>
>>    /* Don't insert watchpoints if we're trying to step past the
>>       instruction that triggered one.  */
>>    if ((bl->loc_type == bp_loc_hardware_watchpoint)
>>        && stepping_past_nonsteppable_watchpoint ())
>>      {
>>        infrun_debug_printf ("stepping past non-steppable watchpoint. "
>>                             "skipping watchpoint at %s:%d\n",
>>                             paddress (bl->gdbarch, bl->address), bl->length);
>>        return 0;
>>      }
>>
>>  From what I recall, GDB replicates the hardware watchpoints to all threads, so disabling all hardware watchpoints for all threads (actually not inserting them) makes sense. GDB doesn't try to add thread-specific hardware watchpoints at the moment.
>>
>> Having per-thread hardware watchpoints might be desirable in some cases, but it may complicate the user interaction a bit.
>>
>> I haven't investigated if GDB, during a step-over operation, keeps threads running with hardware watchpoints disabled. If it doesn't handle it correctly, we may miss hardware watchpoints hits, unless GDB serializes the step-overs.
> 
> That's where I was going too.  Just imagine in non-stop, you have many
> threads running, one hits the watchpoint.  You then want to resume that
> thread, that requires stepping over the watchpoint while other threads
> are running.  If the watchpoint was really removed for all running
> threads, a running thread could miss it.
> 
> But watchpoints aren't like memory breakpoints, they are usually set
> using debug register, which are per-thread, so you can't really remove a
> watchpoint from a running thread... that's why I was thinking that maybe
> the watchpoint was only removed for the thread stepping over the
> watchpoint.  But maybe not.

That's true. It might be the case that GDB is only removing the hardware 
watchpoint from the stopped thread. And it does so through context, by 
having selected a particular thread explicitly at some point. As opposed 
to commanding the inferior to remove hardware watchpoints from all threads.

I'm pretty sure we'd get a ptrace error otherwise.

> 
>> "Presently GDB can only step over one breakpoint at any given time."
> 
> There can be only one in-line step at a time.  But there can be multiple
> displaced steps at a time (since somewhat recently).  There can't be an
> in-line step at the same time as some displaced steps.  While an in-line
> step is in progress, it is guaranteed that there are no other running
> threads.
> 
> Reading keep_going_pass_signal, I might understand that watchpoints are
> always stepped using the in-line mechanism, while all other threads are
> stopped.  I would have to try it.

That makes sense.

> 
> Simon
>