[RFA]: gdb/testsuite/gdb.arch/altivec-regs.exp Broken testcase

[RFA]: gdb/testsuite/gdb.arch/altivec-regs.exp Broken testcase

[Markus Deuling]

Hi,

I'm referring to http://sourceware.org/ml/gdb-patches/2007-01/msg00353.html.

It seems that the output from "show endian" has changed since the last time someone took a 
look into it :-) So I added a "re" to the test.

Do we need this branch or can I delete it ?
>>     -re "(The target endianness is set automatically .currently )(big|little)( endian.*)$gdb_prompt $" {
>>         pass "endianness"
>>        set endianness $expect_out(2,string)
>>     }

This is the result of gdb.arch/altivec-regs.exp with my patch:

# of expected passes            209
# of unexpected failures        1


Is it ok to apply ?

ChangeLog:

       *gdb.arch/altivec-regs.exp: Initialise variable
       endianness in every case. 

==========================================
diff -urN dev/gdb/testsuite/gdb.arch/altivec-regs.exp src/gdb/testsuite/gdb.arch/altivec-regs.exp
--- dev/gdb/testsuite/gdb.arch/altivec-regs.exp 2007-01-10 15:48:12.000000000 +0100
+++ src/gdb/testsuite/gdb.arch/altivec-regs.exp 2007-01-16 07:40:05.000000000 +0100
@@ -88,12 +88,17 @@

 gdb_test "next" "" ""

+set endianness ""
 send_gdb "show endian\n"
 gdb_expect {
     -re "(The target endianness is set automatically .currently )(big|little)( endian.*)$gdb_prompt $" {
         pass "endianness"
        set endianness $expect_out(2,string)
     }
+    -re "The target is assumed to be big endian.*" {
+        pass "endianess"
+        set endianness "big"
+    }
     -re ".*$gdb_prompt $" {
        fail "couldn't get endianness"
     }


Regards,
Markus

-- 
Markus Deuling
GNU Toolchain for Linux on Cell BE
deuling@de.ibm.com 

Re: [RFA]: gdb/testsuite/gdb.arch/altivec-regs.exp Broken testcase

[Mark Kettenis]

> Date: Tue, 16 Jan 2007 07:48:18 +0100
> From: Markus Deuling <deuling@de.ibm.com>
> 
> Hi,
> 
> I'm referring to http://sourceware.org/ml/gdb-patches/2007-01/msg00353.html.
> 
> It seems that the output from "show endian" has changed since the last time someone took a 
> look into it :-) So I added a "re" to the test.
> 
> Do we need this branch or can I delete it ?
> >>     -re "(The target endianness is set automatically .currently )(big|little)( endian.*)$gdb_prompt $" {
> >>         pass "endianness"
> >>        set endianness $expect_out(2,string)
> >>     }

This worries me a bit.  So in the past we were able to determine the
endianness automagically, but now we don't?  Do we understand why?
Doesn't this mean that something is broken that worked before?

> This is the result of gdb.arch/altivec-regs.exp with my patch:
> 
> # of expected passes            209
> # of unexpected failures        1
> 
> 
> Is it ok to apply ?
> 
> ChangeLog:
> 
>        *gdb.arch/altivec-regs.exp: Initialise variable
>        endianness in every case. 
> 
> ==========================================
> diff -urN dev/gdb/testsuite/gdb.arch/altivec-regs.exp src/gdb/testsuite/gdb.arch/altivec-regs.exp
> --- dev/gdb/testsuite/gdb.arch/altivec-regs.exp 2007-01-10 15:48:12.000000000 +0100
> +++ src/gdb/testsuite/gdb.arch/altivec-regs.exp 2007-01-16 07:40:05.000000000 +0100
> @@ -88,12 +88,17 @@
> 
>  gdb_test "next" "" ""
> 
> +set endianness ""
>  send_gdb "show endian\n"
>  gdb_expect {
>      -re "(The target endianness is set automatically .currently )(big|little)( endian.*)$gdb_prompt $" {
>          pass "endianness"
>         set endianness $expect_out(2,string)
>      }
> +    -re "The target is assumed to be big endian.*" {
> +        pass "endianess"
> +        set endianness "big"
> +    }
>      -re ".*$gdb_prompt $" {
>         fail "couldn't get endianness"
>      }

Re: [RFA]: gdb/testsuite/gdb.arch/altivec-regs.exp Broken testcase

[Markus Deuling]

Hi Mark,

Mark Kettenis schrieb:
>> Date: Tue, 16 Jan 2007 07:48:18 +0100
>> From: Markus Deuling <deuling@de.ibm.com>
>>
>> Hi,
>>
>> I'm referring to http://sourceware.org/ml/gdb-patches/2007-01/msg00353.html.
>>
>> It seems that the output from "show endian" has changed since the last time someone took a 
>> look into it :-) So I added a "re" to the test.
>>
>> Do we need this branch or can I delete it ?
>>>>     -re "(The target endianness is set automatically .currently )(big|little)( endian.*)$gdb_prompt $" {
>>>>         pass "endianness"
>>>>        set endianness $expect_out(2,string)
>>>>     }
> 
> This worries me a bit.  So in the past we were able to determine the
> endianness automagically, but now we don't?  Do we understand why?
> Doesn't this mean that something is broken that worked before?
> 
>

