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

Yao Qi
2016-06-07T15:25:35Z

Pedro Alves <palves@redhat.com> writes:

> How do you plan on handling remote targets though? Done that way, it
> sounds to me like the alignment restrictions should either be a gdbarch
> property, or you need some RSP extension, e.g., extend the "watch" stop
> reply to indicate an stop data address range instead of a sole address,
> or make the stub report back the alignment restriction when GDB tells it
> to insert the watchpoint in the first place, instead of just saying
> "OK".
>

I want to use a gdbarch method to decide whether a watchpoint is
triggered in remote target, like this, [note that all the code below is
not compiled at all]

static enum watchpoint_triggered
remote_watchpoint_triggered (struct target_ops *target, struct watchpoint *w)
{
struct thread_info *thread = inferior_thread ();

if (thread->priv != NULL
&& thread->priv->stop_reason == TARGET_STOPPED_BY_WATCHPOINT)
{
return gdbarch_watchpoint_triggered (w->base.gdbarch, w,
thread->priv->watch_data_address);
}

return watchpoint_triggered_unknown;
}

The default implementation of gdbarch_watchpoint_triggered returns
watchpoint_triggered_yes if address is in the range of
loc->address,+loc->length. Otherwise return watchpoint_triggered_no.
Then, we can rewrite breakpoint.c:watchpoints_triggered like this,

int
watchpoints_triggered (void)
{
int ret = 0;

ALL_BREAKPOINTS (b)
if (is_hardware_watchpoint (b))
{
struct watchpoint *w = (struct watchpoint *) b;

w->watchpoint_triggered = target_watchpoint_triggered (w);

if (w->watchpoint_triggered != watch_triggered_no)
ret = 1;
}

return ret;
}

> 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.

> 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. 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,

static enum watchpoint_triggered
aarch64_linux_watchpoint_triggered (struct gdbarch *gdbarch,
struct watchpoint *w,
CORE_ADDR addr)
{
struct bp_location *loc;

for (loc = w->base.loc; loc; loc = loc->next)
{
if (addr >= loc->address && addr < loc->address + loc->length)
{
/* If the address reported by the target falls in the memory
range that watchpoint W is monitoring, the watchpoint is
definitely hit. */
return watchpoint_triggered_yes;
}
else if (addr >= align_down (loc->address, 8) && addr < loc->address)
{
/* The debugging stub, like GDBserver, may adjust the address
to meet kernel's alignment requirements (8-aligned for
example), we are not sure watchpoint W is hit or not. */
return watchpoint_triggered_unknown;
}
}

return watchpoint_triggered_no;
}

--
Yao (齐尧)