570334 - security/coreconf/WIN32.mk doesn't support x86_64 CPU_ARCH.

Status: RESOLVED FIXED

(NSS :: Build, defect)

Description

[Jacek Caban]

Attachment: fix v1.0

WIN32.mk assumes that CPU_ARCH is x386 for AMD64, but in case of mingw-w64 it's x86_64. The attached patch fixes it.

Comment 1

[Wan-Teh Chang]

Comment on attachment 449466 [details] [diff] [review]
fix v1.0

Thanks for the patch.  We may need to check the USE_64 variable as well,
because it's possible to do 32-bit x86 builds on a x86_64 machine.

So we probably should do something like this:

ifeq (,$(filter-out x386 x86_64,$(CPU_ARCH)))
ifdef USE_64
         DEFINES += -D_AMD64_
else
        DEFINES += -D_X86_
endif
endif

Comment 2

[Jacek Caban]

Attachment: fix v1.1

Thanks for the review. I was assuming that CPU_ARCH should be set to target CPU, 32-bit build on x86_64 is crosscompiling (so it would be set to x386) and MSVC is a special case, where x368 is set for x86_64.

Your fix obviously works (tested).

Comment 3

[Wan-Teh Chang]

Comment on attachment 450851 [details] [diff] [review]
fix v1.1

r=wtc.

It's unfortunate that the meaning of CPU_ARCH in
NSS's coreconf build system is different between platforms.

For example, on Linux, CPU_ARCH means the target CPU
architecture.  See
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/coreconf/Linux.mk&rev=1.44&mark=65-67,70#65
where we set CPU_ARCH to x86 if USE_64 is not defined when
we're building on an x86_64 machine.

We should clean this up so that the meaning of CPU_ARCH
is consistent across platforms.  This patch is fine for
now -- in NSS we usually test CPU_ARCH for a particular OS.
As this search query shows:
http://mxr.mozilla.org/security/search?string=$(CPU_ARCH)
it seems that the only places where we test CPU_ARCH for all
OS's are in lib/freebl/Makefile and lib/jar/config.mk:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/nss/lib/freebl/Makefile&rev=1.112&mark=83,85#83
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/nss/lib/jar/config.mk&rev=1.4&mark=51,53#51
and we also test USE_64 there.

Comment 4

[Jacek Caban]

Thank you for the review and explanation.

Comment 5

[Wan-Teh Chang]

I checked in the patch on the NSS trunk (NSS 3.12.7).

Checking in WIN32.mk;
/cvsroot/mozilla/security/coreconf/WIN32.mk,v  <--  WIN32.mk
new revision: 1.39; previous revision: 1.38
done