[Qemu-devel] [PATCH] Fix char signedness

[Qemu-devel] [PATCH] Fix char signedness

[Andreas Schwab]

If you want to put a negative number in a char you must use a signed char.

Andreas.

Index: fpu/softfloat-native.c
===================================================================
RCS file: /sources/qemu/qemu/fpu/softfloat-native.c,v
retrieving revision 1.5
diff -u -a -p -r1.5 softfloat-native.c
--- fpu/softfloat-native.c	22 Oct 2006 00:18:54 -0000	1.5
+++ fpu/softfloat-native.c	27 Oct 2006 17:00:21 -0000
@@ -149,7 +149,7 @@ float32 float32_sqrt( float32 a STATUS_P
 {
     return sqrtf(a);
 }
-char float32_compare( float32 a, float32 b STATUS_PARAM )
+signed char float32_compare( float32 a, float32 b STATUS_PARAM )
 {
     if (a < b) {
         return -1;
@@ -161,7 +161,7 @@ char float32_compare( float32 a, float32
         return 2;
     }
 }
-char float32_compare_quiet( float32 a, float32 b STATUS_PARAM )
+signed char float32_compare_quiet( float32 a, float32 b STATUS_PARAM )
 {
     if (isless(a, b)) {
         return -1;
@@ -258,7 +258,7 @@ float64 float64_sqrt( float64 a STATUS_P
 {
     return sqrt(a);
 }
-char float64_compare( float64 a, float64 b STATUS_PARAM )
+signed char float64_compare( float64 a, float64 b STATUS_PARAM )
 {
     if (a < b) {
         return -1;
@@ -270,7 +270,7 @@ char float64_compare( float64 a, float64
         return 2;
     }
 }
-char float64_compare_quiet( float64 a, float64 b STATUS_PARAM )
+signed char float64_compare_quiet( float64 a, float64 b STATUS_PARAM )
 {
     if (isless(a, b)) {
         return -1;
@@ -350,7 +350,7 @@ floatx80 floatx80_sqrt( floatx80 a STATU
 {
     return sqrtl(a);
 }
-char floatx80_compare( floatx80 a, floatx80 b STATUS_PARAM )
+signed char floatx80_compare( floatx80 a, floatx80 b STATUS_PARAM )
 {
     if (a < b) {
         return -1;
@@ -362,7 +362,7 @@ char floatx80_compare( floatx80 a, float
         return 2;
     }
 }
-char floatx80_compare_quiet( floatx80 a, floatx80 b STATUS_PARAM )
+signed char floatx80_compare_quiet( floatx80 a, floatx80 b STATUS_PARAM )
 {
     if (isless(a, b)) {
         return -1;
Index: fpu/softfloat-native.h
===================================================================
RCS file: /sources/qemu/qemu/fpu/softfloat-native.h,v
retrieving revision 1.7
diff -u -a -p -r1.7 softfloat-native.h
--- fpu/softfloat-native.h	22 Oct 2006 00:18:54 -0000	1.7
+++ fpu/softfloat-native.h	27 Oct 2006 17:00:21 -0000
@@ -181,8 +181,8 @@ INLINE char float32_unordered( float32 a
     return isunordered(a, b);
 
 }
-char float32_compare( float32, float32 STATUS_PARAM );
-char float32_compare_quiet( float32, float32 STATUS_PARAM );
+signed char float32_compare( float32, float32 STATUS_PARAM );
+signed char float32_compare_quiet( float32, float32 STATUS_PARAM );
 char float32_is_signaling_nan( float32 );
 
 INLINE float32 float32_abs(float32 a)
@@ -263,8 +263,8 @@ INLINE char float64_unordered( float64 a
     return isunordered(a, b);
 
 }
-char float64_compare( float64, float64 STATUS_PARAM );
-char float64_compare_quiet( float64, float64 STATUS_PARAM );
+signed char float64_compare( float64, float64 STATUS_PARAM );
+signed char float64_compare_quiet( float64, float64 STATUS_PARAM );
 char float64_is_signaling_nan( float64 );
 flag float64_is_nan( float64 );
 
@@ -345,8 +345,8 @@ INLINE char floatx80_unordered( floatx80
     return isunordered(a, b);
 
 }
-char floatx80_compare( floatx80, floatx80 STATUS_PARAM );
-char floatx80_compare_quiet( floatx80, floatx80 STATUS_PARAM );
+signed char floatx80_compare( floatx80, floatx80 STATUS_PARAM );
+signed char floatx80_compare_quiet( floatx80, floatx80 STATUS_PARAM );
 char floatx80_is_signaling_nan( floatx80 );
 
 INLINE floatx80 floatx80_abs(floatx80 a)
Index: fpu/softfloat.c
===================================================================
RCS file: /sources/qemu/qemu/fpu/softfloat.c,v
retrieving revision 1.3
diff -u -a -p -r1.3 softfloat.c
--- fpu/softfloat.c	22 Oct 2006 00:18:54 -0000	1.3
+++ fpu/softfloat.c	27 Oct 2006 17:00:21 -0000
@@ -5283,7 +5283,7 @@ unsigned int float64_to_uint32_round_to_
 }
 
 #define COMPARE(s, nan_exp)                                                  \
-INLINE char float ## s ## _compare_internal( float ## s a, float ## s b,     \
+INLINE signed char float ## s ## _compare_internal( float ## s a, float ## s b,     \
                                       int is_quiet STATUS_PARAM )            \
 {                                                                            \
     flag aSign, bSign;                                                       \
@@ -5317,12 +5317,12 @@ INLINE char float ## s ## _compare_inter
     }                                                                        \
 }                                                                            \
                                                                              \
-char float ## s ## _compare( float ## s a, float ## s b STATUS_PARAM )       \
+signed char float ## s ## _compare( float ## s a, float ## s b STATUS_PARAM )       \
 {                                                                            \
     return float ## s ## _compare_internal(a, b, 0 STATUS_VAR);              \
 }                                                                            \
                                                                              \
-char float ## s ## _compare_quiet( float ## s a, float ## s b STATUS_PARAM ) \
+signed char float ## s ## _compare_quiet( float ## s a, float ## s b STATUS_PARAM ) \
 {                                                                            \
     return float ## s ## _compare_internal(a, b, 1 STATUS_VAR);              \
 }
Index: fpu/softfloat.h
===================================================================
RCS file: /sources/qemu/qemu/fpu/softfloat.h,v
retrieving revision 1.4
diff -u -a -p -r1.4 softfloat.h
--- fpu/softfloat.h	22 Oct 2006 00:18:54 -0000	1.4
+++ fpu/softfloat.h	27 Oct 2006 17:00:21 -0000
@@ -234,8 +234,8 @@ char float32_lt( float32, float32 STATUS
 char float32_eq_signaling( float32, float32 STATUS_PARAM );
 char float32_le_quiet( float32, float32 STATUS_PARAM );
 char float32_lt_quiet( float32, float32 STATUS_PARAM );
-char float32_compare( float32, float32 STATUS_PARAM );
-char float32_compare_quiet( float32, float32 STATUS_PARAM );
+signed char float32_compare( float32, float32 STATUS_PARAM );
+signed char float32_compare_quiet( float32, float32 STATUS_PARAM );
 char float32_is_signaling_nan( float32 );
 flag float64_is_nan( float64 a );
 
@@ -283,8 +283,8 @@ char float64_lt( float64, float64 STATUS
 char float64_eq_signaling( float64, float64 STATUS_PARAM );
 char float64_le_quiet( float64, float64 STATUS_PARAM );
 char float64_lt_quiet( float64, float64 STATUS_PARAM );
-char float64_compare( float64, float64 STATUS_PARAM );
-char float64_compare_quiet( float64, float64 STATUS_PARAM );
+signed char float64_compare( float64, float64 STATUS_PARAM );
+signed char float64_compare_quiet( float64, float64 STATUS_PARAM );
 char float64_is_signaling_nan( float64 );
 
 INLINE float64 float64_abs(float64 a)

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [Qemu-devel] [PATCH] Fix char signedness

[Johannes Schindelin]

Hi,

On Fri, 27 Oct 2006, Andreas Schwab wrote:

> If you want to put a negative number in a char you must use a signed char.

It has been a really long time I have been working on a broken system that 
did not default to "signed". Just out of curiosity: what system did you 
see the breakage on?

Ciao,
Dscho

Re: [Qemu-devel] [PATCH] Fix char signedness

[andrzej zaborowski]

On 27/10/06, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> On Fri, 27 Oct 2006, Andreas Schwab wrote:
> It has been a really long time I have been working on a broken system that
> did not default to "signed". Just out of curiosity: what system did you
> see the breakage on?

AFAIK, it's not a broken system. The default signedness of char is
undefined and I remember reading something on arm.linux.org.uk about
ARM compilers (including gcc) using the opposite default to the rest
of the archs.

Regards,

Re: [Qemu-devel] [PATCH] Fix char signedness

[Andreas Schwab]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> It has been a really long time I have been working on a broken system that 
> did not default to "signed".

The only thing that is broken is your knowlege of C.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [Qemu-devel] [PATCH] Fix char signedness

[M. Warner Losh]

In message: <jeac3hd4jv.fsf@sykes.suse.de>
            Andreas Schwab <schwab@suse.de> writes:
: Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
: 
: > It has been a really long time I have been working on a broken system that 
: > did not default to "signed".
: 
: The only thing that is broken is your knowlege of C.

gcc on ARM systems default to unsigned.  The C standard specifically
states that char is either signed or unsigned at the whim of the
implementor:

Section 6.2.5 para 15:

       [#15] The three types char, signed char, and  unsigned  char
       are   collectively   called   the   character   types.   The
       implementation shall define char to  have  the  same  range,
       representation,  and  behavior  as  either  signed  char  or
       unsigned char.30)
...
       30)CHAR_MIN, defined in <limits.h>, will  have  one  of  the
          values   0   or  SCHAR_MIN,  and  this  can  be  used  to
          distinguish the two options.  Irrespective of the  choice
          made,  char  is a separate type from the other two and is
          not compatible with either.
...

I just confirmed that gcc on arm does indeed default to unsigned
chars:

% cat xxx.c
signed char cs = -1;
unsigned char cu = -1;
char c = -1;

void foo()
{
        int i = 0;
        if (cs < 0) i++;
        if (cu < 0) i++;
        if (c < 0) i++;
}
%  gcc -v
Using built-in specs.
Configured with: FreeBSD/arm system compiler
Thread model: posix
gcc version 3.4.4 [FreeBSD] 20050518
% gcc -W -c xxx.c
xxx.c: In function `foo':
xxx.c:9: warning: comparison is always false due to limited range of data type
xxx.c:10: warning: comparison is always false due to limited range of data type
%

Note: line 10 is the naked char.

Warner

Re: [Qemu-devel] [PATCH] Fix char signedness

[Johannes Schindelin]

Hi,

On Fri, 27 Oct 2006, Andreas Schwab wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > It has been a really long time I have been working on a broken system that 
> > did not default to "signed".
> 
> The only thing that is broken is your knowlege of C.

Okay.

And what system did you encounter this behaviour on?

Ciao,
Dscho

Re: [Qemu-devel] [PATCH] Fix char signedness

[Paul Brook]

> > > It has been a really long time I have been working on a broken system
> > > that did not default to "signed".
> >
> > The only thing that is broken is your knowlege of C.
>
> Okay.
>
> And what system did you encounter this behaviour on?

Common arm, ppc, s390 and mips systems all have default char as unsigned.
As do a few other less common embedded systems.

Paul

Re: [Qemu-devel] [PATCH] Fix char signedness

[Thiemo Seufer]

Johannes Schindelin wrote:
> Hi,
> 
> On Fri, 27 Oct 2006, Andreas Schwab wrote:
> 
> > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> > 
> > > It has been a really long time I have been working on a broken system that 
> > > did not default to "signed".
> > 
> > The only thing that is broken is your knowlege of C.
> 
> Okay.
> 
> And what system did you encounter this behaviour on?

AFAIR powerpc and ARM are the usual suspects for unsigned chars.


Thiemo

Re: [Qemu-devel] [PATCH] Fix char signedness

[M. Warner Losh]

In message: <Pine.LNX.4.63.0610280057240.26682@wbgn013.biozentrum.uni-wuerzburg.de>
            Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
: Hi,
: 
: On Fri, 27 Oct 2006, Andreas Schwab wrote:
: 
: > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
: > 
: > > It has been a really long time I have been working on a broken system that 
: > > did not default to "signed".
: > 
: > The only thing that is broken is your knowlege of C.
: 
: Okay.
: 
: And what system did you encounter this behaviour on?

It is the default for arm with gcc, for one.

The behavior is standards conforming, per section 6.2.5, para 15 of
ANSI-C (1989) standard.  Your characterization of it as 'broken' is
wrong.

Warner

Re: [Qemu-devel] [PATCH] Fix char signedness

[Laurent Desnogues]

Johannes Schindelin a écrit :
> 
> And what system did you encounter this behaviour on?

Do a:

    grep -wr DEFAULT_SIGNED_CHAR gcc/config

in a gcc source tree and you will get a list for gcc targets :)


			Laurent

Re: [Qemu-devel] [PATCH] Fix char signedness

[Martin Guy]

> gcc on ARM systems default to unsigned.  The C standard specifically
> states that char is either signed or unsigned at the whim of the
> implementor

Or, more to the point, at the behest of the machine architecture.
Having to generate code to sign-extend the hard way every time you do
char-integer promotion if the hardware doesn't do it automatically
would be long and inefficient, specially since it happens all the
time.
This has been a problem since the "All the world's a VAX" days, the
classic boob being

char c;
while ((c = getchar()) != EOF) {
     ...
}

of course, 255 always != -1 so it loops forever.
Check how many of your C primers get this wrong!
The flipside of that boob is that on a signed char architecture, the
loop will exit prematurely when it meets a 255 character.

These days instead, all the world's a 386 and anything different is broken...

24-bit integers, anyone? :)

   M

Re: [Qemu-devel] [PATCH] Fix char signedness

[Rob Landley]

On Friday 27 October 2006 7:09 pm, Paul Brook wrote:
> > > > It has been a really long time I have been working on a broken system
> > > > that did not default to "signed".
> > >
> > > The only thing that is broken is your knowlege of C.
> >
> > Okay.
> >
> > And what system did you encounter this behaviour on?
> 
> Common arm, ppc, s390 and mips systems all have default char as unsigned.
> As do a few other less common embedded systems.

BusyBox added "-funsigned-char" to our build to get consistent behavior.  (I 
picked unsigned because that way we were automatically 8-bit clean in things 
like sed.)

(I note that tcc doesn't work right if built with -funsigned-char, so I 
wouldn't be too surprised if qemu had the same problem.)  In both cases, 
specifying -fsigned-char in the build should fix it.

> Paul

Rob
-- 
"Perfection is reached, not when there is no longer anything to add, but
when there is no longer anything to take away." - Antoine de Saint-Exupery

Re: [Qemu-devel] [PATCH] Fix char signedness

[Rob Landley]

On Saturday 28 October 2006 6:46 am, Martin Guy wrote:
> > gcc on ARM systems default to unsigned.  The C standard specifically
> > states that char is either signed or unsigned at the whim of the
> > implementor
> 
> Or, more to the point, at the behest of the machine architecture.
> Having to generate code to sign-extend the hard way every time you do
> char-integer promotion if the hardware doesn't do it automatically
> would be long and inefficient, specially since it happens all the
> time.

I'd rather have inefficient than broken.  And in general, using "char" as a 
small "int" is going to _suck_ on arm.  The extra code size more than makes 
up for the data size you save if you only have one or two instances of your 
data at a time.

Rob
-- 
"Perfection is reached, not when there is no longer anything to add, but
when there is no longer anything to take away." - Antoine de Saint-Exupery