[v2] Fix PR12526: -location watchpoints for bitfield arguments

Message ID 54186B53.7020809@redhat.com
State Committed
Headers

Commit Message

Pedro Alves Sept. 16, 2014, 4:54 p.m. UTC
  On 09/08/2014 01:10 AM, Sergio Durigan Junior wrote:
> On Sunday, September 07 2014, I wrote:
> 
>>> +    gdb_test "print $expr" "\\$\\d+ = $new\\s"
>>
>>   gdb_test "print $expr" "\\$\\d+ = $new\\s" \
>>       "check if expr == $new"
> 
> I forgot to mention that this is not enough to make the messages unique,
> so I added a "(old val = $old, new val = $new)" after the proposed
> message.

Hey Sergio,

For some odd reason, I completely missed your replies to Patrick,
and wanting to keep this moving, ended up redoing most of your
patch from scratch.  Gah.  /me bangs head in wall :-P.

As penance, I salvaged pieces from both patches, which I think
ends up being better than either version was.

 - function names a bit more concise
 - gdb.sum output is more concise too and shows patterns more clearly.
 - putting things in procedures allows run_to_main returning with
   canceling the other branch of the test (and allows easy hacking
   away part of the test when debugging it).
 - uses gdb_continue_to_end

This is what we have currently:

Running /home/pedro/gdb/mygit/build/../src/gdb/testsuite/gdb.base/watch-bitfields.exp ...
PASS: gdb.base/watch-bitfields.exp: watch -l q.a
PASS: gdb.base/watch-bitfields.exp: watch -l q.e
PASS: gdb.base/watch-bitfields.exp: print q.a
PASS: gdb.base/watch-bitfields.exp: cont
PASS: gdb.base/watch-bitfields.exp: print q.a
PASS: gdb.base/watch-bitfields.exp: print q.e
PASS: gdb.base/watch-bitfields.exp: cont
PASS: gdb.base/watch-bitfields.exp: print q.e
PASS: gdb.base/watch-bitfields.exp: print q.a
PASS: gdb.base/watch-bitfields.exp: cont
PASS: gdb.base/watch-bitfields.exp: print q.a
PASS: gdb.base/watch-bitfields.exp: print q.e
PASS: gdb.base/watch-bitfields.exp: cont
PASS: gdb.base/watch-bitfields.exp: print q.e
PASS: gdb.base/watch-bitfields.exp: cont
PASS: gdb.base/watch-bitfields.exp: watch q.d + q.f + q.g
PASS: gdb.base/watch-bitfields.exp: print q.d + q.f + q.g
PASS: gdb.base/watch-bitfields.exp: cont
PASS: gdb.base/watch-bitfields.exp: print q.d + q.f + q.g
PASS: gdb.base/watch-bitfields.exp: print q.d + q.f + q.g
PASS: gdb.base/watch-bitfields.exp: cont
PASS: gdb.base/watch-bitfields.exp: print q.d + q.f + q.g
PASS: gdb.base/watch-bitfields.exp: print q.d + q.f + q.g
PASS: gdb.base/watch-bitfields.exp: cont
PASS: gdb.base/watch-bitfields.exp: print q.d + q.f + q.g
PASS: gdb.base/watch-bitfields.exp: print q.d + q.f + q.g
PASS: gdb.base/watch-bitfields.exp: cont
PASS: gdb.base/watch-bitfields.exp: print q.d + q.f + q.g
PASS: gdb.base/watch-bitfields.exp: print q.d + q.f + q.g
PASS: gdb.base/watch-bitfields.exp: cont
PASS: gdb.base/watch-bitfields.exp: print q.d + q.f + q.g
PASS: gdb.base/watch-bitfields.exp: print q.d + q.f + q.g
PASS: gdb.base/watch-bitfields.exp: cont
PASS: gdb.base/watch-bitfields.exp: print q.d + q.f + q.g
PASS: gdb.base/watch-bitfields.exp: cont

This is the end result:

Running /home/pedro/gdb/mygit/build/../src/gdb/testsuite/gdb.base/watch-bitfields.exp ...
PASS: gdb.base/watch-bitfields.exp: -location watch against bitfields: watch -location q.a
PASS: gdb.base/watch-bitfields.exp: -location watch against bitfields: watch -location q.e
PASS: gdb.base/watch-bitfields.exp: -location watch against bitfields: q.a: 0->1: print expression before
PASS: gdb.base/watch-bitfields.exp: -location watch against bitfields: q.a: 0->1: continue
PASS: gdb.base/watch-bitfields.exp: -location watch against bitfields: q.a: 0->1: print expression after
PASS: gdb.base/watch-bitfields.exp: -location watch against bitfields: q.e: 0->5: print expression before
PASS: gdb.base/watch-bitfields.exp: -location watch against bitfields: q.e: 0->5: continue
PASS: gdb.base/watch-bitfields.exp: -location watch against bitfields: q.e: 0->5: print expression after
PASS: gdb.base/watch-bitfields.exp: -location watch against bitfields: q.a: 1->0: print expression before
PASS: gdb.base/watch-bitfields.exp: -location watch against bitfields: q.a: 1->0: continue
PASS: gdb.base/watch-bitfields.exp: -location watch against bitfields: q.a: 1->0: print expression after
PASS: gdb.base/watch-bitfields.exp: -location watch against bitfields: q.e: 5->4: print expression before
PASS: gdb.base/watch-bitfields.exp: -location watch against bitfields: q.e: 5->4: continue
PASS: gdb.base/watch-bitfields.exp: -location watch against bitfields: q.e: 5->4: print expression after
PASS: gdb.base/watch-bitfields.exp: -location watch against bitfields: continue until exit
PASS: gdb.base/watch-bitfields.exp: regular watch against bitfields: watch q.d + q.f + q.g
PASS: gdb.base/watch-bitfields.exp: regular watch against bitfields: q.d + q.f + q.g: 0->4: print expression before
PASS: gdb.base/watch-bitfields.exp: regular watch against bitfields: q.d + q.f + q.g: 0->4: continue
PASS: gdb.base/watch-bitfields.exp: regular watch against bitfields: q.d + q.f + q.g: 0->4: print expression after
PASS: gdb.base/watch-bitfields.exp: regular watch against bitfields: q.d + q.f + q.g: 4->10: print expression before
PASS: gdb.base/watch-bitfields.exp: regular watch against bitfields: q.d + q.f + q.g: 4->10: continue
PASS: gdb.base/watch-bitfields.exp: regular watch against bitfields: q.d + q.f + q.g: 4->10: print expression after
PASS: gdb.base/watch-bitfields.exp: regular watch against bitfields: q.d + q.f + q.g: 10->3: print expression before
PASS: gdb.base/watch-bitfields.exp: regular watch against bitfields: q.d + q.f + q.g: 10->3: continue
PASS: gdb.base/watch-bitfields.exp: regular watch against bitfields: q.d + q.f + q.g: 10->3: print expression after
PASS: gdb.base/watch-bitfields.exp: regular watch against bitfields: q.d + q.f + q.g: 3->2: print expression before
PASS: gdb.base/watch-bitfields.exp: regular watch against bitfields: q.d + q.f + q.g: 3->2: continue
PASS: gdb.base/watch-bitfields.exp: regular watch against bitfields: q.d + q.f + q.g: 3->2: print expression after
PASS: gdb.base/watch-bitfields.exp: regular watch against bitfields: q.d + q.f + q.g: 2->1: print expression before
PASS: gdb.base/watch-bitfields.exp: regular watch against bitfields: q.d + q.f + q.g: 2->1: continue
PASS: gdb.base/watch-bitfields.exp: regular watch against bitfields: q.d + q.f + q.g: 2->1: print expression after
PASS: gdb.base/watch-bitfields.exp: regular watch against bitfields: q.d + q.f + q.g: 1->0: print expression before
PASS: gdb.base/watch-bitfields.exp: regular watch against bitfields: q.d + q.f + q.g: 1->0: continue
PASS: gdb.base/watch-bitfields.exp: regular watch against bitfields: q.d + q.f + q.g: 1->0: print expression after
PASS: gdb.base/watch-bitfields.exp: regular watch against bitfields: continue until exit

