Closed Bug 253241 Opened 19 years ago Closed 19 years ago

wrong JS math in 64bit systems

Categories

(Core :: JavaScript Engine, defect)

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

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: wolfiR, Assigned: wolfiR)

References

(
URL
)

Details

(Keywords: fixed-aviary1.0, fixed1.7.5)

Attachments

(2 files)

build output from make -f Makefile.ref
42.59 KB, text/plain
Details
fix endian problems
1.10 KB, patch
shaver
: review+
brendan
: approval-aviary+
brendan
: approval1.7.5+
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8a3) Gecko/20040725
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8a3) Gecko/20040725

use a 64bit build of mozilla (at least AMD64) under Linux and open the given URL.
All prices are zero. They are someway calculated as JavaScript code. So it seems
that the JS engine is not 64bit clean.

Reproducible: Always
Steps to Reproduce:
1.open URL with 64bit mozilla
2.look at the prices
3.open URL with 32bit mozilla
4.look at the prices again

Actual Results:  
with 64bit system all prices are 0

Expected Results:  
you should get the same prices
JS has been 64-bit clean for six+ years on many platforms, including DEC Alpha.
 You'll have to debug this more yourself, and say *exactly* what version and
brand of compiler you are using.

/be
Assignee: general → mozilla
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: wrong JS math in 64bit systems → wrong JS math in 64bit systems
it's on SUSE Linux 9.1 for AMD64 using the compiler:
gcc (GCC) 3.3.3 (SUSE Linux)

If you tell me what else I could debug myself I will try it.
Perhaps I can try it on IA64 tomorrow.
cvs co mozilla/js/src
cd mozilla/js/src
make -f Makefile.ref
cd *.OBJ/
./js

--
js>

evaluate js here until you find your problem.

when you find the math that doesn't work, repeat the process on an i386 system.
I will see what I can do. But at this time I only get a compile error for this
and count 65 times:

warning: cast from pointer to integer of different size
or 
warning: cast to pointer from integer of different size

I can really imagine that this can lead to wrong math-functions.

Wolfgang: what *error* do you get?  Can you give file and line for those
warnings (which I assume were not the error you mentioned)?

/be
that'd probably be Bug 249478
the build error is this bug.
But the main problem is that something is calculating wrong in JS.
And in general I count 333 build warnings about invalid casts on AMD64 for the
mozilla source. It seems AMD64 is a problematic platform if it comes to 
integer <-> pointer casts.

And sorry. I was unable to strip down a testcase because my HTML and JS 
experience is not good and the example URL has complicated contents.
So any news?
This should be easier to track down than "something is calculating wrong in JS"

[nicholas@entropy firefox]$ ./run-mozilla.sh ./xpcshell
js> Math.acos(1)
1.5707963267948966
js> Math.acos(0.99)
NaN
js>
[nicholas@entropy firefox]$ uname -a
Linux entropy 2.6.8-1.549smp #1 SMP Mon Sep 6 16:20:20 EDT 2004 x86_64 x86_64
x86_64 GNU/Linux
Looking at fdlibm.h, it appears that an architecture's endianness must be
explicitly specified, and big endian is assumed.

The fix is trivial, but that begs the question of why fdlibm is used in the
first place instead of the OS provided math library.
If your OS's math lib satisfies ECMA-262 Edition 3's requirements, then you can
dispense with fdlibm.  The js/tests suite may not be enough to verify that your
OS's math lib satisfies, however.

/be
This fixes the endian problems for AMD64, in a broken non-future proof manner.
As a bonus, VA_COPY is changed to the correct va_copy.
Comment on attachment 158276 [details] [diff] [review]
fix endian problems

I'm down with this.  Thanks for the diagnosis and patch.

I'd like this on both branches, as it's a serious porting fix for an important
platform.
Attachment #158276 - Flags: review+
Attachment #158276 - Flags: approval1.7.x?
Attachment #158276 - Flags: approval-aviary?
Comment on attachment 158276 [details] [diff] [review]
fix endian problems

Yeah, this should go into both branches.

/be
Attachment #158276 - Flags: approval1.7.x?
Attachment #158276 - Flags: approval1.7.x+
Attachment #158276 - Flags: approval-aviary?
Attachment #158276 - Flags: approval-aviary+
Actually, a better fix would be to make fdlibm.h include jscpucfg.h/jsautocfg.h
and do
#if IS_LITTLE_ENDIAN
#define __LITTLE_ENDIAN
#endif
Has a fix for this been landed anywhere?  This needs to be fixed for 1.0 IMO
Flags: blocking-aviary1.0?
Comment 15 points the way, but I'm checking in the reviewed and approved patch
for the branches and trunk right now.

/be
Status: NEW → RESOLVED
Closed: 19 years ago
Flags: blocking-aviary1.0? → blocking-aviary1.0+
Keywords: fixed-aviary1.0, fixed1.7.x
Resolution: --- → FIXED
brendan, this busted: monkeypox, speedracer, and worms
I undid the jsprf.c change.  Nicholas, please file a separate bug and provide a
patch for it that doesn't break the ports tinderboxes broken by your jsprf.c
patch attached here.

/be
*** Bug 257221 has been marked as a duplicate of this bug. ***
You need to log in before you can comment on or make changes to this bug.