1273764 - Uses wrong compiler definitions to check for sparc64 architecture

Status: RESOLVED DUPLICATE of bug 1275204

(Firefox Build System :: General, defect)

Description

[John Paul Adrian Glaubitz]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:46.0) Gecko/20100101 Firefox/46.0
Build ID: 20160508001106

Steps to reproduce:

Hello!

Firefox currently fails to build on Linux with gcc on sparc64 because the compiler definitions used to detect the sparc64 architecture aren't correct.

The first problem is the embedded version of protobuf which will define "GOOGLE_PROTOBUF_ARCH_64_BIT 1" only if "SOLARIS_64BIT_ENABLED" is defined as well which is obviously not the case on a Linux machine. This can be fixed by:

--- firefox-46.0.1.orig/toolkit/components/protobuf/src/google/protobuf/stubs/platform_macros.h
+++ firefox-46.0.1/toolkit/components/protobuf/src/google/protobuf/stubs/platform_macros.h
@@ -67,7 +67,7 @@
 #define GOOGLE_PROTOBUF_ARCH_32_BIT 1
 #elif defined(sparc)
 #define GOOGLE_PROTOBUF_ARCH_SPARC 1
-#ifdef SOLARIS_64BIT_ENABLED
+#if defined(SOLARIS_64BIT_ENABLED) || defined(__sparc64__) && defined(__arch64__)
 #define GOOGLE_PROTOBUF_ARCH_64_BIT 1
 #else
 #define GOOGLE_PROTOBUF_ARCH_32_BIT 1

Secondly, ipc/chromium has the same issue:

--- firefox-46.0.1.orig/ipc/chromium/src/build/build_config.h
+++ firefox-46.0.1/ipc/chromium/src/build/build_config.h
@@ -81,7 +81,7 @@
 #elif defined(__ppc__) || defined(__powerpc__)
 #define ARCH_CPU_PPC 1
 #define ARCH_CPU_32_BITS 1
-#elif defined(__sparc64__)
+#elif defined(__sparc__) && defined(__arch64__)
 #define ARCH_CPU_SPARC 1
 #define ARCH_CPU_64_BITS 1
 #elif defined(__sparc__)

For reference, this is what gcc (5.3.1) defines on Linux/sparc64:


root@landau:~# dpkg --print-architecture
sparc64
root@landau:~# echo | gcc -E -dM - |grep sparc64
root@landau:~#

Instead, we need to check for both __sparc__ and __arch64__:

root@landau:~# echo | gcc -E -dM - |grep __sparc__
#define __sparc__ 1
root@landau:~# echo | gcc -E -dM - |grep __arch64__
#define __arch64__ 1
root@landau:~#

Please see the corresponding Debian bug report for more:

> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=824449

After applying these two fixes, the build comes as far as outlined in #953211:

> https://bugzilla.mozilla.org/show_bug.cgi?id=953211

failing with "Error: Error while running startup cache precompilation".

If anyone who wants to fix this issue needs access to sparc64 hardware, please let me know. I can create accounts on a fast SPARC-T5 server (192 threads, 192 GiB RAM) which will allow building Firefox in a reasonable amount of time. These machines are running Debian unstable with a current set of tools (gcc, binutils, emacs etc).

Thanks,
Adrian

Comment 1

[Mike Hommey [:glandium]]

(Moving to Build Config short of a better component for these kinds of things)

Please consult e.g. https://developer.mozilla.org/en-US/docs/Mercurial/Using_Mercurial#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F for how to prepare patch(es) for review here, and https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch#Submitting_the_patch for how to submit it.

(In reply to glaubitz from comment #0)
> +#if defined(SOLARIS_64BIT_ENABLED) || defined(__sparc64__) &&
> defined(__arch64__)

This has to be wrong, considering __sparc64__, as you note, is not defined.

Also, this was fixed in upstream protobuf in a different way:
https://github.com/google/protobuf/commit/97fa4ca1565d216d102af9510b17966c28c7a52a

Please note that modifications to protobuf should be accompanied with a patch in toolkit/components/protobuf and according modifications to toolkit/components/protobuf/upgrade_protobuf.sh.

> --- firefox-46.0.1.orig/ipc/chromium/src/build/build_config.h
> +++ firefox-46.0.1/ipc/chromium/src/build/build_config.h
> @@ -81,7 +81,7 @@
>  #elif defined(__ppc__) || defined(__powerpc__)
>  #define ARCH_CPU_PPC 1
>  #define ARCH_CPU_32_BITS 1
> -#elif defined(__sparc64__)
> +#elif defined(__sparc__) && defined(__arch64__)
>  #define ARCH_CPU_SPARC 1
>  #define ARCH_CPU_64_BITS 1
>  #elif defined(__sparc__)

There is another copy in media/webrtc/trunk/build/build_config.h that will need similar modifications.

Comment 2

[John Paul Adrian Glaubitz]

Interestingly, on *BSD, gcc actually defines __sparc64__ [1,2,3], which is why using this define works there. I'm not sure about __arch64__ though, I unfortunately don't have a *BSD running on sparc64 to verify this. Maybe one of the NetBSD porters in this bug tracker could comment on this.

If *BSD define __sparc__ and __arch64__ on sparc64 as well, it might be better to just use that if it's more portable, i.e. test for "defined(__sparc__) && defined(__arch64__)".

Adrian

> [1] https://github.com/gcc-mirror/gcc/blob/master/gcc/config/sparc/freebsd.h
> [2] https://github.com/gcc-mirror/gcc/blob/master/gcc/config/sparc/netbsd-elf.h
> [3] https://github.com/gcc-mirror/gcc/blob/master/gcc/config/sparc/openbsd64.h

Comment 3

[Martin Husemann]

Indeed (and I keep forgetting that): __sparc64__ is a BSD'ism.

The canonical way explicitly promoted by the pkgsrc team is:

#if defined(__sparc__) && defined(_LP64)

but __arch64__ instead of _LP64 should work as well.

Comment 4

[John Paul Adrian Glaubitz]

Upstream protobuf has changed the SOLARIS_64BIT_ENABLED line like this:

#define GOOGLE_PROTOBUF_ARCH_32_BIT 1
 #elif defined(sparc)
 #define GOOGLE_PROTOBUF_ARCH_SPARC 1
-#ifdef SOLARIS_64BIT_ENABLED
+#if defined(__sparc_v9__) || defined(__sparcv9) || defined(__arch64__)
 #define GOOGLE_PROTOBUF_ARCH_64_BIT 1
 #else
 #define GOOGLE_PROTOBUF_ARCH_32_BIT 1

so I suggest just using that.

And for ipc/chromium/src/build_config.h I suggest the change mentioned above:

 #elif defined(__ppc__) || defined(__powerpc__)
 #define ARCH_CPU_PPC 1
 #define ARCH_CPU_32_BITS 1
-#elif defined(__sparc64__)
+#elif defined(__sparc__) && defined(__arch64__)
 #define ARCH_CPU_SPARC 1
 #define ARCH_CPU_64_BITS 1
 #elif defined(__sparc__)

I will send a patch in a bit.