WDYT?

----------
From d3f6e7f2ab3238ed98133d1f484da623dc55e8aa Mon Sep 17 00:00:00 2001
From: Sergio Durigan Junior <sergiodj@redhat.com>
Date: Tue, 16 Sep 2014 17:32:04 +0100
Subject: [PATCH] gdb.base/watch-bitfields.exp: Improve test

Make test messages unique, and a couple other tweaks.

gdb/testsuite/
2014-09-16  Sergio Durigan Junior  <sergiodj@redhat.com>
	    Pedro Alves  <palves@redhat.com>

	* gdb.base/watch-bitfields.exp: Pass string other than test file
	name to prepare_for_testing.
	(watch): New procedure.
	(expect_watchpoint): Use with_test_prefix.
	(top level): Factor out tests to ...
	(test_watch_location, test_regular_watch): ... these new
	procedures, and use with_test_prefix and gdb_continue_to_end.
---
 gdb/testsuite/gdb.base/watch-bitfields.exp | 77 ++++++++++++++++++++----------
 1 file changed, 51 insertions(+), 26 deletions(-)
  

Comments

Sergio Durigan Junior Sept. 16, 2014, 5:05 p.m. UTC | #1
On Tuesday, September 16 2014, Pedro Alves wrote:

> Hey Sergio,
>
> For some odd reason, I completely missed your replies to Patrick,
> and wanting to keep this moving, ended up redoing most of your
> patch from scratch.  Gah.  /me bangs head in wall :-P.

No worries :-).

> As penance, I salvaged pieces from both patches, which I think
> ends up being better than either version was.
>
>  - function names a bit more concise
>  - gdb.sum output is more concise too and shows patterns more clearly.
>  - putting things in procedures allows run_to_main returning with
>    canceling the other branch of the test (and allows easy hacking
>    away part of the test when debugging it).
>  - uses gdb_continue_to_end
>
> This is what we have currently:
>
[...]
>
> WDYT?

Looks neat, thanks for doing that!

