[comp.sys.dec] BUG is MIPS cc for "?" operator

schuh@whey.cs.wisc.edu (Dan Schuh) (02/02/90)

In <21624@pasteur.Berkeley.EDU> jac@modoc.berkeley.edu (Gordon Jacobs) writes:

--The program below demonstrates various uses of the "?"
--selection operator in c that seem to not work correctly on
--a DEC3100 running Ultrix3.1.  This has also been compiled
--on a unit running a test version of Ultrix4.0 with the
--exact same results.  This seems like a fairly major bug and
--it could affect much old software.  Has anyone experience ...
( relatively long example deleted, excerpt below)


This is not actually a bug, though I'm sure many have been and will
continue to be bitten by it.  It's actually an ansi-ism in the mips C
compiler.  The root of the problem is that sizeof is treated as
unsigned by the mips C compiler..  K&R I listed sizeof as a signed
value, but K&R II and ansi are both clear  in making it an unsigned
value.  This can result in unexpected results in comparisions as the
unsigned-ness of sizeof propagates through expressions, under the
normal c rules.

In the first 'wrong' result instance in your code, you have
#define MAX(A,B)        ((A)>(B))?(A):(B)
#define BITS_PER_INT    (sizeof(int)*8)
...
	int high, low;
	high= 19;
...
	low = MAX(0, high-BITS_PER_INT+1);

and low ends up unexpectedly as -12 instead of 0.  BITS_PER_INT is an
unsigned expression due to the presence of sizeof, and therefore
high-BITS_PER_INT+1 also becomes unsigned, turning into something
undeniably greater that zero.  When the value of (B) gets assigned to
low, it is coerced back into a sized value and becomes -12; but in its
unsized incarnation it couldn't possibly be less than 0. 

Note that in all the working examples, the comparable expressions
do not contain sizeof, vis:

#define BITS_PER_INT 32
low = MAX(0, high-BITS_PER_INT+1);

When I first came across this I was pulling out hair as I looked at the
assembler output of the offending code compared to the dissassembled
output of the same code in the debugger; I had written something like

if ( 0 < ( value - sizeof(something)) ....

The assembler output  at the relevant point in the expression jump code was:

bgeu <dest>

while the disassembled object said

br  <dest>

So I was running around telling people bizarre things were happening
in the mips linker/ backend optimizer or something, until enlightenment
dawned and I realized that a branch greater than or equal unsigned (bgeu)
was indeed a branch.

Cheers, Dan, schuh@cs.wisc.edu

lai@mips.COM (David Lai) (02/02/90)

In article <21624@pasteur.Berkeley.EDU> jac@modoc.berkeley.edu (Gordon Jacobs) writes:
>The program below demonstrates various uses of the "?"
>selection operator in c that seem to not work correctly on
>a DEC3100 running Ultrix3.1.  This has also been compiled
>on a unit running a test version of Ultrix4.0 with the 
>exact same results.  This seems like a fairly major bug and
>it could affect much old software.  Has anyone experienced
>this or confirmed it??  The test program shows version which
>work and don't work...
>
>Thanks,
>Gordie Jacobs,  UCB  
>------------------ cut here --------------------------------------
>
>#define MAX(A,B)  	((A)>(B))?(A):(B)
>
>#define BITS_PER_INT 	(sizeof(int)*8)
>#define INTS_PER_BUS 	((int)(20/BITS_PER_INT)+1)
>
>main () {
>
>	int	high, low;
>	int	bits_per_int, sizeofint;
>
>	printf("INTS_PER_BUS=%d BITS_PER_INT=%d\n",
>	       INTS_PER_BUS, BITS_PER_INT);
>
>	bits_per_int= BITS_PER_INT;
>	sizeofint= sizeof(int);
>
>	high= 19;
>	
>	/* All of these compute the wrong answer (-12 instead of 0) */ 
>	low = MAX(0, high-BITS_PER_INT+1);
>	low = (0>(high-BITS_PER_INT+1))?(0):(high-BITS_PER_INT+1);
>	low = MAX(0, high-sizeof(int)*8+1);
>	low = (0>(high-sizeof(int)*8+1))?(0):(high-sizeof(int)*8+1);
>
>	/* Works:
>	#define BITS_PER_INT 32
>	low = MAX(0, high-BITS_PER_INT+1);
>	*/
>
>	/* Works:
>	low = MAX(0, high-sizeofint*8+1);
>	*/
>
>	/* Works:
>	   low = MAX(0, low=high-BITS_PER_INT+1);
>	   */
>	
>	/* Works:
>	   low = (0>(high-bits_per_int+1))?(0):(high-bits_per_int+1);
>	   */
>	
>	/* Works:
>	   low = (0>(high-32+1))?(0):(high-32+1);
>	   */
>	
>	/* Works:
>	   low = high-BITS_PER_INT+1;
>	   low = MAX(0,low);
>	   */
>	
>	printf("high=%d low=%d MAX(0,-12)=%d\n", high, low, MAX(0,-12));
>}

The problem is the sizeof() operator is unsigned, therefore the expression
of the form
	(int) - (unsigned) + (int)
yields an unsigned result.  This result just happens to be a very large
number which cannot accurately be represented by an int.  This number is
assigned to an int, which causes the upper bit to be treated as a sign
bit, and the value appears to be -12.

A quick fix is to do

	#define SIZEOF(x) ((int)sizeof(x))

and to use SIZEOF instead of sizeof() in the program.
-- 
        "What is a DJ if he can't scratch?"  - Uncle Jamms Army
     David Lai (lai@mips.com || {ames,prls,pyramid,decwrl}!mips!lai)