Closed Bug 1409276 Opened 7 years ago Closed 6 years ago

Rust modules compiling C/C++ code are not necessarily using the same compiler as the rest of the build

Categories

(Firefox Build System :: General, enhancement)

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

Tracking

(firefox63 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla63
Tracking Flags:
Tracking Status
firefox63 --- fixed

People

(Reporter: glandium, Assigned: ted)

References

(Blocks 2 open bugs)

Details

Attachments

(4 files)

bug 1409276 - emit COMPUTED_{CFLAGS,CXXFLAGS} for Rust-only directories in the recursive make backend.
59 bytes, text/x-review-board-request
chmanchester
: review+
Details
bug 1409276 - disable warnings-as-errors for Rust libraries/programs.
59 bytes, text/x-review-board-request
chmanchester
: review+
Details
bug 1409276 - vendor a newer version of the cc crate.
59 bytes, text/x-review-board-request
mbrubeck
: review+
Details
bug 1409276 - pass C/C++ compilers and flags to cargo for use in build scripts.
59 bytes, text/x-review-board-request
chmanchester
: review+
Details 
When CC is not set, which is the case on our linux builds, rust sets it to "cc" before invoking subconfigures. "cc" is the system gcc, and never our gcc from toolchain artifacts. On the centos6 images, it's gcc 4.4 (FWIW, that was why building sccache with gcc was not straightforward when I originally tried (see commit message in bug 1408284)).

It currently kind of works by chance, but causes problems:
- The code is compiled with an old codegen that might generate slow code.
- At some random point, something that rust compiles might not build with older versions of gcc.
- Because we also build with PATH pointing to our gcc toolchain bin/ directory, the system gcc can end up trying to run our ld, and while working on bug 1399679, I got this funny failure while building backtrace-sys:
configure:2999: cc -O3 -ffunction-sections -fdata-sections -fPIC -m64   conftest.c  >&5
/builds/worker/workspace/build/src/gcc/bin/ld: this linker was not configured to use sysroots
Nathan, do you want to take a look at this? This is kind of a cousin of the BINDGEN_FLAGS problem.
Flags: needinfo?(nfroyd)
This sounds a bit scary since it can lead to unexpected build outcomes. Should we track this for uplift?
If it only affects build.rs scripts, which are used for build-time utilities like code generators, it's unlikely to affect the final output, assuming it runs at all. I don't think it needs uplift tracking.
It affects any C/C++ library that rust crates build on their own. It looks like *at the moment* no such libraries end up in libxul, but there's nothing preventing that from happening in the future.
FWIW, this bug made the landing of bug 1407487 burn hazard builds because it triggered the backtrace-sys crate to be built, which exposes this problem in some environments.
Blocks: 1417646
This apparently also causes bug 1417646, where the geckodriver binary can't build on cross-mac builds because a few C libraries that get built by some crates' build scripts aren't being built properly. If they're using the system compiler in that case then they're probably not even building for the right CPU architecture.
When I got Servo to cross-compile from linux->mac a while ago I took notes:
https://gist.github.com/luser/a33e5070d1c55a7d2c46fe763a9d1543

I had a frustrating time with build scripts that wanted to compile C libraries--many of them invoke whatever build system the library builds with natively, so there are crates like `cmake` etc to wrap that invocation, and then you have to convince all of those build systems to use the right tools. I wound up having to patch the cmake crate to support passing in a cmake "toolchain file" that specifies a few things.

The `cc` crate, which is used by most simple things does respect the `CC` environment variable, so I'm surprised that it wouldn't work properly here, unless we're not passing those variables down through cargo.
This doesn't block bug 1399679 anymore because there's a workaround from bug 1426283.
No longer blocks: 1399679
FWIW, it seems to go deeper than just not respecting HOST_CC. Apparently, host things are built according to the host system (or rustc binary platform, maybe), irrespective of --host given for configure, such that despite the firefox build system building e.g. elfhack or nsinstall as 32-bits on 32-bits builds, bindgen is built 64-bits.
I had to work around that precise issue in bug 1401647, where making our win32 builds (which build 32-bit host and target binaries) use a 64-bit rustc meant that building host binaries was broken. My solution there was to generate a wrapper script and tell cargo to use that as the linker for the x86_64-pc-windows-msvc target:
https://hg.mozilla.org/mozilla-central/annotate/273a99be7191/build/moz.configure/rust.configure#l294

...and have that script set the environment appropriately. I think for the case where we'd want to build a host rust library and link it together with host C/C++ code into a host binary this isn't actually a problem--we could just explicitly build the host rust code with `--target=whatever`. This issue only manifests when cargo is building *its* host binaries: build scripts and their dependencies, and presumably also procedural macro crates (which are built as shared libraries that get loaded by rustc).
Product: Core → Firefox Build System
Depends on: 1473121
My patch in bug 1473121 shows this to be true. From the log of one of the builds in the try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ae1c2b0185fb9db845583ab5441745460ee3e453&selectedJob=186255184

[task 2018-07-03T19:47:15.791Z] 19:47:15     INFO -       Running `/builds/worker/workspace/build/src/obj-firefox/testing/geckodriver/./release/build/bzip2-sys-8bf6e02f8bc93b0c/build-script-build`
<...>
[task 2018-07-03T19:47:15.794Z] 19:47:15     INFO -  running: "cc" "-O2" "-ffunction-sections" "-fdata-sections" "-fPIC" "-m64" "-I" "bzip2-1.0.6" "-DBZ_NO_STDIO" "-o" "/builds/worker/workspace/build/src/obj-firefox/testing/geckodriver/./x86_64-unknown-linux-gnu/release/build/bzip2-sys-32cb3989ac669bdd/out/bzip2-1.0.6/blocksort.o" "-c" "bzip2-1.0.6/blocksort.c"
Assignee: nobody → ted
I wrote what I thought was the straightforward patch for this: pass CC/CXX/CFLAGS/CXXFLAGS to the cargo invocation, but I hit some snags:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b9501a493963098627ab1953d093401764c25ccf

1) For cross-compiles, sometimes we need the HOST_ variables as well! On Linux there's a build script that gets built as part of a host crate (libloading) that compiles C sources:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b9501a493963098627ab1953d093401764c25ccf&selectedJob=186599019

