Opened 10 months ago

Closed 7 weeks ago

#28716 closed task (fixed)

Create a mingw-w64-clang project

Reported by: gk Owned by: tbb-team
Priority: Very High Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-rbm, tbb-9.0-must-nightly, TorBrowserTeam201908R, GeorgKoppen201908
Cc: tom, boklm, ahf Actual Points:
Parent ID: #28238 Points:
Reviewer: Sponsor:

Description

The mingw-w64 project is currently GCC based. We should start creating a parallel project, mingw-w64-clang, that helps us introducing this new toolchain step by step to all the other components we need for the Windows bundles.

Child Tickets

TicketStatusOwnerSummaryComponent
#30585closedtbb-teamProvide standalone clang 8 project usable across all platformsApplications/Tor Browser

Change History (69)

comment:1 Changed 10 months ago by gk

Priority: MediumHigh

comment:2 Changed 10 months ago by gk

Note: taskcluster/scripts/misc/build-clang-trunk-mingw.sh is the script we want to look at.

comment:3 Changed 9 months ago by ahf

Cc: ahf added

comment:4 Changed 9 months ago by gk

Keywords: GeorgKoppen201901 added; GeorgKoppen201812 removed

Moving my tickets.

comment:5 Changed 9 months ago by gk

Keywords: TorBrowserTeam201901R added; TorBrowserTeam201812 removed
Status: newneeds_review

