ARM constant number definitions are not correct for infinities

VERIFIED FIXED in mozilla1.4final



16 years ago
16 years ago


(Reporter: justin.fletcher, Assigned: brendan)


({fixed1.4, js1.5})

fixed1.4, js1.5
Bug Flags:
blocking1.4 +

Firefox Tracking Flags

(Not tracked)




(1 attachment)



16 years ago
User-Agent:       Mozilla/4.72 [en] (Compatible; RISC OS 4.36; Oregano 1.10)
Build Identifier: JavaScript-C 1.5 pre-release 5 2003-01-10 [checked out of CVS trunk 2003-30-05]

The values declared for POSITIVE_INFINITY, NEGATIVE_INFINITY and NaN are not 
correct for the ARM platform. As noted in the jsnum.h source as a comment from 
Stefan Hanske, the words are in little endian byte order, but the two words are 
reversed. The code was changed to reflect this and macros declared in jsnum.h 
to access these values in a generic manner.

The define CPU_IS_ARM is used, together with IS_LITTLE_ENDIAN to select the 
correct macros to use at jsnum.h, line 104. This appears to all be correct.

jsnum.c, however, has its own declaration of 'union dpun' which is used in the 
js_InitRuntimeNumberState functions to initialise the number structures.

If clauses within the 'union dpun' need to be updated to cater for the ARM CPU 
with something like :

#if defined(IS_LITTLE_ENDIAN) && !defined(CPU_IS_ARM)

or the assignment of the values should be updated to use the macros declared
in jsnum.c.

Reproducible: Always

Steps to Reproduce:
1. Build the standard engine and shell application.
2. print(Number.POSITIVE_INFINITY);
3. print(Number.NEGATIVE_INFINITY);

Actual Results:  
2.  1.06047983e-314
3.  2.1214777256e-314

Expected Results:  
Should have produced:
2.  Infinity
3.  -Infinity

Making the conditional in 'union dpun' as suggested above generated the 
Infinity/-Infinity output as expected.

I believe that whilst the above change to the 'union dpun' will fix the 
problem, the correct solution would be to make the code use the macros from 
jsnum.h as they were intended. If you want a patch putting together for that, 
let me know and I'll do that.

This bug has apparently been unnoticed for quite some time which leads me to 
believe that I may be incorrect in my assertion.

Comment 1

16 years ago
Reassigning; cc'ing Brendan and Waldemar -
Assignee: rogerl → khanson
Ever confirmed: true

Comment 2

16 years ago
Created attachment 124841 [details] [diff] [review]
proposed fix

Thanks, I think this is a real bug.  Not sure why it wasn't discovered earlier.

I took the opportunity to consolidate the union type, preferring shorter names
(making for shorter ugly macros).


Comment 3

16 years ago
Easy portability fix, candidate for the 1.4 branch as well as the 1.5 trunk.

Assignee: khanson → brendan
Flags: blocking1.4?
Keywords: js1.5
Priority: -- → P1
Target Milestone: --- → mozilla1.4final

Comment 4

16 years ago
Comment on attachment 124841 [details] [diff] [review]
proposed fix

Roger, can you eyeball this?  Thanks,

Attachment #124841 - Flags: review?(rogerl)
Flags: blocking1.4? → blocking1.4+


16 years ago
Attachment #124841 - Flags: review?(rogerl) → review+

Comment 5

16 years ago
Comment on attachment 124841 [details] [diff] [review]
proposed fix

rjesup pre-approved pending review.

Attachment #124841 - Flags: approval1.4+

Comment 6

16 years ago
Fixed in trunk and branch, thanks to Justin for reporting this.

Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 7

16 years ago
Marking Verified.

No regressions introduced on Linux or WinNT, as per the JS testsuite.
Also, checkin verified on 1.4 branch.
Keywords: fixed1.4, verified1.4
You need to log in before you can comment on or make changes to this bug.