[task 2018-07-05T12:20:22.613Z] 12:20:22     INFO -  cargo:rustc-link-lib=dl
[task 2018-07-05T12:20:22.613Z] 12:20:22     INFO -  TARGET = Some("x86_64-unknown-linux-gnu")
[task 2018-07-05T12:20:22.613Z] 12:20:22     INFO -  OPT_LEVEL = Some("2")
[task 2018-07-05T12:20:22.613Z] 12:20:22     INFO -  TARGET = Some("x86_64-unknown-linux-gnu")
[task 2018-07-05T12:20:22.613Z] 12:20:22     INFO -  HOST = Some("x86_64-unknown-linux-gnu")
[task 2018-07-05T12:20:22.613Z] 12:20:22     INFO -  TARGET = Some("x86_64-unknown-linux-gnu")
[task 2018-07-05T12:20:22.613Z] 12:20:22     INFO -  TARGET = Some("x86_64-unknown-linux-gnu")
[task 2018-07-05T12:20:22.613Z] 12:20:22     INFO -  HOST = Some("x86_64-unknown-linux-gnu")
[task 2018-07-05T12:20:22.614Z] 12:20:22     INFO -  CC_x86_64-unknown-linux-gnu = None
[task 2018-07-05T12:20:22.614Z] 12:20:22     INFO -  CC_x86_64_unknown_linux_gnu = None
[task 2018-07-05T12:20:22.614Z] 12:20:22     INFO -  HOST_CC = None
[task 2018-07-05T12:20:22.614Z] 12:20:22     INFO -  CC = Some(" /builds/worker/workspace/build/src/sccache2/sccache /builds/worker/workspace/build/src/gcc/bin/gcc -m32 -march=pentium-m -std=gnu99")
<...>
[task 2018-07-05T12:20:22.615Z] 12:20:22     INFO -  running: "/builds/worker/workspace/build/src/sccache2/sccache" "/builds/worker/workspace/build/src/gcc/bin/gcc" "-m32" "-march=pentium-m" "-std=gnu99" "-O2" "-ffunction-sections" "-fdata-sections" "-fPIC" "-m64" "-Wall" "-Wextra" "-o" "/builds/worker/workspace/build/src/obj-firefox/toolkit/library/release/build/libloading-feccbb59597a3d5c/out/src/os/unix/global_static.o" "-c" "src/os/unix/global_static.c"
[task 2018-07-05T12:20:22.615Z] 12:20:22     INFO -  cargo:warning=cc1: error: CPU you selected does not support x86-64 instruction set

2) On Windows, MSYS path translation makes a mess of the CC/CXX env vars:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b9501a493963098627ab1953d093401764c25ccf&selectedJob=186598966

12:48:20     INFO -  env   RUSTC_BOOTSTRAP=1 RUSTFLAGS='-C opt-level=2 -C debuginfo=2 ' RUSTC_WRAPPER='z:/build/build/src/sccache2/sccache.exe' CARGO_TARGET_DIR=. RUSTC=z:/build/build/src/rustc/bin/rustc.exe RUSTDOC=z:/build/build/src/rustc/bin/rustdoc.exe RUSTFMT=z:/build/build/src/rustc/bin/rustfmt.exe CC=" z:/build/build/src/sccache2/sccache.exe z:/build/build/src/vs2017_15.6.6/VC/bin/Hostx64/x86/cl.exe" CXX=" z:/build/build/src/sccache2/sccache.exe z:/build/build/src/vs2017_15.6.6/VC/bin/Hostx64/x86/cl.exe" CFLAGS="" CXXFLAGS="" MOZ_SRC=z:/build/build/src MOZ_DIST=z:/build/build/src/obj-firefox/dist LIBCLANG_PATH="z:/build/build/src/clang/bin" CLANG_PATH="z:/build/build/src/clang/bin/clang.exe" PKG_CONFIG_ALLOW_CROSS=1 RUST_BACKTRACE=full MOZ_TOPOBJDIR=z:/build/build/src/obj-firefox CARGO_INCREMENTAL=0  z:/build/build/src/rustc/bin/cargo.exe rustc  --release --frozen --manifest-path z:/build/build/src/testing/geckodriver/Cargo.toml -vv --bin geckodriver --target=i686-pc-windows-msvc
<...>
12:48:20     INFO -  TARGET = Some("i686-pc-windows-msvc")
12:48:20     INFO -  OPT_LEVEL = Some("2")
12:48:20     INFO -  TARGET = Some("i686-pc-windows-msvc")
12:48:20     INFO -  HOST = Some("x86_64-pc-windows-msvc")
12:48:20     INFO -  TARGET = Some("i686-pc-windows-msvc")
12:48:20     INFO -  TARGET = Some("i686-pc-windows-msvc")
12:48:20     INFO -  HOST = Some("x86_64-pc-windows-msvc")
12:48:20     INFO -  CC_i686-pc-windows-msvc = None
12:48:20     INFO -  CC_i686_pc_windows_msvc = None
12:48:20     INFO -  TARGET_CC = None
12:48:20     INFO -  CC = Some(" z;C:\\mozilla-build\\msys\\build\\build\\src\\sccache2\\sccache.exe z;C:\\mozilla-build\\msys\\build\\build\\src\\vs2017_15.6.6\\VC\\bin\\Hostx64\\x86\\cl.exe")

I did some local testing on Windows and I don't think it's possible in MSYS to pass an env var containing two Windows paths like that without it getting mangled. Not doing this on Windows should be OK for now because we have MSVC in PATH so the cc crate can find everything it needs.

3) The mac builds failed after linking geckodriver because rustc tries to run `dsymutil` on the resulting binary and we don't have that in PATH:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b9501a493963098627ab1953d093401764c25ccf&selectedJob=186599010

[task 2018-07-05T12:22:43.192Z] 12:22:43     INFO -       Running `/builds/worker/workspace/build/src/sccache2/sccache /builds/worker/workspace/build/src/rustc/bin/rustc --crate-name geckodriver testing/geckodriver/src/main.rs --crate-type bin --emit=dep-info,link -C opt-level=2 -C panic=abort -C codegen-units=1 -C metadata=03b63c00fce7be1a -C extra-filename=-03b63c00fce7be1a --out-dir /builds/worker/workspace/build/src/obj-firefox/testing/geckodriver/./x86_64-apple-darwin/release/deps --target x86_64-apple-darwin -C linker=/builds/worker/workspace/build/src/build/cargo-linker -L dependency=/builds/worker/workspace/build/src/obj-firefox/testing/geckodriver/./x86_64-apple-darwin/release/deps -L dependency=/builds/worker/workspace/build/src/obj-firefox/testing/geckodriver/./release/deps --extern chrono=/builds/worker/workspace/build/src/obj-firefox/testing/geckodriver/./x86_64-apple-darwin/release/deps/libchrono-251d3070576e9b84.rlib --extern clap=/builds/worker/workspace/build/src/obj-firefox/testing/geckodriver/./x86_64-apple-darwin/release/deps/libclap-6f1c649452faf914.rlib --extern hyper=/builds/worker/workspace/build/src/obj-firefox/testing/geckodriver/./x86_64-apple-darwin/release/deps/libhyper-3e40db16a885a2a6.rlib --extern lazy_static=/builds/worker/workspace/build/src/obj-firefox/testing/geckodriver/./x86_64-apple-darwin/release/deps/liblazy_static-2cf5007dac72c8c5.rlib --extern log=/builds/worker/workspace/build/src/obj-firefox/testing/geckodriver/./x86_64-apple-darwin/release/deps/liblog-7d3506b30a865f44.rlib --extern mozprofile=/builds/worker/workspace/build/src/obj-firefox/testing/geckodriver/./x86_64-apple-darwin/release/deps/libmozprofile-6f67c603de673c19.rlib --extern mozrunner=/builds/worker/workspace/build/src/obj-firefox/testing/geckodriver/./x86_64-apple-darwin/release/deps/libmozrunner-3fabcf5cf05f69a0.rlib --extern mozversion=/builds/worker/workspace/build/src/obj-firefox/testing/geckodriver/./x86_64-apple-darwin/release/deps/libmozversion-1863333a070de140.rlib --extern regex=/builds/worker/workspace/build/src/obj-firefox/testing/geckodriver/./x86_64-apple-darwin/release/deps/libregex-d94c0ed8d3c9f18c.rlib --extern rustc_serialize=/builds/worker/workspace/build/src/obj-firefox/testing/geckodriver/./x86_64-apple-darwin/release/deps/librustc_serialize-ea2a6fb68cd10c2c.rlib --extern uuid=/builds/worker/workspace/build/src/obj-firefox/testing/geckodriver/./x86_64-apple-darwin/release/deps/libuuid-fe7cfe70deb98180.rlib --extern webdriver=/builds/worker/workspace/build/src/obj-firefox/testing/geckodriver/./x86_64-apple-darwin/release/deps/libwebdriver-2678ff84d6ce5ba2.rlib --extern zip=/builds/worker/workspace/build/src/obj-firefox/testing/geckodriver/./x86_64-apple-darwin/release/deps/libzip-91d63dc309eec00c.rlib -C opt-level=2 -C debuginfo=2 -L native=/builds/worker/workspace/build/src/obj-firefox/testing/geckodriver/./x86_64-apple-darwin/release/build/bzip2-sys-32cb3989ac669bdd/out`
[task 2018-07-05T12:22:43.192Z] 12:22:43     INFO -  error: failed to run dsymutil: No such file or directory (os error 2)

