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

RESOLVED FIXED in Firefox 50

Status

defect
RESOLVED FIXED
3 years ago
Last year

People

(Reporter: glandium, Assigned: glandium)

Tracking

(Blocks 1 bug)

unspecified
mozilla50
Dependency tree / graph

Firefox Tracking Flags

(firefox50 fixed)

Details

Attachments

(4 attachments)

No description provided.
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/
Attachment #8773143 - Flags: review?(cmanchester)
Attachment #8773144 - Flags: review?(cmanchester)
Attachment #8773145 - Flags: review?(cmanchester)
Attachment #8773146 - Flags: review?(cmanchester)
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 on attachment 8773143 [details]
Bug 1288313 - Explicitly reject unknown CPU types.

https://reviewboard.mozilla.org/r/65858/#review63064
Attachment #8773143 - Flags: review?(cmanchester) → review+
Attachment #8773144 - Flags: review?(cmanchester) → review+
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 on attachment 8773145 [details]
Bug 1288313 - Ensure the host and target compilers build for the right kernel.

https://reviewboard.mozilla.org/r/65862/#review63244
Attachment #8773145 - Flags: review?(cmanchester) → review+
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
Attachment #8773146 - Flags: review?(cmanchester) → review+
Pushed by mh@glandium.org:
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
Depends on: 1289946
Duplicate of this bug: 977817
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.
Or -- adding "#undef WINNT" to the start of the preprocessed blob in toolchain.configure as in https://github.com/vvuk/mozjs/commit/6dfd03f81affb1e501d1cbdc2713f385bf272dd4 works.
(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.
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.