1288313 - Ensure the host and target compilers build for the right CPU, kernel and endianness

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Comment 1

[Mike Hommey [:glandium]]

Attachment: Bug 1288313 - Explicitly reject unknown CPU types.

Since bug 1264482, unknown CPU types end up triggering a ValueError
exception because of the CPU EnumString. Even if somehow the CPU is
valid, the endianness is not and would trigger a ValueError exception
as well.

So, instead of letting the exceptions happen, use a nicer failure mode
with an explicit die().

Review commit: https://reviewboard.mozilla.org/r/65858/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/65858/

Comment 2

[Mike Hommey [:glandium]]

Attachment: Bug 1288313 - Ensure the host and target compilers build for the right CPU.

And for GCC and clang, try to see if adding -m32, -m64 or --target
works if they don't.

Review commit: https://reviewboard.mozilla.org/r/65860/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/65860/

Comment 3

[Mike Hommey [:glandium]]

Attachment: Bug 1288313 - Ensure the host and target compilers build for the right kernel.

Review commit: https://reviewboard.mozilla.org/r/65862/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/65862/

Comment 4

[Mike Hommey [:glandium]]

Attachment: Bug 1288313 - Ensure the host and target compilers build for the right endianness.

Review commit: https://reviewboard.mozilla.org/r/65864/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/65864/

Comment 5

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8773143 [details]
Bug 1288313 - Explicitly reject unknown CPU types.

https://reviewboard.mozilla.org/r/65858/#review63064

Comment 6

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8773144 [details]
Bug 1288313 - Ensure the host and target compilers build for the right CPU.

https://reviewboard.mozilla.org/r/65860/#review63238

Comment 7

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8773145 [details]
Bug 1288313 - Ensure the host and target compilers build for the right kernel.

https://reviewboard.mozilla.org/r/65862/#review63244

Comment 8

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8773146 [details]
Bug 1288313 - Ensure the host and target compilers build for the right endianness.

https://reviewboard.mozilla.org/r/65864/#review63248

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d4d67c4aa318
Explicitly reject unknown CPU types. r=chmanchester
https://hg.mozilla.org/integration/autoland/rev/f0889768e16d
Ensure the host and target compilers build for the right CPU. r=chmanchester
https://hg.mozilla.org/integration/autoland/rev/ba8cf05d0ee7
Ensure the host and target compilers build for the right kernel. r=chmanchester
https://hg.mozilla.org/integration/autoland/rev/51fe63c13c4c
Ensure the host and target compilers build for the right endianness. r=chmanchester

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d4d67c4aa318
https://hg.mozilla.org/mozilla-central/rev/f0889768e16d
https://hg.mozilla.org/mozilla-central/rev/ba8cf05d0ee7
https://hg.mozilla.org/mozilla-central/rev/51fe63c13c4c

Comment 12

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

kernel_preprocessor_checks has:

    'WINNT': '_WIN32 || __CYGWIN__',

which causes the generated file to contain:

    #elif _WIN32 || __CYGWIN__
    %KERNEL WINNT

unfortunately, "WINNT" is defined by the mingw gcc as 1:

vladimir@glacier[5205]$ gcc -x c -E -dM /dev/null | grep WINNT
#define __WINNT 1
#define __WINNT__ 1
#define WINNT 1

thus resulting in a KERNEL = 1, which then breaks.  I agree that this is kind of stupid for gcc:

vladimir@glacier[5206]$ gcc -x c -E -dM /dev/null | grep -v 'define _'
#define WIN32 1
#define WIN64 1
#define WINNT 1

clang on the other hand is less stupid, but still has a non-_ standard define:

vladimir@glacier[5207]$ clang -x c -E -dM /dev/null | grep -v 'define _'
#define WIN64 1

either way, we probably shouldn't use "WINNT" as the kernel name to avoid this.

Comment 13

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Or -- adding "#undef WINNT" to the start of the preprocessed blob in toolchain.configure as in https://github.com/vvuk/mozjs/commit/6dfd03f81affb1e501d1cbdc2713f385bf272dd4 works.

Comment 14

[Mike Hommey [:glandium]]

(In reply to Vladimir Vukicevic [:vlad] [:vladv] from comment #12)
> kernel_preprocessor_checks has:
> 
>     'WINNT': '_WIN32 || __CYGWIN__',
> 
> which causes the generated file to contain:
> 
>     #elif _WIN32 || __CYGWIN__
>     %KERNEL WINNT
> 
> unfortunately, "WINNT" is defined by the mingw gcc as 1:
> 
> vladimir@glacier[5205]$ gcc -x c -E -dM /dev/null | grep WINNT
> #define __WINNT 1
> #define __WINNT__ 1
> #define WINNT 1
> 
> thus resulting in a KERNEL = 1, which then breaks.  I agree that this is
> kind of stupid for gcc:
> 
> vladimir@glacier[5206]$ gcc -x c -E -dM /dev/null | grep -v 'define _'
> #define WIN32 1
> #define WIN64 1
> #define WINNT 1
> 
> clang on the other hand is less stupid, but still has a non-_ standard
> define:
> 
> vladimir@glacier[5207]$ clang -x c -E -dM /dev/null | grep -v 'define _'
> #define WIN64 1
> 
> either way, we probably shouldn't use "WINNT" as the kernel name to avoid
> this.

You're describing bug 1289946.