Re: gdb requires watchpoints to fire after the write

[Pedro Alves <palves at redhat dot com>]

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