Closed Bug 1401647 Opened 2 years ago Closed 2 years ago

Combine win32 and win64 rust toolchains

Categories

(Firefox Build System :: General, enhancement)

enhancement
Not set

Tracking

(firefox57 wontfix, firefox59 fixed)

RESOLVED FIXED
mozilla59
Tracking Status
firefox57 --- wontfix
firefox59 --- fixed

People

(Reporter: rillian, Assigned: ted)

References

Details

Attachments

(4 files, 2 obsolete files)

Our Windows builds are always compiled on 64-bit hosts, so ideally we could use a single, win64-hosted rust toolchain for both 32- and 64-bit targets.

Unfortunately, this doesn't work because of cross-compilation bugs in some crates. This bug is about resolving those issues so we can unify the toolchain artifact we use.
In particular the heapsize crate build script fails.

> error: linking with `link.exe` failed: exit code: 1112
> = note: "link.exe" "/NOLOGO" "/NXCOMPAT" "/LIBPATH:Z:\\task_1505326399\\build\\src\\rustc\\lib\\rustlib\\x86_64-pc-windows-msvc\\lib" "z:/build/build/src/obj-firefox/toolkit/library\\debug\\build\\heapsize-898b905972f36ec2\\build_script_build-898b905972f36ec2.0.o" "z:/build/build/src/obj-firefox/toolkit/library\\debug\\build\\heapsize-898b905972f36ec2\\build_script_build-898b905972f36ec2.1.o" "z:/build/build/src/obj-firefox/toolkit/library\\debug\\build\\heapsize-898b905972f36ec2\\build_script_build-898b905972f36ec2.2.o" "z:/build/build/src/obj-firefox/toolkit/library\\debug\\build\\heapsize-898b905972f36ec2\\build_script_build-898b905972f36ec2.3.o" "/OUT:z:/build/build/src/obj-firefox/toolkit/library\\debug\\build\\heapsize-898b905972f36ec2\\build_script_build-898b905972f36ec2.exe" "/OPT:REF,ICF" "/DEBUG" "/LIBPATH:z:/build/build/src/obj-firefox/toolkit/library\\debug\\deps" "/LIBPATH:Z:\\task_1505326399\\build\\src\\rustc\\lib\\rustlib\\x86_64-pc-windows-msvc\\lib" "Z:\\task_1505326399\\build\\src\\rustc\\lib\\rustlib\\x86_64-pc-windows-msvc\\lib\\libstd-26aaf8685d64fee9.rlib" "Z:\\task_1505326399\\build\\src\\rustc\\lib\\rustlib\\x86_64-pc-windows-msvc\\lib\\librand-ce86047000b56785.rlib" "Z:\\task_1505326399\\build\\src\\rustc\\lib\\rustlib\\x86_64-pc-windows-msvc\\lib\\libcollections-b8b9a576d130e244.rlib" "Z:\\task_1505326399\\build\\src\\rustc\\lib\\rustlib\\x86_64-pc-windows-msvc\\lib\\libstd_unicode-9fbe5d3bbc85c563.rlib" "Z:\\task_1505326399\\build\\src\\rustc\\lib\\rustlib\\x86_64-pc-windows-msvc\\lib\\libpanic_unwind-14e8bb7ca07ebf2c.rlib" "Z:\\task_1505326399\\build\\src\\rustc\\lib\\rustlib\\x86_64-pc-windows-msvc\\lib\\libunwind-a4bc20050f473f79.rlib" "Z:\\task_1505326399\\build\\src\\rustc\\lib\\rustlib\\x86_64-pc-windows-msvc\\lib\\liblibc-892bd58ec617c3bd.rlib" "Z:\\task_1505326399\\build\\src\\rustc\\lib\\rustlib\\x86_64-pc-windows-msvc\\lib\\liballoc-b9c9173c47c20c41.rlib" "Z:\\task_1505326399\\build\\src\\rustc\\lib\\rustlib\\x86_64-pc-windows-msvc\\lib\\liballoc_system-141f235246f01712.rlib" "Z:\\task_1505326399\\build\\src\\rustc\\lib\\rustlib\\x86_64-pc-windows-msvc\\lib\\libcore-3a6338503b91076c.rlib" "Z:\\task_1505326399\\build\\src\\rustc\\lib\\rustlib\\x86_64-pc-windows-msvc\\lib\\libcompiler_builtins-e9e280acad4314a4.rlib" "advapi32.lib" "ws2_32.lib" "userenv.lib" "shell32.lib" "msvcrt.lib"
> = note: msvcrt.lib(chkstk.obj) : fatal error LNK1112: module machine type 'X86' conflicts with target machine type 'x64'
> error: aborting due to previous error(s)
> error: Could not compile `heapsize`. 
> Caused by:
>  process didn't exit successfully: `z:/build/build/src/sccache2/sccache.exe z:/build/build/src/rustc/bin/rustc.exe --crate-name build_script_build Z:\build\build\src\third_party\rust\heapsize\build.rs --crate-type bin --emit=dep-info,link -C opt-level=1 -C codegen-units=4 -C debuginfo=2 -C debug-assertions=on -C metadata=898b905972f36ec2 -C extra-filename=-898b905972f36ec2 --out-dir z:/build/build/src/obj-firefox/toolkit/library\debug\build\heapsize-898b905972f36ec2 -L dependency=z:/build/build/src/obj-firefox/toolkit/library\debug\deps --cap-lints allow` (exit code: 101)

It works for me on a local windows build of the crate, so this may be something with our automation environment.
Just a note that this is still reproducible, although in the latest try push the failures were on the build scripts of rayon-core, libloading, and webrender, which suggests it's a general problem with out environment rather than an issues with a specific build.rs making assumptions.
It could be related to bug 1409276
Blocks: 1417268
This sounds strikingly similar to bug 1350001, honestly. I think the issue is that rustc is invoking link.exe to link the build script binary, for which it has compiled native 64-bit objects. However, our Windows mozconfigs set INCLUDE/LIB/etc in the environment so that link.exe winds up looking at 32-bit system libraries, so it tries to link a 32-bit msvcrt.lib with the 64-bit object files and fails.

The fix for bug 1350001 was to clear the environment and let rustc find MSVC on its own, which works fine for developer machines where MSVC is installed normally. In automation that won't work.

.cargo/config does allow specifying the linker to use for specific targets using `[target.$triple] linker = "..."`:
http://doc.crates.io/config.html#configuration-keys

...but that wouldn't quite solve our problem because we would also need to unset LIB/INCLUDE/etc.
I guess we could write a Python script that would invoke the linker in the proper environment and specify that as the linker in .cargo/config?
rustc has special handling for specifying a .bat file as the linker on Windows, so we should be able to use that:
https://github.com/rust-lang/rust/blob/3dfbc88a626625be01e112da11ec367e2fc71bb3/src/librustc_trans/back/link.rs#L63
stealin' this...
Assignee: giles → ted
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #14)
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=8fea1cd995b23545bd596824e97941500c56f2be

I had missed a spot in test_toolchain_configure where it mocks the output of `rustc --version --verbose` so those tests failed. The latest try push was all green, though.
Comment on attachment 8938091 [details]
Bug 1401647 - Add i686 target to win64-rust.

https://reviewboard.mozilla.org/r/207906/#review214548
Attachment #8938091 - Flags: review?(ted) → review+
Attachment #8936866 - Attachment is obsolete: true
Attachment #8936867 - Attachment is obsolete: true
Comment on attachment 8938092 [details]
bug 1401647 - use a 64-bit Rust toolchain for win32 builds.

https://reviewboard.mozilla.org/r/207908/#review214584

Thanks for getting this working. Let's ship it!
Attachment #8938092 - Flags: review?(giles) → review+
Pushed by tmielczarek@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0542716bb901
Add i686 target to win64-rust. r=ted
https://hg.mozilla.org/integration/autoland/rev/b5c9bb05168d
use a 64-bit Rust toolchain for win32 builds. r=rillian
I worked around the SM build failure like this: https://hg.mozilla.org/try/rev/eab5abee00b8531f452923d7feb248f6f71c8b8e

