Note: There are a few cases of duplicates in user autocompletion which are being worked on.

fix JS compilation for Darwin/ARM

RESOLVED FIXED in mozilla9

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: dougt, Assigned: ted)

Tracking

unspecified
mozilla9
ARM
iOS 4
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [iOS][fixed-in-bs][fixed-in-nanojit])

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

6 years ago
Created attachment 550749 [details] [diff] [review]
work in progress.

ted: 

-#include <prtypes.h>
+//#include <prtypes.h>

I am not sure why this change.
(Reporter)

Updated

6 years ago
Whiteboard: [iOS]
(Reporter)

Comment 1

6 years ago
Created attachment 550750 [details] [diff] [review]
build on ios complication fixes
Attachment #550750 - Flags: review?(cdleary)
Comment on attachment 550750 [details] [diff] [review]
build on ios complication fixes

Passing over to Marty, who has the ARM builds going these days.
Attachment #550750 - Flags: review?(cdleary) → review?(mrosenberg)
Comment on attachment 550750 [details] [diff] [review]
build on ios complication fixes

Review of attachment 550750 [details] [diff] [review]:
-----------------------------------------------------------------

I don't see anything wrong with the patch per se.  It doesn't look like it breaks any existing code, and I can only assume that it builds firefox correctly for ios, so lgtm
Attachment #550750 - Flags: review?(mrosenberg) → review+
(Assignee)

Updated

6 years ago
Attachment #550749 - Attachment is obsolete: true
(Assignee)

Updated

6 years ago
Assignee: doug.turner → ted.mielczarek
(Assignee)

Updated

6 years ago
OS: FreeBSD → iOS 4
Hardware: All → ARM
(Assignee)

Comment 4

6 years ago
I hit a snag actually trying to build with this patch on an updated tree. I'll put a new patch up soon.
(Assignee)

Comment 5

6 years ago
Created attachment 554141 [details] [diff] [review]
js arm-darwin fixes

Okay, I had to merge in some bits from some other patches I had laying around. I'll comment specifically on some of the bits here, since they're non-obvious.
Attachment #550750 - Attachment is obsolete: true
Attachment #554141 - Flags: review?
(Assignee)

Comment 6

6 years ago
Comment on attachment 554141 [details] [diff] [review]
js arm-darwin fixes

Review of attachment 554141 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/assembler/jit/ExecutableAllocator.h
@@ +374,4 @@
>          _flush_cache(reinterpret_cast<char*>(code), size, BCACHE);
>  #endif
>      }
> +#elif WTF_CPU_ARM && WTF_OS_IOS

We never set WTF_CPU_ARM_THUMB2 (different from upstream webkit), so this code wouldn't get built previously.

::: js/src/assembler/wtf/Platform.h
@@ +329,4 @@
>  /* WTF_CPU_ARMV5_OR_LOWER - ARM instruction set v5 or earlier */
>  /* On ARMv5 and below the natural alignment is required. 
>     And there are some other differences for v5 or earlier. */
> +#if !defined(ARMV5_OR_LOWER) && WTF_CPU_ARM && !(WTF_ARM_ARCH_VERSION >= 6)

This is just wrong, I don't know if it's necessary, but it doesn't match upstream certainly.

@@ +1115,5 @@
>  /* Setting this flag prevents the assembler from using RWX memory; this may improve
>     security but currectly comes at a significant performance cost. */
>  #if WTF_PLATFORM_IOS
> +//XXX: this doesn't currently compile in the spidermonkey build
> +#define ENABLE_ASSEMBLER_WX_EXCLUSIVE 0

The build breaks with this defined, maybe an artifact of our build system, but it's easier to just turn this off for now.

::: js/src/configure.in
@@ -2006,4 @@
>      STRIP="$STRIP -x -S"
>      _PLATFORM_DEFAULT_TOOLKIT='cairo-cocoa'
>      TARGET_NSPR_MDCPUCFG='\"md/_darwin.cfg\"'
> -    LDFLAGS="$LDFLAGS -framework Cocoa -lobjc"

