Re: [patch] aarch64: PR 19806: watchpoints: false negatives -> false positives

[Pedro Alves <palves@redhat.com>]

On 06/07/2016 04:25 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:

>> A gdbarch method poses problems for remote stubs that are actually emulators,
>> and thus can support hardware watchpoints without these restrictions.
> 
> Then, the address reported by the target should fall in the range of
> loc->address,+loc->length.

Isn't that what Jan's patch did?

In any case, with the gdbarch method, if the target supports watching all
kinds of unaligned addresses/ranges, and reports the correct address in
"T05 watch" stop replies, it then faces the problem that gdb has hardcoded
knowledge of watch region alignment restrictions, and then e.g., watchpoints
on consecutive addresses are misinterpreted, even though there's
no reason for that.

E.g., say gdb believes the machine only supports watching 32-bit-aligned
words.  Then with:

union
{
  char buf[4];
  uint32_t force_align;
} global;

(gdb) watch global.buf[1];
Hardware watchpoint 1 ...
(gdb) watch global.buf[3];
Hardware watchpoint 2 ...

... if the program writes to global.buf[1], and the target reports
a memory access to 'global.buf + 1', gdb will believe that
watchpoint 2 _also_ triggered, when it did not.  That's a false positive you
can't help with with real machines, but there's no reason a
simulator/emulator has to suffer from that.

> 
>> I think it's actually problematic for real machines, as the restrictions
>> will often depend on process revisions/models.  So a gdbarch approach
>> would be undesirable, IMO.
> 
> On the real machine, nowadays, the restriction is that address must be
> 8-byte-aligned on linux. 

I think we need to consider all architectures, and the design going
forward, not just Aarch64.  For example, PPC has:

static int
ppc_linux_watchpoint_addr_within_range (struct target_ops *target,
					CORE_ADDR addr,
					CORE_ADDR start, int length)
{
  int mask;

  if (have_ptrace_hwdebug_interface ()
      && ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
    return start <= addr && start + length >= addr;
  else if (ppc_linux_get_hwcap () & PPC_FEATURE_BOOKE)
    mask = 3;
  else
    mask = 7;

So e.g., here, the alignment restrictions depend on both
the processor model and kernel.

(I'll guess that other embedded architectures that gdb supports probably
have similar restrictions that gdb was never taught about.)

> The restriction can only be relaxed and
> may be removed finally in the future, IOW, the restriction won't become
> 16-byte aligned, so we can write the gdbarch method for aarch64-linux
> like this,

How can gdb determine whether the restriction has been lifted?
The way to do it is probably either by checking kernel version or having the
ptrace code that "inserts" the watchpoint to first try watching the unaligned
region exactly as gdb requested, and if that doesn't work, try a wider,
aligned region.  Only that target-side ptrace code is aware of these finer details
and the correct restrictions in effect for the running system.  If we put this
in a gdbarch method, how can the gdbarch method maintain compatibility with
older kernels and at the same time reflect that newer kernels no longer
impose the restriction?

It's actually not just simulators/emulators that can have different
watchpoint restrictions from the machine architecture's debug hardware
limitations.  This is in good part a debug API issue as well --
a target may well support watchpoints implemented in a totally different
way -- for example, I believe Solaris supports "unlimited" watchpoints and
address ranges by implementing watchpoints not by using debug registers, but
instead by changing memory page protections and trapping faults internally,
all invisibly to userspace.

All this is why I believe that hardcoding this knowledge in gdb, which is
what a gdbarch method does, is not the best approach. 

Thanks,
Pedro Alves