Mozilla incl. firebird does not build on Amd64 cpu

RESOLVED FIXED

Status

()

defect
--
blocker
RESOLVED FIXED
16 years ago
15 years ago

People

(Reporter: henrik, Assigned: henrik)

Tracking

({64bit, fixed1.7})

Trunk
x86
Linux
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.7a -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

Assignee

Description

16 years ago
I have just got a new amd64 bit system and naturally wanted to build mozilla on it.

I'm running gentoo linux in 64 bit mode, and It comes with mozilla so I knew it
should work. However my own builds from cvs failed.

The gentoo ebuilds has some amd64 patches it applies to the mozilla source
before building it, so I tried to manually apply the same patch to my cvs tree,
and it worked for firebird (currently testing the suite)

I will attach the patch to this bugreport
Assignee

Comment 2

16 years ago
This patch is taken from gentoo linux's portage and it makes the gentoo mozilla
build, along with my cvs tree build (pulled today) both firebird and the suite.


ps. I set severity to blocker, since this bug blocks the building and testing
of mozilla apps.

pps. I choose xpcom since i believe the patch touches some xpcom files (but Im
not sure).

afaik it patches:
henrik@workstation MozillaBuild $ patch -p1
</usr/portage/net-www/mozilla/files/mozilla-1.4-amd64.patch
patching file config/mkdepend/imakemdep.h
patching file directory/c-sdk/ldap/libraries/liblber/lber-int.h
patching file nsprpub/configure
Hunk #1 succeeded at 3736 (offset 7 lines).
patching file nsprpub/configure.in
Hunk #1 succeeded at 1222 (offset 4 lines).
patching file nsprpub/pr/include/md/_linux.cfg
patching file nsprpub/pr/include/md/_linux.h
Hunk #2 succeeded at 111 (offset 2 lines).
patching file nsprpub/pr/src/io/prprf.c
patching file nsprpub/pr/src/md/unix/os_Linux_x86_64.s
patching file security/coreconf/Linux.mk
Hunk #2 succeeded at 116 (offset 5 lines).
patching file xpcom/reflect/xptcall/src/md/unix/Makefile.in
patching file xpcom/reflect/xptcall/src/md/unix/xptcinvoke_x86_64_linux.cpp
patching file xpcom/reflect/xptcall/src/md/unix/xptcstubs_x86_64_linux.cpp
patching file xpcom/reflect/xptcall/tests/TestXPTCInvoke.cpp
Assignee

Comment 3

16 years ago
Requesting that this blocks 1.7a..
Flags: blocking1.7a?
Assignee

Updated

16 years ago
Blocks: 163013

Comment 4

16 years ago
FWIW, the same patch is included with Blizzard's SRPMS (except that the attached
patch includes changes to TestXPTCInvoke.cpp)

Comment 5

16 years ago
Can someone update the patch for mozilla1.7a trunk and request review for it,
please ?
We're not going to hold the 1.7a release because porting fixes aren't in. 
Trying to get porting fixes in is a good thing, but blocking flags aren't the
way to do it.
Flags: blocking1.7a? → blocking1.7a-

Comment 7

16 years ago
Comment on attachment 140309 [details] [diff] [review]
This patch makes mozilla build on amd64 under gentoo linux

Requesting r= from wtc for the NSPR and NSS changes per bryner's suggestion...
Attachment #140309 - Flags: superreview?(bryner)
Attachment #140309 - Flags: review?(wchang0222)

Comment 8

16 years ago
Comment on attachment 140309 [details] [diff] [review]
This patch makes mozilla build on amd64 under gentoo linux

>diff -ruN mozilla.old/config/mkdepend/imakemdep.h 

>@@ -236,23 +236,22 @@

>-#elif !defined(__alpha) || defined(VMS)
>+#elif !defined(__x86_64__) && (!defined(__alpha) || defined(VMS))
> 
> #define LBER_HTONL( l )	htonl( l )
> #define LBER_NTOHL( l )	ntohl( l )
> 
> #else /* __alpha */

 nit: comment here should be updated to include '__x86_64__'