Turns out this is totally unnecessary. Probably just copy/pasted from the top-level configure.
(Assignee)

Updated

6 years ago
Attachment #554141 - Flags: review? → review?(mrosenberg)
Comment on attachment 554141 [details] [diff] [review]
js arm-darwin fixes


>-#if !defined(ARMV5_OR_LOWER) && WTF_CPU_ARM && WTF_ARM_ARCH_VERSION >= 6
>+#if !defined(ARMV5_OR_LOWER) && WTF_CPU_ARM && !(WTF_ARM_ARCH_VERSION >= 6)
This seems rather bad.

>-#define ENABLE_ASSEMBLER_WX_EXCLUSIVE 1
>+//XXX: this doesn't currently compile in the spidermonkey build
>+#define ENABLE_ASSEMBLER_WX_EXCLUSIVE 0
I assume this is done through a system call we haven't bothered to research/use
Attachment #554141 - Flags: review?(mrosenberg) → review+
(Assignee)

Comment 8

6 years ago
(In reply to Marty Rosenberg [:Marty] from comment #7)
> Comment on attachment 554141 [details] [diff] [review]
> js arm-darwin fixes
> 
> 
> >-#if !defined(ARMV5_OR_LOWER) && WTF_CPU_ARM && WTF_ARM_ARCH_VERSION >= 6
> >+#if !defined(ARMV5_OR_LOWER) && WTF_CPU_ARM && !(WTF_ARM_ARCH_VERSION >= 6)
> This seems rather bad.

I don't know how that got flipped from upstream. It probably hasn't been hurting anything because we force the use of the standard ARM backend and not the thumb backend.

> >-#define ENABLE_ASSEMBLER_WX_EXCLUSIVE 1
> >+//XXX: this doesn't currently compile in the spidermonkey build
> >+#define ENABLE_ASSEMBLER_WX_EXCLUSIVE 0
> I assume this is done through a system call we haven't bothered to
> research/use

No, I think it's just a build configuration that we haven't tested, so it doesn't quite build properly. We can probably make it work, it just seemed simpler to disable it for the moment.
(Assignee)

Comment 9

6 years ago
http://hg.mozilla.org/projects/build-system/rev/06defc1b4250
Whiteboard: [iOS] → [iOS] fixed-in-bs
http://hg.mozilla.org/mozilla-central/rev/06defc1b4250
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla9
Ok, bad news.  Nanojit changes have to be landed on nanojit-central and then merged to mozilla-central.  See https://developer.mozilla.org/en/NanojitMerge for details.  The important thing is that if you don't do that, any Nanojit changes that land on mozilla-central get clobbered when the next Nanojit merge happens.  I just did one, and the changes from this bug got clobbered.

I can reland the Nanojit part of this patch in nanojit-central and do a merge.  Is it urgent?  (Nanojit changes are supposed to get a review from an Adobe person too, but these ones are innocuous enough that we can probably let it slide, though I've CC'd Edwin Smith just in case.)

Comment 12

6 years ago
Informal R+ from me.
(In reply to Edwin Smith from comment #12)
> Informal R+ from me.

Ok, thanks, I'll land and merge this later today.
http://hg.mozilla.org/projects/nanojit-central/rev/d66d2e24ef16
Whiteboard: [iOS] fixed-in-bs → [iOS][fixed-in-bs][fixed-in-nanojit]
http://hg.mozilla.org/integration/mozilla-inbound/rev/6974e199548d
Whiteboard: [iOS][fixed-in-bs][fixed-in-nanojit] → [iOS][fixed-in-bs][fixed-in-nanojit][inbound]
(Assignee)

Comment 16

6 years ago
Thanks, sorry about that!
http://hg.mozilla.org/mozilla-central/rev/6974e199548d
Whiteboard: [iOS][fixed-in-bs][fixed-in-nanojit][inbound] → [iOS][fixed-in-bs][fixed-in-nanojit]
You need to log in before you can comment on or make changes to this bug.