Re: mi*-watch.exp: Test both hardware and software watchpoints

["Maciej W. Rozycki" <macro@mips.com>]

On Fri, 14 Sep 2007, Daniel Jacobowitz wrote:

> I don't think it's safe to extend the epilogue arbitrarily - we can't
> recognize the instruction because we have no way to query where the
> frame pointer is at this point.  We might extend it arbitrarily far
> backwards if we're not careful.

 Well, I would have thought the destruction of the frame pointer marks the 
beginning of the epilogue.  Though I can imagine an ABI relaxed enough to 
let the compiler shuffle instructions around while optimising so that some 
belonging to the body of a function actually arrive past the boundary.

> I don't intend to fix it right now, anyway.  And I really do not like
> leaving failing tests.  Every few years, I try to reduce the failures
> for a couple of platforms to zero.  I'm going to do it again after I
> finish with multi-threaded watchpoints (next week hopefully; patches
> coming this weekend).

 I guess they could act as incentive for platform maintainers. ;-)

> I think the right fix is going to have to involve GCC.  It should emit
> correct unwind info for the epilogue; even without also adding the
> epilogue markers to the line table, correct unwind info will solve
> this problem 99% of the time.

 Fair enough.

> > +    set old_prefix $pf_prefix
> > +    set pf_prefix "$pf_prefix $type: "
> 
> Is there a double space here?  IIUC pf_prefix will be something like
> "gdb.mi/mi-watch.exp: ".

 I would have thought so as well, but the sequence above actually places a 
single space character between the variables.  Perhaps it is another TCL 
feature -- I am not sure if I can be bothered to get to the bottom of it 
-- should I?

> Otherwise OK to commit.

 OK, here is an XFAIL patch.  Results for native i386-linux-gnu:

# of expected passes		26
# of expected failures		2

2007-09-14  Maciej W. Rozycki  <macro@mips.com>

	* gdb.mi/mi-watch.exp (test_watchpoint_all): Pass the watchpoint
	type down.
	(test_watchpoint_triggering): XFAIL the sw watchpoint scope
	test.
	* gdb.mi/mi2-watch.exp (test_watchpoint_all): Pass the
	watchpoint type down.
	(test_watchpoint_triggering): XFAIL the sw watchpoint scope
	test.

 It is not completely bullet-proof, like the whole set of changes anyway, 
as for platforms that do not support hw watchpoints at all the tests are 
run twice using sw watchpoints and do FAIL for the variation assumed to be 
hw.  It is not a regression though and a small waste of time only.  I can 
see whether determining if hw watchpoints are truly available is doable at 
some point, but I cannot promise any timeline at the moment.

 OK for this bit?

  Maciej

gdb-watch-scope-xfail.diff
Index: binutils-quilt/src/gdb/testsuite/gdb.mi/mi-watch.exp
===================================================================
--- binutils-quilt.orig/src/gdb/testsuite/gdb.mi/mi-watch.exp	2007-09-14 14:32:27.000000000 +0100
+++ binutils-quilt/src/gdb/testsuite/gdb.mi/mi-watch.exp	2007-09-14 16:04:14.000000000 +0100
@@ -41,7 +41,7 @@
      return -1
 }
 
-proc test_watchpoint_creation_and_listing {} {
+proc test_watchpoint_creation_and_listing {type} {
     global mi_gdb_prompt
     global srcfile
     global hex
@@ -65,7 +65,7 @@
 }
 
 # UNUSED at the time
-proc test_awatch_creation_and_listing {} {
+proc test_awatch_creation_and_listing {type} {
     global mi_gdb_prompt
     global srcfile
     global hex
@@ -92,7 +92,7 @@
 }
 
 # UNUSED at the time
-proc test_rwatch_creation_and_listing {} {
+proc test_rwatch_creation_and_listing {type} {
     global mi_gdb_prompt
     global srcfile
     global hex
@@ -118,7 +118,7 @@
 	    "delete read watchpoint"
 }
 
-proc test_watchpoint_triggering {} {
+proc test_watchpoint_triggering {type} {
     global mi_gdb_prompt
     global hex fullname_syntax srcfile
 
@@ -147,6 +147,9 @@
       timeout {fail "watchpoint trigger (timeout 1)"}
     }
 
+    if { $type == "sw" } {
+      setup_xfail *-*-*
+    }
     send_gdb "223-exec-continue\n"
     gdb_expect {
       -re "223\\^running\r\n$mi_gdb_prompt" {
@@ -161,6 +164,7 @@
       -re ".*$mi_gdb_prompt$" {fail "wp out of scope (1)"}
       timeout {fail "wp out of scope (timeout 1)"}
     }
+    clear_xfail *-*-*
 }
 
 proc test_watchpoint_all {type} {
@@ -177,10 +181,10 @@
     mi_gdb_load ${binfile}
 
     mi_runto callee4
-    test_watchpoint_creation_and_listing
-    #test_rwatch_creation_and_listing
-    #test_awatch_creation_and_listing
-    test_watchpoint_triggering
+    test_watchpoint_creation_and_listing $type
+    #test_rwatch_creation_and_listing $type
+    #test_awatch_creation_and_listing $type
+    test_watchpoint_triggering $type
 
     set pf_prefix $old_prefix
 }
Index: binutils-quilt/src/gdb/testsuite/gdb.mi/mi2-watch.exp
===================================================================
--- binutils-quilt.orig/src/gdb/testsuite/gdb.mi/mi2-watch.exp	2007-09-14 14:36:48.000000000 +0100
+++ binutils-quilt/src/gdb/testsuite/gdb.mi/mi2-watch.exp	2007-09-14 16:04:27.000000000 +0100
@@ -41,7 +41,7 @@
      return -1
 }
 
-proc test_watchpoint_creation_and_listing {} {
+proc test_watchpoint_creation_and_listing {type} {
     global mi_gdb_prompt
     global srcfile
     global hex
@@ -65,7 +65,7 @@
 }
 
 # UNUSED at the time
-proc test_awatch_creation_and_listing {} {
+proc test_awatch_creation_and_listing {type} {
     global mi_gdb_prompt
     global srcfile
     global hex
@@ -92,7 +92,7 @@
 }
 
 # UNUSED at the time
-proc test_rwatch_creation_and_listing {} {
+proc test_rwatch_creation_and_listing {type} {
     global mi_gdb_prompt
     global srcfile
     global hex
@@ -118,7 +118,7 @@
 	    "delete read watchpoint"
 }
 
-proc test_watchpoint_triggering {} {
+proc test_watchpoint_triggering {type} {
     global mi_gdb_prompt
     global hex
 
@@ -147,6 +147,9 @@
       timeout {fail "watchpoint trigger (timeout 1)"}
     }
 
+    if { $type == "sw" } {
+      setup_xfail *-*-*
+    }
     send_gdb "223-exec-continue\n"
     gdb_expect {
       -re "223\\^running\r\n$mi_gdb_prompt" {
@@ -161,6 +164,7 @@
       -re ".*$mi_gdb_prompt$" {fail "wp out of scope (1)"}
       timeout {fail "wp out of scope (timeout 1)"}
     }
+    clear_xfail *-*-*
 }
 
 proc test_watchpoint_all {type} {
@@ -177,10 +181,10 @@
     mi_gdb_load ${binfile}
 
     mi_runto callee4
-    test_watchpoint_creation_and_listing
-    #test_rwatch_creation_and_listing
-    #test_awatch_creation_and_listing
-    test_watchpoint_triggering
+    test_watchpoint_creation_and_listing $type
+    #test_rwatch_creation_and_listing $type
+    #test_awatch_creation_and_listing $type
+    test_watchpoint_triggering $type
 
     set pf_prefix $old_prefix
 }