Thankfully acrichto added a `-Z run-dsymutil=no` option to rustc recently, so I'm going to use that in a patch on bug 1417646.
Flags: needinfo?(nfroyd)
Having done part of HOST_* support for bug 1469091, and seeing your 1, I'm starting to wonder if it wouldn't be easier to have HOST_ variables completely removed from the builds, and host stuff treated as a sub-build with a possibly different target.
Oh, and the Windows msys problem may be because of the use of double quotes. Try single quotes.
(In reply to Mike Hommey [:glandium] from comment #14)
> Oh, and the Windows msys problem may be because of the use of double quotes.
> Try single quotes.

I did a bunch of local testing with an env.exe binary I made that just prints out an environment variable, and single quotes don't fix it unfortunately:

ted@DESKTOP-32V9ND8 /c/build
$ FOO=z:/build/foo.exe ./env.exe
FOO=z:/build/foo.exe

ted@DESKTOP-32V9ND8 /c/build
$ FOO="z:/build/foo.exe" ./env.exe
FOO=z:/build/foo.exe

ted@DESKTOP-32V9ND8 /c/build
$ FOO=" z:/build/foo.exe" ./env.exe
FOO= z;C:\mozilla-build\msys\build\foo.exe

ted@DESKTOP-32V9ND8 /c/build
$ FOO="z:/build/foo.exe z:/build/bar.exe" ./env.exe
FOO=z;C:\mozilla-build\msys\build\foo.exe z;C:\mozilla-build\msys\build\bar.exe

ted@DESKTOP-32V9ND8 /c/build
$ FOO='z:/build/foo.exe z:/build/bar.exe' ./env.exe
FOO=z;C:\mozilla-build\msys\build\foo.exe z;C:\mozilla-build\msys\build\bar.exe
(In reply to Mike Hommey [:glandium] from comment #13)
> Having done part of HOST_* support for bug 1469091, and seeing your 1, I'm
> starting to wonder if it wouldn't be easier to have HOST_ variables
> completely removed from the builds, and host stuff treated as a sub-build
> with a possibly different target.

It's not unreasonable. The `cc` crate does support env vars of the form `CC_$target`, so we could set like `CC_x86_64_unknown_linux_gnu`. Setting `HOST_CC` doesn't work for our 32-bit Linux builds because they're running on a 64-bit OS, but they build with `--host=i686-pc-linux` so we pass `HOST_CC='gcc -m32'` to cargo which builds its host binaries as 64-bit and then things break. (There's no way to change the host type for cargo builds, I checked.)

https://treeherder.mozilla.org/logviewer.html#?job_id=186644513&repo=try&lineNumber=16390
(In reply to Ted Mielczarek [:ted] [:ted.mielczarek] from comment #15)
> (In reply to Mike Hommey [:glandium] from comment #14)
> > Oh, and the Windows msys problem may be because of the use of double quotes.
> > Try single quotes.
> 
> I did a bunch of local testing with an env.exe binary I made that just
> prints out an environment variable, and single quotes don't fix it
> unfortunately:

make is special. It's a win32 application, tat may or may not call commands through the msys shell. Whether it does or not depends on the contents of the command line. Double quotes are guaranteed to go through an msys shell, which definitely fucks up the environment. With double quotes out of the way, there are more chances for make to actually call the command directly and avoid the msys mangling.
Things are still busted because our `HOST_CFLAGS` are `-MD -MP -MF .deps/.pp` and so compiles in build scripts fail because the .deps dir doesn't exist:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6fcc6a411c22923138e0a22a0d97df0187987bd1
I tried these same tests in a Makefile on my Windows machine with mozmake and single quotes don't seem to make a difference. The only thing I could get to work was using `export FOO=...` in a standalone variable statement, but I'm not sure that will work given the way we invoke cargo.
I changed things to set `{CC,CXX}_$(RUST_TARGET)` variables instead, which fixed a few things, but a few other things are still broken:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b3d5ab57eebed8a883348145ec21c5674988f6d2

The macOS ASAN build is failing to link the geckodriver binary:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b3d5ab57eebed8a883348145ec21c5674988f6d2&selectedJob=187345775

I think this is because we don't wrap the cargo linker for ASAN/TSAN builds, so it's trying to use the system linker and failing:
https://dxr.mozilla.org/mozilla-central/rev/085cdfb90903d4985f0de1dc7786522d9fb45596/config/rules.mk#927

We should probably just not build geckodriver in ASAN builds.

Android builds are still broken as well:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b3d5ab57eebed8a883348145ec21c5674988f6d2&selectedJob=187345711


[task 2018-07-10T11:00:42.173Z] 11:00:42     INFO -  running: "/builds/worker/workspace/build/src/sccache2/sccache" "/builds/worker/workspace/build/src/android-ndk/toolchains/llvm/prebuilt/linux-x86_64/bin/clang" "-std=gnu99" "--target=arm-linux-androideabi" "-O2" "-ffunction-sections" "-fdata-sections" "-fPIC" "--target=armv7-linux-androideabi" "-I" "bzip2-1.0.6" "-DBZ_NO_STDIO" "-o" "/builds/worker/workspace/build/src/obj-firefox/testing/geckodriver/./armv7-linux-androideabi/release/build/bzip2-sys-32cb3989ac669bdd/out/bzip2-1.0.6/blocksort.o" "-c" "bzip2-1.0.6/blocksort.c"
[task 2018-07-10T11:00:42.173Z] 11:00:42     INFO -  thread 'main' panicked at '
[task 2018-07-10T11:00:42.173Z] 11:00:42     INFO -  Internal error occurred: Command "/builds/worker/workspace/build/src/sccache2/sccache" "/builds/worker/workspace/build/src/android-ndk/toolchains/llvm/prebuilt/linux-x86_64/bin/clang" "-std=gnu99" "--target=arm-linux-androideabi" "-O2" "-ffunction-sections" "-fdata-sections" "-fPIC" "--target=armv7-linux-androideabi" "-I" "bzip2-1.0.6" "-DBZ_NO_STDIO" "-o" "/builds/worker/workspace/build/src/obj-firefox/testing/geckodriver/./armv7-linux-androideabi/release/build/bzip2-sys-32cb3989ac669bdd/out/bzip2-1.0.6/blocksort.o" "-c" "bzip2-1.0.6/blocksort.c" with args "clang" did not execute successfully (status code exit code: 1).
[task 2018-07-10T11:00:42.173Z] 11:00:42     INFO -  ', third_party/rust/cc/src/lib.rs:2070:5
[task 2018-07-10T11:00:42.173Z] 11:00:42     INFO -  stack backtrace:
[task 2018-07-10T11:00:42.174Z] 11:00:42     INFO -  cargo:warning=In file included from bzip2-1.0.6/blocksort.c:22:
[task 2018-07-10T11:00:42.174Z] 11:00:42     INFO -  cargo:warning=In file included from bzip2-1.0.6/bzlib_private.h:25:
[task 2018-07-10T11:00:42.174Z] 11:00:42     INFO -  cargo:warning=In file included from /usr/include/stdlib.h:24:
[task 2018-07-10T11:00:42.174Z] 11:00:42     INFO -  cargo:warning=/usr/include/features.h:364:12: fatal error: 'sys/cdefs.h' file not found
[task 2018-07-10T11:00:42.174Z] 11:00:42     INFO -  cargo:warning=#  include <sys/cdefs.h>
[task 2018-07-10T11:00:42.174Z] 11:00:42     INFO -  cargo:warning=           ^~~~~~~~~~~~~
[task 2018-07-10T11:00:42.174Z] 11:00:42     INFO -  cargo:warning=1 error generated.

It looks like we're passing the compiler correctly, but it doesn't have all the requisite include paths being passed. I noticed that I'm stupidly trying to pass `$(CFLAGS)` and `$(CXXFLAGS)` but those variables are empty:
[task 2018-07-10T11:00:42.122Z] 11:00:42     INFO -  env   RUSTFLAGS='-C opt-level=2 -C debuginfo=2 ' RUSTC_WRAPPER='/builds/worker/workspace/build/src/sccache2/sccache' CARGO_TARGET_DIR=. RUSTC=/builds/worker/workspace/build/src/rustc/bin/rustc RUSTDOC=/builds/worker/workspace/build/src/rustc/bin/rustdoc RUSTFMT=/builds/worker/workspace/build/src/rustc/bin/rustfmt CC_armv7_linux_androideabi=" /builds/worker/workspace/build/src/sccache2/sccache /builds/worker/workspace/build/src/android-ndk/toolchains/llvm/prebuilt/linux-x86_64/bin/clang -std=gnu99 --target=arm-linux-androideabi" CXX_armv7_linux_androideabi=" /builds/worker/workspace/build/src/sccache2/sccache /builds/worker/workspace/build/src/android-ndk/toolchains/llvm/prebuilt/linux-x86_64/bin/clang++ -std=gnu++14 --target=arm-linux-androideabi" CFLAGS_armv7_linux_androideabi="" CXXFLAGS_armv7_linux_androideabi=""  MOZ_SRC=/builds/worker/workspace/build/src MOZ_DIST=/builds/worker/workspace/build/src/obj-firefox/dist LIBCLANG_PATH="/builds/worker/workspace/build/src/clang/lib" CLANG_PATH="/builds/worker/workspace/build/src/clang/bin/clang" PKG_CONFIG_ALLOW_CROSS=1 RUST_BACKTRACE=full MOZ_TOPOBJDIR=/builds/worker/workspace/build/src/obj-firefox CARGO_INCREMENTAL=0 MOZ_CARGO_WRAP_LDFLAGS="-L/builds/worker/workspace/build/src/android-ndk/platforms/android-9/arch-arm/usr/lib -Wl,-rpath-link=/builds/worker/workspace/build/src/android-ndk/platforms/android-9/arch-arm/usr/lib --sysroot=/builds/worker/workspace/build/src/android-ndk/platforms/android-9/arch-arm -Wl,--allow-shlib-undefined -gcc-toolchain /builds/worker/workspace/build/src/android-ndk/toolchains/arm-linux-androideabi-4.9/prebuilt/linux-x86_64 -mthumb -Wl,-z,noexecstack -Wl,-z,text -Wl,-z,relro -Wl,--build-id -Wl,--icf=safe -Wl,--hash-style=sysv -Wl,-rpath-link,/builds/worker/workspace/build/src/obj-firefox/dist/bin -Wl,-rpath-link,/usr/local/lib" MOZ_CARGO_WRAP_LD=" /builds/worker/workspace/build/src/sccache2/sccache /builds/worker/workspace/build/src/android-ndk/toolchains/llvm/prebuilt/linux-x86_64/bin/clang -std=gnu99 --target=arm-linux-androideabi" CARGO_TARGET_ARMV7_LINUX_ANDROIDEABI_LINKER=/builds/worker/workspace/build/src/build/cargo-linker /builds/worker/workspace/build/src/rustc/bin/cargo rustc  --release --frozen --manifest-path /builds/worker/workspace/build/src/testing/geckodriver/Cargo.toml -vv --bin geckodriver --target=armv7-linux-androideabi

Additionally the hazard build failed trying to compile something from a Rust build script with its weird compiler:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b3d5ab57eebed8a883348145ec21c5674988f6d2&selectedJob=187345727

[task 2018-07-10T11:00:51.414Z]  3:57.67 running: "/builds/worker/workspace/sixgill/usr/libexec/sixgill/scripts/wrap_gcc/basecc" "-O1" "-ffunction-sections" "-fdata-sections" "-fPIC" "-g" "-m64" "-Wall" "-Wextra" "-o" "/builds/worker/checkouts/gecko/obj-analyzed/toolkit/library/debug/build/libloading-d397eae4f2ab8722/out/src/os/unix/global_static.o" "-c" "src/os/unix/global_static.c"
[task 2018-07-10T11:00:51.416Z]  3:57.67    Compiling nserror v0.1.0 (file:///builds/worker/checkouts/gecko/xpcom/rust/nserror)
[task 2018-07-10T11:00:51.416Z]  3:57.67      Running `/builds/worker/workspace/rustc/bin/rustc --crate-name env_logger third_party/rust/env_logger/src/lib.rs --crate-type lib --emit=dep-info,link -C opt-level=1 -C panic=abort -C debuginfo=2 -C debug-assertions=on -C metadata=177d8d27c3a63cb7 -C extra-filename=-177d8d27c3a63cb7 --out-dir /builds/worker/checkouts/gecko/obj-analyzed/toolkit/library/x86_64-unknown-linux-gnu/debug/deps --target x86_64-unknown-linux-gnu -C linker=/builds/worker/checkouts/gecko/build/cargo-linker -L dependency=/builds/worker/checkouts/gecko/obj-analyzed/toolkit/library/x86_64-unknown-linux-gnu/debug/deps -L dependency=/builds/worker/checkouts/gecko/obj-analyzed/toolkit/library/debug/deps --extern atty=/builds/worker/checkouts/gecko/obj-analyzed/toolkit/library/x86_64-unknown-linux-gnu/debug/deps/libatty-d11ac0bd382c9160.rlib --extern humantime=/builds/worker/checkouts/gecko/obj-analyzed/toolkit/library/x86_64-unknown-linux-gnu/debug/deps/libhumantime-07642495340b9ef2.rlib --extern log=/builds/worker/checkouts/gecko/obj-analyzed/toolkit/library/x86_64-unknown-linux-gnu/debug/deps/liblog-3dc93f6f5ac826cb.rlib --extern termcolor=/builds/worker/checkouts/gecko/obj-analyzed/toolkit/library/x86_64-unknown-linux-gnu/debug/deps/libtermcolor-775390a4306da635.rlib --cap-lints warn -C opt-level=2 -C debuginfo=2`
[task 2018-07-10T11:00:51.416Z]  3:57.67      Running `/builds/worker/workspace/rustc/bin/rustc --crate-name nserror xpcom/rust/nserror/src/lib.rs --crate-type lib --emit=dep-info,link -C opt-level=1 -C panic=abort -C debuginfo=2 -C debug-assertions=on -C metadata=88defee33fb1ef26 -C extra-filename=-88defee33fb1ef26 --out-dir /builds/worker/checkouts/gecko/obj-analyzed/toolkit/library/x86_64-unknown-linux-gnu/debug/deps --target x86_64-unknown-linux-gnu -C linker=/builds/worker/checkouts/gecko/build/cargo-linker -L dependency=/builds/worker/checkouts/gecko/obj-analyzed/toolkit/library/x86_64-unknown-linux-gnu/debug/deps -L dependency=/builds/worker/checkouts/gecko/obj-analyzed/toolkit/library/debug/deps --extern nsstring=/builds/worker/checkouts/gecko/obj-analyzed/toolkit/library/x86_64-unknown-linux-gnu/debug/deps/libnsstring-35d9080c673384d7.rlib -C opt-level=2 -C debuginfo=2`
[task 2018-07-10T11:00:51.422Z]  3:57.68 cargo:warning=Cannot find real gcc binaries at /builds/worker/workspace/sixgill/usr/libexec/sixgill/scripts/wrap_gcc/basecc line 49.

I don't know if there's a simple fix there or if we should just not try to fix this for hazard builds.
Aside from those issues things look pretty good, though. All the other macOS builds are green, which means bug 
1417646 is fixed, and all the Linux and Windows builds are green as well.
re: hazard builds. There are two variants, one that builds the full browser and one that only builds the JS shell. The browser is built via ./mach build; the shell via js/src/configure; make.

https://searchfox.org/mozilla-central/source/taskcluster/scripts/builder/hazard-browser.sh
https://searchfox.org/mozilla-central/source/taskcluster/scripts/builder/hazard-shell.sh

They find their compiler a little differently. Let me ignore the shell one for now, since it isn't built by default anyway. The browser one uses --with-compiler-wrapper:

ac_add_options --with-compiler-wrapper=$TOOLTOOL_DIR/sixgill/usr/libexec/sixgill/scripts/wrap_gcc/basecc

from

https://searchfox.org/mozilla-central/source/browser/config/mozconfigs/linux64/hazards#34

basecc expects the absolute path of the "real" compiler as its first argument. From the above log, it seems that is not happening (its first argument is -O1 instead). One of its usage scenarios involves just the name "gcc" or "g++" passed in instead, which it searches for in an edited $PATH, and it tries that here (looking for things like /usr/bin/-O1 in this case) and fails.

From my reading of https://searchfox.org/mozilla-central/source/build/moz.configure/toolchain.configure#866 , it seems like --with-compiler-wrapper does similarly crazy things, but *should* try to invoke the wrapper with the full path of the compiler as its first argument? Why is that not happening here?

For what it's worth, the hazard build would be just fine *not* being invoked for this rust-related stuff.

Ugh. And I just looked at an actual hazard job, and it's getting something like:

checking for gcc... /builds/worker/workspace/sixgill/usr/libexec/sixgill/scripts/wrap_gcc/basecc /builds/worker/workspace/sixgill/usr/libexec/sixgill/scripts/wrap_gcc/gcc -std=gnu99

So what's going to end up happening there is that basecc sees that it is rather uselessly given the full path to its own wrapper script, so it discards the directory but remembers that it's 'gcc' we want to eventually invoke. It searches for that in the remainder of $PATH and apparently finds it successfully. It would probably be better if it were passed the actual compiler path; I probably ought to figure out what's going on there.

The above line is identical in your job.

Ooh, depending on what you're doing here, this might be relevant: the hazard build has an extra layer so that it can collect output from the full compile in one place. Because some configurations point to the hazard compiler wrapper via $CC/$CXX, it blows them away:

https://searchfox.org/mozilla-central/source/js/src/devtools/rootAnalysis/run_complete#255

That currently breaks developer hazard builds because of the switch to using clang by default. Locally, I have it unconditionally setting CC=gcc CXX=g++ instead (the hazard analysis is a gcc plugin, so it'll always want gcc not clang.) Perhaps that is eating the stuff you're doing?

Your build has

CC_x86_64_unknown_linux_gnu = Some(" /builds/worker/workspace/sixgill/usr/libexec/sixgill/scripts/wrap_gcc/basecc /builds/worker/workspace/sixgill/usr/libexec/sixgill/scripts/wrap_gcc/gcc -std=gnu99")

Is something perhaps taking only the first component of that?

The hazard stuff clearly has too much magic, and at least when it's going through --with-compiler-wrapper, it shouldn't be guessing much of anything or mucking with $CC/$CXX. But I'm not sure if that mechanism is quite working out here; it may need to be fixed first. Or maybe this is just a matter of never using CC to trick things to go through the hazard compiler?
Skimming the rest of this bug, I'm feeling like this is a problem with --with-compiler-wrapper setting things up with a couple of different arguments (essentially "basecc gcc -std=gnu99") embedded in a single notion of what the compiler is, and then the build system only looking at the first argument (because Windows). So while removing magic from the hazard stuff might make things generally better, it's probably not going to help here; here, I'm guessing that --with-compiler-wrapper just doesn't work when compiling C/C++ underneath Rust.

I'm curious what would happen if you attempted --with-compiler-wrapper=sccache or something.

Anyway, if --with-compiler-wrapper needs to die, the hazard builds could probably switch to the alternate mechanism by setting CC=.../wrap_gcc/gcc. (What happens there is rather convoluted, but is at least intended to work it all out properly and wouldn't be too hard to fix if not.) The actual compiler would need to come from PATH then, though, which is kind of ugly.
I've shaved enough yaks here that I'm not inclined to go much further. I might just avoid doing this in hazard builds for now and we can sort it out in a followup. This will mean that C/C++ sources built via Rust build scripts in hazard builds will continue to use whatever compiler they currently are, but that's the status quo.

This seems to be an actual bug in the `cc` crate. I wrote a reduced testcase and filed it:
https://github.com/alexcrichton/cc-rs/issues/328
Alex landed a fix for that upstream, so I'll try vendoring that revision and see how well things work:
https://github.com/alexcrichton/cc-rs/commit/9eaa56536ec420c7248cb52e10d3538568700d62
Let's try again:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6deb400600766e4c85baf696b42f3abd093dbdc9

* The hazard builds should hopefully be fixed by picking up the fix to `cc`.
* I added a change to emit COMPUTED_{CFLAGS,CXXFLAGS} for Rust-only directories in the recursive make backend, and pass those to cargo as {CFLAGS,CXXFLAGS}, which I hope will fix Android builds.
* I ifdef'ed all these vars off for ASAN/TSAN builds since they don't pass our custom linker already anyway. We should not build geckodriver for asan builds anyway, but MOZ_ASAN is still defined in old-configure. :-/
I screwed up patching/vendoring the `cc` crate in that push, but Alex released a new version so I'm going to try again. Looking at our Cargo.lock, I note that the older `gcc` crate is still being pulled in by way of two crates: libz-sys and cubeb-sys. They've both been updated to use `cc` in newer versions, so we should be able to just update them to make sure everything is using `cc` and behaving properly.
I'm getting hung up on an issue with cargo-vendor, I need to get that resolved before I can actually fix this because I need to pick up a new version of `cc`.
I fixed my cargo-vendor issue and uncovered a few more issues!

* Using COMPUTED_{CFLAGS,CXXFLAGS} meant that we were passing our warnings-as-errors flags into these compiles, which of course made them fail. I disabled that for Rust directories.
* The macOS ASAN build failed to link geckodriver because we don't pass a linker wrapper to cargo in that situation, so I explicitly disabled building geckodriver for ASAN/TSAN builds.
* I missed updating some build system Python tests, I fixed those.
* Coverage builds are failing because now we're passing `--coverage` in `CFLAGS` and cargo is failing to link stylo's build script. The stylo build script links the clang crate, which links libloading, which compiles a C source file for the host to link in, which gets built with `--coverage` now:
https://treeherder.mozilla.org/logviewer.html#?job_id=188378388&repo=try&lineNumber=16609
* Android builds were failing because they couldn't find a suitable `ar` binary. I made it so we pass down `$(AR)` appropriately.
Looking much better now:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3bccdf25dde25d20f63874bee5f2a5b24229ec66

Still broken:
* ccov builds per comment 30. Need to figure out a solution here.
* android-x86: see bug 1441656 comment 7
I think I understand what's actually broken in the ccov builds--they're building that C source file with `--coverage`, but then rustc passes `-nodefaultlibs` on the link line, and libgcov does not expect that. The link error is that it's missing `atexit`, which comes from libgcc.

I have two possible solutions:
1) Remove `--coverage` from our CFLAGS/LDFLAGS for Rust compilation. This probably won't hurt anything, but it might be a little ugly to execute on currently, since the ccov builds stick those flags directly in CFLAGS/LDFLAGS, so we can't easily omit them.
2) Put something in toolkit/library/rust/moz.build like:
if config['MOZ_COVERAGE'] and config['CC_TYPE'] == 'gcc':
  LDFLAGS += ['-lgcc']
I banged my head against the ccov bustage for a bit until I sat down and reproduced it with a simple testcase locally:
gcc-6 --coverage -o simple.o -c simple.c
gcc-6 --coverage -o simple simple.o -nodefaultlibs

(Where simple.c is just `int main(int argc, char** argv) { return 0; }`.)

Looking at the output of `gcc -v`, specifically the actual link lines it was generating for collect2 showed that the problem is link line ordering (I should have guessed!). Normally gcc passes `-lgcov -lgcc` after anything given on the commandline, but with `-nodefaultlibs` it just passes `-gcov` there and leaves out the rest, and even if you explicitly pass `-lgcc` it doesn't work because `-lgcov` winds up *after* that on the link line and symbol resolution goes the other way. Explicitly passing `-lgcov -lgcc` on the link line fixes things for my simple testcase:
gcc-6 --coverage -o simple simple.o -nodefaultlibs -lgcov -lgcc_s -lc

I'm going to see if this works on try and if so I'm going to disable the geckodriver build on android-x86 and that should fix all remaining builds.
Blocks: 1477305
I give up on making ccov builds work. I filed a followup so someone else can deal with them.

I also filed a few cargo issues for features that would make this easier:
https://github.com/rust-lang/cargo/issues/5754
https://github.com/rust-lang/cargo/issues/5755
Looking greener after adding some more workarounds:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f05df6e87ab6b97096502b07b177b9d32a056c7f

The macosx-ccov build is still broken, I'll just --disable-geckodriver there for now.
Attachment #8994978 - Flags: review?(mbrubeck)
Comment on attachment 8994978 [details]
bug 1409276 - vendor a newer version of the cc crate.

https://reviewboard.mozilla.org/r/259468/#review266544
Attachment #8994978 - Flags: review?(mbrubeck) → review+
Attachment #8994976 - Flags: review?(core-build-config-reviews) → review?(cmanchester)
Attachment #8994977 - Flags: review?(core-build-config-reviews) → review?(cmanchester)
Attachment #8994979 - Flags: review?(core-build-config-reviews) → review?(cmanchester)
Comment on attachment 8994976 [details]
bug 1409276 - emit COMPUTED_{CFLAGS,CXXFLAGS} for Rust-only directories in the recursive make backend.

https://reviewboard.mozilla.org/r/259464/#review267112

Two nits, otherwise good to go.

::: build/templates.mozbuild:56
(Diff revision 1)
> +@template
> +def AllowCompilerWarnings():
> +    COMPILE_FLAGS['WARNINGS_AS_ERRORS'] = []

This change looks like it belongs in the next commit.

::: python/mozbuild/mozbuild/frontend/emitter.py
(Diff revision 1)
> -        elif context.objdir in self._rust_compile_dirs:
> -            yield computed_link_flags

Now that this is gone we can get rid of `self._rust_compile_dirs` altogether.
Attachment #8994976 - Flags: review?(cmanchester) → review+
Comment on attachment 8994977 [details]
bug 1409276 - disable warnings-as-errors for Rust libraries/programs.

https://reviewboard.mozilla.org/r/259466/#review267114
Attachment #8994977 - Flags: review?(cmanchester) → review+
Comment on attachment 8994979 [details]
bug 1409276 - pass C/C++ compilers and flags to cargo for use in build scripts.

https://reviewboard.mozilla.org/r/259470/#review267116

I guess we'll need to fix bug 1468546 for any of this to work with the tup build.
Attachment #8994979 - Flags: review?(cmanchester) → review+
Comment on attachment 8994976 [details]
bug 1409276 - emit COMPUTED_{CFLAGS,CXXFLAGS} for Rust-only directories in the recursive make backend.

https://reviewboard.mozilla.org/r/259464/#review267250

::: build/templates.mozbuild:56
(Diff revision 1)
> +@template
> +def AllowCompilerWarnings():
> +    COMPILE_FLAGS['WARNINGS_AS_ERRORS'] = []

Oops, yeah. Thanks!

::: python/mozbuild/mozbuild/frontend/emitter.py
(Diff revision 1)
> -        elif context.objdir in self._rust_compile_dirs:
> -            yield computed_link_flags

I had thought about that but wasn't sure if we wanted to keep it around for anything else. Might as well remove it and we can put it back if we need it again.
https://hg.mozilla.org/integration/mozilla-inbound/rev/198bccea5398cb722aa0bd919a483f7eddd51227
bug 1409276 - emit COMPUTED_{CFLAGS,CXXFLAGS} for Rust-only directories in the recursive make backend. r=chmanchester

https://hg.mozilla.org/integration/mozilla-inbound/rev/5f20a8a55268a579911b0494243a34311e836ec6
bug 1409276 - disable warnings-as-errors for Rust libraries/programs. r=chmanchester

https://hg.mozilla.org/integration/mozilla-inbound/rev/ee4bd7464329a1b284d484554e22fdc1ee6fb954
bug 1409276 - vendor a newer version of the cc crate. r=mbrubreck

https://hg.mozilla.org/integration/mozilla-inbound/rev/43eacc8458b87a83278cb6aed42d88c9409a73c7
bug 1409276 - pass C/C++ compilers and flags to cargo for use in build scripts. r=chmanchester
(In reply to Chris Manchester (:chmanchester) from comment #45)
> I guess we'll need to fix bug 1468546 for any of this to work with the tup
> build.

We could just fix this separately in the Tup backend for now. The actual fixes weren't that hard (working around all the weirdness of our various build configurations was most of the work). Replicating this fix would just involve setting a few extra environment variables when running build script executables.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: