adjust watchpoint testing

adjust watchpoint testing

[Nathan Sidwell]

[-- Attachment #1: Type: text/plain, Size: 475 bytes --]

the recurse.exp test presume there are at least two hardware watchpoints.  This
patch adds a gdb,hardware_watchpoint target info variable and adjust the test to
only test where two watchpoints are used, if they exist on the target.

I found it necessary to adjust the watchpoint.exp expected reply too.

ok?

nathan
-- 
Nathan Sidwell    ::   http://www.codesourcery.com   ::         CodeSourcery
nathan@codesourcery.com    ::     http://www.planetfall.pwp.blueyonder.co.uk


[-- Attachment #2: test-watch.patch --]
[-- Type: text/x-patch, Size: 6357 bytes --]

2006-06-09  Nathan Sidwell  <nathan@codesourcery.com>

	gdb/testsuite/
	* gdb.base/recurse.exp: Add gdb,hardware_watchpoint target
	info.  Adjust testing to cope with a single watchpoint.
	* gdb.base/watchpoint.exp: Adjust expected string.

Index: gdb/testsuite/gdb.base/recurse.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/recurse.exp,v
retrieving revision 1.5
diff -c -3 -p -r1.5 recurse.exp
*** gdb/testsuite/gdb.base/recurse.exp	19 Sep 2001 08:21:17 -0000	1.5
--- gdb/testsuite/gdb.base/recurse.exp	9 Jun 2006 20:50:09 -0000
*************** proc recurse_tests {} {
*** 47,52 ****
--- 47,56 ----
      if [target_info exists gdb,no_hardware_watchpoints] {
  	gdb_test "set can-use-hw-watchpoints 0" "" ""
      }
+     set nwatch 99
+     if [target_info exists gdb,hardware_watchpoints] {
+ 	set nwatch [target_info gdb,hardware_watchpoints]
+     }
  
      if [runto recurse] then {
  	# First we need to step over the assignment of b, so it has a known
*************** proc recurse_tests {} {
*** 78,91 ****
  	# First we need to step over the assignment of b, so it has a known
  	# value.
  	gdb_test "next" "if \\(a == 1\\)" "next over b = 0 in second instance"
- 	gdb_test "watch b" ".*\[Ww\]atchpoint \[0-9]*: b" \
- 	    "set second instance watchpoint"
  
! 	# Continue until initial set of b (second instance).
! 	if [gdb_test "continue" \
! 	    "Continuing.*\[Ww\]atchpoint.*: b.*Old value = 0.*New value = 5.*"\
! 	    "continue to second instance watchpoint, first time"] then {
! 	gdb_suppress_tests;
  	}
  
  	# Continue inward for a few iterations
--- 82,98 ----
  	# First we need to step over the assignment of b, so it has a known
  	# value.
  	gdb_test "next" "if \\(a == 1\\)" "next over b = 0 in second instance"
  
! 	if { $nwatch > 1 } {
! 	    gdb_test "watch b" ".*\[Ww\]atchpoint \[0-9]*: b" \
! 		"set second instance watchpoint"
! 
! 	    # Continue until initial set of b (second instance).
! 	    if [gdb_test "continue" \
! 		    "Continuing.*\[Ww\]atchpoint.*: b.*Old value = 0.*New value = 5.*"\
! 		    "continue to second instance watchpoint, first time"] then {
! 		gdb_suppress_tests;
! 	    }
  	}
  
  	# Continue inward for a few iterations
*************** proc recurse_tests {} {
*** 98,128 ****
  	gdb_test "continue" "Breakpoint.* recurse \\(a=1\\).*" \
  	    "continue to recurse (a = 1)"
  
! 	# Continue until second set of b (second instance).
! 	if [gdb_test "continue" \
! 	    "Continuing.*\[Ww\]atchpoint.*: b.*Old value = 5.*New value = 120.*return.*" \
! 	    "continue to second instance watchpoint, second time"] then { 
! 	    gdb_suppress_tests;
  	}
! 
! 	# Continue again.  We should have a watchpoint go out of scope now
! 	if [gdb_test "continue" \
! 	    "Continuing.*\[Ww\]atchpoint.*deleted.*recurse \\(a=6\\) .*" \
! 	    "second instance watchpoint deleted when leaving scope"] then {
! 	    gdb_suppress_tests;
! 	}
! 
  	# Continue until second set of b (first instance).
  	# 24320 is allowed as the final value for b as that's the value
  	# b would have on systems with 16bit integers.
  	#
  	# We could fix the test program to deal with this too.
  	if [gdb_test "continue" \
! 	    "Continuing.*\[Ww\]atchpoint.*b.*Old value = 10.*New value = \(3628800|24320\).*return.*" \
! 	    "continue to first instance watchpoint, second time"] then {
  	    gdb_suppress_tests
  	}
! 
  	# Continue again.  We should have a watchpoint go out of scope now.
  	#
  	# The former version expected the test to return to main().
--- 105,137 ----
  	gdb_test "continue" "Breakpoint.* recurse \\(a=1\\).*" \
  	    "continue to recurse (a = 1)"
  
! 	if { $nwatch  > 1 } {
! 	    # Continue until second set of b (second instance).
! 	    if [gdb_test "continue" \
! 		    "Continuing.*\[Ww\]atchpoint.*: b.*Old value = 5.*New value = 120.*return.*" \
! 		    "continue to second instance watchpoint, second time"] then { 
! 		gdb_suppress_tests;
! 	    }
! 
! 	    # Continue again.  We should have a watchpoint go out of scope now
! 	    if [gdb_test "continue" \
! 		    "Continuing.*\[Ww\]atchpoint.*deleted.*recurse \\(a=6\\) .*" \
! 		    "second instance watchpoint deleted when leaving scope"] then {
! 		gdb_suppress_tests;
! 	    }
  	}
! 	
  	# Continue until second set of b (first instance).
  	# 24320 is allowed as the final value for b as that's the value
  	# b would have on systems with 16bit integers.
  	#
  	# We could fix the test program to deal with this too.
  	if [gdb_test "continue" \
! 		"Continuing.*\[Ww\]atchpoint.*b.*Old value = 10.*New value = \(3628800|24320\).*return.*" \
! 		"continue to first instance watchpoint, second time"] then {
  	    gdb_suppress_tests
  	}
! 	
  	# Continue again.  We should have a watchpoint go out of scope now.
  	#
  	# The former version expected the test to return to main().
Index: gdb/testsuite/gdb.base/watchpoint.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/watchpoint.exp,v
retrieving revision 1.11
diff -c -3 -p -r1.11 watchpoint.exp
*** gdb/testsuite/gdb.base/watchpoint.exp	22 Mar 2004 14:24:18 -0000	1.11
--- gdb/testsuite/gdb.base/watchpoint.exp	9 Jun 2006 20:50:10 -0000
*************** proc initialize {} {
*** 120,126 ****
        return 0; 
      }
  
- 
      return 1
  }
  
--- 120,125 ----
*************** proc test_simple_watchpoint {} {
*** 148,154 ****
      gdb_run_cmd
      set timeout 600
      gdb_expect {
! 	-re "Breakpoint 1, marker1 .*$gdb_prompt $" {
  	    pass "run to marker1 in test_simple_watchpoint"
  	}
  	-re ".*$gdb_prompt $" {
--- 147,153 ----
      gdb_run_cmd
      set timeout 600
      gdb_expect {
! 	-re "Breakpoint 1, .*marker1 .*$gdb_prompt $" {
  	    pass "run to marker1 in test_simple_watchpoint"
  	}
  	-re ".*$gdb_prompt $" {
*************** proc test_disabling_watchpoints {} {
*** 314,320 ****
      gdb_run_cmd
      set timeout 600
      gdb_expect {
! 	-re "Breakpoint 1, marker1 .*$gdb_prompt $" {
  	    pass "run to marker1 in test_disabling_watchpoints"
  	}
  	-re ".*$gdb_prompt $" {
--- 313,319 ----
      gdb_run_cmd
      set timeout 600
      gdb_expect {
! 	-re "Breakpoint 1, .*marker1 .*$gdb_prompt $" {
  	    pass "run to marker1 in test_disabling_watchpoints"
  	}
  	-re ".*$gdb_prompt $" {

Re: adjust watchpoint testing

[Wu Zhou]

Nathan,

I had similar concerns about these watchpoint tests with you.  However, 
I'd like to propose a somewhat different solution.

On Fri, 9 Jun 2006, Nathan Sidwell wrote:

> the recurse.exp test presume there are at least two hardware watchpoints.  

In my opinion, recurse.exp doesn't presume that. If there is no hardware 
watchpoint support, it will use software simulated ones.  If the target 
can support only one h/w watchpoint, the second will switch to software 
watchpoint.

This target-dependent feature might incur confusion some where.  One 
may have difficulty in knowing for sure what this case is trying to 
test: s/w watchpoint, or h/w watchpoint, or both?  If there are errors 
reported, what type of watchpoint trigger that?  To track into the root 
cause, which code to look at? 

To clarify the situation, I propose to let recurse.exp based on totally on 
software simulated watchpoint.  This can be done easily, by using "set 
can-use-hardware-watchpoint 0".

Then we can add another similar testcase, specifically designed for 
hardware watchpoint. This testcase will first detect if there are any 
hardware watchpoint in the underlying target.  If there are any, then go 
on with the test, otherwise skip and quit.

In this h/w watchpoint specific testcase, we can also add some tests for 
rwatch and awatch, which is not touched by any testcase in current gdb  
testsuite (that is my reading of the code, correct me if I am wrong).

For simplicity, we can use only one watchpoint. This has another strength 
in that it don't make any assumption.

This is my rough thought. I had some code, but not finished yet.  So I am 
very appreciated if anyone can provide any comment or insight into my 
proposal.  Thanks a lot in advance.

Regards
- Wu Zhou 

> This
> patch adds a gdb,hardware_watchpoint target info variable and adjust the test to
> only test where two watchpoints are used, if they exist on the target.
> 
> I found it necessary to adjust the watchpoint.exp expected reply too.
> 
> ok?
> 
> nathan
> -- 
> Nathan Sidwell    ::   http://www.codesourcery.com   ::         CodeSourcery
> nathan@codesourcery.com    ::     http://www.planetfall.pwp.blueyonder.co.uk
>  

Re: adjust watchpoint testing

[Nathan Sidwell]

Wu Zhou wrote:
> Nathan,
> 
> I had similar concerns about these watchpoint tests with you.  However, 
> I'd like to propose a somewhat different solution.

> This target-dependent feature might incur confusion some where.  One 
> may have difficulty in knowing for sure what this case is trying to 
> test: s/w watchpoint, or h/w watchpoint, or both?  If there are errors 
> reported, what type of watchpoint trigger that?  To track into the root 
> cause, which code to look at? 
> 
> To clarify the situation, I propose to let recurse.exp based on totally on 
> software simulated watchpoint.  This can be done easily, by using "set 
> can-use-hardware-watchpoint 0".

Thanks for your comments. Over the weekend, I had a thought that the problem is 
the remote stub not telling gdb how many watchpoints it can support.  I think 
this is a thing for qPacketInfo to deal with, and I can talk to Dan about that.

nathan
-- 
Nathan Sidwell    ::   http://www.codesourcery.com   ::         CodeSourcery
nathan@codesourcery.com    ::     http://www.planetfall.pwp.blueyonder.co.uk

Re: adjust watchpoint testing

[Wu Zhou]

Nathan,

I see your code have such two lines:

+     if [target_info exists gdb,hardware_watchpoints] {
+       set nwatch [target_info gdb,hardware_watchpoints]

Where is this target_info variable set up?  Did you set that somewhere in 
the dejagnu package?  I am very curious about how it is implemented.

I am now using a test and trial method.  If rwatch command reports that 
this type of hw watchpoint is not supported, then I will assume this 
target don't support hardware watchpoint; if it replys with a message 
"Hardware read watchpoint...", then I assume it is supported.  But I 
don't figure out how to get precisely how many hw watchpoints the 
target support?  How did you make that?  Can you share your idea with 
me?  Thanks a lot!

Regards
- Wu Zhou

On Mon, 12 Jun 2006, Nathan Sidwell wrote:

> Wu Zhou wrote:
> > Nathan,
> > 
> > I had similar concerns about these watchpoint tests with you.  However, I'd
> > like to propose a somewhat different solution.
> 
> > This target-dependent feature might incur confusion some where.  One may
> > have difficulty in knowing for sure what this case is trying to test: s/w
> > watchpoint, or h/w watchpoint, or both?  If there are errors reported, what
> > type of watchpoint trigger that?  To track into the root cause, which code
> > to look at? 
> > To clarify the situation, I propose to let recurse.exp based on totally on
> > software simulated watchpoint.  This can be done easily, by using "set
> > can-use-hardware-watchpoint 0".
> 
> Thanks for your comments. Over the weekend, I had a thought that the problem
> is the remote stub not telling gdb how many watchpoints it can support.  I
> think this is a thing for qPacketInfo to deal with, and I can talk to Dan
> about that.
> 
> nathan
> -- 
> Nathan Sidwell    ::   http://www.codesourcery.com   ::         CodeSourcery
> nathan@codesourcery.com    ::     http://www.planetfall.pwp.blueyonder.co.uk
> 
> 

Re: adjust watchpoint testing

[Nathan Sidwell]

Wu Zhou wrote:
> Nathan,
> 
> I see your code have such two lines:
> 
> +     if [target_info exists gdb,hardware_watchpoints] {
> +       set nwatch [target_info gdb,hardware_watchpoints]
> 
> Where is this target_info variable set up?  Did you set that somewhere in 
> the dejagnu package?  I am very curious about how it is implemented.

you need to add a line to the dejagnu board support file to initialize it.  In 
just the same way as things like gdb,nosignals is set.

nathan

-- 
Nathan Sidwell    ::   http://www.codesourcery.com   ::         CodeSourcery
nathan@codesourcery.com    ::     http://www.planetfall.pwp.blueyonder.co.uk

Re: adjust watchpoint testing

[Wu Zhou]

On Tue, 13 Jun 2006, Nathan Sidwell wrote:

> Wu Zhou wrote:
> > Nathan,
> > 
> > I see your code have such two lines:
> > 
> > +     if [target_info exists gdb,hardware_watchpoints] {
> > +       set nwatch [target_info gdb,hardware_watchpoints]
> > 
> > Where is this target_info variable set up?  Did you set that somewhere in
> > the dejagnu package?  I am very curious about how it is implemented.
> 
> you need to add a line to the dejagnu board support file to initialize it.  In
> just the same way as things like gdb,nosignals is set.

Do you mean to hard-wire the number of the hw watchpoint the underlying target
support into these base board files, or just set the number to 1?

I guess that we might need to work out a way to detect the number 
automatically. Any thought?

Regards
- Wu Zhou

Re: adjust watchpoint testing

[Nathan Sidwell]

Wu Zhou wrote:

> Do you mean to hard-wire the number of the hw watchpoint the underlying target
> support into these base board files, or just set the number to 1?

the base board file knows the characteristics of the board, so is the 
appropriate place to set this value.

> 
> I guess that we might need to work out a way to detect the number 
> automatically. Any thought?

yes, that's what I was talking about with qPacketInfo

nathan
-- 
Nathan Sidwell    ::   http://www.codesourcery.com   ::         CodeSourcery
nathan@codesourcery.com    ::     http://www.planetfall.pwp.blueyonder.co.uk

Re: adjust watchpoint testing

[Wu Zhou]

On Tue, 13 Jun 2006, Nathan Sidwell wrote:

> Wu Zhou wrote:
> 
> > Do you mean to hard-wire the number of the hw watchpoint the underlying
> > target
> > support into these base board files, or just set the number to 1?
> 
> the base board file knows the characteristics of the board, so is the
> appropriate place to set this value.
> 
> > 
> > I guess that we might need to work out a way to detect the number
> > automatically. Any thought?
> 
> yes, that's what I was talking about with qPacketInfo

qPacketInfo is for remote debugging.  What about native local debugging?

Thanks.
- Wu Zhou

Re: adjust watchpoint testing

[Nathan Sidwell]

Wu Zhou wrote:

> 
> qPacketInfo is for remote debugging.  What about native local debugging?

I don't know.  I thought that local debugging just dtrt in this regard.

nathan
-- 
Nathan Sidwell    ::   http://www.codesourcery.com   ::         CodeSourcery
nathan@codesourcery.com    ::     http://www.planetfall.pwp.blueyonder.co.uk

Re: adjust watchpoint testing

[Eli Zaretskii]

> Date: Tue, 13 Jun 2006 14:43:13 +0800 (CST)
> From: Wu Zhou <woodzltc@cn.ibm.com>
> cc: gdb-patches@sourceware.org
> 
> Do you mean to hard-wire the number of the hw watchpoint the underlying target
> support into these base board files, or just set the number to 1?
> 
> I guess that we might need to work out a way to detect the number 
> automatically. Any thought?

I don't think there's any practical way to detect the number of
supported watchpoints.  It depends on the particulars of the watched
regions, and different targets have different peculiarities.  For
example, x86 can set an unlimited number of watchpoints provided that
they watch the same address and length.

Re: adjust watchpoint testing

[Wu Zhou]

On Tue, 13 Jun 2006, Eli Zaretskii wrote:

> > Date: Tue, 13 Jun 2006 14:43:13 +0800 (CST)
> > From: Wu Zhou <woodzltc@cn.ibm.com>
> > cc: gdb-patches@sourceware.org
> > 
> > Do you mean to hard-wire the number of the hw watchpoint the underlying target
> > support into these base board files, or just set the number to 1?
> > 
> > I guess that we might need to work out a way to detect the number 
> > automatically. Any thought?
> 
> I don't think there's any practical way to detect the number of
> supported watchpoints.  It depends on the particulars of the watched
> regions, and different targets have different peculiarities.  For
> example, x86 can set an unlimited number of watchpoints provided that
> they watch the same address and length.

What about adding a gdbarch specific member function to do this?  If this 
is still target dependent, we can overwrite it in the relevant tdep file.

For x86 case, maybe we can take them as only one watchpoint if they are 
watching on the same address of same length?  

My very initial idea though.  Feel free to correct me if I am wrong.

Thanks
- Wu Zhou

Re: adjust watchpoint testing

[Daniel Jacobowitz]

On Wed, Jun 14, 2006 at 09:21:52AM +0800, Wu Zhou wrote:
> 
> On Tue, 13 Jun 2006, Eli Zaretskii wrote:
> 
> > > Date: Tue, 13 Jun 2006 14:43:13 +0800 (CST)
> > > From: Wu Zhou <woodzltc@cn.ibm.com>
> > > cc: gdb-patches@sourceware.org
> > > 
> > > Do you mean to hard-wire the number of the hw watchpoint the underlying target
> > > support into these base board files, or just set the number to 1?
> > > 
> > > I guess that we might need to work out a way to detect the number 
> > > automatically. Any thought?
> > 
> > I don't think there's any practical way to detect the number of
> > supported watchpoints.  It depends on the particulars of the watched
> > regions, and different targets have different peculiarities.  For
> > example, x86 can set an unlimited number of watchpoints provided that
> > they watch the same address and length.

I'm not entirely sure why this would be specific to x86.  The
x86-specific code handles it, but any other target could too
(I'd rather it were handled at a higher layer, but the current
breakpoint.c layer is too high for that; there may be a middle layer
someday; we've discussed this recently, I think.)

> What about adding a gdbarch specific member function to do this?  If this 
> is still target dependent, we can overwrite it in the relevant tdep file.

The problem is that it is not architecture-specific, but
board-specific.

I think that, if someone wanted, we could write a features
description which explained what hardware watchpoint and breakpoint
resources were available.  While there are quite a variety of types in
existance, they follow some basic templates, which could be stored
in GDB.  I spoke briefly with Nathan about this earlier.  My general
opinion is that it's a big project with relatively little return, so
until I have a target board where it would be particularly valuable,
I'm not planning to think about it too hard.  But if anyone else thinks
this sounds like an interesting project, and wants to learn a whole lot
about the different sorts of watchpoints out there and about XML
schemas, ask me about it and I'll try to put my thoughts in order :-)


-- 
Daniel Jacobowitz
CodeSourcery

Re: adjust watchpoint testing

[Eli Zaretskii]

> Date: Wed, 14 Jun 2006 09:21:52 +0800 (CST)
> From: Wu Zhou <woodzltc@cn.ibm.com>
> cc: nathan@codesourcery.com, gdb-patches@sourceware.org
> 
> > I don't think there's any practical way to detect the number of
> > supported watchpoints.  It depends on the particulars of the watched
> > regions, and different targets have different peculiarities.  For
> > example, x86 can set an unlimited number of watchpoints provided that
> > they watch the same address and length.
> 
> What about adding a gdbarch specific member function to do this?

What would that function do?  I thought I explained that the question
``how many watchpoints can the target give us'' has no meaningful
answer, unless you also supply the specific mix of addresses and sizes
of the regions you wish to watch.

Re: adjust watchpoint testing

[Daniel Jacobowitz]

On Tue, Jun 13, 2006 at 02:12:30PM +0800, Wu Zhou wrote:
> I am now using a test and trial method.  If rwatch command reports that 
> this type of hw watchpoint is not supported, then I will assume this 
> target don't support hardware watchpoint; if it replys with a message 
> "Hardware read watchpoint...", then I assume it is supported.  But I 
> don't figure out how to get precisely how many hw watchpoints the 
> target support?  How did you make that?  Can you share your idea with 
> me?  Thanks a lot!

This is the right way to do it in the testsuite.  Nathan's suggestion
just makes it easier for GDB to give the right responses when doing
this on a remote target.

-- 
Daniel Jacobowitz
CodeSourcery