Closed Bug 220732 Opened 21 years ago Closed 21 years ago

JavaScript math on Alpha chip is broken

Categories

(Core :: JavaScript Engine, defect)

Product:
Core
Component:
JavaScript Engine
Platform:
DEC
Linux
Type:
defect
Priority:
Not set
Severity:
major

Tracking

()

Status:
RESOLVED DUPLICATE of bug 121115

People

(Reporter: csthomas, Assigned: timeless)

References

(
URL
)

Details

(Keywords: crash)

Attachments

(1 file)

patched on left, trunk on right diff of asm, courtesy timeless
1.11 KB, text/plain
Details 
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6a) Gecko/20030929
Build Identifier: Mozilla/5.0 (X11; U; Linux alpha; en-US; rv:1.5a) Gecko/20030929 Mozilla Firebird/0.6.1

alert(1732584193) produces different results on alpha.

Reproducible: Always

Steps to Reproduce:
1. visit provided URL
2. observe number in alert

Actual Results:  
displays -414899455 and then 2147483648 and then 1073741824 and then 9832753273

Expected Results:  
display 1732584193 and then 2147483648 and then 1073741824 and then 9832753273
(note this is greater than 2^32)

The numbers are, in binary:
01100111010001010010001100000001 (correct)
11100111010001010010001100000001 (incorrect)

10000000000000000000000000000000 (correct)

01000000000000000000000000000000 (correct)

1001001010000100111110100001111001 (correct)

Note that most bits are correct.

The broken math was observed and test cases tested by Hebford E'viarti.
does javascript:alert(1732584193) also give the wrong result on Alpha?
Hardware: Other → DEC
Note the URL given above simply does this:

<script>
function runtests()
{
  alert(1732584193);  <<<-------------- problem is with this one
  alert(2147483648);
  alert(1073741824);
  alert(9832753273);
}
</script>

<body onload="runtests();">
</body>



Note the base2 log of the problematic number:

js> function log2(x) {return Math.LOG2E * Math.log(x)}

js> log2(1732584193);
30.69027831421155
On Alpha the result of alert(1732584193) is -414899455

js> log2(414899455 + 1732584193)
31
Summary: javascript math on alpha is broken → JavaScript math on Alpha chip is broken
Hey, I am the fellow that decided there was a bug, mr. Thomas was crafty enough
to design a test based on the yahoo mail login. Yahoo was the first place I
noticed the bug, originally with mozilla 1.0. Mozilla of any type is currently
incapable of logging into yahoo mail. I built a copy of firebird 0.6.1, hoping
to fix that and a few other problems. Yahoo login still failed consistently
(apparently due to an md5 hash it generates, which were wildly different between
intel and alpha) I can say for a fact this bug does not affect sparc64.  The
first reporter of the bug was someone smart enough to figure out what my problem
was. we ran various tests, resulting in the bug report. I have two alphas and am
willing to test other problems, and try new code.  use either my account name,
jenklowiecz@yahoo.com, or  heviarti@puresimplicity.net
running cvs jsshell debug on predator:
js> 0-1073741824
js>

js> function a(){x=0-1073741824}
js> a
Program received signal SIGSEGV, Segmentation fault.
0x12008b55c in Decompile (ss=0x11fffe490, pc=0x1200fd95b "<", nb=10)
(gdb) where
#0  0x12008b55c in Decompile (ss=0x11fffe490, pc=0x1200fd95b "<", nb=10)
    at jsopcode.c:1892
#1  0x12008d134 in js_DecompileCode (jp=0x1200fd990, script=0x1200fd900,
    pc=0x1200fd958 "l", len=10) at jsopcode.c:2347
#2  0x12008d360 in js_DecompileScript (jp=0x1200fd990, script=0x1200fd900)
    at jsopcode.c:2367
#3  0x12008da6c in js_DecompileFunction (jp=0x1200fd990, fun=0x1200fd7c0)
    at jsopcode.c:2473
#4  0x120014cf0 in JS_DecompileFunction (cx=0x1200fe840, fun=0x1200fd7c0,
    indent=0) at jsapi.c:3385
#5  0x12004f2ec in js_fun_toString (cx=0x1200fe840, obj=0x120101fe0, indent=0,
    argc=0, argv=0x120110ce0, rval=0x11fffe738) at jsfun.c:1409

js> dis(a)
main:
00000:  bindname "x"
00003:  number undefined
00006:  setname "x"
00009:  pop

Source notes:
js>

Reading specs from /usr/lib/gcc-lib/alpha-linux/2.95.4/specs
gcc version 2.95.4 20011002 (Debian prerelease)
Linux predator 2.4.19 #1 Fri Sep 5 02:51:40 MDT 2003 alpha unknown

formally confirming that there's something wrong w/ that bit
(1073741824 == 0x40000000)
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: crash
Ok, some analysis:
this change to jsapi.h 'fixes' the 'problem':
-#define INT_FITS_IN_JSVAL(i)    ((jsuint)((i)+JSVAL_INT_MAX) <= 2*JSVAL_INT_MAX)
+#define INT_FITS_IN_JSVAL(i)    ((jsuint)((i)+(jsuint)JSVAL_INT_MAX) <= 2*JSVAL_INT_MAX)