>- * htonl and ntohl on the DEC Alpha under OSF 1 seem to only swap the
>- * lower-order 32-bits of a (64-bit) long, so we define correct versions
>- * here.
>+ * htonl and ntohl on the 64-bit UNIX platforms only swap the lower-order
>+ * 32-bits of a (64-bit) long, so we define correct versions here.

  nit: How about 64bit version of AIX, Solaris and Irix? If they don't need
this, the comment shouldn't say '64-bit UNIX platforms....'. Instead, list
platforms that are affected (Alpha and AMD64).

Comment 9

16 years ago
Comment on attachment 140309 [details] [diff] [review]
This patch makes mozilla build on amd64 under gentoo linux

The NSPR and NSS changes in this patch look good.
I can't really review the x86-64 assembly code in
mozilla/nsprpub/pr/src/md/unix/os_Linux_x86_64.s,
but it looks close enough to the x86 assembly code
in mozilla/nsprpub/pr/src/md/unix/os_Linux_x86.s.
I suggest that you run the following NSPR test:

    % cd nsprpub/pr/tests
    % make atomic
    % ./atomic -d

If someone who knows x86-64 assembly language can
review os_Linux_x86_64.s, I'd appreciate it.

Question: can we add the following assembler directive
at the end of os_Linux_x86_64.s?

/ Magic indicating no need for an executable stack
.section .note.GNU-stack, "", @progbits ; .previous

os_Linux_x86.s has it.

Comment 10

16 years ago
Comment on attachment 140309 [details] [diff] [review]
This patch makes mozilla build on amd64 under gentoo linux

I forgot to mention that I've already checked in
the only NSS change in this patch
(mozilla/security/coreconf/Linux.mk) on the NSS
trunk (NSS 3.10), NSS_3_9_BRANCH (NSS 3.9.1), and
NSS_CLIENT_TAG (Mozilla 1.7a).	I will take care
of the NSPR changes when I have the NSPR "atomic"
test result and the answer to my question about
the non-executable stack assembler directive.

Comment 11

15 years ago
Comment on attachment 140309 [details] [diff] [review]
This patch makes mozilla build on amd64 under gentoo linux

I've checked in the NSPR changes in this patch (after
minor editing) on the NSPR trunk (NSPR 4.5) and
NSPRPUB_PRE_4_2_CLIENT_BRANCH (Mozilla 1.7 beta).
Comment on attachment 140309 [details] [diff] [review]
This patch makes mozilla build on amd64 under gentoo linux

xptcinvoke_x86_64_linux.cpp and xptcstubs_x86_64_linux.cpp are missing license
headers.  sr=bryner with that fixed.  (note that I can't vouch for the
corectness of the xptcall assembly code)
Attachment #140309 - Flags: superreview?(bryner) → superreview+

Comment 13

15 years ago
Comment on attachment 140309 [details] [diff] [review]
This patch makes mozilla build on amd64 under gentoo linux

>diff -ruN mozilla.old/directory/c-sdk/ldap/libraries/liblber/lber-int.h 

>-#elif !defined(__alpha) || defined(VMS)
>+#elif !defined(__x86_64__) && (!defined(__alpha) || defined(VMS))
> 
> #define LBER_HTONL( l )	htonl( l )
> #define LBER_NTOHL( l )	ntohl( l )
> 
> #else /* __alpha */
> /*
>- * htonl and ntohl on the DEC Alpha under OSF 1 seem to only swap the
...
>+ * htonl and ntohl on the 64-bit UNIX platforms only swap the lower-order

how do you know this is true of all 64-bit UNIX platforms?  what about
64-bit Solaris?  should you remove the "/* __alpha */" comment?


>+#endif /* __alpha || __x86_64__ */

this comment isn't really correct.


>diff -ruN mozilla.old/nsprpub/pr/src/md/unix/os_Linux_x86_64.s

>+/ The Initial Developer of the Original Code is Netscape

is Netscape really the initial developer?


>diff -ruN mozilla.old/xpcom/reflect/xptcall/src/md/unix/xptcinvoke_x86_64_linux.cpp 

>+// 6 integral parameters are passed in registers
>+const PRUint32 GPR_COUNT = 6;
>+
>+// 8 floating point parameters are passed in SSE registers
>+const PRUint32 FPR_COUNT = 8;

nit: declare these static


>+    typedef PRUint32 (*Method)(PRUint64, PRUint64, PRUint64, PRUint64, PRUint64, PRUint64);
>+    PRUint32 result = ((Method)methodAddress)(a0, a1, a2, a3, a4, a5);

uhm.. does this mean that this code will only work with methods that take
6 or fewer arguments?  (i admit that i don't know much about xtpcall stuff)

dbradley should probably review this code.


>diff -ruN mozilla.old/xpcom/reflect/xptcall/src/md/unix/xptcstubs_x86_64_linux.cpp 

>+const PRUint32 PARAM_BUFFER_COUNT   = 16;
>+const PRUint32 GPR_COUNT            = 6;
>+const PRUint32 FPR_COUNT            = 8;

declare static


>+    NS_ASSERTION(dispatchParams,"no place for params");
>+    if (! dispatchParams)
>+        return NS_ERROR_OUT_OF_MEMORY;

nit: use NS_ERROR instead


>+            NS_ASSERTION(0, "bad type");
>+            break;

NS_NOTREACHED


>+    NS_ASSERTION(0,"nsXPTCStubBase::Sentinel called"); \

NS_NOTREACHED
Attachment #140309 - Flags: review?(wchang0222) → review+

Updated

15 years ago
Blocks: 237202
as you might know an incompatible patch for the same thing has been checked in
with bug #236599. Please solve this.
Blocks: 236599

Updated

15 years ago
Attachment #140309 - Flags: approval1.7?

Comment 15

15 years ago
(In reply to comment #2)
> Created an attachment (id=140309)
> This patch makes mozilla build on amd64 under gentoo linux

Can you please make a new patch _ASAP_ incl.the corrections requested by comment
#13 (comment #14 is invalid AFAIK, that patch was not checked-in yet) ?
There is a small chanche that this patch can be shipped with mozilla1.7final and
I'd like to grab it.
Assignee: dougt → admin

Comment 16

15 years ago
Comment on attachment 140309 [details] [diff] [review]
This patch makes mozilla build on amd64 under gentoo linux

Please ask for approval again when there is a version of the patch
that matches the current tree (comment #14).
Attachment #140309 - Flags: approval1.7? → approval1.7-
This is the patch I use currently to build on AMD64.
It fits into the current tree and should do it.

Updated

15 years ago
Attachment #146251 - Flags: superreview?(darin)
Attachment #146251 - Flags: review?(darin)

Comment 18

15 years ago
Comment on attachment 146251 [details] [diff] [review]
patch for current tree

seems fine to me, although i haven't carefully reviewed the assembly code. 
since x86-64 is the only platform affected by that code, i'm going to take your
word for it that it does the right thing.

this error seems badly worded:

+#error "can't find a compiler to use"

perhaps it should say something like your compiler is not supported instead.
Attachment #146251 - Flags: superreview?(darin)
Attachment #146251 - Flags: superreview+
Attachment #146251 - Flags: review?(darin)
Attachment #146251 - Flags: review+
I went ahead and corrected the comment, and added the lisence blocks to the new
files and checked this in. Marking FIXED.
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Flags: blocking1.7?
Resolution: --- → FIXED
Comment on attachment 146629 [details] [diff] [review]
Same thing for the 1.7 branch.

Requesting approval for landing this Linux/x86-64 only change on the 1.7
branch.
Attachment #146629 - Flags: superreview+
Attachment #146629 - Flags: review+
Attachment #146629 - Flags: approval1.7?
Comment on attachment 146629 [details] [diff] [review]
Same thing for the 1.7 branch.

a=brendan@mozilla.org for 1.7 final.

/be
Attachment #146629 - Flags: approval1.7? → approval1.7+
Fix checked in (and tested on the branch too).
Flags: blocking1.7?
Keywords: fixed1.7
hmm, the patch didn't appear in the 1.7 branch. Can you please check?
Hmm, not sure what happened there. Now it's in, for sure!
You need to log in before you can comment on or make changes to this bug.