New tests to watch regions larger than a machine word (Re: [PATCH] PR threads/10729: x86 hw watchpoints and non-stop mode)

[Pedro Alves <pedro at codesourcery dot com>]

On Monday 05 December 2011 17:02:54, Eli Zaretskii wrote:

> Be sure to test it with several watchpoints watching the same address,
> either with different conditions or different types (rwatch vs awatch
> vs watch), 

That's actually already tested by watch-read.exp.

> and also with watchpoints that watch regions longer than 4
> bytes on IA32 (and similarly on 64-bit hosts).

This however doesn't seem to be tested anywhere.

Here are a couple new tests to exercise that.  They pass cleanly
before and after the proposed patch, native and gdbserver linux,
x86 and x86_64.

The tests expects the regions to be coverable by hardware watchpoints by
default, unless gdb,no_hardware_watchpoints is set in the board file.
Rather than only doing that selectively on x86, it's better to default
to expect the target can support the largish watchpoints,
and explicitly list targets where we find it's not possible to watch a
region larger than a word (not sure which, but I'm sure they're out there).
Doing the opposite tends to have tests never enabled on targets where
they could and should be enabled.  We have such cases already in
watchpoint.exp even.  E.g.:

    # Only enabled for some targets merely because it has not been tested 
    # elsewhere.
    # On sparc-sun-sunos4.1.3, GDB was running all the way to the marker4 
    # breakpoint before stopping for the watchpoint.  I don't know why.
    if {[istarget "hppa*-*-*"]} then {
	test_watchpoint_triggered_in_syscall
    }

Comments?

gdb/testsuite/
2011-12-09  Pedro Alves  <pedro@codesourcery.com>

	* gdb.base/watchpoint.c (struct foo2, foo2, struct foo4, foo4)
	(func6, func7): New.
	(main): Call func6 and func7.
	* gdb.base/watchpoint.exp (test_wide_location_1)
	(test_wide_location_2): New.
	(top level): Re-enable hardware watchpoints if necessary.  Call
	test_wide_location_1 and test_wide_location_2.
---

 gdb/testsuite/gdb.base/watchpoint.c   |   32 +++++++++++++++
 gdb/testsuite/gdb.base/watchpoint.exp |   70 +++++++++++++++++++++++++++++++++
 2 files changed, 102 insertions(+), 0 deletions(-)

diff --git a/gdb/testsuite/gdb.base/watchpoint.c b/gdb/testsuite/gdb.base/watchpoint.c
index 9ef9253..c4d8a69 100644
--- a/gdb/testsuite/gdb.base/watchpoint.c
+++ b/gdb/testsuite/gdb.base/watchpoint.c
@@ -42,6 +42,18 @@ int doread = 0;
 char *global_ptr;
 char **global_ptr_ptr;
 
+struct foo2
+{
+  int val[2];
+};
+struct foo2 foo2;
+
+struct foo4
+{
+  int val[4];
+};
+struct foo4 foo4;
+
 void marker1 ()
 {
 }
@@ -137,6 +149,22 @@ func5 ()
   val = 27;
 }
 
+void
+func6 (void)
+{
+  /* func6 breakpoint here */
+  foo2.val[1] = 0;
+  foo2.val[1] = 11;
+}
+
+void
+func7 (void)
+{
+  /* func7 breakpoint here */
+  foo4.val[3] = 0;
+  foo4.val[3] = 33;
+}
+
 int main ()
 {
 #ifdef usestubs
@@ -216,5 +244,9 @@ int main ()
 
   func5 ();
 
+  func6 ();
+
+  func7 ();
+
   return 0;
 }
diff --git a/gdb/testsuite/gdb.base/watchpoint.exp b/gdb/testsuite/gdb.base/watchpoint.exp
index 331b181..574dd8b 100644
--- a/gdb/testsuite/gdb.base/watchpoint.exp
+++ b/gdb/testsuite/gdb.base/watchpoint.exp
@@ -660,6 +660,68 @@ proc test_watch_location {} {
     gdb_test_no_output "delete \$bpnum" "delete watch -location"
 }
 
+# Tests watching areas larger than a word.
+
+proc test_wide_location_1 {} {
+    # This test watches two words on most 32-bit ABIs, and one word on
+    # most 64-bit ABIs.
+
+    # Platforms where the target can't watch such a large region
+    # should clear hw_expected below.
+    if [target_info exists gdb,no_hardware_watchpoints] {
+	set hw_expected 0
+    } else {
+	set hw_expected 1
+    }
+
+    gdb_breakpoint [gdb_get_line_number "func6 breakpoint here"]
+    gdb_continue_to_breakpoint "func6 breakpoint here"
+
+    if { $hw_expected } {
+	gdb_test "watch foo2" "Hardware watchpoint .*: .*" "watch foo2"
+	gdb_test "continue" \
+	    "Continuing.*Hardware watchpoint .*: .*New value = \\\{val = \\\{0, 11\\\}\\\}.*" \
+	    "continue with watch foo2"
+    } else {
+	gdb_test "watch foo2" "atchpoint .*: .*" "watch foo2"
+	gdb_test "continue" \
+	    "Continuing.*\[Ww\]atchpoint .*: .*New value = \\\{val = \\\{0, 11\\\}\\\}.*" \
+	    "continue with watch foo2"
+    }
+
+    gdb_test_no_output "delete \$bpnum" "delete watch foo2"
+}
+
+proc test_wide_location_2 {} {
+    # This test watches four words on most 32-bit ABIs, and two words
+    # on 64-bit ABIs.
+
+    # Platforms where the target can't watch such a large region
+    # should clear hw_expected below.
+    if [target_info exists gdb,no_hardware_watchpoints] {
+	set hw_expected 0
+    } else {
+	set hw_expected 1
+    }
+
+    gdb_breakpoint [gdb_get_line_number "func7 breakpoint here"]
+    gdb_continue_to_breakpoint "func7 breakpoint here"
+
+    if { $hw_expected } {
+	gdb_test "watch foo4" "Hardware watchpoint .*: .*" "watch foo4"
+	gdb_test "continue" \
+	    "Continuing.*Hardware watchpoint .*: .*New value = \\\{val = \\\{0, 0, 0, 33\\\}\\\}.*" \
+	    "continue with watch foo4"
+    } else {
+	gdb_test "watch foo4" "atchpoint .*: .*" "watch foo4"
+	gdb_test "continue" \
+	    "Continuing.*\[Ww\]atchpoint .*: .*New value = \\\{val = \\\{0, 0, 0, 33\\\}\\\}.*" \
+	    "continue with watch foo4"
+    }
+
+    gdb_test_no_output "delete \$bpnum" "delete watch foo4"
+}
+
 proc test_inaccessible_watchpoint {} {
     global gdb_prompt
 
@@ -923,6 +985,14 @@ if [initialize] then {
     test_disable_enable_software_watchpoint
 
     test_watch_location
+
+    # Re-enable hardware watchpoints if necessary.
+    if ![target_info exists gdb,no_hardware_watchpoints] {
+        gdb_test_no_output "set can-use-hw-watchpoints 1" ""
+    }
+
+    test_wide_location_1
+    test_wide_location_2
 }
 
 # Restore old timeout