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

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

[Maciej W. Rozycki]

Hello,

 This change rearranges how the MI watchpoints tests are done so that both 
hardware and software watchpoints are examined by running the same set of 
tests with both.

 This change has been tested natively for mips-linux-gnu and 
i386-linux-gnu and remotely for mipsisa32-sde-elf, with mips-sim-sde32/-EB 
and mips-sim-sde32/-EL.  The change adds a failure for all the targets for 
previously untested sw watchpoints (mips-linux-gnu fails already for no hw 
watchpoint support for this platform is there still, sigh...) with the 
watchpoint-out-of-scope test like below:

FAIL: gdb.mi/mi-watch.exp: wp out of scope (2)

 The reason for MIPS is (and I suppose for i386 likewise) as soon as the 
frame pointer is destroyed in the callee, the watchpoint vanishes.  There 
is this in_function_epilogue_p() hook that may be used to single-step out 
of the function epilogue which is currently not implemented.

 For MIPS I have a fix for it which I will sent separately and which works 
for non-PIC o32 (tested as above) and should work for all the new ABIs 
(untested).  It does not quite work for o32 PIC, because the GOT pointer 
restoration in the caller is still considered a part of the source line 
associated with the function call.  It is still a step forward though -- 
at least it triggers in the right function and only one load instruction 
too early.

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

	* gdb.mi/mi-watch.exp (test_watchpoint_all): New function.
	Move all the tests here and run them twice, once using software
	watchpoints and once using hardware watchpoints.
	* gdb.mi/mi2-watch.exp (test_watchpoint_all): Likewise.

 OK to apply?

  Maciej

14678.diff
Index: binutils-quilt/src/gdb/testsuite/gdb.mi/mi2-watch.exp
===================================================================
--- binutils-quilt.orig/src/gdb/testsuite/gdb.mi/mi2-watch.exp	2007-09-07 12:50:38.000000000 +0100
+++ binutils-quilt/src/gdb/testsuite/gdb.mi/mi2-watch.exp	2007-09-07 13:03:36.000000000 +0100
@@ -41,10 +41,6 @@
      return -1
 }
 
-mi_delete_breakpoints
-mi_gdb_reinitialize_dir $srcdir/$subdir
-mi_gdb_load ${binfile}
-
 proc test_watchpoint_creation_and_listing {} {
     global mi_gdb_prompt
     global srcfile
@@ -167,16 +163,42 @@
     }
 }
 
-# Disable hardware watchpoints if necessary.
+proc test_watchpoint_all {} {
+    upvar srcdir srcdir
+    upvar subdir subdir
+    upvar binfile binfile
+
+    mi_delete_breakpoints
+    mi_gdb_reinitialize_dir $srcdir/$subdir
+    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
+}
+
+# Run the tests twice, once using software watchpoints...
+mi_gdb_test "567-gdb-set can-use-hw-watchpoints 0" \
+	"567\\^done" \
+	"hw watchpoints toggle"
+test_watchpoint_all
+
+mi_gdb_exit
+
+# ... and unless requested otherwise...
 if [target_info exists gdb,no_hardware_watchpoints] {
-    mi_gdb_test "-gdb-set can-use-hw-watchpoints 0" "\\^done" ""
+    return 0
 }
 
-mi_runto callee4
-test_watchpoint_creation_and_listing
-#test_rwatch_creation_and_listing
-#test_awatch_creation_and_listing
-test_watchpoint_triggering
+mi_gdb_start
+
+# ... once using hardware watchpoints (if available).
+mi_gdb_test "890-gdb-set can-use-hw-watchpoints 1" \
+	"890\\^done" \
+	"hw watchpoints toggle"
+test_watchpoint_all
 
 mi_gdb_exit
 return 0
Index: binutils-quilt/src/gdb/testsuite/gdb.mi/mi-watch.exp
===================================================================
--- binutils-quilt.orig/src/gdb/testsuite/gdb.mi/mi-watch.exp	2007-09-07 12:50:38.000000000 +0100
+++ binutils-quilt/src/gdb/testsuite/gdb.mi/mi-watch.exp	2007-09-07 13:02:52.000000000 +0100
@@ -41,10 +41,6 @@
      return -1
 }
 
-mi_delete_breakpoints
-mi_gdb_reinitialize_dir $srcdir/$subdir
-mi_gdb_load ${binfile}
-
 proc test_watchpoint_creation_and_listing {} {
     global mi_gdb_prompt
     global srcfile
@@ -167,16 +163,42 @@
     }
 }
 
-# Disable hardware watchpoints if necessary.
+proc test_watchpoint_all {} {
+    upvar srcdir srcdir
+    upvar subdir subdir
+    upvar binfile binfile
+
+    mi_delete_breakpoints
+    mi_gdb_reinitialize_dir $srcdir/$subdir
+    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
+}
+
+# Run the tests twice, once using software watchpoints...
+mi_gdb_test "567-gdb-set can-use-hw-watchpoints 0" \
+	"567\\^done" \
+	"hw watchpoints toggle"
+test_watchpoint_all
+
+mi_gdb_exit
+
+# ... and unless requested otherwise...
 if [target_info exists gdb,no_hardware_watchpoints] {
-    mi_gdb_test "-gdb-set can-use-hw-watchpoints 0" "\\^done" ""
+    return 0
 }
 
-mi_runto callee4
-test_watchpoint_creation_and_listing
-#test_rwatch_creation_and_listing
-#test_awatch_creation_and_listing
-test_watchpoint_triggering
+mi_gdb_start
+
+# ... once using hardware watchpoints (if available).
+mi_gdb_test "890-gdb-set can-use-hw-watchpoints 1" \
+	"890\\^done" \
+	"hw watchpoints toggle"
+test_watchpoint_all
 
 mi_gdb_exit
 return 0

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

[Daniel Jacobowitz]

On Tue, Sep 11, 2007 at 04:42:05PM +0100, Maciej W. Rozycki wrote:
>  The reason for MIPS is (and I suppose for i386 likewise) as soon as the 
> frame pointer is destroyed in the callee, the watchpoint vanishes.  There 
> is this in_function_epilogue_p() hook that may be used to single-step out 
> of the function epilogue which is currently not implemented.

It isn't generally sufficient, either.  It's implemented on PowerPC
but the test still fails.  The frame pointer is corrupted one
instruction before the epilogue is detected, and I couldn't come up
with a reasonable way of making it work.

> 2007-09-11  Maciej W. Rozycki  <macro@mips.com>
> 
> 	* gdb.mi/mi-watch.exp (test_watchpoint_all): New function.
> 	Move all the tests here and run them twice, once using software
> 	watchpoints and once using hardware watchpoints.
> 	* gdb.mi/mi2-watch.exp (test_watchpoint_all): Likewise.
> 
>  OK to apply?

In principle, I think so, but give it a day or two to see if anyone
objects.  How about xfailing the out of scope test for software
watchpoints?

In practice, there's one problem.  You're going to create lots of
tests with duplicated test names.  The usual way to fix this is to
pass a prefix around and apply it everywhere.  The clever way to
fix this, inspired by Jan's recent change to sepdebug.exp, is to
modify the global pf_prefix.

If you do that please leave the default prefix present though; save
the value at the beginning of the test (i.e. after default_gdb_init)
and append to it.

-- 
Daniel Jacobowitz
CodeSourcery

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

[Maciej W. Rozycki]

On Tue, 11 Sep 2007, Daniel Jacobowitz wrote:

> It isn't generally sufficient, either.  It's implemented on PowerPC
> but the test still fails.  The frame pointer is corrupted one
> instruction before the epilogue is detected, and I couldn't come up
> with a reasonable way of making it work.

 Hmm, extending the range of the epilogue?  Just an obvious thought -- I 
have no competence with PowerPC.

> In principle, I think so, but give it a day or two to see if anyone
> objects.  How about xfailing the out of scope test for software
> watchpoints?

 Well, should I take it as a declaration of no intent to fix them?

> In practice, there's one problem.  You're going to create lots of
> tests with duplicated test names.  The usual way to fix this is to
> pass a prefix around and apply it everywhere.  The clever way to
> fix this, inspired by Jan's recent change to sepdebug.exp, is to
> modify the global pf_prefix.

 Indeed -- thanks for the hint.