> ----------
> From d3f6e7f2ab3238ed98133d1f484da623dc55e8aa Mon Sep 17 00:00:00 2001
> From: Sergio Durigan Junior <sergiodj@redhat.com>
> Date: Tue, 16 Sep 2014 17:32:04 +0100
> Subject: [PATCH] gdb.base/watch-bitfields.exp: Improve test
>
> Make test messages unique, and a couple other tweaks.
>
> gdb/testsuite/
> 2014-09-16  Sergio Durigan Junior  <sergiodj@redhat.com>
> 	    Pedro Alves  <palves@redhat.com>
>
> 	* gdb.base/watch-bitfields.exp: Pass string other than test file
> 	name to prepare_for_testing.
> 	(watch): New procedure.
> 	(expect_watchpoint): Use with_test_prefix.
> 	(top level): Factor out tests to ...
> 	(test_watch_location, test_regular_watch): ... these new
> 	procedures, and use with_test_prefix and gdb_continue_to_end.
> ---
>  gdb/testsuite/gdb.base/watch-bitfields.exp | 77 ++++++++++++++++++++----------
>  1 file changed, 51 insertions(+), 26 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.base/watch-bitfields.exp b/gdb/testsuite/gdb.base/watch-bitfields.exp
> index 3f25384..7b7fa22 100644
> --- a/gdb/testsuite/gdb.base/watch-bitfields.exp
> +++ b/gdb/testsuite/gdb.base/watch-bitfields.exp
> @@ -17,40 +17,65 @@
>
>  standard_testfile
>
> -if {[prepare_for_testing $testfile.exp $testfile $srcfile debug]} {
> +if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
>      return -1
>  }
>
> -if {![runto_main]} {
> -    return -1
> +# Set a watchpoint watching EXPR.
> +proc watch { expr } {
> +    global decimal
> +
> +    set expr_re [string_to_regexp $expr]
> +    gdb_test "watch $expr" \
> +	"\(Hardware \)?\[Ww\]atchpoint $decimal: $expr_re"
>  }
>
>  # Continue inferior execution, expecting the watchpoint EXPR to be triggered
>  # having old value OLD and new value NEW.
>  proc expect_watchpoint { expr old new } {
> -    set expr_re [string_to_regexp $expr]
> -    gdb_test "print $expr" "\\$\\d+ = $old\\s"
> -    gdb_test "cont" "$expr_re\\s.*Old value = $old\\s+New value = $new\\s.*"
> -    gdb_test "print $expr" "\\$\\d+ = $new\\s"
> +    with_test_prefix "$expr: $old->$new" {
> +	set expr_re [string_to_regexp $expr]
> +	gdb_test "print $expr" "\\$\\d+ = $old\\s" "print expression before"
> +	gdb_test "continue" "$expr_re\\s.*Old value = $old\\s+New value = $new\\s.*"
> +	gdb_test "print $expr" "\\$\\d+ = $new\\s" "print expression after"
> +    }
>  }
>
>  # Check that -location watchpoints against bitfields trigger properly.
> -gdb_test "watch -l q.a"
> -gdb_test "watch -l q.e"
> -expect_watchpoint "q.a" 0 1
> -expect_watchpoint "q.e" 0 5
> -expect_watchpoint "q.a" 1 0
> -expect_watchpoint "q.e" 5 4
> -gdb_test "cont" ".*exited normally.*"
> -
> -# Check that regular watchpoints against expressions involving bitfields
> -# trigger properly.
> -runto_main
> -gdb_test "watch q.d + q.f + q.g"
> -expect_watchpoint "q.d + q.f + q.g" 0 4
> -expect_watchpoint "q.d + q.f + q.g" 4 10
> -expect_watchpoint "q.d + q.f + q.g" 10 3
> -expect_watchpoint "q.d + q.f + q.g" 3 2
> -expect_watchpoint "q.d + q.f + q.g" 2 1
> -expect_watchpoint "q.d + q.f + q.g" 1 0
> -gdb_test "cont" ".*exited normally.*"
> +proc test_watch_location {} {
> +    with_test_prefix "-location watch against bitfields" {
> +	if {![runto_main]} {
> +	    return -1
> +	}
> +
> +	watch "-location q.a"
> +	watch "-location q.e"
> +	expect_watchpoint "q.a" 0 1
> +	expect_watchpoint "q.e" 0 5
> +	expect_watchpoint "q.a" 1 0
> +	expect_watchpoint "q.e" 5 4
> +	gdb_continue_to_end
> +    }
> +}
> +
> +# Check that regular watchpoints against expressions involving
> +# bitfields trigger properly.
> +proc test_regular_watch {} {
> +    with_test_prefix "regular watch against bitfields" {
> +	if {![runto_main]} {
> +	    return -1
> +	}
> +
> +	watch "q.d + q.f + q.g"
> +	expect_watchpoint "q.d + q.f + q.g" 0 4
> +	expect_watchpoint "q.d + q.f + q.g" 4 10
> +	expect_watchpoint "q.d + q.f + q.g" 10 3
> +	expect_watchpoint "q.d + q.f + q.g" 3 2
> +	expect_watchpoint "q.d + q.f + q.g" 2 1
> +	expect_watchpoint "q.d + q.f + q.g" 1 0
> +	gdb_continue_to_end
> +    }
> +}
> +
> +test_watch_location
> +test_regular_watch
> -- 
> 1.9.3

The patch looks good to me.

Thanks!
  
Pedro Alves Sept. 16, 2014, 5:09 p.m. UTC | #2
On 09/16/2014 06:05 PM, Sergio Durigan Junior wrote:
> On Tuesday, September 16 2014, Pedro Alves wrote:

>> For some odd reason, I completely missed your replies to Patrick,
>> and wanting to keep this moving, ended up redoing most of your
>> patch from scratch.  Gah.  /me bangs head in wall :-P.
> 
> No worries :-).

Phew.  :-)

> Looks neat, thanks for doing that!

...

> The patch looks good to me.
> 
> Thanks!

Thank you!  Now pushed.

Pedro Alves
  
Patrick Palka Sept. 17, 2014, 1 p.m. UTC | #3
Awesome!  Thanks to the both of you, for your help and guidance.
  

Patch

diff --git a/gdb/testsuite/gdb.base/watch-bitfields.exp b/gdb/testsuite/gdb.base/watch-bitfields.exp
index 3f25384..7b7fa22 100644
--- a/gdb/testsuite/gdb.base/watch-bitfields.exp
+++ b/gdb/testsuite/gdb.base/watch-bitfields.exp
@@ -17,40 +17,65 @@ 

 standard_testfile

-if {[prepare_for_testing $testfile.exp $testfile $srcfile debug]} {
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
     return -1
 }

-if {![runto_main]} {
-    return -1
+# Set a watchpoint watching EXPR.
+proc watch { expr } {
+    global decimal
+
+    set expr_re [string_to_regexp $expr]
+    gdb_test "watch $expr" \
+	"\(Hardware \)?\[Ww\]atchpoint $decimal: $expr_re"
 }

 # Continue inferior execution, expecting the watchpoint EXPR to be triggered
 # having old value OLD and new value NEW.
 proc expect_watchpoint { expr old new } {
-    set expr_re [string_to_regexp $expr]
-    gdb_test "print $expr" "\\$\\d+ = $old\\s"
-    gdb_test "cont" "$expr_re\\s.*Old value = $old\\s+New value = $new\\s.*"
-    gdb_test "print $expr" "\\$\\d+ = $new\\s"
+    with_test_prefix "$expr: $old->$new" {
+	set expr_re [string_to_regexp $expr]
+	gdb_test "print $expr" "\\$\\d+ = $old\\s" "print expression before"
+	gdb_test "continue" "$expr_re\\s.*Old value = $old\\s+New value = $new\\s.*"
+	gdb_test "print $expr" "\\$\\d+ = $new\\s" "print expression after"
+    }
 }

 # Check that -location watchpoints against bitfields trigger properly.
-gdb_test "watch -l q.a"
-gdb_test "watch -l q.e"
-expect_watchpoint "q.a" 0 1
-expect_watchpoint "q.e" 0 5
-expect_watchpoint "q.a" 1 0
-expect_watchpoint "q.e" 5 4
-gdb_test "cont" ".*exited normally.*"
-
-# Check that regular watchpoints against expressions involving bitfields
-# trigger properly.
-runto_main
-gdb_test "watch q.d + q.f + q.g"
-expect_watchpoint "q.d + q.f + q.g" 0 4
-expect_watchpoint "q.d + q.f + q.g" 4 10
-expect_watchpoint "q.d + q.f + q.g" 10 3
-expect_watchpoint "q.d + q.f + q.g" 3 2
-expect_watchpoint "q.d + q.f + q.g" 2 1
-expect_watchpoint "q.d + q.f + q.g" 1 0
-gdb_test "cont" ".*exited normally.*"
+proc test_watch_location {} {
+    with_test_prefix "-location watch against bitfields" {
+	if {![runto_main]} {
+	    return -1
+	}
+
+	watch "-location q.a"
+	watch "-location q.e"
+	expect_watchpoint "q.a" 0 1
+	expect_watchpoint "q.e" 0 5
+	expect_watchpoint "q.a" 1 0
+	expect_watchpoint "q.e" 5 4
+	gdb_continue_to_end
+    }
+}
+
+# Check that regular watchpoints against expressions involving
+# bitfields trigger properly.
+proc test_regular_watch {} {
+    with_test_prefix "regular watch against bitfields" {
+	if {![runto_main]} {
+	    return -1
+	}
+
+	watch "q.d + q.f + q.g"
+	expect_watchpoint "q.d + q.f + q.g" 0 4
+	expect_watchpoint "q.d + q.f + q.g" 4 10
+	expect_watchpoint "q.d + q.f + q.g" 10 3
+	expect_watchpoint "q.d + q.f + q.g" 3 2
+	expect_watchpoint "q.d + q.f + q.g" 2 1
+	expect_watchpoint "q.d + q.f + q.g" 1 0
+	gdb_continue_to_end
+    }
+}
+
+test_watch_location
+test_regular_watch