If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

ARM constant number definitions are not correct for infinities

VERIFIED FIXED in mozilla1.4final

Status

()

Core
JavaScript Engine
P1
normal
VERIFIED FIXED
15 years ago
15 years ago

People

(Reporter: Justin Fletcher, Assigned: brendan)

Tracking

({fixed1.4, js1.5})

Trunk
mozilla1.4final
Other
Other
fixed1.4, js1.5
Points:
---
Bug Flags:
blocking1.4 +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment)

(Reporter)

Description

15 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:  
Output:
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

15 years ago
Reassigning; cc'ing Brendan and Waldemar -
Assignee: rogerl → khanson
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 2

15 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).

/be
(Assignee)

Comment 3

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

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

Comment 4

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

Roger, can you eyeball this?  Thanks,

/be
Attachment #124841 - Flags: review?(rogerl)

Updated

15 years ago
Flags: blocking1.4? → blocking1.4+

Updated

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

Comment 5

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

rjesup pre-approved pending review.

/be
Attachment #124841 - Flags: approval1.4+
(Assignee)

Comment 6

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

/be
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED

Comment 7

15 years ago
Marking Verified.

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