> If you do that please leave the default prefix present though; save
> the value at the beginning of the test (i.e. after default_gdb_init)
> and append to it.

 Yes, of course -- I had a look at sepdebug.exp and was quite annoyed by 
the output having the script name swallowed at one point.

 Here is an updated version.  This change has been tested natively for 
mips-linux-gnu and i386-linux-gnu and remotely for mipsisa32-sde-elf, with 
mips-sim-sde32/-EB and mips-sim-sde32/-EL.

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

	* gdb.mi/mi-watch.exp (test_watchpoint_all): New function.
	Move all the tests here and run them twice, once using software
	watchpoints and once using hardware watchpoints.
	* gdb.mi/mi2-watch.exp (test_watchpoint_all): Likewise.

 OK to apply?

  Maciej

14678.diff
Index: binutils-quilt/ChangeLog-14678
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ binutils-quilt/ChangeLog-14678	2007-09-14 14:35:32.000000000 +0100
@@ -0,0 +1,7 @@
+2006-09-18  Maciej W. Rozycki  <macro@mips.com>
+
+	[mti-fix-scope]
+	* gdb.mi/mi-watch.exp (test_watchpoint_all): New function.
+	Move all the tests here and run them twice, once using software
+	watchpoints and once using hardware watchpoints.
+	* gdb.mi/mi2-watch.exp (test_watchpoint_all): Likewise.
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:07:28.000000000 +0100
+++ binutils-quilt/src/gdb/testsuite/gdb.mi/mi2-watch.exp	2007-09-14 14:36:48.000000000 +0100
@@ -41,10 +41,6 @@
      return -1
 }
 
-mi_delete_breakpoints
-mi_gdb_reinitialize_dir $srcdir/$subdir
-mi_gdb_load ${binfile}
-
 proc test_watchpoint_creation_and_listing {} {
     global mi_gdb_prompt
     global srcfile
@@ -167,16 +163,48 @@
     }
 }
 
-# Disable hardware watchpoints if necessary.
+proc test_watchpoint_all {type} {
+    global pf_prefix
+    upvar srcdir srcdir
+    upvar subdir subdir
+    upvar binfile binfile
+
+    set old_prefix $pf_prefix
+    set pf_prefix "$pf_prefix $type: "
+
+    mi_delete_breakpoints
+    mi_gdb_reinitialize_dir $srcdir/$subdir
+    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
+
+    set pf_prefix $old_prefix
+}
+
+# Run the tests twice, once using software watchpoints...
+mi_gdb_test "567-gdb-set can-use-hw-watchpoints 0" \
+	"567\\^done" \
+	"hw watchpoints toggle (1)"
+test_watchpoint_all sw
+
+mi_gdb_exit
+
+# ... and unless requested otherwise...
 if [target_info exists gdb,no_hardware_watchpoints] {
-    mi_gdb_test "-gdb-set can-use-hw-watchpoints 0" "\\^done" ""
+    return 0
 }
 
-mi_runto callee4
-test_watchpoint_creation_and_listing
-#test_rwatch_creation_and_listing
-#test_awatch_creation_and_listing
-test_watchpoint_triggering
+mi_gdb_start
+
+# ... once using hardware watchpoints (if available).
+mi_gdb_test "890-gdb-set can-use-hw-watchpoints 1" \
+	"890\\^done" \
+	"hw watchpoints toggle (2)"
+test_watchpoint_all hw
 
 mi_gdb_exit
 return 0
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:07:28.000000000 +0100
+++ binutils-quilt/src/gdb/testsuite/gdb.mi/mi-watch.exp	2007-09-14 14:32:27.000000000 +0100
@@ -41,10 +41,6 @@
      return -1
 }
 
-mi_delete_breakpoints
-mi_gdb_reinitialize_dir $srcdir/$subdir
-mi_gdb_load ${binfile}
-
 proc test_watchpoint_creation_and_listing {} {
     global mi_gdb_prompt
     global srcfile
@@ -167,16 +163,48 @@
     }
 }
 
