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: