This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] PPC Call-clobbered registers testcase


On Tue, Oct 23, 2007 at 08:05:57PM -0300, Luis Machado wrote:
> Hi,
> 
> I've modified the patch to reflect the behaviour on x86 systems. On x86
> it shows the values, but they're incorrectly shown as being equal, and
> they shouldn't be.
> 
> On ppc they're expected to be "optmized out".
> 
> This patch includes a new directory, "gdb.opt", for testing optimized
> binaries.
> 
> Does this look OK?

No, sorry, but you're going the right direction :-)

> 2007-10-23  Luis Machado  <luisgpm@br.ibm.com>
> 
>     * gdb.opt/clobbered-registers-O2.c: New testcase source file.
>     * gdb.opt/clobbered-registers-O2.exp: New testcase expect file.
>     * gdb.opt/Makefile.in: New makefile
>     * Makefile.in: Create new directory "gdb.opt"
>     * configure.ac: Add "gdb.opt" directory
>     * configure: Add "gdb.opt" directory

Periods at the end of entries.  You should just say "Regenerated." for
things like the configure script.

> +   Please email any bugs, comments, and/or additions to this file to:
> +   bug-gdb@prep.ai.mit.edu  */

It's not a big deal, but please don't add this to new files.  I don't
think that address has worked in a decade or more.  We should get
rid of it.

> +send_gdb "frame 1\n"
> +
> +gdb_expect 10 {
> +    -re "#1.*in gen_movsd.*" { pass "frame 1 backtrace" }
> +    default { fail "frame 1 backtrace error" }
> +}

Please avoid using send_gdb or gdb_expect, unless you have special
needs for them.  If you have only one pass pattern, use gdb_test;
if you have more than one, use gdb_test_multiple.  It works like
gdb_expect but includes the send_gdb and handles all sorts of
unexpected output for you.

> +gdb_expect 5 {
> +    -re "\\$\[0-9\]* = <(.*)>.*$gdb_prompt $" {
> +        set operand0 "optimized"
> +    }

No need for the wildcare here.  You can match <value optimized out> or
however that's spelled.  Also, you need one more backslash on the
$; the two \\ become a backslash in the regular expression, but that
leaves the unescaped $ at risk of TCL variable expansion.

> +set result1 [string compare $operand0 "optimized"]
> +set result2 [string compare $operand1 "optimized"]
> +
> +# The PowerPC test ends here. If both values are optimized, then
> +# we are fine with it.
> +if {!$result1 && !$result2} {
> +    pass "Both values were optimized"
> +    return
> +}
> +
> +# This is a x86-specific test since it doesn't show the values
> +# as optimized, but they appear as equal values. That is
> +# incorrect.
> +set result1 [string compare $operand0 $operand1]
> +
> +if {!$result1} {
> +    fail "Values are displayed but are incorrect"
> +    return -1
> +}

Every test should be either pass or fail, and it should pass and fail
with the same message.  We make an exception in the GDB testsuite, by
allowing (reason) after a failure message, but that's it.

This suggests a simpler formulation for your test.  What are you
trying to avoid?  Incorrect values.  So <value optimized out> is a
pass, and 13 (or 14 for the second one) is a pass, and everything
else is a fail.

  gdb_test "print operand0" \
    "\\\$$decimal = (<value optimized out>|13)"
  gdb_test "print operand1" \
    "\\\$$decimal = (<value optimized out>|14)"

-- 
Daniel Jacobowitz
CodeSourcery


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]