209814 - PR_dtoa blows up when executed on an ARM platform

Status: RESOLVED FIXED

(NSPR :: NSPR, defect)

Description

[Darin Fisher]

despite the fact that ARM is a little endian architecture, it stores doubles in
big endian format.  this means that the conversion macros in prdtoa.c (word0 and
word1) need to be defined differently for the ARM platform.

Comment 1

[Darin Fisher]

Attachment: v1 patch

note: this patch is basically ripped from mozilla/js/src/jsnum.h

Comment 2

[Wan-Teh Chang]

Comment on attachment 125923 [details] [diff] [review]
v1 patch

>-#ifdef IEEE_8087
>+#ifdef IEEE_8087 && !defined(CPU_IS_ARM)

Instead of this, I think we should say:

#if  defined(IS_LITTLE_ENDIAN) && !defined(CPU_IS_ARM)
#define IEEE_8087
#else
#define IEEE_MC68k
#endif

Also, maybe we should also test for defined(LINUX),
because there are other OS's that run on ARM and
I'm not sure if they also need this fix.

Comment 3

[Darin Fisher]

wtc: i originally wrote code like that and it didn't work.  the problem is that
we need to assume IEEE_8087 semantics in all cases except when we try to access
the individual DWORDs stored in a double.  you see, the DWORDS are stored in
reverse order, but are still little endian.  also, i'm pretty sure the same
format is used by WinCE.  i think we should try to follow what spidermonkey
(mozjs) does, which is the patch i wrote.

Comment 4

[Darin Fisher]

so, this bug causes NSS initialization to fail on ARM.  NSS appears to be the
only caller of PR_dtoa.

wan-teh: your thoughts on comment #3 ?

Comment 5

[Darin Fisher]

Attachment: v1.1 patch

the previous patch was malformed (i think i must have edited it without
testing).  this is what i meant.

Comment 6

[Darin Fisher]

-> me

Comment 7

[Wan-Teh Chang]

Attachment: wtc's alternate patch

Darin, I believe that something along the line of
this patch should work.

I added a new case for IEEE arithmetic called IEEE_ARM.

For the word0 and word1 macros, IEEE_ARM is treated like
IEEE_MC68k (big endian).

For the Storeinc macro, IEEE_ARM is treated like
IEEE_8087 (little endian).

Please review and test this patch.  Please run the
dtoa.c and strod.c tests in nsprpub/pr/tests.
Thanks.

Comment 8

[Darin Fisher]

Comment on attachment 141345 [details] [diff] [review]
wtc's alternate patch

dtoa test passed.  patch looks good to me.

r=darin

thanks wtc!

Comment 9

[Darin Fisher]

Comment on attachment 141345 [details] [diff] [review]
wtc's alternate patch

requesting approval to land this patch.  it only affects the ARM build of NSPR.

Comment 10

[Brendan Eich [:brendan]]

Comment on attachment 141345 [details] [diff] [review]
wtc's alternate patch

a=[e-mail] for 1.7b.

/be

Comment 11

[Darin Fisher]

fixed NSPRPUB_PRE_4_2_CLIENT_BRANCH for mozilla 1.7 beta 
 ... and the NSPR trunk.