-# Disable hardware watchpoints if necessary.
+proc test_watchpoint_all {type} {
+    global pf_prefix
+    upvar srcdir srcdir
+    upvar subdir subdir
+    upvar binfile binfile
+
+    set old_prefix $pf_prefix
+    set pf_prefix "$pf_prefix $type: "
+
+    mi_delete_breakpoints
+    mi_gdb_reinitialize_dir $srcdir/$subdir
+    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
+
+    set pf_prefix $old_prefix
+}
+
+# Run the tests twice, once using software watchpoints...
+mi_gdb_test "567-gdb-set can-use-hw-watchpoints 0" \
+	"567\\^done" \
+	"hw watchpoints toggle (1)"
+test_watchpoint_all sw
+
+mi_gdb_exit
+
+# ... and unless requested otherwise...
 if [target_info exists gdb,no_hardware_watchpoints] {
-    mi_gdb_test "-gdb-set can-use-hw-watchpoints 0" "\\^done" ""
+    return 0
 }
 
-mi_runto callee4
-test_watchpoint_creation_and_listing
-#test_rwatch_creation_and_listing
-#test_awatch_creation_and_listing
-test_watchpoint_triggering
+mi_gdb_start
+
+# ... once using hardware watchpoints (if available).
+mi_gdb_test "890-gdb-set can-use-hw-watchpoints 1" \
+	"890\\^done" \
+	"hw watchpoints toggle (2)"
+test_watchpoint_all hw
 
 mi_gdb_exit
 return 0

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

[Daniel Jacobowitz]

On Fri, Sep 14, 2007 at 03:12:51PM +0100, Maciej W. Rozycki wrote:
> On Tue, 11 Sep 2007, Daniel Jacobowitz wrote:
> 
> > It isn't generally sufficient, either.  It's implemented on PowerPC
> > but the test still fails.  The frame pointer is corrupted one
> > instruction before the epilogue is detected, and I couldn't come up
> > with a reasonable way of making it work.
> 
>  Hmm, extending the range of the epilogue?  Just an obvious thought -- I 
> have no competence with PowerPC.

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.

> > In principle, I think so, but give it a day or two to see if anyone
> > objects.  How about xfailing the out of scope test for software
> > watchpoints?
> 
>  Well, should I take it as a declaration of no intent to fix them?

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 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.

> +    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: ".

Otherwise OK to commit.

-- 
Daniel Jacobowitz
CodeSourcery

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

[Maciej W. Rozycki]

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
 }

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

[Daniel Jacobowitz]

On Fri, Sep 14, 2007 at 04:28:36PM +0100, Maciej W. Rozycki wrote:
>  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.

Yes, some ABIs are so relaxed.  You can't migrate things past moving
the stack pointer (interrupts, signals) but the frame pointer is just
a convention on a lot of platforms.

> 
> > 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?

How about a double space after "$type: " in the output?  I bet there
is:

    if {[info exists pf_prefix]} {
        set message [concat $pf_prefix " " $message]
    }

(from dejagnu/framework.exp).

>  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.

It's quite hard to do.  I recommend the use of board files :-)

>  OK for this bit?

Yes, thanks again.

-- 
Daniel Jacobowitz
CodeSourcery

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

[Maciej W. Rozycki]

On Fri, 14 Sep 2007, Daniel Jacobowitz wrote:

> How about a double space after "$type: " in the output?  I bet there
> is:
> 
>     if {[info exists pf_prefix]} {
>         set message [concat $pf_prefix " " $message]
>     }
> 
> (from dejagnu/framework.exp).

 Looks like yours is the right direction -- it is "concat" that is 
responsible for making sure there is exactly one space between its 
arguments.  Thus I have now rewritten this bit as:

lappend pf_prefix "$type:"

-- truly in the TCL spirit. ;-)  Here's the final version.  Thanks for 
your inquisitiveness.

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

	[mti-fix-scope]
	* gdb.mi/mi-watch.exp (test_watchpoint_all): New function.
	Move all the tests here and run them twice, once using software
	watchpoints and once using hardware watchpoints.
	* gdb.mi/mi2-watch.exp (test_watchpoint_all): Likewise.

  Maciej

