* [RFA] Fix setting of VSX registers @ 2010-07-21 18:59 Thiago Jung Bauermann 2010-07-22 16:06 ` Tom Tromey 0 siblings, 1 reply; 11+ messages in thread From: Thiago Jung Bauermann @ 2010-07-21 18:59 UTC (permalink / raw) To: gdb-patches ml [-- 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, ®s); + ret = ptrace (PTRACE_GETVSXREGS, tid, 0, ®s); 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] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFA] Fix setting of VSX registers 2010-07-21 18:59 [RFA] Fix setting of VSX registers Thiago Jung Bauermann @ 2010-07-22 16:06 ` Tom Tromey 2010-07-26 17:31 ` Thiago Jung Bauermann 2010-08-18 21:18 ` Thiago Jung Bauermann 0 siblings, 2 replies; 11+ messages in thread From: Tom Tromey @ 2010-07-22 16:06 UTC (permalink / raw) To: Thiago Jung Bauermann; +Cc: gdb-patches ml >>>>> "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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFA] Fix setting of VSX registers 2010-07-22 16:06 ` Tom Tromey @ 2010-07-26 17:31 ` Thiago Jung Bauermann 2010-07-27 15:20 ` Joel Brobecker 2010-07-28 21:34 ` Tom Tromey 2010-08-18 21:18 ` Thiago Jung Bauermann 1 sibling, 2 replies; 11+ messages in thread From: Thiago Jung Bauermann @ 2010-07-26 17:31 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches ml, Joel Brobecker [-- 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, ®s); + ret = ptrace (PTRACE_GETVSXREGS, tid, 0, ®s); 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] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFA] Fix setting of VSX registers 2010-07-26 17:31 ` Thiago Jung Bauermann @ 2010-07-27 15:20 ` Joel Brobecker 2010-07-27 16:14 ` Thiago Jung Bauermann 2010-07-28 21:34 ` Tom Tromey 1 sibling, 1 reply; 11+ messages in thread From: Joel Brobecker @ 2010-07-27 15:20 UTC (permalink / raw) To: Thiago Jung Bauermann; +Cc: Tom Tromey, gdb-patches ml > 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFA] Fix setting of VSX registers 2010-07-27 15:20 ` Joel Brobecker @ 2010-07-27 16:14 ` Thiago Jung Bauermann 0 siblings, 0 replies; 11+ messages in thread From: Thiago Jung Bauermann @ 2010-07-27 16:14 UTC (permalink / raw) To: Joel Brobecker; +Cc: Tom Tromey, gdb-patches ml 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFA] Fix setting of VSX registers 2010-07-26 17:31 ` Thiago Jung Bauermann 2010-07-27 15:20 ` Joel Brobecker @ 2010-07-28 21:34 ` Tom Tromey 2010-07-28 21:43 ` Joel Brobecker 1 sibling, 1 reply; 11+ messages in thread From: Tom Tromey @ 2010-07-28 21:34 UTC (permalink / raw) To: Thiago Jung Bauermann; +Cc: gdb-patches ml, Joel Brobecker >>>>> "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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFA] Fix setting of VSX registers 2010-07-28 21:34 ` Tom Tromey @ 2010-07-28 21:43 ` Joel Brobecker 2010-07-28 21:57 ` Tom Tromey 0 siblings, 1 reply; 11+ messages in thread From: Joel Brobecker @ 2010-07-28 21:43 UTC (permalink / raw) To: Tom Tromey; +Cc: Thiago Jung Bauermann, gdb-patches ml > 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFA] Fix setting of VSX registers 2010-07-28 21:43 ` Joel Brobecker @ 2010-07-28 21:57 ` Tom Tromey 0 siblings, 0 replies; 11+ messages in thread From: Tom Tromey @ 2010-07-28 21:57 UTC (permalink / raw) To: Joel Brobecker; +Cc: Thiago Jung Bauermann, gdb-patches ml >>>>> "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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFA] Fix setting of VSX registers 2010-07-22 16:06 ` Tom Tromey 2010-07-26 17:31 ` Thiago Jung Bauermann @ 2010-08-18 21:18 ` Thiago Jung Bauermann 2010-08-19 9:50 ` Joel Brobecker 1 sibling, 1 reply; 11+ messages in thread From: Thiago Jung Bauermann @ 2010-08-18 21:18 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches ml, Joel Brobecker 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] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFA] Fix setting of VSX registers 2010-08-18 21:18 ` Thiago Jung Bauermann @ 2010-08-19 9:50 ` Joel Brobecker 2010-08-19 17:56 ` Thiago Jung Bauermann 0 siblings, 1 reply; 11+ messages in thread From: Joel Brobecker @ 2010-08-19 9:50 UTC (permalink / raw) To: Thiago Jung Bauermann; +Cc: Tom Tromey, gdb-patches ml > 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFA] Fix setting of VSX registers 2010-08-19 9:50 ` Joel Brobecker @ 2010-08-19 17:56 ` Thiago Jung Bauermann 0 siblings, 0 replies; 11+ messages in thread From: Thiago Jung Bauermann @ 2010-08-19 17:56 UTC (permalink / raw) To: Joel Brobecker; +Cc: Tom Tromey, gdb-patches ml 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2010-08-19 17:56 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2010-07-21 18:59 [RFA] Fix setting of VSX registers Thiago Jung Bauermann 2010-07-22 16:06 ` Tom Tromey 2010-07-26 17:31 ` Thiago Jung Bauermann 2010-07-27 15:20 ` Joel Brobecker 2010-07-27 16:14 ` Thiago Jung Bauermann 2010-07-28 21:34 ` Tom Tromey 2010-07-28 21:43 ` Joel Brobecker 2010-07-28 21:57 ` Tom Tromey 2010-08-18 21:18 ` Thiago Jung Bauermann 2010-08-19 9:50 ` Joel Brobecker 2010-08-19 17:56 ` Thiago Jung Bauermann
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).