1010730 - Compilation failure on MIPS64 n32 platforms

Status: RESOLVED INACTIVE

(NSS :: Build, defect, P2)

Description

[Vicente Olivert Riera]

Attachment: libnss-fix-mips-n32.patch

This is the compilation failure:

################################################
In file included from
/home/test/test/1/output/host/usr/mips64el-buildroot-linux-gnu/sysroot/usr/include/nspr/prerror.h:9:0,
                 from drbg.c:10:
drbg.c: In function 'RNG_RandomUpdate':
/home/test/test/1/output/host/usr/mips64el-buildroot-linux-gnu/sysroot/usr/include/nspr/prtypes.h:560:38:
error: size of array 'arg' is negative
     extern void pr_static_assert(int arg[(condition) ? 1 : -1])
                                      ^
drbg.c:512:5: note: in expansion of macro 'PR_STATIC_ASSERT'
     PR_STATIC_ASSERT(sizeof(size_t) > 4);
     ^
make[4]: ***
[Linux2.6_mips64el_glibc_PTH_64_OPT.OBJ/Linux_SINGLE_SHLIB/drbg.o]
Error 1
################################################

The failure comes from this file
"mozilla/security/nss/lib/freebl/drbg.c" on the line #512, which has
an assert of the size of "size_t":

PR_STATIC_ASSERT(sizeof(size_t) > 4)

That line is inside an #if/#endif block which has this form:

#if defined(NS_PTR_GT_32) || (defined(NSS_USE_64) &&
!defined(NS_PTR_LE_32))

So, that code is being executed because "NSS_USE_64" is true.

The problem is that in MIPS64 n32 the size of size_t is exactly 4 (is
an unsigned integer of 32 bits), and then that assert fails.

But, the mechanism to fix that is already present. If you look more
carefully at the #if statement you will see this at the end:

!defined(NS_PTR_LE_32)

That macro is for 64-bit architectures that use 32-bit pointers, which
is exactly the case of MIPS64 n32. So, to fix the problem we only need
to detect when we are building for MIPS64 n32, and then define that
macro so the #if statement will fail and the #else will be executed
instead. Here you have a patch that I have already tested to check if
it works, and it worked.

Comment 1

[Vicente Olivert Riera]

Attachment: libnss-fix-cross-compilation.patch

This patch lets the user set the $CPU_ARCH when cross-compiling.

Currently $CPU_ARCH variable comes from $OS_TEST (see mozilla/security/coreconf/Linux.mk for details) which comes from `uname -m` (see mozilla/security/coreconf/arch.mk for details). So, when you are cross-compiling the result of `uname -m` is the architecture of the host machine, not the target machine.

With this patch you could call make like this:

make TARGET_ARCH=<your target arch here>

, so CPU_ARCH will have the right value for the target architecture you are cross-compiling for.

Comment 2

[Vicente Olivert Riera]

