This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] hardware watchpoints turned off, inferior not yet started
- From: Pedro Alves <palves at redhat dot com>
- To: Andrew Burgess <aburgess at broadcom dot com>
- Cc: "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>
- Date: Wed, 16 Oct 2013 17:28:23 +0100
- Subject: Re: [PATCH] hardware watchpoints turned off, inferior not yet started
- Authentication-results: sourceware.org; auth=none
- References: <525E5EB6 dot 4070305 at broadcom dot com> <525E8761 dot 8060405 at redhat dot com> <525EB424 dot 6010407 at broadcom dot com>
On 10/16/2013 04:43 PM, Andrew Burgess wrote:
> On 16/10/2013 1:32 PM, Pedro Alves wrote:
>> ... this change I think makes it so that access/read
>> watchpoints get converted to software watchpoints, which is wrong.
>
> OK I see that now, I got confused by code later within
> update_watchpoint that seems to unconditionally fallback
> to bp_watchpoint, but I see now how the read/access watchpoints
> actually result in an error. Thanks for pointing this out.
>
> I extended this code to cover this case and added tests.
> gdb/ChangeLog
>
> * breakpoint.c (update_watchpoint): Create software watchpoints if
> the inferior has not yet started and hardware watchpoints are
> turned off.
"Create" isn't really accurate. This function is called for resets too.
So, something like
"file foo; start; watch; kill; set can-use-hw-watchpoint 0; file foo"
will trigger that code path too, which I think will end up resulting
in access/read watchpoints being disabled, with something like:
"file foo; start; awatch; kill; set can-use-hw-watchpoint 0; file foo"
(Replace the last "file" with anything that triggers a reset.)
BTW, could you confirm that?
Anyway, I suggest:
* breakpoint.c (update_watchpoint): If hardware watchpoints are
forced off, downgrade them to software watchpoints if possible,
and error out if not possible.
> + # Ensure that if we turn off hardware watchpoints and set a watch point
> + # before starting the inferior the watchpoint created will not be a
> + # hardware watchpoint.
> + gdb_test_no_output "set can-use-hw-watchpoints 0" ""
> + gdb_test "watch ival1" "Watchpoint \[0-9\]+: ival1" \
> + "Set software watchpoint before starting the inferior"
> +
> + # The next tests are written to match the current state of gdb: access
> + # and read watchpoints require hardware watchpoint support, with this
> + # turned off these can't be created.
> + gdb_test "awatch ival1" \
> + "Target does not support software watchpoints of this type." \
> + "Set software watchpoint before starting the inferior"
> + gdb_test "rwatch ival1" \
> + "Target does not support software watchpoints of this type." \
> + "Set software watchpoint before starting the inferior"
The "Set software watchpoint before starting the inferior" string will
appear in gdb.sum for the three tests. Please make those unique per test.
Otherwise OK.
Thanks,
--
Pedro Alves