gdb requires watchpoints to fire after the write

[Tim Newsome]

Thanks everybody. That's really helpful.

Tim

On Wed, Aug 29, 2018 at 10:29 AM, Pedro Alves <palves@redhat.com> wrote:

> On 08/29/2018 05:02 PM, Simon Marchi wrote:
>
> > I'm just confused by this condition:
> >
> >   if (stopped_by_watchpoint
> >       && (target_have_steppable_watchpoint
> >       || gdbarch_have_nonsteppable_watchpoint (gdbarch)))
> >
> > I don't understand why we check for target_have_steppable_watchpoint OR
> gdbarch_have_nonsteppable_watchpoint, they seem to mean opposite things.
>
> Yeah, it's confusing.
>
> GDB's current "model" is that there are three "kinds" of watchpoints,
> wrt to when they trigger and how you can move past them.
>
> Those are: continuable, steppable, and non-steppable.
>
> Continuable watchpoints are like x86's -- those trigger after
> the memory access's side effects are fully committed to memory.
> I.e., they trap with the PC pointing at the next instruction
> already.  Continuing past such a watchpoint is doable
> by just normally continuing, hence the name.
>
> Both steppable and nonsteppable watchpoints trap before
> the memory access.  I.e, the PC points at the instruction that
> is accessing the memory.  So GDB needs to single-step once past
> the current instruction in order to make the access effective
> and check whether the instruction's side effects change the
> watched expression.
>
> Now, in order to step past that instruction, depending on
> architecture, you can have two situations:
>
> - steppable watchpoints: you can single-step with the watchpoint still
>   armed, and the watchpoint won't trigger again.
>
> - non-steppable watchpoints: if you try to single-step with the watchpoint
>   still armed, you'd trap the watchpoint again and the thread wouldn't
>   make any progress.  So GDB needs to temporarily remove the watchpoint
>   in order to step past it.
>
> So that's why we have all of target_have_continuable_watchpoint,
> target_have_steppable_watchpoint and gdbarch_have_nonsteppable_watchpoint.
>
> Now, the main oddity is that from the definitions above,
> we can tell that "continuable" is the same as "!steppable &&
> !nonsteppable",
> which makes target_continuable_watchpoint redundant.
>
> Some targets do set "have_continuable_watchpoint" (like x86
> in x86-nat.h), but it doesn't seem like target_have_continuable_watchpoint
> is checked anywhere nowadays.
>
> The target_have_steppable_watchpoint property is only set by ia64
> GNU/Linux
> nowadays:
>
>   /* The IA-64 architecture can step over a watch point (without
>      triggering it again) if the "dd" (data debug fault disable) bit
>      in the processor status word is set.
>
>      This PSR bit is set in
>      ia64_linux_nat_target::stopped_by_watchpoint when the code there
>      has determined that a hardware watchpoint has indeed been hit.
>      The CPU will then be able to execute one instruction without
>      triggering a watchpoint.  */
>   bool have_steppable_watchpoint () { return 1; }
>
> There's of course also the oddity that target_have_continuable_watchpoint
> and target_have_steppable_watchpoint are target methods, while
> gdbarch_have_nonsteppable_watchpoint is a gdbarch method...
>
> We could most probably streamline all of this and come up with a better
> design with some thought.  See also the comment in mips-tdep.c:
>
>   /* FIXME: cagney/2003-08-29: The macros target_have_steppable_
> watchpoint,
>      HAVE_NONSTEPPABLE_WATCHPOINT, and target_have_continuable_watchpoint
>      need to all be folded into the target vector.  Since they are
>      being used as guards for target_stopped_by_watchpoint, why not have
>      target_stopped_by_watchpoint return the type of watchpoint that the
> code
>      is sitting on?  */
>   set_gdbarch_have_nonsteppable_watchpoint (gdbarch, 1);
>
> Thanks,
> Pedro Alves
>