I looked into arch-utils.c.  We still need that branch above. But there are also changes, so that
we need to recognize "_("The target is assumed to be big endian\n"));". 

The variable "endianess" should either contain "big" or be empty in that testcase. So the way it's now with that patch should work, I think.

Regards,
Markus

-- 
Markus Deuling
GNU Toolchain for Linux on Cell BE
deuling@de.ibm.com 

Re: [RFA]: gdb/testsuite/gdb.arch/altivec-regs.exp Broken testcase

[Daniel Jacobowitz]

Mark is right again.  The fact that the testsuite isn't copying
should point us at the fact that the output changed, and we don't know
why.

On Tue, Jan 16, 2007 at 10:28:01PM +0100, Mark Kettenis wrote:
> This worries me a bit.  So in the past we were able to determine the
> endianness automagically, but now we don't?  Do we understand why?
> Doesn't this mean that something is broken that worked before?

More precisely:

> > +    -re "The target is assumed to be big endian.*" {
> > +        pass "endianess"
> > +        set endianness "big"
> > +    }

GDB has concluded from something (an earlier set command?  one of my
gdbarch initialization changes?) that the user has specified the
endianness.

Looking at it now, it looks to me like I've got a condition backwards
in show_endian.  The != should actually be an ==.  I can't test a patch
for that until I get home next week though.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFA]: gdb/testsuite/gdb.arch/altivec-regs.exp Broken testcase

[Markus Deuling]

Hi,

Daniel Jacobowitz schrieb:
> Mark is right again.  The fact that the testsuite isn't copying
> should point us at the fact that the output changed, and we don't know
> why.
> 
> On Tue, Jan 16, 2007 at 10:28:01PM +0100, Mark Kettenis wrote:
>> This worries me a bit.  So in the past we were able to determine the
>> endianness automagically, but now we don't?  Do we understand why?
>> Doesn't this mean that something is broken that worked before?
> 
> More precisely:
> 
>>> +    -re "The target is assumed to be big endian.*" {
>>> +        pass "endianess"
>>> +        set endianness "big"
>>> +    }
> 
> GDB has concluded from something (an earlier set command?  one of my
> gdbarch initialization changes?) that the user has specified the
> endianness.
> 
> Looking at it now, it looks to me like I've got a condition backwards
> in show_endian.  The != should actually be an ==.  I can't test a patch
> for that until I get home next week though.
> 
Is that what you meant ?

I changed
  if (target_byte_order_user != BFD_ENDIAN_UNKNOWN)
to
  if (target_byte_order_user == BFD_ENDIAN_UNKNOWN)

in show_endian().

x86:
This GDB was configured as "i686-pc-linux-gnu".
(gdb) show endian
The target endianness is set automatically (currently little endian)

So we wouldn't need the branch I inserted in altivec-regs.exp. 

But I think inserting
+set endianness ""
is a good idea. Then the testcase would have FAILed instead of abort with an error.

Regards,
Markus

-- 
Markus Deuling
GNU Toolchain for Linux on Cell BE
deuling@de.ibm.com 

Re: [RFA]: gdb/testsuite/gdb.arch/altivec-regs.exp Broken testcase

