Your change breaks GDB for ARM

Your change breaks GDB for ARM

[Phil Edwards]

(I am not subscribed to this list.)


This change

  2003-11-13  Andrew Cagney  <cagney@redhat.com>

    * arch-utils.h (selected_architecture_name): Declare.
    (selected_byte_order): Declare.
    * arch-utils.c (selected_byte_order): New function.
    (selected_architecture_name): New function.
    (target_architecture_auto): Make static.
    (set_architecture_string): Make static.
    (target_byte_order): Make static.
    (target_byte_order_auto): Make static.
    * gdbarch.sh (TARGET_BYTE_ORDER, TARGET_ARCHITECTURE): Delete
    non-multi-arch definition.
    (TARGET_ARCHITECTURE_AUTO, TARGET_BYTE_ORDER_AUTO): Delete.
    (target_byte_order, target_architecture): Delete declaration.
    (target_byte_order_auto, target_architecture_auto): Ditto.
    * gdbarch.h: Re-generate.
    * remote-sim.c (gdbsim_open): Use "selected_architecture_name" and
    "selected_byte_order".

didn't get enough testing.  As a trivial search shows,

    fenric 57% cd gdb
    fenric 58% grep -l target_byte_order *.c
    arch-utils.c
    remote-rdp.c
    fenric 59%

this variable is still being used outside of the file where you made
it static, and now GDB will not build for arm-elf targets, which use
remote-rdp.c.

I looked very briefly at the new code, but as remote-rdp.c is setting
target_byte_order, not reading it, there is no obvious change I could make.
Please fix this breakage soon.


Phil

Re: Your change breaks GDB for ARM

[Andrew Cagney]

A not so trivial find/grep shows:

