1390583 - Stylo: Build Broken with MinGW

Status: RESOLVED FIXED

(Firefox Build System :: General, defect, P1)

Description

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

> obj-mingw/dist/include/mozilla/Char16.h:39:3: error: static_assert failed "char16_t and wchar_t sizes differ", err: true
> obj-mingw/dist/include/mozilla/mozalloc_abort.h:21:1: error: unknown type name '__declspec', err: true
> obj-mingw/dist/include/mozilla/mozalloc_abort.h:23:15: error: expected ';' after top level declarator, err: true
> obj-mingw/dist/include/mozilla/mozalloc.h:85:1: error: unknown type name '__declspec', err: true
> obj-mingw/dist/include/mozilla/mozalloc.h:85:9: error: expected ';' after top level declarator, err: true
> obj-mingw/dist/include/mozilla/mozalloc.h:88:1: error: unknown type name '__declspec', err: true
> obj-mingw/dist/include/mozilla/mozalloc.h:88:9: error: expected ';' after top level declarator, err: true
> obj-mingw/dist/include/mozilla/mozalloc.h:91:1: error: unknown type name '__declspec', err: true
> obj-mingw/dist/include/mozilla/mozalloc.h:91:9: error: expected ';' after top level declarator, err: true
> obj-mingw/dist/include/mozilla/mozalloc.h:94:1: error: unknown type name '__declspec', err: true
> obj-mingw/dist/include/mozilla/mozalloc.h:94:9: error: expected ';' after top level declarator, err: true
> obj-mingw/dist/include/mozilla/mozalloc.h:97:1: error: unknown type name '__declspec', err: true
> obj-mingw/dist/include/mozilla/mozalloc.h:97:9: error: expected ';' after top level declarator, err: true
> obj-mingw/dist/include/mozilla/mozalloc.h:99:1: error: unknown type name '__declspec', err: true
> obj-mingw/dist/include/mozilla/mozalloc.h:99:9: error: expected ';' after top level declarator, err: true
> obj-mingw/dist/include/mozilla/Assertions.h:33:8: error: unknown type name '__declspec', err: true
> obj-mingw/dist/include/mozilla/Assertions.h:33:17: error: expected ';' after top level declarator, err: true
> obj-mingw/dist/include/mozilla/Assertions.h:287:1: error: unknown type name '__declspec', err: true
> obj-mingw/dist/include/mozilla/Assertions.h:287:48: error: expected ';' after top level declarator, err: true

Comment 1

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Attachment: Bug 1390583 Patch to illustrate root of problem

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

Comment 2

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

I screwed around with this a bunch.

Editing ServoBindings.toml to pass "-DOS_WIN=1", "-DWIN32=1" and more importantly --target fixed the above errors

I got a series of missing header file errors. The correct way to fix this is something with sysroot or something similar, but --sysroot wasn't recognized, so I hacked together a fix that just hardcoded a bunch of include directories

Then I got errors about unknown type name 'char16_t'. Adding "-std=c++11", "-fno-ms-compatibility" fixed those.

Then I got to a hard blocker:

> /usr/local/lib/gcc/i686-w64-mingw32/5.1.0/include/ia32intrin.h:41:10: error: use of undeclared identifier '__builtin_ia32_bsrsi', err: true
> /usr/local/lib/gcc/i686-w64-mingw32/5.1.0/include/ia32intrin.h:104:1: error: definition of builtin function '__rdtsc', err: true
> /usr/local/lib/gcc/i686-w64-mingw32/5.1.0/include/ia32intrin.h:122:10: error: use of undeclared identifier '__builtin_ia32_rolqi', err: true
> /usr/local/lib/gcc/i686-w64-mingw32/5.1.0/include/ia32intrin.h:130:10: error: use of undeclared identifier '__builtin_ia32_rolhi', err: true
> /usr/local/lib/gcc/i686-w64-mingw32/5.1.0/include/ia32intrin.h:146:10: error: use of undeclared identifier '__builtin_ia32_rorqi', err: true
> /usr/local/lib/gcc/i686-w64-mingw32/5.1.0/include/ia32intrin.h:154:10: error: use of undeclared identifier '__builtin_ia32_rorhi', err: true
> /usr/local/lib/gcc/i686-w64-mingw32/5.1.0/include/xmmintrin.h:127:19: error: use of undeclared identifier '__builtin_ia32_addss', err: true
> /usr/local/lib/gcc/i686-w64-mingw32/5.1.0/include/xmmintrin.h:133:19: error: use of undeclared identifier '__builtin_ia32_subss', err: true
> /usr/local/lib/gcc/i686-w64-mingw32/5.1.0/include/xmmintrin.h:139:19: error: use of undeclared identifier '__builtin_ia32_mulss', err: true
> /usr/local/lib/gcc/i686-w64-mingw32/5.1.0/include/xmmintrin.h:145:19: error: use of undeclared identifier '__builtin_ia32_divss', err: true
> /usr/local/lib/gcc/i686-w64-mingw32/5.1.0/include/xmmintrin.h:239:10: error: use of undeclared identifier '__builtin_ia32_andps', err: true
> /usr/local/lib/gcc/i686-w64-mingw32/5.1.0/include/xmmintrin.h:245:10: error: use of undeclared identifier '__builtin_ia32_andnps', err: true
> /usr/local/lib/gcc/i686-w64-mingw32/5.1.0/include/xmmintrin.h:251:10: error: use of undeclared identifier '__builtin_ia32_orps', err: true
> /usr/local/lib/gcc/i686-w64-mingw32/5.1.0/include/xmmintrin.h:257:10: error: use of undeclared identifier '__builtin_ia32_xorps', err: true
> /usr/local/lib/gcc/i686-w64-mingw32/5.1.0/include/xmmintrin.h:285:19: error: use of undeclared identifier '__builtin_ia32_movss', err: true
> /usr/local/lib/gcc/i686-w64-mingw32/5.1.0/include/xmmintrin.h:295:19: error: use of undeclared identifier '__builtin_ia32_movss', err: true
> /usr/local/lib/gcc/i686-w64-mingw32/5.1.0/include/xmmintrin.h:323:19: error: use of undeclared identifier '__builtin_ia32_movss', err: true
> /usr/local/lib/gcc/i686-w64-mingw32/5.1.0/include/xmmintrin.h:333:19: error: use of undeclared identifier '__builtin_ia32_movss', err: true
> /usr/local/lib/gcc/i686-w64-mingw32/5.1.0/include/xmmintrin.h:377:19: error: use of undeclared identifier '__builtin_ia32_cmpgtps', err: true

My interpretation of these errors is that clang is including gcc headers which expect these __builtin functions to be available and provided by the compiler. clang does not provide them; it implements "an excellent set of native vector operations".

So the problem seems to be that it is not possible to build with clang using gcc headers. But what is the solution?  Disable Stylo? (How long will that work?) There was that super-experimental mingw-clang thing...? 


Resources:
http://clang.llvm.org/compatibility.html#vector_builtins
http://clang-developers.42468.n3.nabble.com/error-use-of-undeclared-identifier-builtin-ia32-addss-while-building-clang-using-clang-tp2585737p2585814.html

Comment 3

[Xidorn Quan [:xidorn] UTC+11]

Comment on attachment 8897540 [details]
Bug 1390583 Patch to illustrate root of problem

https://reviewboard.mozilla.org/r/168800/#review174742

::: layout/style/ServoBindings.toml:44
(Diff revision 1)
>  ]
>  "arch=x86" = ["--target=i686-pc-win32"]
>  "arch=x86_64" = ["--target=x86_64-pc-win32"]
>  
> +# This catches the MinGW build
> +[build."os=windows"]

This would affect msvc build as well... You should add an `env=something` here, probably `env=gnu`.

The common arguments like `-DOS_WIN=1`, `-DWIN32=1` can probably be moved to a separate section with `[build."os=windows"]` so that we don't need to state them twice.

::: layout/style/ServoBindings.toml:49
(Diff revision 1)
> +    "-I/usr/local/i686-w64-mingw32/include/c++/5.1.0/",
> +    "-I/usr/local/lib/gcc/i686-w64-mingw32/5.1.0/include/",
> +    "-I/usr/local/i686-w64-mingw32/include/c++/5.1.0/i686-w64-mingw32/",
> +    "-I/usr/local/i686-w64-mingw32/include/",

These probably should be handled inside clang-sys or bindgen somehow, not here...

Comment 4

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #3)
> Comment on attachment 8897540 [details]
> Bug 1390583 Patch to illustrate root of problem
> 
> https://reviewboard.mozilla.org/r/168800/#review174742
> 
> ::: layout/style/ServoBindings.toml:44
> (Diff revision 1)
> >  ]
> >  "arch=x86" = ["--target=i686-pc-win32"]
> >  "arch=x86_64" = ["--target=x86_64-pc-win32"]
> >  
> > +# This catches the MinGW build
> > +[build."os=windows"]
> 
> This would affect msvc build as well... You should add an `env=something`
> here, probably `env=gnu`.
> 
> The common arguments like `-DOS_WIN=1`, `-DWIN32=1` can probably be moved to
> a separate section with `[build."os=windows"]` so that we don't need to
> state them twice.
> 
> ::: layout/style/ServoBindings.toml:49
> (Diff revision 1)
> > +    "-I/usr/local/i686-w64-mingw32/include/c++/5.1.0/",
> > +    "-I/usr/local/lib/gcc/i686-w64-mingw32/5.1.0/include/",
> > +    "-I/usr/local/i686-w64-mingw32/include/c++/5.1.0/i686-w64-mingw32/",
> > +    "-I/usr/local/i686-w64-mingw32/include/",
> 
> These probably should be handled inside clang-sys or bindgen somehow, not
> here...

So I think our path forward for this problem is to build Stylo with MinGW rather than trying to keep using clang. I don't believe we will ever be able to get this compiled with clang (but I would be happy to be proven wrong).

Comment 5

[Xidorn Quan [:xidorn] UTC+11]

Stylo uses bindgen to generate bindings for Rust, and bindgen relies on libclang to parse the headers. I don't think GCC provides API we need in bindgen to parse headers and generate bindings, does it?

I think the problem is that how can we workaround it via some kind of predefineds. Actually can we just ignore includes for MinGW and use those provided by clang directly in this case? What would happen in that case?

Comment 6

[Nathan Froyd [:froydnj]]

