Closed Bug 966596 Opened 10 years ago Closed 10 years ago

x32 support for nss

Categories

(NSS :: Build, enhancement, P2)

x86_64
Linux
enhancement

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: vapier, Assigned: wtc)

Details

Attachments

(4 files, 2 obsolete files)

Attached patch nss-3.15-x32.patch (obsolete) — Splinter Review
attached patch adds support for the x32 ABI to nss.  see bug 767759 for some background.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Attachment #8369020 - Flags: review?(wtc)
Comment on attachment 8369020 [details] [diff] [review]
nss-3.15-x32.patch

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

Mike,

Thank you for the patch. I have a question for you and an alternative suggestion below.

::: nss-3.15/nss/coreconf/Linux.mk
@@ +58,5 @@
> +	ARCHFLAG	= -m64
> +else
> +ifeq ($(USE_x32),1)
> +	OS_REL_CFLAGS	= -Di386
> +	CPU_ARCH	= x86

We probably should set CPU_ARCH to x32. This may require changes to some other makefiles though.

I am not sure if we should define -Di386. Does "gcc -mx32" define __i386__?

::: nss-3.15/nss/lib/freebl/Makefile
@@ +193,5 @@
>      MPI_SRCS += mpi_amd64.c mp_comba.c
>  endif
>  ifeq ($(CPU_ARCH),x86)
> +    ifeq ($(USE_x32),1)
> +	DEFINES += -DMP_CHAR_STORE_SLOW -DMP_IS_LITTLE_ENDIAN

If we set CPU_ARCH to x32 in Linux.mk, we can just test ifeq ($(CPU_ARCH),x32) here. However, on line 96, we will need to add x32 to the list:

 93 # NSS_X86 means the target is a 32-bits x86 CPU architecture
 94 # NSS_X64 means the target is a 64-bits 64 CPU architecture
 95 # NSS_X86_OR_X64 means the target is either x86 or x64
 96 ifeq (,$(filter-out i386 x386 x86 x86_64,$(CPU_ARCH)))
 97         DEFINES += -DNSS_X86_OR_X64
 98 ifdef USE_64
 99         DEFINES += -DNSS_X64
100 else
101         DEFINES += -DNSS_X86
102 endif
103 endi
Attached patch nss-3.15.4-x32.patch (obsolete) — Splinter Review
(In reply to Wan-Teh Chang from comment #1)
> I am not sure if we should define -Di386. Does "gcc -mx32" define __i386__?

if you have gcc-4.8 or newer, i think it supports -mx32

but no, it does not define __i386__.  it largely looks like -m64 (__x86_64__) than -m32.

i think this patch works and does what you ask.
Attachment #8369020 - Attachment is obsolete: true
Attachment #8369020 - Flags: review?(wtc)
Mike: thanks for answering my question and updating your patch.

I set up a Linux computer with gcc 4.8.1 last night, so I can play with
this now. nss/lib/freebl has a lot of x86_64 assembly code, so we should
try to use that for x32, but I agree it makes sense to first get a
successful build using just C code.

I will test your patch this week.
This patch is necessary for using the x86_64 assembly code
in nss/lib/freebl for x32. The reason is not obvious.

We are compiling NSS with -D_POSIX_SOURCE -D_BSD_SOURCE -D_XOPEN_SOURCE.
These -Dmacro flags were added because we used to compile NSS
the -ansi flag, so these *_SOURCE macros were necessary to
allow NSS to use certain system calls.

It turns out these *_SOURCE macros are the reason why
<limits.h> does not define the LLONG_MAX macro. I wondered
about this while investigating bug 972450, and now I know
why.

nss/lib/freebl/mpi/mpi.h tests LLONG_MAX to determine
if the long long type is available. If LLONG_MAX is not
defined, and if long is 32 bits, mpi.h is forced to
define mp_digit as a 32-bit integer type (unsigned int).
But the x86_64 assembly code in nss/lib/freebl assumes
mp_digit is a 64-bit integer type. If long is 32 bits
bug LLONG_MAX is defined, mpi.h will define mp_digit as
unsigned long long.
This patch allows us to use the x86_64 assembly code
in nss/lib/freebl for x32.

I found that it is more convenient to use set CPU_ARCH
to x86_64 for x32. NSS does that for Solaris sparc and
HP-UX ia64: the 32-bit and 64-bit ABIs use the same
CPU_ARCH value, and they are differentiated by USE_64.

We should not add -Di386 for x32 because GCC doesn't
define __i386__ for x32.

The NSS_X64 macro defined by nss/lib/freebl/Makefile
is the NSS equivalent of the GCC __x86_64__ macro, and
NSS_X86 is the NSS equivalent of __i386__.

In nss/lib/freebl/arcfour.c, if NSS_BEVAND_ARCFOUR is
defined, we are using the x86_64 assembly code for RC4,
and it assumes the WORD type is 64 bits.
Assignee: nobody → wtc
Attachment #8375381 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8379051 - Flags: review?(kaie)
Attachment #8379051 - Flags: feedback?(vapier)
This patch is 

nss/lib/zlib by default is not built on Linux; NSS uses the
system zlib library.

I found that it is necessary to define -DHAVE_UNISTD_H for
Linux x32. It has something to do with zconf.h undefining
_LARGEFILE64_SOURCE on line 374:

    367 /* a little trick to accommodate both "#define _LARGEFILE64_SOURCE" and
    368  * "#define _LARGEFILE64_SOURCE 1" as requesting 64-bit operations, (even
    369  * though the former does not conform to the LFS document), but considering
    370  * both "#undef _LARGEFILE64_SOURCE" and "#define _LARGEFILE64_SOURCE 0" as 
    371  * equivalently requesting no 64-bit operations
    372  */
    373 #if -_LARGEFILE64_SOURCE - -1 == 1
    374 #  undef _LARGEFILE64_SOURCE
    375 #endif
    376  
    377 #if defined(Z_HAVE_UNISTD_H) || defined(_LARGEFILE64_SOURCE)
    378 #  include <unistd.h>       /* for SEEK_* and off_t */

Adding -DHAVE_UNISTD_H ensures that Z_HAVE_UNISTD_H is
defined and <unistd.h> is included.
Attachment #8380245 - Flags: review?(kaie)
Attachment #8379051 - Flags: review?(kaie) → review+
Attachment #8380245 - Flags: review?(kaie) → review+
Attachment #8379061 - Flags: review+
Comment on attachment 8379051 [details] [diff] [review]
Primary NSS x32 patch, partially by  Mike Frysinger

Patch checked in:
https://hg.mozilla.org/projects/nss/rev/2938ac9193a7
https://hg.mozilla.org/projects/nss/rev/9297e47b52cd
Attachment #8379051 - Flags: checked-in+
Comment on attachment 8379061 [details] [diff] [review]
Optional: zlib patch

Patch checked in: https://hg.mozilla.org/projects/nss/rev/f5ecaae0aa62
Attachment #8379061 - Flags: checked-in+
Comment on attachment 8380245 [details] [diff] [review]
Patch for test scripts

Patch checked in: https://hg.mozilla.org/projects/nss/rev/af1b24199cf9
Attachment #8380245 - Flags: checked-in+
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
OS: Other → Linux
Priority: -- → P2
Resolution: --- → FIXED
Target Milestone: --- → 3.16
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: