Re: [PATCH] Extra error message from update_watchpoint

["Andrew Burgess" <aburgess@broadcom.com>]

On 18/10/2013 6:18 PM, Pedro Alves wrote:
> On 10/18/2013 10:20 AM, Andrew Burgess wrote:
>> @@ -1946,8 +1946,13 @@ 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 (_("Software read/access watchpoints not supported."));
> 
> Hmm, is this really "not supported", or rather "explicitly disabled"?
> I think I'd prefer:
> 
> 		error (_("Hardware watchpoints support disabled.  "
> 			 "See set/show can-use-hw-watchpoints."));
> 
> which also hints towards the toggle.  WDYT?

I used that error message as it matches the one I already added in:
  https://sourceware.org/ml/gdb-patches/2013-10/msg00563.html
but I don't really mind.

Ideally you might want both bits of the error message, you've
turned off H/W watchpoints so you need to be told that read/access
watchpoints are not supported in S/W, but I agree, we should also
let the user know that H/W watchpoints have been turned off and
could be turned back on again.  That seems like too much for one
error message though...  I'm happy to go with your suggestion.

Updated patch changes both error messages to match, OK to apply?

Thanks,
Andrew

gdb/ChangeLog

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

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

gdb/testsuite/ChangeLog

2013-10-29  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..68b348d 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 (_("Hardware watchpoint support disabled.  "
+		     "See set/show can-use-hw-watchpoints."));
 	}
     }
   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 (_("Hardware watchpoint support disabled.  "
+			 "See set/show can-use-hw-watchpoints."));
+	      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..0ff5802 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..*" \
+	"Hardware watchpoint support disabled\.  See set/show can-use-hw-watchpoints\." \
 	"rwatch disallowed when can-set-hw-watchpoints cleared"
+    gdb_test "awatch ival3" \
+	"Hardware watchpoint support disabled\.  See set/show can-use-hw-watchpoints\." \
+	"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..e429f91 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." \
+        "Hardware watchpoint support disabled\.  See set/show can-use-hw-watchpoints\." \
         "create access watchpoint"
     gdb_test "rwatch ival1" \
-        "Software read/access watchpoints not supported." \
+        "Hardware watchpoint support disabled\.  See set/show can-use-hw-watchpoints\." \
         "create read watchpoint"
 }