cagney@nettle$ frep target_byte_order
arch-utils.c:376:/* ``target_byte_order'' is only used when non- multi-arch.
arch-utils.c:383:static int target_byte_order = BFD_ENDIAN_BIG;
arch-utils.c:384:static int target_byte_order_auto = 1;
arch-utils.c:389:  if (target_byte_order_auto)
arch-utils.c:392:    return target_byte_order;
arch-utils.c:412:  if (target_byte_order_auto)
arch-utils.c:425:      target_byte_order_auto = 1;
arch-utils.c:430:      target_byte_order_auto = 0;
arch-utils.c:439:      target_byte_order_auto = 0;
arch-utils.c:730:      && !target_byte_order_auto

which are all perfectly fine.  As for:

remote-rdp.c:355:                   target_byte_order = BFD_ENDIAN_LITTLE;
remote-rdp.c:359:                   target_byte_order = BFD_ENDIAN_BIG;

oops, missed them.  I guess I could #ifdef them out (richard?).  Those 
assignments haven't done anything useful since 2002-02-08 when the arm 
became multi-arch partial.

Andrew

Re: Your change breaks GDB for ARM

[Richard Earnshaw]

> A not so trivial find/grep shows:
> 
> cagney@nettle$ frep target_byte_order
> arch-utils.c:376:/* ``target_byte_order'' is only used when non- multi-arch.
> arch-utils.c:383:static int target_byte_order = BFD_ENDIAN_BIG;
> arch-utils.c:384:static int target_byte_order_auto = 1;
> arch-utils.c:389:  if (target_byte_order_auto)
> arch-utils.c:392:    return target_byte_order;
> arch-utils.c:412:  if (target_byte_order_auto)
> arch-utils.c:425:      target_byte_order_auto = 1;
> arch-utils.c:430:      target_byte_order_auto = 0;
> arch-utils.c:439:      target_byte_order_auto = 0;
> arch-utils.c:730:      && !target_byte_order_auto
> 
> which are all perfectly fine.  As for:
> 
> remote-rdp.c:355:                   target_byte_order = BFD_ENDIAN_LITTLE;
> remote-rdp.c:359:                   target_byte_order = BFD_ENDIAN_BIG;
> 
> oops, missed them.  I guess I could #ifdef them out (richard?).  Those 
> assignments haven't done anything useful since 2002-02-08 when the arm 
> became multi-arch partial.

Erm, maybe.  When you open a remote target via RDP you get the option to 
ask the target to set the endianness (or you can ask "tell me your 
endianness").  If you select an endianness and it isn't supported, you 
should get back the error RDIError_WrongByteSex; if you ask it to tell 
you, it will report back either RDIError_BigEndian or 
RDIError_LittleEndian.  The idea is to allow the board to tell you what it 
can support.

What should be done will depend on when we open the connection to the 
board.  If it's after we've selected an image to send, then we should pass 
the endianness of the image in the initial connection.  If it's before, 
then we should ask the board.  I think the best solution is to delay 
connecting to the target until we have an image if possible.  That avoids 
the situation where we have a target that is bi-endian (eg a simulator -- 
most boards are fixed-endian) and it reports the one we don't want -- 
there's no way for the remote end to say "I'm currently foo, but I can 
also to bar".

R.

Re: Your change breaks GDB for ARM

[Andrew Cagney]

[-- Attachment #1: Type: text/plain, Size: 1512 bytes --]

>> oops, missed them.  I guess I could #ifdef them out (richard?).  Those 
>> assignments haven't done anything useful since 2002-02-08 when the arm 
>> became multi-arch partial.
> 
> 
> Erm, maybe.  When you open a remote target via RDP you get the option to 
> ask the target to set the endianness (or you can ask "tell me your 
> endianness").  If you select an endianness and it isn't supported, you 
> should get back the error RDIError_WrongByteSex; if you ask it to tell 
> you, it will report back either RDIError_BigEndian or 
> RDIError_LittleEndian.  The idea is to allow the board to tell you what it 
> can support.
> 
> What should be done will depend on when we open the connection to the 
> board.  If it's after we've selected an image to send, then we should pass 
> the endianness of the image in the initial connection.  If it's before, 
> then we should ask the board.  I think the best solution is to delay 
> connecting to the target until we have an image if possible.  That avoids 
> the situation where we have a target that is bi-endian (eg a simulator -- 
> most boards are fixed-endian) and it reports the one we don't want -- 
> there's no way for the remote end to say "I'm currently foo, but I can 
> also to bar".

Try the attached.  It warns the user of the conflict and points them at 
"set endian" for how to change it.  The other alternative is to force 
the endianess based on what the target indicates, but as you note, that 
could lead to interesting conflicts.

Andrew


[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 1153 bytes --]

2003-11-21  Andrew Cagney  <cagney@redhat.com>

	* remote-rdp.c (rdp_init): Warn the user when there is a conflict
	between GDB's and the target's byte-order.

Index: remote-rdp.c
===================================================================
RCS file: /cvs/src/src/gdb/remote-rdp.c,v
retrieving revision 1.34
diff -u -r1.34 remote-rdp.c
--- remote-rdp.c	11 Jun 2003 13:16:28 -0000	1.34
+++ remote-rdp.c	21 Nov 2003 18:00:53 -0000
@@ -352,11 +352,17 @@
 		  case SERIAL_TIMEOUT:
 		    break;
 		  case RDP_RES_VALUE_LITTLE_ENDIAN:
-		    target_byte_order = BFD_ENDIAN_LITTLE;
+		    if (TARGET_BYTE_ORDER != BFD_ENDIAN_LITTLE)
+		      warning ("\
+little-endian remote target conflicts with GDB's current (big-endian) byte order,\n\
+see \"set endian\" for how to set the byte order");
 		    sync = 1;
 		    break;
 		  case RDP_RES_VALUE_BIG_ENDIAN:
-		    target_byte_order = BFD_ENDIAN_BIG;
+		    if (TARGET_BYTE_ORDER != BFD_ENDIAN_BIG)
+		      warning ("\
+big-endian remote target conflicts with GDB's current (little-endian) byte order,\n\
+see \"set endian\" how to set the byte order");
 		    sync = 1;
 		    break;
 		  default:

Re: Your change breaks GDB for ARM

[Phil Edwards]

On Fri, Nov 21, 2003 at 01:04:25PM -0500, Andrew Cagney wrote:
> Try the attached.  It warns the user of the conflict and points them at 
> "set endian" for how to change it.  The other alternative is to force 
> the endianess based on what the target indicates, but as you note, that 
> could lead to interesting conflicts.

Since I'm not doing any remote debugging (at the present time), I can't
actually test this patch.  At any rate, thank you for ifdefing out the
offending code; it builds for arm-elf now.


-- 
Debugging is twice as hard as writing the code in the first place.
Therefore, if you write the code as cleverly as possible, you are,
by definition, not smart enough to debug it.
    - Brian W. Kernighan