14678.diff
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:07:28.000000000 +0100
+++ binutils-quilt/src/gdb/testsuite/gdb.mi/mi2-watch.exp	2007-09-14 17:07:18.000000000 +0100
@@ -41,10 +41,6 @@
      return -1
 }
 
-mi_delete_breakpoints
-mi_gdb_reinitialize_dir $srcdir/$subdir
-mi_gdb_load ${binfile}
-
 proc test_watchpoint_creation_and_listing {} {
     global mi_gdb_prompt
     global srcfile
@@ -167,16 +163,48 @@
     }
 }
 
-# Disable hardware watchpoints if necessary.
+proc test_watchpoint_all {type} {
+    global pf_prefix
+    upvar srcdir srcdir
+    upvar subdir subdir
+    upvar binfile binfile
+
+    set old_prefix $pf_prefix
+    lappend pf_prefix "$type:"
+
+    mi_delete_breakpoints
+    mi_gdb_reinitialize_dir $srcdir/$subdir
+    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
+
+    set pf_prefix $old_prefix
+}
+
+# Run the tests twice, once using software watchpoints...
+mi_gdb_test "567-gdb-set can-use-hw-watchpoints 0" \
+	"567\\^done" \
+	"hw watchpoints toggle (1)"
+test_watchpoint_all sw
+
+mi_gdb_exit
+
+# ... and unless requested otherwise...
 if [target_info exists gdb,no_hardware_watchpoints] {
-    mi_gdb_test "-gdb-set can-use-hw-watchpoints 0" "\\^done" ""
+    return 0
 }
 
-mi_runto callee4
-test_watchpoint_creation_and_listing
-#test_rwatch_creation_and_listing
-#test_awatch_creation_and_listing
-test_watchpoint_triggering
+mi_gdb_start
+
+# ... once using hardware watchpoints (if available).
+mi_gdb_test "890-gdb-set can-use-hw-watchpoints 1" \
+	"890\\^done" \
+	"hw watchpoints toggle (2)"
+test_watchpoint_all hw
 
 mi_gdb_exit
 return 0
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:07:28.000000000 +0100
+++ binutils-quilt/src/gdb/testsuite/gdb.mi/mi-watch.exp	2007-09-14 17:06:49.000000000 +0100
@@ -41,10 +41,6 @@
      return -1
 }
 
-mi_delete_breakpoints
-mi_gdb_reinitialize_dir $srcdir/$subdir
-mi_gdb_load ${binfile}
-
 proc test_watchpoint_creation_and_listing {} {
     global mi_gdb_prompt
     global srcfile
@@ -167,16 +163,48 @@
     }
 }
 
-# Disable hardware watchpoints if necessary.
+proc test_watchpoint_all {type} {
+    global pf_prefix
+    upvar srcdir srcdir
+    upvar subdir subdir
+    upvar binfile binfile
+
+    set old_prefix $pf_prefix
+    lappend pf_prefix "$type:"
+
+    mi_delete_breakpoints
+    mi_gdb_reinitialize_dir $srcdir/$subdir
+    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
+
+    set pf_prefix $old_prefix
+}
+
+# Run the tests twice, once using software watchpoints...
+mi_gdb_test "567-gdb-set can-use-hw-watchpoints 0" \
+	"567\\^done" \
+	"hw watchpoints toggle (1)"
+test_watchpoint_all sw
+
+mi_gdb_exit
+
+# ... and unless requested otherwise...
 if [target_info exists gdb,no_hardware_watchpoints] {
-    mi_gdb_test "-gdb-set can-use-hw-watchpoints 0" "\\^done" ""
+    return 0
 }
 
-mi_runto callee4
-test_watchpoint_creation_and_listing
-#test_rwatch_creation_and_listing
-#test_awatch_creation_and_listing
-test_watchpoint_triggering
+mi_gdb_start
+
+# ... once using hardware watchpoints (if available).
+mi_gdb_test "890-gdb-set can-use-hw-watchpoints 1" \
+	"890\\^done" \
+	"hw watchpoints toggle (2)"
+test_watchpoint_all hw
 
 mi_gdb_exit
 return 0