This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] RISC-V: enable have_nonsteppable_watchpoint by default
- From: Pedro Alves <palves at redhat dot com>
- To: Andrew Burgess <andrew dot burgess at embecosm dot com>
- Cc: Craig Blackmore <craig dot blackmore at embecosm dot com>, gdb-patches at sourceware dot org
- Date: Mon, 8 Oct 2018 12:29:45 +0100
- Subject: Re: [PATCH] RISC-V: enable have_nonsteppable_watchpoint by default
- References: <c83e39fb-b1e8-cc67-85b5-c7cc66197078@embecosm.com> <539c4dcf-8bfa-b567-5112-42eac55645c8@redhat.com> <20180917133425.GL5952@embecosm.com>
On 09/17/2018 02:34 PM, Andrew Burgess wrote:
> * Pedro Alves <palves@redhat.com> [2018-09-17 13:54:38 +0100]:
>
>> On 09/16/2018 01:13 AM, Craig Blackmore wrote:
>>> The RISC-V debug spec 0.13 recommends that write triggers fire before
>>> the write is committed. If the target follows this behaviour, then
>>> have_nonsteppable_watchpoint needs to be set to 1 so that GDB will step
>>> over the watchpoint before checking if the value has changed.
>>>
>>> This patch adds a setshow for have_nonsteppable_watchpoint which defaults
>>> to 1 to match the recommended behaviour. If a target does not follow
>>> this timing, then 'set riscv have_nonsteppable_watchpoint 0' will need
>>> to be issued on the command line.
Do you know of any implementation that _doesn't_ follow the spec?
Wondering whether we're adding a knob/complexity for nothing.
>>>
>>> gdb/ChangeLog:
>>>
>>> * riscv-tdep.c (set_have_nonsteppable_watchpoint): add
>>> callback for 'set riscv have_nonsteppable_watchpoint'
>>> (riscv_gdbarch_init): initialise gdbarch setting for
>>> have_nonesteppable_watchpoint
>>
>> This is something the target/stub knows, right? I'd be much
>> better to make this automatic, so that users wouldn't have to
>> know to tweak anything.
>
> Sure, you're thinking something like (to pick one at random) how the
> 'org.gnu.gdb.arm.neon' feature on ARM in the target description tells
> GDB how to operate, right? I totally agree.
I wasn't thinking of a target feature, but either a qSupported feature
or maybe better, an extension to the watchpoint stop reply ("stopped before/after").
This came up recently here, btw:
https://sourceware.org/ml/gdb/2018-08/msg00047.html
>
> ... but.... we'd still probably want to keep the flag around (though
> as an auto/on/off maybe) so the user could, if they wanted, override a
> badly behaving target...
>
> ....and.... there's no current remote description support for RiscV at
> all, so having implement that as a prerequisite seems a little steep
> (to me).
>
> My preference would be to allow this in basically as is, then make it
> automatic once we have target description support in place.
>
> Alternatively we could remove the control switch for now, and just go
> with:
>
> set_gdbarch_have_nonsteppable_watchpoint (gdbarch, 1);
>
> for everyone. But if there's anyone out there not following the
> recommendation that makes things a little harder for them in the short
> term.
But is there any evidence of any implementation deviating from
the spec's suggestion? From
https://sourceware.org/ml/gdb/2018-08/msg00047.html
I had assumed that we'd just fix gdb to follow the spec and be done
with it.
>
> What do you think?
>
Thanks,
Pedro Alves