Re: [PATCH] hardware watchpoints turned off, inferior not yet started

[jose dot marchesi at oracle dot com (Jose E. Marchesi)]

[Following https://sourceware.org/ml/gdb-patches/2013-10/msg00563.html]

Hi.

The patch posted above introduced a regression in sparc64-*-linux-gnu,
and possibly in other targets not supporting hardware watchpoints at
all.

Today I was surprised to find this in a sparc64-*-linux-gnu target:

 $ gdb foo
 ... intro text ...
 (gdb) watch global_var
 Hardware watchpoint 1: global_var

The cause for this regression is this code in update_watchpoint
(breakpoint.c):

+  if (!target_has_execution)
     {
       /* Without execution, memory can't change.  No use to try and
         set watchpoint locations.  The watchpoint will be reset when
         the target gains execution, through breakpoint_re_set.  */
+      if (!can_use_hw_watchpoints)
+       {
+         if (b->base.ops->works_in_software_mode (&b->base))
+           b->base.type = bp_watchpoint;
+         else
+           error (_("Software read/access watchpoints not supported."));
+       }

The watchpoint must be downgraded to a software watchpoint if the target
does not support hw watchpoints, even if can_use_hw_watchpoints is 1.

The following patch fixes this and also adds an additional test to
testsuite/watchpoints.exp to catch this problem.

2013-11-07  Jose E. Marchesi  <jose.marchesi@oracle.com>

	* breakpoint.c (update_watchpoint): Downgrade hw watchpoints to sw
	watchpoints in targets not supporting them, when the inferior is
        not running.

2013-11-07  Jose E. Marchesi  <jose.marchesi@oracle.com>

	* gdb.base/watchpoints.exp: Add a test to ensure that a watchpoint
	is not created as a hw watchpoint in targets not supporting them,
	even if can-use-hw-watchpoints is 1 when the inferior is not
        running.

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index ffe73fd..597e6f9 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -1800,7 +1800,10 @@ update_watchpoint (struct watchpoint *b, int reparse)
       /* Without execution, memory can't change.  No use to try and
 	 set watchpoint locations.  The watchpoint will be reset when
 	 the target gains execution, through breakpoint_re_set.  */
-      if (!can_use_hw_watchpoints)
+      int i = hw_breakpoint_used_count ();
+      int target_supports_hw_watchpoints =
+	target_can_use_hardware_watchpoint (bp_hardware_watchpoint, i + 1, 0);
+      if (!target_supports_hw_watchpoints || !can_use_hw_watchpoints)
 	{
 	  if (b->base.ops->works_in_software_mode (&b->base))
 	    b->base.type = bp_watchpoint;
--- a/gdb/testsuite/gdb.base/watchpoints.exp
+++ b/gdb/testsuite/gdb.base/watchpoints.exp
@@ -30,6 +30,15 @@ if {[prepare_for_testing $testfile.exp $testfile $srcfile debug]} {
 }
 
 with_test_prefix "before inferior start" {
+    if [target_info exists gdb,no_hardware_watchpoints] {
+	# Ensure that a watchpoint is not created as a hw watchpoint
+	# even if can-use-hw-watchpoints is 1 when the inferior is not
+	# running.
+	gdb_test_no_output "set can-use-hw-watchpoints 1"
+	gdb_test "watch ival1" "Watchpoint \[0-9\]+: ival1" \
+	    "create sw watchpoint"
+    }
+
     # 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.