Re: [RFA] Fix setting of VSX registers

[Thiago Jung Bauermann <bauerman at br dot ibm dot com>]

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.

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]