Okay, I have something for review. The unsquashed version is available in bug_28716_v2 (https://gitweb.torproject.org/user/gk/tor-browser-build.git/log/?h=bug_28716_v2), the squashed one in bug_28716_v3_squashed (https://gitweb.torproject.org/user/gk/tor-browser-build.git/commit/?h=bug_28716_v3_squashed&id=039f08f5cae5303c5b472a5203096799f0fc6c85). See the commit message of the latter for what I did (mainly following https://hg.mozilla.org/releases/mozilla-esr60/file/a578196b1fe0/taskcluster/scripts/misc/build-clang-trunk-mingw.sh plus a bunch of deviations (I hope I got all of the important ones)).

I am still testing the toolchain(s), but so far the result looks promising: I was able to compile the 64bit Firefox part with a .mozconfig file that resembles the one we ship (I only had to omit the sandbox compilation, as that one is broken right now, which we need to investigate).

comment:6 Changed 9 months ago by gk

I successfully built both our 32bit and 64bit firefox projects using bug_28328 in my tor-browser-build repo.

./rbm/rbm build firefox --target nightly --target torbrowser-windows-x86_64 should be the magical command to invoke (for 64bit; replace "x86_64" with "i686" for 32bit).

I'll look tomorrow into whether this is actually running (I am not even sure if the vanilla esr60 is supposed to be running right now).

And, yes, this is with Stylo enabled. \o/ (but sandboxing disabled as the compilation breaks :( )

Last edited 9 months ago by gk (previous) (diff)

comment:7 Changed 9 months ago by tom

This is missing the patches from this bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1506450
Those haven't been backported to esr60 yet; but they will be at some point.

Another thing that's up in the air is https://bugzilla.mozilla.org/show_bug.cgi?id=1515982 and https://bugzilla.mozilla.org/show_bug.cgi?id=1471698#c9 - the fix in 1515982 doesn't seem to work for me. However I don't think this should afect esr60.

I sent in a build on TC to use this toolchain and see what comes out:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d4b38148421fa501933a5e09f478bc216bbf6905

I haven't tested to confirm it runs at this point. I'll check tomorrow.

I did a file-wise comparison:

Differing Files:

Only in tc/clang/include: ansidecl.h
Only in tc/clang/include: bfd.h
Only in tc/clang/include: bfdlink.h
Only in tc/clang/include: c++
Only in tc/clang/include: dis-asm.h
Only in tc/clang/include/llvm/BinaryFormat: WasmRelocs
Only in tc/clang/include/llvm/MC: MCAnalysis
Only in tc/clang/include: plugin-api.h
Only in tc/clang/include: symcat.h
Only in tc/clang/lib/clang/8.0.0/include: sanitizer
Only in tc/clang/lib/clang/8.0.0/include: xray
Only in tc/clang/lib/clang/8.0.0/lib: linux
Only in tc/clang/lib/clang/8.0.0: share
Only in tc/clang/lib: gcc
Only in tc/clang/lib: libasan.a
Only in tc/clang/lib: libasan.la
Only in tc/clang/lib: libasan_preinit.o
Only in tc/clang/lib: libasan.so
Only in tc/clang/lib: libasan.so.1
Only in tc/clang/lib: libasan.so.1.0.0
Only in tc/clang/lib: libatomic.a
Only in tc/clang/lib: libatomic.la
Only in tc/clang/lib: libatomic.so
Only in tc/clang/lib: libatomic.so.1
Only in tc/clang/lib: libatomic.so.1.1.0
Only in tc/clang/lib: libc++.a
Only in tc/clang/lib: libc++abi.a
Only in tc/clang/lib: libc++abi.so
Only in tc/clang/lib: libc++abi.so.1
Only in tc/clang/lib: libc++abi.so.1.0
Only in tc/clang/lib: libc++experimental.a
Only in tc/clang/lib: libc++fs.a
Only in tc/clang/lib: libcilkrts.a
Only in tc/clang/lib: libcilkrts.la
Only in tc/clang/lib: libcilkrts.so
Only in tc/clang/lib: libcilkrts.so.5
Only in tc/clang/lib: libcilkrts.so.5.0.0
Only in tc/clang/lib: libcilkrts.spec
Only in tc/clang/lib: libc++.so
Only in tc/clang/lib: libc++.so.1
Only in tc/clang/lib: libc++.so.1.0
Only in tc/clang/lib: libgcc_s.so
Only in tc/clang/lib: libgcc_s.so.1
Only in tc/clang/lib: libgomp.a
Only in tc/clang/lib: libgomp.la
Only in tc/clang/lib: libgomp.so
Only in tc/clang/lib: libgomp.so.1
Only in tc/clang/lib: libgomp.so.1.0.0
Only in tc/clang/lib: libgomp.spec
Only in tc/clang/lib: libitm.a
Only in tc/clang/lib: libitm.la
Only in tc/clang/lib: libitm.so
Only in tc/clang/lib: libitm.so.1
Only in tc/clang/lib: libitm.so.1.0.0
Only in tc/clang/lib: libitm.spec
Only in geko/mingw-w64-clang/lib: libLLVMAMDGPUAsmParser.a
Only in geko/mingw-w64-clang/lib: libLLVMAMDGPUAsmPrinter.a
Only in geko/mingw-w64-clang/lib: libLLVMAMDGPUCodeGen.a
Only in geko/mingw-w64-clang/lib: libLLVMAMDGPUDesc.a
Only in geko/mingw-w64-clang/lib: libLLVMAMDGPUDisassembler.a
Only in geko/mingw-w64-clang/lib: libLLVMAMDGPUInfo.a
Only in geko/mingw-w64-clang/lib: libLLVMAMDGPUUtils.a
Only in geko/mingw-w64-clang/lib: libLLVMBPFAsmParser.a
Only in geko/mingw-w64-clang/lib: libLLVMBPFAsmPrinter.a
Only in geko/mingw-w64-clang/lib: libLLVMBPFCodeGen.a
Only in geko/mingw-w64-clang/lib: libLLVMBPFDesc.a
Only in geko/mingw-w64-clang/lib: libLLVMBPFDisassembler.a
Only in geko/mingw-w64-clang/lib: libLLVMBPFInfo.a
Only in geko/mingw-w64-clang/lib: libLLVMHexagonAsmParser.a
Only in geko/mingw-w64-clang/lib: libLLVMHexagonCodeGen.a
Only in geko/mingw-w64-clang/lib: libLLVMHexagonDesc.a
Only in geko/mingw-w64-clang/lib: libLLVMHexagonDisassembler.a
Only in geko/mingw-w64-clang/lib: libLLVMHexagonInfo.a
Only in geko/mingw-w64-clang/lib: libLLVMLanaiAsmParser.a
Only in geko/mingw-w64-clang/lib: libLLVMLanaiAsmPrinter.a
Only in geko/mingw-w64-clang/lib: libLLVMLanaiCodeGen.a
Only in geko/mingw-w64-clang/lib: libLLVMLanaiDesc.a
Only in geko/mingw-w64-clang/lib: libLLVMLanaiDisassembler.a
Only in geko/mingw-w64-clang/lib: libLLVMLanaiInfo.a
Only in geko/mingw-w64-clang/lib: libLLVMMipsAsmParser.a
Only in geko/mingw-w64-clang/lib: libLLVMMipsAsmPrinter.a
Only in geko/mingw-w64-clang/lib: libLLVMMipsCodeGen.a
Only in geko/mingw-w64-clang/lib: libLLVMMipsDesc.a
Only in geko/mingw-w64-clang/lib: libLLVMMipsDisassembler.a
Only in geko/mingw-w64-clang/lib: libLLVMMipsInfo.a
Only in geko/mingw-w64-clang/lib: libLLVMMSP430AsmPrinter.a
Only in geko/mingw-w64-clang/lib: libLLVMMSP430CodeGen.a
Only in geko/mingw-w64-clang/lib: libLLVMMSP430Desc.a
Only in geko/mingw-w64-clang/lib: libLLVMMSP430Info.a
Only in geko/mingw-w64-clang/lib: libLLVMNVPTXAsmPrinter.a
Only in geko/mingw-w64-clang/lib: libLLVMNVPTXCodeGen.a
Only in geko/mingw-w64-clang/lib: libLLVMNVPTXDesc.a
Only in geko/mingw-w64-clang/lib: libLLVMNVPTXInfo.a
Only in geko/mingw-w64-clang/lib: libLLVMPowerPCAsmParser.a
Only in geko/mingw-w64-clang/lib: libLLVMPowerPCAsmPrinter.a
Only in geko/mingw-w64-clang/lib: libLLVMPowerPCCodeGen.a
Only in geko/mingw-w64-clang/lib: libLLVMPowerPCDesc.a
Only in geko/mingw-w64-clang/lib: libLLVMPowerPCDisassembler.a
Only in geko/mingw-w64-clang/lib: libLLVMPowerPCInfo.a
Only in geko/mingw-w64-clang/lib: libLLVMSparcAsmParser.a
Only in geko/mingw-w64-clang/lib: libLLVMSparcAsmPrinter.a
Only in geko/mingw-w64-clang/lib: libLLVMSparcCodeGen.a
Only in geko/mingw-w64-clang/lib: libLLVMSparcDesc.a
Only in geko/mingw-w64-clang/lib: libLLVMSparcDisassembler.a
Only in geko/mingw-w64-clang/lib: libLLVMSparcInfo.a
Only in geko/mingw-w64-clang/lib: libLLVMSystemZAsmParser.a
Only in geko/mingw-w64-clang/lib: libLLVMSystemZAsmPrinter.a
Only in geko/mingw-w64-clang/lib: libLLVMSystemZCodeGen.a
Only in geko/mingw-w64-clang/lib: libLLVMSystemZDesc.a
Only in geko/mingw-w64-clang/lib: libLLVMSystemZDisassembler.a
Only in geko/mingw-w64-clang/lib: libLLVMSystemZInfo.a
Only in geko/mingw-w64-clang/lib: libLLVMXCoreAsmPrinter.a
Only in geko/mingw-w64-clang/lib: libLLVMXCoreCodeGen.a
Only in geko/mingw-w64-clang/lib: libLLVMXCoreDesc.a
Only in geko/mingw-w64-clang/lib: libLLVMXCoreDisassembler.a
Only in geko/mingw-w64-clang/lib: libLLVMXCoreInfo.a
Only in tc/clang/lib: liblsan.a
Only in tc/clang/lib: liblsan.la
Only in tc/clang/lib: liblsan.so
Only in tc/clang/lib: liblsan.so.0
Only in tc/clang/lib: liblsan.so.0.0.0
Only in tc/clang/lib: libquadmath.a
Only in tc/clang/lib: libquadmath.la
Only in tc/clang/lib: libquadmath.so
Only in tc/clang/lib: libquadmath.so.0
Only in tc/clang/lib: libquadmath.so.0.0.0
Only in tc/clang/lib: libsanitizer.spec
Only in tc/clang/lib: libssp.a
Only in tc/clang/lib: libssp.la
Only in tc/clang/lib: libssp_nonshared.a
Only in tc/clang/lib: libssp_nonshared.la
Only in tc/clang/lib: libssp.so
Only in tc/clang/lib: libssp.so.0
Only in tc/clang/lib: libssp.so.0.0.0
Only in tc/clang/lib: libsupc++.a
Only in tc/clang/lib: libsupc++.la
Only in tc/clang/lib: libtsan.a
Only in tc/clang/lib: libtsan.la
Only in tc/clang/lib: libtsan.so
Only in tc/clang/lib: libtsan.so.0
Only in tc/clang/lib: libtsan.so.0.0.0
Only in tc/clang/lib: libubsan.a
Only in tc/clang/lib: libubsan.la
Only in tc/clang/lib: libubsan.so
Only in tc/clang/lib: libubsan.so.0
Only in tc/clang/lib: libubsan.so.0.0.0
Only in tc/clang/lib: libvtv.a
Only in tc/clang/lib: libvtv.la
Only in tc/clang/lib: libvtv.so
Only in tc/clang/lib: libvtv.so.0
Only in tc/clang/lib: libvtv.so.0.0.0
Only in geko/mingw-w64-clang/x86_64-w64-mingw32/include: expandedresources.h
Only in tc/clang/x86_64-w64-mingw32/include: _mingw_print_pop.h
Only in tc/clang/x86_64-w64-mingw32/include: _mingw_print_push.h
Only in geko/mingw-w64-clang/x86_64-w64-mingw32/lib: libntdllcrt.a

comment:8 in reply to:  6 ; Changed 9 months ago by tom

Replying to gk:

I'll look tomorrow into whether this is actually running (I am not even sure if the vanilla esr60 is supposed to be running right now).

A note; my testing indicates you will need the patch from https://bugzilla.mozilla.org/show_bug.cgi?id=1506450 and https://bugzilla.mozilla.org/show_bug.cgi?id=1481633 to get esr60 to run.

Testing x64 opt: https://treeherder.mozilla.org/#/jobs?repo=try&revision=bce4d090bb8ed8a92d887cfb0121b07553084508 does not run. This set of patches backports a bunch of build changes that refactor some stuff. 1481633 is included in it; but that's the only change to FF I think.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=fbb3dab9fe5262f5a9394a6952706894fe15c331 does run. This is the above set of patches plus the single patch listed.

This is just testing the esr60 branch in TC; not using the toolchain you built...

comment:9 Changed 9 months ago by tom

More progress:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6f0a54fc3f08a075308a9bf3aa34edca090db206 (which builds on top of fbb3dab9fe5262f5a9394a6952706894fe15c331) enables the sandbox and runs

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d90df493b2ecb5267009789a47559ebe534d9d87 (which builds on top of 6f0a54fc3f08a075308a9bf3aa34edca090db206) adds pdbs and runs and the pdbs seem to work.

comment:10 in reply to:  7 Changed 9 months ago by gk

Replying to tom:

This is missing the patches from this bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1506450
Those haven't been backported to esr60 yet; but they will be at some point.

Yes, that was kind of intentional at this point for making sure we stay a) closer to esr60 as it is right now and, more importantly, b) I had issues to get the wrapper compiled on 32bit due to some weird header inclusion issue. So, I decided to leave that at the moment.

[snip]

I did a file-wise comparison:

Differing Files:

Yes, I am not surprised given that we build clang differently, and don't copy gcc stuff over. I think at some point we want to match Mozilla closer, given the grown importance of clang for Firefox builds, but not in that bug. I opened #29041 for that.

Only in geko/mingw-w64-clang/x86_64-w64-mingw32/include: expandedresources.h
Only in tc/clang/x86_64-w64-mingw32/include: _mingw_print_pop.h
Only in tc/clang/x86_64-w64-mingw32/include: _mingw_print_push.h
Only in geko/mingw-w64-clang/x86_64-w64-mingw32/lib: libntdllcrt.a

Those four items look interesing, though...

comment:11 in reply to:  8 ; Changed 9 months ago by gk

Keywords: TorBrowserTeam201901 added; TorBrowserTeam201901R removed
Status: needs_reviewneeds_revision

Replying to tom:

Replying to gk:

I'll look tomorrow into whether this is actually running (I am not even sure if the vanilla esr60 is supposed to be running right now).

A note; my testing indicates you will need the patch from https://bugzilla.mozilla.org/show_bug.cgi?id=1506450 and https://bugzilla.mozilla.org/show_bug.cgi?id=1481633 to get esr60 to run.

Why would we need 1481633 fixed on esr60 if the reason it came up (1471132) did not land on esr60? Moreover, from skimming that bug it seems to me that's an issue we should hit during compilation, if at all, which I did not.

Testing x64 opt: https://treeherder.mozilla.org/#/jobs?repo=try&revision=bce4d090bb8ed8a92d887cfb0121b07553084508 does not run. This set of patches backports a bunch of build changes that refactor some stuff. 1481633 is included in it; but that's the only change to FF I think.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=fbb3dab9fe5262f5a9394a6952706894fe15c331 does run. This is the above set of patches plus the single patch listed.

Okay, I guess that's a strong indication of including the fix for bug 1506450 then. Marking this ticket as needs_revision

comment:12 in reply to:  9 Changed 9 months ago by gk

Replying to tom:

More progress:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6f0a54fc3f08a075308a9bf3aa34edca090db206 (which builds on top of fbb3dab9fe5262f5a9394a6952706894fe15c331) enables the sandbox and runs

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d90df493b2ecb5267009789a47559ebe534d9d87 (which builds on top of 6f0a54fc3f08a075308a9bf3aa34edca090db206) adds pdbs and runs and the pdbs seem to work.

Yay!

comment:13 in reply to:  11 ; Changed 9 months ago by gk

Replying to gk:

Replying to tom:

Replying to gk:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=fbb3dab9fe5262f5a9394a6952706894fe15c331 does run. This is the above set of patches plus the single patch listed.

Okay, I guess that's a strong indication of including the fix for bug 1506450 then. Marking this ticket as needs_revision

I've updated bug_28716_v2 to add the changes coming with bug 1506450 and rebased bug_28238_v2 on top of it (after adding the mozconfig changes due to the fix for 1506450). 64bit is still compiling fine, I am looking into the header path issue for 32bit.

FWIW: I have not patched our Firefox related build instructions so that they bundle the urct dlls instead of msvcr100 yet. That will happen in #28238.

comment:14 in reply to:  13 ; Changed 9 months ago by tom

Replying to gk:

I've updated bug_28716_v2 to add the changes coming with bug 1506450 and rebased bug_28238_v2 on top of it (after adding the mozconfig changes due to the fix for 1506450). 64bit is still compiling fine, I am looking into the header path issue for 32bit.

FWIW: I have not patched our Firefox related build instructions so that they bundle the ucrt dlls instead of msvcr100 yet. That will happen in #28238.

I built https://treeherder.mozilla.org/#/jobs?repo=try&revision=c7ca3a0df8ec659bbe75322bea4ab296a91a1c50 with the toolchain from this comment.

It builds on top of d90df493b2ecb5267009789a47559ebe534d9d87 from above and includes these two patches:
https://hg.mozilla.org/try/rev/9857214ebf6b (use your toolchain)
https://hg.mozilla.org/try/rev/00a398b1fa4d4c79cf5651635bed03049378b0b2 (disable check_nsmodules because your toolchain doesn't contain a llvm-readelf -> x86_64-blah-readelf symlink which is needed for this)

I tested the x64 opt build; and it runs and loaded a few websites.

Last edited 9 months ago by gk (previous) (diff)

comment:15 in reply to:  11 Changed 9 months ago by tom

Replying to gk:

Why would we need 1481633 fixed on esr60 if the reason it came up (1471132) did not land on esr60? Moreover, from skimming that bug it seems to me that's an issue we should hit during compilation, if at all, which I did not.

Hrm. nsModule compilation might have been broken before 1471132 came along; and we just didn't have a mingw-clang build consistently working then.

A few different things can go wrong with nsModules: at compile time you could have missing symbols. Or at runtime, the resulting symbols in the binary might not be packed or ordered correctly. We do some ugly stuff were we basically make an array in the resulting binary, and then iterate through it like it was an array. If the layout, padding, or order is wrong, the code assumptions break and you get a runtime problem.

comment:16 in reply to:  14 ; Changed 9 months ago by gk

Replying to tom:

Replying to gk:

I've updated bug_28716_v2 to add the changes coming with bug 1506450 and rebased bug_28238_v2 on top of it (after adding the mozconfig changes due to the fix for 1506450). 64bit is still compiling fine, I am looking into the header path issue for 32bit.

FWIW: I have not patched our Firefox related build instructions so that they bundle the urct dlls instead of msvcr100 yet. That will happen in #28238.

I built https://treeherder.mozilla.org/#/jobs?repo=try&revision=c7ca3a0df8ec659bbe75322bea4ab296a91a1c50 with the toolchain from this comment.

It builds on top of d90df493b2ecb5267009789a47559ebe534d9d87 from above and includes these two patches:
https://hg.mozilla.org/try/rev/9857214ebf6b (use your toolchain)
https://hg.mozilla.org/try/rev/00a398b1fa4d4c79cf5651635bed03049378b0b2 (disable check_nsmodules because your toolchain doesn't contain a llvm-readelf -> x86_64-blah-readelf symlink which is needed for this)

I tested the x64 opt build; and it runs and loaded a few websites.

That's on Windows 10, right? I tested the artifact (https://queue.taskcluster.net/v1/task/X5K6N8O8SMqb9RarbIrp2w/runs/0/artifacts/public/build/target.zip) on Windows 7 and it seems to crash early on. I have not looked closer why but I noticed that firefox.exe etc. is still linked against msvcrt.dll which is not working for Windows 7. So, we need to teach the toolchain again how to link against a non-default runtime lib (ucrt this time) which has always been a fun exercise...

comment:17 in reply to:  16 Changed 9 months ago by watt

Replying to gk:

So, we need to teach the toolchain again how to link against a non-default runtime lib (ucrt this time) which has always been a fun exercise...

It is a default c runtime lib in msvc...

(tom: dxr seems to go nuts https://dxr.mozilla.org/mozilla-central/source/old-configure#3526)

comment:18 Changed 9 months ago by tom

A few more builds (all successful):

esr60 rebased to tip with the build refactoring: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0659d6e265f3b624ad6fbac0c9cd7ce246094596

+ resources, sandbox, and pdbs: https://treeherder.mozilla.org/#/jobs?repo=try&revision=54a3650afb36209f6d7dabe3e445402755904776

+ accessibility: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d3404a63b28e34208551fc8d859d7acb6db3b9c9

+ your toolchain: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e4b1f80f6e24a6a1e53588ee70d46f9c37f194e0

I don't know what is up with the runtime; I asked Jacek and he said "it's in mingw-w64-crt configure and it seems that the script passes --with-default-msvcrt=ucrt, which should do the right thing". (And yes, I am on Windows 10.) I think I have a Windows 7 VM; I will have to check when I have time...

comment:19 in reply to:  18 Changed 9 months ago by watt

Replying to tom:
Shouldn't clang be built with mingw-w64 headers (and not before)?

default_win32_winnt=0x601 - for better threading support.

(What's up with dxr?)

comment:20 Changed 9 months ago by gk

While trying to shave off bits from the toolchain complexity I realized we can omit the binutils dependency by symlinking llvm-nm to $arch-w64-mingw32-nm. Tom, does that work for you as well?

comment:21 in reply to:  18 ; Changed 9 months ago by gk

Replying to tom:

A few more builds (all successful):

esr60 rebased to tip with the build refactoring: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0659d6e265f3b624ad6fbac0c9cd7ce246094596

+ resources, sandbox, and pdbs: https://treeherder.mozilla.org/#/jobs?repo=try&revision=54a3650afb36209f6d7dabe3e445402755904776

+ accessibility: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d3404a63b28e34208551fc8d859d7acb6db3b9c9

+ your toolchain: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e4b1f80f6e24a6a1e53588ee70d46f9c37f194e0

Inspecting the binaries a bit shows that neither the results compiled with moz's nor with our toolchain have ASLR enabled. Is that a known issue? (DEP is fine as is SEH)

comment:22 in reply to:  20 Changed 9 months ago by tom

Replying to gk:

While trying to shave off bits from the toolchain complexity I realized we can omit the binutils dependency by symlinking llvm-nm to $arch-w64-mingw32-nm. Tom, does that work for you as well?

binutil's nm was originally needed; but the original need may be gone now; so I will check into this and ASLR.

comment:23 Changed 9 months ago by gk

Keywords: TorBrowserTeam201901R added; TorBrowserTeam201901 removed
Status: needs_revisionneeds_review

Alright, this looks good so far and is up for review again: The unsquashed commits are still available in bug_28716_v2 (https://gitweb.torproject.org/user/gk/tor-browser-build.git/log/?h=bug_28716_v2). The squashed branch (again with hopefully all relevant deviations from the script currently on esr60 explained) is bug_28716_v4_squashed (https://gitweb.torproject.org/user/gk/tor-browser-build.git/commit/?h=bug_28716_v4_squashed&id=8e0efcb8f4df2fee4eaea015acdfc52b0862e26b).

comment:24 in reply to:  21 Changed 9 months ago by ld

Replying to gk:

Inspecting the binaries a bit shows that neither the results compiled with moz's nor with our toolchain have ASLR enabled. Is that a known issue?

https://reviews.llvm.org/D40017
"Disable dynamicbase by default since it potentially could cause compatibility issues" - most likely, that's due to --dynamicbase is broken in ld after refactoring (removed .reloc section). Bad default for lld.

(DEP is fine as is SEH)

(SEH is not fine, even on Win64, as is CFG)

comment:26 Changed 9 months ago by gk

Keywords: TorBrowserTeam201901 added; TorBrowserTeam201901R removed
Status: needs_reviewneeds_revision

ln -s llvm-readobj [% c("arch") %]-w64-mingw32-readobj is needed too (now)

comment:27 Changed 9 months ago by gk

Hm, so for 32bit it seems we are running out of memory during linking xul.dll which did not happen before. I verify that with a second machine.

comment:28 in reply to:  27 ; Changed 9 months ago by gk

Replying to gk:

Hm, so for 32bit it seems we are running out of memory during linking xul.dll which did not happen before. I verify that with a second machine.

It seems bug 1475562 is the reason for that. I wonder how the 32bit build does not fail on Mozilla's side, but it's at least not a toolchain issue.

comment:29 in reply to:  28 Changed 9 months ago by tom

Replying to gk:

Replying to gk:

Hm, so for 32bit it seems we are running out of memory during linking xul.dll which did not happen before. I verify that with a second machine.

It seems bug 1475562 is the reason for that. I wonder how the 32bit build does not fail on Mozilla's side, but it's at least not a toolchain issue.

IIRC we build on c4.4xlarge and c5.4xlarge which have 30 and 32 GB of RAM....

comment:30 in reply to:  26 Changed 9 months ago by gk

Keywords: TorBrowserTeam201901R added; TorBrowserTeam201901 removed
Status: needs_revisionneeds_review

Replying to gk:

ln -s llvm-readobj [% c("arch") %]-w64-mingw32-readobj is needed too (now)

This is fixed in commit 893aff78867ce85a89d23cb4b9418bb03837bbca on bug_28716_v2 (https://gitweb.torproject.org/user/gk/tor-browser-build.git/log/?h=bug_28716_v2) and I bumped the mingw-w64 commit, too, on that branch (commit 9821668c3fb06c3abe502695635d3dc2f6b8a071) to match Mozilla.

It turns out that the symlink to llvm-nm is not needed. I am not sure why there is in the Mozilla build script. I omitted it while adjusting the build script for readobj.

A squashed version is available as well: bug_28716_v6_squashed (https://gitweb.torproject.org/user/gk/tor-browser-build.git/commit/?h=bug_28716_v6_squashed&id=cfbca8437407f3716d910fec71213a5013c54c3a).

comment:31 Changed 9 months ago by gk

Keywords: TorBrowserTeam201901 added; TorBrowserTeam201901R removed
Status: needs_reviewneeds_revision

That needs another revision. I think we should make sure the project gets rebuilt once at least one of the dependencies does need to (e.g. because we need a compiler or a linker patch). That's currently not the case.

comment:32 in reply to:  31 Changed 9 months ago by boklm

Replying to gk:

That needs another revision. I think we should make sure the project gets rebuilt once at least one of the dependencies does need to (e.g. because we need a compiler or a linker patch). That's currently not the case.

Yes, I think a [% c("var/build_id") %] is missing in the filename of some of the projects.

comment:33 Changed 8 months ago by gk

Keywords: TorBrowserTeam201902R GeorgKoppen201902 added; TorBrowserTeam201901 GeorgKoppen201901 removed
Status: needs_revisionneeds_review

I think I am pretty close to the toolchain I am happy with. I pushed another couple of commits to bug_28716_v2 (https://gitweb.torproject.org/user/gk/tor-browser-build.git/log/?h=bug_28716_v2) and squashed everything into bug_28716_v8_squashed (https://gitweb.torproject.org/user/gk/tor-browser-build.git/log/?h=bug_28716_v8_squashed). That branch has two commits: 736c85a9e533014c70017a9b5a19b36353d06540 has the squashed contents for providing mingw-w64/clang and 7322ee826b486039aa007dc94891d2b7218ace1c on top of that provides an lld patch for optionally dealing with PE header timestamp issues (by zeroing them out similar to ld's -Wl,--no-insert-timestamp). I'll try to get that one upstreamed.

I played a bit with bumping the llvm revision to r351992 in order to get a proper llvm-strip and llvm-objcopy but run into a bunch of issues which made me pause for now (see: https://bugzilla.mozilla.org/show_bug.cgi?id=1471698 for context).

FWIW: that's 9.0a1 material, too.

comment:34 in reply to:  33 ; Changed 8 months ago by cypherpunks

Replying to gk:

for providing mingw-w64/clang

The project is mature enough to be self-hosted and decided to have a name 'LLVM', and clang is used only as a name of one of its front-ends.

4) We are omitting the DEBUG_FLAGS (-g -gcodeview)

But not omitting https://hg.mozilla.org/releases/mozilla-esr60/rev/434bde49b9c8#l3.20

lld patch for optionally dealing with PE header timestamp issues (by zeroing them out similar to ld's -Wl,--no-insert-timestamp). I'll try to get that one upstreamed.

Could you also try to get upstreamed that:

-  if (Args.getLastArgValue(OPT_m) != "thumb2pe" &&
-    Args.getLastArgValue(OPT_m) != "arm64pe" && !Args.hasArg(OPT_dynamicbase))
-      Add("-dynamicbase:no");

because it's a shame in 2019.

I played a bit with bumping the llvm revision to r351992 in order to get a proper llvm-strip and llvm-objcopy but run into a bunch of issues which made me pause for now (see: https://bugzilla.mozilla.org/show_bug.cgi?id=1471698 for context).

Why not use 8.x branch as Mozilla?

comment:35 in reply to:  34 ; Changed 8 months ago by gk

Replying to cypherpunks:

Replying to gk:

for providing mingw-w64/clang

The project is mature enough to be self-hosted and decided to have a name 'LLVM', and clang is used only as a name of one of its front-ends.

Yeah, the name mingw-w64-clang is good for the rbm project it shows that more than one part is involved and it's often contrasted with mignw-w64/gcc. So I think we are fine here.

4) We are omitting the DEBUG_FLAGS (-g -gcodeview)

But not omitting https://hg.mozilla.org/releases/mozilla-esr60/rev/434bde49b9c8#l3.20

Yeah, I think we should make that optional, too. Mostly because we can't build the 32bit version as linking fails due to memory issues on my machines at least. And it might be better suited for setting something like ac_add_options --enable-debug-symbols.

lld patch for optionally dealing with PE header timestamp issues (by zeroing them out similar to ld's -Wl,--no-insert-timestamp). I'll try to get that one upstreamed.

Could you also try to get upstreamed that:

-  if (Args.getLastArgValue(OPT_m) != "thumb2pe" &&
-    Args.getLastArgValue(OPT_m) != "arm64pe" && !Args.hasArg(OPT_dynamicbase))
-      Add("-dynamicbase:no");

because it's a shame in 2019.

We make that explicit by passing on -Wl,--dynamicbase, which seems sufficient to me without extra work and discussion.

I played a bit with bumping the llvm revision to r351992 in order to get a proper llvm-strip and llvm-objcopy but run into a bunch of issues which made me pause for now (see: https://bugzilla.mozilla.org/show_bug.cgi?id=1471698 for context).

Why not use 8.x branch as Mozilla?

We do, but the idea was to have proper fixes for llvm-strip and llvm-objcopy included instead of the hacks/workarounds we and Mozilla have currently in place instead.

comment:36 in reply to:  35 ; Changed 8 months ago by cypherpunks

Replying to gk:

Replying to cypherpunks:

Replying to gk:

for providing mingw-w64/clang

The project is mature enough to be self-hosted and decided to have a name 'LLVM', and clang is used only as a name of one of its front-ends.

Yeah, the name mingw-w64-clang is good for the rbm project it shows that more than one part is involved and it's often contrasted with mignw-w64/gcc. So I think we are fine here.

It could be good if you meant gcc, but not GCC, with mingw-w64/gcc.

4) We are omitting the DEBUG_FLAGS (-g -gcodeview)

But not omitting https://hg.mozilla.org/releases/mozilla-esr60/rev/434bde49b9c8#l3.20

Yeah, I think we should make that optional, too. Mostly because we can't build the 32bit version as linking fails due to memory issues on my machines at least. And it might be better suited for setting something like ac_add_options --enable-debug-symbols.

Looks like a bug.

lld patch for optionally dealing with PE header timestamp issues (by zeroing them out similar to ld's -Wl,--no-insert-timestamp). I'll try to get that one upstreamed.

Could you also try to get upstreamed that:

-  if (Args.getLastArgValue(OPT_m) != "thumb2pe" &&
-    Args.getLastArgValue(OPT_m) != "arm64pe" && !Args.hasArg(OPT_dynamicbase))
-      Add("-dynamicbase:no");

because it's a shame in 2019.

We make that explicit by passing on -Wl,--dynamicbase, which seems sufficient to me without extra work and discussion.

From which try? And how many other flags are missing/improperly passed? Silently, without discussion, yeah.

I played a bit with bumping the llvm revision to r351992 in order to get a proper llvm-strip and llvm-objcopy but run into a bunch of issues which made me pause for now (see: https://bugzilla.mozilla.org/show_bug.cgi?id=1471698 for context).

Why not use 8.x branch as Mozilla?

We do, but the idea was to have proper fixes for llvm-strip and llvm-objcopy included instead of the hacks/workarounds we and Mozilla have currently in place instead.

No, you don't, but LLVM seems is not going to merge those fixes into 8.x :( There is an idea to bump LLVM to rc2 (if you wish), but build llvm-strip and llvm-objcopy separately from trunk.

comment:37 in reply to:  36 ; Changed 8 months ago by gk

Replying to cypherpunks:

Replying to gk:

Replying to cypherpunks:

Replying to gk:

[snip]

4) We are omitting the DEBUG_FLAGS (-g -gcodeview)

But not omitting https://hg.mozilla.org/releases/mozilla-esr60/rev/434bde49b9c8#l3.20

Yeah, I think we should make that optional, too. Mostly because we can't build the 32bit version as linking fails due to memory issues on my machines at least. And it might be better suited for setting something like ac_add_options --enable-debug-symbols.

Looks like a bug.

Yes.

lld patch for optionally dealing with PE header timestamp issues (by zeroing them out similar to ld's -Wl,--no-insert-timestamp). I'll try to get that one upstreamed.

Could you also try to get upstreamed that:

-  if (Args.getLastArgValue(OPT_m) != "thumb2pe" &&
-    Args.getLastArgValue(OPT_m) != "arm64pe" && !Args.hasArg(OPT_dynamicbase))
-      Add("-dynamicbase:no");

because it's a shame in 2019.

We make that explicit by passing on -Wl,--dynamicbase, which seems sufficient to me without extra work and discussion.

From which try? And how many other flags are missing/improperly passed? Silently, without discussion, yeah.

What do you mean with "from which try"? See the patch that landed on esr60 and enabled that for mingw-w64/clang (https://bugzilla.mozilla.org/show_bug.cgi?id=1520308). I you feel we are missing flags compared to what we currently have and what we should have, please file tickets here in case they are not filed yet.

I played a bit with bumping the llvm revision to r351992 in order to get a proper llvm-strip and llvm-objcopy but run into a bunch of issues which made me pause for now (see: https://bugzilla.mozilla.org/show_bug.cgi?id=1471698 for context).

Why not use 8.x branch as Mozilla?

We do, but the idea was to have proper fixes for llvm-strip and llvm-objcopy included instead of the hacks/workarounds we and Mozilla have currently in place instead.

No, you don't, but LLVM seems is not going to merge those fixes into 8.x :( There is an idea to bump LLVM to rc2 (if you wish), but build llvm-strip and llvm-objcopy separately from trunk.

Yes, we *do* use the same revision as Mozilla on esr60: r348363. We could build those things separately but I am not convinced yet that this is worth the effort and the extra complication and potential instability.

comment:38 in reply to:  37 ; Changed 8 months ago by cypherpunks

Replying to gk:

Replying to cypherpunks:

Replying to gk:

Replying to cypherpunks:

Replying to gk:

[snip]

Seems you're burying in it: llvm in mingw-w64-clang, mingw-w64 in mingw-w64-source, LLVM in llvm, mingw-w64-gcc in mingw-w64...
[snip]

lld patch for optionally dealing with PE header timestamp issues (by zeroing them out similar to ld's -Wl,--no-insert-timestamp). I'll try to get that one upstreamed.

Could you also try to get upstreamed that:

-  if (Args.getLastArgValue(OPT_m) != "thumb2pe" &&
-    Args.getLastArgValue(OPT_m) != "arm64pe" && !Args.hasArg(OPT_dynamicbase))
-      Add("-dynamicbase:no");

because it's a shame in 2019.

We make that explicit by passing on -Wl,--dynamicbase, which seems sufficient to me without extra work and discussion.

From which try? And how many other flags are missing/improperly passed? Silently, without discussion, yeah.

What do you mean with "from which try"?

Tries on Try.

See the patch that landed on esr60 and enabled that for mingw-w64/clang (https://bugzilla.mozilla.org/show_bug.cgi?id=1520308).

Yes, there is mentioning about that it's not an optional hardening. Unaddressed.

I you feel we are missing flags compared to what we currently have and what we should have, please file tickets here in case they are not filed yet.

Filed more than a year ago. Unaddressed.

I played a bit with bumping the llvm revision to r351992 in order to get a proper llvm-strip and llvm-objcopy but run into a bunch of issues which made me pause for now (see: https://bugzilla.mozilla.org/show_bug.cgi?id=1471698 for context).

Why not use 8.x branch as Mozilla?

We do, but the idea was to have proper fixes for llvm-strip and llvm-objcopy included instead of the hacks/workarounds we and Mozilla have currently in place instead.

No, you don't, but LLVM seems is not going to merge those fixes into 8.x :( There is an idea to bump LLVM to rc2 (if you wish), but build llvm-strip and llvm-objcopy separately from trunk.

Yes, we *do* use the same revision as Mozilla on esr60: r348363.

It's not Mozilla official. See https://bugzilla.mozilla.org/show_bug.cgi?id=1512921 for the reason.

We could build those things separately but I am not convinced yet that this is worth the effort and the extra complication and potential instability.

So, building binutils now and in LLVM to create wrappers is fine for you, but building standalone llvm (not LLVM) to get those two utils is somewhat different?

comment:39 in reply to:  38 Changed 8 months ago by gk

Replying to cypherpunks:

Replying to gk:

Replying to cypherpunks:

Replying to gk:

Replying to cypherpunks:

Replying to gk:

[snip]

I you feel we are missing flags compared to what we currently have and what we should have, please file tickets here in case they are not filed yet.

Filed more than a year ago. Unaddressed.

So, they are in our bug tracker? Great! We have about 1200 bugs open for Tor Browser and if you look carefully you'll see that the bugs get opened faster than we can fix them. Thus, you are very welcome to provide patches. We'd be happy to review them. This is after all a free software project and we welcome contributions.

I played a bit with bumping the llvm revision to r351992 in order to get a proper llvm-strip and llvm-objcopy but run into a bunch of issues which made me pause for now (see: https://bugzilla.mozilla.org/show_bug.cgi?id=1471698 for context).

Why not use 8.x branch as Mozilla?

We do, but the idea was to have proper fixes for llvm-strip and llvm-objcopy included instead of the hacks/workarounds we and Mozilla have currently in place instead.

No, you don't, but LLVM seems is not going to merge those fixes into 8.x :( There is an idea to bump LLVM to rc2 (if you wish), but build llvm-strip and llvm-objcopy separately from trunk.

Yes, we *do* use the same revision as Mozilla on esr60: r348363.

It's not Mozilla official. See https://bugzilla.mozilla.org/show_bug.cgi?id=1512921 for the reason.

It *is* the one Mozilla uses for mingw-w64/clang builds and thus that is the one we care about right now as it's what is getting tested on Mozilla's infra.

We could build those things separately but I am not convinced yet that this is worth the effort and the extra complication and potential instability.

So, building binutils now and in LLVM to create wrappers is fine for you, but building standalone llvm (not LLVM) to get those two utils is somewhat different?

We don't build binutils, see the patch which is up for review. I am not strictly opposed to build those tools, as I said. It's just that it might be more work than it could be worth it.

comment:40 Changed 8 months ago by cypherpunks

Okay, finally, I have to conclude that we speak different languages. You usually answer something that looks relevant, but wasn't asked. Also you prefer to interpret my answers the way they are usually not intended to. Maybe, it's a canary proof that we should continue our conversations in PM, or it's just your strange way to communicate. Anyway, as it's too time consuming to decipher your messages, and you also try to avoid answering viable questions, we can't make good progress that way.

comment:41 in reply to:  40 Changed 8 months ago by gk

Replying to cypherpunks:

Okay, finally, I have to conclude that we speak different languages. You usually answer something that looks relevant, but wasn't asked. Also you prefer to interpret my answers the way they are usually not intended to.

That's not my intention (and neither is avoiding asnwering viable questions) and I am sorry if that's the impression you get.

comment:42 Changed 8 months ago by gk

One thing we should do in the final revision, I think, is to not symlink every tool we need individually (right now it is just llvm-readobj) but do something like Martin does:

    for exec in ar ranlib nm objcopy strings strip; do
        ln -sf llvm-$exec$EXEEXT $arch-w64-mingw32-$exec$EXEEXT || true
    done

Rust, e.g. is complaining about a missing x86_64-w64-mingw32-ar and we probably can simplify our mozconfig as well that way (assuming RANLIB is just necessary because we don't symlink properly in the first place).

comment:43 Changed 8 months ago by gk

Another thing is we can probably break the mingw-w64-clang project a bit more up. I guess we could factor out the llvm/clang/lld compilation which would make it easier to reuse that part in other projects and we would not need to recompile it if we, say, change some mingw-w64 commit. But I am not so sure about the benefits for doing the same with libcxx* etc.

Last edited 8 months ago by gk (previous) (diff)

comment:44 Changed 8 months ago by cypherpunks33

Some notes:

Bug 28716: Add a mingw-w64-clang toolchain
This commit adds a mingw-w64/clang toolchain

Dash or slash problem. Also this is actually a mingw-w64-llvm toolchain, which looks weird, as it means GNU ABI target that is incompatible with now system/standard/default/universal C runtime. Currently LLVM has almost all we need to make a FOSS toolchain that can have a true Windows target, i.e. MS ABI. Once we can make everything we need Windows-compatible, we should drop MinGW-w64 in the target triple and rename the toolchain to its canonical name 'LLVM toolchain' (llvm-project).

2) We don't build clang in 3 stages but just one like we do for our macOS cross-compilation toolchain.

Hrm, Mozilla also doesn't build clang in 3 stages for their macOS cross-compilation toolchain. Maybe, because, as official GCC docs say "When building a cross compiler, it is not generally possible to do a 3-stage bootstrap of the compiler." But if you are excited in building bootstrapped cross-compilers, let me propose the following:

A way to build a bootstrapped cross-compilation toolchain.

  1. "To build a cross compiler, we recommend first building and installing a native compiler. You can then use the native GCC compiler to build the cross compiler."

But the proposal is to bootstrap the whole LLVM native toolchain on Linux (which will also be useful for doing LLVM builds of TBB for Linux). (stage1 of bootstrap)

  1. Build LLVM cross-compilation toolchain by toolchain from step 1. (stage2)
  2. Build LLVM cross-compilation toolchain in Wine by toolchain from step 2. (yes, cross-compiling from Windows to Linux)
  3. Build LLVM cross-compilation toolchain by toolchain from step 3. (stage3)

This allows us to "Perform a comparison test of the stage2 and stage3 compilers." And, even more, if our LLVM toolchains are reproducible, we should get bit-to-bit identical packages of stage2 and stage3 LLVM cross-compilation toolchains!
What do you think?

3) We don't build libcxx* and compiler-rt for the host system like build-clang.py is doing.

We can do even more as part 1 of the above proposal.

4) We are omitting the DEBUG_FLAGS (-g -gcodeview) as we plan to actually ship the resulting build to users, which seems okay (see bug 1500102).

Could you also move the code block with -g -gcodeview in esr60 to --enable-debug section, so that we can get the fix included in TBB 8.5?

5) We don't add a symlink to llvm-nm as it doesn't seem to be needed.

But we should do it as part of the current GCC 'drop-in replacement' strategy. Also it is officially recommended to do this for all LLVM tools at once (with recommended symlinks: gcc to clang, g++ to clang++ and ld to lld).

comment:45 Changed 8 months ago by cypherpunks33

Additional notes:

The build script follows mostly https://hg.mozilla.org/releases/mozilla-esr60/file/e93a8e6c81dc/taskcluster/scripts/misc/build-clang-trunk-mingw.sh.

echo "Provide either x86 or x64 to specify a toolchain."
There is no reason to ignore all ARM toolchains.

crt_flags="--enable-lib32 --disable-lib64"
crt_flags="--disable-lib32 --enable-lib64"
For bi-arch x86 toolchain only.

WRAPPER_FLAGS="-fsjlj-exceptions"
Better to have no exceptions than that. In short for now: it should be removed. Yes, it requires activating SafeSEH throughout the toolchain by default.

make_flags="-j$(nproc)"
Official builds use Ninja, and we should.

patch -p1 < $HOME_DIR/src/build/build-clang/math.patch
The problem here is that when you add LLVM as a drop-in replacement for GCC (and not only clang for gcc), you shouldn't forget about MinGW-w64 is part of the GNU/GCC toolchain and expects to see GNU C/C++. Therefore, we should compile clang with -std=gnu++14 instead of -std=c++14, used for clang-cl in build-clang.py

git clone https://github.com/llvm-mirror
We should use the official svn repo (deprecated) as in build-clang.py or the official GitHub repo as recommended in https://llvm.org/docs/GettingStarted.html#checkout-llvm-from-git
What's the reason of using unofficial repo from google dog?

compiler_flags="--sysroot \$DIR/../$machine-w64-mingw32 -rtlib=compiler-rt -stdlib=libc++ -fuse-ld=lld $WRAPPER_FLAGS -fuse-cxa-atexit -Qunused-arguments"
Briefly, -rtlib=compiler-rt -stdlib=libc++ -fuse-ld=lld $WRAPPER_FLAGS -fuse-cxa-atexit should be added to defaults in compiler driver as in GCC.

--enable-sdk=all \
Useless.

--with-default-msvcrt=ucrt \
It should be set as default in upstream, because targeting ancient MS VC 7's msvcrt.dll in project that dropped support for Windows XP is a nonsense, and it is forbidden by MS.

                                             CC="$CC" \
                                             AR=llvm-ar \
                                             RANLIB=llvm-ranlib \
                                             DLLTOOL=llvm-dlltool \

and similar. Generally, they shouldn't be needed when symlinks from previous comment are used.

-DCMAKE_C_COMPILER_TARGET=$compiler_rt_machine-windows-gnu \
We want native Windows, not GNU.

# Below, we specify -g -gcodeview to build static libraries with debug information.
Official build-clang.py supports these:
* build_type: The type of build to make. Supported types: Release, Debug, RelWithDebInfo or MinSizeRel.
The bug here seems to be that they add -g -gcodeview to Release instead of RelWithDebInfo.
Also adding build_type feature to our LLVM toolchain could help to make true Release and Debug builds!

-DCMAKE_CXX_FLAGS="${DEBUG_FLAGS} -nostdinc++ -I$SRC_DIR/libcxx/include -DPSAPI_VERSION=2" \
This requires thorough investigation.

-DCMAKE_CXX_FLAGS="${DEBUG_FLAGS} -D_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS -D_LIBCPP_HAS_THREAD_API_WIN32" \
These should be set automatically in upstream.

      -DLIBCXX_ENABLE_EXPERIMENTAL_LIBRARY=OFF \
      -DLIBCXX_ENABLE_FILESYSTEM=OFF \

Really? What else? It's better to minimize the number of explicitly set defines.

To be continued...

Last edited 8 months ago by cypherpunks33 (previous) (diff)

comment:46 Changed 8 months ago by cypherpunks33

Some more notes:
There are different approaches in LLVM to become the default system for everything, and sooner or later it will start adding new features, inherently incompatible with GCC/MSVC/etc. This will require making changes to the current build systems. There are two ways: making build scripts for LLVM or adjusting GCC ones. In the latter we should decide whether to maintain the best compatibility with GCC or transform it into the GCC-incompatible one (which will become a native LLVM one after transition).
IIRC, you usually prefer a transition plan with dropping the previous system support. And you've already started to replace gcc with clang, etc. But it seems it's too early to make incompatible changes. We should check first whether all LLVM tools support that.

I propose to make the full GNU (not only GCC) compatible drop-in replacement.
LLVM even has LLVM_INSTALL_BINUTILS_SYMLINKS=TRUE for that.
Also we should use the latest CMake and Ninja as recommended: "Please use the latest available CMake for your platform to avoid surprises." It is needed for "The check-all target (i.e. ninja check-all) will run the regression tests to ensure everything is in working order." (BTW, where is Mozilla doing it or https://llvm.org/docs/AdvancedBuilds.html#stage-non-determinism after stage3?)

Big thanks for trying to upstream lld patch! It is the right way to go.
Some minor notes:

Since r332613 lld has the option of setting the timestamp in PE headers to 0

Not lld, lld-link. It can set timestamp to any 32-bit value.

However, we need a patch for lld's MingW

Chinese hero Ming W.? :)

to pass this option on in our cross-compilation case

It turns out it's much harder to pass the options properly than hardcode them.

Actually, we need much more changes to lld (and lld-link) to have the required features.
Is it possible to invite Martin to the discussion?

comment:47 Changed 8 months ago by gk

Keywords: TorBrowserTeam201902 added; TorBrowserTeam201902R removed
Status: needs_reviewneeds_revision

comment:48 Changed 7 months ago by gk

Keywords: TorBrowserTeam201903 added; TorBrowserTeam201902 removed

Moving my tickets to March.

comment:49 Changed 7 months ago by gk

Keywords: GeorgKoppen201903 added; GeorgKoppen201902 removed

Now for my keyword.

comment:50 Changed 6 months ago by gk

Keywords: TorBrowserTeam201904 added; TorBrowserTeam201903 removed

Moving tickets to April.

comment:51 Changed 6 months ago by gk

Keywords: GeorgKoppen201904 added; GeorgKoppen201903 removed

Moving my tickets for April

comment:52 Changed 5 months ago by gk

It seems we should switch to clang-8 while we are at it: https://bugzilla.mozilla.org/show_bug.cgi?id=1510897.

comment:53 in reply to:  52 Changed 5 months ago by gk

Replying to gk:

It seems we should switch to clang-8 while we are at it: https://bugzilla.mozilla.org/show_bug.cgi?id=1510897.

However, we might either run into https://bugzilla.mozilla.org/show_bug.cgi?id=1471698 or https://bugzilla.mozilla.org/show_bug.cgi?id=1548624 or both that way. We'll see.

comment:54 Changed 5 months ago by tom

I think we have a workaround for 1471698 presently that wouldn't require updating to a revision that causes us to hit 1548624. I can't remember enough about 1471698 to remember why you did the work on it that you did; and if you felt it was necessary for Tor.

Right now, my biggest concern is https://bugzilla.mozilla.org/show_bug.cgi?id=1547519 which I believe makes the x86 builds too unreliable.

comment:55 in reply to:  54 Changed 5 months ago by gk

Replying to tom:

I think we have a workaround for 1471698 presently that wouldn't require updating to a revision that causes us to hit 1548624. I can't remember enough about 1471698 to remember why you did the work on it that you did; and if you felt it was necessary for Tor.

To use the proper llvm-copy which makes the whole toolchain a bit simpler.

comment:56 Changed 5 months ago by gk

Keywords: TorBrowserTeam201905 added; TorBrowserTeam201904 removed

Moving tickets to May

comment:57 Changed 5 months ago by gk

Keywords: GeorgKoppen201905 added; GeorgKoppen201904 removed

Move my tickets.

comment:58 in reply to:  43 Changed 5 months ago by gk

Replying to gk:

Another thing is we can probably break the mingw-w64-clang project a bit more up. I guess we could factor out the llvm/clang/lld compilation which would make it easier to reuse that part in other projects and we would not need to recompile it if we, say, change some mingw-w64 commit. But I am not so sure about the benefits for doing the same with libcxx* etc.

I opened #30585 for that.

comment:59 Changed 4 months ago by gk

Keywords: TorBrowserTeam201906 added; TorBrowserTeam201905 removed

Moving tickets to June

comment:60 Changed 4 months ago by gk

Keywords: GeorgKoppen201906 added; GeorgKoppen201905 removed

Moving my tickets to June

comment:61 Changed 3 months ago by gk

Keywords: GeorgKoppen201907 added; GeorgKoppen201906 removed

Moving my tickets to July.

comment:62 Changed 3 months ago by gk

Keywords: TorBrowserTeam201907 added; TorBrowserTeam201906 removed

Moving tickets to July

comment:63 Changed 3 months ago by gk

Keywords: tbb-9.0-must-nightly added

Starting with 9.0 keywords

comment:64 Changed 2 months ago by gk

Keywords: TorBrowserTeam201908 added; TorBrowserTeam201907 removed

Moving tickets to August, part 1

comment:65 Changed 8 weeks ago by gk

Keywords: GeorgKoppen201908 added; GeorgKoppen201907 removed

Move my tickets.

comment:66 Changed 7 weeks ago by gk

Keywords: TorBrowserTeam201908R added; TorBrowserTeam201908 removed
Priority: HighVery High
Status: needs_revisionneeds_review

Okay, I updated the toolchain to something close we'll get with ESR68. The result is up for review in bug_28716_v11 (https://gitweb.torproject.org/user/gk/tor-browser-build.git/log/?h=bug_28716_v11). It contains two commits, one for the toolchain (dd62fd43af8f9a49135e600708dd2ee7f7c3d9aa) and one for the first reproducible builds issue we fixed (b81028c143cb5f869d20a267730a334155737063). Please look for further details at the commit messages.

comment:67 Changed 7 weeks ago by gk

I actually forgot to add the patch for https://bugzilla.mozilla.org/show_bug.cgi?id=1536308 which is solved in bug_28716_v12 (https://gitweb.torproject.org/user/gk/tor-browser-build.git/log/?h=bug_28716_v12) and is the only change in this new branch. Please use that one for review instead.

comment:68 Changed 7 weeks ago by gk

bug_28716_v14 (https://gitweb.torproject.org/user/gk/tor-browser-build.git/log/?h=bug_28716_v14) should have the missing llvm-mingw directory now + respective build script and config file. Please use that one for review.

comment:69 Changed 7 weeks ago by boklm

Resolution: fixed
Status: needs_reviewclosed

This is fixed with commits 8aeb04965f9993b7572c7fe98d9ad3e0b2fe4453 and 6ca067d2d716283c47c6b2fb822305e61b16e168.

Note: See TracTickets for help on using tickets.