[RFA] Fix setting of VSX registers

[RFA] Fix setting of VSX registers

[Thiago Jung Bauermann]

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

Hi,

ppc-linux-nat.c:store_vsx_register fetches the VSX registers into a
buffer, changes the value of the relevant register and then stores the
buffer again with ptrace.

The problem is, the function was "fetching" the VSX registers using
PTRACE_SETVSXREGS instead of PTRACE_GETVSXREGS. Ouch.

This patch fixes the typo, and also fixes the vsx-regs.exp testcase to
use gdb_test instead of send_gdb (this also fixes some synchronization
issues in the test), and updates the expect info reg output with the new
v2_double member.

gdbserver doesn't have this bug, the vsx-regs.exp test passes there
(except for the core file tests which are unsupported in gdbserver).

There are no regressions in the testsuite. Ok to commit? Ok for the
branch too?

-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center


2010-07-20  Thiago Jung Bauermann  <bauerman@br.ibm.com>

gdb/
	* ppc-linux-nat.c (store_vsx_register): Use PTRACE_GETVSXREGS to get
	VSX registers contents.

gdb/testsuite/
	* gdb.arch/vsx-regs.exp: Remove wrong comment about testing AltiVec
	registers.  Update data sets with the new v2_double element in the VSX
	register union.  Add vector_register3_vr data set for the AltiVec
	registers.  Use gdb_test instead of send_gdb.



