441767 - arm builds need an ABI defined

Status: RESOLVED FIXED

(Toolkit :: Startup and Profile System, defect)

Description

[Justin Dolske [:Dolske]]

I've got a plain Firefox 3.0 build, compiled in Scratchbox, with the ARMEL target.

Trying to run the Mozilla Labs Weave extension (after building its binary component) fails: "TypeError: Cc['@labs.mozilla.com/Weave/Crypto;1'] is undefined"

It would appear the EM is trying to load it, using nsIXULRuntime.XPCOMABI to determing the path for $extdir/platform/XXX/components/foo.so. If I try to print that value from the JS console, I get a NS_ERROR_NOT_AVAILABLE, which is what nsAppRunner.cpp when $TARGET_XPCOM_ABI isn't defined. And my 2am poking at configure seems to indicate that depends on $CPU_ARCH, which in turn is only set for a handful of known platforms ("arm" not being one of them).

[Sorry about the brain dump, it's late]

Comment 1

[Benjamin Smedberg]

If you put the component in the plain components/ directory (not the platform directory) does it work?

Not having a known XPCOM ABI for various platforms is known and intended in most cases, until somebody can identify precise ABI variations.

Comment 2

[Justin Dolske [:Dolske]]

(In reply to comment #1)
> If you put the component in the plain components/ directory (not the platform
> directory) does it work?

Yeah, that works. Although it's not really a good long-term solution for something Weave, which wants to have a single .xpi with binary components for multiple platforms.

> Not having a known XPCOM ABI for various platforms is known and intended in
> most cases, until somebody can identify precise ABI variations.

In one sense, fixing this bug should be "easy" in that it's "just" a bit of configure plumbing to hook up TARGET_XPCOM_ABI to some value. But I guess what's you're saying is that the tricky bit is figuring out exactly what that value should be? Eg, "arm" might be too generic, while "armv6l" (which is what uname -m yields) might be too specific?

Comment 3

[Benjamin Smedberg]

Yes... I really have no clue what all the variants of ARM are, or what that might mean for binary compatibility, so I've been punting on this issue.

Hrm, maybe we should just make weave not need binary components?

Comment 4

[(not currently active) Ted Mielczarek]

Or just ship a separate weave xpi for mobile, since that would cut down the filesize as well.

Comment 5

[Justin Dolske [:Dolske]]

(In reply to comment #3)
> Yes... I really have no clue what all the variants of ARM are, or what that
> might mean for binary compatibility, so I've been punting on this issue.

This isn't a blocker for Weave (I'm mostly just playing on my own to see if it works). But I'd presume with our interest in mobile and the N810, we'd want a platform-parity so that using binary components work the same way as other platforms...

> Hrm, maybe we should just make weave not need binary components?

Hrm, do you have a scriptable NSS interface somewhere? :-)

Comment 6

[Dave Townsend [:mossop]]

Note that this also has impact when trying to restrict what platforms an extension will install in using the targetPlatform attribute. While you could make an xpi just for mobile, you wouldn't be able to stop it being installed accidentally anywhere else with the current state.

http://developer.mozilla.org/en/docs/install.rdf#targetPlatform

Comment 7

[Justin Dolske [:Dolske]]

Doug, any idea who can give a definitive answer to comment 3?

Comment 8

[Doug Turner (:dougt)]

Guillaume Letellier might be able to help here.

Comment 9

[Guillaume Letellier]

(In reply to comment #8)

I would have expected $CPU_ARCH to be armv5, armv6, armv7 ...
and TARGET_COMPILER_ABI to be "gnueabi" (arm new EABI, same as ARMEL)
But it seems that $CPU_ARCH for the other platforms e.g. PCs are the generic name i.e. x86
Do you use the same binary component for i386 and i686??

If you want to make an xpi just for mobile (and to be consistent with the other platforms), we should use the generic "arm" (and compile for armv5).

Comment 10

[Dan Mills [:thunder]]

Weave has a binary component, so we need this bug to be fixed.

We can of course use whatever you guys decide, but as a point of reference, 'uname -m' returns 'arm' in our scratchbox setup we're using to build.

Comment 11

[Benjamin Smedberg]

Weave doesn't *need* this bug to be fixed: it could ship the ARM component in the generic components directory. That said, I'd still love for somebody to come forward and give a comprehensive overview of the binary compatibility implications of ARM.

Comment 12

[Dan Mills [:thunder]]

(In reply to comment #11)
> Weave doesn't *need* this bug to be fixed: it could ship the ARM component in
> the generic components directory.

Yes, though doing so would prevent us from creating a single XPI for all platforms, which what we're doing for mac/win/linux.

Though since arm is generally used for phones, to keep the download small we might produce a fennec-only XPI anyway.

Comment 13

[Benjamin Smedberg]

Why would you have to do that? No other platform can load that binary, so it would result in a simple "couldn't load component" warning.

Comment 14

[Dan Mills [:thunder]]

Ah yes, you are right.  In that case, we only have an interest in this bug being fixed, but it's not really a blocker for Weave :)

Comment 15

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Morphing this bug a bit.

Right now, on ARM linux, we get no TARGET_XPCOM_ABI because CPU_ARCH is never set at http://mxr.mozilla.org/mozilla-central/source/configure.in#1386 .  In some android patches, we set that to "arm", and so we end up with "arm-gcc3", which is closer, but not accurate enough given that there's two ABIs there, eabi and well, the old abi, let's call it oabi.  msvc also has its own, but that's already handled.

For EABI, __ARM_EABI__ is defined by gcc, so we have a way to check whether we're building EABI.  So, I'd suggest that we have the following ABIs:

arm-msvc
arm-eabi-gcc3
arm-oabi-gcc3

There is one additional wrinkle, which is ARM architecture version, but I think we can ignore that for now, as we'd need to teach xpcom to load multiple ABIs (e.g. arm-eabi-gcc3 and armv7-eabi-gcc3).

Comment 16

[(not currently active) Ted Mielczarek]

That sounds sane to me. Is eabi really tied to gcc3, or could we just use arm-eabi (or gnueabi as suggested in comment 9)?

Comment 17

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

It's not tied to gcc3, but the g++ ABI still matters as well.  eabi is also not gnu specific, so I don't think "gnueabi" is any good; I think "gnueabi" was a hack from linux-arm-gnueabi to avoid having it be linux-arm-gnu-eabi.

Comment 18

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: patch, v0

Perhaps like so.

Comment 19

[(not currently active) Ted Mielczarek]

Comment on attachment 435748 [details] [diff] [review]
patch, v0

>diff --git a/configure.in b/configure.in
>--- a/configure.in
>+++ b/configure.in
>@@ -4025,8 +4021,31 @@ dnl Put your C++ language/feature checks
> dnl ========================================================
> AC_LANG_CPLUSPLUS
> 
>+HAVE_ARM_EABI=

Doesn't look like you use the value of this variable anywhere.

>+ARM_ABI_PREFIX=
> HAVE_GCC3_ABI=
> if test "$GNU_CC"; then
>+  if test "$CPU_ARCH" = "arm" ; then
>+    AC_CACHE_CHECK(for ARM EABI,
>+        ac_cv_gcc_arm_eabi,
>+        [AC_TRY_COMPILE([],
>+                        [
>+#if defined(__ARM_EABI__)
>+  return 0;
>+#else
>+#error Not ARM EABI.
>+#endif
>+                        ],
>+                        ac_cv_gcc_arm_eabi="yes",
>+                        ac_cv_gcc_arm_eabi="no")])
>+    if test "$ac_cv_gcc_arm_eabi" = "yes"; then
>+        HAVE_ARM_EABI=1
>+        ARM_ABI_PREFIX=eabi-
>+    else
>+        ARM_ABI_PREFIX=oabi-

Not that we're shipping on anything with the old ABI, but is it safe to assume that "not EABI" is always "OABI"?

Also, js/src/configure has this same logic. I think you can probably remove it, although it does look like a test runner expects it to be present:
http://mxr.mozilla.org/mozilla-central/source/js/src/tests/manifest.py#51
so maybe you should just copy your changes there instead.

Comment 20

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

(In reply to comment #19)
> >+HAVE_ARM_EABI=
> 
> Doesn't look like you use the value of this variable anywhere.

Was there just for completeness, though any code that actually cares can just check for __ARM_EABI__.  Think mwu's going to nuke it.

> Not that we're shipping on anything with the old ABI, but is it safe to assume
> that "not EABI" is always "OABI"?

Yep, if we're here, we're on gcc already.. and so we either have the old abi or eabi.

Comment 21

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

http://hg.mozilla.org/mozilla-central/rev/f8124bb7a8a0