[PATCH] [testsuite] Probe awatch/rwatch support
Pedro Alves
palves@redhat.com
Tue Apr 14 15:01:00 GMT 2015
On 04/14/2015 12:45 PM, Yao Qi wrote:
> From: Yao Qi <yao.qi@linaro.org>
>
> I see some awatch/rwatch related fails on one arm board which doesn't hw
> watchpoint or it is not enabled in the kernel, like this,
>
> rwatch global^M
> Expression cannot be implemented with read/access watchpoint.^M
> (gdb) FAIL: gdb.base/break-idempotent.exp: always-inserted off: rwatch: once: rwatch global
>
> Although we've had a proc skip_hw_watchpoint_access_tests to check
> whether rwatch/awatch is supported according to target triplet, it
> isn't accurate because HW watchpoint support varies on different
> processor implementations and linux kernel of the same arch.
>
> This patch is to probe the awatch/rwatch support in a new proc
> gdb_read_access_watchpoint, and callers just bail out the test
> if awatch/rwatch isn't supported or isn't successfully created.
> This patch skips these tests on native arm-linux testing, so fails
> go away.
>
> -FAIL: gdb.base/break-idempotent.exp: always-inserted off: rwatch: once: rwatch global
> -FAIL: gdb.base/break-idempotent.exp: always-inserted off: rwatch: twice: rwatch global
> -FAIL: gdb.base/break-idempotent.exp: always-inserted off: awatch: once: awatch global
> -FAIL: gdb.base/break-idempotent.exp: always-inserted off: awatch: twice: awatch global
> -FAIL: gdb.base/break-idempotent.exp: always-inserted on: rwatch: once: rwatch global
> -FAIL: gdb.base/break-idempotent.exp: always-inserted on: rwatch: twice: rwatch global
> -FAIL: gdb.base/break-idempotent.exp: always-inserted on: awatch: once: awatch global
> -FAIL: gdb.base/break-idempotent.exp: always-inserted on: awatch: twice: awatch global
> -FAIL: gdb.base/watch-read.exp: set hardware read watchpoint on global variable
> -FAIL: gdb.base/watch-read.exp: read watchpoint triggers on first read (timeout)
> -FAIL: gdb.base/watch-read.exp: read watchpoint triggers on read after value changed (timeout)
> -FAIL: gdb.base/watch-read.exp: set write watchpoint on global variable (timeout)
> -FAIL: gdb.base/watch-read.exp: write watchpoint triggers (timeout)
> -FAIL: gdb.base/watch-read.exp: only write watchpoint triggers when value changes (timeout)
> -FAIL: gdb.base/watch-read.exp: read watchpoint triggers when value doesn't change, trapping reads and writes (timeout)
> -FAIL: gdb.base/watch-read.exp: only read watchpoint triggers when value doesn't change (timeout)
> -FAIL: gdb.base/watch_thread_num.exp: Watchpoint on shared variable
> -FAIL: gdb.base/watch_thread_num.exp: info breakpoint shows watchpoint is thread-specific
> -FAIL: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 1 (timeout)
> -FAIL: gdb.base/watch_thread_num.exp: Check thread that triggered iteration 1 (timeout)
> -FAIL: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 2 (timeout)
> -FAIL: gdb.base/watch_thread_num.exp: Check thread that triggered iteration 2 (timeout)
> -FAIL: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 3 (timeout)
> -FAIL: gdb.base/watch_thread_num.exp: Check thread that triggered iteration 3 (timeout)
> -FAIL: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 4 (timeout)
> -FAIL: gdb.base/watch_thread_num.exp: Check thread that triggered iteration 4 (timeout)
> -FAIL: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 5 (timeout)
> -FAIL: gdb.base/watch_thread_num.exp: Check thread that triggered iteration 5 (timeout)
> -FAIL: gdb.base/watchpoint-hw-hit-once.exp: continue
> -FAIL: gdb.base/watchpoint-hw-hit-once.exp: continue to break-at-exit (the program exited)
> -FAIL: gdb.multi/watchpoint-multi.exp: awatch c on inferior 2
>
> this patch should also fix fails in gdb.base/watch_thread_num.exp on
> s390x, which were reporeted here:
>
> https://www.sourceware.org/ml/gdb-testers/2015-q2/msg01663.html
>
s390 fails like this:
No breakpoints or watchpoints.
(gdb) awatch shared_var thread 12
Target does not support this type of hardware watchpoint.
(gdb) FAIL: gdb.base/watch_thread_num.exp: Watchpoint on shared variable
> gdb/testsuite:
>
> 2015-04-14 Yao Qi <yao.qi@linaro.org>
>
> * gdb.base/break-idempotent.exp (set_breakpoint): Match the
> output for read/access watchpoint.
> * gdb.base/watch-read.exp: Invoke gdb_read_access_watchpoint
> and return if it returns false.
> * gdb.base/watch_thread_num.exp: Likewise.
> * gdb.base/watchpoint-hw-hit-once.exp: Likewise.
> * gdb.multi/watchpoint-multi.exp: Likewise.
> * gdb.base/watchpoint-reuse-slot.exp: Match the output
> for read/access watchpoint.
> * lib/gdb.exp (gdb_read_access_watchpoint): New proc.
> ---
> gdb/testsuite/gdb.base/break-idempotent.exp | 7 +++++++
> gdb/testsuite/gdb.base/watch-read.exp | 8 ++++----
> gdb/testsuite/gdb.base/watch_thread_num.exp | 7 ++++---
> gdb/testsuite/gdb.base/watchpoint-hw-hit-once.exp | 4 +++-
> gdb/testsuite/gdb.base/watchpoint-reuse-slot.exp | 3 +++
> gdb/testsuite/gdb.multi/watchpoint-multi.exp | 6 +++---
> gdb/testsuite/lib/gdb.exp | 25 +++++++++++++++++++++++
> 7 files changed, 49 insertions(+), 11 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.base/break-idempotent.exp b/gdb/testsuite/gdb.base/break-idempotent.exp
> index c5dae96..ef7db22 100644
> --- a/gdb/testsuite/gdb.base/break-idempotent.exp
> +++ b/gdb/testsuite/gdb.base/break-idempotent.exp
> @@ -107,6 +107,13 @@ proc set_breakpoint { break_command } {
> -re "Could not insert hardware watchpoint.*$gdb_prompt $" {
> unsupported $test
> }
> + -re "Expression cannot be implemented with read/access watchpoint..*$gdb_prompt $" {
> + if { $break_command == "watch" } {
> + fail $test
> + } else {
> + unsupported $test
> + }
> + }
> -re "atchpoint \[0-9\]+: global\r\n$gdb_prompt $" {
> pass $test
> }
> diff --git a/gdb/testsuite/gdb.base/watch-read.exp b/gdb/testsuite/gdb.base/watch-read.exp
> index ddba14e..4bf9a9e 100644
> --- a/gdb/testsuite/gdb.base/watch-read.exp
> +++ b/gdb/testsuite/gdb.base/watch-read.exp
> @@ -42,10 +42,10 @@ set read_line [gdb_get_line_number "read line" $srcfile]
>
> # Test running to a read of `global', with a read watchpoint set
> # watching it.
> -
> -gdb_test "rwatch global" \
> - "Hardware read watchpoint .*: global" \
> - "set hardware read watchpoint on global variable"
> +if { ![gdb_read_access_watchpoint "rwatch" "global" \
> + "set hardware read watchpoint on global variable"] } {
> + return
> +}
This seems to be the only case the test message was changed.
Was that intended?
I think that the API looks a bit awkward to use. I mean, the need to
pass the type as parameter. Why not wrap it in a couple procedures:
proc gdb_read_watchpoint { loc message } {
gdb_read_access_watchpoint "rwatch" $loc $message
}
proc gdb_access_watchpoint { loc message } {
gdb_read_access_watchpoint "awatch" $loc $message
}
or even gdb_rwatch and gdb_awatch, leaving gdb_read_access_watchpoint
as an implementation detail.
>
> # The first read is on entry to the loop.
>
> diff --git a/gdb/testsuite/gdb.base/watch_thread_num.exp b/gdb/testsuite/gdb.base/watch_thread_num.exp
> index d559f22..1995e5d 100644
> --- a/gdb/testsuite/gdb.base/watch_thread_num.exp
> +++ b/gdb/testsuite/gdb.base/watch_thread_num.exp
> @@ -71,9 +71,10 @@ delete_breakpoints
> # simultaneously, on targets with continuable watchpoints, such as
> # x86. See PR breakpoints/10116.
>
> -gdb_test "awatch shared_var thread $thread_num" \
> - "Hardware access \\(read/write\\) watchpoint .*: shared_var.*" \
> - "Watchpoint on shared variable"
> +if { ![gdb_read_access_watchpoint "awatch" "shared_var thread $thread_num" \
> + "Watchpoint on shared variable"] } {
> + return
> +}
>
> gdb_test "info breakpoint \$bpnum" \
> "stop only in thread $thread_num" \
> diff --git a/gdb/testsuite/gdb.base/watchpoint-hw-hit-once.exp b/gdb/testsuite/gdb.base/watchpoint-hw-hit-once.exp
> index 7ae76e0..d0aa2b9 100644
> --- a/gdb/testsuite/gdb.base/watchpoint-hw-hit-once.exp
> +++ b/gdb/testsuite/gdb.base/watchpoint-hw-hit-once.exp
> @@ -27,7 +27,9 @@ if ![runto_main] {
> return -1
> }
>
> -gdb_test "rwatch watchee"
> +if { ![gdb_read_access_watchpoint "rwatch" "watchee" "rwatch watchee"] } {
> + return
> +}
>
> gdb_breakpoint [gdb_get_line_number "dummy = 2"]
>
> diff --git a/gdb/testsuite/gdb.base/watchpoint-reuse-slot.exp b/gdb/testsuite/gdb.base/watchpoint-reuse-slot.exp
> index abe81d6..d9a7cee 100644
> --- a/gdb/testsuite/gdb.base/watchpoint-reuse-slot.exp
> +++ b/gdb/testsuite/gdb.base/watchpoint-reuse-slot.exp
> @@ -106,6 +106,9 @@ foreach cmd {"watch" "awatch" "rwatch"} {
> -re "Target does not support.*$gdb_prompt $" {
> unsupported $test
> }
> + -re "Expression cannot be implemented with read/access watchpoint..*$gdb_prompt $" {
> + unsupported $test
> + }
This seems to revealing something wrong in gdb itself. This is watching
a single byte or a global variable. Odd to see that error, which
should mean that the expression involved non-memory values.
If I'm reading the code correctly, either your board is target remote,
and you're setting "set remote hardware-watchpoint-length-limit 0";
or this is native and arm-linux-nat.c:arm_linux_can_use_hw_breakpoint
isn't return 0 to indicate no support. For the remote case, seems to
me that setting that setting to 0 effectively means watchpoints are
not supported, and thus remote_check_watch_resources should be changed
to return 0 if remote_hw_watchpoint_length_limit==0.
The native case seems like a bug in the backend too -- it should
return 0 when a watchpoint type isn't supported, which can be detected
by arm_linux_get_hw_watchpoint_count returning 0.
And then the core code is checking whether "set can-use-hw-watchpoints"
was used to disable watchpoints, but isn't checking whether the target
supports the watchpoint type at all.
As much as I dislike the watchpoint resource accounting... I think this
patch will make the errors more sensible, and probably trigger the
pre-existing unsupported watchpoint detection paths in the testsuite
(when there are any).
Completely untested...
>From b42292102c769163f91b2bab161710e6e9843de3 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Tue, 14 Apr 2015 15:17:24 +0100
Subject: [PATCH] check whether the target supports the watchpoint type
---
gdb/arm-linux-nat.c | 12 ++++++++++--
gdb/breakpoint.c | 4 ++++
gdb/remote.c | 4 ++++
gdb/target.h | 2 +-
4 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/gdb/arm-linux-nat.c b/gdb/arm-linux-nat.c
index bb8358c..afc5817 100644
--- a/gdb/arm-linux-nat.c
+++ b/gdb/arm-linux-nat.c
@@ -771,12 +771,20 @@ arm_linux_can_use_hw_breakpoint (struct target_ops *self,
if (type == bp_hardware_watchpoint || type == bp_read_watchpoint
|| type == bp_access_watchpoint || type == bp_watchpoint)
{
- if (cnt + ot > arm_linux_get_hw_watchpoint_count ())
+ int count = arm_linux_get_hw_watchpoint_count ();
+
+ if (count == 0)
+ return 0;
+ else if (cnt + ot > count)
return -1;
}
else if (type == bp_hardware_breakpoint)
{
- if (cnt > arm_linux_get_hw_breakpoint_count ())
+ int count = arm_linux_get_hw_breakpoint_count ();
+
+ if (count == 0)
+ return 0;
+ else if (cnt > count)
return -1;
}
else
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 96d2a14..aa7bc02 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -2104,6 +2104,10 @@ update_watchpoint (struct watchpoint *b, int reparse)
if (!can_use_hw_watchpoints)
error (_("Can't set read/access watchpoint when "
"hardware watchpoints are disabled."));
+ else if (target_can_use_hardware_watchpoint (b->base.type, 1, 0)
+ == 0)
+ error (_("Target does not support this type of "
+ "hardware watchpoint."));
else
error (_("Expression cannot be implemented with "
"read/access watchpoint."));
diff --git a/gdb/remote.c b/gdb/remote.c
index e5236b8..a9baa6d 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -8468,6 +8468,10 @@ remote_check_watch_resources (struct target_ops *self,
}
else
{
+ /* Setting "hardware-watchpoint-length-limit" to 0 effectively
+ means you can't watch anything. */
+ if (remote_hw_watchpoint_length_limit == 0)
+ return 0;
if (remote_hw_watchpoint_limit == 0)
return 0;
else if (remote_hw_watchpoint_limit < 0)
diff --git a/gdb/target.h b/gdb/target.h
index 6c57a69..2292c49 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -1834,7 +1834,7 @@ extern char *target_thread_name (struct thread_info *);
#define target_can_use_hardware_watchpoint(TYPE,CNT,OTHERTYPE) \
(*current_target.to_can_use_hw_breakpoint) (¤t_target, \
- TYPE, CNT, OTHERTYPE);
+ TYPE, CNT, OTHERTYPE)
/* Returns the number of debug registers needed to watch the given
memory region, or zero if not supported. */
--
1.9.3
More information about the Gdb-patches
mailing list