Re: [PATCH] Extra error message from update_watchpoint

Andrew Burgess
2013-10-31T13:05:51Z

On 30/10/2013 6:40 PM, Eli Zaretskii wrote:
>> Date: Wed, 30 Oct 2013 17:13:19 +0000
>> From: "Andrew Burgess" <aburgess@broadcom.com>
>> cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
>>
>> I'd rather make you happy if possible, never hurts to suck up right ;-)
>
> Thank you!
>
>> The following follows the structure you gave, I'd be happy to go with
>> this except that it's longer than 80 character... is that an issue?
>>
>> "Can't set read/access watchpoint, software read/access watchpoints not
>> supported, and hardware watchpoints disabled (see set/show
>> can-use-hw-watchpoints)."
>
> This is indeed much better.
>
> As for length, I can suggest a shorter variant:
>
> Can't set read/access watchpoint when hardware watchpoints are disabled.

I've now pushed a version using this shorter message.

Pedro, Eli, thanks for the reviews.

Andrew

gdb/ChangeLog

2013-10-31 Andrew Burgess <aburgess@broadcom.com>

* breakpoint.c (update_watchpoint): Update error message and add
an additional error message.

gdb/testsuite/ChangeLog

2013-10-31 Andrew Burgess <aburgess@broadcom.com>

* gdb.base/watchpoint.exp (test_no_hw_watchpoints): Add additional
tests and update expected error message.
(test_watch_register_location): New tests.
(do_tests): Call test_watch_register_location.
* gdb.base/watchpoints.exp: Update expected error message.

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 608463d..1782c99 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -1805,7 +1805,8 @@ update_watchpoint (struct watchpoint *b, int reparse)
if (b->base.ops->works_in_software_mode (&b->base))
b->base.type = bp_watchpoint;
else
- error (_("Software read/access watchpoints not supported."));
+ error (_("Can't set read/access watchpoint when "
+ "hardware watchpoints are disabled."));
}
}
else if (within_current_scope && b->exp)
@@ -1946,8 +1947,14 @@ update_watchpoint (struct watchpoint *b, int reparse)
}
}
else if (!b->base.ops->works_in_software_mode (&b->base))
- error (_("Expression cannot be implemented with "
- "read/access watchpoint."));
+ {
+ if (!can_use_hw_watchpoints)
+ error (_("Can't set read/access watchpoint when "
+ "hardware watchpoints are disabled."));
+ else
+ error (_("Expression cannot be implemented with "
+ "read/access watchpoint."));
+ }
else
b->base.type = bp_watchpoint;

diff --git a/gdb/testsuite/gdb.base/watchpoint.exp b/gdb/testsuite/gdb.base/watchpoint.exp
index e0d4f81..9576a9e 100644
--- a/gdb/testsuite/gdb.base/watchpoint.exp
+++ b/gdb/testsuite/gdb.base/watchpoint.exp
@@ -825,8 +825,12 @@ proc test_no_hw_watchpoints {} {
# refrains from using them.
#
gdb_test "rwatch ival3" \
- "Expression cannot be implemented with read/access watchpoint..*" \
+ "Can't set read/access watchpoint when hardware watchpoints are disabled." \
"rwatch disallowed when can-set-hw-watchpoints cleared"
+ gdb_test "awatch ival3" \
+ "Can't set read/access watchpoint when hardware watchpoints are disabled." \
+ "awatch disallowed when can-set-hw-watchpoints cleared"
+

# Re-enable hardware watchpoints if necessary.
if ![target_info exists gdb,no_hardware_watchpoints] {
@@ -879,6 +883,22 @@ proc test_watchpoint_in_big_blob {} {
gdb_test_no_output "delete \$bpnum" "delete watch buf"
}

+proc test_watch_register_location {} {
+ global no_hw
+
+ if {!$no_hw && ![target_info exists gdb,no_hardware_watchpoints]} {
+ # Non-memory read/access watchpoints are not supported, they would
+ # require software read/access watchpoint support (which is not
+ # currently available).
+ gdb_test "rwatch \$pc" \
+ "Expression cannot be implemented with read/access watchpoint..*" \
+ "rwatch disallowed for register based expression"
+ gdb_test "awatch \$pc" \
+ "Expression cannot be implemented with read/access watchpoint..*" \
+ "awatch disallowed for register based expression"
+ }
+}
+
# Start with a fresh gdb.

set prev_timeout $timeout
@@ -940,6 +960,8 @@ proc do_tests {} {

test_wide_location_1
test_wide_location_2
+
+ test_watch_register_location
}

# On targets that can do hardware watchpoints, run the tests twice:
diff --git a/gdb/testsuite/gdb.base/watchpoints.exp b/gdb/testsuite/gdb.base/watchpoints.exp
index b70e86c..4e21d14 100644
--- a/gdb/testsuite/gdb.base/watchpoints.exp
+++ b/gdb/testsuite/gdb.base/watchpoints.exp
@@ -41,10 +41,10 @@ with_test_prefix "before inferior start" {
# and read watchpoints require hardware watchpoint support, with this
# turned off these can't be created.
gdb_test "awatch ival1" \
- "Software read/access watchpoints not supported." \
+ "Can't set read/access watchpoint when hardware watchpoints are disabled." \
"create access watchpoint"
gdb_test "rwatch ival1" \
- "Software read/access watchpoints not supported." \
+ "Can't set read/access watchpoint when hardware watchpoints are disabled." \
"create read watchpoint"
}