[-- Attachment #2: vsx.diff --]
[-- Type: text/x-patch, Size: 4909 bytes --]

diff --git a/gdb/ppc-linux-nat.c b/gdb/ppc-linux-nat.c
index e8d96f6..18ddee7 100644
--- a/gdb/ppc-linux-nat.c
+++ b/gdb/ppc-linux-nat.c
@@ -877,7 +877,7 @@ store_vsx_register (const struct regcache *regcache, int tid, int regno)
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   int vsxregsize = register_size (gdbarch, tdep->ppc_vsr0_upper_regnum);
 
-  ret = ptrace (PTRACE_SETVSXREGS, tid, 0, &regs);
+  ret = ptrace (PTRACE_GETVSXREGS, tid, 0, &regs);
   if (ret < 0)
     {
       if (errno == EIO)
diff --git a/gdb/testsuite/gdb.arch/vsx-regs.exp b/gdb/testsuite/gdb.arch/vsx-regs.exp
index f310a6f..117ff25 100644
--- a/gdb/testsuite/gdb.arch/vsx-regs.exp
+++ b/gdb/testsuite/gdb.arch/vsx-regs.exp
@@ -14,8 +14,6 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
 
-# Tests for Powerpc AltiVec register setting and fetching
-
 if $tracelevel then {
     strace $tracelevel
 }
@@ -66,11 +64,13 @@ if ![runto_main] then {
 
 # Data sets used throughout the test
 
-set vector_register1 ".uint128 = 0x3ff4cccccccccccc0000000000000000, v4_float = .0x1, 0xf99999a0, 0x0, 0x0., v4_int32 = .0x3ff4cccc, 0xcccccccc, 0x0, 0x0., v8_int16 = .0x3ff4, 0xcccc, 0xcccc, 0xcccc, 0x0, 0x0, 0x0, 0x0., v16_int8 = .0x3f, 0xf4, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0.."
+set vector_register1 ".uint128 = 0x3ff4cccccccccccc0000000000000000, v2_double = .0x1, 0x0., v4_float = .0x1, 0xf99999a0, 0x0, 0x0., v4_int32 = .0x3ff4cccc, 0xcccccccc, 0x0, 0x0., v8_int16 = .0x3ff4, 0xcccc, 0xcccc, 0xcccc, 0x0, 0x0, 0x0, 0x0., v16_int8 = .0x3f, 0xf4, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0.."
+
+set vector_register2 "uint128 = 0xdeadbeefdeadbeefdeadbeefdeadbeef, v2_double = .0x1, 0x1., v4_float = .0x0, 0x0, 0x0, 0x0., v4_int32 = .0xdeadbeef, 0xdeadbeef, 0xdeadbeef, 0xdeadbeef., v8_int16 = .0xdead, 0xbeef, 0xdead, 0xbeef, 0xdead, 0xbeef, 0xdead, 0xbeef., v16_int8 = .0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef.."
 
-set vector_register2 "uint128 = 0xdeadbeefdeadbeefdeadbeefdeadbeef, v4_float = .0x0, 0x0, 0x0, 0x0., v4_int32 = .0xdeadbeef, 0xdeadbeef, 0xdeadbeef, 0xdeadbeef., v8_int16 = .0xdead, 0xbeef, 0xdead, 0xbeef, 0xdead, 0xbeef, 0xdead, 0xbeef., v16_int8 = .0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef.."
+set vector_register3 ".uint128 = 0x00000001000000010000000100000001, v2_double = .0x0, 0x0., v4_float = .0x0, 0x0, 0x0, 0x0., v4_int32 = .0x1, 0x1, 0x1, 0x1., v8_int16 = .0x0, 0x1, 0x0, 0x1, 0x0, 0x1, 0x0, 0x1., v16_int8 = .0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x1.."
 
-set vector_register3 ".uint128 = 0x00000001000000010000000100000001, v4_float = .0x0, 0x0, 0x0, 0x0., v4_int32 = .0x1, 0x1, 0x1, 0x1., v8_int16 = .0x0, 0x1, 0x0, 0x1, 0x0, 0x1, 0x0, 0x1., v16_int8 = .0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x1.."
+set vector_register3_vr ".uint128 = 0x00000001000000010000000100000001, v4_float = .0x0, 0x0, 0x0, 0x0., v4_int32 = .0x1, 0x1, 0x1, 0x1., v8_int16 = .0x0, 0x1, 0x0, 0x1, 0x0, 0x1, 0x0, 0x1., v16_int8 = .0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x1.."
 
 set float_register ".raw 0xdeadbeefdeadbeef."
 
@@ -78,7 +78,7 @@ set float_register ".raw 0xdeadbeefdeadbeef."
 
 # 1: Set F0~F31 registers and check if it reflects on VS0~VS31.
 for {set i 0} {$i < 32} {incr i 1} {
-    send_gdb "set \$f$i = 1\.3"
+    gdb_test "set \$f$i = 1\.3" "" ""
 }
 
 for {set i 0} {$i < 32} {incr i 1} {
@@ -88,7 +88,7 @@ for {set i 0} {$i < 32} {incr i 1} {
 # 2: Set VS0~VS31 registers and check if it reflects on F0~F31.
 for {set i 0} {$i < 32} {incr i 1} {
         for {set j 0} {$j < 4} {incr j 1} {
-           send_gdb "set \$vs$i.v4_int32\[$j\] = 0xdeadbeef"
+           gdb_test "set \$vs$i.v4_int32\[$j\] = 0xdeadbeef" "" ""
         }
 }
 
@@ -105,7 +105,7 @@ for {set i 0} {$i < 32} {incr i 1} {
 # 1: Set VR0~VR31 registers and check if it reflects on VS32~VS63.
 for {set i 0} {$i < 32} {incr i 1} {
         for {set j 0} {$j < 4} {incr j 1} {
-           send_gdb "set \$vr$i.v4_int32\[$j\] = 1"
+           gdb_test "set \$vr$i.v4_int32\[$j\] = 1" "" ""
         }
 }
 
@@ -115,12 +115,12 @@ for {set i 32} {$i < 64} {incr i 1} {
 # 2: Set VS32~VS63 registers and check if it reflects on VR0~VR31.
 for {set i 32} {$i < 64} {incr i 1} {
         for {set j 0} {$j < 4} {incr j 1} {
-           send_gdb "set \$vs$i.v4_int32\[$j\] = 1"
+           gdb_test "set \$vs$i.v4_int32\[$j\] = 1" "" ""
         }
 }
 
 for {set i 0} {$i < 32} {incr i 1} {
-    gdb_test "info reg vr$i" "vr$i.*$vector_register3" "info reg vr$i"
+    gdb_test "info reg vr$i" "vr$i.*$vector_register3_vr" "info reg vr$i"
 }
 
 set escapedfilename [string_to_regexp ${objdir}/${subdir}/vsx-core.test]

Re: [RFA] Fix setting of VSX registers

[Tom Tromey]

>>>>> "Thiago" == Thiago Jung Bauermann <bauerman@br.ibm.com> writes:

Thiago> The problem is, the function was "fetching" the VSX registers using
Thiago> PTRACE_SETVSXREGS instead of PTRACE_GETVSXREGS. Ouch.

I don't know this code at all, but this change seems obviously ok to me.

Thiago> This patch fixes the typo, and also fixes the vsx-regs.exp testcase to
Thiago> use gdb_test instead of send_gdb (this also fixes some synchronization
Thiago> issues in the test), and updates the expect info reg output with the new
Thiago> v2_double member.

I don't understand why the new gdb_test calls have an empty "message"
argument.

Actually, this code in gdb_test itself looks somewhat bogus.
Aside from parsing arguments by hand (why??), it uses a different
default for the message than gdb_test_multiple.  I don't understand when
this can ever be the right thing to do.

For your patch I suggest just leaving off the 3rd argument.

Also, when the second argument to gdb_test is the empty string ... I
suspect you actually want to use gdb_test_no_output.

Tom

Re: [RFA] Fix setting of VSX registers

[Thiago Jung Bauermann]

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

On Thu, 2010-07-22 at 10:05 -0600, Tom Tromey wrote:
> >>>>> "Thiago" == Thiago Jung Bauermann <bauerman@br.ibm.com> writes:
> 
> Thiago> The problem is, the function was "fetching" the VSX registers using
> Thiago> PTRACE_SETVSXREGS instead of PTRACE_GETVSXREGS. Ouch.
> 
> I don't know this code at all, but this change seems obviously ok to me.

Indeed. I just applied this part to HEAD. Is the obvious rule valid for
the 7.2 branch too?

Joel, would you mind me committing this hunk to the 7.2 branch?

--- a/gdb/ppc-linux-nat.c
+++ b/gdb/ppc-linux-nat.c
@@ -877,7 +877,7 @@ store_vsx_register (const struct regcache *regcache, int tid, int regno)
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   int vsxregsize = register_size (gdbarch, tdep->ppc_vsr0_upper_regnum);

-  ret = ptrace (PTRACE_SETVSXREGS, tid, 0, &regs);
+  ret = ptrace (PTRACE_GETVSXREGS, tid, 0, &regs);
   if (ret < 0)
     {
       if (errno == EIO)


> Thiago> This patch fixes the typo, and also fixes the vsx-regs.exp testcase to
> Thiago> use gdb_test instead of send_gdb (this also fixes some synchronization
> Thiago> issues in the test), and updates the expect info reg output with the new
> Thiago> v2_double member.
> 
> I don't understand why the new gdb_test calls have an empty "message"
> argument.

So that they don't increase the test count. They are just sending
command to GDB to set the stage for the actual tests, they're of no
intrinsic interest to the testcase.

> Actually, this code in gdb_test itself looks somewhat bogus.
> Aside from parsing arguments by hand (why??), it uses a different
> default for the message than gdb_test_multiple.  I don't understand
> when this can ever be the right thing to do.

I agree it's strange, but TCL looks very alien to my eyes and I don't
tend to question the testsuite way of doing things. :-)

> For your patch I suggest just leaving off the 3rd argument.

In that case I wouldn't get the "this is not an actual test" effect that
I'm interested in.

> Also, when the second argument to gdb_test is the empty string ... I
> suspect you actually want to use gdb_test_no_output.

Indeed. I was under the impression that there was a specific function
for that but I couldn't find it. I just updated the GDBTestcaseCookbook
wiki page with that information.

Here's new version of the testcase patch updated to use
gdb_test_no_output, but keeping the empty message argument. WDYT?
-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center


2010-07-26  Thiago Jung Bauermann  <bauerman@br.ibm.com>

	* gdb.arch/vsx-regs.exp: Remove wrong comment about testing AltiVec
	registers.  Update data sets with the new v2_double element in the VSX
	register union.  Add vector_register3_vr data set for the AltiVec
	registers.  Use gdb_test_no_output instead of send_gdb.


[-- Attachment #2: vsx.diff --]
[-- Type: text/x-patch, Size: 4424 bytes --]

diff --git a/gdb/testsuite/gdb.arch/vsx-regs.exp b/gdb/testsuite/gdb.arch/vsx-regs.exp
index f310a6f..8cd8ebe 100644
--- a/gdb/testsuite/gdb.arch/vsx-regs.exp
+++ b/gdb/testsuite/gdb.arch/vsx-regs.exp
@@ -14,8 +14,6 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
 
-# Tests for Powerpc AltiVec register setting and fetching
-
 if $tracelevel then {
     strace $tracelevel
 }
@@ -66,11 +64,13 @@ if ![runto_main] then {
 
 # Data sets used throughout the test
 
-set vector_register1 ".uint128 = 0x3ff4cccccccccccc0000000000000000, v4_float = .0x1, 0xf99999a0, 0x0, 0x0., v4_int32 = .0x3ff4cccc, 0xcccccccc, 0x0, 0x0., v8_int16 = .0x3ff4, 0xcccc, 0xcccc, 0xcccc, 0x0, 0x0, 0x0, 0x0., v16_int8 = .0x3f, 0xf4, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0.."
+set vector_register1 ".uint128 = 0x3ff4cccccccccccc0000000000000000, v2_double = .0x1, 0x0., v4_float = .0x1, 0xf99999a0, 0x0, 0x0., v4_int32 = .0x3ff4cccc, 0xcccccccc, 0x0, 0x0., v8_int16 = .0x3ff4, 0xcccc, 0xcccc, 0xcccc, 0x0, 0x0, 0x0, 0x0., v16_int8 = .0x3f, 0xf4, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0.."
+
+set vector_register2 "uint128 = 0xdeadbeefdeadbeefdeadbeefdeadbeef, v2_double = .0x1, 0x1., v4_float = .0x0, 0x0, 0x0, 0x0., v4_int32 = .0xdeadbeef, 0xdeadbeef, 0xdeadbeef, 0xdeadbeef., v8_int16 = .0xdead, 0xbeef, 0xdead, 0xbeef, 0xdead, 0xbeef, 0xdead, 0xbeef., v16_int8 = .0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef.."
 
-set vector_register2 "uint128 = 0xdeadbeefdeadbeefdeadbeefdeadbeef, v4_float = .0x0, 0x0, 0x0, 0x0., v4_int32 = .0xdeadbeef, 0xdeadbeef, 0xdeadbeef, 0xdeadbeef., v8_int16 = .0xdead, 0xbeef, 0xdead, 0xbeef, 0xdead, 0xbeef, 0xdead, 0xbeef., v16_int8 = .0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef.."
+set vector_register3 ".uint128 = 0x00000001000000010000000100000001, v2_double = .0x0, 0x0., v4_float = .0x0, 0x0, 0x0, 0x0., v4_int32 = .0x1, 0x1, 0x1, 0x1., v8_int16 = .0x0, 0x1, 0x0, 0x1, 0x0, 0x1, 0x0, 0x1., v16_int8 = .0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x1.."
 
-set vector_register3 ".uint128 = 0x00000001000000010000000100000001, v4_float = .0x0, 0x0, 0x0, 0x0., v4_int32 = .0x1, 0x1, 0x1, 0x1., v8_int16 = .0x0, 0x1, 0x0, 0x1, 0x0, 0x1, 0x0, 0x1., v16_int8 = .0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x1.."
+set vector_register3_vr ".uint128 = 0x00000001000000010000000100000001, v4_float = .0x0, 0x0, 0x0, 0x0., v4_int32 = .0x1, 0x1, 0x1, 0x1., v8_int16 = .0x0, 0x1, 0x0, 0x1, 0x0, 0x1, 0x0, 0x1., v16_int8 = .0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x1.."
 
 set float_register ".raw 0xdeadbeefdeadbeef."
 
@@ -78,7 +78,7 @@ set float_register ".raw 0xdeadbeefdeadbeef."
 
 # 1: Set F0~F31 registers and check if it reflects on VS0~VS31.
 for {set i 0} {$i < 32} {incr i 1} {
-    send_gdb "set \$f$i = 1\.3"
+    gdb_test_no_output "set \$f$i = 1\.3" ""
 }
 
 for {set i 0} {$i < 32} {incr i 1} {
@@ -88,7 +88,7 @@ for {set i 0} {$i < 32} {incr i 1} {
 # 2: Set VS0~VS31 registers and check if it reflects on F0~F31.
 for {set i 0} {$i < 32} {incr i 1} {
         for {set j 0} {$j < 4} {incr j 1} {
-           send_gdb "set \$vs$i.v4_int32\[$j\] = 0xdeadbeef"
+           gdb_test_no_output "set \$vs$i.v4_int32\[$j\] = 0xdeadbeef" ""
         }
 }
 
@@ -105,7 +105,7 @@ for {set i 0} {$i < 32} {incr i 1} {
 # 1: Set VR0~VR31 registers and check if it reflects on VS32~VS63.
 for {set i 0} {$i < 32} {incr i 1} {
         for {set j 0} {$j < 4} {incr j 1} {
-           send_gdb "set \$vr$i.v4_int32\[$j\] = 1"
+           gdb_test_no_output "set \$vr$i.v4_int32\[$j\] = 1" ""
         }
 }
 
@@ -115,12 +115,12 @@ for {set i 32} {$i < 64} {incr i 1} {
 # 2: Set VS32~VS63 registers and check if it reflects on VR0~VR31.
 for {set i 32} {$i < 64} {incr i 1} {
         for {set j 0} {$j < 4} {incr j 1} {
-           send_gdb "set \$vs$i.v4_int32\[$j\] = 1"
+           gdb_test_no_output "set \$vs$i.v4_int32\[$j\] = 1" ""
         }
 }
 
 for {set i 0} {$i < 32} {incr i 1} {
-    gdb_test "info reg vr$i" "vr$i.*$vector_register3" "info reg vr$i"
+    gdb_test "info reg vr$i" "vr$i.*$vector_register3_vr" "info reg vr$i"
 }
 
 set escapedfilename [string_to_regexp ${objdir}/${subdir}/vsx-core.test]

Re: [RFA] Fix setting of VSX registers

[Joel Brobecker]

> Joel, would you mind me committing this hunk to the 7.2 branch?

Please go ahead and commit to the branch - the change to the code seems
obviously correct.

-- 
Joel

Re: [RFA] Fix setting of VSX registers

[Thiago Jung Bauermann]

On Tue, 2010-07-27 at 08:20 -0700, Joel Brobecker wrote:
> > Joel, would you mind me committing this hunk to the 7.2 branch?
> 
> Please go ahead and commit to the branch - the change to the code seems
> obviously correct.

Thanks! I just committed it.
-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center

Re: [RFA] Fix setting of VSX registers

[Tom Tromey]

>>>>> "Thiago" == Thiago Jung Bauermann <bauerman@br.ibm.com> writes:

Tom> I don't understand why the new gdb_test calls have an empty "message"
Tom> argument.

Thiago> So that they don't increase the test count. They are just sending
Thiago> command to GDB to set the stage for the actual tests, they're of no
Thiago> intrinsic interest to the testcase.

Yeah, that makes sense.  This is a sort of generic problem in dejagnu.

Anyway, I am not sure this approach will work, because gdb_test calls
gdb_test_multiple, which has a different default and which also may call
'fail'.

One approach would be to refactor these procs so that your use can work.
I think this would be nice to have -- I think it would be good to have
fewer boilerplate tests with "synthetic" names, ones that nobody is
interested in.

Another approach would be to just give up and add a new test.  That is
sort of more traditional ;-)

Tom

Re: [RFA] Fix setting of VSX registers

[Joel Brobecker]

> One approach would be to refactor these procs so that your use can work.
> I think this would be nice to have -- I think it would be good to have
> fewer boilerplate tests with "synthetic" names, ones that nobody is
> interested in.

When we discussed the new gdb-testsuite that AdaCore uses, we talked
about that quite a bit. And unsurprisingly, there was no obvious answer.
These tests can be an annoyance if all hell breaks lose and they all
start to fail. A PASS indeed adds little value to the results.  But
on the other hand, I have always felt that we should verify that these
commands have the expected results, and that we should get a FAIL if
we detect something went wrong. Ideally, we wanted to be able to group
a sequence of commands as one "setup phase" and generate one FAIL if
part of the sequence fails. But in the end, we decided that it was just
simpler to treat everything as a test (this is what I have pretty much
done in dejagnu as well).

-- 
Joel

Re: [RFA] Fix setting of VSX registers

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Tom> One approach would be to refactor these procs so that your use can work.
Tom> I think this would be nice to have -- I think it would be good to have
Tom> fewer boilerplate tests with "synthetic" names, ones that nobody is
Tom> interested in.

Joel> When we discussed the new gdb-testsuite that AdaCore uses, we talked
Joel> about that quite a bit. And unsurprisingly, there was no obvious answer.
Joel> These tests can be an annoyance if all hell breaks lose and they all
Joel> start to fail. A PASS indeed adds little value to the results.  But
Joel> on the other hand, I have always felt that we should verify that these
Joel> commands have the expected results, and that we should get a FAIL if
Joel> we detect something went wrong. Ideally, we wanted to be able to group
Joel> a sequence of commands as one "setup phase" and generate one FAIL if
Joel> part of the sequence fails. But in the end, we decided that it was just
Joel> simpler to treat everything as a test (this is what I have pretty much
Joel> done in dejagnu as well).

Yeah, it is definitely simpler to keep going with what we already have.
At least for me, test suite hacking is in itself not very interesting,
so I usually just opt for the minimal change that works, and move on to
fun things.

If someone is motivated to work on this, though, one approach would be
to separate out the mechanics of sending a command to gdb and matching
responses from the pass/fail reporting.  Then it would be possible to
write a proc that evals its argument and passes or fails based on the
answer:

single_gdb_test "test name here" {
  gdb_send_no_response "set language c";  # throws exception on "failure"
  gdb_send_and_response "print 23" " = 23"
}

... or what have you.

Another thing I've often wanted is to have the tests forced to have
unique names: by having the test frame work automatically include the
.exp name in the test name, and by automatically handling duplicates
somehow (either an error, or appending a counter).

Tom

Re: [RFA] Fix setting of VSX registers

[Thiago Jung Bauermann]

Hi,

[ Context: fix vsx-regs.exp testcase which was broken because of output
changes in GDB. ]

On Thu, 2010-07-22 at 10:05 -0600, Tom Tromey wrote:
> >>>>> "Thiago" == Thiago Jung Bauermann <bauerman@br.ibm.com> writes:
> Thiago> This patch fixes the typo, and also fixes the vsx-regs.exp testcase to
> Thiago> use gdb_test instead of send_gdb (this also fixes some synchronization
> Thiago> issues in the test), and updates the expect info reg output with the new
> Thiago> v2_double member.
> 
> I don't understand why the new gdb_test calls have an empty "message"
> argument.
> 
> Actually, this code in gdb_test itself looks somewhat bogus.
> Aside from parsing arguments by hand (why??), it uses a different
> default for the message than gdb_test_multiple.  I don't understand when
> this can ever be the right thing to do.
> 
> For your patch I suggest just leaving off the 3rd argument.
> 
> Also, when the second argument to gdb_test is the empty string ... I
> suspect you actually want to use gdb_test_no_output.

Decided to just give up the 3rd argument and make the set up commands
count as tests too. Also changed to use gdb_test_no_output.

Ok? Also, ok for the branch?

-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center



2010-08-18  Thiago Jung Bauermann  <bauerman@br.ibm.com>

	* gdb.arch/vsx-regs.exp: Remove wrong comment about testing AltiVec
	registers.  Update data sets with the new v2_double element in the VSX
	register union.  Add vector_register3_vr data set for the AltiVec
	registers.  Use gdb_test_no_output instead of send_gdb.

Index: gdb.git/gdb/testsuite/gdb.arch/vsx-regs.exp
===================================================================
--- gdb.git.orig/gdb/testsuite/gdb.arch/vsx-regs.exp	2010-07-29 16:49:32.000000000 -0300
+++ gdb.git/gdb/testsuite/gdb.arch/vsx-regs.exp	2010-08-18 16:11:13.000000000 -0300
@@ -14,8 +14,6 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
 
-# Tests for Powerpc AltiVec register setting and fetching
-
 if $tracelevel then {
     strace $tracelevel
 }
@@ -66,11 +64,13 @@ if ![runto_main] then {
 
 # Data sets used throughout the test
 
-set vector_register1 ".uint128 = 0x3ff4cccccccccccc0000000000000000, v4_float = .0x1, 0xf99999a0, 0x0, 0x0., v4_int32 = .0x3ff4cccc, 0xcccccccc, 0x0, 0x0., v8_int16 = .0x3ff4, 0xcccc, 0xcccc, 0xcccc, 0x0, 0x0, 0x0, 0x0., v16_int8 = .0x3f, 0xf4, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0.."
+set vector_register1 ".uint128 = 0x3ff4cccccccccccc0000000000000000, v2_double = .0x1, 0x0., v4_float = .0x1, 0xf99999a0, 0x0, 0x0., v4_int32 = .0x3ff4cccc, 0xcccccccc, 0x0, 0x0., v8_int16 = .0x3ff4, 0xcccc, 0xcccc, 0xcccc, 0x0, 0x0, 0x0, 0x0., v16_int8 = .0x3f, 0xf4, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0.."
+
+set vector_register2 "uint128 = 0xdeadbeefdeadbeefdeadbeefdeadbeef, v2_double = .0x1, 0x1., v4_float = .0x0, 0x0, 0x0, 0x0., v4_int32 = .0xdeadbeef, 0xdeadbeef, 0xdeadbeef, 0xdeadbeef., v8_int16 = .0xdead, 0xbeef, 0xdead, 0xbeef, 0xdead, 0xbeef, 0xdead, 0xbeef., v16_int8 = .0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef.."
 
-set vector_register2 "uint128 = 0xdeadbeefdeadbeefdeadbeefdeadbeef, v4_float = .0x0, 0x0, 0x0, 0x0., v4_int32 = .0xdeadbeef, 0xdeadbeef, 0xdeadbeef, 0xdeadbeef., v8_int16 = .0xdead, 0xbeef, 0xdead, 0xbeef, 0xdead, 0xbeef, 0xdead, 0xbeef., v16_int8 = .0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef.."
+set vector_register3 ".uint128 = 0x00000001000000010000000100000001, v2_double = .0x0, 0x0., v4_float = .0x0, 0x0, 0x0, 0x0., v4_int32 = .0x1, 0x1, 0x1, 0x1., v8_int16 = .0x0, 0x1, 0x0, 0x1, 0x0, 0x1, 0x0, 0x1., v16_int8 = .0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x1.."
 
-set vector_register3 ".uint128 = 0x00000001000000010000000100000001, v4_float = .0x0, 0x0, 0x0, 0x0., v4_int32 = .0x1, 0x1, 0x1, 0x1., v8_int16 = .0x0, 0x1, 0x0, 0x1, 0x0, 0x1, 0x0, 0x1., v16_int8 = .0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x1.."
+set vector_register3_vr ".uint128 = 0x00000001000000010000000100000001, v4_float = .0x0, 0x0, 0x0, 0x0., v4_int32 = .0x1, 0x1, 0x1, 0x1., v8_int16 = .0x0, 0x1, 0x0, 0x1, 0x0, 0x1, 0x0, 0x1., v16_int8 = .0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x1.."
 
 set float_register ".raw 0xdeadbeefdeadbeef."
 
@@ -78,7 +78,7 @@ set float_register ".raw 0xdeadbeefdeadb
 
 # 1: Set F0~F31 registers and check if it reflects on VS0~VS31.
 for {set i 0} {$i < 32} {incr i 1} {
-    send_gdb "set \$f$i = 1\.3"
+    gdb_test_no_output "set \$f$i = 1\.3"
 }
 
 for {set i 0} {$i < 32} {incr i 1} {
@@ -88,7 +88,7 @@ for {set i 0} {$i < 32} {incr i 1} {
 # 2: Set VS0~VS31 registers and check if it reflects on F0~F31.
 for {set i 0} {$i < 32} {incr i 1} {
         for {set j 0} {$j < 4} {incr j 1} {
-           send_gdb "set \$vs$i.v4_int32\[$j\] = 0xdeadbeef"
+           gdb_test_no_output "set \$vs$i.v4_int32\[$j\] = 0xdeadbeef"
         }
 }
 
@@ -105,7 +105,7 @@ for {set i 0} {$i < 32} {incr i 1} {
 # 1: Set VR0~VR31 registers and check if it reflects on VS32~VS63.
 for {set i 0} {$i < 32} {incr i 1} {
         for {set j 0} {$j < 4} {incr j 1} {
-           send_gdb "set \$vr$i.v4_int32\[$j\] = 1"
+           gdb_test_no_output "set \$vr$i.v4_int32\[$j\] = 1"
         }
 }
 
@@ -115,12 +115,12 @@ for {set i 32} {$i < 64} {incr i 1} {
 # 2: Set VS32~VS63 registers and check if it reflects on VR0~VR31.
 for {set i 32} {$i < 64} {incr i 1} {
         for {set j 0} {$j < 4} {incr j 1} {
-           send_gdb "set \$vs$i.v4_int32\[$j\] = 1"
+           gdb_test_no_output "set \$vs$i.v4_int32\[$j\] = 1"
         }
 }
 
 for {set i 0} {$i < 32} {incr i 1} {
-    gdb_test "info reg vr$i" "vr$i.*$vector_register3" "info reg vr$i"
+    gdb_test "info reg vr$i" "vr$i.*$vector_register3_vr" "info reg vr$i"
 }
 
 set escapedfilename [string_to_regexp ${objdir}/${subdir}/vsx-core.test]

Re: [RFA] Fix setting of VSX registers

[Joel Brobecker]

> Ok? Also, ok for the branch?
[...]
> 2010-08-18  Thiago Jung Bauermann  <bauerman@br.ibm.com>
> 
> 	* gdb.arch/vsx-regs.exp: Remove wrong comment about testing AltiVec
> 	registers.  Update data sets with the new v2_double element in the VSX
> 	register union.  Add vector_register3_vr data set for the AltiVec
> 	registers.  Use gdb_test_no_output instead of send_gdb.

Looks good to me, Thiago. OK for both HEAD & branch.

-- 
Joel

Re: [RFA] Fix setting of VSX registers

[Thiago Jung Bauermann]

On Thu, 2010-08-19 at 11:49 +0200, Joel Brobecker wrote:
> > 2010-08-18  Thiago Jung Bauermann  <bauerman@br.ibm.com>
> > 
> > 	* gdb.arch/vsx-regs.exp: Remove wrong comment about testing AltiVec
> > 	registers.  Update data sets with the new v2_double element in the VSX
> > 	register union.  Add vector_register3_vr data set for the AltiVec
> > 	registers.  Use gdb_test_no_output instead of send_gdb.
> 
> Looks good to me, Thiago. OK for both HEAD & branch.

Checked in. Thanks!

-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center