Bug 17923 - ARM platform endian defines inaccurate in JavaScriptCore
Summary: ARM platform endian defines inaccurate in JavaScriptCore
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 15416
Blocks:
  Show dependency treegraph
 
Reported: 2008-03-18 15:03 PDT by David Krause
Modified: 2008-03-20 07:34 PDT (History)
1 user (show)

See Also:


Attachments
Proposed patch (1.21 KB, patch)
2008-03-19 06:44 PDT, David Krause
ddkilzer: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Krause 2008-03-18 15:03:30 PDT
As part of bug 15416 (http://bugs.webkit.org/show_bug.cgi?id=15416), a change was made to JavaScriptCore/wtf/Platform.h that affected when the WTF_PLATFORM_MIDDLE_ENDIAN define is set.  The specific changeset is http://trac.webkit.org/projects/webkit/changeset/31115.

Basically, some ARM platforms that define __ARMEL__ still need the MIDDLE_ENDIAN define.  Specifically, it affects those little-endian ARM platforms using FPA floating-point.  See comment on original bug for more details at http://bugs.webkit.org/show_bug.cgi?id=15416#c15.

The 31115 change causes JavaScript issues on those ARM platforms that no longer have the MIDDLE_ENDIAN define, but need it.  The issue seen is that the JavaScript dtoa function falls into an infinite loop.
Comment 1 Mark Rowe (bdash) 2008-03-18 15:08:24 PDT
How can the other platforms that need MIDDLE_ENDIAN be detected?
Comment 2 David Krause 2008-03-18 15:11:53 PDT
The only recommendation I have was in my previous comment.  I am not sure which defines are present for VFP floating-point, but for FPA the following works:

#elif !defined(__ARM_EABI__) && !defined(__ARMEB__) && defined(__ARMEL__) &&
defined(__SOFTFP__)

Right now, I only have access to my version of gcc, which sets __ARMEL__ and __SOFTFP__ for FPA.
Comment 3 David Kilzer (:ddkilzer) 2008-03-18 15:13:35 PDT
Is there not an __ARMEM__ macro for middle endian systems?

Or is this sufficient (per Bug 15416 Comment #15)?

#elif !defined(__ARM_EABI__) && !defined(__ARMEB__) && defined(__ARMEL__) && defined(__SOFTFP__)
Comment 4 David Kilzer (:ddkilzer) 2008-03-18 15:17:25 PDT
(In reply to comment #2)
> The only recommendation I have was in my previous comment.  I am not sure which
> defines are present for VFP floating-point, but for FPA the following works:
> 
> #elif !defined(__ARM_EABI__) && !defined(__ARMEB__) && defined(__ARMEL__) &&
> defined(__SOFTFP__)
> 
> Right now, I only have access to my version of gcc, which sets __ARMEL__ and
> __SOFTFP__ for FPA.

If this is sufficient, please post a patch with changelog to this bug.  Thanks!

Comment 5 David Krause 2008-03-18 15:27:22 PDT
No __ARMEM__ macro exists.  I'll post a patch as requested.
Comment 6 David Krause 2008-03-19 06:44:22 PDT
Created attachment 19884 [details]
Proposed patch

Found a VFP toolchain, and it also defines __ARMEL__ and __SOFTFP__, so my earlier proposal would not have been appropriate.  __VFP_FP__ is the define that indicates VFP support, which should not use MIDDLE_ENDIAN.
Comment 7 David Kilzer (:ddkilzer) 2008-03-20 06:52:34 PDT
Comment on attachment 19884 [details]
Proposed patch

Looks good!  r=me
Comment 8 David Kilzer (:ddkilzer) 2008-03-20 07:34:13 PDT
Committed revision 31176.