Closed Bug 1590624 Opened 2 years ago Closed 2 years ago

Update mingw-clang build to clang 9

Categories

(Firefox Build System :: General: Unsupported Platforms, enhancement)

Product:
Firefox Build System
Component:
General: Unsupported Platforms
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(firefox73 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla73
Tracking Flags:
Tracking Status
firefox73 --- fixed

People

(Reporter: tjr, Assigned: tjr)

References

(Blocks 1 open bug)

Details

Attachments

(4 files)

Bug 1590624 - Update the mingw-clang compiler to clang 9 r?dmajor
47 bytes, text/x-phabricator-request
Details | Review
Bug 1590624 - Use windows-style stl_wrappers for mingw build r?dmajor
47 bytes, text/x-phabricator-request
Details | Review
Bug 1590624 - mingw-clang cannot use _xgetbv without -mavx, work around it r?lsalzman
47 bytes, text/x-phabricator-request
Details | Review
Bug 1590624 - Define a __imp_ alias for _strdup for mingw-clang r?dmajor
47 bytes, text/x-phabricator-request
Details | Review

(This is for -central only, not to be backported to esr68)

To match the rest of our builds, we'd like to update to clang 9.

Bug 1548624 may still be valid, unsure.

Attached is a WIP patch. It fails like so:

In file included from Unified_cpp_gfx_graphite2_src0.cpp:2:
In file included from /builds/worker/workspace/build/src/gfx/graphite2/src/CmapCache.cpp:28:
In file included from /builds/worker/workspace/build/src/gfx/graphite2/src/inc/Main.h:29:
In file included from /builds/worker/workspace/build/src/obj-firefox/dist/stl_wrappers/cstdlib:44:
In file included from /builds/worker/fetches/clang/bin/../x86_64-w64-mingw32/include/c++/v1/cstdlib:85:
In file included from /builds/worker/fetches/clang/bin/../x86_64-w64-mingw32/include/c++/v1/stdlib.h:100:
In file included from /builds/worker/fetches/clang/bin/../x86_64-w64-mingw32/include/c++/v1/math.h:311:
In file included from /builds/worker/workspace/build/src/obj-firefox/dist/stl_wrappers/type_traits:50:
In file included from /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/mozalloc.h:31:
In file included from /builds/worker/workspace/build/src/obj-firefox/dist/stl_wrappers/new:44:
In file included from /builds/worker/fetches/clang/bin/../x86_64-w64-mingw32/include/c++/v1/new:90:
/builds/worker/fetches/clang/bin/../x86_64-w64-mingw32/include/c++/v1/exception:180:5: error: no member named 'abort' in namespace 'std::__1'; did you mean simply 'abort'?
_VSTD::abort();
^~~~~~~
/builds/worker/fetches/clang/bin/../x86_64-w64-mingw32/include/c++/v1/__config:793:15: note: expanded from macro '_VSTD'
#define _VSTD std::_LIBCPP_ABI_NAMESPACE
^
/builds/worker/fetches/clang/bin/../x86_64-w64-mingw32/include/stdlib.h:402:40: note: 'abort' declared here
void __cdecl __MINGW_ATTRIB_NORETURN abort(void);
^
1 error generated.

https://treeherder.mozilla.org/#/jobs?repo=try&selectedJob=271809249&revision=4980be507dad1b1bbe8793b01a61dd49afb523c2

Here's what I think is happening.

gfx/graphite2/src/inc/Main.h asks for cstdlib which goes to our stl_wrappers/cstdlib (fine)
That pulls in the real cstdlib (fine)
The real cstdlib, on line 85, pulls in stdlib.h (fine)
Which pulls in math.h (fine)
Which pulls in type_traits (fine)
It gets our type_traits from stl_wrappers AND HERE'S WHERE IT GOES BAD.
Our type_traits pulls in the real type_traits (not pictured in stack) and THEN it pulls in mozalloc.h
mozalloc.h pulls in new which goes first to our stl_wrappers/new (fine) and then to the real new
The real new pulls in exception
exception references _VSTD::abort() which is undefined.

Why is it undefined? Because it's defined on line 129 of the real cstdlib. And we haven't reached that line yet. We went off the rails over on line 85 of the real cstdlib.

08:21:38 <jacekc> tjr: if I read the code right, here is already a code that should prevent type_traits from including mozalloc.h
08:22:44 <jacekc> cstdlib will define MOZ_INCLUDE_MOZALLOC_H and type_traits will check if it's defined and if it is, it should not include it (and leave it for cstdlib to include it later)
08:23:02 <jacekc> so the question is why it doesn't work...

Looked at this a little bit. From my preprocessor output and a diff between the windows type_traits and linux type_traints - I think we're using linux style. The moz.build likewise implies.

Sending in a try run making the mingw build create a windows-style stl_wrappers and see what that does: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9efc8aaafda3941c9820059e0a886cfaea5b6650

Attachment #9103426 - Attachment description: Bug 1590624 - Try bumping mingwclang to clang 9 → Bug 1590624 - Update the mingw-clang compiler to clang 9 r?dmajor

Turns out we were using the Linux ones. This uses the Windows ones, and
adds _HAS_EXCEPTIONS to the mingw defines so the stl_wrappers behave
correctly.

Depends on D50152

We break them out of Unified builds and then correct a couple random unified
build errors.

Depends on D54530

This is affected by the same problem detailed in the comment.

Depends on D54531

Attachment #9111261 - Attachment description: Bug 1590624 - mingw-clang needs to specify -mavx for a couple of files r?dmajor → Bug 1590624 - mingw-clang cannot use _xgetbv without -mavx, work around it r?lsalzman
Pushed by tritter@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d305e4160ee4
Update the mingw-clang compiler to clang 9 r=dmajor
https://hg.mozilla.org/integration/autoland/rev/2b28154aacdb
Use windows-style stl_wrappers for mingw build r=glandium
https://hg.mozilla.org/integration/autoland/rev/8432398c212a
mingw-clang cannot use _xgetbv without -mavx, work around it r=lsalzman,froydnj
https://hg.mozilla.org/integration/autoland/rev/2ac8dd1d709d
Define a __imp_ alias for _strdup for mingw-clang r=dmajor
Pushed by tritter@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/02a71c2df35c
Update the mingw-clang compiler to clang 9 r=dmajor
https://hg.mozilla.org/integration/autoland/rev/79bf2b523d94
Use windows-style stl_wrappers for mingw build r=glandium
https://hg.mozilla.org/integration/autoland/rev/df32eb1fdc8d
mingw-clang cannot use _xgetbv without -mavx, work around it r=lsalzman,froydnj
https://hg.mozilla.org/integration/autoland/rev/c2cffffd5ca6
Define a __imp_ alias for _strdup for mingw-clang r=dmajor
Blocks: 1609780
You need to log in before you can comment on or make changes to this bug.