[Markus Deuling]

Markus Deuling schrieb:

> Is that what you meant ?
> 
> I changed
>  if (target_byte_order_user != BFD_ENDIAN_UNKNOWN)
> to
>  if (target_byte_order_user == BFD_ENDIAN_UNKNOWN)
> 
> in show_endian().
> 
> x86:
> This GDB was configured as "i686-pc-linux-gnu".
> (gdb) show endian
> The target endianness is set automatically (currently little endian)

Works also for ppc64:
This GDB was configured as "powerpc64-unknown-linux-gnu".
(gdb) show endian
The target endianness is set automatically (currently big endian)
(gdb)    

Regards,
Markus

-- 
Markus Deuling
GNU Toolchain for Linux on Cell BE
deuling@de.ibm.com 

Re: [RFA]: gdb/testsuite/gdb.arch/altivec-regs.exp Broken testcase

[Mark Kettenis]

> Date: Wed, 17 Jan 2007 07:36:59 +0100
> From: Markus Deuling <deuling@de.ibm.com>
> 
> But I think inserting
> +set endianness ""
> is a good idea. Then the testcase would have FAILed instead of abort
> with an error.

I agree.  Feel free to commit that bit.

Mark

Re: [RFA]: gdb/testsuite/gdb.arch/altivec-regs.exp Broken testcase

[Markus Deuling]

Hi,

Mark Kettenis schrieb:
>> Date: Wed, 17 Jan 2007 07:36:59 +0100
>> From: Markus Deuling <deuling@de.ibm.com>
>>
>> But I think inserting
>> +set endianness ""
>> is a good idea. Then the testcase would have FAILed instead of abort
>> with an error.
> 
> I agree.  Feel free to commit that bit.
> 
> Mark
> 

this is the patch without the new branch. Could you please commit it?

Thank you very much.



ChangeLog:

      *gdb.arch/altivec-regs.exp: Initialise variable
      endianness in every case. 

=============================================================

--- src/gdb/testsuite/gdb.arch/altivec-regs.exp 2007-01-17 06:57:45.000000000 +0100
+++ dev/gdb/testsuite/gdb.arch/altivec-regs.exp 2007-01-18 10:12:24.000000000 +0100
@@ -89,6 +89,7 @@
 gdb_test "next" "" ""

 send_gdb "show endian\n"
+set endianness ""
 gdb_expect {
     -re "(The target endianness is set automatically .currently )(big|little)( endian.*)$gdb_prompt $" {
         pass "endianness"



Regards,
Markus

-- 
Markus Deuling
GNU Toolchain for Linux on Cell BE
deuling@de.ibm.com 

Re: [RFA]: gdb/testsuite/gdb.arch/altivec-regs.exp Broken testcase

[Daniel Jacobowitz]

On Wed, Jan 17, 2007 at 12:59:39AM -0500, Daniel Jacobowitz wrote:
> Looking at it now, it looks to me like I've got a condition backwards
> in show_endian.  The != should actually be an ==.  I can't test a patch
> for that until I get home next week though.

Home now.  I've tested and checked in this; sorry for the breakage.

-- 
Daniel Jacobowitz
CodeSourcery

2007-01-20  Daniel Jacobowitz  <dan@codesourcery.com>

	* arch-utils.c (show_endian): Correct reversed condition.

Index: arch-utils.c
===================================================================
RCS file: /cvs/src/src/gdb/arch-utils.c,v
retrieving revision 1.138
diff -u -p -r1.138 arch-utils.c
--- arch-utils.c	13 Jan 2007 23:24:43 -0000	1.138
+++ arch-utils.c	20 Jan 2007 18:16:33 -0000
@@ -322,7 +322,7 @@ static void
 show_endian (struct ui_file *file, int from_tty, struct cmd_list_element *c,
 	     const char *value)
 {
-  if (target_byte_order_user != BFD_ENDIAN_UNKNOWN)
+  if (target_byte_order_user == BFD_ENDIAN_UNKNOWN)
     if (TARGET_BYTE_ORDER == BFD_ENDIAN_BIG)
       fprintf_unfiltered (file, _("The target endianness is set automatically "
 				  "(currently big endian)\n"));