The function which is breaking is EmitNumberOp
     if (JSDOUBLE_IS_INT(dval, ival) && INT_FITS_IN_JSVAL(ival)) {
                                   this ^^^^^^^^^^^^^^^^^^^^^^^
         if (ival == 0)
             return js_Emit1(cx, cg, JSOP_ZERO) >= 0;
         if (ival == 1)
             return js_Emit1(cx, cg, JSOP_ONE) >= 0;
         if ((jsuint)ival < (jsuint)ATOM_INDEX_LIMIT) {

The changed macro corresponds to this change:
-0x12003e300 <EmitNumberOp+192>: addl   t2,zero,t0
+0x12003e300 <EmitNumberOp+192>: zapnot t2,0xf,t0

I found this online:
gcc 3.1 uses addl to truncate t0, whereas 2.95 uses zapnot.
addl sign extends the value it writes to the destination,
thus it is apparent that 3.1 is sign-extending the value
even though the function prototype calls for an unsigned value.

brendan wonders whether the promotions done by the compiler are legal.
i suspect they are. if someone r+'s the concept i'll check in my change.
i'll probably test on sparc64, linux and beos before i make the commit.

anyway. i believe this is the answer:
http://gcc.gnu.org/ml/gcc-bugs/2002-05/msg00618.html
 The Alpha OSF/1 ABI specifies that all 32-bit arguments and return values
 are to be sign-extended in their registers, regardless of the signedness
 of the type involved.

 This is done because manipulating sign-extended values is much more
 efficient on the architecture.  Not that gcc always takes advantage...
Assignee: rogerl → timeless
<darin> timeless: i think your patch works because jsuint is uint32 but
JSVAL_INT_MAX is long... and long is 64bits
<darin> timeless: C would use the larger type
<darin> timeless: so it makes sense that (i)+JSVAL_INT_MAX would be computed
using type long
* timeless nods
<darin> timeless: and then when you cast to jsuint you go back down to uint32,
which truncates

r=darin
Not so fast.

<darin> timeless: so it makes sense that (i)+JSVAL_INT_MAX would be computed
using type long
* timeless nods
<darin> timeless: and then when you cast to jsuint you go back down to uint32,
which truncates

That's what should happen, and that's what I intended to have happen.  The value
in i is 0xc0000000, of 0xffffffffc0000000 as a long on Alpha.  Adding 0x3fffffff
(JSVAL_INT_MAX) to that gives 0xffffffff as an int or 0xffffffffffffffff as a
long.  It doesn't matter which is used, although I'm surprised the compiler is
free to promote to long from int (i is jsint, aka int32).  But in any case, the
outer cast to jsuint (uint32) should chop the -1 result down to 0xffffffff,
which is never <= 2 * JSVAL_INT_MAX (0x7ffffffe).

I still smell a compiler bug.  More in a bit.

/be
In other words, sign extension is fine, is what I want here, and is what the C
standard says must happen.  The issue is not sign extension. Rather, it is
whether the <= comparison is done using unsigned types properly.  If one compares

  (unsigned int)-1 <= 1

on Alpha, it shouldn't matter that the compiler prefers to let things
sign-extend to 64 bits.  The compiler must chop the high 32 bits off and compare
0xffffffff <= 1, and give a false result.

I don't see any argument passing in the macro (no functions), so that doesn't
matter.

/be
I was helping timeless debug this, but never got around to asking, what is in
t0?  That's one crucial question.

Another is: what comparison or branch instruction is used to implement the <= in
the macro?

/be
brendan, thanks for shooting me straight on the casting issue, but...

> But in any case, the
>outer cast to jsuint (uint32) should chop the -1 result down to 0xffffffff,
>which is never <= 2 * JSVAL_INT_MAX (0x7ffffffe).

it is if you use signed arithmetic to perform the <= computation.  perhaps it is
doing that?  0x7ffffffe is a big positive signed integer.  does that jive?
Darin: C's rules are set in stone. If one operand is unsigned, both are promoted
to unsigned.  Unless there's a wrinkle I don't know of, which would violate the
PLA, whereby on 64-bit systems, both operands can then *lose* unsignedness in
the conversion to a wider (long) type, the comparison should fail.

Unsigned is useful in C, and missing in Java, not just for modulus strength
reduction (i % 2048 can be reduced to i & 2047 only if i is unsigned).  It also
helps avoid a branch when comparing min <= i && i <= max, for signed i: change
that to (unsigned)i <= max - min (which works because 0 <= i is always true for
unsigned i).

/be
note that not being able to log in to yahoo mail is bug 121115, which was a
64-bit gcc bug.  The bug has been fixed in gcc (3.2, 3.3 and 3.4), and the fix
is included in the updated gcc (2.96) from Compaq/HP/RH72

Some of the discussion here seems to be rehashing that bug although the original
testcase still exhibits the problem as originally described even using the fixed
gcc.
Here's the meat:

-	zapnot $3,15,$1
+	addl $3,$31,$1
	lda $2,-2
	ldah $2,16384($2)
	ldah $2,16384($2)
-	cmpule $1,$2,$1
+	cmple $1,$2,$1

This appears to be a gcc bug: the addl is wrong given the cast to (jsuint) of
the left operand of <=, but the cmple is even more wrong.  If cmpule were used,
the sign extension done by addl wouldn't matter.  But it seems as though this
broken gcc is completely ignoring the cast.

/be
I tested gcc323 on predator and filed bug 221087 for changes which
enabled me to build.

someone needs to file a bug against debian to have them backport the
64bit gcc fix to their alpha distribution. (i'd nominate heviarti.)

ajschult: which testcase is still broken w/ the fix?
Andrew, thanks for the reminder.  It's amazing how perfectly I can forget about
this sort of bug, once some time has passed!

I'll leave it to timeless to mark this a dup.

/be
> ajschult: which testcase is still broken w/ the fix?

false alarm.  I was testing a gcc3 build (RH72's gcc3 didn't get the patch
backported).  The testcase here (http://ctho9305.res.cmu.edu/alphatest.html)
works fine with the patched gcc296.
marking dupe

*** This bug has been marked as a duplicate of 121115 ***
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: