[RFA] Fix setting of VSX registers

[Thiago Jung Bauermann]

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.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: vsx.diff
Type: text/x-patch
Size: 4424 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/gdb-patches/attachments/20100726/04a74566/attachment.bin>