From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19827 invoked by alias); 30 Mar 2006 11:17:18 -0000 Received: (qmail 19819 invoked by uid 22791); 30 Mar 2006 11:17:17 -0000 X-Spam-Check-By: sourceware.org Received: from lon-del-02.spheriq.net (HELO lon-del-02.spheriq.net) (195.46.50.98) by sourceware.org (qpsmtpd/0.31) with ESMTP; Thu, 30 Mar 2006 11:17:15 +0000 Received: from lon-out-01.spheriq.net ([195.46.50.129]) by lon-del-02.spheriq.net with ESMTP id k2UBHAdi007272 for ; Thu, 30 Mar 2006 11:17:11 GMT Received: from lon-cus-01.spheriq.net (lon-cus-01.spheriq.net [195.46.50.37]) by lon-out-01.spheriq.net with ESMTP id k2UBH90f015161 for ; Thu, 30 Mar 2006 11:17:09 GMT Received: from beta.dmz-eu.st.com (beta.dmz-eu.st.com [164.129.1.35]) by lon-cus-01.spheriq.net with ESMTP id k2UBH7aH019619 (version=TLSv1/SSLv3 cipher=EDH-RSA-DES-CBC3-SHA bits=168 verify=OK) for ; Thu, 30 Mar 2006 11:17:08 GMT Received: from zeta.dmz-eu.st.com (ns2.st.com [164.129.230.9]) by beta.dmz-eu.st.com (STMicroelectronics) with ESMTP id 50E18DA46 for ; Thu, 30 Mar 2006 11:17:07 +0000 (GMT) Received: from mail1.bri.st.com (mail1.bri.st.com [164.129.8.218]) by zeta.dmz-eu.st.com (STMicroelectronics) with ESMTP id 51AE847365 for ; Thu, 30 Mar 2006 11:21:09 +0000 (GMT) Received: from [164.129.15.13] (terrorhawk.bri.st.com [164.129.15.13]) by mail1.bri.st.com (MOS 3.5.8-GR) with ESMTP id CHK22772 (AUTH stubbsa); Thu, 30 Mar 2006 12:17:05 +0100 (BST) Message-ID: <442BBD9A.5020104@st.com> Date: Thu, 30 Mar 2006 14:16:00 -0000 From: Andrew STUBBS User-Agent: Thunderbird 1.5 (Windows/20051201) MIME-Version: 1.0 To: GDB Patches Subject: Re: [PATCH] Fixup internal variables' endian (again) References: <4408847C.9040907@st.com> <20060329232241.GD9916@nevyn.them.org> In-Reply-To: <20060329232241.GD9916@nevyn.them.org> Content-Type: multipart/mixed; boundary="------------070409090006000900050404" X-O-Spoofed: Not Scanned X-O-General-Status: No X-O-Spam1-Status: Not Scanned X-O-Spam2-Status: Not Scanned X-O-URL-Status: Not Scanned X-O-Virus1-Status: No X-O-Virus2-Status: Not Scanned X-O-Virus3-Status: No X-O-Virus4-Status: No X-O-Virus5-Status: Not Scanned X-O-Image-Status: Not Scanned X-O-Attach-Status: Not Scanned X-SpheriQ-Ver: 4.2.01 X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2006-03/txt/msg00358.txt.bz2 This is a multi-part message in MIME format. --------------070409090006000900050404 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-length: 2731 Daniel Jacobowitz wrote: > So, just minor comments. > >> + /* Values are always stored in the target's byte order. When connected to a >> + target this will most likely always be correct, so there's normally no >> + need to worry about it. >> + >> + However, internal variables can be set up before the target endian is >> + known and so may become out of date. Fix it up before anybody sees. >> + >> + Since internal variables cannot hold complex types, such as structs, >> + unions, arrays and strings - those are always held in target memory and >> + the variable just holds a reference, there is no need to worry about >> + those either. >> + >> + Floating point values vary differently across endianness - many targets >> + just keep them the same. If they do no work correctly on your host/target >> + then add support as required here. */ > > How do you feel about replacing those last two paragraphs with: > > Internal variables usually hold simple scalar values, and we can > correct those. More complex values (e.g. structures and floating > point types) are left alone, because they would be too complicated > to correct. > > I don't think we really want someone to fix this by adding floating > point support. Having taken some time looking into this I thought that doing a brain dump might be useful to other people. It's not that structs 'are too complicated to correct' - AFAICT they are almost always right because they require target memory to exist. The user has to go out of their way to confuse the debugger before they are wrong. As to the second point, why shouldn't people fix up broken values? If it is displayed incorrectly then that is a bug and the comment says that this is the place to fix it. People wouldn't have to 'add floating point support' as such, merely reverse the byte order of some binary data. It just so happens that the only scalar data not currently handled is floating point (AFAIK). I don't actually know of a host/target where this is a problem, but I'm sure there are some out there. In fact, it might be as simple as: { case TYPE_CODE_INT: case TYPE_CODE_PTR: +#ifdef ENDIAN_SENSITIVE_FLOATS + case TYPE_CODE_FLOAT: +#endif /* Reverse the bytes. */ for (i=0,j=TYPE_LENGTH (value_enclosing_type (val))-1; i * value.h (struct internalvar): Add field 'endian'. * value.c (lookup_internalvar): Initialise endian. (value_of_internalvar): Flip the endian of built-in types if required. (set_internalvar): Set the endian. (show_convenience): Access the value through value_of_internalvar(). Index: src/gdb/value.c =================================================================== --- src.orig/gdb/value.c 2006-02-01 23:14:10.000000000 +0000 +++ src/gdb/value.c 2006-03-30 12:13:27.000000000 +0100 @@ -755,6 +755,7 @@ lookup_internalvar (char *name) var = (struct internalvar *) xmalloc (sizeof (struct internalvar)); var->name = concat (name, (char *)NULL); var->value = allocate_value (builtin_type_void); + var->endian = TARGET_BYTE_ORDER; release_value (var->value); var->next = internalvars; internalvars = var; @@ -765,12 +766,46 @@ struct value * value_of_internalvar (struct internalvar *var) { struct value *val; + int i, j; + gdb_byte temp; val = value_copy (var->value); if (value_lazy (val)) value_fetch_lazy (val); VALUE_LVAL (val) = lval_internalvar; VALUE_INTERNALVAR (val) = var; + + /* Values are always stored in the target's byte order. When connected to a + target this will most likely always be correct, so there's normally no + need to worry about it. + + However, internal variables can be set up before the target endian is + known and so may become out of date. Fix it up before anybody sees. + + Internal variables usually hold simple scalar values, and we can + correct those. More complex values (e.g. structures and floating + point types) are left alone, because they would be too complicated + to correct. */ + + if (var->endian != TARGET_BYTE_ORDER) + { + gdb_byte *array = value_contents_raw (val); + struct type *type = check_typedef (value_enclosing_type (val)); + switch (TYPE_CODE (type)) + { + case TYPE_CODE_INT: + case TYPE_CODE_PTR: + /* Reverse the bytes. */ + for (i=0, j=TYPE_LENGTH (type) - 1; i < j; i++, j--) + { + temp = array[j]; + array[j] = array[i]; + array[i] = temp; + } + break; + } + } + return val; } @@ -809,6 +844,7 @@ set_internalvar (struct internalvar *var long. */ xfree (var->value); var->value = newval; + var->endian = TARGET_BYTE_ORDER; release_value (newval); /* End code which must not call error(). */ } @@ -877,7 +913,8 @@ show_convenience (char *ignore, int from varseen = 1; } printf_filtered (("$%s = "), var->name); - value_print (var->value, gdb_stdout, 0, Val_pretty_default); + value_print (value_of_internalvar (var), gdb_stdout, + 0, Val_pretty_default); printf_filtered (("\n")); } if (!varseen) Index: src/gdb/value.h =================================================================== --- src.orig/gdb/value.h 2006-02-01 23:14:10.000000000 +0000 +++ src/gdb/value.h 2006-03-30 11:17:29.000000000 +0100 @@ -245,6 +245,7 @@ struct internalvar struct internalvar *next; char *name; struct value *value; + int endian; }; --------------070409090006000900050404--