No idea whether this is the "right" way to do it.
(In reply to David Major [:dmajor] from comment #23)
> I worked around the SM build failure like this:
> https://hg.mozilla.org/try/rev/eab5abee00b8531f452923d7feb248f6f71c8b8e
> 
> No idea whether this is the "right" way to do it.

Well, `| <` is certainly not going to fix anything. My brain is still on holiday, apparently.
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #25)
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=fffbfe05f2f003b51734f5baa96cd667104a8eee

I triggered the failing SM jobs on this try push and they're green with this additional patch. I did a slightly different version of dmajor's patch. I'll get my patches rebased and get that patch up for review.
Flags: needinfo?(ted)
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #30)
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=0da027f54d27888a7e5191fc12846d7971a36217

I tweaked the regex a bit so I retriggered the SM jobs just to be sure.
Comment on attachment 8938092 [details]
bug 1401647 - use a 64-bit Rust toolchain for win32 builds.

https://reviewboard.mozilla.org/r/207908/#review215884

This looks fine.  rillian is the expert here, I think, so if you want more domain knowledge, flag him directly.
Attachment #8938092 - Flags: review+
Comment on attachment 8939883 [details]
bug 1401647 - Fix spidermonkey mozjs / rust-bindings builds.

https://reviewboard.mozilla.org/r/210194/#review215890

I'll take this, but it would be much better to force an error rather than (potentially) silently failing.  That's what `Preprocessor.py` would do.
Attachment #8939883 - Flags: review+
Attachment #8938092 - Flags: review?(core-build-config-reviews)
Attachment #8939883 - Flags: review?(core-build-config-reviews)
Comment on attachment 8939883 [details]
bug 1401647 - Fix spidermonkey mozjs / rust-bindings builds.

https://reviewboard.mozilla.org/r/210194/#review215896
Comment on attachment 8939883 [details]
bug 1401647 - Fix spidermonkey mozjs / rust-bindings builds.

https://reviewboard.mozilla.org/r/210194/#review215890

I agree, but I just don't want to deal with that in a shell script. :) I'm hoping that in the near future Spidermonkey builds will wind up using Rust and this stuff can go away because it'll just be handled by the existing configure machinery.
Pushed by tmielczarek@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6a0ae401534f
Add i686 target to win64-rust. r=ted
https://hg.mozilla.org/integration/autoland/rev/273a99be7191
use a 64-bit Rust toolchain for win32 builds. r=nalexander,rillian
https://hg.mozilla.org/integration/autoland/rev/e2d86922b3ef
Fix spidermonkey mozjs / rust-bindings builds. r=nalexander
Pushed by apavel@mozilla.com:
https://hg.mozilla.org/mozilla-central/rev/d098b3eb9f97
Add i686 target to win64-rust. r=ted
https://hg.mozilla.org/mozilla-central/rev/98ec3a378b91
use a 64-bit Rust toolchain for win32 builds. r=nalexander,rillian
https://hg.mozilla.org/mozilla-central/rev/982eb37f52f6
Fix spidermonkey mozjs / rust-bindings builds. r=nalexander
Blocks: 1414287
This slightly improved at least two of our platform_microbenchmark performance tests:

== Change summary for alert #11087 (as of Thu, 04 Jan 2018 15:38:42 GMT) ==

Improvements:

  5%  Stylo Servo_DeclarationBlock_GetPropertyById_Bench windows7-32 opt      246,421.65 -> 233,046.08
  4%  Strings PerfIsASCIIHundred windows7-32 opt                              2,983.96 -> 2,853.50

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=11087
If it did, that suggests rustc 1.23 beta generated slower code than 1.22, since this bug effectively downgraded to 1.22.
Depends on: 1429947
Product: Core → Firefox Build System
Comment on attachment 8960649 [details]
Port Bug 1401647 (98ec3a378b91): use a 64-bit Rust toolchain for win32 builds; r=Fallen

Philipp Kewisch [:Fallen]  has approved the revision.

https://phabricator.services.mozilla.com/D778
Attachment #8960649 - Flags: review+
Pushed by mozilla@hocat.ca:
https://hg.mozilla.org/comm-central/rev/ace989585be9
Port Bug 1401647 (98ec3a378b91): use a 64-bit Rust toolchain for win32 builds; r=Fallen
You need to log in before you can comment on or make changes to this bug.