(In reply to Vicente Olivert Riera from comment #1)
> With this patch you could call make like this:
> 
> make TARGET_ARCH=<your target arch here>

s/TARGET_ARCH/CROSS_ARCH/

Comment 3

[Robert Relyea]

wan-teh, does this look good to you?

Comment 4

[Wan-Teh Chang]

Comment on attachment 8423003 [details] [diff] [review]
libnss-fix-mips-n32.patch

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

Vicente: are you building NSS with USE_64=1? If so, I
think that's the problem. MIPS64 n32 should be treated
as a 32-bit build by NSS, similar to how NSS treats x32.
See the USE_X32 build variable for x32.

Comment 5

[Wan-Teh Chang]

Comment on attachment 8423759 [details] [diff] [review]
libnss-fix-cross-compilation.patch

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

Vicente: Thank you for the patch.

(In reply to Vicente Olivert Riera from comment #1)
>
> With this patch you could call make like this:
> 
> make CROSS_ARCH=<your target arch here>
> 
> , so CPU_ARCH will have the right value for the target architecture you are
> cross-compiling for.

You can just call make like this:

  make CPU_ARCH=<your target arch here>

This is how Mozilla Firefox cross-compiles NSS. See
http://mxr.mozilla.org/mozilla-central/source/security/build/Makefile.in#195

Comment 6

[Vicente Olivert Riera]

(In reply to Wan-Teh Chang from comment #4)
> Vicente: are you building NSS with USE_64=1? If so, I
> think that's the problem. MIPS64 n32 should be treated
> as a 32-bit build by NSS, similar to how NSS treats x32.
> See the USE_X32 build variable for x32.

Why? MIPS64 n32 is 64-bits. It only has 32-bit pointer and 32-bit long integer.

Comment 7

[Vicente Olivert Riera]

(In reply to Wan-Teh Chang from comment #5)
> You can just call make like this:
> 
>   make CPU_ARCH=<your target arch here>
> 
> This is how Mozilla Firefox cross-compiles NSS. See
> http://mxr.mozilla.org/mozilla-central/source/security/build/Makefile.in#195

Do you mean "make TARGET_CPU=<your target arch here>"?

Comment 8

[Vicente Olivert Riera]

(In reply to Vicente Olivert Riera from comment #7)
> Do you mean "make TARGET_CPU=<your target arch here>"?

Forget about this comment.

Comment 9

[Wan-Teh Chang]

(In reply to Vicente Olivert Riera from comment #6)
>
> Why? MIPS64 n32 is 64-bits. It only has 32-bit pointer and 32-bit long
> integer.

In the NSS build system, USE_64=1 means pointers are 64 bits, and with
the exception of WIN64, long integers are 64 bits (the LP64 data model).

So, both x32 and MIPS64 n32 are considered 32-bit builds by the NSS
build system.

Comment 10

[Vicente Olivert Riera]

(In reply to Wan-Teh Chang from comment #9)
> (In reply to Vicente Olivert Riera from comment #6)
> >
> > Why? MIPS64 n32 is 64-bits. It only has 32-bit pointer and 32-bit long
> > integer.
> 
> In the NSS build system, USE_64=1 means pointers are 64 bits, and with
> the exception of WIN64, long integers are 64 bits (the LP64 data model).
> 
> So, both x32 and MIPS64 n32 are considered 32-bit builds by the NSS
> build system.

Then, what's the purpose of NS_PTR_LE_32 in "mozilla/security/nss/lib/freebl/drbg.c" on the line #488?

There is a comment below that line which includes this text:

"The 90% case (perhaps 100% case), size_t is less than or equal to 32 bits if the platform is not 64 bits, and greater than 32 bits if it is a 64 bit platform. The corner cases are handled with explicit defines NS_PTR_GT_32 and NS_PTR_LE_32."

MIPS64 n32 is one of those corner cases, it's a 64-bit platform with 32-bit pointers, so we can handle this corner case setting NS_PTR_LE_32.

Comment 11

[Wan-Teh Chang]

(In reply to Vicente Olivert Riera from comment #10)
> 
> MIPS64 n32 is one of those corner cases, it's a 64-bit platform with 32-bit
> pointers, so we can handle this corner case setting NS_PTR_LE_32.

I understand MIPS64 is a 64-bit processor architecture. But to
the C code in NSS and the NSS build system, a 64-bit platform
is one that has 64-bit pointers. So, in that context, MIPS64 n32
is a 32-bit platform, so USE_64=1 should not be set. Instead,
you can add a build variable USE_N32=1 to specify the MIPS64 n32
build.

A corner case referred to in that comment is a build where
pointers are 64-bit but size_t is 32-bit. The MIPS64 n32 build
is not such a corner case. I think that comment should be rewritten
as follows:

  The 90% case (perhaps 100% case), size_t is less than or equal to 32 bits
  if pointers are not 64 bits, and greater than 32 bits if pointers are 64
  bits. The corner cases are handled with explicit defines NS_PTR_GT_32 and
  NS_PTR_LE_32.

Comment 12

[Vicente Olivert Riera]

(In reply to Wan-Teh Chang from comment #11)
> (In reply to Vicente Olivert Riera from comment #10)
> > 
> > MIPS64 n32 is one of those corner cases, it's a 64-bit platform with 32-bit
> > pointers, so we can handle this corner case setting NS_PTR_LE_32.
> 
> I understand MIPS64 is a 64-bit processor architecture. But to
> the C code in NSS and the NSS build system, a 64-bit platform
> is one that has 64-bit pointers. So, in that context, MIPS64 n32
> is a 32-bit platform, so USE_64=1 should not be set. Instead,
> you can add a build variable USE_N32=1 to specify the MIPS64 n32
> build.
> 
> A corner case referred to in that comment is a build where
> pointers are 64-bit but size_t is 32-bit. The MIPS64 n32 build
> is not such a corner case. I think that comment should be rewritten
> as follows:
> 
>   The 90% case (perhaps 100% case), size_t is less than or equal to 32 bits
>   if pointers are not 64 bits, and greater than 32 bits if pointers are 64
>   bits. The corner cases are handled with explicit defines NS_PTR_GT_32 and
>   NS_PTR_LE_32.

Ok, understood now. You can close this bug when you consider.

"Xie xie ni" for your help :-)

Comment 13

[Wan-Teh Chang]

Vicente: De nada :-)

Do you need any change to nss/coreconf/Linux.mk to set the appropriate
compiler flags for MIPS64 n32 if USE_N32=1 is set?

Comment 14

[Vicente Olivert Riera]

(In reply to Wan-Teh Chang from comment #13)
> Vicente: De nada :-)
> 
> Do you need any change to nss/coreconf/Linux.mk to set the appropriate
> compiler flags for MIPS64 n32 if USE_N32=1 is set?

I don't think so. If someone complains we will have a look at it. In the meantime just leave it as is.

Comment 15

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last months and this bug has priority 'P2'.
:beurdouche, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 16

[Marco Castelluccio [:marco]]

We have modified the bot to only consider P1 as high priority, so I'm cancelling the needinfo here.