1376411 - third_party/rust/libc is too old, out of sync with rustc

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[Jan Beich]

Despite bug 1374807 bumping Rust version the bundled libc crate hasn't been updated since bug 1332465. This severly limits support for new platforms, especially Tier3 ones. For one, FreeBSD aarch64 support appeared in Rust 1.17.0.

gmake[6]: Entering directory '/wrkdirs/usr/ports/www/firefox/work/firefox-54.0/obj-aarch64-unknown-freebsd11.0/toolkit/library/rust'
force-cargo-library-build
env  CARGO_TARGET_DIR=/wrkdirs/usr/ports/www/firefox/work/firefox-54.0/obj-aarch64-unknown-freebsd11.0/toolkit/library RUSTC=/usr/local/bin/rustc MOZ_DIST=/wrkdirs/usr/ports/www/firefox/work/firefox-54.0/obj-aarch64-unknown-freebsd11.0/dist LIBCLANG_PATH= CLANG_PATH= PKG_CONFIG_ALLOW_CROSS=1 /usr/local/bin/cargo build  --release --frozen --manifest-path /wrkdirs/usr/ports/www/firefox/work/firefox-54.0/toolkit/library/rust/Cargo.toml --lib --target=aarch64-unknown-freebsd 
   Compiling libc v0.2.20
   Compiling byteorder v1.0.0
   Compiling matches v0.1.4
   Compiling nsstring v0.1.0 (file:///wrkdirs/usr/ports/www/firefox/work/firefox-54.0/xpcom/rust/nsstring)
   Compiling bitreader v0.2.0
   Compiling unicode-normalization v0.1.4
   Compiling unicode-bidi v0.2.5
error[E0412]: cannot find type `c_long` in this scope
   --> /wrkdirs/usr/ports/www/firefox/work/firefox-54.0/third_party/rust/libc/src/lib.rs:192:45
    |
192 |     pub fn fseek(stream: *mut FILE, offset: c_long, whence: c_int) -> c_int;
    |                                             ^^^^^^ not found in this scope
    |
    = help: possible candidate is found in another module, you can import it into scope:
              `use core::os::raw::c_long;`
[...]

http://thunderx1.nyi.freebsd.org/data/110arm64-default/444205/logs/errors/firefox-54.0,1.log
https://github.com/rust-lang/libc/commit/1363fe4a03b7

Comment 1

[Jan Beich]

Attachment: Bug 1376411 - Update libc to 0.2.24 to get support for more Tier3 platforms.

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

Comment 2

[Jan Beich]

Comment on attachment 8882152 [details]
Bug 1376411 - Update libc to 0.2.24 to get support for more Tier3 platforms.

Generated by

$ fgrep -m1 libc **/Cargo.lock
js/src/Cargo.lock: "libc 0.2.20 (registry+https://github.com/rust-lang/crates.io-index)",
servo/Cargo.lock: "libc 0.2.23 (registry+https://github.com/rust-lang/crates.io-index)",
testing/geckodriver/Cargo.lock: "libc 0.2.20 (registry+https://github.com/rust-lang/crates.io-index)",
toolkit/library/gtest/rust/Cargo.lock: "libc 0.2.20 (registry+https://github.com/rust-lang/crates.io-index)",
toolkit/library/rust/Cargo.lock: "libc 0.2.20 (registry+https://github.com/rust-lang/crates.io-index)",

$ for f (**/Cargo.lock~servo/*) (cd $f:h; cargo update -p libc)
$ ./mach vendor rust

Comment 3

[Jan Beich]

Attachment: Rebased for Firefox 55

$ hg up beta
$ fetch -qo- https://reviewboard-hg.mozilla.org/gecko/raw-rev/a3e3810cb479 | sed 1d | patch -CEfsp1
6 out of 18 hunks failed while patching toolkit/library/gtest/rust/Cargo.lock
6 out of 18 hunks failed while patching toolkit/library/rust/Cargo.lock

Comment 4

[Ralph Giles (:rillian)]

Comment on attachment 8882152 [details]
Bug 1376411 - Update libc to 0.2.24 to get support for more Tier3 platforms.

https://reviewboard.mozilla.org/r/153272/#review158524

Looks correct, and should be compatible with our current builds. Thanks for the update!

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e8d9d1520395
Update libc to 0.2.24 to get support for more Tier3 platforms. r=rillian

Comment 6

[Carsten Book [:Tomcat]]

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

Comment 7

[Jan Beich]

Comment on attachment 8882157 [details] [diff] [review]
Rebased for Firefox 55

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1284816
[User impact if declined]: Broken build at least on FreeBSD aarch64
[Is this code covered by automated tests?]: Yes but I'm not sure exactly which ones.
[Has the fix been verified in Nightly?]: No, I don't have hardware. Only FF54 backport was verified downstream:
http://thunderx1.nyi.freebsd.org/data/110arm64-default/444857/logs/firefox-54.0.1_1,1.log
https://svnweb.freebsd.org/ports/head/www/firefox/files/patch-bug1376411?revision=444651&view=markup
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: Unlikely
[Why is the change risky/not risky?]: Firefox 55 uses Rust only for MP4 parser and bug 1324243, so only those components have a chance to crash if libc bindings have regressed in a backward-incompatible way.
[String changes made/needed]: None

Comment 8

[Jan Beich]

Comment on attachment 8882157 [details] [diff] [review]
Rebased for Firefox 55

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5e84f1e012f4

Comment 9

[Ralph Giles (:rillian)]

(In reply to Jan Beich from comment #7)

> [Has the fix been verified in Nightly?]: No, I don't have hardware.

The update has been validated on Nightly for Tier-1 platforms, it's just FreeBSD where we don't have nightly testing.

> [Why is the change risky/not risky?]: Firefox 55 uses Rust only for MP4 parser and bug 1324243, so only those components have a chance to crash if libc bindings have regressed in a backward-incompatible way.

Also, the MP4 parser doesn't use the libc crate. This is needed for servo components like webrender and stylo which are pref'd off for 55 release, and the geckodriver testing tool. So long as it builds, the patch is low risk because it doesn't affect shipping code.

Comment 10

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8882157 [details] [diff] [review]
Rebased for Firefox 55

From comments 7 and 9, sounds like this will be useful for webrender and stylo and isn't high risk. Thanks for rebasing for 55 - let's try this for beta 9.

Comment 11

[Julien Cristau [:jcristau]]

Eh, this is very much not about webrender and stylo on 55, it's about a tier3 platform.  Shouldn't hurt though...

Comment 12

[Carsten Book [:Tomcat]]

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

Comment 13

[Andrei Vaida [:avaida]]

(In reply to Jan Beich from comment #7)
> [Is this code covered by automated tests?]: Yes but I'm not sure exactly
> which ones.
> [Has the fix been verified in Nightly?]: No, I don't have hardware. Only
> FF54 backport was verified downstream:
> http://thunderx1.nyi.freebsd.org/data/110arm64-default/444857/logs/firefox-
> 54.0.1_1,1.log
> https://svnweb.freebsd.org/ports/head/www/firefox/files/patch-
> bug1376411?revision=444651&view=markup
> [Needs manual test from QE? If yes, steps to reproduce]: No

Setting qe-verify- based on Jan's assessment on manual testing needs.