(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #5)
> Stylo uses bindgen to generate bindings for Rust, and bindgen relies on
> libclang to parse the headers. I don't think GCC provides API we need in
> bindgen to parse headers and generate bindings, does it?

It does not, and trying to add the necessary APIs to GCC would be a huge undertaking.

(In reply to Tom Ritter [:tjr] from comment #4)
> (In reply to Xidorn Quan [:xidorn] UTC+10 from comment #3)
> > ::: layout/style/ServoBindings.toml:49
> > (Diff revision 1)
> > > +    "-I/usr/local/i686-w64-mingw32/include/c++/5.1.0/",
> > > +    "-I/usr/local/lib/gcc/i686-w64-mingw32/5.1.0/include/",
> > > +    "-I/usr/local/i686-w64-mingw32/include/c++/5.1.0/i686-w64-mingw32/",
> > > +    "-I/usr/local/i686-w64-mingw32/include/",
> > 
> > These probably should be handled inside clang-sys or bindgen somehow, not
> > here...
> 
> So I think our path forward for this problem is to build Stylo with MinGW
> rather than trying to keep using clang. I don't believe we will ever be able
> to get this compiled with clang (but I would be happy to be proven wrong).

Couldn't we provide these via BINDGEN_CFLAGS or something?  Or have configure derive these on its own by examining the compiler's search paths, and set the appropriate variables to pass them in to bindgen?

Comment 7

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

(In reply to Nathan Froyd [:froydnj] from comment #6)
> Couldn't we provide these via BINDGEN_CFLAGS or something?  Or have
> configure derive these on its own by examining the compiler's search paths,
> and set the appropriate variables to pass them in to bindgen?

So I don't know as much about bindgen or clang or gcc as anyone else here but... it seems to me that even if we figured out the correct way to nicely get clang to find MinGW's header files - it still wouldn't work because clang can't use the mingw header files. (The two references I linked earlier talk about that, with the second one being what I think this exact problem is - trying to build with clang using gcc header files.)

Comment 8

[Nathan Froyd [:froydnj]]

I guess trying to make the mingw header files clang compatible would be a...enormous undertaking?  Ideally we'd only have __builtin_ things to deal with and those can't be too hard to deal with, but I'm sure there are other things lurking...

Comment 9

[Jacek Caban]

I'm sorry for so late reply. I was planning to experiment with it myself, but I failed to find the time and I already know I won't be able to do that for another two weeks :/

In general, using mingw headers (and crt) should be possible, but is known to be problematic. As far as I know the main issue is that clang has troubles handling files layout of mingw. However, people have used patched clang versions with mingw-w64 for years, see:
https://github.com/Alexpux/MINGW-packages/tree/master/mingw-w64-clang

There is an other attempt to integrate that:
https://github.com/martell/mingw-w64-clang
This one is interesting for two reasons. Martell slowly but successfully upstreams patches so AFAIK only a few more are needed. It also aims to provide full LLVM stack, including using lld (unlike typical mingw config, which uses GNU ld), which could bring better compatibility with msvc.

I even tried building Gecko with clang+mingw-w64 about 1.5 years ago and I could hack it to compile most of the tree (but linking was problematic back then). I don't remember if I patched clang itself, but I still have mozconfig I used and I can see that the only unusual part was:

export CC="/home/jacek/mingw-w64/llvm-build/bin/clang -fms-extensions --sysroot /usr/local/i686-w64-mingw32/ -target i686-w64-mingw32"
export CXX="/home/jacek/mingw-w64/llvm-build/bin/clang++ -fms-extensions --sysroot /usr/local/i686-w64-mingw32/ -target i686-w64-mingw32 -isystem /usr/local/i686-w64-mingw32/include/c++/5.1.0/ -isystem /usr/local/i686-w64-mingw32/include/c++/5.1.0/i686-w64-mingw32/ -Wno-incompatible-ms-struct"
export CPP="/home/jacek/mingw-w64/llvm-build/bin/clang -fms-extensions --sysroot /usr/local/i686-w64-mingw32/ -target i686-w64-mingw32 -E"



Back to solving this bug, patching clang is not really an option. However, I would expect even unpatched clang to be able to at least parse headers if we force it to do the right thing with some (perhaps sometimes hackish) arguments.

Tom, form your comment 2 it sounds like you were on the right path. The thing that was probably wrong was solving wchar_t/char16_t. The proper solution is probably something like -fms-compatibility-version=19

For the xmmintrin.h problem, clang is surely able to handle them in GCC mode, I'm not sure how is it different here. Where is it included from? Note that mingw's intrin.h includes it to workaround problems on GCC. Maybe we could change intrin.h for clang.

Comment 10

[Martin Storsjö]

(In reply to Jacek Caban from comment #9)
> 
> There is an other attempt to integrate that:
> https://github.com/martell/mingw-w64-clang
> This one is interesting for two reasons. Martell slowly but successfully
> upstreams patches so AFAIK only a few more are needed. It also aims to
> provide full LLVM stack, including using lld (unlike typical mingw config,
> which uses GNU ld), which could bring better compatibility with msvc.

FWIW, I'm working in parallel with Martell on completing this; by now everything essential for building C applications is upstreamed, and C++ code can be built (including exceptions, but is a little messy still). You can follow my progress at https://code.videolan.org/mstorsjo/docker-buildbots/blob/arm-toolchain/vlc-winrt/toolchain-arm/Dockerfile. This builds a full llvm+mingw environment from scratch as a cross compiler from linux, for targeting i686/x86_64/armv7/arm64. This builds a bunch of minimal demo applications plus a larger one at the end, to show off that the toolchain is working.

Comment 11

[Georg Koppen]

(In reply to Martin Storsjö from comment #10)
> (In reply to Jacek Caban from comment #9)
> > 
> > There is an other attempt to integrate that:
> > https://github.com/martell/mingw-w64-clang
> > This one is interesting for two reasons. Martell slowly but successfully
> > upstreams patches so AFAIK only a few more are needed. It also aims to
> > provide full LLVM stack, including using lld (unlike typical mingw config,
> > which uses GNU ld), which could bring better compatibility with msvc.
> 
> FWIW, I'm working in parallel with Martell on completing this; by now
> everything essential for building C applications is upstreamed, and C++ code
> can be built (including exceptions, but is a little messy still). You can
> follow my progress at
> https://code.videolan.org/mstorsjo/docker-buildbots/blob/arm-toolchain/vlc-
> winrt/toolchain-arm/Dockerfile. This builds a full llvm+mingw environment
> from scratch as a cross compiler from linux, for targeting
> i686/x86_64/armv7/arm64. This builds a bunch of minimal demo applications
> plus a larger one at the end, to show off that the toolchain is working.

That's interesting, thanks. In case no one beats me to it, I plan to pick this up. It won't happen in this month though and maybe November is optimistic as well. But I definitely want to look at it in December to have time to fix possible upcoming issues to have everything ready for ESR 59.

Comment 12

[Xidorn Quan [:xidorn] UTC+11]

Supposing we care about downstream browsers which want to use MinGW, this would need to block the removal of the old style system.

Comment 13

[Georg Koppen]

FWIW: The Google people got cross-compiling Chromium on Linux for Windows going recently: https://groups.google.com/a/chromium.org/forum/#!msg/chromium-dev/cIA9fBb9vBE/73cOvZu9BQAJ

That might be an interesting alternative. However, they are tracking LLVM trunk closely and I am not sure how well compiling Firefox works with that right now...

Comment 14

[Jacek Caban]

(In reply to Georg Koppen from comment #13)
> FWIW: The Google people got cross-compiling Chromium on Linux for Windows
> going recently:
> https://groups.google.com/a/chromium.org/forum/#!msg/chromium-dev/
> cIA9fBb9vBE/73cOvZu9BQAJ

It looks like they need a copy of MSVC for that. We can do better than that, Mozilla code is already compatible with mingw-w64 for headers and importlibs. It's just a matter integrating clang instead of gcc to have this config working.

I hope to try that soon, but it's been busy for me lately.

Comment 15

[(not currently active) Ted Mielczarek]

(In reply to Georg Koppen from comment #13)
> That might be an interesting alternative. However, they are tracking LLVM
> trunk closely and I am not sure how well compiling Firefox works with that
> right now...

Shouldn't be too bad, bug 1415689 just updated ASan builds to LLVM trunk.

Comment 16

[Chris Peterson [:cpeterson]]

(In reply to Georg Koppen from comment #11)
> That's interesting, thanks. In case no one beats me to it, I plan to pick
> this up. It won't happen in this month though and maybe November is
> optimistic as well. But I definitely want to look at it in December to have
> time to fix possible upcoming issues to have everything ready for ESR 59.

btw, the next ESR has pushed back from Firefox 59 to 60. Nightly 60 will merge to Beta on 2018-03-12 and to Release on 2018-05-08.

https://wiki.mozilla.org/RapidRelease/Calendar

Comment 17

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Georg is working on this, and it is high importance for Tor

Comment 18

[Jacek Caban]

Attachment: clang hacks

I experimented with clang + mingw-w64 configuration lately with partial success. I chose to try using existing, GCC-based, mingw-w64 toolchain and just use clang as GCC replacement (*). I used clang from Git. As far as I know a fairly recent Git is a good idea for mingw-w64 integration. To use clang, I added those to mozconfig (**):

export CC="clang -target i686-w64-mingw32 -fms-extensions"
export CXX="clang++ -target i686-w64-mingw32 -fms-extensions -Wno-incompatible-ms-struct"
export LDFLAGS=-lwinpthread
export CPP="$CC -E"
export WINDRES=i686-w64-mingw32-windres

A few comments to those:
- -fms-extensions should really be default for that target. It may be a good idea to experiment with -fms-compatibility options.
- I had mingw-w64 installed in /usr/local. For unusual locations specifying sysroot is needed.
- -Wno-incompatible-ms-struct just avoids warnings spam.
- -lwinpthead is really a workaround for clang not being aware of winpthread
- I don't remember details about CPP and WINDRES, but I think it's there to work around a bug in configure scripts. Ideally, this should not be needed.


I attached a WIP with hacks trying to get it to build. I planned to hack it so it links and then look for better solutions for those hacks, but I don't know when I will find more time for this. I think it may be helpful here. A notes from this attempt:

- Build system is often confused about this config. For example it assumes that clang with windows target is clang-cl, so applies msvc-style arguments instead of gcc-style. I just hacked a few places, but it needs a more systematic solution.
- Library folding needs a solution. For GCC we use -mnop-fun-dllimport, which is not available on clang. I tried -Ddllimport=noop instead, but it wasn't enough.
- Static build didn't work, so I disabled it. It should be fixed instead.
- There were more errors I didn't look at yet.


(*) There are other possibly interesting configurations. We could just have plain llvm-based toolchain used instead (eg. llvm-rc and alikes). It should be possible (possibly with clang changes) to use clang-cl and llvm-link, which would use the same arguments syntax as MSVC.

(**) stylo was still disabled in my build. I think that once we get clang-based build working, there should be no more blockers for getting it working as well.

Comment 19

[Martin Storsjö]

(In reply to Jacek Caban from comment #18)
> Created attachment 8945541 [details] [diff] [review]
> clang hacks
> 
> I experimented with clang + mingw-w64 configuration lately with partial
> success. I chose to try using existing, GCC-based, mingw-w64 toolchain and
> just use clang as GCC replacement (*). I used clang from Git. As far as I
> know a fairly recent Git is a good idea for mingw-w64 integration. To use
> clang, I added those to mozconfig (**):
> 
> export CC="clang -target i686-w64-mingw32 -fms-extensions"
> export CXX="clang++ -target i686-w64-mingw32 -fms-extensions
> -Wno-incompatible-ms-struct"
> export LDFLAGS=-lwinpthread
> export CPP="$CC -E"
> export WINDRES=i686-w64-mingw32-windres
> 
> A few comments to those:
> - -fms-extensions should really be default for that target.

Does the build work with a normal GCC based mingw without this flag? I haven't had the need to use this flag so far. I tried it once in some situation to try to get a built-in definition of __int64 (which GCC doesn't define built-in either), but it iirc just caused more issues than it solved. I don't remember the details though. In which cases do you need it with clang where you don't need it with GCC? Such differences are usually unintentional.

> - Build system is often confused about this config. For example it assumes
> that clang with windows target is clang-cl, so applies msvc-style arguments
> instead of gcc-style. I just hacked a few places, but it needs a more
> systematic solution.

In https://github.com/mstorsjo/llvm-mingw, I'm creating wrapper frontend scripts using this script: https://github.com/mstorsjo/llvm-mingw/blob/master/wrappers/clang-target-wrapper.sh

That gives me frontends like i686-w64-mingw32-clang (and I'm also creating ones named <triplet>-gcc and <triplet>-g++, to ease building of dozens of normal autotools based libraries by just adding --host=<triplet>). Something like that might cause less confusion for your build systems.

Comment 20

[Jacek Caban]

(In reply to Martin Storsjö from comment #19)
> (In reply to Jacek Caban from comment #18)
> > Created attachment 8945541 [details] [diff] [review]
> > clang hacks
> > 
> > I experimented with clang + mingw-w64 configuration lately with partial
> > success. I chose to try using existing, GCC-based, mingw-w64 toolchain and
> > just use clang as GCC replacement (*). I used clang from Git. As far as I
> > know a fairly recent Git is a good idea for mingw-w64 integration. To use
> > clang, I added those to mozconfig (**):
> > 
> > export CC="clang -target i686-w64-mingw32 -fms-extensions"
> > export CXX="clang++ -target i686-w64-mingw32 -fms-extensions
> > -Wno-incompatible-ms-struct"
> > export LDFLAGS=-lwinpthread
> > export CPP="$CC -E"
> > export WINDRES=i686-w64-mingw32-windres
> > 
> > A few comments to those:
> > - -fms-extensions should really be default for that target.
> 
> Does the build work with a normal GCC based mingw without this flag?
> I haven't had the need to use this flag so far. I tried it once in some
> situation to try to get a built-in definition of __int64 (which GCC doesn't
> define built-in either), but it iirc just caused more issues than it solved.
> I don't remember the details though. In which cases do you need it with
> clang where you don't need it with GCC? Such differences are usually
> unintentional.

I don't remember details, but in general there are quite a few paths where clang chooses msvc code variant instead of gcc and I think that's nice that it's able to do that.

In any case, it doesn't cause me troubles. I had to fix some intrins issues in mingw-w64 (it's already upstreamed), but after that it works good.

> > - Build system is often confused about this config. For example it assumes
> > that clang with windows target is clang-cl, so applies msvc-style arguments
> > instead of gcc-style. I just hacked a few places, but it needs a more
> > systematic solution.
> 
> In https://github.com/mstorsjo/llvm-mingw, I'm creating wrapper frontend
> scripts using this script:
> https://github.com/mstorsjo/llvm-mingw/blob/master/wrappers/clang-target-
> wrapper.sh
> 
> That gives me frontends like i686-w64-mingw32-clang (and I'm also creating
> ones named <triplet>-gcc and <triplet>-g++, to ease building of dozens of
> normal autotools based libraries by just adding --host=<triplet>). Something
> like that might cause less confusion for your build systems.

Yeah, it looks nice, but I can see that it's mostly for clang/llvm toolchain. I tried to do one step at a time.

Comment 21

[Martin Storsjö]

(In reply to Jacek Caban from comment #20)
> (In reply to Martin Storsjö from comment #19)
> > (In reply to Jacek Caban from comment #18)
> > > - Build system is often confused about this config. For example it assumes
> > > that clang with windows target is clang-cl, so applies msvc-style arguments
> > > instead of gcc-style. I just hacked a few places, but it needs a more
> > > systematic solution.
> > 
> > In https://github.com/mstorsjo/llvm-mingw, I'm creating wrapper frontend
> > scripts using this script:
> > https://github.com/mstorsjo/llvm-mingw/blob/master/wrappers/clang-target-
> > wrapper.sh
> > 
> > That gives me frontends like i686-w64-mingw32-clang (and I'm also creating
> > ones named <triplet>-gcc and <triplet>-g++, to ease building of dozens of
> > normal autotools based libraries by just adding --host=<triplet>). Something
> > like that might cause less confusion for your build systems.
> 
> Yeah, it looks nice, but I can see that it's mostly for clang/llvm
> toolchain. I tried to do one step at a time.

Yes - for your case it'd be enough to just use it for setting -target and leaving the rest of the options at their default values.

Comment 22

[Xidorn Quan [:xidorn] UTC+11]

Georg, do you have some estimation on how long would it take for finishing up this? We'd like to ensure removing the old style system in Firefox 60, and this is one of the blockers for that.

Comment 23

[Georg Koppen]

I am afraid I don't know yet. My current plan is to have something ready before Firefox 59 gets out. I am  looking at getting our ESR 52-based Tor Browser built with the new toolchain right now ignoring all the changes happened between ESR 52 and mozilla-central using the work Martin et al. did and at which he pointed in comment 19. In the past weeks there other things I still had in my backlog which needed to get fixed but this is looking better now and I hope I can make considerable progress in the next weeks.

I thought it might be good to not just focus on one approach to solve this bug, so, Jacek if you are faster with your one-step-at-a-time-approach I am all for using that result for now and improve things later on. Please keep working on it if you find the time. :)

Comment 24

[Makoto Kato [:m_kato]]

For task cluster job, you should add BINDGEN_CFLAGS to mozconfigs for migw build if this job still uses gcc.

Actually, we don't generate BINDGEN_CFLAGS for cross compiling except to Android.

Comment 25

[Georg Koppen]

Okay, let me give you an update on that. The summary is I can build almost all of the Tor Browser code (ESR52-based) using https://github.com/mstorsjo/llvm-mingw with only a small amount of changes. Most of those changes are backports from patches which landed meanwhile in mozilla-central.

I first started trying the build configuration Mozilla currently has for mingw-w64. But ran fast into the following issue:

fakeopenh264.dll
rm -f fakeopenh264.dll
/home/thomas/Arbeit/Tor/debugging/21777/obj-mingw/_virtualenv/bin/python /home/thomas/Arbeit/Tor/debugging/21777/config/expandlibs_exec.py --uselist --  /home/thomas/Arbeit/Tor/debugging/win59/clang_toolchain_martin/bin/x86_64-w64-mingw32-clang++ -v --sysroot /home/thomas/Arbeit/Tor/debugging/win59/clang_toolchain_martin/x86_64-w64-mingw32 -std=gnu++11 -mwindows -shared -Wl,--out-implib -Wl,libfakeopenh264.a -o fakeopenh264.dll  gmp-fake-openh264.o  ./module.res -Wl,--build-id -static             -luuid -lgdi32 -lwinmm -lwsock32 -luserenv -lsecur32 -lole32

breaks with 

lld: error: unknown argument: --start-group

and the ld invocation looks like

"/home/thomas/Arbeit/Tor/debugging/win59/clang_toolchain_martin/bin/ld.lld" --sysroot=../../../../../win59/clang_toolchain_martin/x86_64-w64-mingw32 -m i386pep --subsystem windows -Bstatic -o fakeopenh264.dll ../../../../../win59/clang_toolchain_martin/x86_64-w64-mingw32/lib/dllcrt2.o ../../../../../win59/clang_toolchain_martin/x86_64-w64-mingw32/lib/crtbegin.o -L../../../../../win59/clang_toolchain_martin/x86_64-w64-mingw32/x86_64-w64-mingw32/lib -L../../../../../win59/clang_toolchain_martin/x86_64-w64-mingw32/lib -L../../../../../win59/clang_toolchain_martin/x86_64-w64-mingw32/x86_64-w64-mingw32/sys-root/mingw/lib --out-implib libfakeopenh264.a gmp-fake-openh264.o module.res --build-id -luuid -lgdi32 -lwinmm -lwsock32 -luserenv -lsecur32 -lole32 -lc++ --start-group -lmingw32 /home/thomas/Arbeit/Tor/debugging/win59/clang_toolchain_martin/lib/clang/7.0.0/lib/windows/libclang_rt.builtins-x86_64.a -lmoldname -lmingwex -lmsvcrt -lgdi32 -lcomdlg32 -ladvapi32 -lshell32 -luser32 -lkernel32 --end-group ../../../../../win59/clang_toolchain_martin/x86_64-w64-mingw32/lib/crtend.o

It turns out this is stemming from https://github.com/llvm-mirror/clang/blob/master/lib/Driver/ToolChains/MinGW.cpp#L208. Using that linker flag while lld does not understand it seems to be a bug to me. I tried working around that and got a bit further but run into other issues during that linking step. So I gave up on the static build as Jacek did.

The .mozconfig I am using and the not-yet-upstreamed patches (which are just needed to fix fatal compiler warnings) are attached, omitting the needed STL related workaround which is bug 1392604 and Jacek's changes to the configure scripts mentioned in https://bugzilla.mozilla.org/attachment.cgi?id=8945541.

I am finally at the stage where we want to link nss code and get the following error:

Executing: ../../../../../win59/clang_toolchain_martin/bin/x86_64-w64-mingw32-clang++ --sysroot ../../../../../win59/clang_toolchain_martin/x86_64-w64-mingw32 -std=gnu++11 -mwindows -shared -Wl,--out-implib -Wl,libnss3.a -o nss3.dll module.res -Wl,--build-id @/home/thomas/Arbeit/Tor/debugging/21777/obj-mingw/config/external/nss/tmpy9yYMR.list nss3.dll.def -luuid -lgdi32 -lwinmm -lwsock32 -luserenv -lsecur32 -ladvapi32 -lws2_32 -lmswsock -lwinmm

[snip]

    ../nspr/ds/plhash.o

lld-link: error: ../nspr/pr/prlink.o: undefined symbol: strcmpi
lld-link: error: libmsvcrt.a(ucrtbase.dll): undefined symbol: _strcmpi
lld-link: error: libmsvcrt.a(ucrtbase.dll): undefined symbol: strcmpi
clang-7.0: error: linker command failed with exit code 1 (use -v to see invocation)

Comment 26

[Georg Koppen]

Attachment: 0001-Small-fixups-for-fatal-compiler-warnings.patch

Comment 27

[Georg Koppen]

Attachment: mozconfig used for building Tor Browser with clang-based toolchain

Comment 28

[Jacek Caban]

(In reply to Georg Koppen from comment #25)
> lld-link: error: ../nspr/pr/prlink.o: undefined symbol: strcmpi
> lld-link: error: libmsvcrt.a(ucrtbase.dll): undefined symbol: _strcmpi
> lld-link: error: libmsvcrt.a(ucrtbase.dll): undefined symbol: strcmpi
> clang-7.0: error: linker command failed with exit code 1 (use -v to see
> invocation)

It seems that this toolchain uses ucrtbase.dll as default C runtime. This is interesting and ideally we want to switch to that config (I even experimented a bit with this in GCC-based builds), but it's still a work in progress in mingw-w64, so problems like above are not unexpected. I'd suggest to try a toolchain that uses msvcrt.dll for the time being. To do that, don't pass --with-default-msvcrt=ucrtbase to mingw-w64 headers and crt configure scripts (that's in build-mingw-w64.sh AFAICS). Martin will know better.

Comment 29

[Georg Koppen]

Yeah, that was my first idea, too. But somewhat surprisingly the proposed change breaks things already *earlier*:

Executing: ../../../../../win59/clang_toolchain_martin/bin/x86_64-w64-mingw32-clang++ --sysroot ../../../../../win59/clang_toolchain_martin/x86_64-w64-mingw32 -std=gnu++11 -mwindows -shared -Wl,--out-implib -Wl,libfake.a -o fake.dll @/home/thomas/Arbeit/Tor/debugging/21777/obj-mingw/dom/media/gmp-plugin/tmp9h9F0s.list module.res -Wl,--build-id -luuid -lgdi32 -lwinmm -lwsock32 -luserenv -lsecur32 -lole32
/home/thomas/Arbeit/Tor/debugging/21777/obj-mingw/dom/media/gmp-plugin/tmp9h9F0s.list:
    gmp-fake.o
    gmp-test-decryptor.o
    gmp-test-storage.o

lld-link: error: libc++.a(locale.cpp.obj): undefined symbol: __stdio_common_vsscanf
lld-link: error: libc++.a(locale.cpp.obj): undefined symbol: __imp__strftime_l
lld-link: error: libc++.a(string.cpp.obj): undefined symbol: __stdio_common_vswprintf
clang-7.0: error: linker command failed with exit code 1 (use -v to see invocation)

Comment 30

[Georg Koppen]

Okay, I rebuilt the toolchain (switching to msvcrt.dll as suggested in comment:28) and made a clobber build and this seems to have helped with the issue in comment:25. I think I got everything compiled but am still hitting linking failures:

1) When linking the updater:

Executing: ../../../../../../win59/clang_toolchain_martin/bin/x86_64-w64-mingw32-clang++ --sysroot ../../../../../../win59/clang_toolchain_martin/x86_64-w64-mingw32 -std=gnu++11 -mwindows -o updater.exe -Qunused-arguments -Qunused-arguments -Wall -Wc++11-compat -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wloop-analysis -Wc++11-compat-pedantic -Wc++14-compat -Wc++14-compat-pedantic -Wc++1z-compat -Wimplicit-fallthrough -Wstring-conversion -Wthread-safety -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-format -Wno-unknown-warning-option -Wno-return-type-c-linkage -Wno-incompatible-ms-struct -fno-exceptions -fno-strict-aliasing -mms-bitfields -fno-rtti -fno-exceptions -fno-math-errno -pthreads -pipe -g -O -fomit-frame-pointer @/home/thomas/Arbeit/Tor/debugging/21777/obj-mingw/toolkit/mozapps/update/updater/tmp5LJ3Ww.list module.res -municode -mwindows -Wl,--build-id -DELAYLOAD:crypt32.dll -DELAYLOAD:comctl32.dll -DELAYLOAD:userenv.dll -DELAYLOAD:wsock32.dll ../../../../config/external/nss/libnss3.a -luuid -lgdi32 -lwinmm -lwsock32 -luserenv -lsecur32 -lcomctl32 -lws2_32 -lshell32 -lshlwapi -ldelayimp
/home/thomas/Arbeit/Tor/debugging/21777/obj-mingw/toolkit/mozapps/update/updater/tmp5LJ3Ww.list:
    archivereader.o
    bspatch.o
    loaddlls.o
    progressui_win.o
    updater.o
    win_dirent.o
    ../../../../modules/libmar/verify/Unified_c_libmar_verify0.o
    ../common-standalone/pathhash.o
    ../common-standalone/readstrings.o
    ../common-standalone/uachelper.o
    ../common-standalone/updatecommon.o
    ../common-standalone/updatehelper.o
    ../../../../modules/libmar/sign/Unified_c_modules_libmar_sign0.o
    ../../../../modules/libmar/src/Unified_c_modules_libmar_src0.o
    ../../../../modules/libbz2/src/Unified_c_modules_libbz2_src0.o

lld-link: error: libmingw32.a(lib64_libmingw32_a-crt0_w.o): undefined symbol: wWinMain

2) When linking libxul:

lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol: _ZN14nsXPTCStubBase5Stub3Ev
lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol: _ZN14nsXPTCStubBase5Stub4Ev
lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol: _ZN14nsXPTCStubBase5Stub5Ev
lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol: _ZN14nsXPTCStubBase5Stub6Ev
lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol: _ZN14nsXPTCStubBase5Stub7Ev
lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol: _ZN14nsXPTCStubBase5Stub8Ev
lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol: _ZN14nsXPTCStubBase5Stub9Ev
lld-link: error: too many errors emitted, stopping now (use /ERRORLIMIT:0 to see all errors)

Comment 31

[Nathan Froyd [:froydnj]]

(In reply to Georg Koppen from comment #30)
> 2) When linking libxul:
> 
> lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol:
> _ZN14nsXPTCStubBase5Stub3Ev
> lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol:
> _ZN14nsXPTCStubBase5Stub4Ev
> lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol:
> _ZN14nsXPTCStubBase5Stub5Ev
> lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol:
> _ZN14nsXPTCStubBase5Stub6Ev
> lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol:
> _ZN14nsXPTCStubBase5Stub7Ev
> lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol:
> _ZN14nsXPTCStubBase5Stub8Ev
> lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol:
> _ZN14nsXPTCStubBase5Stub9Ev
> lld-link: error: too many errors emitted, stopping now (use /ERRORLIMIT:0 to
> see all errors)

clang might not be supporting toplevel asms that get used on mingw64:

http://dxr.mozilla.org/mozilla-central/source/xpcom/reflect/xptcall/md/win32/xptcstubs_x86_64_gnu.cpp#181

which is a little surprising to me, since clang supports toplevel asm on other platforms.  Maybe verify that the appropriate files in xpcom/reflect/xptcall/md/win32/ are getting compiled and the symbols therein?

Comment 32

[Martin Storsjö]

(In reply to Georg Koppen from comment #25)
> lld: error: unknown argument: --start-group
> 
> It turns out this is stemming from
> https://github.com/llvm-mirror/clang/blob/master/lib/Driver/ToolChains/MinGW.
> cpp#L208. Using that linker flag while lld does not understand it seems to
> be a bug to me.

It's a limitation of lld yes.

Since you originally would use clang only for the compiler, but still have it invoke the normal GNU binutils ld linker, clang passes pretty much the same linker flags as gcc does - this setup has been working for a few years at least, afaik.

When replacing the linker as well, lld normally emulates MSVC's link.exe and takes parameters in that form (when invoked as lld-link). Recently (within the last 6 months) it got a separate frontend that parses arguments in the unix linker style and internally wraps them to lld-link parameters and invokes that. I've mapped and hooked up most of the arguments that you'll normally run into (or added dummy no-op parameters to avoid such errors). This one I've seen before but I also managed to work around easily back then.


(In reply to Jacek Caban from comment #28)
> (In reply to Georg Koppen from comment #25)
> > lld-link: error: ../nspr/pr/prlink.o: undefined symbol: strcmpi
> > lld-link: error: libmsvcrt.a(ucrtbase.dll): undefined symbol: _strcmpi
> > lld-link: error: libmsvcrt.a(ucrtbase.dll): undefined symbol: strcmpi
> > clang-7.0: error: linker command failed with exit code 1 (use -v to see
> > invocation)
> 
> It seems that this toolchain uses ucrtbase.dll as default C runtime. This is
> interesting and ideally we want to switch to that config (I even
> experimented a bit with this in GCC-based builds), but it's still a work in
> progress in mingw-w64, so problems like above are not unexpected.

Yup - although I've been able to build quite a lot of code with it so far. So while it's definitely less mature than the normal msvcrt.dll setup - achieving that level of general readyness will probably take ages - it already can build quite a bunch of libraries/projects (Qt, VLC and a few dozen of VLC's dependencies). So the main thing missing is testing to point out functions that people actually use that are missing. (If one sits down reading all of mingw's headers and starts looking through that, there's probably hundreds of changed/missing functions to hook up, but that's a ton of work.)

This at least points out another function that I can try to fix for the ucrtbase case, hopefully bringing it closer to working for yet another project.

> I'd suggest to try a toolchain that uses msvcrt.dll for the time being. To do
> that, don't pass --with-default-msvcrt=ucrtbase to mingw-w64 headers and crt
> configure scripts (that's in build-mingw-w64.sh AFAICS). Martin will know
> better.

Yup, that's the way. The reason for me defaulting to ucrtbase is both that I want to move towards that, and that libc++ kinda relies on a bunch of locale functions that are missing in the default msvcrt.dll. I've tried to make sure it still links in that configuration, but I don't test it regularly in that way any longer.


(In reply to Georg Koppen from comment #29)
> Yeah, that was my first idea, too. But somewhat surprisingly the proposed
> change breaks things already *earlier*:
> 
> Executing:
> ../../../../../win59/clang_toolchain_martin/bin/x86_64-w64-mingw32-clang++
> --sysroot ../../../../../win59/clang_toolchain_martin/x86_64-w64-mingw32
> -std=gnu++11 -mwindows -shared -Wl,--out-implib -Wl,libfake.a -o fake.dll
> @/home/thomas/Arbeit/Tor/debugging/21777/obj-mingw/dom/media/gmp-plugin/
> tmp9h9F0s.list module.res -Wl,--build-id -luuid -lgdi32 -lwinmm -lwsock32
> -luserenv -lsecur32 -lole32
> /home/thomas/Arbeit/Tor/debugging/21777/obj-mingw/dom/media/gmp-plugin/
> tmp9h9F0s.list:
>     gmp-fake.o
>     gmp-test-decryptor.o
>     gmp-test-storage.o
> 
> lld-link: error: libc++.a(locale.cpp.obj): undefined symbol:
> __stdio_common_vsscanf
> lld-link: error: libc++.a(locale.cpp.obj): undefined symbol:
> __imp__strftime_l
> lld-link: error: libc++.a(string.cpp.obj): undefined symbol:
> __stdio_common_vswprintf
> clang-7.0: error: linker command failed with exit code 1 (use -v to see
> invocation)

As you noticed later - you need to rebuild libc++ if you change which CRT to use. Theoretically, you shouldn't need to rebuild libraries that already is a linked DLL since that uses whatever CRT it was built with, but libc++ is currently used as a static library only, so that needs to be built targeting the same CRT as you actually will link it with.

(In reply to Georg Koppen from comment #30)
> 
> 1) When linking the updater:
> 
> Executing:
> ../../../../../../win59/clang_toolchain_martin/bin/x86_64-w64-mingw32-
> clang++ --sysroot
> ../../../../../../win59/clang_toolchain_martin/x86_64-w64-mingw32
> -std=gnu++11 -mwindows -o updater.exe -Qunused-arguments -Qunused-arguments
> -Wall -Wc++11-compat -Wempty-body -Wignored-qualifiers -Woverloaded-virtual
> -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code
> -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wloop-analysis
> -Wc++11-compat-pedantic -Wc++14-compat -Wc++14-compat-pedantic
> -Wc++1z-compat -Wimplicit-fallthrough -Wstring-conversion -Wthread-safety
> -Wno-inline-new-delete -Wno-error=deprecated-declarations
> -Wno-error=array-bounds -Wno-format -Wno-unknown-warning-option
> -Wno-return-type-c-linkage -Wno-incompatible-ms-struct -fno-exceptions
> -fno-strict-aliasing -mms-bitfields -fno-rtti -fno-exceptions
> -fno-math-errno -pthreads -pipe -g -O -fomit-frame-pointer
> @/home/thomas/Arbeit/Tor/debugging/21777/obj-mingw/toolkit/mozapps/update/
> updater/tmp5LJ3Ww.list module.res -municode -mwindows -Wl,--build-id
> -DELAYLOAD:crypt32.dll -DELAYLOAD:comctl32.dll -DELAYLOAD:userenv.dll
> -DELAYLOAD:wsock32.dll ../../../../config/external/nss/libnss3.a -luuid
> -lgdi32 -lwinmm -lwsock32 -luserenv -lsecur32 -lcomctl32 -lws2_32 -lshell32
> -lshlwapi -ldelayimp
> /home/thomas/Arbeit/Tor/debugging/21777/obj-mingw/toolkit/mozapps/update/
> updater/tmp5LJ3Ww.list:
>     archivereader.o
>     bspatch.o
>     loaddlls.o
>     progressui_win.o
>     updater.o
>     win_dirent.o
>     ../../../../modules/libmar/verify/Unified_c_libmar_verify0.o
>     ../common-standalone/pathhash.o
>     ../common-standalone/readstrings.o
>     ../common-standalone/uachelper.o
>     ../common-standalone/updatecommon.o
>     ../common-standalone/updatehelper.o
>     ../../../../modules/libmar/sign/Unified_c_modules_libmar_sign0.o
>     ../../../../modules/libmar/src/Unified_c_modules_libmar_src0.o
>     ../../../../modules/libbz2/src/Unified_c_modules_libbz2_src0.o
> 
> lld-link: error: libmingw32.a(lib64_libmingw32_a-crt0_w.o): undefined
> symbol: wWinMain

What's the actual entry point that's intended to be called here, and in what object file is it defined? You can try adding -v and -Wl,-verbose when linking to make it spit out a lot more details about what it does; the -v will print what parameters clang uses when invoking the linker, and the -verbose will both print what parameters lld internally passes to lld-link, and will print out a lot of detail about what object files are loaded and from where.

The -municode here makes sure to link in crt2u.o, which will have an undefined reference to "wmain". And if your object files don't provide it, it will pull in a wmain from libmingw32.a(lib64_libmingw32_a-crt0_w.o) which then adds an undefined reference to wWinMain instead.

Related code in lld is at https://github.com/llvm-mirror/lld/blob/master/COFF/Driver.cpp#L393.


(In reply to Nathan Froyd [:froydnj] from comment #31)
> (In reply to Georg Koppen from comment #30)
> > 2) When linking libxul:
> > 
> > lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol:
> > _ZN14nsXPTCStubBase5Stub3Ev
> > lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol:
> > _ZN14nsXPTCStubBase5Stub4Ev
> > lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol:
> > _ZN14nsXPTCStubBase5Stub5Ev
> > lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol:
> > _ZN14nsXPTCStubBase5Stub6Ev
> > lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol:
> > _ZN14nsXPTCStubBase5Stub7Ev
> > lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol:
> > _ZN14nsXPTCStubBase5Stub8Ev
> > lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol:
> > _ZN14nsXPTCStubBase5Stub9Ev
> > lld-link: error: too many errors emitted, stopping now (use /ERRORLIMIT:0 to
> > see all errors)
> 
> clang might not be supporting toplevel asms that get used on mingw64:
> 
> http://dxr.mozilla.org/mozilla-central/source/xpcom/reflect/xptcall/md/win32/
> xptcstubs_x86_64_gnu.cpp#181
> 
> which is a little surprising to me, since clang supports toplevel asm on
> other platforms.  Maybe verify that the appropriate files in
> xpcom/reflect/xptcall/md/win32/ are getting compiled and the symbols therein?

I tested compiling a copypasted snippet of that with one toplevel piece of asm (the SharedStub part), and it did look like it produced the symbols just like intended. I didn't test the macro and include file part of it though.

Comment 33

[Martin Storsjö]

(In reply to Martin Storsjö from comment #32)
> (In reply to Georg Koppen from comment #25)
> > lld: error: unknown argument: --start-group
> > 
> > It turns out this is stemming from
> > https://github.com/llvm-mirror/clang/blob/master/lib/Driver/ToolChains/MinGW.
> > cpp#L208. Using that linker flag while lld does not understand it seems to
> > be a bug to me.
> 
> It's a limitation of lld yes.
> 
> Since you originally would use clang only for the compiler, but still have
> it invoke the normal GNU binutils ld linker, clang passes pretty much the
> same linker flags as gcc does - this setup has been working for a few years
> at least, afaik.

I forgot to clarify/explain one detail here.

What linker to use by default can be done at compile time when building clang, but I've tried to avoid that so that one can use a vanilla build of clang/lld as well. clang defaults to using the normal binutils ld, and using libstdc++ from gcc, but I'm overriding these by passing -rtlib=compiler-rt -stdlib=libc++ -fuse-ld=lld in the shellscript wrapper that provides <arch>-w64-mingw32-{clang,gcc} by calling clang internally, setting my defaults. If you remove those parts, you'll get a setup much closer to vanilla gcc+mingw (but then you'd probably need a gcc bootstrapped mingw+libstdc++ environment as well).

Comment 34

[Jacek Caban]

(In reply to Georg Koppen from comment #30)
> Okay, I rebuilt the toolchain (switching to msvcrt.dll as suggested in
> comment:28) and made a clobber build and this seems to have helped with the
> issue in comment:25. I think I got everything compiled but am still hitting
> linking failures:
> 
> 1) When linking the updater:
> 
> Executing:
> ../../../../../../win59/clang_toolchain_martin/bin/x86_64-w64-mingw32-
> clang++ --sysroot
> ../../../../../../win59/clang_toolchain_martin/x86_64-w64-mingw32
> -std=gnu++11 -mwindows -o updater.exe -Qunused-arguments -Qunused-arguments
> -Wall -Wc++11-compat -Wempty-body -Wignored-qualifiers -Woverloaded-virtual
> -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code
> -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wloop-analysis
> -Wc++11-compat-pedantic -Wc++14-compat -Wc++14-compat-pedantic
> -Wc++1z-compat -Wimplicit-fallthrough -Wstring-conversion -Wthread-safety
> -Wno-inline-new-delete -Wno-error=deprecated-declarations
> -Wno-error=array-bounds -Wno-format -Wno-unknown-warning-option
> -Wno-return-type-c-linkage -Wno-incompatible-ms-struct -fno-exceptions
> -fno-strict-aliasing -mms-bitfields -fno-rtti -fno-exceptions
> -fno-math-errno -pthreads -pipe -g -O -fomit-frame-pointer
> @/home/thomas/Arbeit/Tor/debugging/21777/obj-mingw/toolkit/mozapps/update/
> updater/tmp5LJ3Ww.list module.res -municode -mwindows -Wl,--build-id
> -DELAYLOAD:crypt32.dll -DELAYLOAD:comctl32.dll -DELAYLOAD:userenv.dll
> -DELAYLOAD:wsock32.dll ../../../../config/external/nss/libnss3.a -luuid
> -lgdi32 -lwinmm -lwsock32 -luserenv -lsecur32 -lcomctl32 -lws2_32 -lshell32
> -lshlwapi -ldelayimp
> /home/thomas/Arbeit/Tor/debugging/21777/obj-mingw/toolkit/mozapps/update/
> updater/tmp5LJ3Ww.list:
>     archivereader.o
>     bspatch.o
>     loaddlls.o
>     progressui_win.o
>     updater.o
>     win_dirent.o
>     ../../../../modules/libmar/verify/Unified_c_libmar_verify0.o
>     ../common-standalone/pathhash.o
>     ../common-standalone/readstrings.o
>     ../common-standalone/uachelper.o
>     ../common-standalone/updatecommon.o
>     ../common-standalone/updatehelper.o
>     ../../../../modules/libmar/sign/Unified_c_modules_libmar_sign0.o
>     ../../../../modules/libmar/src/Unified_c_modules_libmar_src0.o
>     ../../../../modules/libbz2/src/Unified_c_modules_libbz2_src0.o
> 
> lld-link: error: libmingw32.a(lib64_libmingw32_a-crt0_w.o): undefined
> symbol: wWinMain

That's just a guess, but GCC had similar problems with C++ linkage and non-main entry points. In this case NS_main from updater.cpp is in fact wmain. Does it help if you add explicit extern "C" to NS_main?

Comment 35

[Jacek Caban]

(In reply to Nathan Froyd [:froydnj] from comment #31)
> (In reply to Georg Koppen from comment #30)
> > 2) When linking libxul:
> > 
> > lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol:
> > _ZN14nsXPTCStubBase5Stub3Ev
> > lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol:
> > _ZN14nsXPTCStubBase5Stub4Ev
> > lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol:
> > _ZN14nsXPTCStubBase5Stub5Ev
> > lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol:
> > _ZN14nsXPTCStubBase5Stub6Ev
> > lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol:
> > _ZN14nsXPTCStubBase5Stub7Ev
> > lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol:
> > _ZN14nsXPTCStubBase5Stub8Ev
> > lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol:
> > _ZN14nsXPTCStubBase5Stub9Ev
> > lld-link: error: too many errors emitted, stopping now (use /ERRORLIMIT:0 to
> > see all errors)
> 
> clang might not be supporting toplevel asms that get used on mingw64:
> 
> http://dxr.mozilla.org/mozilla-central/source/xpcom/reflect/xptcall/md/win32/
> xptcstubs_x86_64_gnu.cpp#181
> 
> which is a little surprising to me, since clang supports toplevel asm on
> other platforms.  Maybe verify that the appropriate files in
> xpcom/reflect/xptcall/md/win32/ are getting compiled and the symbols therein?

x86 variant is actually implemented here:
https://dxr.mozilla.org/mozilla-central/source/xpcom/reflect/xptcall/md/win32/xptcstubs.cpp#153

Note that those symbols have stdcall decorations in assembly (additional "_" in the beginning and @4 in the end of symbol name), but symbols from this error doesn't have them. I'm not sure why clang doesn't use decorations in this case, but to work around it, you could try removing them in the assembly version.

Comment 36

[Jacek Caban]

(In reply to Martin Storsjö from comment #32)
> Yup - although I've been able to build quite a lot of code with it so far.
> So while it's definitely less mature than the normal msvcrt.dll setup -
> achieving that level of general readyness will probably take ages - it
> already can build quite a bunch of libraries/projects (Qt, VLC and a few
> dozen of VLC's dependencies). So the main thing missing is testing to point
> out functions that people actually use that are missing. (If one sits down
> reading all of mingw's headers and starts looking through that, there's
> probably hundreds of changed/missing functions to hook up, but that's a ton
> of work.)
> 
> This at least points out another function that I can try to fix for the
> ucrtbase case, hopefully bringing it closer to working for yet another
> project.

Yeah, it would be great if we could switch to ucrtbase-based toolchain and I'm happy to help with it. I just think that it shouldn't block progress on this bug.

Comment 37

[Jacek Caban]

(In reply to Jacek Caban from comment #35)
> x86 variant is actually implemented here:
> https://dxr.mozilla.org/mozilla-central/source/xpcom/reflect/xptcall/md/
> win32/xptcstubs.cpp#153
> 
> Note that those symbols have stdcall decorations in assembly (additional "_"
> in the beginning and @4 in the end of symbol name), but symbols from this
> error doesn't have them. I'm not sure why clang doesn't use decorations in
> this case, but to work around it, you could try removing them in the
> assembly version.

I didn't notice you were doing 64-bit build. Still our assembly has additional @4 which probably shouldn't be there (I think it shouldn't be there even for GCC; stdcall is irrelevant for win64).

Comment 38

[Georg Koppen]

(In reply to Jacek Caban from comment #34)
> (In reply to Georg Koppen from comment #30)

[snip]

> > lld-link: error: libmingw32.a(lib64_libmingw32_a-crt0_w.o): undefined
> > symbol: wWinMain
> 
> That's just a guess, but GCC had similar problems with C++ linkage and
> non-main entry points. In this case NS_main from updater.cpp is in fact
> wmain. Does it help if you add explicit extern "C" to NS_main?

It does, thanks! That solves the updater linking problem and now "just" libxul remains.

Comment 39

[Georg Koppen]

(In reply to Jacek Caban from comment #37)
> (In reply to Jacek Caban from comment #35)
> > x86 variant is actually implemented here:
> > https://dxr.mozilla.org/mozilla-central/source/xpcom/reflect/xptcall/md/
> > win32/xptcstubs.cpp#153
> > 
> > Note that those symbols have stdcall decorations in assembly (additional "_"
> > in the beginning and @4 in the end of symbol name), but symbols from this
> > error doesn't have them. I'm not sure why clang doesn't use decorations in
> > this case, but to work around it, you could try removing them in the
> > assembly version.
> 
> I didn't notice you were doing 64-bit build. Still our assembly has
> additional @4 which probably shouldn't be there (I think it shouldn't be
> there even for GCC; stdcall is irrelevant for win64).

Alas, that's not helping.

(In reply to Nathan Froyd [:froydnj] from comment #31)
> (In reply to Georg Koppen from comment #30)
> > 2) When linking libxul:
> > 
> > lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol:
> > _ZN14nsXPTCStubBase5Stub3Ev
> > lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol:
> > _ZN14nsXPTCStubBase5Stub4Ev
> > lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol:
> > _ZN14nsXPTCStubBase5Stub5Ev
> > lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol:
> > _ZN14nsXPTCStubBase5Stub6Ev
> > lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol:
> > _ZN14nsXPTCStubBase5Stub7Ev
> > lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol:
> > _ZN14nsXPTCStubBase5Stub8Ev
> > lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol:
> > _ZN14nsXPTCStubBase5Stub9Ev
> > lld-link: error: too many errors emitted, stopping now (use /ERRORLIMIT:0 to
> > see all errors)
> 
> clang might not be supporting toplevel asms that get used on mingw64:
> 
> http://dxr.mozilla.org/mozilla-central/source/xpcom/reflect/xptcall/md/win32/
> xptcstubs_x86_64_gnu.cpp#181
> 
> which is a little surprising to me, since clang supports toplevel asm on
> other platforms.  Maybe verify that the appropriate files in
> xpcom/reflect/xptcall/md/win32/ are getting compiled and the symbols therein?

The files got compiled and the symbols seem to therein as well. Picking up Jacek's suggested change and recompiling gives me:

strings xptcstubs_x86_64_gnu.o | grep _ZN14nsXPTCStubBase5Stub3Ev
_ZN14nsXPTCStubBase5Stub3Ev

etc. So, hrm.

Comment 40

[Martin Storsjö]

(In reply to Georg Koppen from comment #39)
> (In reply to Nathan Froyd [:froydnj] from comment #31)
> > (In reply to Georg Koppen from comment #30)
> > > 2) When linking libxul:
> > > 
> > > lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol:
> > > _ZN14nsXPTCStubBase5Stub3Ev
> > > lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol:
> > > _ZN14nsXPTCStubBase5Stub4Ev
> > > lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol:
> > > _ZN14nsXPTCStubBase5Stub5Ev
> > > lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol:
> > > _ZN14nsXPTCStubBase5Stub6Ev
> > > lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol:
> > > _ZN14nsXPTCStubBase5Stub7Ev
> > > lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol:
> > > _ZN14nsXPTCStubBase5Stub8Ev
> > > lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol:
> > > _ZN14nsXPTCStubBase5Stub9Ev
> > > lld-link: error: too many errors emitted, stopping now (use /ERRORLIMIT:0 to
> > > see all errors)
> > 
> > clang might not be supporting toplevel asms that get used on mingw64:
> > 
> > http://dxr.mozilla.org/mozilla-central/source/xpcom/reflect/xptcall/md/win32/
> > xptcstubs_x86_64_gnu.cpp#181
> > 
> > which is a little surprising to me, since clang supports toplevel asm on
> > other platforms.  Maybe verify that the appropriate files in
> > xpcom/reflect/xptcall/md/win32/ are getting compiled and the symbols therein?
> 
> The files got compiled and the symbols seem to therein as well. Picking up
> Jacek's suggested change and recompiling gives me:
> 
> strings xptcstubs_x86_64_gnu.o | grep _ZN14nsXPTCStubBase5Stub3Ev
> _ZN14nsXPTCStubBase5Stub3Ev
> 
> etc. So, hrm.

Can you quote the full linking command in this case, and what does "llvm-nm xptcstubs_x86_64_gnu.o | grep _ZN14nsXPTCStubBase5Stub3Ev" show, plus the same for xptcall.o?

Comment 41

[Georg Koppen]

Attachment: libxul_link_failure

In reply to Martin Storsjö from comment #40)
> (In reply to Georg Koppen from comment #39)
> > (In reply to Nathan Froyd [:froydnj] from comment #31)
> > > (In reply to Georg Koppen from comment #30)
> > > > 2) When linking libxul:
> > > > 
> > > > lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol:
> > > > _ZN14nsXPTCStubBase5Stub3Ev
> > > > lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol:
> > > > _ZN14nsXPTCStubBase5Stub4Ev
> > > > lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol:
> > > > _ZN14nsXPTCStubBase5Stub5Ev
> > > > lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol:
> > > > _ZN14nsXPTCStubBase5Stub6Ev
> > > > lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol:
> > > > _ZN14nsXPTCStubBase5Stub7Ev
> > > > lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol:
> > > > _ZN14nsXPTCStubBase5Stub8Ev
> > > > lld-link: error: ../../xpcom/reflect/xptcall/xptcall.o: undefined symbol:
> > > > _ZN14nsXPTCStubBase5Stub9Ev
> > > > lld-link: error: too many errors emitted, stopping now (use /ERRORLIMIT:0 to
> > > > see all errors)
> > > 
> > > clang might not be supporting toplevel asms that get used on mingw64:
> > > 
> > > http://dxr.mozilla.org/mozilla-central/source/xpcom/reflect/xptcall/md/win32/
> > > xptcstubs_x86_64_gnu.cpp#181
> > > 
> > > which is a little surprising to me, since clang supports toplevel asm on
> > > other platforms.  Maybe verify that the appropriate files in
> > > xpcom/reflect/xptcall/md/win32/ are getting compiled and the symbols therein?
> > 
> > The files got compiled and the symbols seem to therein as well. Picking up
> > Jacek's suggested change and recompiling gives me:
> > 
> > strings xptcstubs_x86_64_gnu.o | grep _ZN14nsXPTCStubBase5Stub3Ev
> > _ZN14nsXPTCStubBase5Stub3Ev
> > 
> > etc. So, hrm.
> 
> Can you quote the full linking command in this case, and what does "llvm-nm
> xptcstubs_x86_64_gnu.o | grep _ZN14nsXPTCStubBase5Stub3Ev" show, plus the
> same for xptcall.o?

The linking log is attached. Running the `llvm-nm` command gives me:

llvm-nm xptcstubs_x86_64_gnu.o | grep _ZN14nsXPTCStubBase5Stub3Ev
0000005a t _ZN14nsXPTCStubBase5Stub3Ev

llvm-nm xptcall.o | grep _ZN14nsXPTCStubBase5Stub3Ev
U _ZN14nsXPTCStubBase5Stub3Ev

Comment 42

[Martin Storsjö]

(In reply to Georg Koppen from comment #41)
> Created attachment 8948987 [details]
> libxul_link_failure
> 
> The linking log is attached. Running the `llvm-nm` command gives me:
> 
> llvm-nm xptcstubs_x86_64_gnu.o | grep _ZN14nsXPTCStubBase5Stub3Ev
> 0000005a t _ZN14nsXPTCStubBase5Stub3Ev
> 
> llvm-nm xptcall.o | grep _ZN14nsXPTCStubBase5Stub3Ev
> U _ZN14nsXPTCStubBase5Stub3Ev

Oh, now I see. The lowercase 't' means that the symbol, while present, is local to the file (like "static" in C). So apparently clang/LLVM doesn't interpret the ".globl" keyword when present in inline assembly like this. (I did test assemble such a snippet yesterday but failed to notice the lowercase 't'.)

That makes the issue clear at least - I'll try to see if I can figure out a way around it.

Comment 43

[Martin Storsjö]

The issue seems to be with the .scl 3 directive, which contrary to the earlier .globl says that it's a private symbol. If that's removed or changed into .scl 2, the symbol becomes external as it should.

Comment 44

[Georg Koppen]

(In reply to Martin Storsjö from comment #43)
> The issue seems to be with the .scl 3 directive, which contrary to the
> earlier .globl says that it's a private symbol. If that's removed or changed
> into .scl 2, the symbol becomes external as it should.

That helps, yes. Now I am seeing:

lld-link: error: ../../xpcom/reflect/xptcall/md/win32/xptcinvoke_x86_64.o: undefined symbol: XPTC__InvokebyIndex

running `llvm-nm` on it confirms that

llvm-nm xptcinvoke_x86_64.o | grep XPTC__InvokebyIndex
         U XPTC__InvokebyIndex

Hm, I wonder where this is supposed to come from...

Comment 45

[Jacek Caban]

https://dxr.mozilla.org/mozilla-central/source/xpcom/reflect/xptcall/md/win32/xptcinvoke_asm_x86_64_gnu.s

It has the same problem, it uses .scl 3.

Comment 46

[Georg Koppen]

(In reply to Jacek Caban from comment #45)
> https://dxr.mozilla.org/mozilla-central/source/xpcom/reflect/xptcall/md/
> win32/xptcinvoke_asm_x86_64_gnu.s
> 
> It has the same problem, it uses .scl 3.

Yeah, sorry. I assumed that dxr would index assembly as well and got confused when nothing showed up. So, the good news is that after working around that issue everything gets built and linked and packaging up works as well.

However, the resulting binaries are not running. I just get a "firefox.exe is not a valid win32 application"-error. Replacing that .exe with a "good" old one bypasses that but just shows that libraries are affected, too. I need to find out how to debug that first in order to make progress on that part (ideas are very much appreciated).

Meanwhile, as another next step, I'll try to build mozilla-central with the clang-toolchain. First, without Stylo enabled. Once that works I'll see how we do on the Stylo front.

Comment 47

[Martin Storsjö]

(In reply to Georg Koppen from comment #46)
> (In reply to Jacek Caban from comment #45)
> > https://dxr.mozilla.org/mozilla-central/source/xpcom/reflect/xptcall/md/
> > win32/xptcinvoke_asm_x86_64_gnu.s
> > 
> > It has the same problem, it uses .scl 3.
> 
> Yeah, sorry. I assumed that dxr would index assembly as well and got
> confused when nothing showed up. So, the good news is that after working
> around that issue everything gets built and linked and packaging up works as
> well.
> 
> However, the resulting binaries are not running. I just get a "firefox.exe
> is not a valid win32 application"-error.
> 
> Replacing that .exe with a "good"
> old one bypasses that but just shows that libraries are affected, too. I
> need to find out how to debug that first in order to make progress on that
> part (ideas are very much appreciated).
> 
> Meanwhile, as another next step, I'll try to build mozilla-central with the
> clang-toolchain. First, without Stylo enabled. Once that works I'll see how
> we do on the Stylo front.

Hmm, I haven't ran into that issue so I don't have any good clues about it. (Although, I've done most of my debugging of broken binaries in wine, so maybe some of the issues I've seen would have mapped to this error message.)

One issue I have seen though is cases where constructors have crashed before execution otherwise even gets to the normal main function. One way to debug that, try to find the smallest/simplest DLL that exhibits the same issue. You can try a small dummy executable that does e.g. LoadLibrary("foo.dll") to pinpoint DLLs that fail to load and those that loads fine.

The reason for those crashes were constructors that failed. If you have global structs that contain pointers to dllimported variables, clang initializes them differently than gcc and msvc. With gcc and msvc, the rest of the struct (the non-dllimported members) is statically inited like any other global struct, but a constructor is emitted that fills in the dllimported pointers. Clang on the other hand initializes the whole struct via a constructor if a single field needs to be initialized at runtime. In Qt, this caused race conditions with constructor initializations, causing crashes when a DLL is loaded.

Not sure at all if this is relevant to what you're running into though.

Comment 48

[(Away)]

(In reply to Georg Koppen from comment #46)
> However, the resulting binaries are not running. I just get a "firefox.exe
> is not a valid win32 application"-error. Replacing that .exe with a "good"
> old one bypasses that but just shows that libraries are affected, too. I
> need to find out how to debug that first in order to make progress on that
> part (ideas are very much appreciated).

I would run this under a debugger with the "show loader snaps" output enabled: https://blogs.msdn.microsoft.com/junfeng/2006/11/20/debugging-loadlibrary-failures/

And if it doesn't even get far enough to start under the debugger at all, then I would dump the DOS/PE headers on firefox.exe to be sure they're reasonable.

Comment 49

[Martin Storsjö]

(In reply to Martin Storsjö from comment #47)
> (In reply to Georg Koppen from comment #46)
> > (In reply to Jacek Caban from comment #45)
> > > https://dxr.mozilla.org/mozilla-central/source/xpcom/reflect/xptcall/md/
> > > win32/xptcinvoke_asm_x86_64_gnu.s
> > > 
> > > It has the same problem, it uses .scl 3.
> > 
> > Yeah, sorry. I assumed that dxr would index assembly as well and got
> > confused when nothing showed up. So, the good news is that after working
> > around that issue everything gets built and linked and packaging up works as
> > well.
> > 
> > However, the resulting binaries are not running. I just get a "firefox.exe
> > is not a valid win32 application"-error.
> > 
> > Replacing that .exe with a "good"
> > old one bypasses that but just shows that libraries are affected, too. I
> > need to find out how to debug that first in order to make progress on that
> > part (ideas are very much appreciated).
> > 
> > Meanwhile, as another next step, I'll try to build mozilla-central with the
> > clang-toolchain. First, without Stylo enabled. Once that works I'll see how
> > we do on the Stylo front.
> 
> Hmm, I haven't ran into that issue so I don't have any good clues about it.
> (Although, I've done most of my debugging of broken binaries in wine, so
> maybe some of the issues I've seen would have mapped to this error message.)

It does however sound a lot like a runtime linker error flat out refusing to load the binaries, so it might not even get so far that it could crash at all.

In that case, try reducing a DLL as much as possible, until it stops exhibiting the problem. (When tested with a plain LoadLibrary call, the DLL itself can be total nonsense and doesn't need to actually work in doing anything.) I presume that a minimal DLL created with "<triplet>-clang -shared -o foo.dll minimal.o" loads correctly?

I've done much of my testing with -Wl,-s though, to strip the output and omit the debug info - you might want to try adding that when linking, in case the dwarf debug info is what causes the problem.

Comment 50

[Martin Storsjö]

(In reply to Georg Koppen from comment #25)
> 
> lld-link: error: ../nspr/pr/prlink.o: undefined symbol: strcmpi
> lld-link: error: libmsvcrt.a(ucrtbase.dll): undefined symbol: _strcmpi
> lld-link: error: libmsvcrt.a(ucrtbase.dll): undefined symbol: strcmpi
> clang-7.0: error: linker command failed with exit code 1 (use -v to see
> invocation)

FWIW, a fix for this should be in the latest git master of mingw-w64 now.

Comment 51

[Georg Koppen]

(In reply to Martin Storsjö from comment #50)
> (In reply to Georg Koppen from comment #25)
> > 
> > lld-link: error: ../nspr/pr/prlink.o: undefined symbol: strcmpi
> > lld-link: error: libmsvcrt.a(ucrtbase.dll): undefined symbol: _strcmpi
> > lld-link: error: libmsvcrt.a(ucrtbase.dll): undefined symbol: strcmpi
> > clang-7.0: error: linker command failed with exit code 1 (use -v to see
> > invocation)
> 
> FWIW, a fix for this should be in the latest git master of mingw-w64 now.

Thanks, that helped. Here is another issue using the ucrtbase.dll:

lld-link: error: ../../media/libcubeb/src/cubeb_winmm.o: undefined symbol: __imp__snprintf_s

Comment 52

[Martin Storsjö]

(In reply to Georg Koppen from comment #51)
> 
> Thanks, that helped. Here is another issue using the ucrtbase.dll:
> 
> lld-link: error: ../../media/libcubeb/src/cubeb_winmm.o: undefined symbol:
> __imp__snprintf_s

That looks like an object file that hasn't been rebuilt after switching to ucrtbase, or it hasn't been built with the right ucrtbase aware version of stdio.h/stdio_s.h. (This function should have been handled in mingw-w64 for ucrtbase since a month or two.)

Comment 53

[Georg Koppen]

(In reply to Martin Storsjö from comment #52)
> (In reply to Georg Koppen from comment #51)
> > 
> > Thanks, that helped. Here is another issue using the ucrtbase.dll:
> > 
> > lld-link: error: ../../media/libcubeb/src/cubeb_winmm.o: undefined symbol:
> > __imp__snprintf_s
> 
> That looks like an object file that hasn't been rebuilt after switching to
> ucrtbase, or it hasn't been built with the right ucrtbase aware version of
> stdio.h/stdio_s.h. (This function should have been handled in mingw-w64 for
> ucrtbase since a month or two.)

No, this happens after a clobber build. It turns out that I can't build with mingw-w64 master right now, anyway, as it is broken since 537fbbf3e849c2ee1d3491907f58bc2f50d08e44. Jacek, your update is causing:

x86_64-w64-mingw32-widl: ../src/typetree.h:274: type_alias_get_aliasee: Assertion `type_is_alias(type)' failed.
Aborted
Makefile:109: die Regel für Ziel „typelib_done“ scheiterte
make[5]: *** [typelib_done] Fehler 134
make[5]: Verzeichnis „/home/thomas/Arbeit/Tor/debugging/21777/obj-mingw/accessible/interfaces/ia2“ wird verlassen
/home/thomas/Arbeit/Tor/debugging/21777/config/recurse.mk:79: die Regel für Ziel „accessible/interfaces/ia2/export“ scheiterte
make[4]: *** [accessible/interfaces/ia2/export] Fehler 2

So, I cherry-picked a78ca32ab0b7af2c7098ed542375ff72b67ce86b on top of 66fab9591c250ade399e9fe91ceda239a735649c as it seemed to me there were no other urctbase related commits which are needed.

Comment 54

[Martin Storsjö]

(In reply to Georg Koppen from comment #53)
> (In reply to Martin Storsjö from comment #52)
> > (In reply to Georg Koppen from comment #51)
> > > 
> > > Thanks, that helped. Here is another issue using the ucrtbase.dll:
> > > 
> > > lld-link: error: ../../media/libcubeb/src/cubeb_winmm.o: undefined symbol:
> > > __imp__snprintf_s
> > 
> > That looks like an object file that hasn't been rebuilt after switching to
> > ucrtbase, or it hasn't been built with the right ucrtbase aware version of
> > stdio.h/stdio_s.h. (This function should have been handled in mingw-w64 for
> > ucrtbase since a month or two.)
> 
> No, this happens after a clobber build. It turns out that I can't build with
> mingw-w64 master right now, anyway, as it is broken since
> 537fbbf3e849c2ee1d3491907f58bc2f50d08e44. Jacek, your update is causing:
> 
> x86_64-w64-mingw32-widl: ../src/typetree.h:274: type_alias_get_aliasee:
> Assertion `type_is_alias(type)' failed.
> Aborted
> Makefile:109: die Regel für Ziel „typelib_done“ scheiterte
> make[5]: *** [typelib_done] Fehler 134
> make[5]: Verzeichnis
> „/home/thomas/Arbeit/Tor/debugging/21777/obj-mingw/accessible/interfaces/
> ia2“ wird verlassen
> /home/thomas/Arbeit/Tor/debugging/21777/config/recurse.mk:79: die Regel für
> Ziel „accessible/interfaces/ia2/export“ scheiterte
> make[4]: *** [accessible/interfaces/ia2/export] Fehler 2
> 
> So, I cherry-picked a78ca32ab0b7af2c7098ed542375ff72b67ce86b on top of
> 66fab9591c250ade399e9fe91ceda239a735649c as it seemed to me there were no
> other urctbase related commits which are needed.

Indeed, none of the other ones inbetween should be relevant. And the commit 66fab9591c250ade399e9fe91ceda239a735649c should be exactly the one that implements _snprintf_s among a lot other *_s functions. So in this case, it'd be interesting to see the preprocessor output from when building cubeb_winmm.o - in particular any declarations referring to this function and where they stem from.

But this is of course only a sidetrack, since if building otherwise succeeds with the normal msvcrt.dll but the binaries can't be loaded, that's obviously the much larger issue.

Comment 55

[Jacek Caban]

(In reply to Georg Koppen from comment #53)
> (In reply to Martin Storsjö from comment #52)
> > (In reply to Georg Koppen from comment #51)
> > > 
> > > Thanks, that helped. Here is another issue using the ucrtbase.dll:
> > > 
> > > lld-link: error: ../../media/libcubeb/src/cubeb_winmm.o: undefined symbol:
> > > __imp__snprintf_s
> > 
> > That looks like an object file that hasn't been rebuilt after switching to
> > ucrtbase, or it hasn't been built with the right ucrtbase aware version of
> > stdio.h/stdio_s.h. (This function should have been handled in mingw-w64 for
> > ucrtbase since a month or two.)
> 
> No, this happens after a clobber build. It turns out that I can't build with
> mingw-w64 master right now, anyway, as it is broken since
> 537fbbf3e849c2ee1d3491907f58bc2f50d08e44. Jacek, your update is causing:
> 
> x86_64-w64-mingw32-widl: ../src/typetree.h:274: type_alias_get_aliasee:
> Assertion `type_is_alias(type)' failed.
> Aborted
> Makefile:109: die Regel für Ziel „typelib_done“ scheiterte
> make[5]: *** [typelib_done] Fehler 134
> make[5]: Verzeichnis
> „/home/thomas/Arbeit/Tor/debugging/21777/obj-mingw/accessible/interfaces/
> ia2“ wird verlassen
> /home/thomas/Arbeit/Tor/debugging/21777/config/recurse.mk:79: die Regel für
> Ziel „accessible/interfaces/ia2/export“ scheiterte
> make[4]: *** [accessible/interfaces/ia2/export] Fehler 2
> 
> So, I cherry-picked a78ca32ab0b7af2c7098ed542375ff72b67ce86b on top of
> 66fab9591c250ade399e9fe91ceda239a735649c as it seemed to me there were no
> other urctbase related commits which are needed.

I can't reproduce it here with GCC build, but I have --disable-accessibility in my mozconfig. Without it, it fails for another reason (I think it's a known problem with widl, Tom will know better).

Could you please post the exact widl command that fails?

Comment 56

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

(In reply to Jacek Caban from comment #55)
> I can't reproduce it here with GCC build, but I have --disable-accessibility
> in my mozconfig. Without it, it fails for another reason (I think it's a
> known problem with widl, Tom will know better).

Bug 1430149 is our tracking, upstream is https://sourceforge.net/p/mingw-w64/bugs/648/

Comment 57

[Georg Koppen]

(In reply to Jacek Caban from comment #55)
> (In reply to Georg Koppen from comment #53)
> > (In reply to Martin Storsjö from comment #52)
> > > (In reply to Georg Koppen from comment #51)
> > > > 
> > > > Thanks, that helped. Here is another issue using the ucrtbase.dll:
> > > > 
> > > > lld-link: error: ../../media/libcubeb/src/cubeb_winmm.o: undefined symbol:
> > > > __imp__snprintf_s
> > > 
> > > That looks like an object file that hasn't been rebuilt after switching to
> > > ucrtbase, or it hasn't been built with the right ucrtbase aware version of
> > > stdio.h/stdio_s.h. (This function should have been handled in mingw-w64 for
> > > ucrtbase since a month or two.)
> > 
> > No, this happens after a clobber build. It turns out that I can't build with
> > mingw-w64 master right now, anyway, as it is broken since
> > 537fbbf3e849c2ee1d3491907f58bc2f50d08e44. Jacek, your update is causing:
> > 
> > x86_64-w64-mingw32-widl: ../src/typetree.h:274: type_alias_get_aliasee:
> > Assertion `type_is_alias(type)' failed.
> > Aborted
> > Makefile:109: die Regel für Ziel „typelib_done“ scheiterte
> > make[5]: *** [typelib_done] Fehler 134
> > make[5]: Verzeichnis
> > „/home/thomas/Arbeit/Tor/debugging/21777/obj-mingw/accessible/interfaces/
> > ia2“ wird verlassen
> > /home/thomas/Arbeit/Tor/debugging/21777/config/recurse.mk:79: die Regel für
> > Ziel „accessible/interfaces/ia2/export“ scheiterte
> > make[4]: *** [accessible/interfaces/ia2/export] Fehler 2
> > 
> > So, I cherry-picked a78ca32ab0b7af2c7098ed542375ff72b67ce86b on top of
> > 66fab9591c250ade399e9fe91ceda239a735649c as it seemed to me there were no
> > other urctbase related commits which are needed.
> 
> I can't reproduce it here with GCC build, but I have --disable-accessibility
> in my mozconfig. Without it, it fails for another reason (I think it's a
> known problem with widl, Tom will know better).
> 
> Could you please post the exact widl command that fails?

for idl in /home/thomas/Arbeit/Tor/debugging/21777/accessible/interfaces/ia2/IA2Typelib.idl; do \
	x86_64-w64-mingw32-widl --win64 -m64 -app_config -I /home/thomas/Arbeit/Tor/debugging/21777/other-licenses/ia2 -D _MIDL_DECLARE_WIREM_HANDLE -dlldata `basename $idl .idl`.c -Oicf $idl; \
done

Comment 58

[Georg Koppen]

(In reply to David Major [:dmajor] from comment #48)
> (In reply to Georg Koppen from comment #46)
> > However, the resulting binaries are not running. I just get a "firefox.exe
> > is not a valid win32 application"-error. Replacing that .exe with a "good"
> > old one bypasses that but just shows that libraries are affected, too. I
> > need to find out how to debug that first in order to make progress on that
> > part (ideas are very much appreciated).
> 
> I would run this under a debugger with the "show loader snaps" output
> enabled:
> https://blogs.msdn.microsoft.com/junfeng/2006/11/20/debugging-loadlibrary-
> failures/

Thanks. I've started looking into that and it seems it does indeed not get even that far. While a "normal" firefox.exe compiled with mingw-w64/GCC gives me output like

CommandLine: firefox-alpha.exe
Symbol search path is: srv*
Executable search path is: 
ModLoad: 00000000`00a60000 00000000`00ac0000   image00000000`00a60000
ModLoad: 00000000`77130000 00000000`772da000   ntdll.dll
0b28:0b24 @ 00199104 - LdrpInitializeProcess - INFO: Beginning execution of firefox-alpha.exe (C:\Users\AMO_Test\firefox-alpha.exe)
	Current directory: C:\Users\AMO_Test\
	Search path: C:\Users\AMO_Test;;C:\Windows\system32;C:\Windows\system;C:\Windows;.;c:\Program Files (x86)\Windows Kits\10\Debuggers\x64;C:\ProgramData\Oracle\Java\javapath;C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0\;C:\Program Files (x86)\ATI Technologies\ATI.ACE\Core-Static;C:\Pro
0b28:0b24 @ 00199104 - LdrpInitializeTls - INFO: DLL "C:\Users\AMO_Test\firefox-alpha.exe" has TLS information at 0000000000A85020

The one compiled with clang just results in

Cannot execute 'firefox.exe', Win32 error 0n193
    "%1 is not a valid Win32 application."
 
> And if it doesn't even get far enough to start under the debugger at all,
> then I would dump the DOS/PE headers on firefox.exe to be sure they're
> reasonable.

It seems I need to look closer at that...

Comment 59

[Martin Storsjö]

(In reply to Georg Koppen from comment #58)
> The one compiled with clang just results in
> 
> Cannot execute 'firefox.exe', Win32 error 0n193
>     "%1 is not a valid Win32 application."
>  
> > And if it doesn't even get far enough to start under the debugger at all,
> > then I would dump the DOS/PE headers on firefox.exe to be sure they're
> > reasonable.
> 
> It seems I need to look closer at that...

As I think I hinted before - try linking with -Wl,-s to force stripping (which in lld disables debug mode, avoids outputting debug info and makes some other minor details work differently) - I've done most testing in that mode, and it could be related to dwarf debug info.

Comment 60

[Georg Koppen]

(In reply to Martin Storsjö from comment #59)
> (In reply to Georg Koppen from comment #58)
> > The one compiled with clang just results in
> > 
> > Cannot execute 'firefox.exe', Win32 error 0n193
> >     "%1 is not a valid Win32 application."
> >  
> > > And if it doesn't even get far enough to start under the debugger at all,
> > > then I would dump the DOS/PE headers on firefox.exe to be sure they're
> > > reasonable.
> > 
> > It seems I need to look closer at that...
> 
> As I think I hinted before - try linking with -Wl,-s to force stripping
> (which in lld disables debug mode, avoids outputting debug info and makes
> some other minor details work differently) - I've done most testing in that
> mode, and it could be related to dwarf debug info.

Okay, linking with -Wl,-s breaks the build as there seems to be a check when building xul.dll making sure that NSModules are ordered properly and force stripping leaves no symbols behind:

test "$(x86_64-w64-mingw32-nm -g xul.dll | grep _NSModule$ | grep -vw refptr | sort | sed -n 's/^.* _*\([^ ]*\)$/\1/;1p;$p' | xargs echo)" != "start_kPStaticModules_NSModule end_kPStaticModules_NSModule" && echo "NSModules are not ordered appropriately" && exit 1 || exit 0
NSModules are not ordered appropriately

I am not sure if we can disable that for our case but doing so allows me finally to start the resulting browser. On a Windows 8 system I can see a browser window showing up and surfing with this clang-based Tor Browser seems to work, yay!

Comment 61

[Martin Storsjö]

(In reply to Georg Koppen from comment #60)
> (In reply to Martin Storsjö from comment #59)
> > (In reply to Georg Koppen from comment #58)
> > > The one compiled with clang just results in
> > > 
> > > Cannot execute 'firefox.exe', Win32 error 0n193
> > >     "%1 is not a valid Win32 application."
> > >  
> > > > And if it doesn't even get far enough to start under the debugger at all,
> > > > then I would dump the DOS/PE headers on firefox.exe to be sure they're
> > > > reasonable.
> > > 
> > > It seems I need to look closer at that...
> > 
> > As I think I hinted before - try linking with -Wl,-s to force stripping
> > (which in lld disables debug mode, avoids outputting debug info and makes
> > some other minor details work differently) - I've done most testing in that
> > mode, and it could be related to dwarf debug info.
> 
> Okay, linking with -Wl,-s breaks the build as there seems to be a check when
> building xul.dll making sure that NSModules are ordered properly and force
> stripping leaves no symbols behind:
> 
> test "$(x86_64-w64-mingw32-nm -g xul.dll | grep _NSModule$ | grep -vw refptr
> | sort | sed -n 's/^.* _*\([^ ]*\)$/\1/;1p;$p' | xargs echo)" !=
> "start_kPStaticModules_NSModule end_kPStaticModules_NSModule" && echo
> "NSModules are not ordered appropriately" && exit 1 || exit 0
> NSModules are not ordered appropriately
> 
> I am not sure if we can disable that for our case but doing so allows me
> finally to start the resulting browser. On a Windows 8 system I can see a
> browser window showing up and surfing with this clang-based Tor Browser
> seems to work, yay!

Wow, that's excellent news!

With things working in this configuration, I guess the next step is to try to make a minimal example of linking a binary that refuses to load if linked with debug info. I've done most of my testing on Windows 10, but with a bit of tweaking, I managed to build a hello world exe with debug info which still also loads on XP. (To achieve that I tweaked lld to set the major OS version to 5.) So it shouldn't be an issue in Windows 10 vs older versions at least. It's probably something that only appears once you link a large/complex enough binary...

I guess I can try to build some of the larger projects I've tested with debug info enabled to see if I happen to run into the same, or if it's something specific to the firefox codebase.

(In reply to Martin Storsjö from comment #54)
> > So, I cherry-picked a78ca32ab0b7af2c7098ed542375ff72b67ce86b on top of
> > 66fab9591c250ade399e9fe91ceda239a735649c as it seemed to me there were no
> > other urctbase related commits which are needed.
> 
> Indeed, none of the other ones inbetween should be relevant. And the commit
> 66fab9591c250ade399e9fe91ceda239a735649c should be exactly the one that
> implements _snprintf_s among a lot other *_s functions. So in this case,
> it'd be interesting to see the preprocessor output from when building
> cubeb_winmm.o - in particular any declarations referring to this function
> and where they stem from.

Since the msvcrt.dll based build seems to work now - it'd be intresteing to know what other details there are left for ucrtbase.dll. So if you have time to look at that again, I'd love to see the preprocessor output for the file that ends up referring to _snprintf_s.

Comment 62

[Martin Storsjö]

(In reply to Martin Storsjö from comment #61)
> I guess I can try to build some of the larger projects I've tested with
> debug info enabled to see if I happen to run into the same, or if it's
> something specific to the firefox codebase.

I built Qt and a demo app with debug info enabled into a 296 MB exe, which ran fine on Windows 10, so it's not trivially reproducible at least.

Or do your binaries end up even larger, possibly over some potentially problematic size like 2 GB?

Comment 63

[Georg Koppen]

(In reply to Martin Storsjö from comment #62)
> (In reply to Martin Storsjö from comment #61)
> > I guess I can try to build some of the larger projects I've tested with
> > debug info enabled to see if I happen to run into the same, or if it's
> > something specific to the firefox codebase.
> 
> I built Qt and a demo app with debug info enabled into a 296 MB exe, which
> ran fine on Windows 10, so it's not trivially reproducible at least.
> 
> Or do your binaries end up even larger, possibly over some potentially
> problematic size like 2 GB?

No, the binaries are way smaller than that, less than 1 MB. I am happy to help and test things but I don't have time right now to dig deeper into this issue (although I'd like to see it solved eventually as this would mean one patch less we need to write/ship). I have not forgotten the ucrtbase issue either and will get back to you with the info once I get to redo the things so that I get all the information needed.

Regarding compiling m-c with the cross-compiler I run already early in three issues, two of them need to get fixed upstream. The first one is a compiler crash that still happens with the latest clang code. I am waiting for the LLVM folks giving me a bugzilla account to report that one.

The other one is:

 1:22.89 In file included from /home/thomas/Arbeit/Tor/debugging/21777/js/src/ctypes/CTypes.cpp:7
 1:22.89 In file included from /home/thomas/Arbeit/Tor/debugging/21777/js/src/ctypes/CTypes.h:18:
 1:22.89 In file included from /home/thomas/Arbeit/Tor/debugging/21777/obj-mingw/dist/include/js/GCHashTable.h:10:
 1:22.89 In file included from /home/thomas/Arbeit/Tor/debugging/21777/obj-mingw/dist/include/mozilla/Maybe.h:20:
 1:22.89 In file included from /home/thomas/Arbeit/Tor/debugging/win59/clang_toolchain/x86_64-w64-mingw32/include/c++/v1/ostream:138:
 1:22.89 In file included from /home/thomas/Arbeit/Tor/debugging/win59/clang_toolchain/x86_64-w64-mingw32/include/c++/v1/ios:216:
 1:22.89 In file included from /home/thomas/Arbeit/Tor/debugging/win59/clang_toolchain/x86_64-w64-mingw32/include/c++/v1/__locale:23:
 1:22.89 In file included from /home/thomas/Arbeit/Tor/debugging/win59/clang_toolchain/x86_64-w64-mingw32/include/c++/v1/support/win32/locale_win32.h:16:
 1:22.89 In file included from /home/thomas/Arbeit/Tor/debugging/win59/clang_toolchain/x86_64-w64-mingw32/include/xlocinfo.h:11:
 1:22.89 /home/thomas/Arbeit/Tor/debugging/win59/clang_toolchain/x86_64-w64-mingw32/include/yvals.h:167:14: error: redeclaration of C++ built-in type 'bool'
 1:22.89 typedef bool _Bool;
 1:22.89              ^
 1:22.89 /home/thomas/Arbeit/Tor/debugging/win59/clang_toolchain/lib/clang/7.0.0/include/stdbool.h:36:15: note: expanded from macro '_Bool'
 1:22.89 #define _Bool bool
 1:22.89               ^

Presumably this issue shows up since bug 1367795 got fixed. Jacek, Martin: I am deferring to you for where and how this should actually get fixed.

Comment 64

[Martin Storsjö]

(In reply to Georg Koppen from comment #63)
> The other one is:
> 
>  1:22.89 In file included from
> /home/thomas/Arbeit/Tor/debugging/21777/js/src/ctypes/CTypes.cpp:7
>  1:22.89 In file included from
> /home/thomas/Arbeit/Tor/debugging/21777/js/src/ctypes/CTypes.h:18:
>  1:22.89 In file included from
> /home/thomas/Arbeit/Tor/debugging/21777/obj-mingw/dist/include/js/
> GCHashTable.h:10:
>  1:22.89 In file included from
> /home/thomas/Arbeit/Tor/debugging/21777/obj-mingw/dist/include/mozilla/Maybe.
> h:20:
>  1:22.89 In file included from
> /home/thomas/Arbeit/Tor/debugging/win59/clang_toolchain/x86_64-w64-mingw32/
> include/c++/v1/ostream:138:
>  1:22.89 In file included from
> /home/thomas/Arbeit/Tor/debugging/win59/clang_toolchain/x86_64-w64-mingw32/
> include/c++/v1/ios:216:
>  1:22.89 In file included from
> /home/thomas/Arbeit/Tor/debugging/win59/clang_toolchain/x86_64-w64-mingw32/
> include/c++/v1/__locale:23:
>  1:22.89 In file included from
> /home/thomas/Arbeit/Tor/debugging/win59/clang_toolchain/x86_64-w64-mingw32/
> include/c++/v1/support/win32/locale_win32.h:16:
>  1:22.89 In file included from
> /home/thomas/Arbeit/Tor/debugging/win59/clang_toolchain/x86_64-w64-mingw32/
> include/xlocinfo.h:11:
>  1:22.89
> /home/thomas/Arbeit/Tor/debugging/win59/clang_toolchain/x86_64-w64-mingw32/
> include/yvals.h:167:14: error: redeclaration of C++ built-in type 'bool'
>  1:22.89 typedef bool _Bool;
>  1:22.89              ^
>  1:22.89
> /home/thomas/Arbeit/Tor/debugging/win59/clang_toolchain/lib/clang/7.0.0/
> include/stdbool.h:36:15: note: expanded from macro '_Bool'
>  1:22.89 #define _Bool bool
>  1:22.89               ^
> 
> Presumably this issue shows up since bug 1367795 got fixed. Jacek, Martin: I
> am deferring to you for where and how this should actually get fixed.

This can be fixed within mingw-w64 at least; I've seen a patch for that once but didn't pursue upstreaming it since I didn't know how to reproduce the issue; now I can boil it down to this:

#include <stdbool.h>
#include <iostream>

With libc++, iostream includes xlocinfo.h and yvals.h, which together with stdbool.h cause this issue. If included in the opposite order, the issue vanishes.

I'll write a more thorough commit message for this patch and try to upstream it to mingw-w64.

Comment 65

[Georg Koppen]

(In reply to Georg Koppen from comment #63)
> Regarding compiling m-c with the cross-compiler I run already early in three
> issues, two of them need to get fixed upstream. The first one is a compiler
> crash that still happens with the latest clang code. I am waiting for the
> LLVM folks giving me a bugzilla account to report that one.

FWIW, that's https://bugs.llvm.org/show_bug.cgi?id=36487 now.

Comment 66

[Martin Storsjö]

(In reply to Martin Storsjö from comment #64)
> (In reply to Georg Koppen from comment #63)
> > The other one is:
> > 
> > /home/thomas/Arbeit/Tor/debugging/win59/clang_toolchain/x86_64-w64-mingw32/
> > include/yvals.h:167:14: error: redeclaration of C++ built-in type 'bool'
> >  1:22.89 typedef bool _Bool;
> 
> I'll write a more thorough commit message for this patch and try to upstream
> it to mingw-w64.

This one is fixed upstream in mingw-w64 now.

Comment 67

[Georg Koppen]

Thanks, that worked for me. I am hitting issues with inlining after a bunch of different libraries, which Firefox uses, got updated after ESR 52. I am still investigating but here are they in case anybody has hit those before and/or has ideas:

1) There is a bunch of `static INLINE foo()` declarations which break, e.g.:

 5:40.03 /home/thomas/Arbeit/Tor/debugging/21777/media/libvpx/libvpx/vp8/common/findnearmv.h:24:8: error: cannot combine with previous 'static' declaration specifier
 5:40.03 static INLINE void mv_bias(int refmb_ref_frame_sign_bias, int refframe,
 5:40.03        ^
 5:40.03 /home/thomas/Arbeit/Tor/debugging/21777/media/libvpx/config/win/x64/./vpx_config.h:12:21: note: expanded from macro 'INLINE'
 5:40.03 #define INLINE      __forceinline
 5:40.03                     ^
 5:40.03 /home/thomas/Arbeit/Tor/debugging/win59/clang_toolchain/x86_64-w64-mingw32/include/_mingw.h:266:23: note: expanded from macro '__forceinline'
 5:40.03 #define __forceinline extern __inline__ __attribute__((__always_inline__,__gnu_inline__))
 5:40.03                       ^

2) libpng is unhappy:

 0:01.77 /home/thomas/Arbeit/Tor/debugging/win59/clang_toolchain/bin/x86_64-w64-mingw32-clang --sysroot /home/thomas/Arbeit/Tor/debugging/win59/clang_toolchain/x86_64-w64-mingw32 -std=gnu99 -mwindows -o Unified_c_media_libpng0.o -c -DNDEBUG=1 -DTRIMMED=1 -DMOZ_PNG_USE_INTEL_SSE -DFT_CONFIG_OPTION_USE_PNG -DSTATIC_EXPORTABLE_JS_API -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -I/home/thomas/Arbeit/Tor/debugging/21777/media/libpng -I/home/thomas/Arbeit/Tor/debugging/21777/obj-mingw/media/libpng -I/home/thomas/Arbeit/Tor/debugging/21777/obj-mingw/dist/include -I/home/thomas/Arbeit/Tor/debugging/21777/obj-mingw/dist/include/nspr -I/home/thomas/Arbeit/Tor/debugging/21777/obj-mingw/dist/include/nss -include /home/thomas/Arbeit/Tor/debugging/21777/obj-mingw/mozilla-config.h -DMOZILLA_CLIENT -Qunused-arguments -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Qunused-arguments -Wall -Wempty-body -Wignored-qualifiers -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wunreachable-code-return -Wclass-varargs -Wloop-analysis -Wstring-conversion -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-unknown-pragmas -Wno-unused-function -Wno-conversion-null -Wno-switch -Wno-enum-compare -Wno-gnu-zero-variadic-macro-arguments -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fno-strict-aliasing -mms-bitfields -fno-math-errno -pipe -g -O -fno-omit-frame-pointer -std=c89  -MD -MP -MF .deps/Unified_c_media_libpng0.o.pp   /home/thomas/Arbeit/Tor/debugging/21777/obj-mingw/media/libpng/Unified_c_media_libpng0.c
 0:01.81 In file included from /home/thomas/Arbeit/Tor/debugging/21777/obj-mingw/media/libpng/Unified_c_media_libpng0.c:2:
 0:01.81 In file included from /home/thomas/Arbeit/Tor/debugging/21777/media/libpng/intel/filter_sse2_intrinsics.c:15:
 0:01.81 In file included from /home/thomas/Arbeit/Tor/debugging/21777/media/libpng/intel/../pngpriv.h:44:
 0:01.81 /home/thomas/Arbeit/Tor/debugging/win59/clang_toolchain/x86_64-w64-mingw32/include/stdlib.h:367:3: error: unknown type name 'inline'
 0:01.81   __CRT_INLINE __MINGW_ATTRIB_NORETURN void  __cdecl _Exit(int status)
 0:01.81   ^
 0:01.81 /home/thomas/Arbeit/Tor/debugging/win59/clang_toolchain/x86_64-w64-mingw32/include/_mingw.h:93:31: note: expanded from macro '__CRT_INLINE'
 0:01.81 #  define __CRT_INLINE extern inline __attribute__((__gnu_inline__))
 0:01.81                               ^

Error 2) is probably due to forcing -std=c89 over in bug 1371266 which gives us now something like

if CONFIG['CC_TYPE'] in ('clang', 'gcc'):
    CFLAGS += ['-std=c89']

Comment 68

[(Away)]

(In reply to Georg Koppen from comment #67)

Bug 1412889 may address some of the libvpx and libaom __forceinline issues (whenever we get around to merging new upstreams).

Comment 69

[Georg Koppen]

(In reply to David Major [:dmajor] from comment #68)
> (In reply to Georg Koppen from comment #67)
> 
> Bug 1412889 may address some of the libvpx and libaom __forceinline issues
> (whenever we get around to merging new upstreams).

Yep. I did the same before reading your comment looking at the mingw32 config (where `inline` is used). Not sure, though, whether there is a deeper issue visible: Martin: shouldn't the clang cross-compiler use the mingw32 config provided in libvpx and libaom instead of the x64 one aimed at MSVC? Or does that  not matter at the end if the code compiles and links properly?

Comment 70

[Martin Storsjö]

(In reply to Georg Koppen from comment #69)
> Martin: shouldn't the clang cross-compiler use the mingw32
> config provided in libvpx and libaom instead of the x64 one aimed at MSVC?
> Or does that  not matter at the end if the code compiles and links properly?

Using the mingw32 config probably would be more correct as long as the compiler is run in mingw mode. I guess clang/llvm might support other details there that gcc doesn't, that makes it not blow up completely - I don't remember off-hand what mingw/msvc specific details there are in libvpx.

Comment 71

[Georg Koppen]

Attachment: Link failure when linking mozglue

Three new issues showed up:

1) Some gfx code does not compile:

 0:15.60 In file included from /home/thomas/Arbeit/Tor/debugging/21777/obj-mingw/gfx/gl/Unified_cpp_gfx_gl0.cpp:38:
 0:15.60 /home/thomas/Arbeit/Tor/debugging/21777/gfx/gl/GLBlitHelperD3D.cpp:194:10: error: case value is not a constant expression
 0:15.60     case subdescUnion.TSurfaceDescriptorDXGIYCbCr:
 0:15.60          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 0:15.60 /home/thomas/Arbeit/Tor/debugging/21777/gfx/gl/GLBlitHelperD3D.cpp:194:10: note: initializer of 'subdescUnion' is not a constant expression
 0:15.60 /home/thomas/Arbeit/Tor/debugging/21777/gfx/gl/GLBlitHelperD3D.cpp:187:17: note: declared here
 0:15.60     const auto& subdescUnion = desc.subdesc();
 0:15.60                 ^
 0:15.60 /home/thomas/Arbeit/Tor/debugging/21777/gfx/gl/GLBlitHelperD3D.cpp:189:10: error: case value is not a constant expression
 0:15.60     case subdescUnion.TSurfaceDescriptorD3D10:
 0:15.60          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 0:15.60 /home/thomas/Arbeit/Tor/debugging/21777/gfx/gl/GLBlitHelperD3D.cpp:189:10: note: initializer of 'subdescUnion' is not a constant expression
 0:15.60 /home/thomas/Arbeit/Tor/debugging/21777/gfx/gl/GLBlitHelperD3D.cpp:187:17: note: declared here
 0:15.60     const auto& subdescUnion = desc.subdesc();

2) Compiling the sandbox breaks with things like

 0:03.52 /home/thomas/Arbeit/Tor/debugging/win59/clang_toolchain/bin/x86_64-w64-mingw32-clang++ --sysroot /home/thomas/Arbeit/Tor/debugging/win59/clang_toolchain/x86_64-w64-mingw32 -mwindows -o permissionsService.o -c -DNDEBUG=1 -DTRIMMED=1 -DUNICODE -D_UNICODE -DNS_NO_XPCOM -DSANDBOX_EXPORTS -D_CRT_RAND_S -DCHROMIUM_SANDBOX_BUILD -I/home/thomas/Arbeit/Tor/debugging/21777/security/sandbox -I/home/thomas/Arbeit/Tor/debugging/21777/obj-mingw/security/sandbox -I/home/thomas/Arbeit/Tor/debugging/21777/security/sandbox/chromium-shim -I/home/thomas/Arbeit/Tor/debugging/21777/security/sandbox/chromium -I/home/thomas/Arbeit/Tor/debugging/21777/nsprpub -I/home/thomas/Arbeit/Tor/debugging/21777/obj-mingw/dist/include -I/home/thomas/Arbeit/Tor/debugging/21777/obj-mingw/dist/include/nspr -I/home/thomas/Arbeit/Tor/debugging/21777/obj-mingw/dist/include/nss -DMOZILLA_CLIENT -include /home/thomas/Arbeit/Tor/debugging/21777/obj-mingw/mozilla-config.h -Qunused-arguments -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Qunused-arguments -Wall -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wunreachable-code-return -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wloop-analysis -Wc++1z-compat -Wcomma -Wimplicit-fallthrough -Wstring-conversion -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-unknown-pragmas -Wno-unused-function -Wno-conversion-null -Wno-switch -Wno-enum-compare -Wno-gnu-zero-variadic-macro-arguments -Wno-unknown-warning-option -Wno-return-type-c-linkage -fno-sized-deallocation -Wno-incompatible-ms-struct -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fno-exceptions -fno-strict-aliasing -mms-bitfields -fno-rtti -fno-exceptions -fno-math-errno -pipe -g -O -fno-omit-frame-pointer  -MD -MP -MF .deps/permissionsService.o.pp   /home/thomas/Arbeit/Tor/debugging/21777/security/sandbox/chromium-shim/sandbox/win/permissionsService.cpp
 0:05.64 /home/thomas/Arbeit/Tor/debugging/21777/security/sandbox/chromium-shim/sandbox/win/permissionsService.cpp:16:27: error: implicit instantiation of undefined template 'std::__1::basic_string<wchar_t, std::__1::char_traits<wchar_t>, std::__1::allocator<wchar_t> >'
 0:05.64 static const std::wstring ZONE_IDENTIFIER_STR(L":ZONE.IDENTIFIER");
 0:05.64                           ^
 0:05.64 /home/thomas/Arbeit/Tor/debugging/win59/clang_toolchain/x86_64-w64-mingw32/include/c++/v1/iosfwd:193:32: note: template is declared here
 0:05.64     class _LIBCPP_TEMPLATE_VIS basic_string;
 0:05.64                                ^
 0:05.64 /home/thomas/Arbeit/Tor/debugging/21777/security/sandbox/chromium-shim/sandbox/win/permissionsService.cpp:17:27: error: implicit instantiation of undefined template 'std::__1::basic_string<wchar_t, std::__1::char_traits<wchar_t>, std::__1::allocator<wchar_t> >'
 0:05.64 static const std::wstring ZONE_ID_DATA_STR(L":ZONE.IDENTIFIER:$DATA");
 0:05.64                           ^

etc.

3) mozglue fails to link with

 0:02.89 lld-link: error: duplicate symbol: _ZSt17__throw_bad_allocv in ../misc/ConditionVariable_windows.o and in libc++.a(new.cpp.obj)

(the full log is attached)

Comment 72

[Martin Storsjö]

(In reply to Georg Koppen from comment #71)
> Created attachment 8954157 [details]
> Link failure when linking mozglue
> 
> Three new issues showed up:
> 
> 1) Some gfx code does not compile:
> 
>  0:15.60 In file included from
> /home/thomas/Arbeit/Tor/debugging/21777/obj-mingw/gfx/gl/Unified_cpp_gfx_gl0.
> cpp:38:
>  0:15.60
> /home/thomas/Arbeit/Tor/debugging/21777/gfx/gl/GLBlitHelperD3D.cpp:194:10:
> error: case value is not a constant expression
>  0:15.60     case subdescUnion.TSurfaceDescriptorDXGIYCbCr:
>  0:15.60          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This sounds like something that should be written as `case ClassName::TSurfaceDescriptorDXGIYCbCr:` instead - any particular reason why it isn't?

> 2) Compiling the sandbox breaks with things like
> 
>  0:03.52
> /home/thomas/Arbeit/Tor/debugging/win59/clang_toolchain/bin/x86_64-w64-
> mingw32-clang++ --sysroot
> /home/thomas/Arbeit/Tor/debugging/win59/clang_toolchain/x86_64-w64-mingw32
> -mwindows -o permissionsService.o -c -DNDEBUG=1 -DTRIMMED=1 -DUNICODE
> -D_UNICODE -DNS_NO_XPCOM -DSANDBOX_EXPORTS -D_CRT_RAND_S
> -DCHROMIUM_SANDBOX_BUILD
> -I/home/thomas/Arbeit/Tor/debugging/21777/security/sandbox
> -I/home/thomas/Arbeit/Tor/debugging/21777/obj-mingw/security/sandbox
> -I/home/thomas/Arbeit/Tor/debugging/21777/security/sandbox/chromium-shim
> -I/home/thomas/Arbeit/Tor/debugging/21777/security/sandbox/chromium
> -I/home/thomas/Arbeit/Tor/debugging/21777/nsprpub
> -I/home/thomas/Arbeit/Tor/debugging/21777/obj-mingw/dist/include
> -I/home/thomas/Arbeit/Tor/debugging/21777/obj-mingw/dist/include/nspr
> -I/home/thomas/Arbeit/Tor/debugging/21777/obj-mingw/dist/include/nss
> -DMOZILLA_CLIENT -include
> /home/thomas/Arbeit/Tor/debugging/21777/obj-mingw/mozilla-config.h
> -Qunused-arguments -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Qunused-arguments
> -Wall -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith
> -Wsign-compare -Wtype-limits -Wunreachable-code -Wunreachable-code-return
> -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wloop-analysis
> -Wc++1z-compat -Wcomma -Wimplicit-fallthrough -Wstring-conversion
> -Wno-inline-new-delete -Wno-error=deprecated-declarations
> -Wno-error=array-bounds -Wno-unknown-pragmas -Wno-unused-function
> -Wno-conversion-null -Wno-switch -Wno-enum-compare
> -Wno-gnu-zero-variadic-macro-arguments -Wno-unknown-warning-option
> -Wno-return-type-c-linkage -fno-sized-deallocation
> -Wno-incompatible-ms-struct -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2
> -fno-exceptions -fno-strict-aliasing -mms-bitfields -fno-rtti
> -fno-exceptions -fno-math-errno -pipe -g -O -fno-omit-frame-pointer  -MD -MP
> -MF .deps/permissionsService.o.pp  
> /home/thomas/Arbeit/Tor/debugging/21777/security/sandbox/chromium-shim/
> sandbox/win/permissionsService.cpp
>  0:05.64
> /home/thomas/Arbeit/Tor/debugging/21777/security/sandbox/chromium-shim/
> sandbox/win/permissionsService.cpp:16:27: error: implicit instantiation of
> undefined template 'std::__1::basic_string<wchar_t,
> std::__1::char_traits<wchar_t>, std::__1::allocator<wchar_t> >'
>  0:05.64 static const std::wstring ZONE_IDENTIFIER_STR(L":ZONE.IDENTIFIER");
>  0:05.64                           ^

No good clue about this one

> 3) mozglue fails to link with
> 
>  0:02.89 lld-link: error: duplicate symbol: _ZSt17__throw_bad_allocv in
> ../misc/ConditionVariable_windows.o and in libc++.a(new.cpp.obj)

Interesting. Can you provide the preprocessed source to ConditionVariable_windows.o? What could happen to produce a definition of std::__throw_bad_alloc there?

Comment 73

[(Away)]

(In reply to Martin Storsjö from comment #72)
> Interesting. Can you provide the preprocessed source to
> ConditionVariable_windows.o? What could happen to produce a definition of
> std::__throw_bad_alloc there?

https://searchfox.org/mozilla-central/source/memory/mozalloc/throw_gcc.h#46 deliberately defines some std:: functions, for reasons that I don't entirely understand.

Comment 74

[Martin Storsjö]

(In reply to David Major [:dmajor] from comment #73)
> (In reply to Martin Storsjö from comment #72)
> > Interesting. Can you provide the preprocessed source to
> > ConditionVariable_windows.o? What could happen to produce a definition of
> > std::__throw_bad_alloc there?
> 
> https://searchfox.org/mozilla-central/source/memory/mozalloc/throw_gcc.h#46
> deliberately defines some std:: functions, for reasons that I don't entirely
> understand.

Ok, I see - understandable if the intent is to override the exception handling mechanisms. Since this is pretty advanced use (and only should be used after careful review), I'm not sure if that review also applies to libcxx or if something else differs there.

To make it work linking wise, the definition of std::__throw_bad_alloc in libcxx should be split to a separate translation unit, so that the linker can choose just not to include it, while including other parts of new.cpp.

(If we'd link libcxx dynamically, this would work out better I think. But linking libcxx dynamically is currently blocked by at least https://reviews.llvm.org/D43184.)

Comment 75

[(Away)]

I wonder if this just needs a __MINGW64__ here? https://searchfox.org/mozilla-central/source/memory/mozalloc/throw_gcc.h#36

Comment 76

[Martin Storsjö]

There's no such thing as __MINGW64__, the existing ifdef probably triggers as intended.

But I think the MOZ_ALWAYS_INLINE_EVEN_DEBUG would need to be augmented with static to get the right linker visibility level. A non-static inline definition of a function will (with clang+lld, on windows right now at least) conflict with a non-inline global definition elsewhere.

But then again, as these are C++ functions, the normal C visibility rules don't apply... Not sure if there's any other ways to inhibit the main definition of the function itself other than making it a preprocessor define.

Comment 77

[Georg Koppen]

Attachment: buildlog_freebl_xul_linking_issues

I fixed/worked around the issues mentioned in comment:71, plus a couple of new ones and am close now it seems hitting two linker errors, one with freebl and one with xul. I have not figured out what is causing those yet. The full output is attached.

1) freebl

 0:05.23 lld-link: error: arcfour.o: undefined symbol: ARCFOUR
 0:05.23 lld-link: error: mpi.o: undefined symbol: s_mpv_mul_set_vec64
 0:05.23 lld-link: error: mpi.o: undefined symbol: s_mpv_mul_add_vec64
 0:05.24 lld-link: error: mpi.o: undefined symbol: s_mp_mul_comba_4
 0:05.24 lld-link: error: mpi.o: undefined symbol: s_mp_mul_comba_8
 0:05.24 lld-link: error: mpi.o: undefined symbol: s_mp_mul_comba_16
 0:05.24 lld-link: error: mpi.o: undefined symbol: s_mp_mul_comba_32
 0:05.24 lld-link: error: mpi.o: undefined symbol: s_mp_sqr_comba_4
 0:05.24 lld-link: error: mpi.o: undefined symbol: s_mp_sqr_comba_8
 0:05.24 lld-link: error: mpi.o: undefined symbol: s_mp_sqr_comba_16
 0:05.24 lld-link: error: mpi.o: undefined symbol: s_mp_sqr_comba_32
 0:05.24 lld-link: error: mpi.o: undefined symbol: s_mpv_mul_d_add_prop
 0:05.24 lld-link: error: mpmontg.o: undefined symbol: s_mpv_mul_d_add_prop

etc.

2) xul

 0:08.27 lld-link: Reading ../../xpcom/ds/Unified_cpp_xpcom_ds0.o
 0:08.27 lld-link: error: duplicate symbol: .weak._ZTHN7mozilla17AutoProfilerLabel12sPseudoStackE.default in <internal> and in <internal>
 0:08.27 lld-link: error: duplicate symbol: _ZTWN7mozilla17AutoProfilerLabel12sPseudoStackE in ../../xpcom/base/Unified_cpp_xpcom_base1.o and in ../../xpcom/ds/Unified_cpp_xpcom_ds0.o
 0:08.27 lld-link: Reading ../../xpcom/ds/Unified_cpp_xpcom_ds1.o
 0:08.27 lld-link: Reading ../../xpcom/io/Unified_c_xpcom_io0.o
 0:08.27 lld-link: Reading ../../xpcom/io/FileUtilsWin.o
 0:08.27 lld-link: error: duplicate symbol: .weak._ZTHN7mozilla17AutoProfilerLabel12sPseudoStackE.default in <internal> and in <internal>
 0:08.27 lld-link: error: duplicate symbol: _ZTWN7mozilla17AutoProfilerLabel12sPseudoStackE in ../../xpcom/base/Unified_cpp_xpcom_base1.o and in ../../xpcom/io/FileUtilsWin.o

and similar ones.

Comment 78

[Georg Koppen]

Attachment: buildlog_stylo

Oh, and I tried building with Stylo enabled, too. There it fails way earlier with

34:13.17 --- stderr
34:13.17 /home/thomas/Arbeit/Tor/debugging/21777/obj-mingw/dist/include/mozilla/ServoBindings.h:10:10: fatal error: 'stdint.h' file not found
34:13.17 /home/thomas/Arbeit/Tor/debugging/21777/obj-mingw/dist/include/mozilla/ServoBindings.h:10:10: fatal error: 'stdint.h' file not found, err: true

The log is attached, too.

Comment 79

[Georg Koppen]

(In reply to Georg Koppen from comment #77)
> Created attachment 8954374 [details]
> buildlog_freebl_xul_linking_issues
> 
> I fixed/worked around the issues mentioned in comment:71, plus a couple of
> new ones and am close now it seems hitting two linker errors, one with
> freebl and one with xul. I have not figured out what is causing those yet.
> The full output is attached.
> 
> 1) freebl
> 
>  0:05.23 lld-link: error: arcfour.o: undefined symbol: ARCFOUR
>  0:05.23 lld-link: error: mpi.o: undefined symbol: s_mpv_mul_set_vec64
>  0:05.23 lld-link: error: mpi.o: undefined symbol: s_mpv_mul_add_vec64
>  0:05.24 lld-link: error: mpi.o: undefined symbol: s_mp_mul_comba_4
>  0:05.24 lld-link: error: mpi.o: undefined symbol: s_mp_mul_comba_8
>  0:05.24 lld-link: error: mpi.o: undefined symbol: s_mp_mul_comba_16
>  0:05.24 lld-link: error: mpi.o: undefined symbol: s_mp_mul_comba_32
>  0:05.24 lld-link: error: mpi.o: undefined symbol: s_mp_sqr_comba_4
>  0:05.24 lld-link: error: mpi.o: undefined symbol: s_mp_sqr_comba_8
>  0:05.24 lld-link: error: mpi.o: undefined symbol: s_mp_sqr_comba_16
>  0:05.24 lld-link: error: mpi.o: undefined symbol: s_mp_sqr_comba_32
>  0:05.24 lld-link: error: mpi.o: undefined symbol: s_mpv_mul_d_add_prop
>  0:05.24 lld-link: error: mpmontg.o: undefined symbol: s_mpv_mul_d_add_prop

That's due to the NSS update to 3_31 (bug 1345368) which added a "# Should be copied to mingw defines below" and 

[ 'cc_use_gnu_ld==1 and OS=="win" and target_arch=="x64"', {
        'defines': [
          'MP_IS_LITTLE_ENDIAN',
          'NSS_BEVAND_ARCFOUR',
          'MPI_AMD64',
          'MP_ASSEMBLY_MULTIPLY',
          'NSS_USE_COMBA',
          'USE_HW_AES',
          'INTEL_GCM',
         ],
}],

to the freebl.gyp file. The clang cross-compiler does not like that. We should find a better fix for it but for now adding a "cc_is_clang!=1" preventing those defines is enough to move on.

Comment 80

[Georg Koppen]

(In reply to Georg Koppen from comment #77)
> Created attachment 8954374 [details]
> 2) xul
>  0:08.27 lld-link: Reading ../../xpcom/ds/Unified_cpp_xpcom_ds1.o
>  0:08.27 lld-link: Reading ../../xpcom/io/Unified_c_xpcom_io0.o
>  0:08.27 lld-link: Reading ../../xpcom/io/FileUtilsWin.o
>  0:08.27 lld-link: error: duplicate symbol:
> .weak._ZTHN7mozilla17AutoProfilerLabel12sPseudoStackE.default in <internal>
> and in <internal>
>  0:08.27 lld-link: error: duplicate symbol:
> _ZTWN7mozilla17AutoProfilerLabel12sPseudoStackE in
> ../../xpcom/base/Unified_cpp_xpcom_base1.o and in
> ../../xpcom/io/FileUtilsWin.o

After some bisecting and digging it turns out this is due to bug 1348419, in particular due to switching to `thread_local` for Windows targets. Looking at FileUtilsWin.o and Unified_cpp_xpcom_base1.o I can see:

llvm-nm FileUtilsWin.o | grep sPseudoStack
00000000 A .weak._ZTHN7mozilla17AutoProfilerLabel12sPseudoStackE.default
         U _ZN7mozilla17AutoProfilerLabel12sPseudoStackE
         w _ZTHN7mozilla17AutoProfilerLabel12sPseudoStackE
000061e0 T _ZTWN7mozilla17AutoProfilerLabel12sPseudoStackE

llvm-nm Unified_cpp_xpcom_base1.o | grep sPseudoStack
00000000 A .weak._ZTHN7mozilla17AutoProfilerLabel12sPseudoStackE.default
         U _ZN7mozilla17AutoProfilerLabel12sPseudoStackE
         w _ZTHN7mozilla17AutoProfilerLabel12sPseudoStackE
00009820 T _ZTWN7mozilla17AutoProfilerLabel12sPseudoStackE

I wonder if that's actually a toolchain shortcoming that should be fixed there, given that mingw-w64/GCC does not seem to have an issue with that change.

Then I ran into 

 0:06.04 lld-link: error: ../../gfx/skia/SkJumper.o: undefined symbol: _sk_seed_shader_hsw
 0:06.04 lld-link: error: ../../gfx/skia/SkJumper.o: undefined symbol: _sk_move_src_dst_hsw

indicating that Jumper assembly is still not properly disabled for mingw-w64/clang. It seems we are hitting the `SK_JUMPER_USE_ASSEMBLY 1` in

#if !defined(SK_JUMPER_USE_ASSEMBLY)
#if __has_feature(memory_sanitizer)
#define SK_JUMPER_USE_ASSEMBLY 0
#else
#define SK_JUMPER_USE_ASSEMBLY 1
#endif
#endif

which is causing this.

Now I am stuck with undefined symbols in gkrust.lib

 0:06.55 lld-link: error: gkrust.lib(webrender_bindings-5ebe8843c82df0eb.webrender_bindings0.rcgu.o): undefined symbol: __chkstk
 0:06.55 lld-link: error: gkrust.lib(std-8f33fb13bb6c9735.std5-1f82f62f5c33725abd91b7859524bef2.rs.rcgu.o): undefined symbol: __CxxFrameHandler3
 0:06.55 lld-link: error: gkrust.lib(std-8f33fb13bb6c9735.std13-1f82f62f5c33725abd91b7859524bef2.rs.rcgu.o): undefined symbol: __CxxFrameHandler3

etc. Any ideas where those could come from are appreciated. :)

Comment 81

[Martin Storsjö]

(In reply to Georg Koppen from comment #80)
> (In reply to Georg Koppen from comment #77)
> > Created attachment 8954374 [details]
> > 2) xul
> >  0:08.27 lld-link: Reading ../../xpcom/ds/Unified_cpp_xpcom_ds1.o
> >  0:08.27 lld-link: Reading ../../xpcom/io/Unified_c_xpcom_io0.o
> >  0:08.27 lld-link: Reading ../../xpcom/io/FileUtilsWin.o
> >  0:08.27 lld-link: error: duplicate symbol:
> > .weak._ZTHN7mozilla17AutoProfilerLabel12sPseudoStackE.default in <internal>
> > and in <internal>
> >  0:08.27 lld-link: error: duplicate symbol:
> > _ZTWN7mozilla17AutoProfilerLabel12sPseudoStackE in
> > ../../xpcom/base/Unified_cpp_xpcom_base1.o and in
> > ../../xpcom/io/FileUtilsWin.o
> 
> After some bisecting and digging it turns out this is due to bug 1348419, in
> particular due to switching to `thread_local` for Windows targets. Looking
> at FileUtilsWin.o and Unified_cpp_xpcom_base1.o I can see:
> 
> llvm-nm FileUtilsWin.o | grep sPseudoStack
> 00000000 A .weak._ZTHN7mozilla17AutoProfilerLabel12sPseudoStackE.default
>          U _ZN7mozilla17AutoProfilerLabel12sPseudoStackE
>          w _ZTHN7mozilla17AutoProfilerLabel12sPseudoStackE
> 000061e0 T _ZTWN7mozilla17AutoProfilerLabel12sPseudoStackE
> 
> llvm-nm Unified_cpp_xpcom_base1.o | grep sPseudoStack
> 00000000 A .weak._ZTHN7mozilla17AutoProfilerLabel12sPseudoStackE.default
>          U _ZN7mozilla17AutoProfilerLabel12sPseudoStackE
>          w _ZTHN7mozilla17AutoProfilerLabel12sPseudoStackE
> 00009820 T _ZTWN7mozilla17AutoProfilerLabel12sPseudoStackE
> 
> I wonder if that's actually a toolchain shortcoming that should be fixed
> there, given that mingw-w64/GCC does not seem to have an issue with that
> change.

GCC uses emulated TLS on windows by default (afaik there is some details unimplemented in using the native mechanism like MSVC). Clang does support the native mechanism though (and it works fine afaik, in the cases I've tested). Apparently this might be some issue relating to that? The .weak- part of it also indicate that it would relate to weak symbols somehow, which I'm not completely certain that work as they should in this setup.

You can try adding `-femulated-tls` when building the relevant object files to see if that helps - then it should work similarly to what GCC does.

> Now I am stuck with undefined symbols in gkrust.lib
> 
>  0:06.55 lld-link: error:
> gkrust.lib(webrender_bindings-5ebe8843c82df0eb.webrender_bindings0.rcgu.o):
> undefined symbol: __chkstk
>  0:06.55 lld-link: error:
> gkrust.lib(std-8f33fb13bb6c9735.std5-1f82f62f5c33725abd91b7859524bef2.rs.
> rcgu.o): undefined symbol: __CxxFrameHandler3
>  0:06.55 lld-link: error:
> gkrust.lib(std-8f33fb13bb6c9735.std13-1f82f62f5c33725abd91b7859524bef2.rs.
> rcgu.o): undefined symbol: __CxxFrameHandler3
> 
> etc. Any ideas where those could come from are appreciated. :)

I take it that this is some very low level code that touches such symbols?

The former, __chkstk, is for stack probing (a mechanism where you touch the stack in consecutive 4 KB steps to allow the OS allocate more stack as necessary). This function has different names (and calling convention and other details) depending on whether you're on 32 or 64 bit x86, and whether you're in MSVC or mingw land. See getStackProbeSymbolName  in https://raw.githubusercontent.com/llvm-mirror/llvm/master/lib/Target/X86/X86ISelLowering.cpp at the bottom; __chkstk is the name used in MSVC, as provided by libcmt.lib or msvcrt.lib. In this case it should be supplied by compiler-rt (toolchain/lib/clang/7.0.0/windows/libclang_rt.builtins-x86_64.a), but it only exists there in the mingw/libgcc compatible name (___chkstk_ms for x86_64).

__CxxFrameHandler3 can be provided by msvcrt.dll, but it isn't exposed by default in mingw-w64 (probably since it didn't exist in msvcrt.dll back in XP). It exists in the numbered versions of the DLL from modern MSVC versions though, and in ucrtbase.dll. It seems to have appeared in msvcrt.dll in Vista. If you tweak mingw-w64-crt/lib-common/msvcrt.def.in you should be able to make this function visible here as well. (It's currently only made visible on ARM, since windows on ARM is >= win8.)

Comment 82

[Georg Koppen]

Thanks for your explanations regarding __chkstk and __CxxFrameHandler3, it helped me figuring out that we were on the MSVC rust path due to the `building_with_gcc`check in rust.configure. Making sure we take the -windows-gnu path makes me build the whole thing!

Now, I just need to fix/work around all the breakage due the code changes from the last twenty days which I ran into after rebasing my local mozilla-central branch. One thing that stood out was:

11:20.54 /home/thomas/Arbeit/Tor/debugging/21777/obj-mingw/dist/include/mozilla/FStream.h:24:10: fatal error: 'ext/stdio_filebuf.h' file not found
11:20.54 #include <ext/stdio_filebuf.h>

stemming from

#if defined(__MINGW32__)
#include "mozilla/UniquePtr.h"
#include <fcntl.h>
#include <ext/stdio_filebuf.h>
#endif

in FStream.h.

Martin, it seems that header is missing in the toolchain or am I overlooking something?

Comment 83

[Masatoshi Kimura [:emk]]

(In reply to Georg Koppen from comment #82)
> Now, I just need to fix/work around all the breakage due the code changes
> from the last twenty days which I ran into after rebasing my local
> mozilla-central branch. One thing that stood out was:
> 
> 11:20.54
> /home/thomas/Arbeit/Tor/debugging/21777/obj-mingw/dist/include/mozilla/
> FStream.h:24:10: fatal error: 'ext/stdio_filebuf.h' file not found
> 11:20.54 #include <ext/stdio_filebuf.h>
> 
> stemming from
> 
> #if defined(__MINGW32__)
> #include "mozilla/UniquePtr.h"
> #include <fcntl.h>
> #include <ext/stdio_filebuf.h>
> #endif
> 
> in FStream.h.
> 
> Martin, it seems that header is missing in the toolchain or am I overlooking
> something?

I added this part with the intention of supporting MinGW, but I only confirmed that the patched build passed on the tryserver. Feel free to rewrite if it does not work well.

Comment 84

[Martin Storsjö]

(In reply to Georg Koppen from comment #82)
> 11:20.54
> /home/thomas/Arbeit/Tor/debugging/21777/obj-mingw/dist/include/mozilla/
> FStream.h:24:10: fatal error: 'ext/stdio_filebuf.h' file not found
> 11:20.54 #include <ext/stdio_filebuf.h>
> 
> stemming from
> 
> #if defined(__MINGW32__)
> #include "mozilla/UniquePtr.h"
> #include <fcntl.h>
> #include <ext/stdio_filebuf.h>
> #endif
> 
> in FStream.h.
> 
> Martin, it seems that header is missing in the toolchain or am I overlooking
> something?

No idea about this, I don't know about such a header, and I don't see one anywhere either in the current checkout or in the history of mingw-w64.

Comment 85

[Masatoshi Kimura [:emk]]

ext/stdio_filebuf.h is a GCC header, not MinGW specific one. So it will not appear in the history of mingw-w64. I used this extension to implement a substitute of fstream constructors because they did not have overloads that take a Unicode file paths until C++ 17.

According to <https://libcxx.llvm.org/cxx1z_status.html>, LLVM does not support the overloads yet :(  Does it support an extension that is similar to GCC stdio_filebuf? (That is, FILE* or POSIX fd overloads for streams).

Comment 86

[Georg Koppen]

(In reply to Masatoshi Kimura [:emk] from comment #85)
> ext/stdio_filebuf.h is a GCC header, not MinGW specific one. So it will not
> appear in the history of mingw-w64. I used this extension to implement a
> substitute of fstream constructors because they did not have overloads that
> take a Unicode file paths until C++ 17.
> 
> According to <https://libcxx.llvm.org/cxx1z_status.html>, LLVM does not
> support the overloads yet :(  Does it support an extension that is similar
> to GCC stdio_filebuf? (That is, FILE* or POSIX fd overloads for streams).

I don't know but it seems to me using the code in the XP_WIN path instead of the __MINGW32__ one works at least around the current problem.

Comment 87

[Martin Storsjö]

(In reply to Masatoshi Kimura [:emk] from comment #85)
> ext/stdio_filebuf.h is a GCC header, not MinGW specific one. So it will not
> appear in the history of mingw-w64. I used this extension to implement a
> substitute of fstream constructors because they did not have overloads that
> take a Unicode file paths until C++ 17.

Ah, thanks - that makes sense.

Unfortunately it does indeed seem like the std::filesystem::path part of C++17 is still unimplemented in libc++.

(In reply to Georg Koppen from comment #86)
> I don't know but it seems to me using the code in the XP_WIN path instead of
> the __MINGW32__ one works at least around the current problem.

Indeed, it seems like libc++ has got this feature matching the MSVC C++ standard library even though it's not part of any standard: https://reviews.llvm.org/D42225 (It got the feature very recently though.)

This seems to be included in the pinned version of libc++ I'm using in my builds (even though afaik any later version also should work).

Comment 88

[Masatoshi Kimura [:emk]]

(In reply to Martin Storsjö from comment #87)
> Indeed, it seems like libc++ has got this feature matching the MSVC C++
> standard library even though it's not part of any standard:
> https://reviews.llvm.org/D42225 (It got the feature very recently though.)
> 
> This seems to be included in the pinned version of libc++ I'm using in my
> builds (even though afaik any later version also should work).

Oh, good news. So it will work to change `#ifdef __MINGW32__` to `#if defined(__MINGW32__) && !defined(__clang__)`.

Comment 89

[Martin Storsjö]

(In reply to Masatoshi Kimura [:emk] from comment #88)
> (In reply to Martin Storsjö from comment #87)
> > Indeed, it seems like libc++ has got this feature matching the MSVC C++
> > standard library even though it's not part of any standard:
> > https://reviews.llvm.org/D42225 (It got the feature very recently though.)
> > 
> > This seems to be included in the pinned version of libc++ I'm using in my
> > builds (even though afaik any later version also should work).
> 
> Oh, good news. So it will work to change `#ifdef __MINGW32__` to `#if
> defined(__MINGW32__) && !defined(__clang__)`.

Yes, or perhaps something like `#if defined(__MINGW32__) && !defined(_LIBCPP_VERSION)` instead to be even more precise. Technically you can also use clang just to replace the compiler in a normal GCC/binutils/libstdc++ based MinGW toolchain, while keeping all other components as they were.

Comment 90

[Georg Koppen]

(In reply to Martin Storsjö from comment #81)
> (In reply to Georg Koppen from comment #80)
> > (In reply to Georg Koppen from comment #77)
> > > Created attachment 8954374 [details]
> > > 2) xul
> > >  0:08.27 lld-link: Reading ../../xpcom/ds/Unified_cpp_xpcom_ds1.o
> > >  0:08.27 lld-link: Reading ../../xpcom/io/Unified_c_xpcom_io0.o
> > >  0:08.27 lld-link: Reading ../../xpcom/io/FileUtilsWin.o
> > >  0:08.27 lld-link: error: duplicate symbol:
> > > .weak._ZTHN7mozilla17AutoProfilerLabel12sPseudoStackE.default in <internal>
> > > and in <internal>
> > >  0:08.27 lld-link: error: duplicate symbol:
> > > _ZTWN7mozilla17AutoProfilerLabel12sPseudoStackE in
> > > ../../xpcom/base/Unified_cpp_xpcom_base1.o and in
> > > ../../xpcom/io/FileUtilsWin.o
> > 
> > After some bisecting and digging it turns out this is due to bug 1348419, in
> > particular due to switching to `thread_local` for Windows targets. Looking
> > at FileUtilsWin.o and Unified_cpp_xpcom_base1.o I can see:
> > 
> > llvm-nm FileUtilsWin.o | grep sPseudoStack
> > 00000000 A .weak._ZTHN7mozilla17AutoProfilerLabel12sPseudoStackE.default
> >          U _ZN7mozilla17AutoProfilerLabel12sPseudoStackE
> >          w _ZTHN7mozilla17AutoProfilerLabel12sPseudoStackE
> > 000061e0 T _ZTWN7mozilla17AutoProfilerLabel12sPseudoStackE
> > 
> > llvm-nm Unified_cpp_xpcom_base1.o | grep sPseudoStack
> > 00000000 A .weak._ZTHN7mozilla17AutoProfilerLabel12sPseudoStackE.default
> >          U _ZN7mozilla17AutoProfilerLabel12sPseudoStackE
> >          w _ZTHN7mozilla17AutoProfilerLabel12sPseudoStackE
> > 00009820 T _ZTWN7mozilla17AutoProfilerLabel12sPseudoStackE
> > 
> > I wonder if that's actually a toolchain shortcoming that should be fixed
> > there, given that mingw-w64/GCC does not seem to have an issue with that
> > change.
> 
> GCC uses emulated TLS on windows by default (afaik there is some details
> unimplemented in using the native mechanism like MSVC). Clang does support
> the native mechanism though (and it works fine afaik, in the cases I've
> tested). Apparently this might be some issue relating to that? The .weak-
> part of it also indicate that it would relate to weak symbols somehow, which
> I'm not completely certain that work as they should in this setup.
> 
> You can try adding `-femulated-tls` when building the relevant object files
> to see if that helps - then it should work similarly to what GCC does.

Nope, it does not. We need to come up with something else I guess. Checking the .o file with `llvm-nm` gives me:

llvm-nm Unified_cpp_xpcom_base1.o | grep sPseudoStack
00000000 A .weak._ZTHN7mozilla17AutoProfilerLabel12sPseudoStackE.default
         w _ZTHN7mozilla17AutoProfilerLabel12sPseudoStackE
000096d0 T _ZTWN7mozilla17AutoProfilerLabel12sPseudoStackE
         U __emutls_v._ZN7mozilla17AutoProfilerLabel12sPseudoStackE

and I am getting similar linking errors at the end when linking xul.dll.

That said it seems I worked around all issues and finally get the code compiled and linked. Surprisingly, it runs even without the need to force-strip, so this has been an issue fixed in Mozilla code. Might be worth understanding what it was. I'll post a .mozconfig file and a bunch of hacks later to this ticket, so other folks can look into it as well. The main blocker as I see it right now is the compiler crash dealt with in https://bugs.llvm.org/show_bug.cgi?id=36487 which makes it necessary to back out the lz4 updates. Then there is no ASLR and no DEP enabled but I guess that might not be too hard to get fixed.

Now on to building with Stylo enabled...

Comment 91

[Georg Koppen]

Attachment: buildlog_stylo1

Trying to compile with Stylo the build breaks (log attached).

44:10.23 /usr/lib/gcc/x86_64-linux-gnu/7.3.0/../../../../include/c++/7.3.0/cstdlib:194:11: error: no member named 'lldiv_t' in the global namespace
44:10.23 /usr/lib/gcc/x86_64-linux-gnu/7.3.0/../../../../include/c++/7.3.0/cstdlib:200:11: error: no member named '_Exit' in the global namespace
44:10.23 /usr/lib/gcc/x86_64-linux-gnu/7.3.0/../../../../include/c++/7.3.0/cstdlib:204:11: error: no member named 'llabs' in the global namespace

etc.

jumps out as I am wondering whether it's actually correct that my C++ Linux headers are queried...

Manish: Any idea about what could be wrong when looking at the error log? (as indicated in the previous comment compiling with --disable-stylo works well)

Comment 92

[Emilio Cobos Álvarez (:emilio)]

(In reply to Georg Koppen from comment #91)
> Created attachment 8956056 [details]
> buildlog_stylo1
> 
> Trying to compile with Stylo the build breaks (log attached).
> 
> 44:10.23
> /usr/lib/gcc/x86_64-linux-gnu/7.3.0/../../../../include/c++/7.3.0/cstdlib:
> 194:11: error: no member named 'lldiv_t' in the global namespace
> 44:10.23
> /usr/lib/gcc/x86_64-linux-gnu/7.3.0/../../../../include/c++/7.3.0/cstdlib:
> 200:11: error: no member named '_Exit' in the global namespace
> 44:10.23
> /usr/lib/gcc/x86_64-linux-gnu/7.3.0/../../../../include/c++/7.3.0/cstdlib:
> 204:11: error: no member named 'llabs' in the global namespace

That means that the wrong arguments are passed to libclang, which makes sense, I suspect, since you probably need a block like this:

  https://searchfox.org/mozilla-central/rev/d2b4b40901c15614fad2fa34718eea428774306e/layout/style/ServoBindings.toml#32-47

For MinGW.

Comment 93

[Manish Goregaokar [:manishearth]]

Yeah, it's the flags passed to clang. Not sure what they should be, but you probably need some defines and perhaps some includes.

(Might be worth checking what defines firefox uses on mingw)

Comment 94

[Georg Koppen]

Attachment: 0001-m-c-fixes-for-cross-compiling-with-mingw-w64-clang.patch

I just rebuilt m-c with the fix for https://bugs.llvm.org/show_bug.cgi?id=36487 and attached is the patch I needed + the .mozconfig file.

Comment 95

[Georg Koppen]

Attachment: dot_mozconfig

Comment 96

[Chris Peterson [:cpeterson]]

Just a heads up on the Stylo schedule:

* Firefox 60: Ship Stylo-chrome (bug 1294570) for browser chrome in the parent process. This is the last use of Gecko's old style system code. The old code will still exist, but it won't be used. Firefox 60 is the next ESR.

* Firefox 61: Delete the old style system code in ~April during Nightly 61. Only Stylo will be supported.

Comment 97

[Emilio Cobos Álvarez (:emilio)]

Hi Tom, is there anyway I can help here? The old style system code can go soon enough :)

Comment 98

[Emilio Cobos Álvarez (:emilio)]

Err, can't go, ofc

Comment 99

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

From Comment 91, it sounds like Georg needs to figure out the block needed: https://searchfox.org/mozilla-central/rev/d2b4b40901c15614fad2fa34718eea428774306e/layout/style/ServoBindings.toml#32-47

I assume setting the top line to
> [build."os=windows"]

will trigger for a cross-compile; since the target is Windows?



Once Stylo builds with mingw-clang, we'll (well, Georg) have to make a decision: do we want to switch the entire browser over to mingw-clang, or do we want to try to build Stylo with mingw-clang and the rest of the browser with mingw-gcc?

Once we know that, we can start updating the MinGW build in TaskCluster.  I have a feeling however that we'll need to disable the build for 61 for a bit, as we won't have it working as fast as you'd like to remove the old stuff.

Comment 100

[Georg Koppen]

Emilio, I got back from a week long meeting and try to figure out the right incantations for the mingw-w64 build this week, I hope. That said, I'd say don't wait on that as we probably need to disable the build for a while anyway (see below).

Tom: We'll switch the whole build to mingw-clang I think. The toolchain situation is already complicated enough. No need to make it even more complicated by having two different toolchains for cross-compiling the browser for Windows users. I agree that we'll probably need to disable the build for a while as there are a bunch of issues that come with the switch to mingw-w64/clang that need to get addressed first, probably in child tickets to this bug.

Comment 101

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

I'm not sure where to put this, but I tried switching to ucrt and I would up with graphics errors caused by fxc2.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=73f5e54964063185ee7745150436a024268eb275

This needs more investigation, including building from MinGW trunk - but I didn't want to lose this try link and bit of info.

Comment 102

[Intermittent Failures Robot]

2 failures in 632 pushes (0.003 failures/push) were associated with this bug in the last 7 days.    

Repository breakdown:
* try: 2

Platform breakdown:
* windows-mingw32: 2

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1390583&startday=2018-05-21&endday=2018-05-27&tree=trunk

Comment 103

[Jacek Caban]

Attachment: Bug 1390583 - Stylo: Build Broken with MinGW

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

Comment 104

[Jacek Caban]

Here is a successful try push with clang-based toolchain and stylo enabled on esr60 branch (actually, optimized build works for me, debug one not really):
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0a9807afff955855b264d7a9288a18bd7630cc54

It needs the attached patch. The patch makes ServoBindings.toml aware of mingw config, but it also:

- Changes -std=c++14 to use -std=gnu++14
It's needed because of how mingw headers handle strict mode. math.h skips some nonstandard declarations if __STRICT_ANSI__ (which is defined by compiler in strict mode like c++14). There is room for improvements on mingw side. I think that at least some of those should be made available in non-strict mode. Regardless of that, most of Mozilla codebase uses gnu* C/C++ mode, so changing it here looks fine to me.

- Pass proper BINDGEN_CFLAGS
This part is hackish. Ideally --sysroot should be enough (maybe together with -stdlib=libc++), but apparently it's not and we need to teach it about all required include directories. It's enough for clang itself to understand toolchain's include dirs, but bindgen does not support it. I looked at it a bit, but didn't see how it should be supported. I tried -isystem, -cxx-isystem and alikes but only -I works. 



A side not: while tracing all clang invocations, I found some weird ones looking like this (uninteresting arguments stripped):
clang -x c - -x c++ -std=c++14
error: invalid argument '-std=c++14' not allowed with 'C'
The later "-x c++" is not taken into account because it's after "-". -std=c++14 is there because of args set at the top of ServoBindings.toml. Something like that would work:
clang -x c -x c++ -std=c++14 -
This is not mingw-specific and seems harmless.

Comment 105

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8991919 [details]
Bug 1390583 - Stylo: Build Broken with MinGW

https://reviewboard.mozilla.org/r/256840/#review264034

Looks generally reasonable, but probably Xidorn who is more familiar with Windows than I is a better reviewer. Also maybe a build system peer review would come handy I suspect, though I'd let Xidorn / you decide that.

::: browser/config/mozconfigs/win64/mingwclang:37
(Diff revision 1)
>  ac_add_options --disable-warnings-as-errors
>  
>  # Temporary config settings until we get these working on mingw
>  ac_add_options --disable-accessibility # https://sourceforge.net/p/mingw-w64/bugs/648/
>  
> -ac_add_options --disable-stylo # Bug 1390583
> +BINDGEN_CFLAGS="--target=x86_64-w64-mingw32 --sysroot=$TOOLTOOL_DIR/clang/x86_64-w64-mingw32 -I$TOOLTOOL_DIR/clang/x86_64-w64-mingw32/include/c++/v1 -I$TOOLTOOL_DIR/clang/lib/clang/7.0.0/include -I$TOOLTOOL_DIR/clang/x86_64-w64-mingw32/include"

We may want to move this to `build_gecko.rs` instead? Not sure if it's generally considered just fine to throw env vars to a mozconfig. Looks like at least the `--target` bits are redundant with the toml file.

::: layout/style/ServoBindings.toml:3
(Diff revision 1)
>  [build]
>  args = [
> -    "-x", "c++", "-std=c++14", "-fno-sized-deallocation",
> +    "-x", "c++", "-std=gnu++14", "-fno-sized-deallocation",

I don't know the implications of this change. Why is it needed? Worth explaining it in the commit message.

Comment 106

[Martin Storsjö]

(In reply to Jacek Caban from comment #104)
> - Changes -std=c++14 to use -std=gnu++14
> It's needed because of how mingw headers handle strict mode. math.h skips
> some nonstandard declarations if __STRICT_ANSI__ (which is defined by
> compiler in strict mode like c++14). There is room for improvements on mingw
> side. I think that at least some of those should be made available in
> non-strict mode.

+1 for looking into this if you have time; it seems the mingw headers are overly strict with this compared to other libcs. I have also run into cases elsewhere, where building with mingw requires using -std=gnu++* where the same codebase builds fine on linux and macos with just -std=c++*.

Comment 107

[Xidorn Quan [:xidorn] UTC+11]

Comment on attachment 8991919 [details]
Bug 1390583 - Stylo: Build Broken with MinGW

https://reviewboard.mozilla.org/r/256840/#review264218

r=me with the following issues addressed.

::: browser/config/mozconfigs/win64/mingwclang:37
(Diff revision 1)
>  ac_add_options --disable-warnings-as-errors
>  
>  # Temporary config settings until we get these working on mingw
>  ac_add_options --disable-accessibility # https://sourceforge.net/p/mingw-w64/bugs/648/
>  
> -ac_add_options --disable-stylo # Bug 1390583
> +BINDGEN_CFLAGS="--target=x86_64-w64-mingw32 --sysroot=$TOOLTOOL_DIR/clang/x86_64-w64-mingw32 -I$TOOLTOOL_DIR/clang/x86_64-w64-mingw32/include/c++/v1 -I$TOOLTOOL_DIR/clang/lib/clang/7.0.0/include -I$TOOLTOOL_DIR/clang/x86_64-w64-mingw32/include"

As emilio suggested, you should probably remove the `--target` here.

But I'm not too worried about other pieces as `BINDGEN_CFLAGS` was added for some reason... in bug 1368083. I think the reason was having some specific path passed to bindgen, and I guess that makes sense to some extent, and fit the usecase here as well.

::: layout/style/ServoBindings.toml:35
(Diff revision 1)
>      # To enable the builtin __builtin_offsetof so that CRT wouldn't
>      # use reinterpret_cast in offsetof() which is not allowed inside
>      # static_assert().
>      "-D_CRT_USE_BUILTIN_OFFSETOF",

Does MinGW build use MSVC's C Runtime? If not, I think you can move this to the secion of `env=msvc`.

::: layout/style/ServoBindings.toml:39
(Diff revision 1)
>      # Enable hidden attribute (which is not supported by MSVC and
>      # thus not enabled by default with a MSVC-compatibile build)
>      # to exclude hidden symbols from the generated file.
>      "-DHAVE_VISIBILITY_HIDDEN_ATTRIBUTE=1",

This probably should be moved to `env=msvc` section given it's msvc-specific.

::: layout/style/ServoBindings.toml:44
(Diff revision 1)
>  "arch=x86" = ["--target=i686-pc-win32"]
> -"arch=x86_64" = ["--target=x86_64-pc-win32"]
> +"arch=x86_64" = ["--target=x86_64-w64-mingw32"]

Given that you are adding `arch=` for both envs below, you should be able to remove these two lines.

Comment 108

[Xidorn Quan [:xidorn] UTC+11]

Comment on attachment 8991919 [details]
Bug 1390583 - Stylo: Build Broken with MinGW

https://reviewboard.mozilla.org/r/256840/#review264034

> I don't know the implications of this change. Why is it needed? Worth explaining it in the commit message.

I'm not very worried about this. `gnu++14` is basically `c++14` + some GNU extensions, so it's a superset of c++14 and shouldn't change the behavior of parts which works before.

Comment 109

[Jacek Caban]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #105)
> Comment on attachment 8991919 [details]
> Bug 1390583 - Stylo: Build Broken with MinGW
> 
> https://reviewboard.mozilla.org/r/256840/#review264034
> 
> Looks generally reasonable, but probably Xidorn who is more familiar with
> Windows than I is a better reviewer. Also maybe a build system peer review
> would come handy I suspect, though I'd let Xidorn / you decide that.
> 
> ::: browser/config/mozconfigs/win64/mingwclang:37
> (Diff revision 1)
> >  ac_add_options --disable-warnings-as-errors
> >  
> >  # Temporary config settings until we get these working on mingw
> >  ac_add_options --disable-accessibility # https://sourceforge.net/p/mingw-w64/bugs/648/
> >  
> > -ac_add_options --disable-stylo # Bug 1390583
> > +BINDGEN_CFLAGS="--target=x86_64-w64-mingw32 --sysroot=$TOOLTOOL_DIR/clang/x86_64-w64-mingw32 -I$TOOLTOOL_DIR/clang/x86_64-w64-mingw32/include/c++/v1 -I$TOOLTOOL_DIR/clang/lib/clang/7.0.0/include -I$TOOLTOOL_DIR/clang/x86_64-w64-mingw32/include"
> 
> We may want to move this to `build_gecko.rs` instead? Not sure if it's
> generally considered just fine to throw env vars to a mozconfig. Looks like
> at least the `--target` bits are redundant with the toml file.

FWIW, I think that ideally bindgen should be able to just get those include paths from clang. Since we use clang for the whole build, it could, for example, just run configured clang in verbose mode and get default includes from that. I'm not sure hardcoding such things in build_gecko.rs is the right thing. We currently use LLVM libc++ for standard c++ library (and instruct clang about that by passing -stdlib=libc++), but clang could also use GCC's libstdc++ and directory layout would be different. That's something clang takes care of, we shouldn't need to.

> ::: layout/style/ServoBindings.toml:3
> (Diff revision 1)
> >  [build]
> >  args = [
> > -    "-x", "c++", "-std=c++14", "-fno-sized-deallocation",
> > +    "-x", "c++", "-std=gnu++14", "-fno-sized-deallocation",
> 
> I don't know the implications of this change. Why is it needed? Worth
> explaining it in the commit message.

I briefly described it in comment 104, but let me give more details. Some headers we need to parse use _finite function. It's defined in math.h:
https://github.com/mirror/mingw-w64/blob/master/mingw-w64-headers/crt/math.h#L302
mingw-w64 guards that with check for __STRICT_ANSI__:
https://github.com/mirror/mingw-w64/blob/master/mingw-w64-headers/crt/math.h#L266
__STRICT_ANSI__ is defined when -std=c++14 is used, so it's undeclared.

There are a few more similar cases.

I plan to look at this on mingw-w64 side and remove some of those strict checks, so soon this change will not be needed. However, I still think that changing it here is the right thing. C++ Gecko code is compiled using gnu mode and being closer to that seems right.

Comment 110

[Jacek Caban]

(In reply to Martin Storsjö from comment #106)
> (In reply to Jacek Caban from comment #104)
> > - Changes -std=c++14 to use -std=gnu++14
> > It's needed because of how mingw headers handle strict mode. math.h skips
> > some nonstandard declarations if __STRICT_ANSI__ (which is defined by
> > compiler in strict mode like c++14). There is room for improvements on mingw
> > side. I think that at least some of those should be made available in
> > non-strict mode.
> 
> +1 for looking into this if you have time; it seems the mingw headers are
> overly strict with this compared to other libcs. I have also run into cases
> elsewhere, where building with mingw requires using -std=gnu++* where the
> same codebase builds fine on linux and macos with just -std=c++*.

Sure, I will take a look.

(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #107)
> Comment on attachment 8991919 [details]
> Bug 1390583 - Stylo: Build Broken with MinGW
> 
> https://reviewboard.mozilla.org/r/256840/#review264218
> 
> r=me with the following issues addressed.

Thanks for the reviews, I will attach a new version after testing it.

Comment 111

[Jacek Caban]

Comment on attachment 8991919 [details]
Bug 1390583 - Stylo: Build Broken with MinGW

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/256840/diff/1-2/

Comment 112

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Attachment: Bug 1390583 - Stylo: Build Broken with MinGW r=xidorn

Comment 113

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

With reviewboard gone, I needed to resubmit this via phabricator.

Comment 114

[Xidorn Quan [:xidorn] UTC+11]

Comment on attachment 9004914 [details]
Bug 1390583 - Stylo: Build Broken with MinGW r=xidorn

Xidorn Quan [:xidorn] UTC+10 has approved the revision.

Comment 115

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/978b64d7fe7b
Stylo: Build Broken with MinGW r=xidorn

Comment 116

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/978b64d7fe7b