Martin added support for COFF files in llvm-objcopy, so we may use it now. I'd suggest to update to current LLVM trunk and try to use llvm-objcopy.

(In reply to Jacek Caban from comment #11)

Martin added support for COFF files in llvm-objcopy, so we may use it now.
I'd suggest to update to current LLVM trunk and try to use llvm-objcopy.

I gave it a try and used r351992 which should contain the llvm-objcopy fixes if I see that correctly. I bumped the mingw-w64 revision to 8b2c7826b8d68e2ffc79c286b8792efe4168f666 to avoid the _xgetbv builtin issues.

However, I still run into errors related to that. See e.g. in skia code:

9:15.37 /var/tmp/build/firefox-4d0f9fa5fdd5/gfx/skia/skia/src/core/SkCpu.cpp:20:55: error: '__builtin_ia32_xgetbv' needs target feature xsave
9:15.37 static uint64_t xgetbv(uint32_t xcr) { return _xgetbv(xcr); }
9:15.37 ^
9:15.37 /var/tmp/dist/mingw-w64-clang/lib/clang/9.0.0/include/xsaveintrin.h:49:20: note: expanded from macro '_xgetbv'
9:15.37 #define _xgetbv(A) __builtin_ia32_xgetbv((long long)(A))
9:15.37 ^
9:15.40 1 error generated.

Or when compiling the sandbox:

5:30.69 /var/tmp/build/firefox-4d0f9fa5fdd5/security/sandbox/chromium/base/cpu.cc:84:27: error: expected ')'
5:30.69 uint64_t _xgetbv(uint32_t xcr) {
5:30.69 ^
5:30.69 /var/tmp/build/firefox-4d0f9fa5fdd5/security/sandbox/chromium/base/cpu.cc:84:10: note: to match this '('
5:30.69 uint64_t _xgetbv(uint32_t xcr) {
5:30.69 ^
5:30.69 /var/tmp/dist/mingw-w64-clang/lib/clang/9.0.0/include/xsaveintrin.h:49:53: note: expanded from macro '_xgetbv'
5:30.69 #define _xgetbv(A) __builtin_ia32_xgetbv((long long)(A))
5:30.69 ^
5:30.69 /var/tmp/build/firefox-4d0f9fa5fdd5/security/sandbox/chromium/base/cpu.cc:84:10: error: expected expression
5:30.69 uint64_t _xgetbv(uint32_t xcr) {
5:30.69 ^
5:30.69 /var/tmp/dist/mingw-w64-clang/lib/clang/9.0.0/include/xsaveintrin.h:49:56: note: expanded from macro '_xgetbv'
5:30.69 #define _xgetbv(A) __builtin_ia32_xgetbv((long long)(A))
5:30.69 ^
5:30.69 /var/tmp/build/firefox-4d0f9fa5fdd5/security/sandbox/chromium/base/cpu.cc:84:31: error: expected ';' after top level declarator
5:30.69 uint64_t _xgetbv(uint32_t xcr) {
5:30.69 ^
5:30.69 ;
5:30.69 /var/tmp/build/firefox-4d0f9fa5fdd5/security/sandbox/chromium/base/cpu.cc:187:10: error: called object type 'uint64_t' (aka 'unsigned long long') is not a function or function pointer
5:30.69 (_xgetbv(0) & 6) == 6 /* XSAVE enabled by kernel */;
5:30.69 ^~~~~~~~~~
5:30.69 /var/tmp/dist/mingw-w64-clang/lib/clang/9.0.0/include/xsaveintrin.h:49:41: note: expanded from macro '_xgetbv'
5:30.69 #define _xgetbv(A) __builtin_ia32_xgetbv((long long)(A))
5:30.69 ~~~~~~~~~~~~~~~~~~~~~^
5:30.72 1 warning and 4 errors generated.
5:30.72 /var/tmp/build/firefox-4d0f9fa5fdd5/config/rules.mk:1054: recipe for target 'cpu.o' failed
5:30.72 make[4]: *** [cpu.o] Error 1

That's with ESR 60. I am not sure if that is a problem for mozilla-central, too. Martin: Are those errors expected (i.e. we need updated skia and sandbox code, too) or is the _xgetbv issue not fully resolved yet?

(In reply to Georg Koppen from comment #12)

(In reply to Jacek Caban from comment #11)

Martin added support for COFF files in llvm-objcopy, so we may use it now.
I'd suggest to update to current LLVM trunk and try to use llvm-objcopy.

I gave it a try and used r351992 which should contain the llvm-objcopy fixes if I see that correctly. I bumped the mingw-w64 revision to 8b2c7826b8d68e2ffc79c286b8792efe4168f666 to avoid the _xgetbv builtin issues.

However, I still run into errors related to that. See e.g. in skia code:

9:15.37 /var/tmp/build/firefox-4d0f9fa5fdd5/gfx/skia/skia/src/core/SkCpu.cpp:20:55: error: '__builtin_ia32_xgetbv' needs target feature xsave
9:15.37 static uint64_t xgetbv(uint32_t xcr) { return _xgetbv(xcr); }
9:15.37 ^
9:15.37 /var/tmp/dist/mingw-w64-clang/lib/clang/9.0.0/include/xsaveintrin.h:49:20: note: expanded from macro '_xgetbv'
9:15.37 #define _xgetbv(A) __builtin_ia32_xgetbv((long long)(A))
9:15.37 ^
9:15.40 1 error generated.

Or when compiling the sandbox:

5:30.69 /var/tmp/build/firefox-4d0f9fa5fdd5/security/sandbox/chromium/base/cpu.cc:84:27: error: expected ')'
5:30.69 uint64_t _xgetbv(uint32_t xcr) {
5:30.69 ^

That's with ESR 60. I am not sure if that is a problem for mozilla-central, too. Martin: Are those errors expected (i.e. we need updated skia and sandbox code, too) or is the _xgetbv issue not fully resolved yet?

I don't regularly build the skia and chromium sandbox code, so I don't know, but I would expect them to be resolved very soon, as chromium has got buildbots that run ~daily with latest clang (and they bump the official builds to use a newer clang every couple weeks or so, afaik).

(In reply to Martin Storsjö from comment #13)

(In reply to Georg Koppen from comment #12)

(In reply to Jacek Caban from comment #11)

Martin added support for COFF files in llvm-objcopy, so we may use it now.
I'd suggest to update to current LLVM trunk and try to use llvm-objcopy.

I gave it a try and used r351992 which should contain the llvm-objcopy fixes if I see that correctly. I bumped the mingw-w64 revision to 8b2c7826b8d68e2ffc79c286b8792efe4168f666 to avoid the _xgetbv builtin issues.

However, I still run into errors related to that. See e.g. in skia code:

9:15.37 /var/tmp/build/firefox-4d0f9fa5fdd5/gfx/skia/skia/src/core/SkCpu.cpp:20:55: error: '__builtin_ia32_xgetbv' needs target feature xsave
9:15.37 static uint64_t xgetbv(uint32_t xcr) { return _xgetbv(xcr); }
9:15.37 ^
9:15.37 /var/tmp/dist/mingw-w64-clang/lib/clang/9.0.0/include/xsaveintrin.h:49:20: note: expanded from macro '_xgetbv'
9:15.37 #define _xgetbv(A) __builtin_ia32_xgetbv((long long)(A))
9:15.37 ^
9:15.40 1 error generated.

Or when compiling the sandbox:

5:30.69 /var/tmp/build/firefox-4d0f9fa5fdd5/security/sandbox/chromium/base/cpu.cc:84:27: error: expected ')'
5:30.69 uint64_t _xgetbv(uint32_t xcr) {
5:30.69 ^

That's with ESR 60. I am not sure if that is a problem for mozilla-central, too. Martin: Are those errors expected (i.e. we need updated skia and sandbox code, too) or is the _xgetbv issue not fully resolved yet?

I don't regularly build the skia and chromium sandbox code, so I don't know,
but I would expect them to be resolved very soon, as chromium has got
buildbots that run ~daily with latest clang (and they bump the official
builds to use a newer clang every couple weeks or so, afaik).

Fair enough. Turns out the sandbox issue was already fixed on trunk but the skia one not. I fixed those with attached patches, but there is more broken related to xgetbv in Mozilla code after switching to r351992, alas. :(

(In reply to Georg Koppen from comment #14)

Fair enough. Turns out the sandbox issue was already fixed on trunk but the skia one not. I fixed those with attached patches, but there is more broken related to xgetbv in Mozilla code after switching to r351992, alas. :(

Yes, the _xgetbv change seems to be breaking quite a bit of things all over the place.

The change isn't in the 8.0 release branch (it was committed just before the branch, but reverted due to some breakage, and then reapplied after the branch was made). So if it's generally deemed problematic, perhaps it still can be backed out, but that would require actually reporting the issues to the ones who made the change, instead of just fixing all broken projects silently. So far I've had to fix mingw-w64 and Qt.

I have a green try run for llvm bumped to 359019 (which is right before the bug in Bug 1548624) with three xgetbv patches: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8410a7de12c868559edb782fb2d3efa1da3cadbb

And here's a try run with the objcopy changes and clang on trunk: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d4b2ec95091c09cc8655d9f6bf07fb939da9f345

I tested all four builds and they work.

Okay; I switched to the 8.0 release branch at the urging of GeKo and glandium. Jacek can you take a look at these two patches and confirm they're good, then I'll post them for review: https://treeherder.mozilla.org/#/jobs?repo=try&revision=be9059e23dff13359f594b3bf28717d1b27e5a2f

A couple of comments:

1. Shouldn't the llvm_revision be 356265 like in the other clang-8*.json files?

•  -DCMAKE_CXX_FLAGS="${DEBUG_FLAGS} -Wno-dll-attribute-on-redeclaration -nostdinc++ -I$SRC_DIR/libcxx/include -DPSAPI_VERSION=2" \
  

•   -DCMAKE_C_FLAGS="-Wno-dll-attribute-on-redeclaration" \
  

Indentation.

•  DLLVM_COMPILER_CHECKED=TRUE \
  

It seems you missed a "-" here, but nothing broke. Maybe that option is not needed then?

Okay new try.

Fixed the indent, the dash, and included all the objcopy patches that Martin pointed me to. Build runs and seems good to me. https://treeherder.mozilla.org/#/jobs?repo=try&revision=cf62265191aae0922259dbaec2c07445cb3374bc

binutils part looks good to me. Reverting to branch 8 is a temporary solution does not exactly solve the problem, but I guess that's fine.

As Martin pointed, we could use llvm-objcopy/llvm-strip instead and simplify the toolchain by using those and getting rid of binutils.

Tested this build and it seems to work fine! https://treeherder.mozilla.org/#/jobs?repo=try&revision=be00e317a8f36a376859ab248095185485f1340c

This will match the compiler version Tor would like. We backport several
llvm-objcopy patches that landed right after the 8 branch though. We
also grab some upstream changes from mingw-clang in the build script

Depends on D31347

Pushed by ccoroiu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/adaa62b87e68
Switch the mingw clang compiler to the 8 branch r=froydnj

https://hg.mozilla.org/mozilla-central/rev/adaa62b87e68

Comment on attachment 9065229 [details]
Bug 1471698 - Switch the mingw clang compiler to the 8 branch r?#build

Beta/Release Uplift Approval Request

• User impact if declined: Tor will have to carry the patch and the ESR branch of 68 will use a different compiler than Tor will use.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Affects only the mingw-clang build.
• String changes made/needed:

Comment on attachment 9065229 [details]
Bug 1471698 - Switch the mingw clang compiler to the 8 branch r?#build

mingwclang updates for 68.0b4

Backed out changeset 7de6c5aab8b4 (bug 1471698) for build bustages on Windows MinGW all.

Push that started the failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=superseded%2Ctestfailed%2Cbusted%2Cexception%2Crunnable&revision=7de6c5aab8b4ee7d66e5080b2cefc2a1123dbef9&selectedJob=247759831

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=247759831&repo=autoland&lineNumber=45865

Backout: https://hg.mozilla.org/integration/autoland/rev/ba6a5d200cc5ce1992e159379d637f506d6e0c6c

https://hg.mozilla.org/releases/mozilla-beta/rev/53f8a0553b0e

Now that Bug 1553481 is fixed, we can commit the second half of this (and uplift it to beta).

Pushed by apavel@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/cd5d8525541f
Remove our binutils-corruption-avoiding workaround for mingw-clang r=froydnj

https://hg.mozilla.org/mozilla-central/rev/cd5d8525541f

flagging for beta checkin of part 2.

https://hg.mozilla.org/releases/mozilla-beta/rev/aaeda42f9fbf